[PATCH v1] drm/i915/guc: Always disable interrupt ahead of synchronize_irq

[Andi Shyti]

Hi,

Please, next time, do not remove the mailing and the other folks
you cc'ed.

I'm adding back the mailing list and Daniele who has commented
before.

...

> > > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13454
> > > Fixes: 26705e20752a ("drm/i915: Support for GuC interrupts")
> > > Fixes: 54c52a841250 ("drm/i915/guc: Correctly handle GuC interrupts on Gen11")
> > > Fixes: 2ae096872a2c ("drm/i915/pxp: Implement PXP irq handler")
> > > Fixes: 3e7abf814193 ("drm/i915: Extract GT render power state management")
> > 
> > There is an issue here, each fixes is introduced in different
> > kernel versions and they can't be mixed all together as it would
> > be impossible to backport them to the stable brances.
> > 
> > E.g.:
> > Fixes: 3e7abf814193 ("drm/i915: Extract GT render power state management")
> > Cc: <stable at vger.kernel.org> # v5.5+
> > 
> > Fixes: 2ae096872a2c ("drm/i915/pxp: Implement PXP irq handler")
> > Cc: <stable at vger.kernel.org> # v5.16+
> > 
> > Fixes: 54c52a841250 ("drm/i915/guc: Correctly handle GuC interrupts on Gen11")
> > Cc: <stable at vger.kernel.org> # v5.3+
> > 
> > Fixes: 26705e20752a ("drm/i915: Support for GuC interrupts")
> > Cc: <stable at vger.kernel.org> # v4.10+
> > 
> > Could you please split this patch in the four different patches
> > that address the four different fixes?
> Sure, will split it in next rev.

First of all we need to understand if those Fixes are really
needed or not. Daniele in his review has pointed out that...

> > > 
> > 
> > No blank lines in the tag section, please.
> > 
> > > Signed-off-by: Zhanjun Dong <zhanjun.dong at intel.com>
> > > 
> > > ---
> > > Cc: Alan Previn <alan.previn.teres.alexis at intel.com>
> > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> > > Cc: Andi Shyti <andi.shyti at intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/gt/intel_rps.c      | 3 +--
> > >   drivers/gpu/drm/i915/gt/uc/intel_guc.c   | 4 ++--
> > >   drivers/gpu/drm/i915/pxp/intel_pxp_irq.c | 2 +-
> > >   3 files changed, 4 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> > > index fa304ea088e4..0fe7a8d7f460 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> > > @@ -244,8 +244,8 @@ static void rps_disable_interrupts(struct intel_rps *rps)
> > >   	gen6_gt_pm_disable_irq(gt, GEN6_PM_RPS_EVENTS);
> > >   	spin_unlock_irq(gt->irq_lock);
> > > +	rps_reset_interrupts(rps);

... the interrupts here are already disabled (read a couple of
lines above).

What is the reproduction rate of the issue? And how have you
tested it?

Thanks,
Andi

> > >   	intel_synchronize_irq(gt->i915);
> > > -
> > 
> > Sebastian has already commented in his review, but please don't
> > remove this blank line.
> > 
> > Andi
> > 
> > >   	/*
> > >   	 * Now that we will not be generating any more work, flush any
> > >   	 * outstanding tasks. As we are called on the RPS idle path,