Re: [PATCH 1/2] drm/i915/icl: Default to Thread Group preemption for compute workloads

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Thu, Feb 28, 2019 at 11:02:54AM -0800, Rodrigo Vivi wrote:
> On Wed, Feb 27, 2019 at 04:51:08PM +0100, Michał Winiarski wrote:
> > We assumed that the default preemption granularity is fine for ICL.
> > Unfortunately, it turns out that some drivers don't support mid-thread
> > preemption for compute workloads.
> > If a workload that doesn't support mid-thread preemption gets mid-thread
> > preempted, we're going to observe a GPU hang.
> > While I'm here, let's also update the "workaround" naming.
> > 
> > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> > Cc: Anuj Phogat <anuj.phogat@intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Cc: Rafael Antognolli <rafael.antognolli@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_workarounds.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
> > index 743cf5b00155..a19e1c0052a7 100644
> > --- a/drivers/gpu/drm/i915/intel_workarounds.c
> > +++ b/drivers/gpu/drm/i915/intel_workarounds.c
> > @@ -555,6 +555,11 @@ static void icl_ctx_workarounds_init(struct intel_engine_cs *engine)
> >  			   GEN10_CACHE_MODE_SS,
> >  			   0, /* write-only, so skip validation */
> >  			   _MASKED_BIT_ENABLE(FLOAT_BLEND_OPTIMIZATION_ENABLE));
> > +
> > +	/* WaDisableGPGPUMidThreadPreemption:icl */
> 
> Could you please give me some internal pointers to this WA?
> I couldn't find it on bspec nor on wadb.
> 
> > +	WA_SET_FIELD_MASKED(GEN8_CS_CHICKEN1,
> > +			    GEN9_PREEMPT_GPGPU_LEVEL_MASK,
> > +			    GEN9_PREEMPT_GPGPU_THREAD_GROUP_LEVEL);
> >  }
> >  
> >  void intel_engine_init_ctx_wa(struct intel_engine_cs *engine)
> > @@ -1170,8 +1175,8 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
> >  				    GEN7_DISABLE_SAMPLER_PREFETCH);
> >  	}
> >  
> > -	if (IS_GEN(i915, 9) || IS_CANNONLAKE(i915)) {
> > -		/* WaEnablePreemptionGranularityControlByUMD:skl,bxt,kbl,cfl,cnl */
> 
> Please don't remove the old name that is still part of old
> references. If we have a new name for ICL+ please keep both
> here on the comments.

I'm sorry for my delayed response on irc, I hope you that answer
came in time to avoid your rework.

I just noticed that this name for gen9/10 platforsm is already inside
gen9_whitelist_build() and that on next patch you re-introduce the icl
one...

Also thanks for all the explanations. I checked bits with spec
and everything makes sense now, so:

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

(What I' wondering now is that how this behaviour can change with GuC...
I heard some media folks were using i915 directly and facing some
strange gpu hangs but no gpu hang when using guc submission...)

> 
> 
> > +	if (IS_GEN_RANGE(i915, 9, 11)) {
> > +		/* FtrPerCtxtPreemptionGranularityControl:skl,bxt,kbl,cfl,cnl,icl */
> 
> also please give me a pointer to this...
> 
> Thanks,
> Rodrigo.
> 
> >  		wa_masked_en(wal,
> >  			     GEN7_FF_SLICE_CS_CHICKEN1,
> >  			     GEN9_FFSC_PERCTX_PREEMPT_CTRL);
> > -- 
> > 2.20.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx