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

2023-04-07 Thread Matt Roper
On Fri, Apr 07, 2023 at 12:32:37PM +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
> 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
> Reviewed-by: Haridhar Kalvala 

Applied to drm-intel-gt-next.  Thanks for the patch and review.


Matt


> ---
>  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 492b3de6678d7..fd1f9cd35e9d7 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1145,6 +1145,7 @@
>  #define   SC_DISABLE_POWER_OPTIMIZATION_EBB  REG_BIT(9)
>  #define   GEN11_SAMPLER_ENABLE_HEADLESS_MSG  REG_BIT(5)
>  #define   MTL_DISABLE_SAMPLER_SC_OOO REG_BIT(3)
> +#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 6ea453ddd0116..b925ef47304b6 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -2971,6 +2971,25 @@ general_render_compute_wa_init(struct intel_engine_cs 
> *engine, struct i915_wa_li
>  
>   add_render_compute_tuning_settings(i915, wal);
>  
> + 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 (IS_MTL_GRAPHICS_STEP(i915, M, STEP_B0, STEP_FOREVER) ||
>   IS_MTL_GRAPHICS_STEP(i915, P, STEP_B0, STEP_FOREVER))
>   /* Wa_14017856879 */
> -- 
> 2.34.1
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation


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

2023-04-07 Thread Lionel Landwerlin

On 07/04/2023 12:32, 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
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
Reviewed-by: Haridhar Kalvala 



Screwed up the subject-prefix, but this is v4.

Rebased due to another change touching the same register.


-Lionel



---
  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 492b3de6678d7..fd1f9cd35e9d7 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1145,6 +1145,7 @@
  #define   SC_DISABLE_POWER_OPTIMIZATION_EBB   REG_BIT(9)
  #define   GEN11_SAMPLER_ENABLE_HEADLESS_MSG   REG_BIT(5)
  #define   MTL_DISABLE_SAMPLER_SC_OOO  REG_BIT(3)
+#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 6ea453ddd0116..b925ef47304b6 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -2971,6 +2971,25 @@ general_render_compute_wa_init(struct intel_engine_cs 
*engine, struct i915_wa_li
  
  	add_render_compute_tuning_settings(i915, wal);
  
+	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 (IS_MTL_GRAPHICS_STEP(i915, M, STEP_B0, STEP_FOREVER) ||
IS_MTL_GRAPHICS_STEP(i915, P, STEP_B0, STEP_FOREVER))
/* Wa_14017856879 */





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

2023-04-03 Thread Lionel Landwerlin

On 03/04/2023 21:22, Kalvala, Haridhar wrote:


On 3/31/2023 12:35 PM, Kalvala, Haridhar wrote:


On 3/30/2023 10:49 PM, Lionel Landwerlin wrote:

On 29/03/2023 01:49, Matt Atwood wrote:

On Tue, Mar 28, 2023 at 04:14:33PM +0530, Kalvala, Haridhar wrote:

On 3/9/2023 8:56 PM, 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
only have to have one copy of the border colors). This is 
reproducing

the behavior of the Windows drivers.


Bspec:46052



Sorry, missed your answer.


Should I just add the Bspec number to the commit message ?


Thanks,


-Lionel



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 | 17 
+

   2 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h 
b/drivers/gpu/drm/i915/gt/intel_gt_regs.h

index 08d76aa06974c..1aaa471d08c56 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1141,6 +1141,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 32aa1647721ae..734b64e714647 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -2542,6 +2542,23 @@ rcs_engine_wa_init(struct intel_engine_cs 
*engine, struct i915_wa_list *wal)

    ENABLE_SMALLPL);
   }
+    if (GRAPHICS_VER(i915) >= 11) {

Hi Lionel,

Not sure should this implementation be part of 
"rcs_engine_wa_init" or

"general_render_compute_wa_init" ?



I checked with Matt Ropper as well, looks like this implementation 
should be part of "general_render_compute_wa_init".



I did send a v3 of the patch last Thursday to address this.

Let me know if that's good.


Thanks,


-Lionel







+    /* 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)
+ */
+    wa_mcr_masked_en(wal,
+ GEN10_SAMPLER_MODE,
  since we checking the condition for GEN11 or above, can this 
register be

defined as GEN11_SAMPLER_MODE
We use the name of the first time the register was introduced, gen 
10 is

fine here.

ok

+ 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,

--
Regards,
Haridhar Kalvala


Regards,
MattA







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

2023-04-03 Thread Kalvala, Haridhar



On 3/31/2023 12:35 PM, Kalvala, Haridhar wrote:


On 3/30/2023 10:49 PM, Lionel Landwerlin wrote:

On 29/03/2023 01:49, Matt Atwood wrote:

On Tue, Mar 28, 2023 at 04:14:33PM +0530, Kalvala, Haridhar wrote:

On 3/9/2023 8:56 PM, 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
only have to have one copy of the border colors). This is reproducing
the behavior of the Windows drivers.


Bspec:46052



Sorry, missed your answer.


Should I just add the Bspec number to the commit message ?


Thanks,


-Lionel



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 | 17 +
   2 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h 
b/drivers/gpu/drm/i915/gt/intel_gt_regs.h

index 08d76aa06974c..1aaa471d08c56 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1141,6 +1141,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 32aa1647721ae..734b64e714647 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -2542,6 +2542,23 @@ rcs_engine_wa_init(struct intel_engine_cs 
*engine, struct i915_wa_list *wal)

    ENABLE_SMALLPL);
   }
+    if (GRAPHICS_VER(i915) >= 11) {

Hi Lionel,

Not sure should this implementation be part of "rcs_engine_wa_init" or
"general_render_compute_wa_init" ?



I checked with Matt Ropper as well, looks like this implementation 
should be part of "general_render_compute_wa_init".





+    /* 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)
+ */
+    wa_mcr_masked_en(wal,
+ GEN10_SAMPLER_MODE,
  since we checking the condition for GEN11 or above, can this 
register be

defined as GEN11_SAMPLER_MODE
We use the name of the first time the register was introduced, gen 
10 is

fine here.

ok

+ 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,

--
Regards,
Haridhar Kalvala


Regards,
MattA




--
Regards,
Haridhar Kalvala



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

2023-03-31 Thread Kalvala, Haridhar



On 3/30/2023 10:49 PM, Lionel Landwerlin wrote:

On 29/03/2023 01:49, Matt Atwood wrote:

On Tue, Mar 28, 2023 at 04:14:33PM +0530, Kalvala, Haridhar wrote:

On 3/9/2023 8:56 PM, 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
only have to have one copy of the border colors). This is reproducing
the behavior of the Windows drivers.


Bspec:46052



Sorry, missed your answer.


Should I just add the Bspec number to the commit message ?


Thanks,


-Lionel



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 | 17 +
   2 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h 
b/drivers/gpu/drm/i915/gt/intel_gt_regs.h

index 08d76aa06974c..1aaa471d08c56 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1141,6 +1141,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 32aa1647721ae..734b64e714647 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -2542,6 +2542,23 @@ rcs_engine_wa_init(struct intel_engine_cs 
*engine, struct i915_wa_list *wal)

    ENABLE_SMALLPL);
   }
+    if (GRAPHICS_VER(i915) >= 11) {

Hi Lionel,

Not sure should this implementation be part of "rcs_engine_wa_init" or
"general_render_compute_wa_init" ?



+    /* 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)
+ */
+    wa_mcr_masked_en(wal,
+ GEN10_SAMPLER_MODE,
  since we checking the condition for GEN11 or above, can this 
register be

defined as GEN11_SAMPLER_MODE

We use the name of the first time the register was introduced, gen 10 is
fine here.

ok

+ 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,

--
Regards,
Haridhar Kalvala


Regards,
MattA




--
Regards,
Haridhar Kalvala



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

2023-03-30 Thread Lionel Landwerlin

On 29/03/2023 01:49, Matt Atwood wrote:

On Tue, Mar 28, 2023 at 04:14:33PM +0530, Kalvala, Haridhar wrote:

On 3/9/2023 8:56 PM, 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
only have to have one copy of the border colors). This is reproducing
the behavior of the Windows drivers.


Bspec:46052



Sorry, missed your answer.


Should I just add the Bspec number to the commit message ?


Thanks,


-Lionel



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 | 17 +
   2 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h 
b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index 08d76aa06974c..1aaa471d08c56 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1141,6 +1141,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_SSLAREG_BIT(15)
diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c 
b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 32aa1647721ae..734b64e714647 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -2542,6 +2542,23 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, 
struct i915_wa_list *wal)
 ENABLE_SMALLPL);
}
+   if (GRAPHICS_VER(i915) >= 11) {

Hi Lionel,

Not sure should this implementation be part of "rcs_engine_wa_init" or
"general_render_compute_wa_init".


+   /* 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)
+*/
+   wa_mcr_masked_en(wal,
+GEN10_SAMPLER_MODE,

  since we checking the condition for GEN11 or above, can this register be
defined as GEN11_SAMPLER_MODE

We use the name of the first time the register was introduced, gen 10 is
fine here.

+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,

--
Regards,
Haridhar Kalvala


Regards,
MattA





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

2023-03-28 Thread Matt Atwood
On Tue, Mar 28, 2023 at 04:14:33PM +0530, Kalvala, Haridhar wrote:
> 
> On 3/9/2023 8:56 PM, 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
> > 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 | 17 +
> >   2 files changed, 18 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h 
> > b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > index 08d76aa06974c..1aaa471d08c56 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > @@ -1141,6 +1141,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 32aa1647721ae..734b64e714647 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > @@ -2542,6 +2542,23 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, 
> > struct i915_wa_list *wal)
> >  ENABLE_SMALLPL);
> > }
> > +   if (GRAPHICS_VER(i915) >= 11) {
> 
> Hi Lionel,
> 
> Not sure should this implementation be part of "rcs_engine_wa_init" or
> "general_render_compute_wa_init".
> 
> > +   /* 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)
> > +*/
> > +   wa_mcr_masked_en(wal,
> > +GEN10_SAMPLER_MODE,
> 
>  since we checking the condition for GEN11 or above, can this register be
> defined as GEN11_SAMPLER_MODE
We use the name of the first time the register was introduced, gen 10 is
fine here.
> > +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,
> 
> -- 
> Regards,
> Haridhar Kalvala
>
Regards,
MattA


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

2023-03-28 Thread Kalvala, Haridhar



On 3/9/2023 8:56 PM, 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
only have to have one copy of the border colors). This is reproducing
the behavior of the Windows drivers.

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 | 17 +
  2 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h 
b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
index 08d76aa06974c..1aaa471d08c56 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
@@ -1141,6 +1141,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 32aa1647721ae..734b64e714647 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -2542,6 +2542,23 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, 
struct i915_wa_list *wal)
 ENABLE_SMALLPL);
}
  
+	if (GRAPHICS_VER(i915) >= 11) {


Hi Lionel,

Not sure should this implementation be part of "rcs_engine_wa_init" or 
"general_render_compute_wa_init".



+   /* 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)
+*/
+   wa_mcr_masked_en(wal,
+GEN10_SAMPLER_MODE,


 since we checking the condition for GEN11 or above, can this register 
be defined as GEN11_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,


--
Regards,
Haridhar Kalvala