[PATCH] drm/lcdif: Do not disable clock on already suspended hardware

[Marek Vasut]

On 1/18/24 19:39, Marek Vasut wrote:
> In case the LCDIF is enabled in DT but unused, the clock used by the
> LCDIF are not enabled. Those clock may even have a use count of 0 in
> case there are no other users of those clock. This can happen e.g. in
> case the LCDIF drives HDMI bridge which has no panel plugged into the
> HDMI connector.
> 
> Do not attempt to disable clock in the suspend callback and re-enable
> clock in the resume callback unless the LCDIF is enabled and was in
> use before the system entered suspend, otherwise the driver might end
> up trying to disable clock which are already disabled with use count
> 0, and would trigger a warning from clock core about this condition.
> 
> Note that the lcdif_rpm_suspend() and lcdif_rpm_resume() functions
> internally perform the clock disable and enable operations and act
> as runtime PM hooks too.
> 
> Fixes: 9db35bb349a0 ("drm: lcdif: Add support for i.MX8MP LCDIF variant")
> Signed-off-by: Marek Vasut <marex at denx.de>
> ---
> Cc: Daniel Vetter <daniel at ffwll.ch>
> Cc: David Airlie <airlied at gmail.com>
> Cc: Fabio Estevam <festevam at gmail.com>
> Cc: Liu Ying <victor.liu at nxp.com>
> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> Cc: Maxime Ripard <mripard at kernel.org>
> Cc: NXP Linux Team <linux-imx at nxp.com>
> Cc: Pengutronix Kernel Team <kernel at pengutronix.de>
> Cc: Sascha Hauer <s.hauer at pengutronix.de>
> Cc: Shawn Guo <shawnguo at kernel.org>
> Cc: Stefan Agner <stefan at agner.ch>
> Cc: Thomas Zimmermann <tzimmermann at suse.de>
> Cc: dri-devel at lists.freedesktop.org
> Cc: linux-arm-kernel at lists.infradead.org
> ---
>   drivers/gpu/drm/mxsfb/lcdif_drv.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c
> index ea10bf81582e9..0f895b8a99d62 100644
> --- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
> +++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
> @@ -343,6 +343,9 @@ static int __maybe_unused lcdif_suspend(struct device *dev)
>   	if (ret)
>   		return ret;
>   
> +	if (pm_runtime_suspended(dev))
> +		return 0;
> +
>   	return lcdif_rpm_suspend(dev);
>   }
>   
> @@ -350,7 +353,8 @@ static int __maybe_unused lcdif_resume(struct device *dev)
>   {
>   	struct drm_device *drm = dev_get_drvdata(dev);
>   
> -	lcdif_rpm_resume(dev);
> +	if (!pm_runtime_suspended(dev))
> +		lcdif_rpm_resume(dev);
>   
>   	return drm_mode_config_helper_resume(drm);
>   }

Is this OK to pick up ? Some AB/RB/input would be appreciated, thanks !