diff mbox series

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

Message ID 20250123162351.1364395-1-zhanjun.dong@intel.com (mailing list archive)
State New, archived
Headers show
Series [v1] drm/i915/guc: Always disable interrupt ahead of synchronize_irq | expand

Commit Message

Dong, Zhanjun Jan. 23, 2025, 4:23 p.m. UTC
The purpose of synchronize_irq is to wait for any pending IRQ handlers for the
interrupt to complete, if synchronize_irq called before interrupt disabled, an
tiny timing window created, where no more pending IRQ, but interrupt not
disabled yet. Meanwhile, if the interrupt event happened in this timing window,
an unexpected IRQ handling will be triggered.

Fixed by always disable interrupt ahead of synchronize_irq.

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")

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(-)

Comments

Sebastian Brzezinka Jan. 27, 2025, 2:44 p.m. UTC | #1
Hi Zhanjun

There is a missing newline at drm-tip/drivers/gpu/drm/i915/gt/intel_rps.c:249
Overall, LGTM.

On Thu Jan 23, 2025 at 4:23 PM UTC, Zhanjun Dong wrote:
> The purpose of synchronize_irq is to wait for any pending IRQ handlers for the
> interrupt to complete, if synchronize_irq called before interrupt disabled, an
> tiny timing window created, where no more pending IRQ, but interrupt not
> disabled yet. Meanwhile, if the interrupt event happened in this timing window,
> an unexpected IRQ handling will be triggered.
>
> Fixed by always disable interrupt ahead of synchronize_irq.
>
> 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")
>
> Signed-off-by: Zhanjun Dong <zhanjun.dong@intel.com>
>

Reviewed-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>

--
Best regards,
Sebastian
Andi Shyti Jan. 27, 2025, 3:21 p.m. UTC | #2
Hi Zhanjun,

On Thu, Jan 23, 2025 at 08:23:51AM -0800, Zhanjun Dong wrote:
> The purpose of synchronize_irq is to wait for any pending IRQ handlers for the
> interrupt to complete, if synchronize_irq called before interrupt disabled, an
> tiny timing window created, where no more pending IRQ, but interrupt not
> disabled yet. Meanwhile, if the interrupt event happened in this timing window,
> an unexpected IRQ handling will be triggered.

I get the meaning, but could you please rephrase it to a more
meaningful sentence, please?

> Fixed by always disable interrupt ahead of synchronize_irq.

Please, don't use the past simple form, use the imperative form:

"Always disable interrupt ahead..."

or better:

"Always disable interrupts before calling intel_synchronyze_irq()"

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

> 

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);
>  	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,
Daniele Ceraolo Spurio Jan. 27, 2025, 5:12 p.m. UTC | #3
On 1/23/2025 8:23 AM, Zhanjun Dong wrote:
> The purpose of synchronize_irq is to wait for any pending IRQ handlers for the
> interrupt to complete, if synchronize_irq called before interrupt disabled, an
> tiny timing window created, where no more pending IRQ, but interrupt not
> disabled yet. Meanwhile, if the interrupt event happened in this timing window,
> an unexpected IRQ handling will be triggered.
>
> Fixed by always disable interrupt ahead of synchronize_irq.
>
> 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")
>
> 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);
>   	intel_synchronize_irq(gt->i915);

I don't think this is an issue, because we set the irq mask in 
gen6_gt_pm_disable_irq, so there is no chance of getting any new 
interrupts after that. Not saying that we shouldn't do the re-order, but 
we don't need a fixes tag for this.

> -
>   	/*
>   	 * Now that we will not be generating any more work, flush any
>   	 * outstanding tasks. As we are called on the RPS idle path,
> @@ -254,7 +254,6 @@ static void rps_disable_interrupts(struct intel_rps *rps)
>   	 */
>   	cancel_work_sync(&rps->work);
>   
> -	rps_reset_interrupts(rps);
>   	GT_TRACE(gt, "interrupts:off\n");
>   }
>   
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> index 5949ff0b0161..3e7b2c6cdca4 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> @@ -116,9 +116,9 @@ static void gen9_disable_guc_interrupts(struct intel_guc *guc)
>   	gen6_gt_pm_disable_irq(gt, gt->pm_guc_events);
>   
>   	spin_unlock_irq(gt->irq_lock);
> -	intel_synchronize_irq(gt->i915);
>   
>   	gen9_reset_guc_interrupts(guc);
> +	intel_synchronize_irq(gt->i915);

Same as above with gen6_gt_pm_disable_irq

>   }
>   
>   static bool __gen11_reset_guc_interrupts(struct intel_gt *gt)
> @@ -154,9 +154,9 @@ static void gen11_disable_guc_interrupts(struct intel_guc *guc)
>   	struct intel_gt *gt = guc_to_gt(guc);
>   
>   	guc->interrupts.enabled = false;
> -	intel_synchronize_irq(gt->i915);
>   
>   	gen11_reset_guc_interrupts(guc);
> +	intel_synchronize_irq(gt->i915);

No early disabling here, but I don't think this change helps either. 
AFAICS gen11_reset_guc_interrupts() only calls gen11_gt_reset_one_iir(), 
which just clears the IIR bits for the GuC. There are no changes in 
interrupt enable/mask status, so interrupts can still be generated. The 
way interrupts are stopped for gen11+ is by setting 
guc->interrupts.enabled, because that's checked from both 
guc_irq_handler() and intel_guc_to_host_event_handler(), so any new 
interrupts generated after we set that should be immediately dropped.

>   }
>   
>   static void guc_dead_worker_func(struct work_struct *w)
> diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c
> index d81750b9bdda..b82a667e7ac0 100644
> --- a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c
> +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c
> @@ -101,9 +101,9 @@ void intel_pxp_irq_disable(struct intel_pxp *pxp)
>   	__pxp_set_interrupts(gt, 0);
>   
>   	spin_unlock_irq(gt->irq_lock);
> -	intel_synchronize_irq(gt->i915);
>   
>   	pxp_irq_reset(gt);
> +	intel_synchronize_irq(gt->i915);

Again not a bug here, __pxp_set_interrupts is doing the interrupts 
disabling and that is already happening before intel_synchronize_irq().

Daniele

>   
>   	flush_work(&pxp->session_work);
>   }
Andi Shyti Feb. 3, 2025, 1:29 p.m. UTC | #4
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@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...

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

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,
Dong, Zhanjun Feb. 3, 2025, 11:50 p.m. UTC | #5
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,
Andi Shyti Feb. 4, 2025, 10:25 a.m. UTC | #6
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".

no worries, happens :-)

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

yeah, with such a low reproduction rate, if it's not a
catastrofic failure the Fixes: tag is not really needed and
therefore you don't need to split the patch anymore.

Still we need to understand whether this patch really fixes the
issue.

Thanks,
Andi
diff mbox series

Patch

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);
 	intel_synchronize_irq(gt->i915);
-
 	/*
 	 * Now that we will not be generating any more work, flush any
 	 * outstanding tasks. As we are called on the RPS idle path,
@@ -254,7 +254,6 @@  static void rps_disable_interrupts(struct intel_rps *rps)
 	 */
 	cancel_work_sync(&rps->work);
 
-	rps_reset_interrupts(rps);
 	GT_TRACE(gt, "interrupts:off\n");
 }
 
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index 5949ff0b0161..3e7b2c6cdca4 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -116,9 +116,9 @@  static void gen9_disable_guc_interrupts(struct intel_guc *guc)
 	gen6_gt_pm_disable_irq(gt, gt->pm_guc_events);
 
 	spin_unlock_irq(gt->irq_lock);
-	intel_synchronize_irq(gt->i915);
 
 	gen9_reset_guc_interrupts(guc);
+	intel_synchronize_irq(gt->i915);
 }
 
 static bool __gen11_reset_guc_interrupts(struct intel_gt *gt)
@@ -154,9 +154,9 @@  static void gen11_disable_guc_interrupts(struct intel_guc *guc)
 	struct intel_gt *gt = guc_to_gt(guc);
 
 	guc->interrupts.enabled = false;
-	intel_synchronize_irq(gt->i915);
 
 	gen11_reset_guc_interrupts(guc);
+	intel_synchronize_irq(gt->i915);
 }
 
 static void guc_dead_worker_func(struct work_struct *w)
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c
index d81750b9bdda..b82a667e7ac0 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c
@@ -101,9 +101,9 @@  void intel_pxp_irq_disable(struct intel_pxp *pxp)
 	__pxp_set_interrupts(gt, 0);
 
 	spin_unlock_irq(gt->irq_lock);
-	intel_synchronize_irq(gt->i915);
 
 	pxp_irq_reset(gt);
+	intel_synchronize_irq(gt->i915);
 
 	flush_work(&pxp->session_work);
 }