Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] The driver implements runtime PM support but lacks system sleep callbacks, leaving the hardware fully powered on during system suspend. - [Medium] Using asynchronous autosuspend before devres teardown on driver removal causes pending suspends to be cancelled, permanently leaking hardware clocks. --
--- Patch [15]: [PATCH v11 15/21] drm/rockchip: dw_dp: Add runtime PM support --- commit ee7c08af7fc5043079230468fab4131c048586ea Author: Sebastian Reichel <[email protected]> drm/rockchip: dw_dp: Add runtime PM support Add support for runtime PM to the Rockchip RK3576/3588 Synopsys DesignWare DisplayPort driver. > diff --git a/drivers/gpu/drm/rockchip/dw_dp-rockchip.c > b/drivers/gpu/drm/rockchip/dw_dp-rockchip.c > index ffcfb887d0d2a..770ab042a1879 100644 > --- a/drivers/gpu/drm/rockchip/dw_dp-rockchip.c > +++ b/drivers/gpu/drm/rockchip/dw_dp-rockchip.c [ ... ] > @@ -231,6 +232,7 @@ static int dw_dp_rockchip_probe(struct platform_device > *pdev) > if (IS_ERR(dp->vo_grf)) > return PTR_ERR(dp->vo_grf); > > + plat_data->autosuspend_delay = 500; > plat_data->max_link_rate = plat_data_const->max_link_rate; > plat_data->pixel_mode = plat_data_const->pixel_mode; > plat_data->hpd_sw_sel = dw_dp_rockchip_hpd_sw_sel; > @@ -253,6 +255,24 @@ static void dw_dp_rockchip_remove(struct platform_device > *pdev) > component_del(&pdev->dev, &dw_dp_rockchip_component_ops); > } [Severity: Medium] When the driver is removed, component_del() unbinds the DRM pipeline, queuing an async autosuspend with the 500ms delay set in dw_dp_rockchip_probe(). Because dw_dp_rockchip_remove() returns immediately, devres teardown will call pm_runtime_disable() which cancels the pending autosuspend. Does this bypass dw_dp_runtime_suspend() and permanently leak the apb_clk and aux_clk clocks? Should there be a synchronous suspend here? > +static int dw_dp_rockchip_runtime_suspend(struct device *dev) > +{ > + struct rockchip_dw_dp *dp = dev_get_drvdata(dev); > + > + return dw_dp_runtime_suspend(dp->base); > +} > + > +static int dw_dp_rockchip_runtime_resume(struct device *dev) > +{ > + struct rockchip_dw_dp *dp = dev_get_drvdata(dev); > + > + return dw_dp_runtime_resume(dp->base); > +} > + > +static const struct dev_pm_ops dw_dp_pm_ops = { > + RUNTIME_PM_OPS(dw_dp_rockchip_runtime_suspend, > dw_dp_rockchip_runtime_resume, NULL) > +}; [Severity: Medium] The dev_pm_ops structure uses RUNTIME_PM_OPS but entirely omits SYSTEM_SLEEP_PM_OPS. During system suspend, pm_runtime_disable() cancels pending autosuspend timers. Without system sleep callbacks (like pm_runtime_force_suspend), will the hardware remain in RPM_ACTIVE with clocks running throughout system sleep? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260806-synopsys-dw-dp-improvements-v11-0-0d508505f...@collabora.com?part=15
