2021-11-26 10:17:25

by Kieran Bingham

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

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/[email protected]/

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/[email protected]/

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



2021-11-26 10:17:28

by Kieran Bingham

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

From: Kieran Bingham <[email protected]>

Signed-off-by: Kieran Bingham <[email protected]>
---
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


2021-11-26 10:19:27

by Kieran Bingham

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

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 <[email protected]>
---

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


2021-11-26 10:19:30

by Kieran Bingham

[permalink] [raw]
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 <[email protected]>
---
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


2021-11-26 10:29:42

by Kieran Bingham

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

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 <[email protected]>
---
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


2021-11-26 10:53:54

by Biju Das

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

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 <[email protected]>
> ---
> 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


2021-11-30 02:41:20

by Laurent Pinchart

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

Hi Kieran,

Thank you for the patch.

On Fri, Nov 26, 2021 at 10:15:15AM +0000, Kieran Bingham wrote:
> From: Kieran Bingham <[email protected]>
>
> Signed-off-by: Kieran Bingham <[email protected]>

Reviewed-by: Laurent Pinchart <[email protected]>

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

2021-11-30 02:52:40

by Laurent Pinchart

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

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 <[email protected]>

Reviewed-by: Laurent Pinchart <[email protected]>

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

2021-11-30 03:13:36

by Laurent Pinchart

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

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 <[email protected]>
> ---
>
> 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 <[email protected]>

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

--
Regards,

Laurent Pinchart

2021-11-30 03:25:17

by Laurent Pinchart

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

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 <[email protected]>

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

2021-11-30 07:39:27

by Jagan Teki

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

Hi Kieran,

On Fri, Nov 26, 2021 at 3:45 PM Kieran Bingham
<[email protected]> 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 <[email protected]>
> ---
> 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.

2021-11-30 16:01:50

by Kieran Bingham

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

Hi Jagan,

Quoting Jagan Teki (2021-11-30 07:38:57)
> Hi Kieran,
>
> On Fri, Nov 26, 2021 at 3:45 PM Kieran Bingham
> <[email protected]> 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 <[email protected]>
> > ---
> > 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.