| Message ID | 20200911135413.3654800-1-m.tretter@pengutronix.de |
|---|---|
| Headers | show |
| Series | drm/exynos: Convert driver to drm bridge | expand |
W dniu 11.09.2020 o 15:54, Michael Tretter pisze: > Platform drivers need to be aware if a mipi dsi device attaches or > detaches. Add host_ops to the driver_data for attach and detach > callbacks and move the i80 mode selection and the hotplug handling into > the callback, because these depend on the drm driver. > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> > --- > v2: > - new patch > --- > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 64 ++++++++++++++++++++----- > 1 file changed, 53 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > index 1a15ae71205d..684a2fbef08a 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c > @@ -239,6 +239,12 @@ struct exynos_dsi_transfer { > #define DSIM_STATE_CMD_LPM BIT(2) > #define DSIM_STATE_VIDOUT_AVAILABLE BIT(3) > > +struct exynos_dsi; > +struct exynos_dsi_host_ops { > + int (*attach)(struct device *dev, struct mipi_dsi_device *device); > + int (*detach)(struct device *dev, struct mipi_dsi_device *device); > +}; > + > enum exynos_reg_offset { > EXYNOS_REG_OFS, > EXYNOS5433_REG_OFS > @@ -254,6 +260,7 @@ struct exynos_dsi_driver_data { > unsigned int wait_for_reset; > unsigned int num_bits_resol; > const unsigned int *reg_values; > + const struct exynos_dsi_host_ops *host_ops; > }; > > struct exynos_dsi { > @@ -467,6 +474,41 @@ static const unsigned int exynos5433_reg_values[] = { > [PHYTIMING_HS_TRAIL] = 0x0c, > }; > > +static int __exynos_dsi_host_attach(struct device *dev, > + struct mipi_dsi_device *device) > +{ > + struct exynos_dsi *dsi = dev_get_drvdata(dev); > + struct drm_device *drm = dsi->encoder.dev; As I wrote yesterday drm device was guaranteed to be present here only if mipi dsi host registration was performed in component bind. In case it is performed in probe it will be always NULL, and the code does not make sense. Regards Andrzej > + struct exynos_drm_crtc *crtc; > + > + mutex_lock(&drm->mode_config.mutex); > + crtc = exynos_drm_crtc_get_by_type(drm, EXYNOS_DISPLAY_TYPE_LCD); > + crtc->i80_mode = !(device->mode_flags & MIPI_DSI_MODE_VIDEO); > + mutex_unlock(&drm->mode_config.mutex); > + > + if (drm->mode_config.poll_enabled) > + drm_kms_helper_hotplug_event(drm); > + > + return 0; > +} > + > +static int __exynos_dsi_host_detach(struct device *dev, > + struct mipi_dsi_device *device) > +{ > + struct exynos_dsi *dsi = dev_get_drvdata(dev); > + struct drm_device *drm = dsi->encoder.dev; > + > + if (drm->mode_config.poll_enabled) > + drm_kms_helper_hotplug_event(drm); > + > + return 0; > +} > + > +static const struct exynos_dsi_host_ops exynos_dsi_host_ops = { > + .attach = __exynos_dsi_host_attach, > + .detach = __exynos_dsi_host_detach, > +}; > + > static const struct exynos_dsi_driver_data exynos3_dsi_driver_data = { > .reg_ofs = EXYNOS_REG_OFS, > .plltmr_reg = 0x50, > @@ -477,6 +519,7 @@ static const struct exynos_dsi_driver_data exynos3_dsi_driver_data = { > .wait_for_reset = 1, > .num_bits_resol = 11, > .reg_values = reg_values, > + .host_ops = &exynos_dsi_host_ops, > }; > > static const struct exynos_dsi_driver_data exynos4_dsi_driver_data = { > @@ -489,6 +532,7 @@ static const struct exynos_dsi_driver_data exynos4_dsi_driver_data = { > .wait_for_reset = 1, > .num_bits_resol = 11, > .reg_values = reg_values, > + .host_ops = &exynos_dsi_host_ops, > }; > > static const struct exynos_dsi_driver_data exynos5_dsi_driver_data = { > @@ -499,6 +543,7 @@ static const struct exynos_dsi_driver_data exynos5_dsi_driver_data = { > .wait_for_reset = 1, > .num_bits_resol = 11, > .reg_values = reg_values, > + .host_ops = &exynos_dsi_host_ops, > }; > > static const struct exynos_dsi_driver_data exynos5433_dsi_driver_data = { > @@ -510,6 +555,7 @@ static const struct exynos_dsi_driver_data exynos5433_dsi_driver_data = { > .wait_for_reset = 0, > .num_bits_resol = 12, > .reg_values = exynos5433_reg_values, > + .host_ops = &exynos_dsi_host_ops, > }; > > static const struct exynos_dsi_driver_data exynos5422_dsi_driver_data = { > @@ -521,6 +567,7 @@ static const struct exynos_dsi_driver_data exynos5422_dsi_driver_data = { > .wait_for_reset = 1, > .num_bits_resol = 12, > .reg_values = exynos5422_reg_values, > + .host_ops = &exynos_dsi_host_ops, > }; > > static const struct of_device_id exynos_dsi_of_match[] = { > @@ -1551,8 +1598,8 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host, > struct mipi_dsi_device *device) > { > struct exynos_dsi *dsi = host_to_dsi(host); > + const struct exynos_dsi_host_ops *ops = dsi->driver_data->host_ops; > struct drm_encoder *encoder = &dsi->encoder; > - struct drm_device *drm = encoder->dev; > struct drm_bridge *out_bridge; > > out_bridge = of_drm_find_bridge(device->dev.of_node); > @@ -1590,18 +1637,12 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host, > return ret; > } > > - mutex_lock(&drm->mode_config.mutex); > - > dsi->lanes = device->lanes; > dsi->format = device->format; > dsi->mode_flags = device->mode_flags; > - exynos_drm_crtc_get_by_type(drm, EXYNOS_DISPLAY_TYPE_LCD)->i80_mode = > - !(dsi->mode_flags & MIPI_DSI_MODE_VIDEO); > > - mutex_unlock(&drm->mode_config.mutex); > - > - if (drm->mode_config.poll_enabled) > - drm_kms_helper_hotplug_event(drm); > + if (ops && ops->attach) > + ops->attach(dsi->dsi_host.dev, device); > > return 0; > } > @@ -1610,6 +1651,7 @@ static int exynos_dsi_host_detach(struct mipi_dsi_host *host, > struct mipi_dsi_device *device) > { > struct exynos_dsi *dsi = host_to_dsi(host); > + const struct exynos_dsi_host_ops *ops = dsi->driver_data->host_ops; > struct drm_device *drm = dsi->encoder.dev; > > if (dsi->panel) { > @@ -1625,8 +1667,8 @@ static int exynos_dsi_host_detach(struct mipi_dsi_host *host, > INIT_LIST_HEAD(&dsi->bridge_chain); > } > > - if (drm->mode_config.poll_enabled) > - drm_kms_helper_hotplug_event(drm); > + if (ops && ops->detach) > + ops->detach(dsi->dsi_host.dev, device); > > exynos_dsi_unregister_te_irq(dsi); >
On Fri, Sep 11, 2020 at 03:53:59PM +0200, Michael Tretter wrote: > We do not need to keep a reference to the in_bridge_node, but we can > simply drop it, once we found and attached the previous bridge. > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> Reviewed-by: Sam Ravnborg <sam@ravnborg.org> Note: I expect exynos people to pick it up. Sam
Hi Michael. On Fri, Sep 11, 2020 at 03:54:03PM +0200, Michael Tretter wrote: > The phy timings are already shifted to the field position. If the driver > is reused on multiple platforms, this exposes the field positions to the > platform code. > > Store only the timing values in the platform data and shift the value to > the field when writing the fields to the registers. > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> This and the following patch smells like the regmap functionality is partly open coded. regmaps supports defining different register layouts and select the correct layout at runtime. See for example: https://www.collabora.com/news-and-blog/blog/2020/05/27/using-regmaps-to-make-linux-drivers-more-generic/ or https://www.youtube.com/watch?v=0RPDGANArFc Some parts is not a perfect fit - but using regmaps will make it better as a general and well-known solution is used. @Adrian - see https://lore.kernel.org/dri-devel/20200911135413.3654800-1-m.tretter@pengutronix.de/T/#m8e211c8cce915168cf2b8c4eef1c7ec9b8447af8 for the original patch. Sam
20. 11. 8. 오전 7:27에 Sam Ravnborg 이(가) 쓴 글: > On Fri, Sep 11, 2020 at 03:54:01PM +0200, Michael Tretter wrote: >> As the driver shall be usable with drivers that use the component >> framework and drivers that don't, split the common probing code into a >> separate function that can be called from different locations. >> >> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> > Reviewed-by: Sam Ravnborg <sam@ravnborg.org> This patch and other are related to what this patch series do. However, this patch makes Exynos board not working yet. So excepting patches 2 to 3, I will wait for that this patch series will be fixed and reviewed more. Thanks, Inki Dae >
Hi Michael, Thanks for your contribution. 20. 9. 11. 오후 10:53에 Michael Tretter 이(가) 쓴 글: > This is v2 of the series to convert the Exynos MIPI DSI driver into a drm > bridge and make it usable with other drivers. Although the driver is > converted, it still supports the component framework API to stay compliant > with the Exynos DRM driver. > > The Exynos MIPI DSI Phy is also found on the i.MX8M Mini. However, on the > i.MX8M Mini, the bridge is driven by an LCDIF display controller instead of > the Exynos Decon. The driver for the LCDIF does not use the component > framework, but uses drm bridges. > > I don't have any Exynos SoC to actually test the series. I build a dummy to > test the bridge with a component driver, to make sure that at least the > initialization is working. Furthermore, tested the driver as a bridge with a > few additional unfinished patches on the i.MX8M Mini EVK. However, somebody > should verify that the driver is still working on Exynos hardware. > > I also changed the order of the patches to first make the driver more platform > independent (patches 2 to 8), then convert to a drm bridge driver (patches 10 Just a fundamental question, A MIPI-DSI(Display Serial Interface) bus device would be one of an encoder type of devices not bridge such as DSI to LVDS and LVDS to DSI bridge devices, and also image enhancer and image compressor in case of Exynos. Why do you want to convert such MIPI-DSI driver to bridge type of driver? Seems not sensible. The reason would be just to share MIPI-DSI phy driver for Exynos with i.MX8M Mini? Thanks, Inki Dae > to 13) and finally expose the API, split the code and move the platform > independent driver to the bridges (patches 14 - 16). Hopefully this simplifies > testing/bisecting and helps me to understand potential error reports. > > Also I added host_ops for attach/detach and the tearing effect handler to make > the calls into the platform code more visible. > > Furthermore, the series should now apply to linux-next and correctly build the > exynos_defconfig. > > Thanks, > > Michael > > Changelog: > > v2: > - rebase on linux-next > - verify with exynos_defconfig > - fix crashes reported by Marek Szyprowski Exynos3250-based Rinato > - reorder patches > - add host_ops for platform specific code > - roughly test component framework integration with dummy > > Michael Tretter (16): > drm/encoder: remove obsolete documentation of bridge > drm/exynos: remove in_bridge_node from exynos_dsi > drm/exynos: use exynos_dsi as drvdata > drm/exynos: extract helper functions for probe > drm/exynos: move dsi host registration to probe > drm/exynos: shift register values to fields on write > drm/exynos: use identifier instead of register offsets > drm/exynos: add host_ops callback for platform drivers > drm/exynos: add callback for tearing effect handler > drm/exynos: implement a drm bridge > drm/exynos: convert encoder functions to bridge function > drm/exynos: configure mode on drm bridge > drm/exynos: get encoder from bridge whenever possible > drm/exynos: add API functions for platform drivers > drm/exynos: split out platform specific code > drm/exynos: move bridge driver to bridges > > drivers/gpu/drm/bridge/Kconfig | 9 + > drivers/gpu/drm/bridge/Makefile | 1 + > drivers/gpu/drm/bridge/samsung-dsim.c | 1790 +++++++++++++++++++++ > drivers/gpu/drm/exynos/Kconfig | 5 +- > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 1927 ++--------------------- > include/drm/bridge/samsung-dsim.h | 64 + > include/drm/drm_encoder.h | 1 - > 7 files changed, 2027 insertions(+), 1770 deletions(-) > create mode 100644 drivers/gpu/drm/bridge/samsung-dsim.c > create mode 100644 include/drm/bridge/samsung-dsim.h >
On Mon, 09 Nov 2020 12:15:39 +0900, Inki Dae wrote: > 20. 9. 11. 오후 10:53에 Michael Tretter 이(가) 쓴 글: > > This is v2 of the series to convert the Exynos MIPI DSI driver into a drm > > bridge and make it usable with other drivers. Although the driver is > > converted, it still supports the component framework API to stay compliant > > with the Exynos DRM driver. > > > > The Exynos MIPI DSI Phy is also found on the i.MX8M Mini. However, on the > > i.MX8M Mini, the bridge is driven by an LCDIF display controller instead of > > the Exynos Decon. The driver for the LCDIF does not use the component > > framework, but uses drm bridges. > > > > I don't have any Exynos SoC to actually test the series. I build a dummy to > > test the bridge with a component driver, to make sure that at least the > > initialization is working. Furthermore, tested the driver as a bridge with a > > few additional unfinished patches on the i.MX8M Mini EVK. However, somebody > > should verify that the driver is still working on Exynos hardware. > > > > I also changed the order of the patches to first make the driver more platform > > independent (patches 2 to 8), then convert to a drm bridge driver (patches 10 > > Just a fundamental question, A MIPI-DSI(Display Serial Interface) bus device > would be one of an encoder type of devices not bridge such as DSI to LVDS > and LVDS to DSI bridge devices, and also image enhancer and image compressor > in case of Exynos. I don't understand, why the MIPI-DSI bus device would be an encoder type and DSI to LVDS or MIPI-DSI to HDMI would be bridges. For example, the device tree documentation for the DSIM states that the DSIM receives RGB video as input and produces MIPI-DSI as output. Thus, the DSIM is basically a parallel RGB to MIPI-DSI bridge and the encoder is the LCD controller that encodes the video data as parallel RGB. On the i.MX8MM, the LCDIF is already the encoder. On Exynos, the series implements the encoder in the platform glue, but in the end the encoder can probably be moved to the DECON. > Why do you want to convert such MIPI-DSI driver to bridge type of driver? > Seems not sensible. The reason would be just to share MIPI-DSI phy driver > for Exynos with i.MX8M Mini? Yes, the reason is that the driver should be shared between Exynos and i.MX8MM. It is the same IP and I don't see a reason why we should introduce another driver for the same IP if the driver works for both SoCs. Michael
On Sat, 07 Nov 2020 23:39:30 +0100, Sam Ravnborg wrote: > On Fri, Sep 11, 2020 at 03:54:03PM +0200, Michael Tretter wrote: > > The phy timings are already shifted to the field position. If the driver > > is reused on multiple platforms, this exposes the field positions to the > > platform code. > > > > Store only the timing values in the platform data and shift the value to > > the field when writing the fields to the registers. > > > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> > > This and the following patch smells like the regmap functionality is > partly open coded. regmaps supports defining different register layouts > and select the correct layout at runtime. > > See for example: > https://www.collabora.com/news-and-blog/blog/2020/05/27/using-regmaps-to-make-linux-drivers-more-generic/ > or > https://www.youtube.com/watch?v=0RPDGANArFc > > Some parts is not a perfect fit - but using regmaps will make it better > as a general and well-known solution is used. I considered using regmaps, but there was something that didn't work out. Unfortunately, I don't remember, what it actually was. Therefore, it is probably best to use regmaps here. Michael > > @Adrian - see https://lore.kernel.org/dri-devel/20200911135413.3654800-1-m.tretter@pengutronix.de/T/#m8e211c8cce915168cf2b8c4eef1c7ec9b8447af8 > for the original patch. > > Sam >
Hi Michael, On 10.11.2020 09:13, Michael Tretter wrote: > On Mon, 09 Nov 2020 12:15:39 +0900, Inki Dae wrote: >> 20. 9. 11. 오후 10:53에 Michael Tretter 이(가) 쓴 글: >>> This is v2 of the series to convert the Exynos MIPI DSI driver into a drm >>> bridge and make it usable with other drivers. Although the driver is >>> converted, it still supports the component framework API to stay compliant >>> with the Exynos DRM driver. >>> >>> The Exynos MIPI DSI Phy is also found on the i.MX8M Mini. However, on the >>> i.MX8M Mini, the bridge is driven by an LCDIF display controller instead of >>> the Exynos Decon. The driver for the LCDIF does not use the component >>> framework, but uses drm bridges. >>> >>> I don't have any Exynos SoC to actually test the series. I build a dummy to >>> test the bridge with a component driver, to make sure that at least the >>> initialization is working. Furthermore, tested the driver as a bridge with a >>> few additional unfinished patches on the i.MX8M Mini EVK. However, somebody >>> should verify that the driver is still working on Exynos hardware. >>> >>> I also changed the order of the patches to first make the driver more platform >>> independent (patches 2 to 8), then convert to a drm bridge driver (patches 10 >> Just a fundamental question, A MIPI-DSI(Display Serial Interface) bus device >> would be one of an encoder type of devices not bridge such as DSI to LVDS >> and LVDS to DSI bridge devices, and also image enhancer and image compressor >> in case of Exynos. > I don't understand, why the MIPI-DSI bus device would be an encoder type and > DSI to LVDS or MIPI-DSI to HDMI would be bridges. For example, the device tree > documentation for the DSIM states that the DSIM receives RGB video as input > and produces MIPI-DSI as output. Thus, the DSIM is basically a parallel RGB to > MIPI-DSI bridge and the encoder is the LCD controller that encodes the video > data as parallel RGB. > > On the i.MX8MM, the LCDIF is already the encoder. On Exynos, the series > implements the encoder in the platform glue, but in the end the encoder can > probably be moved to the DECON. This is probably the historical decision. That time when Exynos DSI driver has been implemented, support for DRM bridges wasn't ready enough to use to for such purpose. Frankly, I'm still not convinced that the current DRM bridge framework provides everything needed to reimplement the Exynos DSI driver with all its features. There are a lots of corner cases and order-specific bits in turning on/off the display pipeline, which don't map nicely to the bridge pre_enable (called in post-order) and enable (called in pre-order) callbacks. Especially if you consider that there might be another bridge before and after. I think that Andrzej Hajda already pointed those drawbacks of the current design. Last week I've spent some significant amount of time playing with exynos dsi code to check how to match its operations (especially the runtime power management) to this design with the current boards (Arndale with additional DSI->LVDS bridge and panel, Trats2 with DSI panel and TM2e with MIC 'in-bridge' and DSI panel), but without a success. >> Why do you want to convert such MIPI-DSI driver to bridge type of driver? >> Seems not sensible. The reason would be just to share MIPI-DSI phy driver >> for Exynos with i.MX8M Mini? > Yes, the reason is that the driver should be shared between Exynos and > i.MX8MM. It is the same IP and I don't see a reason why we should introduce > another driver for the same IP if the driver works for both SoCs. I think that the easiest way to share this driver between Exynos and iMX would be to extract the low-level code (the code that plays with hardware registers) to the common plane, while keeping the separate DRM glue, device component and platform device parts. This way the Exynos will continue to use it in the current form, while iMX can start using it as DRM bridge. A bit similar approach is used with exynos_dp driver and analogix_dp bridge shared with other SoCs. I hope that later, when the bridge related issue are resolved, the Exynos can be converted too, so the encoder creation moved to FIMD and Decon. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
Hi Marek, On Tue, Nov 10, 2020 at 01:34:26PM +0100, Marek Szyprowski wrote: > Hi Michael, > > On 10.11.2020 09:13, Michael Tretter wrote: > > On Mon, 09 Nov 2020 12:15:39 +0900, Inki Dae wrote: > >> 20. 9. 11. 오후 10:53에 Michael Tretter 이(가) 쓴 글: > >>> This is v2 of the series to convert the Exynos MIPI DSI driver into a drm > >>> bridge and make it usable with other drivers. Although the driver is > >>> converted, it still supports the component framework API to stay compliant > >>> with the Exynos DRM driver. > >>> > >>> The Exynos MIPI DSI Phy is also found on the i.MX8M Mini. However, on the > >>> i.MX8M Mini, the bridge is driven by an LCDIF display controller instead of > >>> the Exynos Decon. The driver for the LCDIF does not use the component > >>> framework, but uses drm bridges. > >>> > >>> I don't have any Exynos SoC to actually test the series. I build a dummy to > >>> test the bridge with a component driver, to make sure that at least the > >>> initialization is working. Furthermore, tested the driver as a bridge with a > >>> few additional unfinished patches on the i.MX8M Mini EVK. However, somebody > >>> should verify that the driver is still working on Exynos hardware. > >>> > >>> I also changed the order of the patches to first make the driver more platform > >>> independent (patches 2 to 8), then convert to a drm bridge driver (patches 10 > >> Just a fundamental question, A MIPI-DSI(Display Serial Interface) bus device > >> would be one of an encoder type of devices not bridge such as DSI to LVDS > >> and LVDS to DSI bridge devices, and also image enhancer and image compressor > >> in case of Exynos. > > I don't understand, why the MIPI-DSI bus device would be an encoder type and > > DSI to LVDS or MIPI-DSI to HDMI would be bridges. For example, the device tree > > documentation for the DSIM states that the DSIM receives RGB video as input > > and produces MIPI-DSI as output. Thus, the DSIM is basically a parallel RGB to > > MIPI-DSI bridge and the encoder is the LCD controller that encodes the video > > data as parallel RGB. > > > > On the i.MX8MM, the LCDIF is already the encoder. On Exynos, the series > > implements the encoder in the platform glue, but in the end the encoder can > > probably be moved to the DECON. > > This is probably the historical decision. That time when Exynos DSI > driver has been implemented, support for DRM bridges wasn't ready enough > to use to for such purpose. > > Frankly, I'm still not convinced that the current DRM bridge framework > provides everything needed to reimplement the Exynos DSI driver with all > its features. There are a lots of corner cases and order-specific bits > in turning on/off the display pipeline, which don't map nicely to the > bridge pre_enable (called in post-order) and enable (called in > pre-order) callbacks. Especially if you consider that there might be > another bridge before and after. > > I think that Andrzej Hajda already pointed those drawbacks of the > current design. Last week I've spent some significant amount of time > playing with exynos dsi code to check how to match its operations > (especially the runtime power management) to this design with the > current boards (Arndale with additional DSI->LVDS bridge and panel, > Trats2 with DSI panel and TM2e with MIC 'in-bridge' and DSI panel), but > without a success. Can you help by iterating the missing pieces in the current bridge infrastructure? Maybe it is something we can work out in a way that benefits more than one bridge driver. It would be nice with specific issues to look into. Thanks in advance, Sam
20. 11. 10. 오후 5:13에 Michael Tretter 이(가) 쓴 글: > On Mon, 09 Nov 2020 12:15:39 +0900, Inki Dae wrote: >> 20. 9. 11. 오후 10:53에 Michael Tretter 이(가) 쓴 글: >>> This is v2 of the series to convert the Exynos MIPI DSI driver into a drm >>> bridge and make it usable with other drivers. Although the driver is >>> converted, it still supports the component framework API to stay compliant >>> with the Exynos DRM driver. >>> >>> The Exynos MIPI DSI Phy is also found on the i.MX8M Mini. However, on the >>> i.MX8M Mini, the bridge is driven by an LCDIF display controller instead of >>> the Exynos Decon. The driver for the LCDIF does not use the component >>> framework, but uses drm bridges. >>> >>> I don't have any Exynos SoC to actually test the series. I build a dummy to >>> test the bridge with a component driver, to make sure that at least the >>> initialization is working. Furthermore, tested the driver as a bridge with a >>> few additional unfinished patches on the i.MX8M Mini EVK. However, somebody >>> should verify that the driver is still working on Exynos hardware. >>> >>> I also changed the order of the patches to first make the driver more platform >>> independent (patches 2 to 8), then convert to a drm bridge driver (patches 10 >> >> Just a fundamental question, A MIPI-DSI(Display Serial Interface) bus device >> would be one of an encoder type of devices not bridge such as DSI to LVDS >> and LVDS to DSI bridge devices, and also image enhancer and image compressor >> in case of Exynos. > > I don't understand, why the MIPI-DSI bus device would be an encoder type and > DSI to LVDS or MIPI-DSI to HDMI would be bridges. For example, the device tree > documentation for the DSIM states that the DSIM receives RGB video as input > and produces MIPI-DSI as output. Thus, the DSIM is basically a parallel RGB to MIPI-DSI receives RGB video as input and encodes it to MIPI packet and then transfers the packet to MIPI panel. And finally, MIPI panel decodes the packet and updates it - RGB data - on its SRAM. MIPI-DSI driver programs how the RGB video should be made as MIPI packet. For more detail, you could refer to MIPI-DSI spec. This would be why we handle MIPI-DSI driver as an encoder like other ARM SoC DRM drivers did. > MIPI-DSI bridge and the encoder is the LCD controller that encodes the video > data as parallel RGB. > > On the i.MX8MM, the LCDIF is already the encoder. On Exynos, the series > implements the encoder in the platform glue, but in the end the encoder can > probably be moved to the DECON. As you know, Display controller can transfer RGB video to various devices such as RGB panel, CPU panel, LVDS panel via LVDS bridge, MIPI panel via MIPI-DSI bus device, and so on like below, Display Controller --> RGB panel or CPU panel. Display Controller --> LVDS bridge --> LVDS panel. Display Controller --> MIPI DSI bus device --> MIPI Panel. ... Display controller drivers such as FIMD and DECON series in case of Exynos don't create an encoder driver-internally instead of it depends on Display pipeline configuration - what kind of Display panel is used. In fact, if the Display pipeline uses RGB panel or CPU panel without Display bus device such as MIPI-DSI, then FIMD and DECON drivers create an encoder for it internally - just we separated the code to consider other type of panels. On the I.MX8MM, could you share how to handle an encoder if some board has only MIPI-DSI panel, and in this case, where is an encoder for it created? I looked into I.MX8MM DRM driver but didn't find MIPI-DSI driver. Seems that they support only parallel display, HDMI and LVDS panel. Thanks, Inki Dae > >> Why do you want to convert such MIPI-DSI driver to bridge type of driver? >> Seems not sensible. The reason would be just to share MIPI-DSI phy driver >> for Exynos with i.MX8M Mini? > > Yes, the reason is that the driver should be shared between Exynos and > i.MX8MM. It is the same IP and I don't see a reason why we should introduce > another driver for the same IP if the driver works for both SoCs. > > Michael >