On Fri, 1 Oct 2021 at 16:29, Dmitry Osipenko digetx@gmail.com wrote:
01.10.2021 16:39, Ulf Hansson пишет:
On Mon, 27 Sept 2021 at 00:42, Dmitry Osipenko digetx@gmail.com wrote:
Add runtime power management and support generic power domains.
Tested-by: Peter Geis pgwipeout@gmail.com # Ouya T30 Tested-by: Paul Fertser fercerpav@gmail.com # PAZ00 T20 Tested-by: Nicolas Chauvet kwizart@gmail.com # PAZ00 T20 and TK1 T124 Tested-by: Matt Merhar mattmerhar@protonmail.com # Ouya T30 Signed-off-by: Dmitry Osipenko digetx@gmail.com
drivers/gpu/drm/tegra/gr2d.c | 155 +++++++++++++++++++++++++++++++++--
[...]
static int gr2d_remove(struct platform_device *pdev) @@ -259,15 +312,101 @@ static int gr2d_remove(struct platform_device *pdev) return err; }
pm_runtime_dont_use_autosuspend(&pdev->dev);pm_runtime_disable(&pdev->dev);There is no guarantee that the ->runtime_suspend() has been invoked here, which means that clock may be left prepared/enabled beyond this point.
I suggest you call pm_runtime_force_suspend(), instead of pm_runtime_disable(), to make sure that gets done.
The pm_runtime_disable() performs the final synchronization, please see [1].
[1] https://elixir.bootlin.com/linux/v5.15-rc3/source/drivers/base/power/runtime...
pm_runtime_disable() end up calling _pm_runtime_barrier(), which calls cancel_work_sync() if dev->power.request_pending has been set.
If the work that was punted to the pm_wq in rpm_idle() has not been started yet, we end up just canceling it. In other words, there are no guarantees it runs to completion.
Moreover, use space may have bumped the usage count via sysfs for the device (pm_runtime_forbid()) to keep the device runtime resumed.
Calling pm_runtime_force_suspend() isn't correct because each 'enable' must have the corresponding 'disable'. Hence there is no problem here.
pm_runtime_force_suspend() calls pm_runtime_disable(), so I think that should be fine. No?
Kind regards Uffe