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

["Dong, Zhanjun" <zhanjun.dong@intel.com>]

On 2025-02-03 8:29 a.m., Andi Shyti wrote:
> 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.

Thanks, I also found my previous response click on "reply", not the 
"reply all".

> 
> ...
> 
>>>> 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@vger.kernel.org> # v5.5+
>>>
>>> Fixes: 2ae096872a2c ("drm/i915/pxp: Implement PXP irq handler")
>>> Cc: <stable@vger.kernel.org> # v5.16+
>>>
>>> Fixes: 54c52a841250 ("drm/i915/guc: Correctly handle GuC interrupts on Gen11")
>>> Cc: <stable@vger.kernel.org> # v5.3+
>>>
>>> Fixes: 26705e20752a ("drm/i915: Support for GuC interrupts")
>>> Cc: <stable@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...
I agree with Daniele, these fixes is not needed.

> 
>>>>
>>>
>>> No blank lines in the tag section, please.
>>>
>>>> Signed-off-by: Zhanjun Dong <zhanjun.dong@intel.com>
>>>>
>>>> ---
>>>> Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
>>>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>>> Cc: Andi Shyti <andi.shyti@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?
I run the issue igt test 
(https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13454) on ARL 
for more than 3000 loops, but no reproduce at all. This issue seems 
happens few month a time, and auto closed by Jira as no reproduce after 
few weeks.
Due to the issue is hard to reproduce, what I do now is by code analysis.

Regards,
Zhanjun Dong

> 
> 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,