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

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

[Kieran Bingham]

These patches are against the as-yet-unmerged posting from Laurent of
"drm: rcar-du: Add R-Car DSI driver" from [0]

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

Notably, the comments blocking the integration of the series on the
handling of bridge probe ordering have hopefully been solved by 4/4
here. This has been validated and is working on the Falcon-V3U with an
SN65DSI86 on a merge of the latest drm/drm-next and
drm-misc/drm-misc-next.

Other updates to the SN65DSI86 were also used to support the bridge
connecting to non-eDP, and have previously been posted by Laurent [1].
I've made no specific changes to those, so I'll refrain from reposting
them.

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

Kieran Bingham (4):
  drm: rcar-du: Fix Makefile indentation for DSI
  drm: rcar-du: Select DRM_MIPI_DSI with DRM_RCAR_MIPI_DSI
  drm: rcar-du: mipi-dsi: Ensure correct fout is reported
  drm: rcar-du: mipi-dsi: Support bridge probe ordering

 drivers/gpu/drm/rcar-du/Kconfig         |  1 +
 drivers/gpu/drm/rcar-du/Makefile        |  2 +-
 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 52 ++++++++++++++-----------
 3 files changed, 31 insertions(+), 24 deletions(-)

-- 
2.30.2

[PATCH 1/4] drm: rcar-du: Fix Makefile indentation for DSI

[Kieran Bingham]

From: Kieran Bingham <kieran.bingham@ideasonboard.com>

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile
index adc1b49d02cf..286bc81b3e7c 100644
--- a/drivers/gpu/drm/rcar-du/Makefile
+++ b/drivers/gpu/drm/rcar-du/Makefile
@@ -19,7 +19,7 @@ obj-$(CONFIG_DRM_RCAR_CMM)		+= rcar_cmm.o
 obj-$(CONFIG_DRM_RCAR_DU)		+= rcar-du-drm.o
 obj-$(CONFIG_DRM_RCAR_DW_HDMI)		+= rcar_dw_hdmi.o
 obj-$(CONFIG_DRM_RCAR_LVDS)		+= rcar_lvds.o
-obj-$(CONFIG_DRM_RCAR_MIPI_DSI)	+= rcar_mipi_dsi.o
+obj-$(CONFIG_DRM_RCAR_MIPI_DSI)		+= rcar_mipi_dsi.o
 
 # 'remote-endpoint' is fixed up at run-time
 DTC_FLAGS_rcar_du_of_lvds_r8a7790 += -Wno-graph_endpoint
-- 
2.30.2

Re: [PATCH 1/4] drm: rcar-du: Fix Makefile indentation for DSI

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Fri, Nov 26, 2021 at 10:15:15AM +0000, Kieran Bingham wrote:
> From: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/rcar-du/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile
> index adc1b49d02cf..286bc81b3e7c 100644
> --- a/drivers/gpu/drm/rcar-du/Makefile
> +++ b/drivers/gpu/drm/rcar-du/Makefile
> @@ -19,7 +19,7 @@ obj-$(CONFIG_DRM_RCAR_CMM)		+= rcar_cmm.o
>  obj-$(CONFIG_DRM_RCAR_DU)		+= rcar-du-drm.o
>  obj-$(CONFIG_DRM_RCAR_DW_HDMI)		+= rcar_dw_hdmi.o
>  obj-$(CONFIG_DRM_RCAR_LVDS)		+= rcar_lvds.o
> -obj-$(CONFIG_DRM_RCAR_MIPI_DSI)	+= rcar_mipi_dsi.o
> +obj-$(CONFIG_DRM_RCAR_MIPI_DSI)		+= rcar_mipi_dsi.o
>  
>  # 'remote-endpoint' is fixed up at run-time
>  DTC_FLAGS_rcar_du_of_lvds_r8a7790 += -Wno-graph_endpoint

-- 
Regards,

Laurent Pinchart

[PATCH 2/4] drm: rcar-du: Select DRM_MIPI_DSI with DRM_RCAR_MIPI_DSI

[Kieran Bingham]

The RCAR_MIPI_DSI uses the DRM_MIPI_DSI interface.

Ensure that it is selected when the option is enabled.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
index 8cb94fe90639..8145c6d4cbc8 100644
--- a/drivers/gpu/drm/rcar-du/Kconfig
+++ b/drivers/gpu/drm/rcar-du/Kconfig
@@ -41,6 +41,7 @@ config DRM_RCAR_LVDS
 config DRM_RCAR_MIPI_DSI
 	tristate "R-Car DU MIPI DSI Encoder Support"
 	depends on DRM && DRM_BRIDGE && OF
+	select DRM_MIPI_DSI
 	help
 	  Enable support for the R-Car Display Unit embedded MIPI DSI encoders.
 
-- 
2.30.2

Re: [PATCH 2/4] drm: rcar-du: Select DRM_MIPI_DSI with DRM_RCAR_MIPI_DSI

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Fri, Nov 26, 2021 at 10:15:16AM +0000, Kieran Bingham wrote:
> The RCAR_MIPI_DSI uses the DRM_MIPI_DSI interface.
> 
> Ensure that it is selected when the option is enabled.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

I'll squash it with the appropriate patch.

> ---
>  drivers/gpu/drm/rcar-du/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
> index 8cb94fe90639..8145c6d4cbc8 100644
> --- a/drivers/gpu/drm/rcar-du/Kconfig
> +++ b/drivers/gpu/drm/rcar-du/Kconfig
> @@ -41,6 +41,7 @@ config DRM_RCAR_LVDS
>  config DRM_RCAR_MIPI_DSI
>  	tristate "R-Car DU MIPI DSI Encoder Support"
>  	depends on DRM && DRM_BRIDGE && OF
> +	select DRM_MIPI_DSI
>  	help
>  	  Enable support for the R-Car Display Unit embedded MIPI DSI encoders.
>  

-- 
Regards,

Laurent Pinchart

[PATCH 3/4] drm: rcar-du: mipi-dsi: Ensure correct fout is reported

[Kieran Bingham]

The debug reporting for the clock calculations was erroneously reporting
the last calculation of fout, rather than the fout that was determined
to have the least error, and therefore be the values chosen to operate
with.

Fix the reporting to show the correct output by storing the determined
fout, along with the error value.

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

I spent /way/ too long confused why my clock values didn't make sense
before I noticed this.. :-(


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

diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
index e94245029f95..833f4480bdf3 100644
--- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
+++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
@@ -168,6 +168,7 @@ static int rcar_mipi_dsi_phtw_test(struct rcar_mipi_dsi *dsi, u32 phtw)
  */
 
 struct dsi_setup_info {
+	unsigned long fout;
 	unsigned int err;
 	u16 vco_cntrl;
 	u16 prop_cntrl;
@@ -247,6 +248,7 @@ static void rcar_mipi_dsi_parameters_calc(struct rcar_mipi_dsi *dsi,
 				setup_info->m = m - 2;
 				setup_info->n = n - 1;
 				setup_info->err = err;
+				setup_info->fout = fout;
 				if (err == 0)
 					goto done;
 			}
@@ -256,7 +258,7 @@ static void rcar_mipi_dsi_parameters_calc(struct rcar_mipi_dsi *dsi,
 done:
 	dev_dbg(dsi->dev,
 		"%pC %lu Hz -> Fout %lu Hz (target %lu Hz, error %d.%02u%%), PLL M/N/DIV %u/%u/%u\n",
-		clk, fin, fout, fout_target, setup_info->err / 100,
+		clk, fin, setup_info->fout, fout_target, setup_info->err / 100,
 		setup_info->err % 100, setup_info->m,
 		setup_info->n, setup_info->div);
 	dev_dbg(dsi->dev,
-- 
2.30.2

Re: [PATCH 3/4] drm: rcar-du: mipi-dsi: Ensure correct fout is reported

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Fri, Nov 26, 2021 at 10:15:17AM +0000, Kieran Bingham wrote:
> The debug reporting for the clock calculations was erroneously reporting
> the last calculation of fout, rather than the fout that was determined
> to have the least error, and therefore be the values chosen to operate
> with.
> 
> Fix the reporting to show the correct output by storing the determined
> fout, along with the error value.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
> 
> I spent /way/ too long confused why my clock values didn't make sense
> before I noticed this.. :-(

Oops :-S Sorry about that.

>  drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> index e94245029f95..833f4480bdf3 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> @@ -168,6 +168,7 @@ static int rcar_mipi_dsi_phtw_test(struct rcar_mipi_dsi *dsi, u32 phtw)
>   */
>  
>  struct dsi_setup_info {
> +	unsigned long fout;
>  	unsigned int err;
>  	u16 vco_cntrl;
>  	u16 prop_cntrl;
> @@ -247,6 +248,7 @@ static void rcar_mipi_dsi_parameters_calc(struct rcar_mipi_dsi *dsi,
>  				setup_info->m = m - 2;
>  				setup_info->n = n - 1;
>  				setup_info->err = err;
> +				setup_info->fout = fout;
>  				if (err == 0)
>  					goto done;
>  			}
> @@ -256,7 +258,7 @@ static void rcar_mipi_dsi_parameters_calc(struct rcar_mipi_dsi *dsi,
>  done:
>  	dev_dbg(dsi->dev,
>  		"%pC %lu Hz -> Fout %lu Hz (target %lu Hz, error %d.%02u%%), PLL M/N/DIV %u/%u/%u\n",
> -		clk, fin, fout, fout_target, setup_info->err / 100,
> +		clk, fin, setup_info->fout, fout_target, setup_info->err / 100,

We don't need the fout in the caller, so it could be a local variable
(best_fout for instance). I can however imagine that we the frequency
could become useful in the caller, so

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  		setup_info->err % 100, setup_info->m,
>  		setup_info->n, setup_info->div);
>  	dev_dbg(dsi->dev,

-- 
Regards,

Laurent Pinchart

[PATCH 4/4] 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>
---
 drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 48 +++++++++++++------------
 1 file changed, 26 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
index 833f4480bdf3..f783bacee8da 100644
--- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
+++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
@@ -639,6 +639,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;
@@ -646,12 +648,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;
 }
 
@@ -766,21 +792,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;
@@ -788,11 +799,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;
 }
 
@@ -800,8 +806,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

RE: [PATCH 4/4] drm: rcar-du: mipi-dsi: Support bridge probe ordering

[Biju Das]

Hi Kieran,

Thanks for the patch.

> Subject: [PATCH 4/4] drm: rcar-du: mipi-dsi: Support bridge probe ordering
> 
> 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>
> ---
>  drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 48 +++++++++++++------------
>  1 file changed, 26 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> index 833f4480bdf3..f783bacee8da 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> @@ -639,6 +639,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;
> @@ -646,12 +648,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;

This can merged with previous line. return dev_err_probe(dsi->dev, ret, "could not find next bridge\n");
> +	}
> +
> +	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);

Why not return dev_err_probe??

Cheers,
Biju

> +		}
> +	}
> +
> +	/* 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;
>  }
> 
> @@ -766,21 +792,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; @@ -788,11 +799,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;
>  }
> 
> @@ -800,8 +806,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

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

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Fri, Nov 26, 2021 at 10:15:18AM +0000, Kieran Bingham wrote:
> The bridge probe ordering for DSI devices has been clarified and further
> documented in

In what ? :-)

> 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>

Will you send a new version of this patch with Biju's comments taken
into account ? I've already applied 1/4 to 3/4 to my tree, so there's no
need to repost them.

> ---
>  drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 48 +++++++++++++------------
>  1 file changed, 26 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> index 833f4480bdf3..f783bacee8da 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> @@ -639,6 +639,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;
> @@ -646,12 +648,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;
>  }
>  
> @@ -766,21 +792,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;
> @@ -788,11 +799,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;
>  }
>  
> @@ -800,8 +806,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 4/4] drm: rcar-du: mipi-dsi: Support bridge probe ordering

[Jagan Teki]

Hi Kieran,

On Fri, Nov 26, 2021 at 3:45 PM Kieran Bingham
<kieran.bingham+renesas@ideasonboard.com> wrote:
>
> 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>
> ---
>  drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 48 +++++++++++++------------
>  1 file changed, 26 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> index 833f4480bdf3..f783bacee8da 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> @@ -639,6 +639,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;
> @@ -646,12 +648,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);
> +               }
> +       }

Can we use the new function devm_drm_of_get_bridge instead of the entire code?

Jagan.

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

[Kieran Bingham]

Hi Jagan,

Quoting Jagan Teki (2021-11-30 07:38:57)
> Hi Kieran,
> 
> On Fri, Nov 26, 2021 at 3:45 PM Kieran Bingham
> <kieran.bingham+renesas@ideasonboard.com> wrote:
> >
> > 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>
> > ---
> >  drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 48 +++++++++++++------------
> >  1 file changed, 26 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> > index 833f4480bdf3..f783bacee8da 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> > @@ -639,6 +639,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;
> > @@ -646,12 +648,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);
> > +               }
> > +       }
> 
> Can we use the new function devm_drm_of_get_bridge instead of the entire code?

I've just tried this, and it works, so yes we can.
I'll update the branch and re-send it for Laurent to collect and
apply/squash whichever works best.

Thanks.

Kieran



> 
> Jagan.