diff mbox series

[04/14] drm/bridge: analogix_dp: handle clock via runtime PM

Message ID 20240503151129.3901815-5-l.stach@pengutronix.de (mailing list archive)
State New, archived
Headers show
Series improve Analogix DP AUX channel handling | expand

Commit Message

Lucas Stach May 3, 2024, 3:11 p.m. UTC 
There is no reason to enable the controller clock in driver probe, as
there is no HW initialization done in this function. Instead rely on
either runtime PM to handle the controller clock or statically enable
it in the driver bind routine, after which real hardware access is
required to work.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 .../drm/bridge/analogix/analogix_dp_core.c    | 78 +++++++++++--------
 1 file changed, 45 insertions(+), 33 deletions(-)

Comments

Robert Foss May 7, 2024, 12:57 p.m. UTC | #1 
On Fri, May 3, 2024 at 5:12 PM Lucas Stach <l.stach@pengutronix.de> wrote:
>
> There is no reason to enable the controller clock in driver probe, as
> there is no HW initialization done in this function. Instead rely on
> either runtime PM to handle the controller clock or statically enable
> it in the driver bind routine, after which real hardware access is
> required to work.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  .../drm/bridge/analogix/analogix_dp_core.c    | 78 +++++++++++--------
>  1 file changed, 45 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 0af2a70ae5bf..9e3308257586 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1658,8 +1658,6 @@ analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data)
>                 return ERR_CAST(dp->clock);
>         }
>
> -       clk_prepare_enable(dp->clock);
> -
>         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>
>         dp->reg_base = devm_ioremap_resource(&pdev->dev, res);
> @@ -1721,6 +1719,29 @@ analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data)
>  }
>  EXPORT_SYMBOL_GPL(analogix_dp_probe);
>
> +
> +int analogix_dp_suspend(struct analogix_dp_device *dp)
> +{
> +       clk_disable_unprepare(dp->clock);
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(analogix_dp_suspend);
> +
> +int analogix_dp_resume(struct analogix_dp_device *dp)
> +{
> +       int ret;
> +
> +       ret = clk_prepare_enable(dp->clock);
> +       if (ret < 0) {
> +               DRM_ERROR("Failed to prepare_enable the clock clk [%d]\n", ret);
> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(analogix_dp_resume);
> +
>  int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev)
>  {
>         int ret;
> @@ -1728,9 +1749,15 @@ int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev)
>         dp->drm_dev = drm_dev;
>         dp->encoder = dp->plat_data->encoder;
>
> -       pm_runtime_use_autosuspend(dp->dev);
> -       pm_runtime_set_autosuspend_delay(dp->dev, 100);
> -       pm_runtime_enable(dp->dev);
> +       if (IS_ENABLED(CONFIG_PM)) {
> +               pm_runtime_use_autosuspend(dp->dev);
> +               pm_runtime_set_autosuspend_delay(dp->dev, 100);
> +               pm_runtime_enable(dp->dev);
> +       } else {
> +               ret = analogix_dp_resume(dp);
> +               if (ret)
> +                       return ret;
> +       }
>
>         dp->aux.name = "DP-AUX";
>         dp->aux.transfer = analogix_dpaux_transfer;
> @@ -1754,8 +1781,12 @@ int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev)
>  err_unregister_aux:
>         drm_dp_aux_unregister(&dp->aux);
>  err_disable_pm_runtime:
> -       pm_runtime_dont_use_autosuspend(dp->dev);
> -       pm_runtime_disable(dp->dev);
> +       if (IS_ENABLED(CONFIG_PM)) {
> +               pm_runtime_dont_use_autosuspend(dp->dev);
> +               pm_runtime_disable(dp->dev);
> +       } else {
> +               analogix_dp_suspend(dp);
> +       }
>
>         return ret;
>  }
> @@ -1772,40 +1803,21 @@ void analogix_dp_unbind(struct analogix_dp_device *dp)
>         }
>
>         drm_dp_aux_unregister(&dp->aux);
> -       pm_runtime_dont_use_autosuspend(dp->dev);
> -       pm_runtime_disable(dp->dev);
> +
> +       if (IS_ENABLED(CONFIG_PM)) {
> +               pm_runtime_dont_use_autosuspend(dp->dev);
> +               pm_runtime_disable(dp->dev);
> +       } else {
> +               analogix_dp_suspend(dp);
> +       }
>  }
>  EXPORT_SYMBOL_GPL(analogix_dp_unbind);
>
>  void analogix_dp_remove(struct analogix_dp_device *dp)
>  {
> -       clk_disable_unprepare(dp->clock);
>  }
>  EXPORT_SYMBOL_GPL(analogix_dp_remove);
>
> -#ifdef CONFIG_PM
> -int analogix_dp_suspend(struct analogix_dp_device *dp)
> -{
> -       clk_disable_unprepare(dp->clock);
> -       return 0;
> -}
> -EXPORT_SYMBOL_GPL(analogix_dp_suspend);
> -
> -int analogix_dp_resume(struct analogix_dp_device *dp)
> -{
> -       int ret;
> -
> -       ret = clk_prepare_enable(dp->clock);
> -       if (ret < 0) {
> -               DRM_ERROR("Failed to prepare_enable the clock clk [%d]\n", ret);
> -               return ret;
> -       }
> -
> -       return 0;
> -}
> -EXPORT_SYMBOL_GPL(analogix_dp_resume);
> -#endif
> -
>  int analogix_dp_start_crc(struct drm_connector *connector)
>  {
>         struct analogix_dp_device *dp = to_dp(connector);
> --
> 2.39.2
>

Reviewed-by: Robert Foss <rfoss@kernel.org>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 0af2a70ae5bf..9e3308257586 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1658,8 +1658,6 @@  analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data)
 		return ERR_CAST(dp->clock);
 	}
 
-	clk_prepare_enable(dp->clock);
-
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 
 	dp->reg_base = devm_ioremap_resource(&pdev->dev, res);
@@ -1721,6 +1719,29 @@  analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data)
 }
 EXPORT_SYMBOL_GPL(analogix_dp_probe);
 
+
+int analogix_dp_suspend(struct analogix_dp_device *dp)
+{
+	clk_disable_unprepare(dp->clock);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(analogix_dp_suspend);
+
+int analogix_dp_resume(struct analogix_dp_device *dp)
+{
+	int ret;
+
+	ret = clk_prepare_enable(dp->clock);
+	if (ret < 0) {
+		DRM_ERROR("Failed to prepare_enable the clock clk [%d]\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(analogix_dp_resume);
+
 int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev)
 {
 	int ret;
@@ -1728,9 +1749,15 @@  int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev)
 	dp->drm_dev = drm_dev;
 	dp->encoder = dp->plat_data->encoder;
 
-	pm_runtime_use_autosuspend(dp->dev);
-	pm_runtime_set_autosuspend_delay(dp->dev, 100);
-	pm_runtime_enable(dp->dev);
+	if (IS_ENABLED(CONFIG_PM)) {
+		pm_runtime_use_autosuspend(dp->dev);
+		pm_runtime_set_autosuspend_delay(dp->dev, 100);
+		pm_runtime_enable(dp->dev);
+	} else {
+		ret = analogix_dp_resume(dp);
+		if (ret)
+			return ret;
+	}
 
 	dp->aux.name = "DP-AUX";
 	dp->aux.transfer = analogix_dpaux_transfer;
@@ -1754,8 +1781,12 @@  int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev)
 err_unregister_aux:
 	drm_dp_aux_unregister(&dp->aux);
 err_disable_pm_runtime:
-	pm_runtime_dont_use_autosuspend(dp->dev);
-	pm_runtime_disable(dp->dev);
+	if (IS_ENABLED(CONFIG_PM)) {
+		pm_runtime_dont_use_autosuspend(dp->dev);
+		pm_runtime_disable(dp->dev);
+	} else {
+		analogix_dp_suspend(dp);
+	}
 
 	return ret;
 }
@@ -1772,40 +1803,21 @@  void analogix_dp_unbind(struct analogix_dp_device *dp)
 	}
 
 	drm_dp_aux_unregister(&dp->aux);
-	pm_runtime_dont_use_autosuspend(dp->dev);
-	pm_runtime_disable(dp->dev);
+
+	if (IS_ENABLED(CONFIG_PM)) {
+		pm_runtime_dont_use_autosuspend(dp->dev);
+		pm_runtime_disable(dp->dev);
+	} else {
+		analogix_dp_suspend(dp);
+	}
 }
 EXPORT_SYMBOL_GPL(analogix_dp_unbind);
 
 void analogix_dp_remove(struct analogix_dp_device *dp)
 {
-	clk_disable_unprepare(dp->clock);
 }
 EXPORT_SYMBOL_GPL(analogix_dp_remove);
 
-#ifdef CONFIG_PM
-int analogix_dp_suspend(struct analogix_dp_device *dp)
-{
-	clk_disable_unprepare(dp->clock);
-	return 0;
-}
-EXPORT_SYMBOL_GPL(analogix_dp_suspend);
-
-int analogix_dp_resume(struct analogix_dp_device *dp)
-{
-	int ret;
-
-	ret = clk_prepare_enable(dp->clock);
-	if (ret < 0) {
-		DRM_ERROR("Failed to prepare_enable the clock clk [%d]\n", ret);
-		return ret;
-	}
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(analogix_dp_resume);
-#endif
-
 int analogix_dp_start_crc(struct drm_connector *connector)
 {
 	struct analogix_dp_device *dp = to_dp(connector);