Re: [Intel-gfx] [PATCH v8] drm/i915: Enable WaProgramMgsrForCorrectSliceSpecificMmioReads for Gen9

2021-11-01 Thread Tvrtko Ursulin



On 01/11/2021 10:40, Joonas Lahtinen wrote:

Quoting Tvrtko Ursulin (2021-11-01 12:15:19)


On 25/10/2021 05:26, Cooper Chiou wrote:

This implements WaProgramMgsrForCorrectSliceSpecificMmioReads which
was omitted by mistake from Gen9 documentation, while it is actually
applicable to fused off parts.

Workaround consists of making sure MCR packet control register is
programmed to point to enabled slice/subslice pair before doing any
MMIO reads from the affected registers.

Failure do to this can result in complete system hangs when running
certain workloads. Two known cases which can cause system hangs are:

1. "test_basic progvar_prog_scope_uninit" test which is part of
  Khronos OpenCL conformance suite
  (https://github.com/KhronosGroup/OpenCL-CTS) with the Intel
  OpenCL driver (https://github.com/intel/compute-runtime).

2. VP8 media hardware encoding using the full-feature build of the
  Intel media-driver (https://github.com/intel/media-driver) and
  ffmpeg.

For the former case patch was verified to fix the hard system hang
when executing the OCL test on Intel Pentium CPU 6405U which contains
fused off GT1 graphics.

Reference: HSD#1508045018,1405586840, BSID#0575

Cc: Ville Syrjälä 
Cc: Rodrigo Vivi 
Cc: Jani Nikula 
Cc: Chris Wilson 
Cc: Tvrtko Ursulin 
Cc: William Tseng 
Cc: Shawn C Lee 
Cc: Pawel Wilma 
Signed-off-by: Cooper Chiou 


Reviewed-by: Tvrtko Ursulin 


Thanks for following through with all the testing and validation for the
patch!

Acked-by: Joonas Lahtinen 


I've pushed it, thanks!

Regards,

Tvrtko


Re: [Intel-gfx] [PATCH v8] drm/i915: Enable WaProgramMgsrForCorrectSliceSpecificMmioReads for Gen9

2021-11-01 Thread Joonas Lahtinen
Quoting Tvrtko Ursulin (2021-11-01 12:15:19)
> 
> On 25/10/2021 05:26, Cooper Chiou wrote:
> > This implements WaProgramMgsrForCorrectSliceSpecificMmioReads which
> > was omitted by mistake from Gen9 documentation, while it is actually
> > applicable to fused off parts.
> > 
> > Workaround consists of making sure MCR packet control register is
> > programmed to point to enabled slice/subslice pair before doing any
> > MMIO reads from the affected registers.
> > 
> > Failure do to this can result in complete system hangs when running
> > certain workloads. Two known cases which can cause system hangs are:
> > 
> > 1. "test_basic progvar_prog_scope_uninit" test which is part of
> >  Khronos OpenCL conformance suite
> >  (https://github.com/KhronosGroup/OpenCL-CTS) with the Intel
> >  OpenCL driver (https://github.com/intel/compute-runtime).
> > 
> > 2. VP8 media hardware encoding using the full-feature build of the
> >  Intel media-driver (https://github.com/intel/media-driver) and
> >  ffmpeg.
> > 
> > For the former case patch was verified to fix the hard system hang
> > when executing the OCL test on Intel Pentium CPU 6405U which contains
> > fused off GT1 graphics.
> > 
> > Reference: HSD#1508045018,1405586840, BSID#0575
> > 
> > Cc: Ville Syrjälä 
> > Cc: Rodrigo Vivi 
> > Cc: Jani Nikula 
> > Cc: Chris Wilson 
> > Cc: Tvrtko Ursulin 
> > Cc: William Tseng 
> > Cc: Shawn C Lee 
> > Cc: Pawel Wilma 
> > Signed-off-by: Cooper Chiou 
> 
> Reviewed-by: Tvrtko Ursulin 

Thanks for following through with all the testing and validation for the
patch!

Acked-by: Joonas Lahtinen 

Regards, Joonas

> 
> Regards,
> 
> Tvrtko
> 
> P.S. You could have preserved my r-b from an earlier version since it is 
> the same code, just different commit message.
> 
> > ---
> >   drivers/gpu/drm/i915/gt/intel_workarounds.c | 41 +
> >   1 file changed, 41 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c 
> > b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > index e1f362530889..8ae24da601b0 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > @@ -877,11 +877,52 @@ hsw_gt_workarounds_init(struct intel_gt *gt, struct 
> > i915_wa_list *wal)
> >   wa_write_clr(wal, GEN7_FF_THREAD_MODE, GEN7_FF_VS_REF_CNT_FFME);
> >   }
> >   
> > +static void
> > +gen9_wa_init_mcr(struct drm_i915_private *i915, struct i915_wa_list *wal)
> > +{
> > + const struct sseu_dev_info *sseu = >gt.info.sseu;
> > + unsigned int slice, subslice;
> > + u32 mcr, mcr_mask;
> > +
> > + GEM_BUG_ON(GRAPHICS_VER(i915) != 9);
> > +
> > + /*
> > +  * WaProgramMgsrForCorrectSliceSpecificMmioReads:gen9,glk,kbl,cml
> > +  * Before any MMIO read into slice/subslice specific registers, MCR
> > +  * packet control register needs to be programmed to point to any
> > +  * enabled s/ss pair. Otherwise, incorrect values will be returned.
> > +  * This means each subsequent MMIO read will be forwarded to an
> > +  * specific s/ss combination, but this is OK since these registers
> > +  * are consistent across s/ss in almost all cases. In the rare
> > +  * occasions, such as INSTDONE, where this value is dependent
> > +  * on s/ss combo, the read should be done with read_subslice_reg.
> > +  */
> > + slice = ffs(sseu->slice_mask) - 1;
> > + GEM_BUG_ON(slice >= ARRAY_SIZE(sseu->subslice_mask));
> > + subslice = ffs(intel_sseu_get_subslices(sseu, slice));
> > + GEM_BUG_ON(!subslice);
> > + subslice--;
> > +
> > + /*
> > +  * We use GEN8_MCR..() macros to calculate the |mcr| value for
> > +  * Gen9 to address WaProgramMgsrForCorrectSliceSpecificMmioReads
> > +  */
> > + mcr = GEN8_MCR_SLICE(slice) | GEN8_MCR_SUBSLICE(subslice);
> > + mcr_mask = GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK;
> > +
> > + drm_dbg(>drm, "MCR slice:%d/subslice:%d = %x\n", slice, 
> > subslice, mcr);
> > +
> > + wa_write_clr_set(wal, GEN8_MCR_SELECTOR, mcr_mask, mcr);
> > +}
> > +
> >   static void
> >   gen9_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
> >   {
> >   struct drm_i915_private *i915 = gt->i915;
> >   
> > + /* WaProgramMgsrForCorrectSliceSpecificMmioReads:glk,kbl,cml,gen9 */
> > + gen9_wa_init_mcr(i915, wal);
> > +
> >   /* WaDisableKillLogic:bxt,skl,kbl */
> >   if (!IS_COFFEELAKE(i915) && !IS_COMETLAKE(i915))
> >   wa_write_or(wal,
> > 


Re: [Intel-gfx] [PATCH v8] drm/i915: Enable WaProgramMgsrForCorrectSliceSpecificMmioReads for Gen9

2021-11-01 Thread Tvrtko Ursulin



On 25/10/2021 05:26, Cooper Chiou wrote:

This implements WaProgramMgsrForCorrectSliceSpecificMmioReads which
was omitted by mistake from Gen9 documentation, while it is actually
applicable to fused off parts.

Workaround consists of making sure MCR packet control register is
programmed to point to enabled slice/subslice pair before doing any
MMIO reads from the affected registers.

Failure do to this can result in complete system hangs when running
certain workloads. Two known cases which can cause system hangs are:

1. "test_basic progvar_prog_scope_uninit" test which is part of
 Khronos OpenCL conformance suite
 (https://github.com/KhronosGroup/OpenCL-CTS) with the Intel
 OpenCL driver (https://github.com/intel/compute-runtime).

2. VP8 media hardware encoding using the full-feature build of the
 Intel media-driver (https://github.com/intel/media-driver) and
 ffmpeg.

For the former case patch was verified to fix the hard system hang
when executing the OCL test on Intel Pentium CPU 6405U which contains
fused off GT1 graphics.

Reference: HSD#1508045018,1405586840, BSID#0575

Cc: Ville Syrjälä 
Cc: Rodrigo Vivi 
Cc: Jani Nikula 
Cc: Chris Wilson 
Cc: Tvrtko Ursulin 
Cc: William Tseng 
Cc: Shawn C Lee 
Cc: Pawel Wilma 
Signed-off-by: Cooper Chiou 


Reviewed-by: Tvrtko Ursulin 

Regards,

Tvrtko

P.S. You could have preserved my r-b from an earlier version since it is 
the same code, just different commit message.



---
  drivers/gpu/drm/i915/gt/intel_workarounds.c | 41 +
  1 file changed, 41 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c 
b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index e1f362530889..8ae24da601b0 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -877,11 +877,52 @@ hsw_gt_workarounds_init(struct intel_gt *gt, struct 
i915_wa_list *wal)
wa_write_clr(wal, GEN7_FF_THREAD_MODE, GEN7_FF_VS_REF_CNT_FFME);
  }
  
+static void

+gen9_wa_init_mcr(struct drm_i915_private *i915, struct i915_wa_list *wal)
+{
+   const struct sseu_dev_info *sseu = >gt.info.sseu;
+   unsigned int slice, subslice;
+   u32 mcr, mcr_mask;
+
+   GEM_BUG_ON(GRAPHICS_VER(i915) != 9);
+
+   /*
+* WaProgramMgsrForCorrectSliceSpecificMmioReads:gen9,glk,kbl,cml
+* Before any MMIO read into slice/subslice specific registers, MCR
+* packet control register needs to be programmed to point to any
+* enabled s/ss pair. Otherwise, incorrect values will be returned.
+* This means each subsequent MMIO read will be forwarded to an
+* specific s/ss combination, but this is OK since these registers
+* are consistent across s/ss in almost all cases. In the rare
+* occasions, such as INSTDONE, where this value is dependent
+* on s/ss combo, the read should be done with read_subslice_reg.
+*/
+   slice = ffs(sseu->slice_mask) - 1;
+   GEM_BUG_ON(slice >= ARRAY_SIZE(sseu->subslice_mask));
+   subslice = ffs(intel_sseu_get_subslices(sseu, slice));
+   GEM_BUG_ON(!subslice);
+   subslice--;
+
+   /*
+* We use GEN8_MCR..() macros to calculate the |mcr| value for
+* Gen9 to address WaProgramMgsrForCorrectSliceSpecificMmioReads
+*/
+   mcr = GEN8_MCR_SLICE(slice) | GEN8_MCR_SUBSLICE(subslice);
+   mcr_mask = GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK;
+
+   drm_dbg(>drm, "MCR slice:%d/subslice:%d = %x\n", slice, subslice, 
mcr);
+
+   wa_write_clr_set(wal, GEN8_MCR_SELECTOR, mcr_mask, mcr);
+}
+
  static void
  gen9_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
  {
struct drm_i915_private *i915 = gt->i915;
  
+	/* WaProgramMgsrForCorrectSliceSpecificMmioReads:glk,kbl,cml,gen9 */

+   gen9_wa_init_mcr(i915, wal);
+
/* WaDisableKillLogic:bxt,skl,kbl */
if (!IS_COFFEELAKE(i915) && !IS_COMETLAKE(i915))
wa_write_or(wal,



[Intel-gfx] [PATCH v8] drm/i915: Enable WaProgramMgsrForCorrectSliceSpecificMmioReads for Gen9

2021-10-24 Thread Cooper Chiou
This implements WaProgramMgsrForCorrectSliceSpecificMmioReads which
was omitted by mistake from Gen9 documentation, while it is actually
applicable to fused off parts.

Workaround consists of making sure MCR packet control register is
programmed to point to enabled slice/subslice pair before doing any
MMIO reads from the affected registers.

Failure do to this can result in complete system hangs when running
certain workloads. Two known cases which can cause system hangs are:

1. "test_basic progvar_prog_scope_uninit" test which is part of
Khronos OpenCL conformance suite
(https://github.com/KhronosGroup/OpenCL-CTS) with the Intel
OpenCL driver (https://github.com/intel/compute-runtime).

2. VP8 media hardware encoding using the full-feature build of the
Intel media-driver (https://github.com/intel/media-driver) and
ffmpeg.

For the former case patch was verified to fix the hard system hang
when executing the OCL test on Intel Pentium CPU 6405U which contains
fused off GT1 graphics.

Reference: HSD#1508045018,1405586840, BSID#0575

Cc: Ville Syrjälä 
Cc: Rodrigo Vivi 
Cc: Jani Nikula 
Cc: Chris Wilson 
Cc: Tvrtko Ursulin 
Cc: William Tseng 
Cc: Shawn C Lee 
Cc: Pawel Wilma 
Signed-off-by: Cooper Chiou 
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 41 +
 1 file changed, 41 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c 
b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index e1f362530889..8ae24da601b0 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -877,11 +877,52 @@ hsw_gt_workarounds_init(struct intel_gt *gt, struct 
i915_wa_list *wal)
wa_write_clr(wal, GEN7_FF_THREAD_MODE, GEN7_FF_VS_REF_CNT_FFME);
 }
 
+static void
+gen9_wa_init_mcr(struct drm_i915_private *i915, struct i915_wa_list *wal)
+{
+   const struct sseu_dev_info *sseu = >gt.info.sseu;
+   unsigned int slice, subslice;
+   u32 mcr, mcr_mask;
+
+   GEM_BUG_ON(GRAPHICS_VER(i915) != 9);
+
+   /*
+* WaProgramMgsrForCorrectSliceSpecificMmioReads:gen9,glk,kbl,cml
+* Before any MMIO read into slice/subslice specific registers, MCR
+* packet control register needs to be programmed to point to any
+* enabled s/ss pair. Otherwise, incorrect values will be returned.
+* This means each subsequent MMIO read will be forwarded to an
+* specific s/ss combination, but this is OK since these registers
+* are consistent across s/ss in almost all cases. In the rare
+* occasions, such as INSTDONE, where this value is dependent
+* on s/ss combo, the read should be done with read_subslice_reg.
+*/
+   slice = ffs(sseu->slice_mask) - 1;
+   GEM_BUG_ON(slice >= ARRAY_SIZE(sseu->subslice_mask));
+   subslice = ffs(intel_sseu_get_subslices(sseu, slice));
+   GEM_BUG_ON(!subslice);
+   subslice--;
+
+   /*
+* We use GEN8_MCR..() macros to calculate the |mcr| value for
+* Gen9 to address WaProgramMgsrForCorrectSliceSpecificMmioReads
+*/
+   mcr = GEN8_MCR_SLICE(slice) | GEN8_MCR_SUBSLICE(subslice);
+   mcr_mask = GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK;
+
+   drm_dbg(>drm, "MCR slice:%d/subslice:%d = %x\n", slice, subslice, 
mcr);
+
+   wa_write_clr_set(wal, GEN8_MCR_SELECTOR, mcr_mask, mcr);
+}
+
 static void
 gen9_gt_workarounds_init(struct intel_gt *gt, struct i915_wa_list *wal)
 {
struct drm_i915_private *i915 = gt->i915;
 
+   /* WaProgramMgsrForCorrectSliceSpecificMmioReads:glk,kbl,cml,gen9 */
+   gen9_wa_init_mcr(i915, wal);
+
/* WaDisableKillLogic:bxt,skl,kbl */
if (!IS_COFFEELAKE(i915) && !IS_COMETLAKE(i915))
wa_write_or(wal,
-- 
2.33.1