On Mon, Mar 18, 2024 at 09:58:55AM +0100, Boris Brezillon wrote:
> Putting a hard dependency on CONFIG_PM is not possible because of a
> circular dependency issue, and it's actually not desirable either. In
> order to support this use case, we forcibly resume at init time, and
> suspend at unplug time.
>
> Reported-by: kernel test robot <l...@intel.com>
> Closes:
> https://lore.kernel.org/oe-kbuild-all/202403031944.eoimq8wk-...@intel.com/
> Signed-off-by: Boris Brezillon <boris.brezil...@collabora.com>
> ---
> Tested by faking CONFIG_PM=n in the driver (basically commenting
> all pm_runtime calls, and making the panthor_device_suspend/resume()
> calls unconditional in the panthor_device_unplug/init() path) since
> CONFIG_ARCH_ROCKCHIP selects CONFIG_PM. Seems to work fine, but I
> can't be 100% sure this will work correctly on a platform that has
> CONFIG_PM=n.
Yes, there are no realistic platforms to test where you can enable panthor
but disable runtime power management. All our emulator configuration are
based on Juno (ARCH_VEXPRESS) which select CONFIG_PM.
Reviewed-by: Liviu Dudau <liviu.du...@arm.com>
Best regards,
Liviu
> ---
> drivers/gpu/drm/panthor/panthor_device.c | 13 +++++++++++--
> drivers/gpu/drm/panthor/panthor_drv.c | 4 +++-
> 2 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_device.c
> b/drivers/gpu/drm/panthor/panthor_device.c
> index 69deb8e17778..ba7aedbb4931 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.c
> +++ b/drivers/gpu/drm/panthor/panthor_device.c
> @@ -87,6 +87,10 @@ void panthor_device_unplug(struct panthor_device *ptdev)
> pm_runtime_dont_use_autosuspend(ptdev->base.dev);
> pm_runtime_put_sync_suspend(ptdev->base.dev);
>
> + /* If PM is disabled, we need to call the suspend handler manually. */
> + if (!IS_ENABLED(CONFIG_PM))
> + panthor_device_suspend(ptdev->base.dev);
> +
> /* Report the unplug operation as done to unblock concurrent
> * panthor_device_unplug() callers.
> */
> @@ -218,6 +222,13 @@ int panthor_device_init(struct panthor_device *ptdev)
> if (ret)
> return ret;
>
> + /* If PM is disabled, we need to call panthor_device_resume() manually.
> */
> + if (!IS_ENABLED(CONFIG_PM)) {
> + ret = panthor_device_resume(ptdev->base.dev);
> + if (ret)
> + return ret;
> + }
> +
> ret = panthor_gpu_init(ptdev);
> if (ret)
> goto err_rpm_put;
> @@ -402,7 +413,6 @@ int panthor_device_mmap_io(struct panthor_device *ptdev,
> struct vm_area_struct *
> return 0;
> }
>
> -#ifdef CONFIG_PM
> int panthor_device_resume(struct device *dev)
> {
> struct panthor_device *ptdev = dev_get_drvdata(dev);
> @@ -547,4 +557,3 @@ int panthor_device_suspend(struct device *dev)
> mutex_unlock(&ptdev->pm.mmio_lock);
> return ret;
> }
> -#endif
> diff --git a/drivers/gpu/drm/panthor/panthor_drv.c
> b/drivers/gpu/drm/panthor/panthor_drv.c
> index ff484506229f..2ea6a9f436db 100644
> --- a/drivers/gpu/drm/panthor/panthor_drv.c
> +++ b/drivers/gpu/drm/panthor/panthor_drv.c
> @@ -1407,17 +1407,19 @@ static const struct of_device_id dt_match[] = {
> };
> MODULE_DEVICE_TABLE(of, dt_match);
>
> +#ifdef CONFIG_PM
> static DEFINE_RUNTIME_DEV_PM_OPS(panthor_pm_ops,
> panthor_device_suspend,
> panthor_device_resume,
> NULL);
> +#endif
>
> static struct platform_driver panthor_driver = {
> .probe = panthor_probe,
> .remove_new = panthor_remove,
> .driver = {
> .name = "panthor",
> - .pm = &panthor_pm_ops,
> + .pm = pm_ptr(&panthor_pm_ops),
> .of_match_table = dt_match,
> },
> };
> --
> 2.43.0
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
