Hi Baihui, Troy,
On Fri, Oct 30, 2026 at 10:55 AM Troy Mitchell <[email protected]> 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 <[email protected]> > > --- > > 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
