Re: [PATCH 03/14] drm/bridge: analogix_dp: register AUX bus after enabling runtime PM

[Robert Foss]

On Fri, May 3, 2024 at 5:12 PM Lucas Stach  wrote:
>
> AUX transactions require the controller to be in working state and
> take a runtime PM reference. To avoid potential races beween the
> first transactions on the bus and runtime PM being set up, move the
> AUX registration behind the runtime PM setup.
>
> Signed-off-by: Lucas Stach 
> ---
>  .../drm/bridge/analogix/analogix_dp_core.c| 19 +++
>  1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c 
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index b39721588980..0af2a70ae5bf 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1728,31 +1728,34 @@ 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);
> +
> dp->aux.name = "DP-AUX";
> dp->aux.transfer = analogix_dpaux_transfer;
> dp->aux.dev = dp->dev;
> dp->aux.drm_dev = drm_dev;
>
> ret = drm_dp_aux_register(>aux);
> -   if (ret)
> -   return ret;
> -
> -   pm_runtime_use_autosuspend(dp->dev);
> -   pm_runtime_set_autosuspend_delay(dp->dev, 100);
> -   pm_runtime_enable(dp->dev);
> +   if (ret) {
> +   DRM_ERROR("failed to register AUX (%d)\n", ret);
> +   goto err_disable_pm_runtime;
> +   }
>
> ret = analogix_dp_create_bridge(drm_dev, dp);
> if (ret) {
> DRM_ERROR("failed to create bridge (%d)\n", ret);
> -   goto err_disable_pm_runtime;
> +   goto err_unregister_aux;
> }
>
> return 0;
>
> +err_unregister_aux:
> +   drm_dp_aux_unregister(>aux);
>  err_disable_pm_runtime:
> pm_runtime_dont_use_autosuspend(dp->dev);
> pm_runtime_disable(dp->dev);
> -   drm_dp_aux_unregister(>aux);
>
> return ret;
>  }
> --
> 2.39.2
>

Reviewed-by: Robert Foss 

[PATCH 03/14] drm/bridge: analogix_dp: register AUX bus after enabling runtime PM

[Lucas Stach]

AUX transactions require the controller to be in working state and
take a runtime PM reference. To avoid potential races beween the
first transactions on the bus and runtime PM being set up, move the
AUX registration behind the runtime PM setup.

Signed-off-by: Lucas Stach 
---
 .../drm/bridge/analogix/analogix_dp_core.c| 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c 
b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index b39721588980..0af2a70ae5bf 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1728,31 +1728,34 @@ 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);
+
dp->aux.name = "DP-AUX";
dp->aux.transfer = analogix_dpaux_transfer;
dp->aux.dev = dp->dev;
dp->aux.drm_dev = drm_dev;
 
ret = drm_dp_aux_register(>aux);
-   if (ret)
-   return ret;
-
-   pm_runtime_use_autosuspend(dp->dev);
-   pm_runtime_set_autosuspend_delay(dp->dev, 100);
-   pm_runtime_enable(dp->dev);
+   if (ret) {
+   DRM_ERROR("failed to register AUX (%d)\n", ret);
+   goto err_disable_pm_runtime;
+   }
 
ret = analogix_dp_create_bridge(drm_dev, dp);
if (ret) {
DRM_ERROR("failed to create bridge (%d)\n", ret);
-   goto err_disable_pm_runtime;
+   goto err_unregister_aux;
}
 
return 0;
 
+err_unregister_aux:
+   drm_dp_aux_unregister(>aux);
 err_disable_pm_runtime:
pm_runtime_dont_use_autosuspend(dp->dev);
pm_runtime_disable(dp->dev);
-   drm_dp_aux_unregister(>aux);
 
return ret;
 }
-- 
2.39.2