Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] The DW DP core driver uses `guard(pm_runtime_noresume)` in its 
interrupt handler, allowing hardware register accesses while the device is 
suspended and its clocks are disabled.
--

--- Patch [12]: [PATCH v6 12/16] drm/rockchip: dw_dp: Add runtime PM support ---
commit 76fb970e196b210c9742e601b51113d5f65dd02f
Author: Sebastian Reichel <[email protected]>

drm/rockchip: dw_dp: Add runtime PM support

This commit adds 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 ee0bd696064eb..b8138e796c4d5 100644
> --- a/drivers/gpu/drm/rockchip/dw_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_dp-rockchip.c

[ ... ]

> @@ -247,6 +260,12 @@ static int dw_dp_rockchip_probe(struct platform_device 
> *pdev)
>       if (IS_ERR(dp->base))
>               return PTR_ERR(dp->base);
>  
> +     pm_runtime_use_autosuspend(dev);
> +     pm_runtime_set_autosuspend_delay(dev, 500);
> +     ret = devm_pm_runtime_enable(dev);

[Severity: High]
Does enabling runtime PM here expose a bus fault in the core dw_dp_irq
handler on suspended devices?

The core DW DP driver uses guard(pm_runtime_noresume) which only increments
the PM usage counter without checking the actual power state:

drivers/gpu/drm/bridge/synopsys/dw-dp.c:dw_dp_irq() {
        /* interrupt can only trigger for running device */
        guard(pm_runtime_noresume)(dp->dev);

        regmap_read(dp->regmap, DW_DP_GENERAL_INTERRUPT, &value);
        ...
}

If a spurious or delayed interrupt fires while the DP controller is
suspended and its clocks are disabled, this allows the handler to execute
and read from DW_DP_GENERAL_INTERRUPT.

Accessing memory-mapped registers over a disabled APB bus can cause a bus
fault (such as an SError on ARM) resulting in a kernel panic.

Should the core driver be updated to use pm_runtime_get_if_active()
to safely verify the device is active before accessing hardware registers?

> +     if (ret)
> +             return ret;
> +
>       return component_add(&pdev->dev, &dw_dp_rockchip_component_ops);
>  }

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/20260724-synopsys-dw-dp-improvements-v6-0-041d99a19...@collabora.com?part=12

Reply via email to