[Intel-gfx] [PATCH] drm/i915/icl: Apply WaEnablePreemptionGranularityControlByUMD

[Michał Winiarski]

On Mon, Jan 07, 2019 at 11:19:31AM -0800, Matt Roper wrote:
> On Mon, Jan 07, 2019 at 01:23:50PM +0100, Michał Winiarski wrote:
> > On Mon, Jan 07, 2019 at 01:01:16PM +0200, Joonas Lahtinen wrote:
> > > Quoting José Roberto de Souza (2019-01-04 19:37:00)
> > > > According to Workaround database ICL also needs
> > > > WaEnablePreemptionGranularityControlByUMD, to allow userspace to do
> > > > fine-granularity preemptions per-context.
> > > 
> > > I must wonder where is the userspace component that needs this, and why
> > > it hasn't been noticed earlier?
> > > 
> > > Or is this one more of the cases when no userspace actually uses the
> > > register?
> > 
> > It's used:
> > https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/mesa/drivers/dri/i965/brw_state_upload.c#L64
> > 
> > -Michał
> 
> Wasn't this just an artificial i915-only workaround that was added to
> prevent breakage of pre-preemption UMD's?  Initial gen9 driver releases
> didn't support preemption, so when preemption support did get added to
> i915, the kernel had to force object-level off by default at context
> creation to avoid breaking old userspace that didn't build batch buffers
> with all the necessary preemption workarounds.  This CS_CHICKEN1
> register was then exposed to userspace so that newer, preemption-aware
> userspace could opt back in if it properly supported preemption.

It wasn't just old userspace vs preeemption-aware userspace.
Even the preemption-aware userspace needs to downgrade its preemption
granularity as a WA, see:
https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/mesa/drivers/dri/i965/brw_draw.c#L876

> For gen11, there shouldn't be any "old" userspace around that doesn't
> support preemption, so shouldn't the kernel just leave object-level
> preemption enabled by default (meaning there's no need to expose this
> register to userspace to allow it to explicitly opt-in)?

Agreed, as a bonus we don't allow anyone to explicity opt-out - which is
great, as long as everything works reliably.

-Michał

> 
> Matt
> 
> > 
> > > Regards, Joonas
> > > 
> > > > 
> > > > BSpec: 11348
> > > > Cc: Oscar Mateo <oscar.mateo at intel.com>
> > > > Cc: Radhakrishna Sripada <radhakrishna.sripada at intel.com>
> > > > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_workarounds.c | 9 ++++++---
> > > >  1 file changed, 6 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
> > > > index 480c53a2ecb5..bbc5a66faa07 100644
> > > > --- a/drivers/gpu/drm/i915/intel_workarounds.c
> > > > +++ b/drivers/gpu/drm/i915/intel_workarounds.c
> > > > @@ -1014,7 +1014,7 @@ static void gen9_whitelist_build(struct i915_wa_list *w)
> > > >         /* WaVFEStateAfterPipeControlwithMediaStateClear:skl,bxt,glk,cfl */
> > > >         whitelist_reg(w, GEN9_CTX_PREEMPT_REG);
> > > >  
> > > > -       /* WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,[cnl] */
> > > > +       /* WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,[cnl,icl] */
> > > >         whitelist_reg(w, GEN8_CS_CHICKEN1);
> > > >  
> > > >         /* WaAllowUMDToModifyHDCChicken1:skl,bxt,kbl,glk,cfl */
> > > > @@ -1068,6 +1068,9 @@ static void icl_whitelist_build(struct i915_wa_list *w)
> > > >  
> > > >         /* WaAllowUMDToModifySamplerMode:icl */
> > > >         whitelist_reg(w, GEN10_SAMPLER_MODE);
> > > > +
> > > > +       /* WaEnablePreemptionGranularityControlByUMD:icl */
> > > > +       whitelist_reg(w, GEN8_CS_CHICKEN1);
> > > >  }
> > > >  
> > > >  void intel_engine_init_whitelist(struct intel_engine_cs *engine)
> > > > @@ -1186,8 +1189,8 @@ static void rcs_engine_wa_init(struct intel_engine_cs *engine)
> > > >                                     GEN7_DISABLE_SAMPLER_PREFETCH);
> > > >         }
> > > >  
> > > > -       if (IS_GEN(i915, 9) || IS_CANNONLAKE(i915)) {
> > > > -               /* WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,cnl */
> > > > +       if (IS_GEN_RANGE(i915, 9, 11)) {
> > > > +               /* WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,cnl,icl */
> > > >                 wa_masked_en(wal,
> > > >                              GEN7_FF_SLICE_CS_CHICKEN1,
> > > >                              GEN9_FFSC_PERCTX_PREEMPT_CTRL);
> > > > -- 
> > > > 2.20.1
> > > > 
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795