diff mbox series

drm/i915: Enable workaround for pixel shader dispatch hang

Message ID 20190415144529.8949-1-mika.kuoppala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Enable workaround for pixel shader dispatch hang | expand

Commit Message

Mika Kuoppala April 15, 2019, 2:45 p.m. UTC
Set chicken bits to workaround a possible pixel shader
dispatch hang.

v2: no need to filter out preprod skl (Ville, Chris)
v3: formatting

Bspec: 14091, ID#0651
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h          | 4 ++++
 drivers/gpu/drm/i915/intel_workarounds.c | 4 ++++
 2 files changed, 8 insertions(+)

Comments

Chris Wilson April 16, 2019, 8:24 a.m. UTC | #1
Quoting Mika Kuoppala (2019-04-15 15:45:29)
> Set chicken bits to workaround a possible pixel shader
> dispatch hang.
> 
> v2: no need to filter out preprod skl (Ville, Chris)
> v3: formatting
> 
> Bspec: 14091, ID#0651
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h          | 4 ++++
>  drivers/gpu/drm/i915/intel_workarounds.c | 4 ++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index c1c0f7ab03e9..499cc843443d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8902,11 +8902,15 @@ enum {
>  #define GEN7_ROW_CHICKEN2_GT2          _MMIO(0xf4f4)
>  #define   DOP_CLOCK_GATING_DISABLE     (1 << 0)
>  #define   PUSH_CONSTANT_DEREF_DISABLE  (1 << 8)
> +#define   GEN8_DISABLE_RR_ARBITRATION  (1 << 1)
>  #define   GEN11_TDL_CLOCK_GATING_FIX_DISABLE   (1 << 1)
>  
>  #define HSW_ROW_CHICKEN3               _MMIO(0xe49c)
>  #define  HSW_ROW_CHICKEN3_L3_GLOBAL_ATOMICS_DISABLE    (1 << 6)
>  
> +#define GEN8_ROW_CHICKEN4              _MMIO(0xe48c)
> +#define  GEN8_DISABLE_TDL_FIX          (1 << 3)
> +
>  #define HALF_SLICE_CHICKEN2            _MMIO(0xe180)
>  #define   GEN8_ST_PO_DISABLE           (1 << 13)
>  
> diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
> index ccaf63679435..b4709de49552 100644
> --- a/drivers/gpu/drm/i915/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/intel_workarounds.c
> @@ -294,6 +294,10 @@ static void gen9_ctx_workarounds_init(struct intel_engine_cs *engine)
>                           FLOW_CONTROL_ENABLE |
>                           PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE);
>  
> +       /* Bspec wa: 0651, disable rr arb and enable tdl fix */
> +       WA_SET_BIT_MASKED(GEN7_ROW_CHICKEN2, GEN8_DISABLE_RR_ARBITRATION);
> +       WA_CLR_BIT_MASKED(GEN8_ROW_CHICKEN4, GEN8_DISABLE_TDL_FIX);

In your first patch, you had this only applying to Skylake. Is that
still the case? i.e. do we need if (IS_SKYLAKE()) { ... }
-Chris
Mika Kuoppala April 16, 2019, 1:26 p.m. UTC | #2
Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2019-04-15 15:45:29)
>> Set chicken bits to workaround a possible pixel shader
>> dispatch hang.
>> 
>> v2: no need to filter out preprod skl (Ville, Chris)
>> v3: formatting
>> 
>> Bspec: 14091, ID#0651
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h          | 4 ++++
>>  drivers/gpu/drm/i915/intel_workarounds.c | 4 ++++
>>  2 files changed, 8 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index c1c0f7ab03e9..499cc843443d 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -8902,11 +8902,15 @@ enum {
>>  #define GEN7_ROW_CHICKEN2_GT2          _MMIO(0xf4f4)
>>  #define   DOP_CLOCK_GATING_DISABLE     (1 << 0)
>>  #define   PUSH_CONSTANT_DEREF_DISABLE  (1 << 8)
>> +#define   GEN8_DISABLE_RR_ARBITRATION  (1 << 1)
>>  #define   GEN11_TDL_CLOCK_GATING_FIX_DISABLE   (1 << 1)
>>  
>>  #define HSW_ROW_CHICKEN3               _MMIO(0xe49c)
>>  #define  HSW_ROW_CHICKEN3_L3_GLOBAL_ATOMICS_DISABLE    (1 << 6)
>>  
>> +#define GEN8_ROW_CHICKEN4              _MMIO(0xe48c)
>> +#define  GEN8_DISABLE_TDL_FIX          (1 << 3)
>> +
>>  #define HALF_SLICE_CHICKEN2            _MMIO(0xe180)
>>  #define   GEN8_ST_PO_DISABLE           (1 << 13)
>>  
>> diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
>> index ccaf63679435..b4709de49552 100644
>> --- a/drivers/gpu/drm/i915/intel_workarounds.c
>> +++ b/drivers/gpu/drm/i915/intel_workarounds.c
>> @@ -294,6 +294,10 @@ static void gen9_ctx_workarounds_init(struct intel_engine_cs *engine)
>>                           FLOW_CONTROL_ENABLE |
>>                           PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE);
>>  
>> +       /* Bspec wa: 0651, disable rr arb and enable tdl fix */
>> +       WA_SET_BIT_MASKED(GEN7_ROW_CHICKEN2, GEN8_DISABLE_RR_ARBITRATION);
>> +       WA_CLR_BIT_MASKED(GEN8_ROW_CHICKEN4, GEN8_DISABLE_TDL_FIX);
>
> In your first patch, you had this only applying to Skylake. Is that
> still the case? i.e. do we need if (IS_SKYLAKE()) { ... }

I had !SKL_REVID() so I only wanted to exclude everything up to G0.

Bspec on row chicken 2 says that we want this from skl G0 onwards and
bxt b0 onwards up to ICL. So with preprods out, I think it holds.

-Mika
Chris Wilson April 16, 2019, 1:30 p.m. UTC | #3
Quoting Mika Kuoppala (2019-04-16 14:26:19)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Quoting Mika Kuoppala (2019-04-15 15:45:29)
> >> Set chicken bits to workaround a possible pixel shader
> >> dispatch hang.
> >> 
> >> v2: no need to filter out preprod skl (Ville, Chris)
> >> v3: formatting
> >> 
> >> Bspec: 14091, ID#0651
> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/i915_reg.h          | 4 ++++
> >>  drivers/gpu/drm/i915/intel_workarounds.c | 4 ++++
> >>  2 files changed, 8 insertions(+)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >> index c1c0f7ab03e9..499cc843443d 100644
> >> --- a/drivers/gpu/drm/i915/i915_reg.h
> >> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> @@ -8902,11 +8902,15 @@ enum {
> >>  #define GEN7_ROW_CHICKEN2_GT2          _MMIO(0xf4f4)
> >>  #define   DOP_CLOCK_GATING_DISABLE     (1 << 0)
> >>  #define   PUSH_CONSTANT_DEREF_DISABLE  (1 << 8)
> >> +#define   GEN8_DISABLE_RR_ARBITRATION  (1 << 1)
> >>  #define   GEN11_TDL_CLOCK_GATING_FIX_DISABLE   (1 << 1)
> >>  
> >>  #define HSW_ROW_CHICKEN3               _MMIO(0xe49c)
> >>  #define  HSW_ROW_CHICKEN3_L3_GLOBAL_ATOMICS_DISABLE    (1 << 6)
> >>  
> >> +#define GEN8_ROW_CHICKEN4              _MMIO(0xe48c)
> >> +#define  GEN8_DISABLE_TDL_FIX          (1 << 3)
> >> +
> >>  #define HALF_SLICE_CHICKEN2            _MMIO(0xe180)
> >>  #define   GEN8_ST_PO_DISABLE           (1 << 13)
> >>  
> >> diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
> >> index ccaf63679435..b4709de49552 100644
> >> --- a/drivers/gpu/drm/i915/intel_workarounds.c
> >> +++ b/drivers/gpu/drm/i915/intel_workarounds.c
> >> @@ -294,6 +294,10 @@ static void gen9_ctx_workarounds_init(struct intel_engine_cs *engine)
> >>                           FLOW_CONTROL_ENABLE |
> >>                           PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE);
> >>  
> >> +       /* Bspec wa: 0651, disable rr arb and enable tdl fix */
> >> +       WA_SET_BIT_MASKED(GEN7_ROW_CHICKEN2, GEN8_DISABLE_RR_ARBITRATION);
> >> +       WA_CLR_BIT_MASKED(GEN8_ROW_CHICKEN4, GEN8_DISABLE_TDL_FIX);
> >
> > In your first patch, you had this only applying to Skylake. Is that
> > still the case? i.e. do we need if (IS_SKYLAKE()) { ... }
> 
> I had !SKL_REVID() so I only wanted to exclude everything up to G0.
> 
> Bspec on row chicken 2 says that we want this from skl G0 onwards and
> bxt b0 onwards up to ICL. So with preprods out, I think it holds.

Ok, I'm easily convinced.
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c1c0f7ab03e9..499cc843443d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8902,11 +8902,15 @@  enum {
 #define GEN7_ROW_CHICKEN2_GT2		_MMIO(0xf4f4)
 #define   DOP_CLOCK_GATING_DISABLE	(1 << 0)
 #define   PUSH_CONSTANT_DEREF_DISABLE	(1 << 8)
+#define   GEN8_DISABLE_RR_ARBITRATION	(1 << 1)
 #define   GEN11_TDL_CLOCK_GATING_FIX_DISABLE	(1 << 1)
 
 #define HSW_ROW_CHICKEN3		_MMIO(0xe49c)
 #define  HSW_ROW_CHICKEN3_L3_GLOBAL_ATOMICS_DISABLE    (1 << 6)
 
+#define GEN8_ROW_CHICKEN4		_MMIO(0xe48c)
+#define  GEN8_DISABLE_TDL_FIX		(1 << 3)
+
 #define HALF_SLICE_CHICKEN2		_MMIO(0xe180)
 #define   GEN8_ST_PO_DISABLE		(1 << 13)
 
diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
index ccaf63679435..b4709de49552 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -294,6 +294,10 @@  static void gen9_ctx_workarounds_init(struct intel_engine_cs *engine)
 			  FLOW_CONTROL_ENABLE |
 			  PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE);
 
+	/* Bspec wa: 0651, disable rr arb and enable tdl fix */
+	WA_SET_BIT_MASKED(GEN7_ROW_CHICKEN2, GEN8_DISABLE_RR_ARBITRATION);
+	WA_CLR_BIT_MASKED(GEN8_ROW_CHICKEN4, GEN8_DISABLE_TDL_FIX);
+
 	/* Syncing dependencies between camera and graphics:skl,bxt,kbl */
 	if (!IS_COFFEELAKE(i915))
 		WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN3,