[3/3] anv: Disable constant buffer 0 being relative.

Submitted by Rafael Antognolli on June 15, 2018, 8:12 p.m.

Details

Message ID 20180615201232.12664-3-rafael.antognolli@intel.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Rafael Antognolli June 15, 2018, 8:12 p.m.
If we are on gen8+ and have context isolation support, just make that
constant buffer address be absolute, so we can use it for push UBOs too.
---
 src/intel/vulkan/anv_device.c  |  5 ++++-
 src/intel/vulkan/anv_private.h |  1 +
 src/intel/vulkan/genX_state.c  | 27 +++++++++++++++++++++++++++
 3 files changed, 32 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
index d1637f097e8..002b05f15f8 100644
--- a/src/intel/vulkan/anv_device.c
+++ b/src/intel/vulkan/anv_device.c
@@ -430,7 +430,8 @@  anv_physical_device_init(struct anv_physical_device *device,
    device->compiler->shader_debug_log = compiler_debug_log;
    device->compiler->shader_perf_log = compiler_perf_log;
    device->compiler->supports_pull_constants = false;
-   device->compiler->constant_buffer_0_is_relative = true;
+   device->compiler->constant_buffer_0_is_relative =
+      device->info.gen < 8 || !device->has_context_isolation;
 
    isl_device_init(&device->isl_dev, &device->info, swizzled);
 
@@ -1519,6 +1520,8 @@  VkResult anv_CreateDevice(
    device->chipset_id = physical_device->chipset_id;
    device->no_hw = physical_device->no_hw;
    device->lost = false;
+   device->constant_buffer_0_is_relative =
+      physical_device->compiler->constant_buffer_0_is_relative;
 
    if (pAllocator)
       device->alloc = *pAllocator;
diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
index 6af2a0bd3f3..d7297da6f57 100644
--- a/src/intel/vulkan/anv_private.h
+++ b/src/intel/vulkan/anv_private.h
@@ -983,6 +983,7 @@  struct anv_device {
     pthread_mutex_t                             mutex;
     pthread_cond_t                              queue_submit;
     bool                                        lost;
+    bool                                        constant_buffer_0_is_relative;
 };
 
 static inline struct anv_state_pool *
diff --git a/src/intel/vulkan/genX_state.c b/src/intel/vulkan/genX_state.c
index c6e54046910..46e020c8134 100644
--- a/src/intel/vulkan/genX_state.c
+++ b/src/intel/vulkan/genX_state.c
@@ -169,6 +169,33 @@  genX(init_device_state)(struct anv_device *device)
    gen10_emit_wa_lri_to_cache_mode_zero(&batch);
 #endif
 
+   /* Set the "CONSTANT_BUFFER Address Offset Disable" bit, so
+    * 3DSTATE_CONSTANT_XS buffer 0 is an absolute address.
+    *
+    * This is only safe on kernels with context isolation support.
+    */
+   if (GEN_GEN >= 8 &&
+       !device->constant_buffer_0_is_relative) {
+      UNUSED uint32_t tmp_reg;
+#if GEN_GEN >= 9
+      anv_pack_struct(&tmp_reg, GENX(CS_DEBUG_MODE2),
+                      .CONSTANT_BUFFERAddressOffsetDisable = true,
+                      .CONSTANT_BUFFERAddressOffsetDisableMask = true);
+      anv_batch_emit(&batch, GENX(MI_LOAD_REGISTER_IMM), lri) {
+         lri.RegisterOffset = GENX(CS_DEBUG_MODE2_num);
+         lri.DataDWord      = tmp_reg;
+      }
+#elif GEN_GEN == 8
+      anv_pack_struct(&tmp_reg, GENX(INSTPM),
+                      .CONSTANT_BUFFERAddressOffsetDisable = true,
+                      .CONSTANT_BUFFERAddressOffsetDisableMask = true);
+      anv_batch_emit(&batch, GENX(MI_LOAD_REGISTER_IMM), lri) {
+         lri.RegisterOffset = GENX(INSTPM_num);
+         lri.DataDWord      = tmp_reg;
+      }
+#endif
+   }
+
    anv_batch_emit(&batch, GENX(MI_BATCH_BUFFER_END), bbe);
 
    assert(batch.next <= batch.end);

Comments

On Fri, Jun 15, 2018 at 1:12 PM, Rafael Antognolli <
rafael.antognolli@intel.com> wrote:

> If we are on gen8+ and have context isolation support, just make that
> constant buffer address be absolute, so we can use it for push UBOs too.
> ---
>  src/intel/vulkan/anv_device.c  |  5 ++++-
>  src/intel/vulkan/anv_private.h |  1 +
>  src/intel/vulkan/genX_state.c  | 27 +++++++++++++++++++++++++++
>  3 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
> index d1637f097e8..002b05f15f8 100644
> --- a/src/intel/vulkan/anv_device.c
> +++ b/src/intel/vulkan/anv_device.c
> @@ -430,7 +430,8 @@ anv_physical_device_init(struct anv_physical_device
> *device,
>     device->compiler->shader_debug_log = compiler_debug_log;
>     device->compiler->shader_perf_log = compiler_perf_log;
>     device->compiler->supports_pull_constants = false;
> -   device->compiler->constant_buffer_0_is_relative = true;
> +   device->compiler->constant_buffer_0_is_relative =
> +      device->info.gen < 8 || !device->has_context_isolation;
>
>     isl_device_init(&device->isl_dev, &device->info, swizzled);
>
> @@ -1519,6 +1520,8 @@ VkResult anv_CreateDevice(
>     device->chipset_id = physical_device->chipset_id;
>     device->no_hw = physical_device->no_hw;
>     device->lost = false;
> +   device->constant_buffer_0_is_relative =
> +      physical_device->compiler->constant_buffer_0_is_relative;
>
>     if (pAllocator)
>        device->alloc = *pAllocator;
> diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_
> private.h
> index 6af2a0bd3f3..d7297da6f57 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -983,6 +983,7 @@ struct anv_device {
>      pthread_mutex_t                             mutex;
>      pthread_cond_t                              queue_submit;
>      bool                                        lost;
> +    bool
> constant_buffer_0_is_relative;
>

I don't think we need a device boolean for this.  Let's just dig it out of
the compiler or the physical device.  I think the physical device probably
makes a bit more sense.  Other than that, the series looks good to me.

--Jason


>  };
>
>  static inline struct anv_state_pool *
> diff --git a/src/intel/vulkan/genX_state.c b/src/intel/vulkan/genX_state.c
> index c6e54046910..46e020c8134 100644
> --- a/src/intel/vulkan/genX_state.c
> +++ b/src/intel/vulkan/genX_state.c
> @@ -169,6 +169,33 @@ genX(init_device_state)(struct anv_device *device)
>     gen10_emit_wa_lri_to_cache_mode_zero(&batch);
>  #endif
>
> +   /* Set the "CONSTANT_BUFFER Address Offset Disable" bit, so
> +    * 3DSTATE_CONSTANT_XS buffer 0 is an absolute address.
> +    *
> +    * This is only safe on kernels with context isolation support.
> +    */
> +   if (GEN_GEN >= 8 &&
> +       !device->constant_buffer_0_is_relative) {
> +      UNUSED uint32_t tmp_reg;
> +#if GEN_GEN >= 9
> +      anv_pack_struct(&tmp_reg, GENX(CS_DEBUG_MODE2),
> +                      .CONSTANT_BUFFERAddressOffsetDisable = true,
> +                      .CONSTANT_BUFFERAddressOffsetDisableMask = true);
> +      anv_batch_emit(&batch, GENX(MI_LOAD_REGISTER_IMM), lri) {
> +         lri.RegisterOffset = GENX(CS_DEBUG_MODE2_num);
> +         lri.DataDWord      = tmp_reg;
> +      }
> +#elif GEN_GEN == 8
> +      anv_pack_struct(&tmp_reg, GENX(INSTPM),
> +                      .CONSTANT_BUFFERAddressOffsetDisable = true,
> +                      .CONSTANT_BUFFERAddressOffsetDisableMask = true);
> +      anv_batch_emit(&batch, GENX(MI_LOAD_REGISTER_IMM), lri) {
> +         lri.RegisterOffset = GENX(INSTPM_num);
> +         lri.DataDWord      = tmp_reg;
> +      }
> +#endif
> +   }
> +
>     anv_batch_emit(&batch, GENX(MI_BATCH_BUFFER_END), bbe);
>
>     assert(batch.next <= batch.end);
> --
> 2.14.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
On Fri, Jun 15, 2018 at 01:21:17PM -0700, Jason Ekstrand wrote:
> On Fri, Jun 15, 2018 at 1:12 PM, Rafael Antognolli <rafael.antognolli@intel.com
> > wrote:
> 
>     If we are on gen8+ and have context isolation support, just make that
>     constant buffer address be absolute, so we can use it for push UBOs too.
>     ---
>      src/intel/vulkan/anv_device.c  |  5 ++++-
>      src/intel/vulkan/anv_private.h |  1 +
>      src/intel/vulkan/genX_state.c  | 27 +++++++++++++++++++++++++++
>      3 files changed, 32 insertions(+), 1 deletion(-)
> 
>     diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
>     index d1637f097e8..002b05f15f8 100644
>     --- a/src/intel/vulkan/anv_device.c
>     +++ b/src/intel/vulkan/anv_device.c
>     @@ -430,7 +430,8 @@ anv_physical_device_init(struct anv_physical_device
>     *device,
>         device->compiler->shader_debug_log = compiler_debug_log;
>         device->compiler->shader_perf_log = compiler_perf_log;
>         device->compiler->supports_pull_constants = false;
>     -   device->compiler->constant_buffer_0_is_relative = true;
>     +   device->compiler->constant_buffer_0_is_relative =
>     +      device->info.gen < 8 || !device->has_context_isolation;
> 
>         isl_device_init(&device->isl_dev, &device->info, swizzled);
> 
>     @@ -1519,6 +1520,8 @@ VkResult anv_CreateDevice(
>         device->chipset_id = physical_device->chipset_id;
>         device->no_hw = physical_device->no_hw;
>         device->lost = false;
>     +   device->constant_buffer_0_is_relative =
>     +      physical_device->compiler->constant_buffer_0_is_relative;
> 
>         if (pAllocator)
>            device->alloc = *pAllocator;
>     diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_
>     private.h
>     index 6af2a0bd3f3..d7297da6f57 100644
>     --- a/src/intel/vulkan/anv_private.h
>     +++ b/src/intel/vulkan/anv_private.h
>     @@ -983,6 +983,7 @@ struct anv_device {
>          pthread_mutex_t                             mutex;
>          pthread_cond_t                              queue_submit;
>          bool                                        lost;
>     +    bool                                       
>     constant_buffer_0_is_relative;
> 
> 
> I don't think we need a device boolean for this.  Let's just dig it out of the
> compiler or the physical device.  I think the physical device probably makes a
> bit more sense.  Other than that, the series looks good to me.

Do you mean store constant_buffer_0_is_relative into the physical
device? Or just check for physical_device->context_isolation?

>      };
> 
>      static inline struct anv_state_pool *
>     diff --git a/src/intel/vulkan/genX_state.c b/src/intel/vulkan/genX_state.c
>     index c6e54046910..46e020c8134 100644
>     --- a/src/intel/vulkan/genX_state.c
>     +++ b/src/intel/vulkan/genX_state.c
>     @@ -169,6 +169,33 @@ genX(init_device_state)(struct anv_device *device)
>         gen10_emit_wa_lri_to_cache_mode_zero(&batch);
>      #endif
> 
>     +   /* Set the "CONSTANT_BUFFER Address Offset Disable" bit, so
>     +    * 3DSTATE_CONSTANT_XS buffer 0 is an absolute address.
>     +    *
>     +    * This is only safe on kernels with context isolation support.
>     +    */
>     +   if (GEN_GEN >= 8 &&
>     +       !device->constant_buffer_0_is_relative) {
>     +      UNUSED uint32_t tmp_reg;
>     +#if GEN_GEN >= 9
>     +      anv_pack_struct(&tmp_reg, GENX(CS_DEBUG_MODE2),
>     +                      .CONSTANT_BUFFERAddressOffsetDisable = true,
>     +                      .CONSTANT_BUFFERAddressOffsetDisableMask = true);
>     +      anv_batch_emit(&batch, GENX(MI_LOAD_REGISTER_IMM), lri) {
>     +         lri.RegisterOffset = GENX(CS_DEBUG_MODE2_num);
>     +         lri.DataDWord      = tmp_reg;
>     +      }
>     +#elif GEN_GEN == 8
>     +      anv_pack_struct(&tmp_reg, GENX(INSTPM),
>     +                      .CONSTANT_BUFFERAddressOffsetDisable = true,
>     +                      .CONSTANT_BUFFERAddressOffsetDisableMask = true);
>     +      anv_batch_emit(&batch, GENX(MI_LOAD_REGISTER_IMM), lri) {
>     +         lri.RegisterOffset = GENX(INSTPM_num);
>     +         lri.DataDWord      = tmp_reg;
>     +      }
>     +#endif
>     +   }
>     +
>         anv_batch_emit(&batch, GENX(MI_BATCH_BUFFER_END), bbe);
> 
>         assert(batch.next <= batch.end);
>     --
>     2.14.3
> 
>     _______________________________________________
>     mesa-dev mailing list
>     mesa-dev@lists.freedesktop.org
>     https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
>
On Fri, Jun 15, 2018 at 1:31 PM, Rafael Antognolli <
rafael.antognolli@intel.com> wrote:

> On Fri, Jun 15, 2018 at 01:21:17PM -0700, Jason Ekstrand wrote:
> > On Fri, Jun 15, 2018 at 1:12 PM, Rafael Antognolli <
> rafael.antognolli@intel.com
> > > wrote:
> >
> >     If we are on gen8+ and have context isolation support, just make that
> >     constant buffer address be absolute, so we can use it for push UBOs
> too.
> >     ---
> >      src/intel/vulkan/anv_device.c  |  5 ++++-
> >      src/intel/vulkan/anv_private.h |  1 +
> >      src/intel/vulkan/genX_state.c  | 27 +++++++++++++++++++++++++++
> >      3 files changed, 32 insertions(+), 1 deletion(-)
> >
> >     diff --git a/src/intel/vulkan/anv_device.c
> b/src/intel/vulkan/anv_device.c
> >     index d1637f097e8..002b05f15f8 100644
> >     --- a/src/intel/vulkan/anv_device.c
> >     +++ b/src/intel/vulkan/anv_device.c
> >     @@ -430,7 +430,8 @@ anv_physical_device_init(struct
> anv_physical_device
> >     *device,
> >         device->compiler->shader_debug_log = compiler_debug_log;
> >         device->compiler->shader_perf_log = compiler_perf_log;
> >         device->compiler->supports_pull_constants = false;
> >     -   device->compiler->constant_buffer_0_is_relative = true;
> >     +   device->compiler->constant_buffer_0_is_relative =
> >     +      device->info.gen < 8 || !device->has_context_isolation;
> >
> >         isl_device_init(&device->isl_dev, &device->info, swizzled);
> >
> >     @@ -1519,6 +1520,8 @@ VkResult anv_CreateDevice(
> >         device->chipset_id = physical_device->chipset_id;
> >         device->no_hw = physical_device->no_hw;
> >         device->lost = false;
> >     +   device->constant_buffer_0_is_relative =
> >     +      physical_device->compiler->constant_buffer_0_is_relative;
> >
> >         if (pAllocator)
> >            device->alloc = *pAllocator;
> >     diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_
> >     private.h
> >     index 6af2a0bd3f3..d7297da6f57 100644
> >     --- a/src/intel/vulkan/anv_private.h
> >     +++ b/src/intel/vulkan/anv_private.h
> >     @@ -983,6 +983,7 @@ struct anv_device {
> >          pthread_mutex_t                             mutex;
> >          pthread_cond_t                              queue_submit;
> >          bool                                        lost;
> >     +    bool
> >     constant_buffer_0_is_relative;
> >
> >
> > I don't think we need a device boolean for this.  Let's just dig it out
> of the
> > compiler or the physical device.  I think the physical device probably
> makes a
> > bit more sense.  Other than that, the series looks good to me.
>
> Do you mean store constant_buffer_0_is_relative into the physical
> device? Or just check for physical_device->context_isolation?
>

Just check physical_device->context_isolation.  We don't need three
booleans to get out of sync.

--Jason



> >      };
> >
> >      static inline struct anv_state_pool *
> >     diff --git a/src/intel/vulkan/genX_state.c
> b/src/intel/vulkan/genX_state.c
> >     index c6e54046910..46e020c8134 100644
> >     --- a/src/intel/vulkan/genX_state.c
> >     +++ b/src/intel/vulkan/genX_state.c
> >     @@ -169,6 +169,33 @@ genX(init_device_state)(struct anv_device
> *device)
> >         gen10_emit_wa_lri_to_cache_mode_zero(&batch);
> >      #endif
> >
> >     +   /* Set the "CONSTANT_BUFFER Address Offset Disable" bit, so
> >     +    * 3DSTATE_CONSTANT_XS buffer 0 is an absolute address.
> >     +    *
> >     +    * This is only safe on kernels with context isolation support.
> >     +    */
> >     +   if (GEN_GEN >= 8 &&
> >     +       !device->constant_buffer_0_is_relative) {
> >     +      UNUSED uint32_t tmp_reg;
> >     +#if GEN_GEN >= 9
> >     +      anv_pack_struct(&tmp_reg, GENX(CS_DEBUG_MODE2),
> >     +                      .CONSTANT_BUFFERAddressOffsetDisable = true,
> >     +                      .CONSTANT_BUFFERAddressOffsetDisableMask =
> true);
> >     +      anv_batch_emit(&batch, GENX(MI_LOAD_REGISTER_IMM), lri) {
> >     +         lri.RegisterOffset = GENX(CS_DEBUG_MODE2_num);
> >     +         lri.DataDWord      = tmp_reg;
> >     +      }
> >     +#elif GEN_GEN == 8
> >     +      anv_pack_struct(&tmp_reg, GENX(INSTPM),
> >     +                      .CONSTANT_BUFFERAddressOffsetDisable = true,
> >     +                      .CONSTANT_BUFFERAddressOffsetDisableMask =
> true);
> >     +      anv_batch_emit(&batch, GENX(MI_LOAD_REGISTER_IMM), lri) {
> >     +         lri.RegisterOffset = GENX(INSTPM_num);
> >     +         lri.DataDWord      = tmp_reg;
> >     +      }
> >     +#endif
> >     +   }
> >     +
> >         anv_batch_emit(&batch, GENX(MI_BATCH_BUFFER_END), bbe);
> >
> >         assert(batch.next <= batch.end);
> >     --
> >     2.14.3
> >
> >     _______________________________________________
> >     mesa-dev mailing list
> >     mesa-dev@lists.freedesktop.org
> >     https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> >
>