diff mbox series

drm/msm/dp: call dp_display_get_next_bridge() during probe

Message ID 20231107004424.2112698-1-dmitry.baryshkov@linaro.org
State Accepted
Commit d4ca26ac4be0d9aea7005c40df75e6775749671b
Headers show
Series drm/msm/dp: call dp_display_get_next_bridge() during probe | expand

Commit Message

Dmitry Baryshkov Nov. 7, 2023, 12:43 a.m. UTC
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@linaro.org>
---

Dependencies: https://patchwork.freedesktop.org/series/120375/

---
 drivers/gpu/drm/msm/dp/dp_display.c | 36 +++++++++++++++++------------
 1 file changed, 21 insertions(+), 15 deletions(-)

Comments

Konrad Dybcio Dec. 11, 2023, 5:02 p.m. UTC | #1
On 7.11.2023 01:43, 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@linaro.org>
> ---
Tested-by: Konrad Dybcio <konrad.dybcio@linaro.org> # sc8180x-primus

Konrad
Kuogee Hsieh Dec. 11, 2023, 9:17 p.m. UTC | #2
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@linaro.org>
> ---
>
> Dependencies: https://patchwork.freedesktop.org/series/120375/
>
> ---

Reviewed-by: Kuogee Hsieh <quic_khsieh@quicinc.com>


>   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,
Dmitry Baryshkov Dec. 13, 2023, 11:11 p.m. UTC | #3
On Tue, 07 Nov 2023 02:43:33 +0200, 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.
> 
> 

Applied, thanks!

[1/1] drm/msm/dp: call dp_display_get_next_bridge() during probe
      https://gitlab.freedesktop.org/lumag/msm/-/commit/6a242720884f

Best regards,
diff mbox series

Patch

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,