| Message ID | 20210704090230.26489-1-jagan@amarulasolutions.com |
|---|---|
| Headers | show |
| Series | drm: bridge: Samsung MIPI DSIM bridge | expand |
On 04.07.2021 11:02, Jagan Teki wrote: > Use drm_panel_bridge to replace manual panel and > bridge_chain handling code. > > This makes the driver simpler to allow all components > in the display pipeline to be treated as bridges by > cleaning the way to generic connector handling. > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> This breaks Exysos DSI driver operation (Trats board worked fine with only patches 1-2): [ 2.540066] exynos4-fb 11c00000.fimd: Adding to iommu group 0 [ 2.554733] OF: graph: no port node found in /soc/fimd@11c00000 [ 2.602819] [drm] Exynos DRM: using 11c00000.fimd device for DMA mapping operations [ 2.609649] exynos-drm exynos-drm: bound 11c00000.fimd (ops fimd_component_ops) [ 2.632558] exynos-drm exynos-drm: failed to bind 11c80000.dsi (ops exynos_dsi_component_ops): -22 [ 2.642263] exynos-drm exynos-drm: master bind failed: -22 [ 2.651017] exynos-drm: probe of exynos-drm failed with error -22 > --- > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 167 ++++-------------------- > 1 file changed, 23 insertions(+), 144 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > index d7d60aee465b..24f0b082ac6d 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > @@ -254,9 +254,6 @@ struct exynos_dsi_driver_data { > struct exynos_dsi { > struct drm_encoder encoder; > struct mipi_dsi_host dsi_host; > - struct drm_connector connector; > - struct drm_panel *panel; > - struct list_head bridge_chain; > struct drm_bridge bridge; > struct drm_bridge *out_bridge; > struct drm_device *drm; > @@ -287,7 +284,6 @@ struct exynos_dsi { > }; > > #define host_to_dsi(host) container_of(host, struct exynos_dsi, dsi_host) > -#define connector_to_dsi(c) container_of(c, struct exynos_dsi, connector) > > static inline struct exynos_dsi *bridge_to_dsi(struct drm_bridge *b) > { > @@ -1379,7 +1375,6 @@ static void exynos_dsi_unregister_te_irq(struct exynos_dsi *dsi) > static void exynos_dsi_bridge_enable(struct drm_bridge *bridge) > { > struct exynos_dsi *dsi = bridge_to_dsi(bridge); > - struct drm_bridge *iter; > int ret; > > if (dsi->state & DSIM_STATE_ENABLED) > @@ -1393,134 +1388,51 @@ static void exynos_dsi_bridge_enable(struct drm_bridge *bridge) > > dsi->state |= DSIM_STATE_ENABLED; > > - if (dsi->panel) { > - ret = drm_panel_prepare(dsi->panel); > - if (ret < 0) > - goto err_put_sync; > - } else { > - list_for_each_entry_reverse(iter, &dsi->bridge_chain, > - chain_node) { > - if (iter->funcs->pre_enable) > - iter->funcs->pre_enable(iter); > - } > - } > - > exynos_dsi_set_display_mode(dsi); > exynos_dsi_set_display_enable(dsi, true); > > - if (dsi->panel) { > - ret = drm_panel_enable(dsi->panel); > - if (ret < 0) > - goto err_display_disable; > - } else { > - list_for_each_entry(iter, &dsi->bridge_chain, chain_node) { > - if (iter->funcs->enable) > - iter->funcs->enable(iter); > - } > - } > - > dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE; > return; > - > -err_display_disable: > - exynos_dsi_set_display_enable(dsi, false); > - drm_panel_unprepare(dsi->panel); > - > -err_put_sync: > - dsi->state &= ~DSIM_STATE_ENABLED; > - pm_runtime_put(dsi->dev); > } > > static void exynos_dsi_bridge_disable(struct drm_bridge *bridge) > { > struct exynos_dsi *dsi = bridge_to_dsi(bridge); > - struct drm_bridge *iter; > > if (!(dsi->state & DSIM_STATE_ENABLED)) > return; > > dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE; > > - drm_panel_disable(dsi->panel); > - > - list_for_each_entry_reverse(iter, &dsi->bridge_chain, chain_node) { > - if (iter->funcs->disable) > - iter->funcs->disable(iter); > - } > - > exynos_dsi_set_display_enable(dsi, false); > - drm_panel_unprepare(dsi->panel); > - > - list_for_each_entry(iter, &dsi->bridge_chain, chain_node) { > - if (iter->funcs->post_disable) > - iter->funcs->post_disable(iter); > - } > > dsi->state &= ~DSIM_STATE_ENABLED; > pm_runtime_put_sync(dsi->dev); > } > > -static enum drm_connector_status > -exynos_dsi_detect(struct drm_connector *connector, bool force) > -{ > - return connector->status; > -} > - > -static void exynos_dsi_connector_destroy(struct drm_connector *connector) > +static int exynos_dsi_panel_or_bridge(struct exynos_dsi *dsi, > + struct device_node *node) > { > - drm_connector_unregister(connector); > - drm_connector_cleanup(connector); > - connector->dev = NULL; > -} > - > -static const struct drm_connector_funcs exynos_dsi_connector_funcs = { > - .detect = exynos_dsi_detect, > - .fill_modes = drm_helper_probe_single_connector_modes, > - .destroy = exynos_dsi_connector_destroy, > - .reset = drm_atomic_helper_connector_reset, > - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, > - .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > -}; > - > -static int exynos_dsi_get_modes(struct drm_connector *connector) > -{ > - struct exynos_dsi *dsi = connector_to_dsi(connector); > - > - if (dsi->panel) > - return drm_panel_get_modes(dsi->panel, connector); > - > - return 0; > -} > + struct drm_bridge *panel_bridge; > + struct drm_panel *panel; > > -static const struct drm_connector_helper_funcs exynos_dsi_connector_helper_funcs = { > - .get_modes = exynos_dsi_get_modes, > -}; > + panel_bridge = of_drm_find_bridge(node); > + if (!panel_bridge) { > + panel = of_drm_find_panel(node); > + if (!IS_ERR(panel)) { > + panel_bridge = drm_panel_bridge_add(panel); > + if (IS_ERR(panel_bridge)) > + return PTR_ERR(panel_bridge); > + } > + } > > -static int exynos_dsi_create_connector(struct exynos_dsi *dsi, > - struct drm_device *drm) > -{ > - struct drm_encoder *encoder = &dsi->encoder; > - struct drm_connector *connector = &dsi->connector; > - int ret; > + of_node_put(node); > > - connector->polled = DRM_CONNECTOR_POLL_HPD; > + dsi->out_bridge = panel_bridge; > > - ret = drm_connector_init(drm, connector, &exynos_dsi_connector_funcs, > - DRM_MODE_CONNECTOR_DSI); > - if (ret) { > - DRM_DEV_ERROR(dsi->dev, > - "Failed to initialize connector with drm\n"); > - return ret; > - } > + if (!dsi->out_bridge) > + return -EPROBE_DEFER; > > - connector->status = connector_status_disconnected; > - drm_connector_helper_add(connector, &exynos_dsi_connector_helper_funcs); > - drm_connector_attach_encoder(connector, encoder); > - if (!drm->registered) > - return 0; > - > - connector->funcs->reset(connector); > - drm_connector_register(connector); > return 0; > } > > @@ -1531,7 +1443,8 @@ static int exynos_dsi_bridge_attach(struct drm_bridge *bridge, > > dsi->drm = bridge->dev; > > - return 0; > + return drm_bridge_attach(bridge->encoder, dsi->out_bridge, bridge, > + flags); > } > > static const struct drm_bridge_funcs exynos_dsi_bridge_funcs = { > @@ -1546,32 +1459,12 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host, > struct mipi_dsi_device *device) > { > struct exynos_dsi *dsi = host_to_dsi(host); > - struct drm_encoder *encoder = &dsi->encoder; > struct drm_device *drm = dsi->drm; > - struct drm_bridge *out_bridge; > - > - out_bridge = of_drm_find_bridge(device->dev.of_node); > - if (out_bridge) { > - drm_bridge_attach(encoder, out_bridge, NULL, 0); > - dsi->out_bridge = out_bridge; > - list_splice_init(&encoder->bridge_chain, &dsi->bridge_chain); > - } else { > - int ret = exynos_dsi_create_connector(dsi, drm); > - > - if (ret) { > - DRM_DEV_ERROR(dsi->dev, > - "failed to create connector ret = %d\n", > - ret); > - drm_encoder_cleanup(encoder); > - return ret; > - } > + int ret; > > - dsi->panel = of_drm_find_panel(device->dev.of_node); > - if (IS_ERR(dsi->panel)) > - dsi->panel = NULL; > - else > - dsi->connector.status = connector_status_connected; > - } > + ret = exynos_dsi_panel_or_bridge(dsi, device->dev.of_node); > + if (ret) > + return ret; > > /* > * This is a temporary solution and should be made by more generic way. > @@ -1607,19 +1500,6 @@ static int exynos_dsi_host_detach(struct mipi_dsi_host *host, > struct exynos_dsi *dsi = host_to_dsi(host); > struct drm_device *drm = dsi->drm; > > - if (dsi->panel) { > - mutex_lock(&drm->mode_config.mutex); > - exynos_dsi_bridge_disable(&dsi->bridge); > - dsi->panel = NULL; > - dsi->connector.status = connector_status_disconnected; > - mutex_unlock(&drm->mode_config.mutex); > - } else { > - if (dsi->out_bridge->funcs->detach) > - dsi->out_bridge->funcs->detach(dsi->out_bridge); > - dsi->out_bridge = NULL; > - INIT_LIST_HEAD(&dsi->bridge_chain); > - } > - > if (drm->mode_config.poll_enabled) > drm_kms_helper_hotplug_event(drm); > > @@ -1770,7 +1650,6 @@ static int exynos_dsi_probe(struct platform_device *pdev) > init_completion(&dsi->completed); > spin_lock_init(&dsi->transfer_lock); > INIT_LIST_HEAD(&dsi->transfer_list); > - INIT_LIST_HEAD(&dsi->bridge_chain); > > dsi->dsi_host.ops = &exynos_dsi_ops; > dsi->dsi_host.dev = dev; Best regards
On Mon, Jul 5, 2021 at 5:18 PM Marek Szyprowski <m.szyprowski@samsung.com> wrote: > > On 04.07.2021 11:02, Jagan Teki wrote: > > Use drm_panel_bridge to replace manual panel and > > bridge_chain handling code. > > > > This makes the driver simpler to allow all components > > in the display pipeline to be treated as bridges by > > cleaning the way to generic connector handling. > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > > This breaks Exysos DSI driver operation (Trats board worked fine with > only patches 1-2): > > [ 2.540066] exynos4-fb 11c00000.fimd: Adding to iommu group 0 > [ 2.554733] OF: graph: no port node found in /soc/fimd@11c00000 > [ 2.602819] [drm] Exynos DRM: using 11c00000.fimd device for DMA > mapping operations > [ 2.609649] exynos-drm exynos-drm: bound 11c00000.fimd (ops > fimd_component_ops) > [ 2.632558] exynos-drm exynos-drm: failed to bind 11c80000.dsi (ops > exynos_dsi_component_ops): -22 > [ 2.642263] exynos-drm exynos-drm: master bind failed: -22 > [ 2.651017] exynos-drm: probe of exynos-drm failed with error -22 Thanks for testing it. Can you check Squash of 3,4 or 3,4,5 will work or not? Thanks, Jagan.
On 05.07.2021 14:00, Jagan Teki wrote: > On Mon, Jul 5, 2021 at 5:18 PM Marek Szyprowski > <m.szyprowski@samsung.com> wrote: >> On 04.07.2021 11:02, Jagan Teki wrote: >>> Use drm_panel_bridge to replace manual panel and >>> bridge_chain handling code. >>> >>> This makes the driver simpler to allow all components >>> in the display pipeline to be treated as bridges by >>> cleaning the way to generic connector handling. >>> >>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> >> This breaks Exysos DSI driver operation (Trats board worked fine with >> only patches 1-2): >> >> [ 2.540066] exynos4-fb 11c00000.fimd: Adding to iommu group 0 >> [ 2.554733] OF: graph: no port node found in /soc/fimd@11c00000 >> [ 2.602819] [drm] Exynos DRM: using 11c00000.fimd device for DMA >> mapping operations >> [ 2.609649] exynos-drm exynos-drm: bound 11c00000.fimd (ops >> fimd_component_ops) >> [ 2.632558] exynos-drm exynos-drm: failed to bind 11c80000.dsi (ops >> exynos_dsi_component_ops): -22 >> [ 2.642263] exynos-drm exynos-drm: master bind failed: -22 >> [ 2.651017] exynos-drm: probe of exynos-drm failed with error -22 > Thanks for testing it. > > Can you check Squash of 3,4 or 3,4,5 will work or not? I've check both sets: 1-4 and 1-5 and none of them works. The result is same as above. If I remember correctly, last time when I played with that code, there was a problem with DRM core calling bridge ops in different order than when they are used by the Exynos DSI driver. Best regards
On Mon, Jul 5, 2021 at 5:43 PM Marek Szyprowski <m.szyprowski@samsung.com> wrote: > > On 05.07.2021 14:00, Jagan Teki wrote: > > On Mon, Jul 5, 2021 at 5:18 PM Marek Szyprowski > > <m.szyprowski@samsung.com> wrote: > >> On 04.07.2021 11:02, Jagan Teki wrote: > >>> Use drm_panel_bridge to replace manual panel and > >>> bridge_chain handling code. > >>> > >>> This makes the driver simpler to allow all components > >>> in the display pipeline to be treated as bridges by > >>> cleaning the way to generic connector handling. > >>> > >>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > >> This breaks Exysos DSI driver operation (Trats board worked fine with > >> only patches 1-2): > >> > >> [ 2.540066] exynos4-fb 11c00000.fimd: Adding to iommu group 0 > >> [ 2.554733] OF: graph: no port node found in /soc/fimd@11c00000 > >> [ 2.602819] [drm] Exynos DRM: using 11c00000.fimd device for DMA > >> mapping operations > >> [ 2.609649] exynos-drm exynos-drm: bound 11c00000.fimd (ops > >> fimd_component_ops) > >> [ 2.632558] exynos-drm exynos-drm: failed to bind 11c80000.dsi (ops > >> exynos_dsi_component_ops): -22 > >> [ 2.642263] exynos-drm exynos-drm: master bind failed: -22 > >> [ 2.651017] exynos-drm: probe of exynos-drm failed with error -22 > > Thanks for testing it. > > > > Can you check Squash of 3,4 or 3,4,5 will work or not? > > I've check both sets: 1-4 and 1-5 and none of them works. The result is > same as above. If I remember correctly, last time when I played with > that code, there was a problem with DRM core calling bridge ops in > different order than when they are used by the Exynos DSI driver. Okay. Let me check with sun6i-mipi-dsi as it is component_ops based. Jagan.
Hi Sam, On Sun, Jul 25, 2021 at 10:35 PM Sam Ravnborg <sam@ravnborg.org> wrote: > > Hi Jagan, > > On Sun, Jul 04, 2021 at 02:32:13PM +0530, Jagan Teki wrote: > > This series supports common bridge support for Samsung MIPI DSIM > > which is used in Exynos and i.MX8MM SoC's. > > > > The final bridge supports both the Exynos and i.MX8MM DSI devices. > > > > Right now bridge offers two sets of implementations. > > > > A. With component_ops and exynos specific code exclusively for > > exynos dsi drivers and it's legacy bindings. > > > > B. Without componenet_ops for newly implemented bridges and its > > users like i.MX8MM. > > > > The future plan is to fix the implementation A) by dropping > > component_ops and fixing exynos specific code in order to make > > the bridge more mature to use and the same is mentioned in > > drivers TODO. > > > > Patch 0001 - 0006: Bridge conversion > > Patch 0007 - 0017: Samsung MIPI DSIM bridge fixes, additions > > > > Tested in Engicam i.Core MX8M Mini SoM. > > > > Anyone interest, please have a look on this repo > > https://github.com/openedev/linux/tree/070421-imx8mm-dsim > > > > Would appreciate anyone from the exynos team to test it on > > the exynos platform? > > > > Any inputs? > > I really like where you are headign with this! > No testing - sorry. But I will try to provide a bit of feedback on the > individual patches. > > I hope you find a way to move forward with this. Thanks for the response. We have found some issues with Bridge conversion on existing exynos drivers. The component based DSI drivers(like exynos) are difficult to attach if it involves kms hotplug. kms hotplug would require drm pointer and that pointer would only available after the bind call finishes. But the bridge attach in bind call will defer till it find the attached bridge. Right now I'm trying to find the proper way to attach the bridges for component based DSI drivers which involves kms hot-plug. If you have any ideas on this, please let me know. Thanks, Jagan.
This series supports common bridge support for Samsung MIPI DSIM which is used in Exynos and i.MX8MM SoC's. The final bridge supports both the Exynos and i.MX8MM DSI devices. Right now bridge offers two sets of implementations. A. With component_ops and exynos specific code exclusively for exynos dsi drivers and it's legacy bindings. B. Without componenet_ops for newly implemented bridges and its users like i.MX8MM. The future plan is to fix the implementation A) by dropping component_ops and fixing exynos specific code in order to make the bridge more mature to use and the same is mentioned in drivers TODO. Patch 0001 - 0006: Bridge conversion Patch 0007 - 0017: Samsung MIPI DSIM bridge fixes, additions Tested in Engicam i.Core MX8M Mini SoM. Anyone interest, please have a look on this repo https://github.com/openedev/linux/tree/070421-imx8mm-dsim Would appreciate anyone from the exynos team to test it on the exynos platform? Any inputs? Jagan. Jagan Teki (17): drm/exynos: dsi: Convert to bridge driver drm/exynos: dsi: Handle drm_device for bridge drm/exynos: dsi: Use the drm_panel_bridge API drm/exynos: dsi: Create bridge connector for encoder drm/exynos: dsi: Get the mode from bridge drm/exynos: dsi: Handle exynos specifics via driver_data drm: bridge: Move exynos_drm_dsi into bridges dt-bindings: display: bridge: Add Samsung MIPI DSIM bridge drm: bridge: samsung-dsim: Add module init, exit drm: bridge: samsung-dsim: Update the of_node for port(s) drm: bridge: samsung-dsim: Find the possible DSI devices dt-bindings: display: bridge: samsung,mipi-dsim: Add i.MX8MM support drm: bridge: samsung-dsim: Add i.MX8MM support drm: bridge: samsung-dsim: Add input_bus_flags drm: bridge: samsung-dsim: Move DSI init in bridge enable drm: bridge: samsung-dsim: Fix PLL_P offset drm: bridge: samsung-dsim: Add bridge mode_fixup .../display/bridge/samsung,mipi-dsim.yaml | 360 +++++++++ .../bindings/display/exynos/exynos_dsim.txt | 90 --- MAINTAINERS | 12 + drivers/gpu/drm/bridge/Kconfig | 15 + drivers/gpu/drm/bridge/Makefile | 1 + .../samsung-dsim.c} | 758 ++++++++++-------- drivers/gpu/drm/exynos/Kconfig | 9 - drivers/gpu/drm/exynos/Makefile | 1 - 8 files changed, 795 insertions(+), 451 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml delete mode 100644 Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt rename drivers/gpu/drm/{exynos/exynos_drm_dsi.c => bridge/samsung-dsim.c} (69%)