* [PATCH] drm/panfrost: fix autosuspend cleanup during teardown
@ 2026-08-08 13:50 Guangshuo Li
2026-08-08 14:11 ` sashiko-bot
2026-08-12 14:34 ` Adrián Larumbe
0 siblings, 2 replies; 5+ messages in thread
From: Guangshuo Li @ 2026-08-08 13:50 UTC (permalink / raw)
To: Boris Brezillon, Rob Herring, Steven Price, Adrián Larumbe,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Tomeu Vizoso, Eric Anholt, Marty E. Plummer,
dri-devel, linux-kernel
Cc: Guangshuo Li, stable
panfrost_probe() calls pm_runtime_use_autosuspend(), but neither the
probe error path nor panfrost_remove() calls the matching
pm_runtime_dont_use_autosuspend() during teardown.
If the autosuspend delay is set to a negative value while autosuspend
is enabled, the runtime PM core increments usage_count to prevent
runtime suspend. Without calling pm_runtime_dont_use_autosuspend()
during teardown, this reference is not dropped.
The documentation for pm_runtime_use_autosuspend() also notes that it
is important to undo it with pm_runtime_dont_use_autosuspend() at
driver exit time, unless runtime PM was initially enabled with
devm_pm_runtime_enable().
Add the missing pm_runtime_dont_use_autosuspend() calls to both the
probe failure and device removal paths.
This issue was found by manual code inspection.
Fixes: f3ba91228e8e ("drm/panfrost: Add initial panfrost driver")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
drivers/gpu/drm/panfrost/panfrost_drv.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 784e36d72c2b..007dcba62e62 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -1011,6 +1011,7 @@ static int panfrost_probe(struct platform_device *pdev)
err_out2:
drm_dev_unregister(&pfdev->base);
err_out1:
+ pm_runtime_dont_use_autosuspend(pfdev->base.dev);
pm_runtime_disable(pfdev->base.dev);
panfrost_device_fini(pfdev);
pm_runtime_set_suspended(pfdev->base.dev);
@@ -1025,6 +1026,7 @@ static void panfrost_remove(struct platform_device *pdev)
drm_dev_unregister(&pfdev->base);
panfrost_gem_shrinker_cleanup(&pfdev->base);
+ pm_runtime_dont_use_autosuspend(pfdev->base.dev);
pm_runtime_get_sync(pfdev->base.dev);
pm_runtime_disable(pfdev->base.dev);
panfrost_device_fini(pfdev);
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] drm/panfrost: fix autosuspend cleanup during teardown 2026-08-08 13:50 [PATCH] drm/panfrost: fix autosuspend cleanup during teardown Guangshuo Li @ 2026-08-08 14:11 ` sashiko-bot 2026-08-12 14:34 ` Adrián Larumbe 1 sibling, 0 replies; 5+ messages in thread From: sashiko-bot @ 2026-08-08 14:11 UTC (permalink / raw) To: Guangshuo Li; +Cc: dri-devel Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: New issues: - [Critical] Calling pm_runtime_dont_use_autosuspend() before pm_runtime_disable() in the probe error path triggers a synchronous runtime suspend, causing a fatal system crash when hardware is subsequently accessed. - [Medium] Calling `pm_runtime_dont_use_autosuspend()` before `pm_runtime_get_sync()` in `panfrost_remove()` causes an unnecessary synchronous suspend followed immediately by a resume. Pre-existing issues: - [High] Missing pm_runtime_put_noidle() at the end of panfrost_remove() permanently leaks the PM usage count on driver unbind. - [High] Premature drm_dev_register() exposes the device before initialization is complete, risking UAF/Data Abort on the error path. -- --- Patch [1]: [PATCH] drm/panfrost: fix autosuspend cleanup during teardown --- commit 20fb0c04bf706e7ac3773cb2ee067fc667a9f893 Author: Guangshuo Li <lgs201920130244@gmail.com> drm/panfrost: fix autosuspend cleanup during teardown This patch adds missing calls to pm_runtime_dont_use_autosuspend() during the probe error path and panfrost_remove(). It aims to ensure that the autosuspend reference count is properly dropped during device teardown. > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > @@ -1012,6 +1012,7 @@ static int panfrost_probe(struct platform_device *pdev) > err_out2: > drm_dev_unregister(&pfdev->base); [Severity: High] This is a pre-existing issue, but does registering the device before initializing the shrinker create a race condition here? If panfrost_gem_shrinker_init() fails, the probe aborts and calls drm_dev_unregister(), but this does not revoke open file descriptors. Can userspace race to open the device during this window and then trigger a use-after-free or data abort by accessing torn-down hardware mappings through the open file descriptor? > err_out1: > + pm_runtime_dont_use_autosuspend(pfdev->base.dev); [Severity: Critical] Does placing pm_runtime_dont_use_autosuspend() before pm_runtime_disable() trigger a synchronous suspend? When the usage count is zero, clearing the autosuspend flag causes an immediate idle check which synchronously powers off the hardware. Will the unconditional register accesses in panfrost_device_fini() then cause a fatal data abort since the hardware is already powered off? > pm_runtime_disable(pfdev->base.dev); > panfrost_device_fini(pfdev); [ ... ] > @@ -1026,6 +1027,7 @@ static void panfrost_remove(struct platform_device *pdev) > drm_dev_unregister(&pfdev->base); > panfrost_gem_shrinker_cleanup(&pfdev->base); > > + pm_runtime_dont_use_autosuspend(pfdev->base.dev); > pm_runtime_get_sync(pfdev->base.dev); [Severity: Medium] Could this sequence cause the hardware to suspend and immediately resume? If the usage count is zero here, pm_runtime_dont_use_autosuspend() will synchronously power off the device. The subsequent pm_runtime_get_sync() will immediately power it back on before hardware teardown. Should the autosuspend cleanup happen after we ensure the device is awake? > pm_runtime_disable(pfdev->base.dev); > panfrost_device_fini(pfdev); [Severity: High] This isn't a bug introduced by this patch, but does this function leak the power management usage count? The call to pm_runtime_get_sync() earlier in the function increments the usage count, but there is no matching pm_runtime_put_noidle() before the function exits. Since the struct device is tied to the platform device and outlives the driver binding, will this permanently break runtime power management by blocking future suspends if the driver is unbound and rebound? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260808135025.2872956-1-lgs201920130244@gmail.com?part=1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/panfrost: fix autosuspend cleanup during teardown 2026-08-08 13:50 [PATCH] drm/panfrost: fix autosuspend cleanup during teardown Guangshuo Li 2026-08-08 14:11 ` sashiko-bot @ 2026-08-12 14:34 ` Adrián Larumbe 2026-08-13 2:10 ` Guangshuo Li 1 sibling, 1 reply; 5+ messages in thread From: Adrián Larumbe @ 2026-08-12 14:34 UTC (permalink / raw) To: Guangshuo Li Cc: Boris Brezillon, Rob Herring, Steven Price, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Tomeu Vizoso, Eric Anholt, Marty E. Plummer, dri-devel, linux-kernel, stable Hi Guangshuo, Recently I've been working on a patch series that addresses PM refcnt imbalances in Panfrost. One of the fixes introduces devm_pm_runtime_enable(), so manually calling pm_runtime_dont_use_autosuspend() is no longer necessary like you mentioned in the commit message. However, there might be a point in using it during device teardown and probe error just like Panthor does, becasue it doesn't make sense to postpone suspension when the device is going away. Anyway, you can have a look at it here: https://lore.kernel.org/r/20260811-claude-fixes-v5-0-3d692c9e98c2@collabora.com I was thinking, I'll let you know when I've v6 ready and then you could rebase this patch onto it before I submit the series to the ML so that I can send them all together? Kind Regards, Adrian On 08.08.2026 21:50, Guangshuo Li wrote: > panfrost_probe() calls pm_runtime_use_autosuspend(), but neither the > probe error path nor panfrost_remove() calls the matching > pm_runtime_dont_use_autosuspend() during teardown. > > If the autosuspend delay is set to a negative value while autosuspend > is enabled, the runtime PM core increments usage_count to prevent > runtime suspend. Without calling pm_runtime_dont_use_autosuspend() > during teardown, this reference is not dropped. > > The documentation for pm_runtime_use_autosuspend() also notes that it > is important to undo it with pm_runtime_dont_use_autosuspend() at > driver exit time, unless runtime PM was initially enabled with > devm_pm_runtime_enable(). > > Add the missing pm_runtime_dont_use_autosuspend() calls to both the > probe failure and device removal paths. > > This issue was found by manual code inspection. > > Fixes: f3ba91228e8e ("drm/panfrost: Add initial panfrost driver") > Cc: stable@vger.kernel.org > Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com> > --- > drivers/gpu/drm/panfrost/panfrost_drv.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > index 784e36d72c2b..007dcba62e62 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > @@ -1011,6 +1011,7 @@ static int panfrost_probe(struct platform_device *pdev) > err_out2: > drm_dev_unregister(&pfdev->base); > err_out1: > + pm_runtime_dont_use_autosuspend(pfdev->base.dev); > pm_runtime_disable(pfdev->base.dev); > panfrost_device_fini(pfdev); > pm_runtime_set_suspended(pfdev->base.dev); > @@ -1025,6 +1026,7 @@ static void panfrost_remove(struct platform_device *pdev) > drm_dev_unregister(&pfdev->base); > panfrost_gem_shrinker_cleanup(&pfdev->base); > > + pm_runtime_dont_use_autosuspend(pfdev->base.dev); > pm_runtime_get_sync(pfdev->base.dev); > pm_runtime_disable(pfdev->base.dev); > panfrost_device_fini(pfdev); > -- > 2.43.0 Adrian Larumbe ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/panfrost: fix autosuspend cleanup during teardown 2026-08-12 14:34 ` Adrián Larumbe @ 2026-08-13 2:10 ` Guangshuo Li 2026-09-11 23:33 ` Adrián Larumbe 0 siblings, 1 reply; 5+ messages in thread From: Guangshuo Li @ 2026-08-13 2:10 UTC (permalink / raw) To: Adrián Larumbe Cc: Boris Brezillon, Rob Herring, Steven Price, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Tomeu Vizoso, Eric Anholt, Marty E. Plummer, dri-devel, linux-kernel, stable Hi Adrian, On Wed, 12 Aug 2026 at 22:35, Adrián Larumbe <adrian.larumbe@collabora.com> wrote: > > Hi Guangshuo, > > Recently I've been working on a patch series that addresses PM refcnt imbalances in Panfrost. > One of the fixes introduces devm_pm_runtime_enable(), so manually calling pm_runtime_dont_use_autosuspend() > is no longer necessary like you mentioned in the commit message. > > However, there might be a point in using it during device teardown and probe error just like Panthor does, > becasue it doesn't make sense to postpone suspension when the device is going away. > > Anyway, you can have a look at it here: > https://lore.kernel.org/r/20260811-claude-fixes-v5-0-3d692c9e98c2@collabora.com > > I was thinking, I'll let you know when I've v6 ready and then you could rebase this patch onto it > before I submit the series to the ML so that I can send them all together? > > Kind Regards, > Adrian > > On 08.08.2026 21:50, Guangshuo Li wrote: > > panfrost_probe() calls pm_runtime_use_autosuspend(), but neither the > > probe error path nor panfrost_remove() calls the matching > > pm_runtime_dont_use_autosuspend() during teardown. > > > > If the autosuspend delay is set to a negative value while autosuspend > > is enabled, the runtime PM core increments usage_count to prevent > > runtime suspend. Without calling pm_runtime_dont_use_autosuspend() > > during teardown, this reference is not dropped. > > > > The documentation for pm_runtime_use_autosuspend() also notes that it > > is important to undo it with pm_runtime_dont_use_autosuspend() at > > driver exit time, unless runtime PM was initially enabled with > > devm_pm_runtime_enable(). > > > > Add the missing pm_runtime_dont_use_autosuspend() calls to both the > > probe failure and device removal paths. > > > > This issue was found by manual code inspection. > > > > Fixes: f3ba91228e8e ("drm/panfrost: Add initial panfrost driver") > > Cc: stable@vger.kernel.org > > Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com> > > --- > > drivers/gpu/drm/panfrost/panfrost_drv.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > > index 784e36d72c2b..007dcba62e62 100644 > > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > > @@ -1011,6 +1011,7 @@ static int panfrost_probe(struct platform_device *pdev) > > err_out2: > > drm_dev_unregister(&pfdev->base); > > err_out1: > > + pm_runtime_dont_use_autosuspend(pfdev->base.dev); > > pm_runtime_disable(pfdev->base.dev); > > panfrost_device_fini(pfdev); > > pm_runtime_set_suspended(pfdev->base.dev); > > @@ -1025,6 +1026,7 @@ static void panfrost_remove(struct platform_device *pdev) > > drm_dev_unregister(&pfdev->base); > > panfrost_gem_shrinker_cleanup(&pfdev->base); > > > > + pm_runtime_dont_use_autosuspend(pfdev->base.dev); > > pm_runtime_get_sync(pfdev->base.dev); > > pm_runtime_disable(pfdev->base.dev); > > panfrost_device_fini(pfdev); > > -- > > 2.43.0 > > Adrian Larumbe Sure, I’d be glad to do that. Please let me know when v6 is ready, and I’ll rebase my patch on top of it. Best regards, Guangshuo ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/panfrost: fix autosuspend cleanup during teardown 2026-08-13 2:10 ` Guangshuo Li @ 2026-09-11 23:33 ` Adrián Larumbe 0 siblings, 0 replies; 5+ messages in thread From: Adrián Larumbe @ 2026-09-11 23:33 UTC (permalink / raw) To: Guangshuo Li Cc: Boris Brezillon, Rob Herring, Steven Price, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Tomeu Vizoso, Eric Anholt, Marty E. Plummer, dri-devel, linux-kernel, stable Hi Guangshou, I'm afraid in the end I can't apply this patch because the series I wrote, besides including this fix, has quite a few others to deal with existing issues with power management in the driver. It felt awkard to apply this one in isolation without addresing the other bugs at the same time. Check https://lore.kernel.org/r/20260912-claude-fixes-v9-0-e588feaa61ef@collabora.com for further context, patch 6/16 to be more precise. However, like I said in a previous message, I'll soon be compiling a list of existing issues and post them on a Freedesktop Gitlab issue for anyone curious to have a go at. Cheers, Adrian On 13.08.2026 10:10, Guangshuo Li wrote: > Hi Adrian, > > On Wed, 12 Aug 2026 at 22:35, Adrián Larumbe > <adrian.larumbe@collabora.com> wrote: > > > > Hi Guangshuo, > > > > Recently I've been working on a patch series that addresses PM refcnt imbalances in Panfrost. > > One of the fixes introduces devm_pm_runtime_enable(), so manually calling pm_runtime_dont_use_autosuspend() > > is no longer necessary like you mentioned in the commit message. > > > > However, there might be a point in using it during device teardown and probe error just like Panthor does, > > becasue it doesn't make sense to postpone suspension when the device is going away. > > > > Anyway, you can have a look at it here: > > https://lore.kernel.org/r/20260811-claude-fixes-v5-0-3d692c9e98c2@collabora.com > > > > I was thinking, I'll let you know when I've v6 ready and then you could rebase this patch onto it > > before I submit the series to the ML so that I can send them all together? > > > > Kind Regards, > > Adrian > > > > On 08.08.2026 21:50, Guangshuo Li wrote: > > > panfrost_probe() calls pm_runtime_use_autosuspend(), but neither the > > > probe error path nor panfrost_remove() calls the matching > > > pm_runtime_dont_use_autosuspend() during teardown. > > > > > > If the autosuspend delay is set to a negative value while autosuspend > > > is enabled, the runtime PM core increments usage_count to prevent > > > runtime suspend. Without calling pm_runtime_dont_use_autosuspend() > > > during teardown, this reference is not dropped. > > > > > > The documentation for pm_runtime_use_autosuspend() also notes that it > > > is important to undo it with pm_runtime_dont_use_autosuspend() at > > > driver exit time, unless runtime PM was initially enabled with > > > devm_pm_runtime_enable(). > > > > > > Add the missing pm_runtime_dont_use_autosuspend() calls to both the > > > probe failure and device removal paths. > > > > > > This issue was found by manual code inspection. > > > > > > Fixes: f3ba91228e8e ("drm/panfrost: Add initial panfrost driver") > > > Cc: stable@vger.kernel.org > > > Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com> > > > --- > > > drivers/gpu/drm/panfrost/panfrost_drv.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > > > index 784e36d72c2b..007dcba62e62 100644 > > > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > > > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > > > @@ -1011,6 +1011,7 @@ static int panfrost_probe(struct platform_device *pdev) > > > err_out2: > > > drm_dev_unregister(&pfdev->base); > > > err_out1: > > > + pm_runtime_dont_use_autosuspend(pfdev->base.dev); > > > pm_runtime_disable(pfdev->base.dev); > > > panfrost_device_fini(pfdev); > > > pm_runtime_set_suspended(pfdev->base.dev); > > > @@ -1025,6 +1026,7 @@ static void panfrost_remove(struct platform_device *pdev) > > > drm_dev_unregister(&pfdev->base); > > > panfrost_gem_shrinker_cleanup(&pfdev->base); > > > > > > + pm_runtime_dont_use_autosuspend(pfdev->base.dev); > > > pm_runtime_get_sync(pfdev->base.dev); > > > pm_runtime_disable(pfdev->base.dev); > > > panfrost_device_fini(pfdev); > > > -- > > > 2.43.0 > > > > Adrian Larumbe > > Sure, I’d be glad to do that. Please let me know when v6 is ready, and > I’ll rebase my patch on top of it. > > Best regards, > Guangshuo Adrian Larumbe ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-09-11 23:33 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-08 13:50 [PATCH] drm/panfrost: fix autosuspend cleanup during teardown Guangshuo Li 2026-08-08 14:11 ` sashiko-bot 2026-08-12 14:34 ` Adrián Larumbe 2026-08-13 2:10 ` Guangshuo Li 2026-09-11 23:33 ` Adrián Larumbe
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.