[PATCH] drm/imagination: retry runtime idle when firmware is not yet idle

[PATCH] drm/imagination: retry runtime idle when firmware is not yet idle

[Baihui Liang]

Returning -EBUSY from ->runtime_idle() prevents the PM core from
retrying the idle check. Schedule an autosuspend so the idle state
can be re-evaluated once the firmware reaches ROGUE_FWIF_POW_IDLE.

Signed-off-by: Baihui Liang <liangbaihui@linux.spacemit.com>
---
Tested on SpacemiT K3 (PowerVR Rogue BXM-4-64) by running vulkaninfo
repeatedly. Without this change the device intermittently stayed
runtime-active after the last user went away, because the firmware had
not yet reached ROGUE_FWIF_POW_IDLE when the idle check ran. With the
change the device reliably suspends.
---
 drivers/gpu/drm/imagination/pvr_power.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/imagination/pvr_power.c b/drivers/gpu/drm/imagination/pvr_power.c
index eb4b6ecdf4f..6f38249ffcb 100644
--- a/drivers/gpu/drm/imagination/pvr_power.c
+++ b/drivers/gpu/drm/imagination/pvr_power.c
@@ -434,7 +434,16 @@ pvr_power_device_idle(struct device *dev)
 	struct drm_device *drm_dev = platform_get_drvdata(plat_dev);
 	struct pvr_device *pvr_dev = to_pvr_device(drm_dev);
 
-	return pvr_power_is_idle(pvr_dev) ? 0 : -EBUSY;
+	if (pvr_power_is_idle(pvr_dev))
+		return 0;
+
+	/*
+	 * Firmware not yet idle; reject but schedule autosuspend so
+	 * runtime_idle() can be retried.
+	 */
+	pm_runtime_autosuspend(dev);
+
+	return -EBUSY;
 }
 
 static int

---
base-commit: c339f4010dea8b0d754f84ace70cd1b089878ef8
change-id: 20261029-img-pm-autosuspend-7772d83416f7

Best regards,
--  
Baihui Liang <liangbaihui@linux.spacemit.com>

Re: [PATCH] drm/imagination: retry runtime idle when firmware is not yet idle

[Troy Mitchell]

[-- Attachment #1: Type: text/plain, Size: 1896 bytes --]

Hi Baihui,

On Thu Oct 29, 2026 at 4:38 PM +0800, Baihui Liang wrote:
> Returning -EBUSY from ->runtime_idle() prevents the PM core from
> retrying the idle check. Schedule an autosuspend so the idle state
> can be re-evaluated once the firmware reaches ROGUE_FWIF_POW_IDLE.
>
> Signed-off-by: Baihui Liang <liangbaihui@linux.spacemit.com>
> ---
> Tested on SpacemiT K3 (PowerVR Rogue BXM-4-64) by running vulkaninfo
> repeatedly. Without this change the device intermittently stayed
> runtime-active after the last user went away, because the firmware had
> not yet reached ROGUE_FWIF_POW_IDLE when the idle check ran. With the
> change the device reliably suspends.
> ---
>  drivers/gpu/drm/imagination/pvr_power.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/imagination/pvr_power.c b/drivers/gpu/drm/imagination/pvr_power.c
> index eb4b6ecdf4f..6f38249ffcb 100644
> --- a/drivers/gpu/drm/imagination/pvr_power.c
> +++ b/drivers/gpu/drm/imagination/pvr_power.c
> @@ -434,7 +434,16 @@ pvr_power_device_idle(struct device *dev)
>  	struct drm_device *drm_dev = platform_get_drvdata(plat_dev);
>  	struct pvr_device *pvr_dev = to_pvr_device(drm_dev);
>  
> -	return pvr_power_is_idle(pvr_dev) ? 0 : -EBUSY;
> +	if (pvr_power_is_idle(pvr_dev))
> +		return 0;
> +
> +	/*
> +	 * Firmware not yet idle; reject but schedule autosuspend so
> +	 * runtime_idle() can be retried.
> +	 */
> +	pm_runtime_autosuspend(dev);

If I read the PM core correctly, when this autosuspend timer fires it
calls ->runtime_suspend() directly rather than retrying
->runtime_idle(), so this looks equivalent to returning 0. Could the
real issue be the pm_runtime_put() in pvr_watchdog_worker(), which
perhaps should be pm_runtime_put_autosuspend()?

> +
> +	return -EBUSY;
>  }
>  
>  static int

-- 
Troy Mitchell

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] drm/imagination: retry runtime idle when firmware is not yet idle

[Gyeyoung Baek]

Hi Baihui, Troy,


On Fri, Oct 30, 2026 at 10:55 AM Troy Mitchell <troy@kernel.org> wrote:
>
> Hi Baihui,
>
> On Thu Oct 29, 2026 at 4:38 PM +0800, Baihui Liang wrote:
> > Returning -EBUSY from ->runtime_idle() prevents the PM core from
> > retrying the idle check. Schedule an autosuspend so the idle state
> > can be re-evaluated once the firmware reaches ROGUE_FWIF_POW_IDLE.
> >
> > Signed-off-by: Baihui Liang <liangbaihui@linux.spacemit.com>
> > ---
> > Tested on SpacemiT K3 (PowerVR Rogue BXM-4-64) by running vulkaninfo
> > repeatedly. Without this change the device intermittently stayed
> > runtime-active after the last user went away, because the firmware had
> > not yet reached ROGUE_FWIF_POW_IDLE when the idle check ran. With the
> > change the device reliably suspends.
> > ---
> >  drivers/gpu/drm/imagination/pvr_power.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/imagination/pvr_power.c b/drivers/gpu/drm/imagination/pvr_power.c
> > index eb4b6ecdf4f..6f38249ffcb 100644
> > --- a/drivers/gpu/drm/imagination/pvr_power.c
> > +++ b/drivers/gpu/drm/imagination/pvr_power.c
> > @@ -434,7 +434,16 @@ pvr_power_device_idle(struct device *dev)
> >       struct drm_device *drm_dev = platform_get_drvdata(plat_dev);
> >       struct pvr_device *pvr_dev = to_pvr_device(drm_dev);
> >
> > -     return pvr_power_is_idle(pvr_dev) ? 0 : -EBUSY;
> > +     if (pvr_power_is_idle(pvr_dev))
> > +             return 0;
> > +
> > +     /*
> > +      * Firmware not yet idle; reject but schedule autosuspend so
> > +      * runtime_idle() can be retried.
> > +      */
> > +     pm_runtime_autosuspend(dev);
>
> If I read the PM core correctly, when this autosuspend timer fires it
> calls ->runtime_suspend() directly rather than retrying
> ->runtime_idle(), so this looks equivalent to returning 0. Could the
> real issue be the pm_runtime_put() in pvr_watchdog_worker(), which
> perhaps should be pm_runtime_put_autosuspend()?

I think sashiko's point is valid.

Calling pm_runtime_autosuspend() from pvr_power_device_idle() when the
FW is busy only adds a delay, and the device is then suspended without
re-checking idle, and this is effectively the same as
returning 0. I guess it worked in testing because the FW was busy at
pvr_power_device_idle() time but reached idle before the autosuspend
delay expired.

How about moving the pvr_power_is_idle() check into
pvr_power_device_suspend() (and dropping pvr_power_device_idle()), and
returning -EBUSY after pm_runtime_mark_last_busy() if the FW is busy?
The PM core then reschedules the autosuspend regardless of whether the
last put comes from the watchdog or elsewhere, so suspend is retried
until the FW becomes idle.

I think this is the pattern described in
Documentation/power/runtime_pm.rst, section 9 ("If the
->runtime_suspend() callback returns -EAGAIN or -EBUSY ...").

--
Thanks,
Gyeyoung

Re: [PATCH] drm/imagination: retry runtime idle when firmware is not yet idle

[Baihui Liang]

Hi all,

Thanks for the review. All three of you are right, and v1 was wrong.

On Sun Nov 1, 2026 at 12:34 AM CST, Gyeyoung Baek wrote:
> Hi Baihui, Troy,
>
>
> On Fri, Oct 30, 2026 at 10:55 AM Troy Mitchell <troy@kernel.org> wrote:
> >
> > Hi Baihui,
> >
> > On Thu Oct 29, 2026 at 4:38 PM +0800, Baihui Liang wrote:
> > > Returning -EBUSY from ->runtime_idle() prevents the PM core from
> > > retrying the idle check. Schedule an autosuspend so the idle state
> > > can be re-evaluated once the firmware reaches ROGUE_FWIF_POW_IDLE.
> > >
> > > Signed-off-by: Baihui Liang <liangbaihui@linux.spacemit.com>
> > > ---
> > > Tested on SpacemiT K3 (PowerVR Rogue BXM-4-64) by running vulkaninfo
> > > repeatedly. Without this change the device intermittently stayed
> > > runtime-active after the last user went away, because the firmware had
> > > not yet reached ROGUE_FWIF_POW_IDLE when the idle check ran. With the
> > > change the device reliably suspends.
> > > ---
> > >  drivers/gpu/drm/imagination/pvr_power.c | 11 ++++++++++-
> > >  1 file changed, 10 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/imagination/pvr_power.c b/drivers/gpu/drm/imagination/pvr_power.c
> > > index eb4b6ecdf4f..6f38249ffcb 100644
> > > --- a/drivers/gpu/drm/imagination/pvr_power.c
> > > +++ b/drivers/gpu/drm/imagination/pvr_power.c
> > > @@ -434,7 +434,16 @@ pvr_power_device_idle(struct device *dev)
> > >       struct drm_device *drm_dev = platform_get_drvdata(plat_dev);
> > >       struct pvr_device *pvr_dev = to_pvr_device(drm_dev);
> > >
> > > -     return pvr_power_is_idle(pvr_dev) ? 0 : -EBUSY;
> > > +     if (pvr_power_is_idle(pvr_dev))
> > > +             return 0;
> > > +
> > > +     /*
> > > +      * Firmware not yet idle; reject but schedule autosuspend so
> > > +      * runtime_idle() can be retried.
> > > +      */
> > > +     pm_runtime_autosuspend(dev);
> >
> > If I read the PM core correctly, when this autosuspend timer fires it
> > calls ->runtime_suspend() directly rather than retrying
> > ->runtime_idle(), so this looks equivalent to returning 0. 

That's right. It merely delays unconditional FW teardown without
re-checking idle state. I'll send v2 with a different approach.

> > Could the
> > real issue be the pm_runtime_put() in pvr_watchdog_worker(), which
> > perhaps should be pm_runtime_put_autosuspend()?

That put is only reached when pm_runtime_get_if_in_use() actually
returned a reference. Our stuck-active case was the opposite case: the
device was active with a zero usage count, which jumps directly to
out_requeue and skips pm_runtime_put entirely. Changing this call
won't help.

> I think sashiko's point is valid.
>
> Calling pm_runtime_autosuspend() from pvr_power_device_idle() when the
> FW is busy only adds a delay, and the device is then suspended without
> re-checking idle, and this is effectively the same as
> returning 0. I guess it worked in testing because the FW was busy at
> pvr_power_device_idle() time but reached idle before the autosuspend
> delay expired.
>
> How about moving the pvr_power_is_idle() check into
> pvr_power_device_suspend() (and dropping pvr_power_device_idle()), and
> returning -EBUSY after pm_runtime_mark_last_busy() if the FW is busy?
> The PM core then reschedules the autosuspend regardless of whether the
> last put comes from the watchdog or elsewhere, so suspend is retried
> until the FW becomes idle.

Agreed, v2 will follow this approach.

> I think this is the pattern described in
> Documentation/power/runtime_pm.rst, section 9 ("If the
> ->runtime_suspend() callback returns -EAGAIN or -EBUSY ...").
> --
> Thanks,
> Gyeyoung

Thanks,
Baihui