Re: [PATCH v4 05/10] drm/rockchip: analogix_dp: Enable hclk for RK3588

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added `hclk` clock is explicitly kept permanently enabled, bypassing the driver's dynamic power management.
--

commit 55f729190dc3e9f8e6e9a2cd8960832ed10a25ba
Author: Damon Ding <damon.ding@rock-chips.com>

drm/rockchip: analogix_dp: Enable hclk for RK3588

This commit explicitly acquires and enables the HCLK_VO1 bus clock for the
RK3588 eDP controller. It replaces an implicit enablement via the
rockchip,vo-grf phandle reference to align with updated device tree bindings.

> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index 06072efd7fca3..d2af5eb29dbb5 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c

[ ... ]

> @@ -327,6 +328,11 @@ static int rockchip_dp_of_probe(struct rockchip_dp_device *dp)
>  		return dev_err_probe(dev, PTR_ERR(dp->pclk),
>  				     "failed to get pclk property\n");
>  
> +	clk = devm_clk_get_optional_enabled(dev, "hclk");
> +	if (IS_ERR(clk))
> +		return dev_err_probe(dev, PTR_ERR(clk),
> +				     "failed to get hclk property\n");
> +

Does keeping this clock permanently enabled bypass the driver's dynamic power
management?

Since the local variable clk is discarded after probe, the clock will
remain enabled as long as the driver is bound. The driver currently toggles
the APB clock (dp->pclk) in rockchip_dp_poweron() and rockchip_dp_powerdown()
to save power when the display is inactive.

Should hclk be saved in the rockchip_dp_device struct and managed
symmetrically with pclk to ensure associated power domains can fully enter
low-power states during display blanking?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512095644.1946084-1-damon.ding@rock-chips.com?part=5