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

[Chris Wilson]

Quoting Chris Wilson (2021-03-04 11:56:16)
> Quoting Chris Wilson (2021-03-04 09:19:24)
> > Quoting Tvrtko Ursulin (2021-03-04 09:12:26)
> > > 
> > > On 02/03/2021 06:27, Cooper Chiou wrote:
> > > > WaProgramMgsrForCorrectSliceSpecificMmioReads applies for Gen9 to
> > > > resolve VP8 hardware encoding system hang up on GT1 sku for
> > > > ChromiumOS projects
> > > > 
> > > > Slice specific MMIO read inaccurate so MGSR needs to be programmed
> > > > appropriately to get correct reads from these slicet-related MMIOs.
> > > > 
> > > > It dictates that before any MMIO read into Slice/Subslice specific
> > > > registers, MCR packet control register(0xFDC) needs to be programmed
> > > > to point to any enabled slice/subslice pair, especially GT1 fused sku
> > > > since this issue can be reproduced on VP8 hardware encoding via ffmpeg
> > > > on ChromiumOS devices.
> > > > When exit PC7, MGSR will reset so that we have to skip fused subslice ID.
> > > > 
> > > > Reference: HSD#1508045018,1405586840, BSID#0575
> > > > 
> > > > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > > Cc: Jani Nikula <jani.nikula at intel.com>
> > > > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > > > Cc: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
> > > > Cc: William Tseng <william.tseng at intel.com>
> > > > Cc: Lee Shawn C <shawn.c.lee at intel.com>
> > > > 
> > > > Signed-off-by: Cooper Chiou <cooper.chiou at intel.com>
> > > > ---
> > > >   drivers/gpu/drm/i915/gt/intel_workarounds.c | 38 +++++++++++++++++++++
> > > >   1 file changed, 38 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > > index 3b4a7da60f0b..4ad598a727a6 100644
> > > > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > > @@ -878,9 +878,47 @@ hsw_gt_workarounds_init(struct drm_i915_private *i915, 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 = &i915->gt.info.sseu;
> > > > +     unsigned int slice, subslice;
> > > > +     u32 mcr, mcr_mask;
> > > > +
> > > > +     GEM_BUG_ON(INTEL_GEN(i915) < 9);
> > > > +
> > > > +     /*
> > > > +      * WaProgramMgsrForCorrectSliceSpecificMmioReads: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 = fls(sseu->slice_mask) - 1;
> > > > +     GEM_BUG_ON(slice >= ARRAY_SIZE(sseu->subslice_mask));
> > > > +     subslice = fls(intel_sseu_get_subslices(sseu, slice));
> > > > +     GEM_BUG_ON(!subslice);
> > > > +     subslice--;
> > > > +
> > > > +     mcr = GEN8_MCR_SLICE(slice) | GEN8_MCR_SUBSLICE(subslice);
> > > > +     mcr_mask = GEN8_MCR_SLICE_MASK | GEN8_MCR_SUBSLICE_MASK;
> > > > +
> > > > +     drm_dbg(&i915->drm, "MCR slice:%d/subslice:%d = %x\n", slice, subslice, mcr);
> > > > +
> > > > +     wa_write_clr_set(wal, GEN8_MCR_SELECTOR, mcr_mask, mcr);
> > > > +}
> > > 
> > > Have you considered reusing existing wa_init_mcr? Just needs the 
> > > top-level assert changed and otherwise it looks it would do the right 
> > > thing for Gen9. Advantage being a smaller patch and less code to carry.
> > 
> > That was the first patch, and fails for the same reason. The problem
> > would appear to be in the mcr_mask for gt3.
> 
> For the record, it appears to be an issue with fls vs ffs. Switching to
> ffs also removes the warnings for workaround failures on ehl/jsl.

Of course the icl in farm2 started spewing warnigns, but the other icl
in farm1 were happy.
-Chris