Re: [PATCH 9/9] drm/bridge: imx: Add i.MX93 MIPI DSI support

Jagan Teki
2023-07-19T06:30:03Z

Hi,

On Tue, Jul 18, 2023 at 3:19 PM Ying Liu <victor.liu@nxp.com> wrote:
>
> On Tuesday, July 18, 2023 5:35 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > >
> > > Hi Jagan,
> > >
> > > On Monday, July 17, 2023 2:44 PM Jagan Teki
> > <jagan@amarulasolutions.com> wrote:
> > > > On Mon, Jul 17, 2023 at 11:44 AM Liu Ying <victor.liu@nxp.com> wrote:
> > > > >
> > > > > Freescale i.MX93 SoC embeds a Synopsys Designware MIPI DSI host
> > > > > controller and a Synopsys Designware MIPI DPHY. Some configurations
> > > > > and extensions to them are controlled by i.MX93 media blk-ctrl.
> > > > >
> > > > > Add a DRM bridge for i.MX93 MIPI DSI by using existing DW MIPI DSI
> > > > > bridge helpers and implementing i.MX93 MIPI DSI specific extensions.
> > > >
> > > > I think the better way would add compatibility to be part of existing
> > > > dw-mipi-dsi.c with specific driver data. This way it avoids all the
> > > > platform-related helpers(extensions) and makes the driver generic to
> > > > all SoCs which use DW DSI IP. It would be a straightforward change as
> > > > the imx93 drm pipeline already supports bridge topology.
> > >
> > > The platform-related stuff is handed over to dw-mipi-dsi.c via struct
> > > dw_mipi_dsi_plat_data as an argument of dw_mipi_dsi_probe(). It looks
> > > ok for vendor drivers to call dw_mipi_dsi_probe() to set the platform-
> > related
> > > information(rockchip, meson and stm do that), like pdata.phy_ops and
> > > pdata.host_ops.
> >
> > I understand this topology of having soc-platform drivers with
> > dw-mipi-dsi bridge. What I'm suggesting is to not add a soc-platform
> > driver for imx93 instead add add support directly on dw-mipi-dsi
> > bridge.
>
> It seems that directly supporting i.MX93 MIPI DSI in dw-mipi-dsi.c is
> not feasible. The main reason is that struct dw_mipi_dsi_plat_data
> contains phy_ops and each vendor driver provides very different
> methods for phy, while...

Cannot this phy_ops goes to PHY core somewhere around and even it is
possible to add via driver data for imx93 by untouching existing
handling? I know it is not as direct as I describe but it is worth
maintaining the driver this way to keep control of the future
soc-drivers to include in dw-mipi-dsi instead of handling platform
code separately.

>
> >
> > >
> > > dw-mipi-dsi.c is generic w/wo this patch series.
> > >
> > > Can you elaborate more about adding compatibility to be part of existing
> > > dw-mipi-dsi.c with specific driver data? I don't see clear approach to do
> > > that.
> >
> > Please check the above comments, an example of samsung-dsim.c
>
> ... it seems that samsung-dsim.c uses struct samsung_dsim_driver_data to
> differential platform information and it doesn't contain any callback, which
> means comparing to DW MIPI DSI, Samsung DSIM shows more consistency
> across vendor SoCs from HW IP/SoC integration PoV.

Yes, but is it possible to adjust struct according to DW MIPI DSI.

Thanks
Jagan.