| Message ID | 20211126101518.938783-5-kieran.bingham+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | drm: rcar-du: mipi-dsi: Cleanup and Fixes | expand |
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
> Subject: RE: [PATCH 4/4] drm: rcar-du: mipi-dsi: Support bridge probe > ordering > > Quoting Biju Das (2021-11-26 14:19:35) > > > Subject: RE: [PATCH 4/4] drm: rcar-du: mipi-dsi: Support bridge > > > probe ordering > > > > > > Quoting Biju Das (2021-11-26 10:51:48) > > > > 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?? > > > > > > Yes, I think it probably should. This was just a code move, so I > > > didn't change it. > > > > OK. Do you have any plan to add DSI{0,1} in rcar_du_output_name[1] or is > it in pipeline? > > > > Aha, no I've missed that. > > If you'd like to submit the patch, then please do. Otherwise I'll send one > next week. I prefer you to submit the patch next week, since nothing ready for DSI from my side. Regards, Biju
Hi Kieran, > Subject: RE: [PATCH 4/4] drm: rcar-du: mipi-dsi: Support bridge probe > ordering > > Quoting Biju Das (2021-11-26 14:19:35) > > > Subject: RE: [PATCH 4/4] drm: rcar-du: mipi-dsi: Support bridge > > > probe ordering > > > > > > Quoting Biju Das (2021-11-26 10:51:48) > > > > 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; After this change, panel is unused in probe function?? Regards, Biju
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;
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.
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.
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;
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(-)