Re: [PATCH] drm/msm/dp: call dp_display_get_next_bridge() during probe

[Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx>]

On 11/6/2023 4:43 PM, Dmitry Baryshkov wrote:
> The funcion dp_display_get_next_bridge() can return -EPROBE_DEFER if the
> next bridge is not (yet) available. However returning -EPROBE_DEFER from
> msm_dp_modeset_init() is not ideal. This leads to -EPROBE return from
> component_bind, which can easily result in -EPROBE_DEFR loops.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> ---
>
> Dependencies: https://patchwork.freedesktop.org/series/120375/
>
> ---

Reviewed-by: Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx>


>   drivers/gpu/drm/msm/dp/dp_display.c | 36 +++++++++++++++++------------
>   1 file changed, 21 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index d542db37763a..ddb3c84f39a2 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1197,15 +1197,27 @@ static const struct msm_dp_desc *dp_display_get_desc(struct platform_device *pde
>   	return NULL;
>   }
>   
> -static int dp_auxbus_done_probe(struct drm_dp_aux *aux)
> +static int dp_display_get_next_bridge(struct msm_dp *dp);
> +
> +static int dp_display_probe_tail(struct device *dev)
>   {
> -	int rc;
> +	struct msm_dp *dp = dev_get_drvdata(dev);
> +	int ret;
>   
> -	rc = component_add(aux->dev, &dp_display_comp_ops);
> -	if (rc)
> -		DRM_ERROR("eDP component add failed, rc=%d\n", rc);
> +	ret = dp_display_get_next_bridge(dp);
> +	if (ret)
> +		return ret;
>   
> -	return rc;
> +	ret = component_add(dev, &dp_display_comp_ops);
> +	if (ret)
> +		DRM_ERROR("component add failed, rc=%d\n", ret);
> +
> +	return ret;
> +}
> +
> +static int dp_auxbus_done_probe(struct drm_dp_aux *aux)
> +{
> +	return dp_display_probe_tail(aux->dev);
>   }
>   
>   static int dp_display_probe(struct platform_device *pdev)
> @@ -1280,11 +1292,9 @@ static int dp_display_probe(struct platform_device *pdev)
>   			goto err;
>   		}
>   	} else {
> -		rc = component_add(&pdev->dev, &dp_display_comp_ops);
> -		if (rc) {
> -			DRM_ERROR("component add failed, rc=%d\n", rc);
> +		rc = dp_display_probe_tail(&pdev->dev);
> +		if (rc)
>   			goto err;
> -		}
>   	}
>   
>   	return rc;
> @@ -1415,7 +1425,7 @@ static int dp_display_get_next_bridge(struct msm_dp *dp)
>   	 * For DisplayPort interfaces external bridges are optional, so
>   	 * silently ignore an error if one is not present (-ENODEV).
>   	 */
> -	rc = devm_dp_parser_find_next_bridge(dp->drm_dev->dev, dp_priv->parser);
> +	rc = devm_dp_parser_find_next_bridge(&dp->pdev->dev, dp_priv->parser);
>   	if (!dp->is_edp && rc == -ENODEV)
>   		return 0;
>   
> @@ -1435,10 +1445,6 @@ int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
>   
>   	dp_priv = container_of(dp_display, struct dp_display_private, dp_display);
>   
> -	ret = dp_display_get_next_bridge(dp_display);
> -	if (ret)
> -		return ret;
> -
>   	ret = dp_bridge_init(dp_display, dev, encoder);
>   	if (ret) {
>   		DRM_DEV_ERROR(dev->dev,