Re: [Intel-gfx] [v2] drm/i915: disable sampler indirect state in bindless heap

2023-03-30 Thread Lionel Landwerlin

On 30/03/2023 22:38, Matt Atwood wrote:

On Thu, Mar 30, 2023 at 12:27:33PM -0700, Matt Atwood wrote:

On Thu, Mar 30, 2023 at 08:47:40PM +0300, Lionel Landwerlin wrote:

By default the indirect state sampler data (border colors) are stored
in the same heap as the SAMPLER_STATE structure. For userspace drivers
that can be 2 different heaps (dynamic state heap & bindless sampler
state heap). This means that border colors have to copied in 2
different places so that the same SAMPLER_STATE structure find the
right data.

This change is forcing the indirect state sampler data to only be in
the dynamic state pool (more convinient for userspace drivers, they

   convenient

only have to have one copy of the border colors). This is reproducing
the behavior of the Windows drivers.

BSpec: 46052


Assuming still good CI results..
Reviewed-by: Matt Atwood 

My mistake version 3 required. comments inline.

Signed-off-by: Lionel Landwerlin 
Cc: sta...@vger.kernel.org
---
  drivers/gpu/drm/i915/gt/intel_gt_regs.h |  1 +
  drivers/gpu/drm/i915/gt/intel_workarounds.c | 19 +++
  2 files changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h 
b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index 4aecb5a7b6318..f298dc461a72f 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1144,6 +1144,7 @@
  #define   ENABLE_SMALLPL  REG_BIT(15)
  #define   SC_DISABLE_POWER_OPTIMIZATION_EBB   REG_BIT(9)
  #define   GEN11_SAMPLER_ENABLE_HEADLESS_MSG   REG_BIT(5)
+#define   GEN11_INDIRECT_STATE_BASE_ADDR_OVERRIDE  REG_BIT(0)
  
  #define GEN9_HALF_SLICE_CHICKEN7		MCR_REG(0xe194)

  #define   DG2_DISABLE_ROUND_ENABLE_ALLOW_FOR_SSLA REG_BIT(15)
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c 
b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index e7ee24bcad893..0ce1c8c23c631 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -2535,6 +2535,25 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, 
struct i915_wa_list *wal)
 ENABLE_SMALLPL);
}
  

This workaround belongs in general render workarounds not rcs, as per
the address space in i915_regs.h 0x2xxx.

#define RENDER_RING_BASE0x02000



Thanks makes sense.


-Lionel






+   if (GRAPHICS_VER(i915) >= 11) {
+   /* This is not a Wa (although referred to as
+* WaSetInidrectStateOverride in places), this allows
+* applications that reference sampler states through
+* the BindlessSamplerStateBaseAddress to have their
+* border color relative to DynamicStateBaseAddress
+* rather than BindlessSamplerStateBaseAddress.
+*
+* Otherwise SAMPLER_STATE border colors have to be
+* copied in multiple heaps (DynamicStateBaseAddress &
+* BindlessSamplerStateBaseAddress)
+*
+* BSpec: 46052
+*/
+   wa_mcr_masked_en(wal,
+GEN10_SAMPLER_MODE,
+GEN11_INDIRECT_STATE_BASE_ADDR_OVERRIDE);
+   }
+
if (GRAPHICS_VER(i915) == 11) {
/* This is not an Wa. Enable for better image quality */
wa_masked_en(wal,
--
2.34.1


MattA





Re: [Intel-gfx] [v2] drm/i915: disable sampler indirect state in bindless heap

2023-03-30 Thread Matt Atwood
On Thu, Mar 30, 2023 at 12:27:33PM -0700, Matt Atwood wrote:
> On Thu, Mar 30, 2023 at 08:47:40PM +0300, Lionel Landwerlin wrote:
> > By default the indirect state sampler data (border colors) are stored
> > in the same heap as the SAMPLER_STATE structure. For userspace drivers
> > that can be 2 different heaps (dynamic state heap & bindless sampler
> > state heap). This means that border colors have to copied in 2
> > different places so that the same SAMPLER_STATE structure find the
> > right data.
> > 
> > This change is forcing the indirect state sampler data to only be in
> > the dynamic state pool (more convinient for userspace drivers, they
>  convenient 
> > only have to have one copy of the border colors). This is reproducing
> > the behavior of the Windows drivers.
> > 
> > BSpec: 46052
> > 
> Assuming still good CI results..
> Reviewed-by: Matt Atwood 
My mistake version 3 required. comments inline.
> > Signed-off-by: Lionel Landwerlin 
> > Cc: sta...@vger.kernel.org
> > ---
> >  drivers/gpu/drm/i915/gt/intel_gt_regs.h |  1 +
> >  drivers/gpu/drm/i915/gt/intel_workarounds.c | 19 +++
> >  2 files changed, 20 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h 
> > b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > index 4aecb5a7b6318..f298dc461a72f 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > @@ -1144,6 +1144,7 @@
> >  #define   ENABLE_SMALLPL   REG_BIT(15)
> >  #define   SC_DISABLE_POWER_OPTIMIZATION_EBBREG_BIT(9)
> >  #define   GEN11_SAMPLER_ENABLE_HEADLESS_MSGREG_BIT(5)
> > +#define   GEN11_INDIRECT_STATE_BASE_ADDR_OVERRIDE  REG_BIT(0)
> >  
> >  #define GEN9_HALF_SLICE_CHICKEN7   MCR_REG(0xe194)
> >  #define   DG2_DISABLE_ROUND_ENABLE_ALLOW_FOR_SSLA  REG_BIT(15)
> > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c 
> > b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > index e7ee24bcad893..0ce1c8c23c631 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > @@ -2535,6 +2535,25 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, 
> > struct i915_wa_list *wal)
> >  ENABLE_SMALLPL);
> > }
> >  
This workaround belongs in general render workarounds not rcs, as per
the address space in i915_regs.h 0x2xxx.

#define RENDER_RING_BASE0x02000


> > +   if (GRAPHICS_VER(i915) >= 11) {
> > +   /* This is not a Wa (although referred to as
> > +* WaSetInidrectStateOverride in places), this allows
> > +* applications that reference sampler states through
> > +* the BindlessSamplerStateBaseAddress to have their
> > +* border color relative to DynamicStateBaseAddress
> > +* rather than BindlessSamplerStateBaseAddress.
> > +*
> > +* Otherwise SAMPLER_STATE border colors have to be
> > +* copied in multiple heaps (DynamicStateBaseAddress &
> > +* BindlessSamplerStateBaseAddress)
> > +*
> > +* BSpec: 46052
> > +*/
> > +   wa_mcr_masked_en(wal,
> > +GEN10_SAMPLER_MODE,
> > +GEN11_INDIRECT_STATE_BASE_ADDR_OVERRIDE);
> > +   }
> > +
> > if (GRAPHICS_VER(i915) == 11) {
> > /* This is not an Wa. Enable for better image quality */
> > wa_masked_en(wal,
> > -- 
> > 2.34.1
> > 
MattA


Re: [Intel-gfx] [v2] drm/i915: disable sampler indirect state in bindless heap

2023-03-30 Thread Matt Atwood
On Thu, Mar 30, 2023 at 08:47:40PM +0300, Lionel Landwerlin wrote:
> By default the indirect state sampler data (border colors) are stored
> in the same heap as the SAMPLER_STATE structure. For userspace drivers
> that can be 2 different heaps (dynamic state heap & bindless sampler
> state heap). This means that border colors have to copied in 2
> different places so that the same SAMPLER_STATE structure find the
> right data.
> 
> This change is forcing the indirect state sampler data to only be in
> the dynamic state pool (more convinient for userspace drivers, they
   convenient 
> only have to have one copy of the border colors). This is reproducing
> the behavior of the Windows drivers.
> 
> BSpec: 46052
> 
Assuming still good CI results..
Reviewed-by: Matt Atwood 
> Signed-off-by: Lionel Landwerlin 
> Cc: sta...@vger.kernel.org
> ---
>  drivers/gpu/drm/i915/gt/intel_gt_regs.h |  1 +
>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 19 +++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h 
> b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index 4aecb5a7b6318..f298dc461a72f 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1144,6 +1144,7 @@
>  #define   ENABLE_SMALLPL REG_BIT(15)
>  #define   SC_DISABLE_POWER_OPTIMIZATION_EBB  REG_BIT(9)
>  #define   GEN11_SAMPLER_ENABLE_HEADLESS_MSG  REG_BIT(5)
> +#define   GEN11_INDIRECT_STATE_BASE_ADDR_OVERRIDEREG_BIT(0)
>  
>  #define GEN9_HALF_SLICE_CHICKEN7 MCR_REG(0xe194)
>  #define   DG2_DISABLE_ROUND_ENABLE_ALLOW_FOR_SSLAREG_BIT(15)
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c 
> b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index e7ee24bcad893..0ce1c8c23c631 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -2535,6 +2535,25 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, 
> struct i915_wa_list *wal)
>ENABLE_SMALLPL);
>   }
>  
> + if (GRAPHICS_VER(i915) >= 11) {
> + /* This is not a Wa (although referred to as
> +  * WaSetInidrectStateOverride in places), this allows
> +  * applications that reference sampler states through
> +  * the BindlessSamplerStateBaseAddress to have their
> +  * border color relative to DynamicStateBaseAddress
> +  * rather than BindlessSamplerStateBaseAddress.
> +  *
> +  * Otherwise SAMPLER_STATE border colors have to be
> +  * copied in multiple heaps (DynamicStateBaseAddress &
> +  * BindlessSamplerStateBaseAddress)
> +  *
> +  * BSpec: 46052
> +  */
> + wa_mcr_masked_en(wal,
> +  GEN10_SAMPLER_MODE,
> +  GEN11_INDIRECT_STATE_BASE_ADDR_OVERRIDE);
> + }
> +
>   if (GRAPHICS_VER(i915) == 11) {
>   /* This is not an Wa. Enable for better image quality */
>   wa_masked_en(wal,
> -- 
> 2.34.1
> 


[Intel-gfx] [v2] drm/i915: disable sampler indirect state in bindless heap

2023-03-30 Thread Lionel Landwerlin
By default the indirect state sampler data (border colors) are stored
in the same heap as the SAMPLER_STATE structure. For userspace drivers
that can be 2 different heaps (dynamic state heap & bindless sampler
state heap). This means that border colors have to copied in 2
different places so that the same SAMPLER_STATE structure find the
right data.

This change is forcing the indirect state sampler data to only be in
the dynamic state pool (more convinient for userspace drivers, they
only have to have one copy of the border colors). This is reproducing
the behavior of the Windows drivers.

BSpec: 46052

Signed-off-by: Lionel Landwerlin 
Cc: sta...@vger.kernel.org
---
 drivers/gpu/drm/i915/gt/intel_gt_regs.h |  1 +
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 19 +++
 2 files changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h 
b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index 4aecb5a7b6318..f298dc461a72f 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1144,6 +1144,7 @@
 #define   ENABLE_SMALLPL   REG_BIT(15)
 #define   SC_DISABLE_POWER_OPTIMIZATION_EBBREG_BIT(9)
 #define   GEN11_SAMPLER_ENABLE_HEADLESS_MSGREG_BIT(5)
+#define   GEN11_INDIRECT_STATE_BASE_ADDR_OVERRIDE  REG_BIT(0)
 
 #define GEN9_HALF_SLICE_CHICKEN7   MCR_REG(0xe194)
 #define   DG2_DISABLE_ROUND_ENABLE_ALLOW_FOR_SSLA  REG_BIT(15)
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c 
b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index e7ee24bcad893..0ce1c8c23c631 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -2535,6 +2535,25 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, 
struct i915_wa_list *wal)
 ENABLE_SMALLPL);
}
 
+   if (GRAPHICS_VER(i915) >= 11) {
+   /* This is not a Wa (although referred to as
+* WaSetInidrectStateOverride in places), this allows
+* applications that reference sampler states through
+* the BindlessSamplerStateBaseAddress to have their
+* border color relative to DynamicStateBaseAddress
+* rather than BindlessSamplerStateBaseAddress.
+*
+* Otherwise SAMPLER_STATE border colors have to be
+* copied in multiple heaps (DynamicStateBaseAddress &
+* BindlessSamplerStateBaseAddress)
+*
+* BSpec: 46052
+*/
+   wa_mcr_masked_en(wal,
+GEN10_SAMPLER_MODE,
+GEN11_INDIRECT_STATE_BASE_ADDR_OVERRIDE);
+   }
+
if (GRAPHICS_VER(i915) == 11) {
/* This is not an Wa. Enable for better image quality */
wa_masked_en(wal,
-- 
2.34.1