* [PATCH v11 0/3] drm: exynos: dsi: Restore the bridge chain @ 2022-12-12 18:29 ` Jagan Teki 0 siblings, 0 replies; 29+ messages in thread From: Jagan Teki @ 2022-12-12 18:29 UTC (permalink / raw) To: Marek Szyprowski, Inki Dae, Seung-Woo Kim, Kyungmin Park, Neil Armstrong, Robert Foss, Andrzej Hajda, Sam Ravnborg Cc: Marek Vasut, linux-samsung-soc, linux-amarula, Jagan Teki, dri-devel Split the Exynos DSI bridge chain update patches from Samsung DSIM bridge driver for easy to apply. Changes for v11: - enable bridge.pre_enable_prev_first Changes for v10: - collect Marek.V Review tag Any inputs? Jagan. Jagan Teki (2): drm: panel: Enable prepare_prev_first flag for samsung-s6e panels drm: exynos: dsi: Restore proper bridge chain order Marek Szyprowski (1): drm/bridge: tc358764: Enable pre_enable_prev_first flag drivers/gpu/drm/bridge/tc358764.c | 1 + drivers/gpu/drm/exynos/exynos_drm_dsi.c | 9 +++++++-- drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c | 1 + drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c | 1 + drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c | 1 + 5 files changed, 11 insertions(+), 2 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v11 0/3] drm: exynos: dsi: Restore the bridge chain @ 2022-12-12 18:29 ` Jagan Teki 0 siblings, 0 replies; 29+ messages in thread From: Jagan Teki @ 2022-12-12 18:29 UTC (permalink / raw) To: Marek Szyprowski, Inki Dae, Seung-Woo Kim, Kyungmin Park, Neil Armstrong, Robert Foss, Andrzej Hajda, Sam Ravnborg Cc: Marek Vasut, linux-samsung-soc, dri-devel, linux-amarula, Jagan Teki Split the Exynos DSI bridge chain update patches from Samsung DSIM bridge driver for easy to apply. Changes for v11: - enable bridge.pre_enable_prev_first Changes for v10: - collect Marek.V Review tag Any inputs? Jagan. Jagan Teki (2): drm: panel: Enable prepare_prev_first flag for samsung-s6e panels drm: exynos: dsi: Restore proper bridge chain order Marek Szyprowski (1): drm/bridge: tc358764: Enable pre_enable_prev_first flag drivers/gpu/drm/bridge/tc358764.c | 1 + drivers/gpu/drm/exynos/exynos_drm_dsi.c | 9 +++++++-- drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c | 1 + drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c | 1 + drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c | 1 + 5 files changed, 11 insertions(+), 2 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v11 1/3] drm: panel: Enable prepare_prev_first flag for samsung-s6e panels 2022-12-12 18:29 ` Jagan Teki @ 2022-12-12 18:29 ` Jagan Teki -1 siblings, 0 replies; 29+ messages in thread From: Jagan Teki @ 2022-12-12 18:29 UTC (permalink / raw) To: Marek Szyprowski, Inki Dae, Seung-Woo Kim, Kyungmin Park, Neil Armstrong, Robert Foss, Andrzej Hajda, Sam Ravnborg Cc: Marek Vasut, linux-samsung-soc, linux-amarula, Jagan Teki, dri-devel Enable the drm panel prepare_prev_first flag so-that the previous controller should be prepared first before the prepare for the panel is called. samsung-s6e3ha2, samsung-s6e63j0x03 and samsung-s6e8aa0 are the effected samsung-s6e panels for this change. This makes sure that the previous controller, likely to be a DSI host controller should be initialized to LP-11 before the panel is powered up. Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> --- Changes for v11, v10: - none drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c | 1 + drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c | 1 + drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c | 1 + 3 files changed, 3 insertions(+) diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c b/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c index 5c621b15e84c..1355b2c27932 100644 --- a/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c +++ b/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c @@ -731,6 +731,7 @@ static int s6e3ha2_probe(struct mipi_dsi_device *dsi) drm_panel_init(&ctx->panel, dev, &s6e3ha2_drm_funcs, DRM_MODE_CONNECTOR_DSI); + ctx->panel.prepare_prev_first = true; drm_panel_add(&ctx->panel); diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c b/drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c index e06fd35de814..3223a9d06a50 100644 --- a/drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c +++ b/drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c @@ -462,6 +462,7 @@ static int s6e63j0x03_probe(struct mipi_dsi_device *dsi) drm_panel_init(&ctx->panel, dev, &s6e63j0x03_funcs, DRM_MODE_CONNECTOR_DSI); + ctx->panel.prepare_prev_first = true; ctx->bl_dev = backlight_device_register("s6e63j0x03", dev, ctx, &s6e63j0x03_bl_ops, NULL); diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c b/drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c index 54213beafaf5..362eb10f10ce 100644 --- a/drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c +++ b/drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c @@ -1018,6 +1018,7 @@ static int s6e8aa0_probe(struct mipi_dsi_device *dsi) drm_panel_init(&ctx->panel, dev, &s6e8aa0_drm_funcs, DRM_MODE_CONNECTOR_DSI); + ctx->panel.prepare_prev_first = true; drm_panel_add(&ctx->panel); -- 2.25.1 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v11 1/3] drm: panel: Enable prepare_prev_first flag for samsung-s6e panels @ 2022-12-12 18:29 ` Jagan Teki 0 siblings, 0 replies; 29+ messages in thread From: Jagan Teki @ 2022-12-12 18:29 UTC (permalink / raw) To: Marek Szyprowski, Inki Dae, Seung-Woo Kim, Kyungmin Park, Neil Armstrong, Robert Foss, Andrzej Hajda, Sam Ravnborg Cc: Marek Vasut, linux-samsung-soc, dri-devel, linux-amarula, Jagan Teki Enable the drm panel prepare_prev_first flag so-that the previous controller should be prepared first before the prepare for the panel is called. samsung-s6e3ha2, samsung-s6e63j0x03 and samsung-s6e8aa0 are the effected samsung-s6e panels for this change. This makes sure that the previous controller, likely to be a DSI host controller should be initialized to LP-11 before the panel is powered up. Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> --- Changes for v11, v10: - none drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c | 1 + drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c | 1 + drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c | 1 + 3 files changed, 3 insertions(+) diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c b/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c index 5c621b15e84c..1355b2c27932 100644 --- a/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c +++ b/drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c @@ -731,6 +731,7 @@ static int s6e3ha2_probe(struct mipi_dsi_device *dsi) drm_panel_init(&ctx->panel, dev, &s6e3ha2_drm_funcs, DRM_MODE_CONNECTOR_DSI); + ctx->panel.prepare_prev_first = true; drm_panel_add(&ctx->panel); diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c b/drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c index e06fd35de814..3223a9d06a50 100644 --- a/drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c +++ b/drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c @@ -462,6 +462,7 @@ static int s6e63j0x03_probe(struct mipi_dsi_device *dsi) drm_panel_init(&ctx->panel, dev, &s6e63j0x03_funcs, DRM_MODE_CONNECTOR_DSI); + ctx->panel.prepare_prev_first = true; ctx->bl_dev = backlight_device_register("s6e63j0x03", dev, ctx, &s6e63j0x03_bl_ops, NULL); diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c b/drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c index 54213beafaf5..362eb10f10ce 100644 --- a/drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c +++ b/drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c @@ -1018,6 +1018,7 @@ static int s6e8aa0_probe(struct mipi_dsi_device *dsi) drm_panel_init(&ctx->panel, dev, &s6e8aa0_drm_funcs, DRM_MODE_CONNECTOR_DSI); + ctx->panel.prepare_prev_first = true; drm_panel_add(&ctx->panel); -- 2.25.1 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v11 1/3] drm: panel: Enable prepare_prev_first flag for samsung-s6e panels 2022-12-12 18:29 ` Jagan Teki (?) @ 2022-12-14 8:48 ` Frieder Schrempf -1 siblings, 0 replies; 29+ messages in thread From: Frieder Schrempf @ 2022-12-14 8:48 UTC (permalink / raw) To: Jagan Teki, Marek Szyprowski, Inki Dae, Seung-Woo Kim, Kyungmin Park, Neil Armstrong, Robert Foss, Andrzej Hajda, Sam Ravnborg Cc: Marek Vasut, linux-samsung-soc, linux-amarula, dri-devel On 12.12.22 19:29, Jagan Teki wrote: > Enable the drm panel prepare_prev_first flag so-that the previous > controller should be prepared first before the prepare for the > panel is called. > > samsung-s6e3ha2, samsung-s6e63j0x03 and samsung-s6e8aa0 are the > effected samsung-s6e panels for this change. > > This makes sure that the previous controller, likely to be a DSI > host controller should be initialized to LP-11 before the panel > is powered up. > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de> ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v11 2/3] drm/bridge: tc358764: Enable pre_enable_prev_first flag 2022-12-12 18:29 ` Jagan Teki @ 2022-12-12 18:29 ` Jagan Teki -1 siblings, 0 replies; 29+ messages in thread From: Jagan Teki @ 2022-12-12 18:29 UTC (permalink / raw) To: Marek Szyprowski, Inki Dae, Seung-Woo Kim, Kyungmin Park, Neil Armstrong, Robert Foss, Andrzej Hajda, Sam Ravnborg Cc: Marek Vasut, linux-samsung-soc, linux-amarula, Jagan Teki, dri-devel From: Marek Szyprowski <m.szyprowski@samsung.com> Enable the drm bridge pre_enable_prev_first flag so that the previous bridge pre_enable should be called first before the pre_enable for the tc358764 bridge is called. This makes sure that the previous bridge should be initialized properly before the tc358764 bridge is powered up. Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> --- Changes for v11, v10: - none drivers/gpu/drm/bridge/tc358764.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/bridge/tc358764.c b/drivers/gpu/drm/bridge/tc358764.c index 53259c12d777..f85654f1b104 100644 --- a/drivers/gpu/drm/bridge/tc358764.c +++ b/drivers/gpu/drm/bridge/tc358764.c @@ -369,6 +369,7 @@ static int tc358764_probe(struct mipi_dsi_device *dsi) ctx->bridge.funcs = &tc358764_bridge_funcs; ctx->bridge.of_node = dev->of_node; + ctx->bridge.pre_enable_prev_first = true; drm_bridge_add(&ctx->bridge); -- 2.25.1 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v11 2/3] drm/bridge: tc358764: Enable pre_enable_prev_first flag @ 2022-12-12 18:29 ` Jagan Teki 0 siblings, 0 replies; 29+ messages in thread From: Jagan Teki @ 2022-12-12 18:29 UTC (permalink / raw) To: Marek Szyprowski, Inki Dae, Seung-Woo Kim, Kyungmin Park, Neil Armstrong, Robert Foss, Andrzej Hajda, Sam Ravnborg Cc: Marek Vasut, linux-samsung-soc, dri-devel, linux-amarula, Jagan Teki From: Marek Szyprowski <m.szyprowski@samsung.com> Enable the drm bridge pre_enable_prev_first flag so that the previous bridge pre_enable should be called first before the pre_enable for the tc358764 bridge is called. This makes sure that the previous bridge should be initialized properly before the tc358764 bridge is powered up. Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> --- Changes for v11, v10: - none drivers/gpu/drm/bridge/tc358764.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/bridge/tc358764.c b/drivers/gpu/drm/bridge/tc358764.c index 53259c12d777..f85654f1b104 100644 --- a/drivers/gpu/drm/bridge/tc358764.c +++ b/drivers/gpu/drm/bridge/tc358764.c @@ -369,6 +369,7 @@ static int tc358764_probe(struct mipi_dsi_device *dsi) ctx->bridge.funcs = &tc358764_bridge_funcs; ctx->bridge.of_node = dev->of_node; + ctx->bridge.pre_enable_prev_first = true; drm_bridge_add(&ctx->bridge); -- 2.25.1 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v11 2/3] drm/bridge: tc358764: Enable pre_enable_prev_first flag 2022-12-12 18:29 ` Jagan Teki (?) @ 2022-12-14 8:48 ` Frieder Schrempf -1 siblings, 0 replies; 29+ messages in thread From: Frieder Schrempf @ 2022-12-14 8:48 UTC (permalink / raw) To: Jagan Teki, Marek Szyprowski, Inki Dae, Seung-Woo Kim, Kyungmin Park, Neil Armstrong, Robert Foss, Andrzej Hajda, Sam Ravnborg Cc: Marek Vasut, linux-samsung-soc, linux-amarula, dri-devel On 12.12.22 19:29, Jagan Teki wrote: > From: Marek Szyprowski <m.szyprowski@samsung.com> > > Enable the drm bridge pre_enable_prev_first flag so that the > previous bridge pre_enable should be called first before the > pre_enable for the tc358764 bridge is called. > > This makes sure that the previous bridge should be initialized > properly before the tc358764 bridge is powered up. > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de> ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v11 3/3] drm: exynos: dsi: Restore proper bridge chain order 2022-12-12 18:29 ` Jagan Teki @ 2022-12-12 18:29 ` Jagan Teki -1 siblings, 0 replies; 29+ messages in thread From: Jagan Teki @ 2022-12-12 18:29 UTC (permalink / raw) To: Marek Szyprowski, Inki Dae, Seung-Woo Kim, Kyungmin Park, Neil Armstrong, Robert Foss, Andrzej Hajda, Sam Ravnborg Cc: Marek Vasut, linux-samsung-soc, linux-amarula, Jagan Teki, dri-devel Restore the proper bridge chain by finding the previous bridge in the chain instead of passing NULL. This establishes a proper bridge chain while attaching downstream bridges. Reviewed-by: Marek Vasut <marex@denx.de> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> --- Changes for v11: - add bridge.pre_enable_prev_first Changes for v10: - collect Marek review tag drivers/gpu/drm/exynos/exynos_drm_dsi.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c index ec673223d6b7..9d10a89d28f1 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c @@ -1428,7 +1428,8 @@ static int exynos_dsi_attach(struct drm_bridge *bridge, { struct exynos_dsi *dsi = bridge_to_dsi(bridge); - return drm_bridge_attach(bridge->encoder, dsi->out_bridge, NULL, flags); + return drm_bridge_attach(bridge->encoder, dsi->out_bridge, bridge, + flags); } static const struct drm_bridge_funcs exynos_dsi_bridge_funcs = { @@ -1474,7 +1475,10 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host, drm_bridge_add(&dsi->bridge); - drm_bridge_attach(encoder, &dsi->bridge, NULL, 0); + drm_bridge_attach(encoder, &dsi->bridge, + list_first_entry_or_null(&encoder->bridge_chain, + struct drm_bridge, + chain_node), 0); /* * This is a temporary solution and should be made by more generic way. @@ -1709,6 +1713,7 @@ static int exynos_dsi_probe(struct platform_device *pdev) dsi->bridge.funcs = &exynos_dsi_bridge_funcs; dsi->bridge.of_node = dev->of_node; dsi->bridge.type = DRM_MODE_CONNECTOR_DSI; + dsi->bridge.pre_enable_prev_first = true; ret = component_add(dev, &exynos_dsi_component_ops); if (ret) -- 2.25.1 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v11 3/3] drm: exynos: dsi: Restore proper bridge chain order @ 2022-12-12 18:29 ` Jagan Teki 0 siblings, 0 replies; 29+ messages in thread From: Jagan Teki @ 2022-12-12 18:29 UTC (permalink / raw) To: Marek Szyprowski, Inki Dae, Seung-Woo Kim, Kyungmin Park, Neil Armstrong, Robert Foss, Andrzej Hajda, Sam Ravnborg Cc: Marek Vasut, linux-samsung-soc, dri-devel, linux-amarula, Jagan Teki Restore the proper bridge chain by finding the previous bridge in the chain instead of passing NULL. This establishes a proper bridge chain while attaching downstream bridges. Reviewed-by: Marek Vasut <marex@denx.de> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> --- Changes for v11: - add bridge.pre_enable_prev_first Changes for v10: - collect Marek review tag drivers/gpu/drm/exynos/exynos_drm_dsi.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c index ec673223d6b7..9d10a89d28f1 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c @@ -1428,7 +1428,8 @@ static int exynos_dsi_attach(struct drm_bridge *bridge, { struct exynos_dsi *dsi = bridge_to_dsi(bridge); - return drm_bridge_attach(bridge->encoder, dsi->out_bridge, NULL, flags); + return drm_bridge_attach(bridge->encoder, dsi->out_bridge, bridge, + flags); } static const struct drm_bridge_funcs exynos_dsi_bridge_funcs = { @@ -1474,7 +1475,10 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host, drm_bridge_add(&dsi->bridge); - drm_bridge_attach(encoder, &dsi->bridge, NULL, 0); + drm_bridge_attach(encoder, &dsi->bridge, + list_first_entry_or_null(&encoder->bridge_chain, + struct drm_bridge, + chain_node), 0); /* * This is a temporary solution and should be made by more generic way. @@ -1709,6 +1713,7 @@ static int exynos_dsi_probe(struct platform_device *pdev) dsi->bridge.funcs = &exynos_dsi_bridge_funcs; dsi->bridge.of_node = dev->of_node; dsi->bridge.type = DRM_MODE_CONNECTOR_DSI; + dsi->bridge.pre_enable_prev_first = true; ret = component_add(dev, &exynos_dsi_component_ops); if (ret) -- 2.25.1 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v11 3/3] drm: exynos: dsi: Restore proper bridge chain order 2022-12-12 18:29 ` Jagan Teki (?) @ 2022-12-14 8:50 ` Frieder Schrempf -1 siblings, 0 replies; 29+ messages in thread From: Frieder Schrempf @ 2022-12-14 8:50 UTC (permalink / raw) To: Jagan Teki, Marek Szyprowski, Inki Dae, Seung-Woo Kim, Kyungmin Park, Neil Armstrong, Robert Foss, Andrzej Hajda, Sam Ravnborg Cc: Marek Vasut, linux-samsung-soc, linux-amarula, dri-devel On 12.12.22 19:29, Jagan Teki wrote: > Restore the proper bridge chain by finding the previous bridge > in the chain instead of passing NULL. > > This establishes a proper bridge chain while attaching downstream > bridges. > > Reviewed-by: Marek Vasut <marex@denx.de> > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de> ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v11 3/3] drm: exynos: dsi: Restore proper bridge chain order 2022-12-12 18:29 ` Jagan Teki @ 2023-01-20 18:56 ` Dave Stevenson -1 siblings, 0 replies; 29+ messages in thread From: Dave Stevenson @ 2023-01-20 18:56 UTC (permalink / raw) To: Jagan Teki Cc: Marek Szyprowski, Inki Dae, Seung-Woo Kim, Kyungmin Park, Neil Armstrong, Robert Foss, Andrzej Hajda, Sam Ravnborg, Marek Vasut, linux-samsung-soc, linux-amarula, dri-devel Hi Jagan Responding due to Marek's comment on the "Add Samsung MIPI DSIM bridge" series, although I know very little about the Exynos specifics, and may well be missing context of what you're trying to achieve. On Mon, 12 Dec 2022 at 18:29, Jagan Teki <jagan@amarulasolutions.com> wrote: > > Restore the proper bridge chain by finding the previous bridge > in the chain instead of passing NULL. > > This establishes a proper bridge chain while attaching downstream > bridges. > > Reviewed-by: Marek Vasut <marex@denx.de> > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > --- > Changes for v11: > - add bridge.pre_enable_prev_first > Changes for v10: > - collect Marek review tag > > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > index ec673223d6b7..9d10a89d28f1 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > @@ -1428,7 +1428,8 @@ static int exynos_dsi_attach(struct drm_bridge *bridge, > { > struct exynos_dsi *dsi = bridge_to_dsi(bridge); > > - return drm_bridge_attach(bridge->encoder, dsi->out_bridge, NULL, flags); > + return drm_bridge_attach(bridge->encoder, dsi->out_bridge, bridge, > + flags); Agreed on this change. > } > > static const struct drm_bridge_funcs exynos_dsi_bridge_funcs = { > @@ -1474,7 +1475,10 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host, > > drm_bridge_add(&dsi->bridge); > > - drm_bridge_attach(encoder, &dsi->bridge, NULL, 0); > + drm_bridge_attach(encoder, &dsi->bridge, > + list_first_entry_or_null(&encoder->bridge_chain, > + struct drm_bridge, > + chain_node), 0); What bridge are you expecting between the encoder and this bridge? The encoder is the drm_simple_encoder_init encoder that you've created in exynos_dsi_bind, so separating that from the bridge you're also creating here seems weird. > > /* > * This is a temporary solution and should be made by more generic way. > @@ -1709,6 +1713,7 @@ static int exynos_dsi_probe(struct platform_device *pdev) > dsi->bridge.funcs = &exynos_dsi_bridge_funcs; > dsi->bridge.of_node = dev->of_node; > dsi->bridge.type = DRM_MODE_CONNECTOR_DSI; > + dsi->bridge.pre_enable_prev_first = true; Setting dsi->bridge.pre_enable_prev_first on what is presumably the DSI host controller seems a little odd. Same question again - what bridge are you expecting to be upstream of the DSI host that needs to be preenabled before it? Whilst it's possible that there's another bridge, I'd have expected that to be the first link from your encoder as they appear to both belong to the same bit of driver. Dave > ret = component_add(dev, &exynos_dsi_component_ops); > if (ret) > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v11 3/3] drm: exynos: dsi: Restore proper bridge chain order @ 2023-01-20 18:56 ` Dave Stevenson 0 siblings, 0 replies; 29+ messages in thread From: Dave Stevenson @ 2023-01-20 18:56 UTC (permalink / raw) To: Jagan Teki Cc: Marek Vasut, linux-samsung-soc, linux-amarula, Seung-Woo Kim, Neil Armstrong, Robert Foss, Kyungmin Park, dri-devel, Andrzej Hajda, Sam Ravnborg, Marek Szyprowski Hi Jagan Responding due to Marek's comment on the "Add Samsung MIPI DSIM bridge" series, although I know very little about the Exynos specifics, and may well be missing context of what you're trying to achieve. On Mon, 12 Dec 2022 at 18:29, Jagan Teki <jagan@amarulasolutions.com> wrote: > > Restore the proper bridge chain by finding the previous bridge > in the chain instead of passing NULL. > > This establishes a proper bridge chain while attaching downstream > bridges. > > Reviewed-by: Marek Vasut <marex@denx.de> > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > --- > Changes for v11: > - add bridge.pre_enable_prev_first > Changes for v10: > - collect Marek review tag > > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > index ec673223d6b7..9d10a89d28f1 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > @@ -1428,7 +1428,8 @@ static int exynos_dsi_attach(struct drm_bridge *bridge, > { > struct exynos_dsi *dsi = bridge_to_dsi(bridge); > > - return drm_bridge_attach(bridge->encoder, dsi->out_bridge, NULL, flags); > + return drm_bridge_attach(bridge->encoder, dsi->out_bridge, bridge, > + flags); Agreed on this change. > } > > static const struct drm_bridge_funcs exynos_dsi_bridge_funcs = { > @@ -1474,7 +1475,10 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host, > > drm_bridge_add(&dsi->bridge); > > - drm_bridge_attach(encoder, &dsi->bridge, NULL, 0); > + drm_bridge_attach(encoder, &dsi->bridge, > + list_first_entry_or_null(&encoder->bridge_chain, > + struct drm_bridge, > + chain_node), 0); What bridge are you expecting between the encoder and this bridge? The encoder is the drm_simple_encoder_init encoder that you've created in exynos_dsi_bind, so separating that from the bridge you're also creating here seems weird. > > /* > * This is a temporary solution and should be made by more generic way. > @@ -1709,6 +1713,7 @@ static int exynos_dsi_probe(struct platform_device *pdev) > dsi->bridge.funcs = &exynos_dsi_bridge_funcs; > dsi->bridge.of_node = dev->of_node; > dsi->bridge.type = DRM_MODE_CONNECTOR_DSI; > + dsi->bridge.pre_enable_prev_first = true; Setting dsi->bridge.pre_enable_prev_first on what is presumably the DSI host controller seems a little odd. Same question again - what bridge are you expecting to be upstream of the DSI host that needs to be preenabled before it? Whilst it's possible that there's another bridge, I'd have expected that to be the first link from your encoder as they appear to both belong to the same bit of driver. Dave > ret = component_add(dev, &exynos_dsi_component_ops); > if (ret) > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v11 3/3] drm: exynos: dsi: Restore proper bridge chain order 2023-01-20 18:56 ` Dave Stevenson @ 2023-01-20 19:09 ` Jagan Teki -1 siblings, 0 replies; 29+ messages in thread From: Jagan Teki @ 2023-01-20 19:09 UTC (permalink / raw) To: Dave Stevenson Cc: Marek Szyprowski, Inki Dae, Seung-Woo Kim, Kyungmin Park, Neil Armstrong, Robert Foss, Andrzej Hajda, Sam Ravnborg, Marek Vasut, linux-samsung-soc, linux-amarula, dri-devel Hi Dave, On Sat, Jan 21, 2023 at 12:26 AM Dave Stevenson <dave.stevenson@raspberrypi.com> wrote: > > Hi Jagan > > Responding due to Marek's comment on the "Add Samsung MIPI DSIM > bridge" series, although I know very little about the Exynos > specifics, and may well be missing context of what you're trying to > achieve. > > On Mon, 12 Dec 2022 at 18:29, Jagan Teki <jagan@amarulasolutions.com> wrote: > > > > Restore the proper bridge chain by finding the previous bridge > > in the chain instead of passing NULL. > > > > This establishes a proper bridge chain while attaching downstream > > bridges. > > > > Reviewed-by: Marek Vasut <marex@denx.de> > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > > --- > > Changes for v11: > > - add bridge.pre_enable_prev_first > > Changes for v10: > > - collect Marek review tag > > > > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > index ec673223d6b7..9d10a89d28f1 100644 > > --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > @@ -1428,7 +1428,8 @@ static int exynos_dsi_attach(struct drm_bridge *bridge, > > { > > struct exynos_dsi *dsi = bridge_to_dsi(bridge); > > > > - return drm_bridge_attach(bridge->encoder, dsi->out_bridge, NULL, flags); > > + return drm_bridge_attach(bridge->encoder, dsi->out_bridge, bridge, > > + flags); > > Agreed on this change. > > > } > > > > static const struct drm_bridge_funcs exynos_dsi_bridge_funcs = { > > @@ -1474,7 +1475,10 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host, > > > > drm_bridge_add(&dsi->bridge); > > > > - drm_bridge_attach(encoder, &dsi->bridge, NULL, 0); > > + drm_bridge_attach(encoder, &dsi->bridge, > > + list_first_entry_or_null(&encoder->bridge_chain, > > + struct drm_bridge, > > + chain_node), 0); > > What bridge are you expecting between the encoder and this bridge? > The encoder is the drm_simple_encoder_init encoder that you've created > in exynos_dsi_bind, so separating that from the bridge you're also > creating here seems weird. > > > > > /* > > * This is a temporary solution and should be made by more generic way. > > @@ -1709,6 +1713,7 @@ static int exynos_dsi_probe(struct platform_device *pdev) > > dsi->bridge.funcs = &exynos_dsi_bridge_funcs; > > dsi->bridge.of_node = dev->of_node; > > dsi->bridge.type = DRM_MODE_CONNECTOR_DSI; > > + dsi->bridge.pre_enable_prev_first = true; > > Setting dsi->bridge.pre_enable_prev_first on what is presumably the > DSI host controller seems a little odd. > Same question again - what bridge are you expecting to be upstream of > the DSI host that needs to be preenabled before it? Whilst it's > possible that there's another bridge, I'd have expected that to be the > first link from your encoder as they appear to both belong to the same > bit of driver. Let me answer all together here. I can explain a bit about one of the pipelines used in Exynos. Exynos DSI DRM drivers have some strict host initialization which is not the same as what we used in i.MX8M even though it uses the same DSIM IP. Exynos5433 Decon -> Exynos MIC -> Exynos DSI -> s6e3ha2 DSI panel Here MIC is the bridge, Exynos DSI is the bridge and the requirement is to expect the upstream bridge to pre_enable first from DSI which means the MIC. Jagan. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v11 3/3] drm: exynos: dsi: Restore proper bridge chain order @ 2023-01-20 19:09 ` Jagan Teki 0 siblings, 0 replies; 29+ messages in thread From: Jagan Teki @ 2023-01-20 19:09 UTC (permalink / raw) To: Dave Stevenson Cc: Marek Vasut, linux-samsung-soc, linux-amarula, Seung-Woo Kim, Neil Armstrong, Robert Foss, Kyungmin Park, dri-devel, Andrzej Hajda, Sam Ravnborg, Marek Szyprowski Hi Dave, On Sat, Jan 21, 2023 at 12:26 AM Dave Stevenson <dave.stevenson@raspberrypi.com> wrote: > > Hi Jagan > > Responding due to Marek's comment on the "Add Samsung MIPI DSIM > bridge" series, although I know very little about the Exynos > specifics, and may well be missing context of what you're trying to > achieve. > > On Mon, 12 Dec 2022 at 18:29, Jagan Teki <jagan@amarulasolutions.com> wrote: > > > > Restore the proper bridge chain by finding the previous bridge > > in the chain instead of passing NULL. > > > > This establishes a proper bridge chain while attaching downstream > > bridges. > > > > Reviewed-by: Marek Vasut <marex@denx.de> > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > > --- > > Changes for v11: > > - add bridge.pre_enable_prev_first > > Changes for v10: > > - collect Marek review tag > > > > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > index ec673223d6b7..9d10a89d28f1 100644 > > --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > @@ -1428,7 +1428,8 @@ static int exynos_dsi_attach(struct drm_bridge *bridge, > > { > > struct exynos_dsi *dsi = bridge_to_dsi(bridge); > > > > - return drm_bridge_attach(bridge->encoder, dsi->out_bridge, NULL, flags); > > + return drm_bridge_attach(bridge->encoder, dsi->out_bridge, bridge, > > + flags); > > Agreed on this change. > > > } > > > > static const struct drm_bridge_funcs exynos_dsi_bridge_funcs = { > > @@ -1474,7 +1475,10 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host, > > > > drm_bridge_add(&dsi->bridge); > > > > - drm_bridge_attach(encoder, &dsi->bridge, NULL, 0); > > + drm_bridge_attach(encoder, &dsi->bridge, > > + list_first_entry_or_null(&encoder->bridge_chain, > > + struct drm_bridge, > > + chain_node), 0); > > What bridge are you expecting between the encoder and this bridge? > The encoder is the drm_simple_encoder_init encoder that you've created > in exynos_dsi_bind, so separating that from the bridge you're also > creating here seems weird. > > > > > /* > > * This is a temporary solution and should be made by more generic way. > > @@ -1709,6 +1713,7 @@ static int exynos_dsi_probe(struct platform_device *pdev) > > dsi->bridge.funcs = &exynos_dsi_bridge_funcs; > > dsi->bridge.of_node = dev->of_node; > > dsi->bridge.type = DRM_MODE_CONNECTOR_DSI; > > + dsi->bridge.pre_enable_prev_first = true; > > Setting dsi->bridge.pre_enable_prev_first on what is presumably the > DSI host controller seems a little odd. > Same question again - what bridge are you expecting to be upstream of > the DSI host that needs to be preenabled before it? Whilst it's > possible that there's another bridge, I'd have expected that to be the > first link from your encoder as they appear to both belong to the same > bit of driver. Let me answer all together here. I can explain a bit about one of the pipelines used in Exynos. Exynos DSI DRM drivers have some strict host initialization which is not the same as what we used in i.MX8M even though it uses the same DSIM IP. Exynos5433 Decon -> Exynos MIC -> Exynos DSI -> s6e3ha2 DSI panel Here MIC is the bridge, Exynos DSI is the bridge and the requirement is to expect the upstream bridge to pre_enable first from DSI which means the MIC. Jagan. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v11 3/3] drm: exynos: dsi: Restore proper bridge chain order 2023-01-20 19:09 ` Jagan Teki @ 2023-01-20 19:42 ` Dave Stevenson -1 siblings, 0 replies; 29+ messages in thread From: Dave Stevenson @ 2023-01-20 19:42 UTC (permalink / raw) To: Jagan Teki Cc: Marek Szyprowski, Inki Dae, Seung-Woo Kim, Kyungmin Park, Neil Armstrong, Robert Foss, Andrzej Hajda, Sam Ravnborg, Marek Vasut, linux-samsung-soc, linux-amarula, dri-devel Hi Jagan On Fri, 20 Jan 2023 at 19:10, Jagan Teki <jagan@amarulasolutions.com> wrote: > > Hi Dave, > > On Sat, Jan 21, 2023 at 12:26 AM Dave Stevenson > <dave.stevenson@raspberrypi.com> wrote: > > > > Hi Jagan > > > > Responding due to Marek's comment on the "Add Samsung MIPI DSIM > > bridge" series, although I know very little about the Exynos > > specifics, and may well be missing context of what you're trying to > > achieve. > > > > On Mon, 12 Dec 2022 at 18:29, Jagan Teki <jagan@amarulasolutions.com> wrote: > > > > > > Restore the proper bridge chain by finding the previous bridge > > > in the chain instead of passing NULL. > > > > > > This establishes a proper bridge chain while attaching downstream > > > bridges. > > > > > > Reviewed-by: Marek Vasut <marex@denx.de> > > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > > > --- > > > Changes for v11: > > > - add bridge.pre_enable_prev_first > > > Changes for v10: > > > - collect Marek review tag > > > > > > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 9 +++++++-- > > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > > index ec673223d6b7..9d10a89d28f1 100644 > > > --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > > +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > > @@ -1428,7 +1428,8 @@ static int exynos_dsi_attach(struct drm_bridge *bridge, > > > { > > > struct exynos_dsi *dsi = bridge_to_dsi(bridge); > > > > > > - return drm_bridge_attach(bridge->encoder, dsi->out_bridge, NULL, flags); > > > + return drm_bridge_attach(bridge->encoder, dsi->out_bridge, bridge, > > > + flags); > > > > Agreed on this change. > > > > > } > > > > > > static const struct drm_bridge_funcs exynos_dsi_bridge_funcs = { > > > @@ -1474,7 +1475,10 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host, > > > > > > drm_bridge_add(&dsi->bridge); > > > > > > - drm_bridge_attach(encoder, &dsi->bridge, NULL, 0); > > > + drm_bridge_attach(encoder, &dsi->bridge, > > > + list_first_entry_or_null(&encoder->bridge_chain, > > > + struct drm_bridge, > > > + chain_node), 0); > > > > What bridge are you expecting between the encoder and this bridge? > > The encoder is the drm_simple_encoder_init encoder that you've created > > in exynos_dsi_bind, so separating that from the bridge you're also > > creating here seems weird. > > > > > > > > /* > > > * This is a temporary solution and should be made by more generic way. > > > @@ -1709,6 +1713,7 @@ static int exynos_dsi_probe(struct platform_device *pdev) > > > dsi->bridge.funcs = &exynos_dsi_bridge_funcs; > > > dsi->bridge.of_node = dev->of_node; > > > dsi->bridge.type = DRM_MODE_CONNECTOR_DSI; > > > + dsi->bridge.pre_enable_prev_first = true; > > > > Setting dsi->bridge.pre_enable_prev_first on what is presumably the > > DSI host controller seems a little odd. > > Same question again - what bridge are you expecting to be upstream of > > the DSI host that needs to be preenabled before it? Whilst it's > > possible that there's another bridge, I'd have expected that to be the > > first link from your encoder as they appear to both belong to the same > > bit of driver. > > Let me answer all together here. I can explain a bit about one of the > pipelines used in Exynos. Exynos DSI DRM drivers have some strict host > initialization which is not the same as what we used in i.MX8M even > though it uses the same DSIM IP. > > Exynos5433 Decon -> Exynos MIC -> Exynos DSI -> s6e3ha2 DSI panel > > Here MIC is the bridge, Exynos DSI is the bridge and the requirement > is to expect the upstream bridge to pre_enable first from DSI which > means the MIC. That makes sense for the pre_enable_prev_first flag. The drm_bridge_attach(... list_first_entry_or_null) still seems a little weird. I think you are making the assumption that there is only ever going to be the zero or one bridge (the MIC) between encoder and DSI bridge - the DSI bridge is linking itself to the first entry off the encoder bridge_chain (or NULL to link to the encoder). Is that reasonable? I've no idea! I must confess to not having looked at the attaching sequence recently, and I'm about to head home for the weekend. I have no real knowledge of how Exynos is working, and am aware that you're having to rejuggle stuff to try and support i.MX8M and Exynos, so leave that one up to you. Cheers Dave ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v11 3/3] drm: exynos: dsi: Restore proper bridge chain order @ 2023-01-20 19:42 ` Dave Stevenson 0 siblings, 0 replies; 29+ messages in thread From: Dave Stevenson @ 2023-01-20 19:42 UTC (permalink / raw) To: Jagan Teki Cc: Marek Vasut, linux-samsung-soc, linux-amarula, Seung-Woo Kim, Neil Armstrong, Robert Foss, Kyungmin Park, dri-devel, Andrzej Hajda, Sam Ravnborg, Marek Szyprowski Hi Jagan On Fri, 20 Jan 2023 at 19:10, Jagan Teki <jagan@amarulasolutions.com> wrote: > > Hi Dave, > > On Sat, Jan 21, 2023 at 12:26 AM Dave Stevenson > <dave.stevenson@raspberrypi.com> wrote: > > > > Hi Jagan > > > > Responding due to Marek's comment on the "Add Samsung MIPI DSIM > > bridge" series, although I know very little about the Exynos > > specifics, and may well be missing context of what you're trying to > > achieve. > > > > On Mon, 12 Dec 2022 at 18:29, Jagan Teki <jagan@amarulasolutions.com> wrote: > > > > > > Restore the proper bridge chain by finding the previous bridge > > > in the chain instead of passing NULL. > > > > > > This establishes a proper bridge chain while attaching downstream > > > bridges. > > > > > > Reviewed-by: Marek Vasut <marex@denx.de> > > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > > > --- > > > Changes for v11: > > > - add bridge.pre_enable_prev_first > > > Changes for v10: > > > - collect Marek review tag > > > > > > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 9 +++++++-- > > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > > index ec673223d6b7..9d10a89d28f1 100644 > > > --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > > +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > > @@ -1428,7 +1428,8 @@ static int exynos_dsi_attach(struct drm_bridge *bridge, > > > { > > > struct exynos_dsi *dsi = bridge_to_dsi(bridge); > > > > > > - return drm_bridge_attach(bridge->encoder, dsi->out_bridge, NULL, flags); > > > + return drm_bridge_attach(bridge->encoder, dsi->out_bridge, bridge, > > > + flags); > > > > Agreed on this change. > > > > > } > > > > > > static const struct drm_bridge_funcs exynos_dsi_bridge_funcs = { > > > @@ -1474,7 +1475,10 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host, > > > > > > drm_bridge_add(&dsi->bridge); > > > > > > - drm_bridge_attach(encoder, &dsi->bridge, NULL, 0); > > > + drm_bridge_attach(encoder, &dsi->bridge, > > > + list_first_entry_or_null(&encoder->bridge_chain, > > > + struct drm_bridge, > > > + chain_node), 0); > > > > What bridge are you expecting between the encoder and this bridge? > > The encoder is the drm_simple_encoder_init encoder that you've created > > in exynos_dsi_bind, so separating that from the bridge you're also > > creating here seems weird. > > > > > > > > /* > > > * This is a temporary solution and should be made by more generic way. > > > @@ -1709,6 +1713,7 @@ static int exynos_dsi_probe(struct platform_device *pdev) > > > dsi->bridge.funcs = &exynos_dsi_bridge_funcs; > > > dsi->bridge.of_node = dev->of_node; > > > dsi->bridge.type = DRM_MODE_CONNECTOR_DSI; > > > + dsi->bridge.pre_enable_prev_first = true; > > > > Setting dsi->bridge.pre_enable_prev_first on what is presumably the > > DSI host controller seems a little odd. > > Same question again - what bridge are you expecting to be upstream of > > the DSI host that needs to be preenabled before it? Whilst it's > > possible that there's another bridge, I'd have expected that to be the > > first link from your encoder as they appear to both belong to the same > > bit of driver. > > Let me answer all together here. I can explain a bit about one of the > pipelines used in Exynos. Exynos DSI DRM drivers have some strict host > initialization which is not the same as what we used in i.MX8M even > though it uses the same DSIM IP. > > Exynos5433 Decon -> Exynos MIC -> Exynos DSI -> s6e3ha2 DSI panel > > Here MIC is the bridge, Exynos DSI is the bridge and the requirement > is to expect the upstream bridge to pre_enable first from DSI which > means the MIC. That makes sense for the pre_enable_prev_first flag. The drm_bridge_attach(... list_first_entry_or_null) still seems a little weird. I think you are making the assumption that there is only ever going to be the zero or one bridge (the MIC) between encoder and DSI bridge - the DSI bridge is linking itself to the first entry off the encoder bridge_chain (or NULL to link to the encoder). Is that reasonable? I've no idea! I must confess to not having looked at the attaching sequence recently, and I'm about to head home for the weekend. I have no real knowledge of how Exynos is working, and am aware that you're having to rejuggle stuff to try and support i.MX8M and Exynos, so leave that one up to you. Cheers Dave ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v11 3/3] drm: exynos: dsi: Restore proper bridge chain order 2023-01-20 19:42 ` Dave Stevenson @ 2023-01-23 9:16 ` Frieder Schrempf -1 siblings, 0 replies; 29+ messages in thread From: Frieder Schrempf @ 2023-01-23 9:16 UTC (permalink / raw) To: Dave Stevenson, Jagan Teki Cc: Marek Vasut, linux-samsung-soc, linux-amarula, Seung-Woo Kim, Neil Armstrong, Robert Foss, Kyungmin Park, dri-devel, Andrzej Hajda, Sam Ravnborg, Marek Szyprowski On 20.01.23 20:42, Dave Stevenson wrote: > Hi Jagan > > On Fri, 20 Jan 2023 at 19:10, Jagan Teki <jagan@amarulasolutions.com> wrote: >> >> Hi Dave, >> >> On Sat, Jan 21, 2023 at 12:26 AM Dave Stevenson >> <dave.stevenson@raspberrypi.com> wrote: >>> >>> Hi Jagan >>> >>> Responding due to Marek's comment on the "Add Samsung MIPI DSIM >>> bridge" series, although I know very little about the Exynos >>> specifics, and may well be missing context of what you're trying to >>> achieve. >>> >>> On Mon, 12 Dec 2022 at 18:29, Jagan Teki <jagan@amarulasolutions.com> wrote: >>>> >>>> Restore the proper bridge chain by finding the previous bridge >>>> in the chain instead of passing NULL. >>>> >>>> This establishes a proper bridge chain while attaching downstream >>>> bridges. >>>> >>>> Reviewed-by: Marek Vasut <marex@denx.de> >>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >>>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> >>>> --- >>>> Changes for v11: >>>> - add bridge.pre_enable_prev_first >>>> Changes for v10: >>>> - collect Marek review tag >>>> >>>> drivers/gpu/drm/exynos/exynos_drm_dsi.c | 9 +++++++-- >>>> 1 file changed, 7 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c >>>> index ec673223d6b7..9d10a89d28f1 100644 >>>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c >>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c >>>> @@ -1428,7 +1428,8 @@ static int exynos_dsi_attach(struct drm_bridge *bridge, >>>> { >>>> struct exynos_dsi *dsi = bridge_to_dsi(bridge); >>>> >>>> - return drm_bridge_attach(bridge->encoder, dsi->out_bridge, NULL, flags); >>>> + return drm_bridge_attach(bridge->encoder, dsi->out_bridge, bridge, >>>> + flags); >>> >>> Agreed on this change. >>> >>>> } >>>> >>>> static const struct drm_bridge_funcs exynos_dsi_bridge_funcs = { >>>> @@ -1474,7 +1475,10 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host, >>>> >>>> drm_bridge_add(&dsi->bridge); >>>> >>>> - drm_bridge_attach(encoder, &dsi->bridge, NULL, 0); >>>> + drm_bridge_attach(encoder, &dsi->bridge, >>>> + list_first_entry_or_null(&encoder->bridge_chain, >>>> + struct drm_bridge, >>>> + chain_node), 0); >>> >>> What bridge are you expecting between the encoder and this bridge? >>> The encoder is the drm_simple_encoder_init encoder that you've created >>> in exynos_dsi_bind, so separating that from the bridge you're also >>> creating here seems weird. >>> >>>> >>>> /* >>>> * This is a temporary solution and should be made by more generic way. >>>> @@ -1709,6 +1713,7 @@ static int exynos_dsi_probe(struct platform_device *pdev) >>>> dsi->bridge.funcs = &exynos_dsi_bridge_funcs; >>>> dsi->bridge.of_node = dev->of_node; >>>> dsi->bridge.type = DRM_MODE_CONNECTOR_DSI; >>>> + dsi->bridge.pre_enable_prev_first = true; >>> >>> Setting dsi->bridge.pre_enable_prev_first on what is presumably the >>> DSI host controller seems a little odd. >>> Same question again - what bridge are you expecting to be upstream of >>> the DSI host that needs to be preenabled before it? Whilst it's >>> possible that there's another bridge, I'd have expected that to be the >>> first link from your encoder as they appear to both belong to the same >>> bit of driver. >> >> Let me answer all together here. I can explain a bit about one of the >> pipelines used in Exynos. Exynos DSI DRM drivers have some strict host >> initialization which is not the same as what we used in i.MX8M even >> though it uses the same DSIM IP. >> >> Exynos5433 Decon -> Exynos MIC -> Exynos DSI -> s6e3ha2 DSI panel >> >> Here MIC is the bridge, Exynos DSI is the bridge and the requirement >> is to expect the upstream bridge to pre_enable first from DSI which >> means the MIC. > > That makes sense for the pre_enable_prev_first flag. > > The drm_bridge_attach(... list_first_entry_or_null) still seems a > little weird. I think you are making the assumption that there is only > ever going to be the zero or one bridge (the MIC) between encoder and > DSI bridge - the DSI bridge is linking itself to the first entry off > the encoder bridge_chain (or NULL to link to the encoder). Is that > reasonable? I've no idea! I think the assumption is reasonable for now, as it covers both types of chains this driver is currently used in (Exynos and i.MX). And IIUC this change is mainly needed for (backward) compatibility with the somewhat "special" chain in Exynos. > > I must confess to not having looked at the attaching sequence > recently, and I'm about to head home for the weekend. > I have no real knowledge of how Exynos is working, and am aware that > you're having to rejuggle stuff to try and support i.MX8M and Exynos, > so leave that one up to you. I strongly vote for leaving this as is for now. We already spent too much time finding a satisfying solution that covers Exynos and i.MX. There might be room for improvements in the future, but in my opinion this is good enough to get merged. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v11 3/3] drm: exynos: dsi: Restore proper bridge chain order @ 2023-01-23 9:16 ` Frieder Schrempf 0 siblings, 0 replies; 29+ messages in thread From: Frieder Schrempf @ 2023-01-23 9:16 UTC (permalink / raw) To: Dave Stevenson, Jagan Teki Cc: Marek Vasut, linux-samsung-soc, Sam Ravnborg, Seung-Woo Kim, Neil Armstrong, Robert Foss, Kyungmin Park, dri-devel, Andrzej Hajda, linux-amarula, Marek Szyprowski On 20.01.23 20:42, Dave Stevenson wrote: > Hi Jagan > > On Fri, 20 Jan 2023 at 19:10, Jagan Teki <jagan@amarulasolutions.com> wrote: >> >> Hi Dave, >> >> On Sat, Jan 21, 2023 at 12:26 AM Dave Stevenson >> <dave.stevenson@raspberrypi.com> wrote: >>> >>> Hi Jagan >>> >>> Responding due to Marek's comment on the "Add Samsung MIPI DSIM >>> bridge" series, although I know very little about the Exynos >>> specifics, and may well be missing context of what you're trying to >>> achieve. >>> >>> On Mon, 12 Dec 2022 at 18:29, Jagan Teki <jagan@amarulasolutions.com> wrote: >>>> >>>> Restore the proper bridge chain by finding the previous bridge >>>> in the chain instead of passing NULL. >>>> >>>> This establishes a proper bridge chain while attaching downstream >>>> bridges. >>>> >>>> Reviewed-by: Marek Vasut <marex@denx.de> >>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >>>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> >>>> --- >>>> Changes for v11: >>>> - add bridge.pre_enable_prev_first >>>> Changes for v10: >>>> - collect Marek review tag >>>> >>>> drivers/gpu/drm/exynos/exynos_drm_dsi.c | 9 +++++++-- >>>> 1 file changed, 7 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c >>>> index ec673223d6b7..9d10a89d28f1 100644 >>>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c >>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c >>>> @@ -1428,7 +1428,8 @@ static int exynos_dsi_attach(struct drm_bridge *bridge, >>>> { >>>> struct exynos_dsi *dsi = bridge_to_dsi(bridge); >>>> >>>> - return drm_bridge_attach(bridge->encoder, dsi->out_bridge, NULL, flags); >>>> + return drm_bridge_attach(bridge->encoder, dsi->out_bridge, bridge, >>>> + flags); >>> >>> Agreed on this change. >>> >>>> } >>>> >>>> static const struct drm_bridge_funcs exynos_dsi_bridge_funcs = { >>>> @@ -1474,7 +1475,10 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host, >>>> >>>> drm_bridge_add(&dsi->bridge); >>>> >>>> - drm_bridge_attach(encoder, &dsi->bridge, NULL, 0); >>>> + drm_bridge_attach(encoder, &dsi->bridge, >>>> + list_first_entry_or_null(&encoder->bridge_chain, >>>> + struct drm_bridge, >>>> + chain_node), 0); >>> >>> What bridge are you expecting between the encoder and this bridge? >>> The encoder is the drm_simple_encoder_init encoder that you've created >>> in exynos_dsi_bind, so separating that from the bridge you're also >>> creating here seems weird. >>> >>>> >>>> /* >>>> * This is a temporary solution and should be made by more generic way. >>>> @@ -1709,6 +1713,7 @@ static int exynos_dsi_probe(struct platform_device *pdev) >>>> dsi->bridge.funcs = &exynos_dsi_bridge_funcs; >>>> dsi->bridge.of_node = dev->of_node; >>>> dsi->bridge.type = DRM_MODE_CONNECTOR_DSI; >>>> + dsi->bridge.pre_enable_prev_first = true; >>> >>> Setting dsi->bridge.pre_enable_prev_first on what is presumably the >>> DSI host controller seems a little odd. >>> Same question again - what bridge are you expecting to be upstream of >>> the DSI host that needs to be preenabled before it? Whilst it's >>> possible that there's another bridge, I'd have expected that to be the >>> first link from your encoder as they appear to both belong to the same >>> bit of driver. >> >> Let me answer all together here. I can explain a bit about one of the >> pipelines used in Exynos. Exynos DSI DRM drivers have some strict host >> initialization which is not the same as what we used in i.MX8M even >> though it uses the same DSIM IP. >> >> Exynos5433 Decon -> Exynos MIC -> Exynos DSI -> s6e3ha2 DSI panel >> >> Here MIC is the bridge, Exynos DSI is the bridge and the requirement >> is to expect the upstream bridge to pre_enable first from DSI which >> means the MIC. > > That makes sense for the pre_enable_prev_first flag. > > The drm_bridge_attach(... list_first_entry_or_null) still seems a > little weird. I think you are making the assumption that there is only > ever going to be the zero or one bridge (the MIC) between encoder and > DSI bridge - the DSI bridge is linking itself to the first entry off > the encoder bridge_chain (or NULL to link to the encoder). Is that > reasonable? I've no idea! I think the assumption is reasonable for now, as it covers both types of chains this driver is currently used in (Exynos and i.MX). And IIUC this change is mainly needed for (backward) compatibility with the somewhat "special" chain in Exynos. > > I must confess to not having looked at the attaching sequence > recently, and I'm about to head home for the weekend. > I have no real knowledge of how Exynos is working, and am aware that > you're having to rejuggle stuff to try and support i.MX8M and Exynos, > so leave that one up to you. I strongly vote for leaving this as is for now. We already spent too much time finding a satisfying solution that covers Exynos and i.MX. There might be room for improvements in the future, but in my opinion this is good enough to get merged. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v11 3/3] drm: exynos: dsi: Restore proper bridge chain order 2023-01-20 19:42 ` Dave Stevenson @ 2023-01-23 10:34 ` Jagan Teki -1 siblings, 0 replies; 29+ messages in thread From: Jagan Teki @ 2023-01-23 10:34 UTC (permalink / raw) To: Dave Stevenson Cc: Marek Szyprowski, Inki Dae, Seung-Woo Kim, Kyungmin Park, Neil Armstrong, Robert Foss, Andrzej Hajda, Sam Ravnborg, Marek Vasut, linux-samsung-soc, linux-amarula, dri-devel Hi Dave, On Sat, Jan 21, 2023 at 1:12 AM Dave Stevenson <dave.stevenson@raspberrypi.com> wrote: > > Hi Jagan > > On Fri, 20 Jan 2023 at 19:10, Jagan Teki <jagan@amarulasolutions.com> wrote: > > > > Hi Dave, > > > > On Sat, Jan 21, 2023 at 12:26 AM Dave Stevenson > > <dave.stevenson@raspberrypi.com> wrote: > > > > > > Hi Jagan > > > > > > Responding due to Marek's comment on the "Add Samsung MIPI DSIM > > > bridge" series, although I know very little about the Exynos > > > specifics, and may well be missing context of what you're trying to > > > achieve. > > > > > > On Mon, 12 Dec 2022 at 18:29, Jagan Teki <jagan@amarulasolutions.com> wrote: > > > > > > > > Restore the proper bridge chain by finding the previous bridge > > > > in the chain instead of passing NULL. > > > > > > > > This establishes a proper bridge chain while attaching downstream > > > > bridges. > > > > > > > > Reviewed-by: Marek Vasut <marex@denx.de> > > > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > > > > --- > > > > Changes for v11: > > > > - add bridge.pre_enable_prev_first > > > > Changes for v10: > > > > - collect Marek review tag > > > > > > > > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 9 +++++++-- > > > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > > > index ec673223d6b7..9d10a89d28f1 100644 > > > > --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > > > +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > > > @@ -1428,7 +1428,8 @@ static int exynos_dsi_attach(struct drm_bridge *bridge, > > > > { > > > > struct exynos_dsi *dsi = bridge_to_dsi(bridge); > > > > > > > > - return drm_bridge_attach(bridge->encoder, dsi->out_bridge, NULL, flags); > > > > + return drm_bridge_attach(bridge->encoder, dsi->out_bridge, bridge, > > > > + flags); > > > > > > Agreed on this change. > > > > > > > } > > > > > > > > static const struct drm_bridge_funcs exynos_dsi_bridge_funcs = { > > > > @@ -1474,7 +1475,10 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host, > > > > > > > > drm_bridge_add(&dsi->bridge); > > > > > > > > - drm_bridge_attach(encoder, &dsi->bridge, NULL, 0); > > > > + drm_bridge_attach(encoder, &dsi->bridge, > > > > + list_first_entry_or_null(&encoder->bridge_chain, > > > > + struct drm_bridge, > > > > + chain_node), 0); > > > > > > What bridge are you expecting between the encoder and this bridge? > > > The encoder is the drm_simple_encoder_init encoder that you've created > > > in exynos_dsi_bind, so separating that from the bridge you're also > > > creating here seems weird. > > > > > > > > > > > /* > > > > * This is a temporary solution and should be made by more generic way. > > > > @@ -1709,6 +1713,7 @@ static int exynos_dsi_probe(struct platform_device *pdev) > > > > dsi->bridge.funcs = &exynos_dsi_bridge_funcs; > > > > dsi->bridge.of_node = dev->of_node; > > > > dsi->bridge.type = DRM_MODE_CONNECTOR_DSI; > > > > + dsi->bridge.pre_enable_prev_first = true; > > > > > > Setting dsi->bridge.pre_enable_prev_first on what is presumably the > > > DSI host controller seems a little odd. > > > Same question again - what bridge are you expecting to be upstream of > > > the DSI host that needs to be preenabled before it? Whilst it's > > > possible that there's another bridge, I'd have expected that to be the > > > first link from your encoder as they appear to both belong to the same > > > bit of driver. > > > > Let me answer all together here. I can explain a bit about one of the > > pipelines used in Exynos. Exynos DSI DRM drivers have some strict host > > initialization which is not the same as what we used in i.MX8M even > > though it uses the same DSIM IP. > > > > Exynos5433 Decon -> Exynos MIC -> Exynos DSI -> s6e3ha2 DSI panel > > > > Here MIC is the bridge, Exynos DSI is the bridge and the requirement > > is to expect the upstream bridge to pre_enable first from DSI which > > means the MIC. > > That makes sense for the pre_enable_prev_first flag. > > The drm_bridge_attach(... list_first_entry_or_null) still seems a > little weird. I think you are making the assumption that there is only > ever going to be the zero or one bridge (the MIC) between encoder and > DSI bridge - the DSI bridge is linking itself to the first entry off > the encoder bridge_chain (or NULL to link to the encoder). Is that > reasonable? I've no idea! That is true, the reason would be that DSI bridges still use an encoder, usually, the first bridge in the chain will use an encoder and subsequent bridges are fully bridge-driven drivers (without an encoder) in order to follow the DRM bridge chain topology. Unfortunately, the Exynos DRM drivers follow component-based binding so DSI is part of that topology any attempt to exclude the encoder from it not compatible with Exynos DRM drivers. So, in order to make the bridge chain work properly we assume that linking. I think this bridge attach from the DSI driver bind can be dropped if we make Exynos DSIM as an independent bridge driver. Jagan. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v11 3/3] drm: exynos: dsi: Restore proper bridge chain order @ 2023-01-23 10:34 ` Jagan Teki 0 siblings, 0 replies; 29+ messages in thread From: Jagan Teki @ 2023-01-23 10:34 UTC (permalink / raw) To: Dave Stevenson Cc: Marek Vasut, linux-samsung-soc, linux-amarula, Seung-Woo Kim, Neil Armstrong, Robert Foss, Kyungmin Park, dri-devel, Andrzej Hajda, Sam Ravnborg, Marek Szyprowski Hi Dave, On Sat, Jan 21, 2023 at 1:12 AM Dave Stevenson <dave.stevenson@raspberrypi.com> wrote: > > Hi Jagan > > On Fri, 20 Jan 2023 at 19:10, Jagan Teki <jagan@amarulasolutions.com> wrote: > > > > Hi Dave, > > > > On Sat, Jan 21, 2023 at 12:26 AM Dave Stevenson > > <dave.stevenson@raspberrypi.com> wrote: > > > > > > Hi Jagan > > > > > > Responding due to Marek's comment on the "Add Samsung MIPI DSIM > > > bridge" series, although I know very little about the Exynos > > > specifics, and may well be missing context of what you're trying to > > > achieve. > > > > > > On Mon, 12 Dec 2022 at 18:29, Jagan Teki <jagan@amarulasolutions.com> wrote: > > > > > > > > Restore the proper bridge chain by finding the previous bridge > > > > in the chain instead of passing NULL. > > > > > > > > This establishes a proper bridge chain while attaching downstream > > > > bridges. > > > > > > > > Reviewed-by: Marek Vasut <marex@denx.de> > > > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > > > > --- > > > > Changes for v11: > > > > - add bridge.pre_enable_prev_first > > > > Changes for v10: > > > > - collect Marek review tag > > > > > > > > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 9 +++++++-- > > > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > > > index ec673223d6b7..9d10a89d28f1 100644 > > > > --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > > > +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > > > > @@ -1428,7 +1428,8 @@ static int exynos_dsi_attach(struct drm_bridge *bridge, > > > > { > > > > struct exynos_dsi *dsi = bridge_to_dsi(bridge); > > > > > > > > - return drm_bridge_attach(bridge->encoder, dsi->out_bridge, NULL, flags); > > > > + return drm_bridge_attach(bridge->encoder, dsi->out_bridge, bridge, > > > > + flags); > > > > > > Agreed on this change. > > > > > > > } > > > > > > > > static const struct drm_bridge_funcs exynos_dsi_bridge_funcs = { > > > > @@ -1474,7 +1475,10 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host, > > > > > > > > drm_bridge_add(&dsi->bridge); > > > > > > > > - drm_bridge_attach(encoder, &dsi->bridge, NULL, 0); > > > > + drm_bridge_attach(encoder, &dsi->bridge, > > > > + list_first_entry_or_null(&encoder->bridge_chain, > > > > + struct drm_bridge, > > > > + chain_node), 0); > > > > > > What bridge are you expecting between the encoder and this bridge? > > > The encoder is the drm_simple_encoder_init encoder that you've created > > > in exynos_dsi_bind, so separating that from the bridge you're also > > > creating here seems weird. > > > > > > > > > > > /* > > > > * This is a temporary solution and should be made by more generic way. > > > > @@ -1709,6 +1713,7 @@ static int exynos_dsi_probe(struct platform_device *pdev) > > > > dsi->bridge.funcs = &exynos_dsi_bridge_funcs; > > > > dsi->bridge.of_node = dev->of_node; > > > > dsi->bridge.type = DRM_MODE_CONNECTOR_DSI; > > > > + dsi->bridge.pre_enable_prev_first = true; > > > > > > Setting dsi->bridge.pre_enable_prev_first on what is presumably the > > > DSI host controller seems a little odd. > > > Same question again - what bridge are you expecting to be upstream of > > > the DSI host that needs to be preenabled before it? Whilst it's > > > possible that there's another bridge, I'd have expected that to be the > > > first link from your encoder as they appear to both belong to the same > > > bit of driver. > > > > Let me answer all together here. I can explain a bit about one of the > > pipelines used in Exynos. Exynos DSI DRM drivers have some strict host > > initialization which is not the same as what we used in i.MX8M even > > though it uses the same DSIM IP. > > > > Exynos5433 Decon -> Exynos MIC -> Exynos DSI -> s6e3ha2 DSI panel > > > > Here MIC is the bridge, Exynos DSI is the bridge and the requirement > > is to expect the upstream bridge to pre_enable first from DSI which > > means the MIC. > > That makes sense for the pre_enable_prev_first flag. > > The drm_bridge_attach(... list_first_entry_or_null) still seems a > little weird. I think you are making the assumption that there is only > ever going to be the zero or one bridge (the MIC) between encoder and > DSI bridge - the DSI bridge is linking itself to the first entry off > the encoder bridge_chain (or NULL to link to the encoder). Is that > reasonable? I've no idea! That is true, the reason would be that DSI bridges still use an encoder, usually, the first bridge in the chain will use an encoder and subsequent bridges are fully bridge-driven drivers (without an encoder) in order to follow the DRM bridge chain topology. Unfortunately, the Exynos DRM drivers follow component-based binding so DSI is part of that topology any attempt to exclude the encoder from it not compatible with Exynos DRM drivers. So, in order to make the bridge chain work properly we assume that linking. I think this bridge attach from the DSI driver bind can be dropped if we make Exynos DSIM as an independent bridge driver. Jagan. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v11 0/3] drm: exynos: dsi: Restore the bridge chain 2022-12-12 18:29 ` Jagan Teki @ 2023-01-06 17:56 ` Jagan Teki -1 siblings, 0 replies; 29+ messages in thread From: Jagan Teki @ 2023-01-06 17:56 UTC (permalink / raw) To: Marek Szyprowski, Inki Dae, Seung-Woo Kim, Kyungmin Park, Neil Armstrong, Robert Foss, Andrzej Hajda, Sam Ravnborg Cc: Marek Vasut, linux-samsung-soc, dri-devel, linux-amarula On Mon, Dec 12, 2022 at 11:59 PM Jagan Teki <jagan@amarulasolutions.com> wrote: > > Split the Exynos DSI bridge chain update patches from Samsung DSIM > bridge driver for easy to apply. > > Changes for v11: > - enable bridge.pre_enable_prev_first > Changes for v10: > - collect Marek.V Review tag Any update on this? Jagan. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v11 0/3] drm: exynos: dsi: Restore the bridge chain @ 2023-01-06 17:56 ` Jagan Teki 0 siblings, 0 replies; 29+ messages in thread From: Jagan Teki @ 2023-01-06 17:56 UTC (permalink / raw) To: Marek Szyprowski, Inki Dae, Seung-Woo Kim, Kyungmin Park, Neil Armstrong, Robert Foss, Andrzej Hajda, Sam Ravnborg Cc: Marek Vasut, linux-samsung-soc, linux-amarula, dri-devel On Mon, Dec 12, 2022 at 11:59 PM Jagan Teki <jagan@amarulasolutions.com> wrote: > > Split the Exynos DSI bridge chain update patches from Samsung DSIM > bridge driver for easy to apply. > > Changes for v11: > - enable bridge.pre_enable_prev_first > Changes for v10: > - collect Marek.V Review tag Any update on this? Jagan. ^ permalink raw reply [flat|nested] 29+ messages in thread
* RE: [PATCH v11 0/3] drm: exynos: dsi: Restore the bridge chain 2023-01-06 17:56 ` Jagan Teki @ 2023-01-12 2:26 ` 대인기/Tizen Platform Lab(SR)/삼성전자 -1 siblings, 0 replies; 29+ messages in thread From: 대인기/Tizen Platform Lab(SR)/삼성전자 @ 2023-01-12 2:26 UTC (permalink / raw) To: 'Jagan Teki', 'Marek Szyprowski', 'Seung-Woo Kim', 'Kyungmin Park', 'Neil Armstrong', 'Robert Foss', 'Andrzej Hajda', 'Sam Ravnborg', thierry.reding Cc: 'Marek Vasut', linux-samsung-soc, dri-devel, 'linux-amarula' Hi Jagan Teki, Sorry for late. > -----Original Message----- > From: Jagan Teki <jagan@amarulasolutions.com> > Sent: Saturday, January 7, 2023 2:56 AM > To: Marek Szyprowski <m.szyprowski@samsung.com>; Inki Dae > <inki.dae@samsung.com>; Seung-Woo Kim <sw0312.kim@samsung.com>; Kyungmin Park > <kyungmin.park@samsung.com>; Neil Armstrong <narmstrong@linaro.org>; Robert > Foss <robert.foss@linaro.org>; Andrzej Hajda <andrzej.hajda@intel.com>; Sam > Ravnborg <sam@ravnborg.org> > Cc: Marek Vasut <marex@denx.de>; linux-samsung-soc@vger.kernel.org; dri- > devel@lists.freedesktop.org; linux-amarula <linux- > amarula@amarulasolutions.com> > Subject: Re: [PATCH v11 0/3] drm: exynos: dsi: Restore the bridge chain > > On Mon, Dec 12, 2022 at 11:59 PM Jagan Teki <jagan@amarulasolutions.com> wrote: > > > > Split the Exynos DSI bridge chain update patches from Samsung DSIM > > bridge driver for easy to apply. > > > > Changes for v11: > > - enable bridge.pre_enable_prev_first > > Changes for v10: > > - collect Marek.V Review tag > > Any update on this? > Added Thierry Reding who is a maintainer of DRM panel drivers. I will pick this patch series - including panel and bride patches - up. Thierry and Andrzej, please let me know if any problem. Thanks, Inki Dae > Jagan. ^ permalink raw reply [flat|nested] 29+ messages in thread
* RE: [PATCH v11 0/3] drm: exynos: dsi: Restore the bridge chain @ 2023-01-12 2:26 ` 대인기/Tizen Platform Lab(SR)/삼성전자 0 siblings, 0 replies; 29+ messages in thread From: 대인기/Tizen Platform Lab(SR)/삼성전자 @ 2023-01-12 2:26 UTC (permalink / raw) To: 'Jagan Teki', 'Marek Szyprowski', 'Seung-Woo Kim', 'Kyungmin Park', 'Neil Armstrong', 'Robert Foss', 'Andrzej Hajda', 'Sam Ravnborg', thierry.reding Cc: 'Marek Vasut', linux-samsung-soc, 'linux-amarula', dri-devel Hi Jagan Teki, Sorry for late. > -----Original Message----- > From: Jagan Teki <jagan@amarulasolutions.com> > Sent: Saturday, January 7, 2023 2:56 AM > To: Marek Szyprowski <m.szyprowski@samsung.com>; Inki Dae > <inki.dae@samsung.com>; Seung-Woo Kim <sw0312.kim@samsung.com>; Kyungmin Park > <kyungmin.park@samsung.com>; Neil Armstrong <narmstrong@linaro.org>; Robert > Foss <robert.foss@linaro.org>; Andrzej Hajda <andrzej.hajda@intel.com>; Sam > Ravnborg <sam@ravnborg.org> > Cc: Marek Vasut <marex@denx.de>; linux-samsung-soc@vger.kernel.org; dri- > devel@lists.freedesktop.org; linux-amarula <linux- > amarula@amarulasolutions.com> > Subject: Re: [PATCH v11 0/3] drm: exynos: dsi: Restore the bridge chain > > On Mon, Dec 12, 2022 at 11:59 PM Jagan Teki <jagan@amarulasolutions.com> wrote: > > > > Split the Exynos DSI bridge chain update patches from Samsung DSIM > > bridge driver for easy to apply. > > > > Changes for v11: > > - enable bridge.pre_enable_prev_first > > Changes for v10: > > - collect Marek.V Review tag > > Any update on this? > Added Thierry Reding who is a maintainer of DRM panel drivers. I will pick this patch series - including panel and bride patches - up. Thierry and Andrzej, please let me know if any problem. Thanks, Inki Dae > Jagan. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v11 0/3] drm: exynos: dsi: Restore the bridge chain 2023-01-12 2:26 ` 대인기/Tizen Platform Lab(SR)/삼성전자 @ 2023-01-19 18:19 ` Jagan Teki -1 siblings, 0 replies; 29+ messages in thread From: Jagan Teki @ 2023-01-19 18:19 UTC (permalink / raw) To: 대인기/Tizen Platform Lab(SR)/삼성전자 Cc: Marek Szyprowski, Seung-Woo Kim, Kyungmin Park, Neil Armstrong, Robert Foss, Andrzej Hajda, Sam Ravnborg, thierry.reding, Marek Vasut, linux-samsung-soc, dri-devel, linux-amarula Hi Inki Dae, On Thu, Jan 12, 2023 at 7:56 AM 대인기/Tizen Platform Lab(SR)/삼성전자 <inki.dae@samsung.com> wrote: > > Hi Jagan Teki, > > Sorry for late. > > > -----Original Message----- > > From: Jagan Teki <jagan@amarulasolutions.com> > > Sent: Saturday, January 7, 2023 2:56 AM > > To: Marek Szyprowski <m.szyprowski@samsung.com>; Inki Dae > > <inki.dae@samsung.com>; Seung-Woo Kim <sw0312.kim@samsung.com>; Kyungmin Park > > <kyungmin.park@samsung.com>; Neil Armstrong <narmstrong@linaro.org>; Robert > > Foss <robert.foss@linaro.org>; Andrzej Hajda <andrzej.hajda@intel.com>; Sam > > Ravnborg <sam@ravnborg.org> > > Cc: Marek Vasut <marex@denx.de>; linux-samsung-soc@vger.kernel.org; dri- > > devel@lists.freedesktop.org; linux-amarula <linux- > > amarula@amarulasolutions.com> > > Subject: Re: [PATCH v11 0/3] drm: exynos: dsi: Restore the bridge chain > > > > On Mon, Dec 12, 2022 at 11:59 PM Jagan Teki <jagan@amarulasolutions.com> wrote: > > > > > > Split the Exynos DSI bridge chain update patches from Samsung DSIM > > > bridge driver for easy to apply. > > > > > > Changes for v11: > > > - enable bridge.pre_enable_prev_first > > > Changes for v10: > > > - collect Marek.V Review tag > > > > Any update on this? > > > > Added Thierry Reding who is a maintainer of DRM panel drivers. > > I will pick this patch series - including panel and bride patches - up. > > Thierry and Andrzej, please let me know if any problem. Any further update on this? Thanks, Jagan. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v11 0/3] drm: exynos: dsi: Restore the bridge chain @ 2023-01-19 18:19 ` Jagan Teki 0 siblings, 0 replies; 29+ messages in thread From: Jagan Teki @ 2023-01-19 18:19 UTC (permalink / raw) To: 대인기/Tizen Platform Lab(SR)/삼성전자 Cc: Marek Vasut, linux-samsung-soc, linux-amarula, Seung-Woo Kim, Neil Armstrong, Robert Foss, Kyungmin Park, thierry.reding, dri-devel, Andrzej Hajda, Sam Ravnborg, Marek Szyprowski Hi Inki Dae, On Thu, Jan 12, 2023 at 7:56 AM 대인기/Tizen Platform Lab(SR)/삼성전자 <inki.dae@samsung.com> wrote: > > Hi Jagan Teki, > > Sorry for late. > > > -----Original Message----- > > From: Jagan Teki <jagan@amarulasolutions.com> > > Sent: Saturday, January 7, 2023 2:56 AM > > To: Marek Szyprowski <m.szyprowski@samsung.com>; Inki Dae > > <inki.dae@samsung.com>; Seung-Woo Kim <sw0312.kim@samsung.com>; Kyungmin Park > > <kyungmin.park@samsung.com>; Neil Armstrong <narmstrong@linaro.org>; Robert > > Foss <robert.foss@linaro.org>; Andrzej Hajda <andrzej.hajda@intel.com>; Sam > > Ravnborg <sam@ravnborg.org> > > Cc: Marek Vasut <marex@denx.de>; linux-samsung-soc@vger.kernel.org; dri- > > devel@lists.freedesktop.org; linux-amarula <linux- > > amarula@amarulasolutions.com> > > Subject: Re: [PATCH v11 0/3] drm: exynos: dsi: Restore the bridge chain > > > > On Mon, Dec 12, 2022 at 11:59 PM Jagan Teki <jagan@amarulasolutions.com> wrote: > > > > > > Split the Exynos DSI bridge chain update patches from Samsung DSIM > > > bridge driver for easy to apply. > > > > > > Changes for v11: > > > - enable bridge.pre_enable_prev_first > > > Changes for v10: > > > - collect Marek.V Review tag > > > > Any update on this? > > > > Added Thierry Reding who is a maintainer of DRM panel drivers. > > I will pick this patch series - including panel and bride patches - up. > > Thierry and Andrzej, please let me know if any problem. Any further update on this? Thanks, Jagan. ^ permalink raw reply [flat|nested] 29+ messages in thread
* RE: [PATCH v11 0/3] drm: exynos: dsi: Restore the bridge chain 2023-01-19 18:19 ` Jagan Teki @ 2023-01-26 6:20 ` 대인기/Tizen Platform Lab(SR)/삼성전자 -1 siblings, 0 replies; 29+ messages in thread From: 대인기/Tizen Platform Lab(SR)/삼성전자 @ 2023-01-26 6:20 UTC (permalink / raw) To: 'Jagan Teki' Cc: 'Marek Szyprowski', 'Seung-Woo Kim', 'Kyungmin Park', 'Neil Armstrong', 'Robert Foss', 'Andrzej Hajda', 'Sam Ravnborg', thierry.reding, 'Marek Vasut', linux-samsung-soc, dri-devel, 'linux-amarula' Hi Jagan Teki, > -----Original Message----- > From: Jagan Teki <jagan@amarulasolutions.com> > Sent: Friday, January 20, 2023 3:19 AM > To: 대인기/Tizen Platform Lab(SR)/삼성전자 <inki.dae@samsung.com> > Cc: Marek Szyprowski <m.szyprowski@samsung.com>; Seung-Woo Kim > <sw0312.kim@samsung.com>; Kyungmin Park <kyungmin.park@samsung.com>; Neil > Armstrong <narmstrong@linaro.org>; Robert Foss <robert.foss@linaro.org>; > Andrzej Hajda <andrzej.hajda@intel.com>; Sam Ravnborg <sam@ravnborg.org>; > thierry.reding@gmail.com; Marek Vasut <marex@denx.de>; linux-samsung- > soc@vger.kernel.org; dri-devel@lists.freedesktop.org; linux-amarula <linux- > amarula@amarulasolutions.com> > Subject: Re: [PATCH v11 0/3] drm: exynos: dsi: Restore the bridge chain > > Hi Inki Dae, > > On Thu, Jan 12, 2023 at 7:56 AM 대인기/Tizen Platform Lab(SR)/삼성전자 > <inki.dae@samsung.com> wrote: > > > > Hi Jagan Teki, > > > > Sorry for late. > > > > > -----Original Message----- > > > From: Jagan Teki <jagan@amarulasolutions.com> > > > Sent: Saturday, January 7, 2023 2:56 AM > > > To: Marek Szyprowski <m.szyprowski@samsung.com>; Inki Dae > > > <inki.dae@samsung.com>; Seung-Woo Kim <sw0312.kim@samsung.com>; Kyungmin > Park > > > <kyungmin.park@samsung.com>; Neil Armstrong <narmstrong@linaro.org>; > Robert > > > Foss <robert.foss@linaro.org>; Andrzej Hajda <andrzej.hajda@intel.com>; > Sam > > > Ravnborg <sam@ravnborg.org> > > > Cc: Marek Vasut <marex@denx.de>; linux-samsung-soc@vger.kernel.org; dri- > > > devel@lists.freedesktop.org; linux-amarula <linux- > > > amarula@amarulasolutions.com> > > > Subject: Re: [PATCH v11 0/3] drm: exynos: dsi: Restore the bridge chain > > > > > > On Mon, Dec 12, 2022 at 11:59 PM Jagan Teki <jagan@amarulasolutions.com> > wrote: > > > > > > > > Split the Exynos DSI bridge chain update patches from Samsung DSIM > > > > bridge driver for easy to apply. > > > > > > > > Changes for v11: > > > > - enable bridge.pre_enable_prev_first > > > > Changes for v10: > > > > - collect Marek.V Review tag > > > > > > Any update on this? > > > > > > > Added Thierry Reding who is a maintainer of DRM panel drivers. > > > > I will pick this patch series - including panel and bride patches - up. > > > > Thierry and Andrzej, please let me know if any problem. > > Any further update on this? Already merged to exynos-drm-next so this patch series will go to -next as long as no feedback from Thierry and Andrzej. I will request a PR this or next week. Thanks, Inki Dae > > Thanks, > Jagan. ^ permalink raw reply [flat|nested] 29+ messages in thread
* RE: [PATCH v11 0/3] drm: exynos: dsi: Restore the bridge chain @ 2023-01-26 6:20 ` 대인기/Tizen Platform Lab(SR)/삼성전자 0 siblings, 0 replies; 29+ messages in thread From: 대인기/Tizen Platform Lab(SR)/삼성전자 @ 2023-01-26 6:20 UTC (permalink / raw) To: 'Jagan Teki' Cc: 'Marek Vasut', linux-samsung-soc, 'linux-amarula', 'Seung-Woo Kim', 'Neil Armstrong', 'Robert Foss', 'Kyungmin Park', thierry.reding, dri-devel, 'Andrzej Hajda', 'Sam Ravnborg', 'Marek Szyprowski' Hi Jagan Teki, > -----Original Message----- > From: Jagan Teki <jagan@amarulasolutions.com> > Sent: Friday, January 20, 2023 3:19 AM > To: 대인기/Tizen Platform Lab(SR)/삼성전자 <inki.dae@samsung.com> > Cc: Marek Szyprowski <m.szyprowski@samsung.com>; Seung-Woo Kim > <sw0312.kim@samsung.com>; Kyungmin Park <kyungmin.park@samsung.com>; Neil > Armstrong <narmstrong@linaro.org>; Robert Foss <robert.foss@linaro.org>; > Andrzej Hajda <andrzej.hajda@intel.com>; Sam Ravnborg <sam@ravnborg.org>; > thierry.reding@gmail.com; Marek Vasut <marex@denx.de>; linux-samsung- > soc@vger.kernel.org; dri-devel@lists.freedesktop.org; linux-amarula <linux- > amarula@amarulasolutions.com> > Subject: Re: [PATCH v11 0/3] drm: exynos: dsi: Restore the bridge chain > > Hi Inki Dae, > > On Thu, Jan 12, 2023 at 7:56 AM 대인기/Tizen Platform Lab(SR)/삼성전자 > <inki.dae@samsung.com> wrote: > > > > Hi Jagan Teki, > > > > Sorry for late. > > > > > -----Original Message----- > > > From: Jagan Teki <jagan@amarulasolutions.com> > > > Sent: Saturday, January 7, 2023 2:56 AM > > > To: Marek Szyprowski <m.szyprowski@samsung.com>; Inki Dae > > > <inki.dae@samsung.com>; Seung-Woo Kim <sw0312.kim@samsung.com>; Kyungmin > Park > > > <kyungmin.park@samsung.com>; Neil Armstrong <narmstrong@linaro.org>; > Robert > > > Foss <robert.foss@linaro.org>; Andrzej Hajda <andrzej.hajda@intel.com>; > Sam > > > Ravnborg <sam@ravnborg.org> > > > Cc: Marek Vasut <marex@denx.de>; linux-samsung-soc@vger.kernel.org; dri- > > > devel@lists.freedesktop.org; linux-amarula <linux- > > > amarula@amarulasolutions.com> > > > Subject: Re: [PATCH v11 0/3] drm: exynos: dsi: Restore the bridge chain > > > > > > On Mon, Dec 12, 2022 at 11:59 PM Jagan Teki <jagan@amarulasolutions.com> > wrote: > > > > > > > > Split the Exynos DSI bridge chain update patches from Samsung DSIM > > > > bridge driver for easy to apply. > > > > > > > > Changes for v11: > > > > - enable bridge.pre_enable_prev_first > > > > Changes for v10: > > > > - collect Marek.V Review tag > > > > > > Any update on this? > > > > > > > Added Thierry Reding who is a maintainer of DRM panel drivers. > > > > I will pick this patch series - including panel and bride patches - up. > > > > Thierry and Andrzej, please let me know if any problem. > > Any further update on this? Already merged to exynos-drm-next so this patch series will go to -next as long as no feedback from Thierry and Andrzej. I will request a PR this or next week. Thanks, Inki Dae > > Thanks, > Jagan. ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2023-01-26 6:20 UTC | newest] Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-12-12 18:29 [PATCH v11 0/3] drm: exynos: dsi: Restore the bridge chain Jagan Teki 2022-12-12 18:29 ` Jagan Teki 2022-12-12 18:29 ` [PATCH v11 1/3] drm: panel: Enable prepare_prev_first flag for samsung-s6e panels Jagan Teki 2022-12-12 18:29 ` Jagan Teki 2022-12-14 8:48 ` Frieder Schrempf 2022-12-12 18:29 ` [PATCH v11 2/3] drm/bridge: tc358764: Enable pre_enable_prev_first flag Jagan Teki 2022-12-12 18:29 ` Jagan Teki 2022-12-14 8:48 ` Frieder Schrempf 2022-12-12 18:29 ` [PATCH v11 3/3] drm: exynos: dsi: Restore proper bridge chain order Jagan Teki 2022-12-12 18:29 ` Jagan Teki 2022-12-14 8:50 ` Frieder Schrempf 2023-01-20 18:56 ` Dave Stevenson 2023-01-20 18:56 ` Dave Stevenson 2023-01-20 19:09 ` Jagan Teki 2023-01-20 19:09 ` Jagan Teki 2023-01-20 19:42 ` Dave Stevenson 2023-01-20 19:42 ` Dave Stevenson 2023-01-23 9:16 ` Frieder Schrempf 2023-01-23 9:16 ` Frieder Schrempf 2023-01-23 10:34 ` Jagan Teki 2023-01-23 10:34 ` Jagan Teki 2023-01-06 17:56 ` [PATCH v11 0/3] drm: exynos: dsi: Restore the bridge chain Jagan Teki 2023-01-06 17:56 ` Jagan Teki 2023-01-12 2:26 ` 대인기/Tizen Platform Lab(SR)/삼성전자 2023-01-12 2:26 ` 대인기/Tizen Platform Lab(SR)/삼성전자 2023-01-19 18:19 ` Jagan Teki 2023-01-19 18:19 ` Jagan Teki 2023-01-26 6:20 ` 대인기/Tizen Platform Lab(SR)/삼성전자 2023-01-26 6:20 ` 대인기/Tizen Platform Lab(SR)/삼성전자
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.