* Re: [PATCH] drm/bridge: dw-mipi-dsi: Move drm_bridge_add into probe
2021-02-03 9:13 [PATCH] drm/bridge: dw-mipi-dsi: Move drm_bridge_add into probe Jagan Teki
@ 2021-02-03 12:05 ` Heiko Stübner
2021-02-15 11:32 ` Laurent Pinchart
2021-02-15 8:09 ` yannick Fertre
2021-06-18 13:10 ` Jonathan Liu
2 siblings, 1 reply; 10+ messages in thread
From: Heiko Stübner @ 2021-02-03 12:05 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Laurent Pinchart, Sandy Huang,
Yannick Fertre, Philippe Cornu, Benjamin Gaignard,
Vincent Abriou, Jagan Teki
Cc: Jonas Karlman, Jernej Skrabec, Sam Ravnborg, dri-devel,
linux-kernel, linux-amarula, Jagan Teki
Am Mittwoch, 3. Februar 2021, 10:13:06 CET schrieb Jagan Teki:
> Usual I2C configured DSI bridge drivers have drm_bridge_add
> in probe and mipi_dsi_attach in bridge attach functions.
>
> With, this approach the drm pipeline is unable to find the
> dsi bridge in stm drm drivers since the dw-mipi-dsi bridge is
> adding drm bridge during bridge attach operations instead of
> the probe.
Shouldn't the STM drm driver not simply defer if it's missing
a bridge that is referenced in the devicetree or somewhere?
> This specific issue may not encounter for rockchip drm dsi
> drivers, since rockchip drm uses component binding operations,
> unlike stm drm drivers.
>
> So, possible solutions are
> 1. Move drm_bridge_add into the dw-mipi-dsi probe.
> 2. Add mipi_dsi_attach in the bridge drivers probe.
> 3. Add component binding operations for stm drm drivers.
personally I'd like number (3) a lot ;-) .
With your approach, at least the component-based variants would
end up with multiple probe cycles, getting clocks etc until at some point
the panel has probed, where in the current way of things, the probe is
done once and we continue bringup once the panel has probed and called
dsi-attach to signal it is present.
Which was actually what Andrzej wished for, when I moved the Rockchip
dsi to the common driver.
Or at least make it configurable via a param to the common dw-dsi probe
function. Especially as I also need the dsi bridge-less when used as a
simple means for the configuring the internal dphy to rx-mode, see [0]
Heiko
[0] https://lore.kernel.org/dri-devel/20210202145632.1263136-1-heiko@sntech.de/
> Option 1 is a relatively possible solution as most of the
> mainline drm dsi with bridge drivers have a similar approach
> to their dsi host vs bridge registration.
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 35 +++++++++----------
> 1 file changed, 17 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index 6b268f9445b3..8a535041f071 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -314,8 +314,6 @@ static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host,
> {
> struct dw_mipi_dsi *dsi = host_to_dsi(host);
> const struct dw_mipi_dsi_plat_data *pdata = dsi->plat_data;
> - struct drm_bridge *bridge;
> - struct drm_panel *panel;
> int ret;
>
> if (device->lanes > dsi->plat_data->max_data_lanes) {
> @@ -329,22 +327,6 @@ static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host,
> dsi->format = device->format;
> dsi->mode_flags = device->mode_flags;
>
> - ret = drm_of_find_panel_or_bridge(host->dev->of_node, 1, 0,
> - &panel, &bridge);
> - if (ret)
> - return ret;
> -
> - if (panel) {
> - bridge = drm_panel_bridge_add_typed(panel,
> - DRM_MODE_CONNECTOR_DSI);
> - if (IS_ERR(bridge))
> - return PTR_ERR(bridge);
> - }
> -
> - dsi->panel_bridge = bridge;
> -
> - drm_bridge_add(&dsi->bridge);
> -
> if (pdata->host_ops && pdata->host_ops->attach) {
> ret = pdata->host_ops->attach(pdata->priv_data, device);
> if (ret < 0)
> @@ -1105,6 +1087,8 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> struct device *dev = &pdev->dev;
> struct reset_control *apb_rst;
> struct dw_mipi_dsi *dsi;
> + struct drm_bridge *bridge;
> + struct drm_panel *panel;
> int ret;
>
> dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
> @@ -1167,6 +1151,20 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> dw_mipi_dsi_debugfs_init(dsi);
> pm_runtime_enable(dev);
>
> + ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0,
> + &panel, &bridge);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + if (panel) {
> + bridge = drm_panel_bridge_add_typed(panel,
> + DRM_MODE_CONNECTOR_DSI);
> + if (IS_ERR(bridge))
> + return ERR_PTR(-ENODEV);
> + }
> +
> + dsi->panel_bridge = bridge;
> +
> dsi->dsi_host.ops = &dw_mipi_dsi_host_ops;
> dsi->dsi_host.dev = dev;
> ret = mipi_dsi_host_register(&dsi->dsi_host);
> @@ -1181,6 +1179,7 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> #ifdef CONFIG_OF
> dsi->bridge.of_node = pdev->dev.of_node;
> #endif
> + drm_bridge_add(&dsi->bridge);
>
> return dsi;
> }
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] drm/bridge: dw-mipi-dsi: Move drm_bridge_add into probe
2021-02-03 12:05 ` Heiko Stübner
@ 2021-02-15 11:32 ` Laurent Pinchart
0 siblings, 0 replies; 10+ messages in thread
From: Laurent Pinchart @ 2021-02-15 11:32 UTC (permalink / raw)
To: Heiko Stübner
Cc: Andrzej Hajda, Neil Armstrong, Sandy Huang, Yannick Fertre,
Philippe Cornu, Benjamin Gaignard, Vincent Abriou, Jagan Teki,
Jonas Karlman, Jernej Skrabec, Sam Ravnborg, dri-devel,
linux-kernel, linux-amarula
Hi Heiko,
On Wed, Feb 03, 2021 at 01:05:43PM +0100, Heiko Stübner wrote:
> Am Mittwoch, 3. Februar 2021, 10:13:06 CET schrieb Jagan Teki:
> > Usual I2C configured DSI bridge drivers have drm_bridge_add
> > in probe and mipi_dsi_attach in bridge attach functions.
> >
> > With, this approach the drm pipeline is unable to find the
> > dsi bridge in stm drm drivers since the dw-mipi-dsi bridge is
> > adding drm bridge during bridge attach operations instead of
> > the probe.
>
> Shouldn't the STM drm driver not simply defer if it's missing
> a bridge that is referenced in the devicetree or somewhere?
>
> > This specific issue may not encounter for rockchip drm dsi
> > drivers, since rockchip drm uses component binding operations,
> > unlike stm drm drivers.
> >
> > So, possible solutions are
> > 1. Move drm_bridge_add into the dw-mipi-dsi probe.
> > 2. Add mipi_dsi_attach in the bridge drivers probe.
> > 3. Add component binding operations for stm drm drivers.
>
> personally I'd like number (3) a lot ;-) .
We have different opinions on this topic :-) The component framework
adds a layer of complexity that can be avoided with probe deferral in
most cases. The main reason why probe deferral isn't always enough is
circular dependencies, which unless I'm mistaken isn't an issue here.
> With your approach, at least the component-based variants would
> end up with multiple probe cycles, getting clocks etc until at some point
> the panel has probed, where in the current way of things, the probe is
> done once and we continue bringup once the panel has probed and called
> dsi-attach to signal it is present.
>
> Which was actually what Andrzej wished for, when I moved the Rockchip
> dsi to the common driver.
>
> Or at least make it configurable via a param to the common dw-dsi probe
> function. Especially as I also need the dsi bridge-less when used as a
> simple means for the configuring the internal dphy to rx-mode, see [0]
>
> Heiko
>
> [0] https://lore.kernel.org/dri-devel/20210202145632.1263136-1-heiko@sntech.de/
>
> > Option 1 is a relatively possible solution as most of the
> > mainline drm dsi with bridge drivers have a similar approach
> > to their dsi host vs bridge registration.
> >
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 35 +++++++++----------
> > 1 file changed, 17 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > index 6b268f9445b3..8a535041f071 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > @@ -314,8 +314,6 @@ static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host,
> > {
> > struct dw_mipi_dsi *dsi = host_to_dsi(host);
> > const struct dw_mipi_dsi_plat_data *pdata = dsi->plat_data;
> > - struct drm_bridge *bridge;
> > - struct drm_panel *panel;
> > int ret;
> >
> > if (device->lanes > dsi->plat_data->max_data_lanes) {
> > @@ -329,22 +327,6 @@ static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host,
> > dsi->format = device->format;
> > dsi->mode_flags = device->mode_flags;
> >
> > - ret = drm_of_find_panel_or_bridge(host->dev->of_node, 1, 0,
> > - &panel, &bridge);
> > - if (ret)
> > - return ret;
> > -
> > - if (panel) {
> > - bridge = drm_panel_bridge_add_typed(panel,
> > - DRM_MODE_CONNECTOR_DSI);
> > - if (IS_ERR(bridge))
> > - return PTR_ERR(bridge);
> > - }
> > -
> > - dsi->panel_bridge = bridge;
> > -
> > - drm_bridge_add(&dsi->bridge);
> > -
> > if (pdata->host_ops && pdata->host_ops->attach) {
> > ret = pdata->host_ops->attach(pdata->priv_data, device);
> > if (ret < 0)
> > @@ -1105,6 +1087,8 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> > struct device *dev = &pdev->dev;
> > struct reset_control *apb_rst;
> > struct dw_mipi_dsi *dsi;
> > + struct drm_bridge *bridge;
> > + struct drm_panel *panel;
> > int ret;
> >
> > dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
> > @@ -1167,6 +1151,20 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> > dw_mipi_dsi_debugfs_init(dsi);
> > pm_runtime_enable(dev);
> >
> > + ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0,
> > + &panel, &bridge);
> > + if (ret)
> > + return ERR_PTR(ret);
> > +
> > + if (panel) {
> > + bridge = drm_panel_bridge_add_typed(panel,
> > + DRM_MODE_CONNECTOR_DSI);
> > + if (IS_ERR(bridge))
> > + return ERR_PTR(-ENODEV);
> > + }
> > +
> > + dsi->panel_bridge = bridge;
> > +
> > dsi->dsi_host.ops = &dw_mipi_dsi_host_ops;
> > dsi->dsi_host.dev = dev;
> > ret = mipi_dsi_host_register(&dsi->dsi_host);
> > @@ -1181,6 +1179,7 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> > #ifdef CONFIG_OF
> > dsi->bridge.of_node = pdev->dev.of_node;
> > #endif
> > + drm_bridge_add(&dsi->bridge);
> >
> > return dsi;
> > }
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/bridge: dw-mipi-dsi: Move drm_bridge_add into probe
2021-02-03 9:13 [PATCH] drm/bridge: dw-mipi-dsi: Move drm_bridge_add into probe Jagan Teki
2021-02-03 12:05 ` Heiko Stübner
@ 2021-02-15 8:09 ` yannick Fertre
2021-02-24 17:01 ` Jagan Teki
2021-06-18 13:10 ` Jonathan Liu
2 siblings, 1 reply; 10+ messages in thread
From: yannick Fertre @ 2021-02-15 8:09 UTC (permalink / raw)
To: Jagan Teki, Andrzej Hajda, Neil Armstrong, Laurent Pinchart,
Sandy Huang, Heiko Stubner, Yannick Fertre, Philippe Cornu
Cc: Marek Vasut, Vincent Abriou, Jernej Skrabec, Jonas Karlman,
Sam Ravnborg, linux-kernel, dri-devel, linux-amarula
Hello Jagan, I tested your patch on the stm32mp1 board.
Unfortunately, the dsi panel does not probe well with this patch. The
problem is due to the panel which is placed in the node of the dsi
bridge (no problem with i2c devices).
Regarding component bindings for stm drivers, I am currently working on
a new version.
Best regards
Yannick
On 2/3/21 10:13 AM, Jagan Teki wrote:
> Usual I2C configured DSI bridge drivers have drm_bridge_add
> in probe and mipi_dsi_attach in bridge attach functions.
>
> With, this approach the drm pipeline is unable to find the
> dsi bridge in stm drm drivers since the dw-mipi-dsi bridge is
> adding drm bridge during bridge attach operations instead of
> the probe.
>
> This specific issue may not encounter for rockchip drm dsi
> drivers, since rockchip drm uses component binding operations,
> unlike stm drm drivers.
>
> So, possible solutions are
> 1. Move drm_bridge_add into the dw-mipi-dsi probe.
> 2. Add mipi_dsi_attach in the bridge drivers probe.
> 3. Add component binding operations for stm drm drivers.
>
> Option 1 is a relatively possible solution as most of the
> mainline drm dsi with bridge drivers have a similar approach
> to their dsi host vs bridge registration.
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 35 +++++++++----------
> 1 file changed, 17 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index 6b268f9445b3..8a535041f071 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -314,8 +314,6 @@ static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host,
> {
> struct dw_mipi_dsi *dsi = host_to_dsi(host);
> const struct dw_mipi_dsi_plat_data *pdata = dsi->plat_data;
> - struct drm_bridge *bridge;
> - struct drm_panel *panel;
> int ret;
>
> if (device->lanes > dsi->plat_data->max_data_lanes) {
> @@ -329,22 +327,6 @@ static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host,
> dsi->format = device->format;
> dsi->mode_flags = device->mode_flags;
>
> - ret = drm_of_find_panel_or_bridge(host->dev->of_node, 1, 0,
> - &panel, &bridge);
> - if (ret)
> - return ret;
> -
> - if (panel) {
> - bridge = drm_panel_bridge_add_typed(panel,
> - DRM_MODE_CONNECTOR_DSI);
> - if (IS_ERR(bridge))
> - return PTR_ERR(bridge);
> - }
> -
> - dsi->panel_bridge = bridge;
> -
> - drm_bridge_add(&dsi->bridge);
> -
> if (pdata->host_ops && pdata->host_ops->attach) {
> ret = pdata->host_ops->attach(pdata->priv_data, device);
> if (ret < 0)
> @@ -1105,6 +1087,8 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> struct device *dev = &pdev->dev;
> struct reset_control *apb_rst;
> struct dw_mipi_dsi *dsi;
> + struct drm_bridge *bridge;
> + struct drm_panel *panel;
> int ret;
>
> dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
> @@ -1167,6 +1151,20 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> dw_mipi_dsi_debugfs_init(dsi);
> pm_runtime_enable(dev);
>
> + ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0,
> + &panel, &bridge);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + if (panel) {
> + bridge = drm_panel_bridge_add_typed(panel,
> + DRM_MODE_CONNECTOR_DSI);
> + if (IS_ERR(bridge))
> + return ERR_PTR(-ENODEV);
> + }
> +
> + dsi->panel_bridge = bridge;
> +
> dsi->dsi_host.ops = &dw_mipi_dsi_host_ops;
> dsi->dsi_host.dev = dev;
> ret = mipi_dsi_host_register(&dsi->dsi_host);
> @@ -1181,6 +1179,7 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> #ifdef CONFIG_OF
> dsi->bridge.of_node = pdev->dev.of_node;
> #endif
> + drm_bridge_add(&dsi->bridge);
>
> return dsi;
> }
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] drm/bridge: dw-mipi-dsi: Move drm_bridge_add into probe
2021-02-15 8:09 ` yannick Fertre
@ 2021-02-24 17:01 ` Jagan Teki
0 siblings, 0 replies; 10+ messages in thread
From: Jagan Teki @ 2021-02-24 17:01 UTC (permalink / raw)
To: yannick Fertre, Laurent Pinchart, Heiko Stubner
Cc: Andrzej Hajda, Neil Armstrong, Sandy Huang, Yannick Fertre,
Philippe Cornu, Marek Vasut, Vincent Abriou, Jernej Skrabec,
Jonas Karlman, Sam Ravnborg, linux-kernel, dri-devel,
linux-amarula
Hi Yannick,
Thanks for testing this change.
On Mon, Feb 15, 2021 at 1:39 PM yannick Fertre
<yannick.fertre@foss.st.com> wrote:
>
> Hello Jagan, I tested your patch on the stm32mp1 board.
> Unfortunately, the dsi panel does not probe well with this patch. The
> problem is due to the panel which is placed in the node of the dsi
> bridge (no problem with i2c devices).
>
> Regarding component bindings for stm drivers, I am currently working on
> a new version.
1. All non-I2C bridges are attaching dsi via mipi_dsi_attach during
the bridge controller probe and that would be expecting
panel_or_bridge need to be in DSI host attach.
2. I2C bridges are attaching dsi via mipi_dsi_attach during the bridge
attach function and that would be expecting panel_or_bridge need to be
in DSI probe.
I believe these types of DSI controllers followed by DSI panels,
bridges are not available in Mainline. if I'm not mistaken.
Adding component bindings in this regards never helps, this seems to
be common for component or non-components DSI host drivers.
One way to handle this issue can be during drm helper initialization,
like attaching the dsi host instead of calling directly from bridge or
panel drivers.
Laurent, Heiko - let me know your suggestions if it make sense, thanks.
Jagan.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/bridge: dw-mipi-dsi: Move drm_bridge_add into probe
2021-02-03 9:13 [PATCH] drm/bridge: dw-mipi-dsi: Move drm_bridge_add into probe Jagan Teki
2021-02-03 12:05 ` Heiko Stübner
2021-02-15 8:09 ` yannick Fertre
@ 2021-06-18 13:10 ` Jonathan Liu
2021-06-24 12:34 ` Jagan Teki
2 siblings, 1 reply; 10+ messages in thread
From: Jonathan Liu @ 2021-06-18 13:10 UTC (permalink / raw)
To: Jagan Teki
Cc: Andrzej Hajda, Neil Armstrong, Laurent Pinchart, Sandy Huang,
Heiko Stubner, Yannick Fertre, Philippe Cornu, Benjamin Gaignard,
Vincent Abriou, Jernej Skrabec, Jonas Karlman, Sam Ravnborg,
linux-kernel, dri-devel, linux-amarula
Hi Jagan,
On Wed, 3 Feb 2021 at 09:13, Jagan Teki <jagan@amarulasolutions.com> wrote:
> @@ -1167,6 +1151,20 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> dw_mipi_dsi_debugfs_init(dsi);
> pm_runtime_enable(dev);
>
> + ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0,
> + &panel, &bridge);
> + if (ret)
> + return ERR_PTR(ret);
On RK3399 if the error is EPROBE_DEFER, __dw_mipi_dsi_probe can be
called again and result in the following errors:
[ 0.717589] debugfs: Directory 'ff960000.mipi' with parent '/'
already present!
[ 0.717601] dw-mipi-dsi-rockchip ff960000.mipi: failed to create debugfs root
[ 0.717606] dw-mipi-dsi-rockchip ff960000.mipi: Unbalanced pm_runtime_enable!
Regards,
Jonathan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/bridge: dw-mipi-dsi: Move drm_bridge_add into probe
2021-06-18 13:10 ` Jonathan Liu
@ 2021-06-24 12:34 ` Jagan Teki
2021-06-24 12:39 ` Jonathan Liu
0 siblings, 1 reply; 10+ messages in thread
From: Jagan Teki @ 2021-06-24 12:34 UTC (permalink / raw)
To: Jonathan Liu
Cc: Andrzej Hajda, Neil Armstrong, Laurent Pinchart, Sandy Huang,
Heiko Stubner, Yannick Fertre, Philippe Cornu, Benjamin Gaignard,
Vincent Abriou, Jernej Skrabec, Jonas Karlman, Sam Ravnborg,
linux-kernel, dri-devel, linux-amarula
Hi Jonathan,
On Fri, Jun 18, 2021 at 6:40 PM Jonathan Liu <net147@gmail.com> wrote:
>
> Hi Jagan,
>
> On Wed, 3 Feb 2021 at 09:13, Jagan Teki <jagan@amarulasolutions.com> wrote:
> > @@ -1167,6 +1151,20 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> > dw_mipi_dsi_debugfs_init(dsi);
> > pm_runtime_enable(dev);
> >
> > + ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0,
> > + &panel, &bridge);
> > + if (ret)
> > + return ERR_PTR(ret);
>
> On RK3399 if the error is EPROBE_DEFER, __dw_mipi_dsi_probe can be
> called again and result in the following errors:
> [ 0.717589] debugfs: Directory 'ff960000.mipi' with parent '/'
> already present!
> [ 0.717601] dw-mipi-dsi-rockchip ff960000.mipi: failed to create debugfs root
> [ 0.717606] dw-mipi-dsi-rockchip ff960000.mipi: Unbalanced pm_runtime_enable!
Is this when you test bridge on rk3399 or panel?
Jagan.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/bridge: dw-mipi-dsi: Move drm_bridge_add into probe
2021-06-24 12:34 ` Jagan Teki
@ 2021-06-24 12:39 ` Jonathan Liu
2021-06-24 12:51 ` Laurent Pinchart
0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Liu @ 2021-06-24 12:39 UTC (permalink / raw)
To: Jagan Teki
Cc: Andrzej Hajda, Neil Armstrong, Laurent Pinchart, Sandy Huang,
Heiko Stubner, Yannick Fertre, Philippe Cornu, Benjamin Gaignard,
Vincent Abriou, Jernej Skrabec, Jonas Karlman, Sam Ravnborg,
linux-kernel, dri-devel, linux-amarula
Hi Jagan,
On Thu, 24 Jun 2021 at 22:34, Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> Hi Jonathan,
>
> On Fri, Jun 18, 2021 at 6:40 PM Jonathan Liu <net147@gmail.com> wrote:
> >
> > Hi Jagan,
> >
> > On Wed, 3 Feb 2021 at 09:13, Jagan Teki <jagan@amarulasolutions.com> wrote:
> > > @@ -1167,6 +1151,20 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> > > dw_mipi_dsi_debugfs_init(dsi);
> > > pm_runtime_enable(dev);
> > >
> > > + ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0,
> > > + &panel, &bridge);
> > > + if (ret)
> > > + return ERR_PTR(ret);
> >
> > On RK3399 if the error is EPROBE_DEFER, __dw_mipi_dsi_probe can be
> > called again and result in the following errors:
> > [ 0.717589] debugfs: Directory 'ff960000.mipi' with parent '/'
> > already present!
> > [ 0.717601] dw-mipi-dsi-rockchip ff960000.mipi: failed to create debugfs root
> > [ 0.717606] dw-mipi-dsi-rockchip ff960000.mipi: Unbalanced pm_runtime_enable!
>
> Is this when you test bridge on rk3399 or panel?
MIPI-DSI to LVDS bridge.
Regards,
Jonathan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/bridge: dw-mipi-dsi: Move drm_bridge_add into probe
2021-06-24 12:39 ` Jonathan Liu
@ 2021-06-24 12:51 ` Laurent Pinchart
2021-06-28 10:12 ` Maxime Ripard
0 siblings, 1 reply; 10+ messages in thread
From: Laurent Pinchart @ 2021-06-24 12:51 UTC (permalink / raw)
To: Jonathan Liu
Cc: Jagan Teki, Andrzej Hajda, Neil Armstrong, Sandy Huang,
Heiko Stubner, Yannick Fertre, Philippe Cornu, Benjamin Gaignard,
Vincent Abriou, Jernej Skrabec, Jonas Karlman, Sam Ravnborg,
linux-kernel, dri-devel, linux-amarula, Maxime Ripard
CC'ing Maxime Ripard.
Maxime, is this similar to the issue we've recently discussed with the
VC4 DSI encoder ?
On Thu, Jun 24, 2021 at 10:39:48PM +1000, Jonathan Liu wrote:
> On Thu, 24 Jun 2021 at 22:34, Jagan Teki wrote:
> > On Fri, Jun 18, 2021 at 6:40 PM Jonathan Liu wrote:
> > > On Wed, 3 Feb 2021 at 09:13, Jagan Teki wrote:
> > > > @@ -1167,6 +1151,20 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> > > > dw_mipi_dsi_debugfs_init(dsi);
> > > > pm_runtime_enable(dev);
> > > >
> > > > + ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0,
> > > > + &panel, &bridge);
> > > > + if (ret)
> > > > + return ERR_PTR(ret);
> > >
> > > On RK3399 if the error is EPROBE_DEFER, __dw_mipi_dsi_probe can be
> > > called again and result in the following errors:
> > > [ 0.717589] debugfs: Directory 'ff960000.mipi' with parent '/' already present!
> > > [ 0.717601] dw-mipi-dsi-rockchip ff960000.mipi: failed to create debugfs root
> > > [ 0.717606] dw-mipi-dsi-rockchip ff960000.mipi: Unbalanced pm_runtime_enable!
> >
> > Is this when you test bridge on rk3399 or panel?
>
> MIPI-DSI to LVDS bridge.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/bridge: dw-mipi-dsi: Move drm_bridge_add into probe
2021-06-24 12:51 ` Laurent Pinchart
@ 2021-06-28 10:12 ` Maxime Ripard
0 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2021-06-28 10:12 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Jonathan Liu, Jagan Teki, Andrzej Hajda, Neil Armstrong,
Sandy Huang, Heiko Stubner, Yannick Fertre, Philippe Cornu,
Benjamin Gaignard, Vincent Abriou, Jernej Skrabec, Jonas Karlman,
Sam Ravnborg, linux-kernel, dri-devel, linux-amarula
[-- Attachment #1: Type: text/plain, Size: 1456 bytes --]
Hi,
On Thu, Jun 24, 2021 at 03:51:05PM +0300, Laurent Pinchart wrote:
> CC'ing Maxime Ripard.
>
> Maxime, is this similar to the issue we've recently discussed with the
> VC4 DSI encoder ?
>
> On Thu, Jun 24, 2021 at 10:39:48PM +1000, Jonathan Liu wrote:
> > On Thu, 24 Jun 2021 at 22:34, Jagan Teki wrote:
> > > On Fri, Jun 18, 2021 at 6:40 PM Jonathan Liu wrote:
> > > > On Wed, 3 Feb 2021 at 09:13, Jagan Teki wrote:
> > > > > @@ -1167,6 +1151,20 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> > > > > dw_mipi_dsi_debugfs_init(dsi);
> > > > > pm_runtime_enable(dev);
> > > > >
> > > > > + ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0,
> > > > > + &panel, &bridge);
> > > > > + if (ret)
> > > > > + return ERR_PTR(ret);
> > > >
> > > > On RK3399 if the error is EPROBE_DEFER, __dw_mipi_dsi_probe can be
> > > > called again and result in the following errors:
> > > > [ 0.717589] debugfs: Directory 'ff960000.mipi' with parent '/' already present!
> > > > [ 0.717601] dw-mipi-dsi-rockchip ff960000.mipi: failed to create debugfs root
> > > > [ 0.717606] dw-mipi-dsi-rockchip ff960000.mipi: Unbalanced pm_runtime_enable!
> > >
> > > Is this when you test bridge on rk3399 or panel?
> >
> > MIPI-DSI to LVDS bridge.
It looks more like a driver that doesn't free its resources properly on EPROBE_DEFER?
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread