Re: [PATCH v3] drm/i915/gt: Do not consider preemption during execlists_dequeue for gen8

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

On Thu, Jul 11, 2024 at 04:28:53PM +0000, Cavitt, Jonathan wrote:
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Nitin Gote
> Sent: Thursday, July 11, 2024 9:32 AM
> To: Wilson, Chris P <chris.p.wilson@intel.com>; tursulin@ursulin.net; intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Shyti, Andi <andi.shyti@intel.com>; Das, Nirmoy <nirmoy.das@intel.com>; janusz.krzysztofik@linux.intel.com; Gote, Nitin R <nitin.r.gote@intel.com>; Chris Wilson <chris.p.wilson@linux.intel.com>; stable@vger.kernel.org
> Subject: [PATCH v3] drm/i915/gt: Do not consider preemption during execlists_dequeue for gen8
> > 
> > We're seeing a GPU HANG issue on a CHV platform, which was caused by
> > bac24f59f454 ("drm/i915/execlists: Enable coarse preemption boundaries for gen8").
> > 
> > Gen8 platform has only timeslice and doesn't support a preemption mechanism
> > as engines do not have a preemption timer and doesn't send an irq if the
> > preemption timeout expires.
> 
> That seems to mean the original can_preempt function was inaccurately built,
> so fixing it here makes the most sense to me, especially if it's causing problems.
> 
> Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> -Jonathan Cavitt
> 
> > So, add a fix to not consider preemption
> > during dequeuing for gen8 platforms.
> > 
> > v2: Simplify can_preempt() function (Tvrtko Ursulin)
> > 
> > v3:
> >  - Inside need_preempt(), condition of can_preempt() is not required
> >    as simplified can_preempt() is enough. (Chris Wilson)
> > 
> > Fixes: bac24f59f454 ("drm/i915/execlists: Enable coarse preemption boundaries for gen8")

Something strange in here...

This patch is not using directly or indirectly (I915_ENGINE_HAS_PREEMPTION) the
can_preempt()...

> > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11396
> > Suggested-by: Andi Shyti <andi.shyti@intel.com>
> > Signed-off-by: Nitin Gote <nitin.r.gote@intel.com>
> > Cc: Chris Wilson <chris.p.wilson@linux.intel.com>
> > CC: <stable@vger.kernel.org> # v5.2+
> > ---
> >  drivers/gpu/drm/i915/gt/intel_execlists_submission.c | 6 +-----
> >  1 file changed, 1 insertion(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> > index 21829439e686..72090f52fb85 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> > @@ -3315,11 +3315,7 @@ static void remove_from_engine(struct i915_request *rq)
> >  
> >  static bool can_preempt(struct intel_engine_cs *engine)
> >  {
> > -	if (GRAPHICS_VER(engine->i915) > 8)
> > -		return true;
> > -
> > -	/* GPGPU on bdw requires extra w/a; not implemented */
> > -	return engine->class != RENDER_CLASS;
> > +	return GRAPHICS_VER(engine->i915) > 8;
> >  }
> >  
> >  static void kick_execlists(const struct i915_request *rq, int prio)
> > -- 
> > 2.25.1
> > 
> >