[PATCH] media: rkvdec: Do not keep the clocks enabled from probe

[PATCH] media: rkvdec: Do not keep the clocks enabled from probe

[Igor Velkov via B4 Relay]

From: Igor Velkov <iav@iav.lv>

Since commit 6a846f7d72c7 ("media: rkvdec: Enable all clocks without
naming them") the clocks are fetched with devm_clk_bulk_get_all_enabled(),
which keeps them enabled from probe until unbind. rkvdec_runtime_suspend()
disables them on top of that, so their enable count never drops to zero
and the clock gates stay open while the decoder is idle.

On RK3399 this also breaks restore from hibernation when the boot kernel
has not enabled these clocks, for example with rkvdec built as a module:
the gates are closed when the restored kernel takes over, while it still
counts the clocks as enabled. Powering on the vdu power domain then finds
the count at 1 and leaves the gates closed, and the first QoS register
write of the domain never returns.

Get the clocks without enabling them and let runtime PM enable them while
the decoder is in use, as before that commit.

Fixes: 6a846f7d72c7 ("media: rkvdec: Enable all clocks without naming them")
Cc: stable@vger.kernel.org # v7.0+
Signed-off-by: Igor Velkov <iav@iav.lv>
---
Tested on ROCK Pi 4A (RK3399) with v7.3, rkvdec and panfrost built as
modules, panfrost not loaded:
- idle decoder: the vdu clocks have enable_count 0 (1 without the patch);
- hibernate, power cycle, restore: 3 of 3 (0 of 3 without the patch, the
  restored kernel stops after bringing the secondary CPUs up);
- H.264 decode with GStreamer v4l2slh264dec, 300 frames of 720p High
  profile: output identical to avdec_h264 (md5 of all frames).
The QoS hang was traced with debug prints in rockchip_pd_power() on a
7.3-based kernel.

This is separate from the unbind leak in Jonas's patch that Francesco
posted [1]: here the extra enable comes from probe.

[1] https://lore.kernel.org/all/20260717154505.83935-1-pavone.lawyer@gmail.com/
---
 drivers/media/platform/rockchip/rkvdec/rkvdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/rockchip/rkvdec/rkvdec.c b/drivers/media/platform/rockchip/rkvdec/rkvdec.c
index 061281f903f3..45c541ae3029 100644
--- a/drivers/media/platform/rockchip/rkvdec/rkvdec.c
+++ b/drivers/media/platform/rockchip/rkvdec/rkvdec.c
@@ -1788,7 +1788,7 @@ static int rkvdec_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ret = devm_clk_bulk_get_all_enabled(&pdev->dev, &rkvdec->clocks);
+	ret = devm_clk_bulk_get_all(&pdev->dev, &rkvdec->clocks);
 	if (ret < 0)
 		return ret;
 

---
base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222
change-id: 20261030-rkvdec-runtime-pm-clocks-41adcf141271

Best regards,
-- 
Igor Velkov <iav@iav.lv>

Re: [PATCH] media: rkvdec: Do not keep the clocks enabled from probe

[Nicolas Dufresne]

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

Le vendredi 30 octobre 2026 à 03:33 +0200, Igor Velkov via B4 Relay a écrit :
> From: Igor Velkov <iav@iav.lv>
> 
> Since commit 6a846f7d72c7 ("media: rkvdec: Enable all clocks without
> naming them") the clocks are fetched with devm_clk_bulk_get_all_enabled(),
> which keeps them enabled from probe until unbind. rkvdec_runtime_suspend()
> disables them on top of that, so their enable count never drops to zero
> and the clock gates stay open while the decoder is idle.
> 
> On RK3399 this also breaks restore from hibernation when the boot kernel
> has not enabled these clocks, for example with rkvdec built as a module:
> the gates are closed when the restored kernel takes over, while it still
> counts the clocks as enabled. Powering on the vdu power domain then finds
> the count at 1 and leaves the gates closed, and the first QoS register
> write of the domain never returns.
> 
> Get the clocks without enabling them and let runtime PM enable them while
> the decoder is in use, as before that commit.
> 
> Fixes: 6a846f7d72c7 ("media: rkvdec: Enable all clocks without naming them")
> Cc: stable@vger.kernel.org # v7.0+
> Signed-off-by: Igor Velkov <iav@iav.lv>
> ---
> Tested on ROCK Pi 4A (RK3399) with v7.3, rkvdec and panfrost built as
> modules, panfrost not loaded:
> - idle decoder: the vdu clocks have enable_count 0 (1 without the patch);
> - hibernate, power cycle, restore: 3 of 3 (0 of 3 without the patch, the
>   restored kernel stops after bringing the secondary CPUs up);
> - H.264 decode with GStreamer v4l2slh264dec, 300 frames of 720p High
>   profile: output identical to avdec_h264 (md5 of all frames).
> The QoS hang was traced with debug prints in rockchip_pd_power() on a
> 7.3-based kernel.
> 
> This is separate from the unbind leak in Jonas's patch that Francesco
> posted [1]: here the extra enable comes from probe.
> 
> [1] https://lore.kernel.org/all/20260717154505.83935-1-pavone.lawyer@gmail.com/
> ---
>  drivers/media/platform/rockchip/rkvdec/rkvdec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/rockchip/rkvdec/rkvdec.c b/drivers/media/platform/rockchip/rkvdec/rkvdec.c
> index 061281f903f3..45c541ae3029 100644
> --- a/drivers/media/platform/rockchip/rkvdec/rkvdec.c
> +++ b/drivers/media/platform/rockchip/rkvdec/rkvdec.c
> @@ -1788,7 +1788,7 @@ static int rkvdec_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> -	ret = devm_clk_bulk_get_all_enabled(&pdev->dev, &rkvdec->clocks);
> +	ret = devm_clk_bulk_get_all(&pdev->dev, &rkvdec->clocks);

With the explanation above, it seems pretty clear we have an issue. Now, the
problem with this is that PM ops are set with:

  #define pm_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM), (_ptr))

So if CONFIG_PM is not set, the initial resume will never occur, and first mmio
access will likely hang the CPU. We need a clean pattern to handle this, and
perhaps review other similar drivers while at it.

Nicolas

>  	if (ret < 0)
>  		return ret;
>  
> 
> ---
> base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222
> change-id: 20261030-rkvdec-runtime-pm-clocks-41adcf141271
> 
> Best regards,

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]