[PATCH v2 0/2] drm: rcar-du: mipi-dsi: Cleanup and Fixes

[PATCH v2 0/2] drm: rcar-du: mipi-dsi: Cleanup and Fixes

[Kieran Bingham]

Following on from the earlier version of this series, is an updated
patch to move the bridge registration from rcar_mipi_dsi_probe() to
rcar_mipi_dsi_host_attach() followed by an immediate update to that code
to refactor it to use the new devm_drm_of_get_bridge helper.

These two patches are kept distinct to support review, and are both
expected to be squashed directly into the still-to-be-posted update of
the DSI driver by Laurent [0].

[0] https://lore.kernel.org/all/20210623135639.17125-1-laurent.pinchart+renesas@ideasonboard.com/

The previous patches 1/4, 2/4 and 3/4 from the v1 of this series have
already been accepted by Laurent and squashed into his tree, so are not
reposted.

Kieran Bingham (2):
  drm: rcar-du: mipi-dsi: Support bridge probe ordering
  drm: rcar-du: mipi-dsi: Use devm_drm_of_get_bridge helper

 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 38 ++++++++++---------------
 1 file changed, 15 insertions(+), 23 deletions(-)

-- 
2.30.2

[PATCH v2 1/2] drm: rcar-du: mipi-dsi: Support bridge probe ordering

[Kieran Bingham]

The bridge probe ordering for DSI devices has been clarified and further
documented in

To support connecting with the SN65DSI86 device after commit c3b75d4734cb
("drm/bridge: sn65dsi86: Register and attach our DSI device at probe"),
update to the new probe ordering to remove a perpetual -EPROBE_DEFER
loop between the two devices.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

---
v2
- Remove now unused panel variable from rcar_mipi_dsi_probe()

 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 49 +++++++++++++------------
 1 file changed, 26 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
index 50e922328fed..0a9f197ef62c 100644
--- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
+++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
@@ -637,6 +637,8 @@ static int rcar_mipi_dsi_host_attach(struct mipi_dsi_host *host,
 					struct mipi_dsi_device *device)
 {
 	struct rcar_mipi_dsi *dsi = host_to_rcar_mipi_dsi(host);
+	struct drm_panel *panel;
+	int ret;
 
 	if (device->lanes > dsi->num_data_lanes)
 		return -EINVAL;
@@ -644,12 +646,36 @@ static int rcar_mipi_dsi_host_attach(struct mipi_dsi_host *host,
 	dsi->lanes = device->lanes;
 	dsi->format = device->format;
 
+	ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 1, 0, &panel,
+					  &dsi->next_bridge);
+	if (ret) {
+		dev_err_probe(dsi->dev, ret, "could not find next bridge\n");
+		return ret;
+	}
+
+	if (!dsi->next_bridge) {
+		dsi->next_bridge = devm_drm_panel_bridge_add(dsi->dev, panel);
+		if (IS_ERR(dsi->next_bridge)) {
+			dev_err(dsi->dev, "failed to create panel bridge\n");
+			return PTR_ERR(dsi->next_bridge);
+		}
+	}
+
+	/* Initialize the DRM bridge. */
+	dsi->bridge.funcs = &rcar_mipi_dsi_bridge_ops;
+	dsi->bridge.of_node = dsi->dev->of_node;
+	drm_bridge_add(&dsi->bridge);
+
 	return 0;
 }
 
 static int rcar_mipi_dsi_host_detach(struct mipi_dsi_host *host,
 					struct mipi_dsi_device *device)
 {
+	struct rcar_mipi_dsi *dsi = host_to_rcar_mipi_dsi(host);
+
+	drm_bridge_remove(&dsi->bridge);
+
 	return 0;
 }
 
@@ -731,7 +757,6 @@ static int rcar_mipi_dsi_get_clocks(struct rcar_mipi_dsi *dsi)
 static int rcar_mipi_dsi_probe(struct platform_device *pdev)
 {
 	struct rcar_mipi_dsi *dsi;
-	struct drm_panel *panel;
 	struct resource *mem;
 	int ret;
 
@@ -764,21 +789,6 @@ static int rcar_mipi_dsi_probe(struct platform_device *pdev)
 		return PTR_ERR(dsi->rstc);
 	}
 
-	ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 1, 0, &panel,
-					  &dsi->next_bridge);
-	if (ret) {
-		dev_err_probe(dsi->dev, ret, "could not find next bridge\n");
-		return ret;
-	}
-
-	if (!dsi->next_bridge) {
-		dsi->next_bridge = devm_drm_panel_bridge_add(dsi->dev, panel);
-		if (IS_ERR(dsi->next_bridge)) {
-			dev_err(dsi->dev, "failed to create panel bridge\n");
-			return PTR_ERR(dsi->next_bridge);
-		}
-	}
-
 	/* Initialize the DSI host. */
 	dsi->host.dev = dsi->dev;
 	dsi->host.ops = &rcar_mipi_dsi_host_ops;
@@ -786,11 +796,6 @@ static int rcar_mipi_dsi_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
-	/* Initialize the DRM bridge. */
-	dsi->bridge.funcs = &rcar_mipi_dsi_bridge_ops;
-	dsi->bridge.of_node = dsi->dev->of_node;
-	drm_bridge_add(&dsi->bridge);
-
 	return 0;
 }
 
@@ -798,8 +803,6 @@ static int rcar_mipi_dsi_remove(struct platform_device *pdev)
 {
 	struct rcar_mipi_dsi *dsi = platform_get_drvdata(pdev);
 
-	drm_bridge_remove(&dsi->bridge);
-
 	mipi_dsi_host_unregister(&dsi->host);
 
 	return 0;
-- 
2.30.2

[PATCH v2 2/2] drm: rcar-du: mipi-dsi: Use devm_drm_of_get_bridge helper

[Kieran Bingham]

Instead of open coding the calls for
  drm_of_find_panel_or_bridge()
  devm_drm_panel_bridge_add()

use the devm_drm_of_get_bridge() helper directly.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
v2:
 - New patch

 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
index 0a9f197ef62c..1dfe20d3d0f2 100644
--- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
+++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
@@ -637,7 +637,7 @@ static int rcar_mipi_dsi_host_attach(struct mipi_dsi_host *host,
 					struct mipi_dsi_device *device)
 {
 	struct rcar_mipi_dsi *dsi = host_to_rcar_mipi_dsi(host);
-	struct drm_panel *panel;
+	struct device *dev = dsi->dev;
 	int ret;
 
 	if (device->lanes > dsi->num_data_lanes)
@@ -646,20 +646,9 @@ static int rcar_mipi_dsi_host_attach(struct mipi_dsi_host *host,
 	dsi->lanes = device->lanes;
 	dsi->format = device->format;
 
-	ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 1, 0, &panel,
-					  &dsi->next_bridge);
-	if (ret) {
-		dev_err_probe(dsi->dev, ret, "could not find next bridge\n");
-		return ret;
-	}
-
-	if (!dsi->next_bridge) {
-		dsi->next_bridge = devm_drm_panel_bridge_add(dsi->dev, panel);
-		if (IS_ERR(dsi->next_bridge)) {
-			dev_err(dsi->dev, "failed to create panel bridge\n");
-			return PTR_ERR(dsi->next_bridge);
-		}
-	}
+	dsi->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
+	if (IS_ERR(dsi->next_bridge))
+		return PTR_ERR(dsi->next_bridge);
 
 	/* Initialize the DRM bridge. */
 	dsi->bridge.funcs = &rcar_mipi_dsi_bridge_ops;
-- 
2.30.2

Re: [PATCH v2 2/2] drm: rcar-du: mipi-dsi: Use devm_drm_of_get_bridge helper

[Kieran Bingham]

Hi Laurent,

Quoting Kieran Bingham (2021-11-30 16:25:13)
> Instead of open coding the calls for
>   drm_of_find_panel_or_bridge()
>   devm_drm_panel_bridge_add()
> 
> use the devm_drm_of_get_bridge() helper directly.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
> v2:
>  - New patch
> 
>  drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 19 ++++---------------
>  1 file changed, 4 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> index 0a9f197ef62c..1dfe20d3d0f2 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> @@ -637,7 +637,7 @@ static int rcar_mipi_dsi_host_attach(struct mipi_dsi_host *host,
>                                         struct mipi_dsi_device *device)
>  {
>         struct rcar_mipi_dsi *dsi = host_to_rcar_mipi_dsi(host);
> -       struct drm_panel *panel;
> +       struct device *dev = dsi->dev;
>         int ret;
>  
>         if (device->lanes > dsi->num_data_lanes)
> @@ -646,20 +646,9 @@ static int rcar_mipi_dsi_host_attach(struct mipi_dsi_host *host,
>         dsi->lanes = device->lanes;
>         dsi->format = device->format;
>  
> -       ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 1, 0, &panel,
> -                                         &dsi->next_bridge);
> -       if (ret) {
> -               dev_err_probe(dsi->dev, ret, "could not find next bridge\n");
> -               return ret;
> -       }
> -
> -       if (!dsi->next_bridge) {
> -               dsi->next_bridge = devm_drm_panel_bridge_add(dsi->dev, panel);
> -               if (IS_ERR(dsi->next_bridge)) {
> -                       dev_err(dsi->dev, "failed to create panel bridge\n");
> -                       return PTR_ERR(dsi->next_bridge);
> -               }
> -       }
> +       dsi->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
> +       if (IS_ERR(dsi->next_bridge))
> +               return PTR_ERR(dsi->next_bridge);

I did make a change here to make this:

	dsi->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
	if (IS_ERR(dsi->next_bridge)) {
		dev_err(dev, "failed to get next bridge\n");
		return PTR_ERR(dsi->next_bridge);
	}

But it seems I got out of sequence and saved out the wrong patch ;-(

If you think it's better with the error print, please add it while
squashing, or if you really need, I can send an updated patch and
retest.

--
Kieran


>  
>         /* Initialize the DRM bridge. */
>         dsi->bridge.funcs = &rcar_mipi_dsi_bridge_ops;
> -- 
> 2.30.2
>

Re: [PATCH v2 1/2] drm: rcar-du: mipi-dsi: Support bridge probe ordering

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Tue, Nov 30, 2021 at 04:25:12PM +0000, Kieran Bingham wrote:
> The bridge probe ordering for DSI devices has been clarified and further
> documented in

I've read the document and


:-)

> To support connecting with the SN65DSI86 device after commit c3b75d4734cb
> ("drm/bridge: sn65dsi86: Register and attach our DSI device at probe"),
> update to the new probe ordering to remove a perpetual -EPROBE_DEFER
> loop between the two devices.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> ---
> v2
> - Remove now unused panel variable from rcar_mipi_dsi_probe()
> 
>  drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 49 +++++++++++++------------
>  1 file changed, 26 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> index 50e922328fed..0a9f197ef62c 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> @@ -637,6 +637,8 @@ static int rcar_mipi_dsi_host_attach(struct mipi_dsi_host *host,
>  					struct mipi_dsi_device *device)
>  {
>  	struct rcar_mipi_dsi *dsi = host_to_rcar_mipi_dsi(host);
> +	struct drm_panel *panel;
> +	int ret;
>  
>  	if (device->lanes > dsi->num_data_lanes)
>  		return -EINVAL;
> @@ -644,12 +646,36 @@ static int rcar_mipi_dsi_host_attach(struct mipi_dsi_host *host,
>  	dsi->lanes = device->lanes;
>  	dsi->format = device->format;
>  
> +	ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 1, 0, &panel,
> +					  &dsi->next_bridge);
> +	if (ret) {
> +		dev_err_probe(dsi->dev, ret, "could not find next bridge\n");

dev_err_probe() should only be used in probe(), and this function isn't
guaranteed to be called at probe time only.

This isn't a big deal as the next patch fixes this, and both will be
squashed. Furthermore, rcar_mipi_dsi_host_attach() should only be called
when the DSI device gets registered, which should happen after it
registers its bridge, so I don't think we can see a probe deferral here.

Other than that the patch looks fine, I'll squash it with the DSI
driver.

> +		return ret;
> +	}
> +
> +	if (!dsi->next_bridge) {
> +		dsi->next_bridge = devm_drm_panel_bridge_add(dsi->dev, panel);
> +		if (IS_ERR(dsi->next_bridge)) {
> +			dev_err(dsi->dev, "failed to create panel bridge\n");
> +			return PTR_ERR(dsi->next_bridge);
> +		}
> +	}
> +
> +	/* Initialize the DRM bridge. */
> +	dsi->bridge.funcs = &rcar_mipi_dsi_bridge_ops;
> +	dsi->bridge.of_node = dsi->dev->of_node;
> +	drm_bridge_add(&dsi->bridge);
> +
>  	return 0;
>  }
>  
>  static int rcar_mipi_dsi_host_detach(struct mipi_dsi_host *host,
>  					struct mipi_dsi_device *device)
>  {
> +	struct rcar_mipi_dsi *dsi = host_to_rcar_mipi_dsi(host);
> +
> +	drm_bridge_remove(&dsi->bridge);
> +
>  	return 0;
>  }
>  
> @@ -731,7 +757,6 @@ static int rcar_mipi_dsi_get_clocks(struct rcar_mipi_dsi *dsi)
>  static int rcar_mipi_dsi_probe(struct platform_device *pdev)
>  {
>  	struct rcar_mipi_dsi *dsi;
> -	struct drm_panel *panel;
>  	struct resource *mem;
>  	int ret;
>  
> @@ -764,21 +789,6 @@ static int rcar_mipi_dsi_probe(struct platform_device *pdev)
>  		return PTR_ERR(dsi->rstc);
>  	}
>  
> -	ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 1, 0, &panel,
> -					  &dsi->next_bridge);
> -	if (ret) {
> -		dev_err_probe(dsi->dev, ret, "could not find next bridge\n");
> -		return ret;
> -	}
> -
> -	if (!dsi->next_bridge) {
> -		dsi->next_bridge = devm_drm_panel_bridge_add(dsi->dev, panel);
> -		if (IS_ERR(dsi->next_bridge)) {
> -			dev_err(dsi->dev, "failed to create panel bridge\n");
> -			return PTR_ERR(dsi->next_bridge);
> -		}
> -	}
> -
>  	/* Initialize the DSI host. */
>  	dsi->host.dev = dsi->dev;
>  	dsi->host.ops = &rcar_mipi_dsi_host_ops;
> @@ -786,11 +796,6 @@ static int rcar_mipi_dsi_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		return ret;
>  
> -	/* Initialize the DRM bridge. */
> -	dsi->bridge.funcs = &rcar_mipi_dsi_bridge_ops;
> -	dsi->bridge.of_node = dsi->dev->of_node;
> -	drm_bridge_add(&dsi->bridge);
> -
>  	return 0;
>  }
>  
> @@ -798,8 +803,6 @@ static int rcar_mipi_dsi_remove(struct platform_device *pdev)
>  {
>  	struct rcar_mipi_dsi *dsi = platform_get_drvdata(pdev);
>  
> -	drm_bridge_remove(&dsi->bridge);
> -
>  	mipi_dsi_host_unregister(&dsi->host);
>  
>  	return 0;

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 2/2] drm: rcar-du: mipi-dsi: Use devm_drm_of_get_bridge helper

[Laurent Pinchart]

Hi Kieran,

On Tue, Nov 30, 2021 at 04:52:19PM +0000, Kieran Bingham wrote:
> Quoting Kieran Bingham (2021-11-30 16:25:13)
> > Instead of open coding the calls for
> >   drm_of_find_panel_or_bridge()
> >   devm_drm_panel_bridge_add()
> > 
> > use the devm_drm_of_get_bridge() helper directly.
> > 
> > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> > ---
> > v2:
> >  - New patch
> > 
> >  drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 19 ++++---------------
> >  1 file changed, 4 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> > index 0a9f197ef62c..1dfe20d3d0f2 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> > @@ -637,7 +637,7 @@ static int rcar_mipi_dsi_host_attach(struct mipi_dsi_host *host,
> >                                         struct mipi_dsi_device *device)
> >  {
> >         struct rcar_mipi_dsi *dsi = host_to_rcar_mipi_dsi(host);
> > -       struct drm_panel *panel;
> > +       struct device *dev = dsi->dev;
> >         int ret;
> >  
> >         if (device->lanes > dsi->num_data_lanes)
> > @@ -646,20 +646,9 @@ static int rcar_mipi_dsi_host_attach(struct mipi_dsi_host *host,
> >         dsi->lanes = device->lanes;
> >         dsi->format = device->format;
> >  
> > -       ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 1, 0, &panel,
> > -                                         &dsi->next_bridge);
> > -       if (ret) {
> > -               dev_err_probe(dsi->dev, ret, "could not find next bridge\n");
> > -               return ret;
> > -       }
> > -
> > -       if (!dsi->next_bridge) {
> > -               dsi->next_bridge = devm_drm_panel_bridge_add(dsi->dev, panel);
> > -               if (IS_ERR(dsi->next_bridge)) {
> > -                       dev_err(dsi->dev, "failed to create panel bridge\n");
> > -                       return PTR_ERR(dsi->next_bridge);
> > -               }
> > -       }
> > +       dsi->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
> > +       if (IS_ERR(dsi->next_bridge))
> > +               return PTR_ERR(dsi->next_bridge);
> 
> I did make a change here to make this:
> 
> 	dsi->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
> 	if (IS_ERR(dsi->next_bridge)) {
> 		dev_err(dev, "failed to get next bridge\n");
> 		return PTR_ERR(dsi->next_bridge);
> 	}
> 
> But it seems I got out of sequence and saved out the wrong patch ;-(
> 
> If you think it's better with the error print, please add it while
> squashing, or if you really need, I can send an updated patch and
> retest.

I think an error message is useful, yes. I'll add it manually.

> >  
> >         /* Initialize the DRM bridge. */
> >         dsi->bridge.funcs = &rcar_mipi_dsi_bridge_ops;

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 2/2] drm: rcar-du: mipi-dsi: Use devm_drm_of_get_bridge helper

[Laurent Pinchart]

Hi Kieran,

On Tue, Nov 30, 2021 at 04:52:19PM +0000, Kieran Bingham wrote:
> Quoting Kieran Bingham (2021-11-30 16:25:13)
> > Instead of open coding the calls for
> >   drm_of_find_panel_or_bridge()
> >   devm_drm_panel_bridge_add()
> > 
> > use the devm_drm_of_get_bridge() helper directly.
> > 
> > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> > ---
> > v2:
> >  - New patch
> > 
> >  drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 19 ++++---------------
> >  1 file changed, 4 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> > index 0a9f197ef62c..1dfe20d3d0f2 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> > @@ -637,7 +637,7 @@ static int rcar_mipi_dsi_host_attach(struct mipi_dsi_host *host,
> >                                         struct mipi_dsi_device *device)
> >  {
> >         struct rcar_mipi_dsi *dsi = host_to_rcar_mipi_dsi(host);
> > -       struct drm_panel *panel;
> > +       struct device *dev = dsi->dev;
> >         int ret;
> >  
> >         if (device->lanes > dsi->num_data_lanes)
> > @@ -646,20 +646,9 @@ static int rcar_mipi_dsi_host_attach(struct mipi_dsi_host *host,
> >         dsi->lanes = device->lanes;
> >         dsi->format = device->format;
> >  
> > -       ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 1, 0, &panel,
> > -                                         &dsi->next_bridge);
> > -       if (ret) {
> > -               dev_err_probe(dsi->dev, ret, "could not find next bridge\n");
> > -               return ret;
> > -       }
> > -
> > -       if (!dsi->next_bridge) {
> > -               dsi->next_bridge = devm_drm_panel_bridge_add(dsi->dev, panel);
> > -               if (IS_ERR(dsi->next_bridge)) {
> > -                       dev_err(dsi->dev, "failed to create panel bridge\n");
> > -                       return PTR_ERR(dsi->next_bridge);
> > -               }
> > -       }
> > +       dsi->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
> > +       if (IS_ERR(dsi->next_bridge))
> > +               return PTR_ERR(dsi->next_bridge);
> 
> I did make a change here to make this:
> 
> 	dsi->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
> 	if (IS_ERR(dsi->next_bridge)) {
> 		dev_err(dev, "failed to get next bridge\n");
> 		return PTR_ERR(dsi->next_bridge);
> 	}
> 
> But it seems I got out of sequence and saved out the wrong patch ;-(
> 
> If you think it's better with the error print, please add it while
> squashing, or if you really need, I can send an updated patch and
> retest.

I think an error message is useful, yes. I'll add it manually.

> >  
> >         /* Initialize the DRM bridge. */
> >         dsi->bridge.funcs = &rcar_mipi_dsi_bridge_ops;

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 1/2] drm: rcar-du: mipi-dsi: Support bridge probe ordering

[Kieran Bingham]

Quoting Laurent Pinchart (2021-12-01 04:38:46)
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Tue, Nov 30, 2021 at 04:25:12PM +0000, Kieran Bingham wrote:
> > The bridge probe ordering for DSI devices has been clarified and further
> > documented in
> 
> I've read the document and
> 

Good, I'm glad you've fully got the

> 
> :-)

Sorry, not sure how I missed that, I must have failed to paste in the
link to the DSI probe documentation. But as you've read that, and now
this is squashed, I think this is done anyway.

Thanks for handling the integrations.

--
Kieran


> 
> > To support connecting with the SN65DSI86 device after commit c3b75d4734cb
> > ("drm/bridge: sn65dsi86: Register and attach our DSI device at probe"),
> > update to the new probe ordering to remove a perpetual -EPROBE_DEFER
> > loop between the two devices.
> > 
> > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> > 
> > ---
> > v2
> > - Remove now unused panel variable from rcar_mipi_dsi_probe()
> > 
> >  drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 49 +++++++++++++------------
> >  1 file changed, 26 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> > index 50e922328fed..0a9f197ef62c 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> > @@ -637,6 +637,8 @@ static int rcar_mipi_dsi_host_attach(struct mipi_dsi_host *host,
> >                                       struct mipi_dsi_device *device)
> >  {
> >       struct rcar_mipi_dsi *dsi = host_to_rcar_mipi_dsi(host);
> > +     struct drm_panel *panel;
> > +     int ret;
> >  
> >       if (device->lanes > dsi->num_data_lanes)
> >               return -EINVAL;
> > @@ -644,12 +646,36 @@ static int rcar_mipi_dsi_host_attach(struct mipi_dsi_host *host,
> >       dsi->lanes = device->lanes;
> >       dsi->format = device->format;
> >  
> > +     ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 1, 0, &panel,
> > +                                       &dsi->next_bridge);
> > +     if (ret) {
> > +             dev_err_probe(dsi->dev, ret, "could not find next bridge\n");
> 
> dev_err_probe() should only be used in probe(), and this function isn't
> guaranteed to be called at probe time only.
> 
> This isn't a big deal as the next patch fixes this, and both will be
> squashed. Furthermore, rcar_mipi_dsi_host_attach() should only be called
> when the DSI device gets registered, which should happen after it
> registers its bridge, so I don't think we can see a probe deferral here.
> 
> Other than that the patch looks fine, I'll squash it with the DSI
> driver.
> 
> > +             return ret;
> > +     }
> > +
> > +     if (!dsi->next_bridge) {
> > +             dsi->next_bridge = devm_drm_panel_bridge_add(dsi->dev, panel);
> > +             if (IS_ERR(dsi->next_bridge)) {
> > +                     dev_err(dsi->dev, "failed to create panel bridge\n");
> > +                     return PTR_ERR(dsi->next_bridge);
> > +             }
> > +     }
> > +
> > +     /* Initialize the DRM bridge. */
> > +     dsi->bridge.funcs = &rcar_mipi_dsi_bridge_ops;
> > +     dsi->bridge.of_node = dsi->dev->of_node;
> > +     drm_bridge_add(&dsi->bridge);
> > +
> >       return 0;
> >  }
> >  
> >  static int rcar_mipi_dsi_host_detach(struct mipi_dsi_host *host,
> >                                       struct mipi_dsi_device *device)
> >  {
> > +     struct rcar_mipi_dsi *dsi = host_to_rcar_mipi_dsi(host);
> > +
> > +     drm_bridge_remove(&dsi->bridge);
> > +
> >       return 0;
> >  }
> >  
> > @@ -731,7 +757,6 @@ static int rcar_mipi_dsi_get_clocks(struct rcar_mipi_dsi *dsi)
> >  static int rcar_mipi_dsi_probe(struct platform_device *pdev)
> >  {
> >       struct rcar_mipi_dsi *dsi;
> > -     struct drm_panel *panel;
> >       struct resource *mem;
> >       int ret;
> >  
> > @@ -764,21 +789,6 @@ static int rcar_mipi_dsi_probe(struct platform_device *pdev)
> >               return PTR_ERR(dsi->rstc);
> >       }
> >  
> > -     ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 1, 0, &panel,
> > -                                       &dsi->next_bridge);
> > -     if (ret) {
> > -             dev_err_probe(dsi->dev, ret, "could not find next bridge\n");
> > -             return ret;
> > -     }
> > -
> > -     if (!dsi->next_bridge) {
> > -             dsi->next_bridge = devm_drm_panel_bridge_add(dsi->dev, panel);
> > -             if (IS_ERR(dsi->next_bridge)) {
> > -                     dev_err(dsi->dev, "failed to create panel bridge\n");
> > -                     return PTR_ERR(dsi->next_bridge);
> > -             }
> > -     }
> > -
> >       /* Initialize the DSI host. */
> >       dsi->host.dev = dsi->dev;
> >       dsi->host.ops = &rcar_mipi_dsi_host_ops;
> > @@ -786,11 +796,6 @@ static int rcar_mipi_dsi_probe(struct platform_device *pdev)
> >       if (ret < 0)
> >               return ret;
> >  
> > -     /* Initialize the DRM bridge. */
> > -     dsi->bridge.funcs = &rcar_mipi_dsi_bridge_ops;
> > -     dsi->bridge.of_node = dsi->dev->of_node;
> > -     drm_bridge_add(&dsi->bridge);
> > -
> >       return 0;
> >  }
> >  
> > @@ -798,8 +803,6 @@ static int rcar_mipi_dsi_remove(struct platform_device *pdev)
> >  {
> >       struct rcar_mipi_dsi *dsi = platform_get_drvdata(pdev);
> >  
> > -     drm_bridge_remove(&dsi->bridge);
> > -
> >       mipi_dsi_host_unregister(&dsi->host);
> >  
> >       return 0;
> 
> -- 
> Regards,
> 
> Laurent Pinchart