mbox series

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

Message ID 20230717061831.1826878-1-victor.liu@nxp.com (mailing list archive)
Headers show
Series drm/bridge: imx: Add i.MX93 MIPI DSI support | expand

Message

Liu Ying July 17, 2023, 6:18 a.m. UTC
Hi,

This series aims to add MIPI DSI support for Freescale i.MX93 SoC.

There is a Synopsys DesignWare MIPI DSI host controller and a Synopsys
Designware MIPI DPHY embedded in i.MX93.  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.

Note that since this series touches the dw-mipi-dsi driver, tests are
needed to be done for meson, rockchip and stm.

Patch 1 ~ 7 do preparation work for adding i.MX93 MIPI DSI DRM bridge driver.

Patch 8 adds DT-binding documentation for i.MX93 MIPI DSI.

Patch 9 adds i.MX93 MIPI DSI DRM bridge.

Liu Ying (9):
  drm/bridge: synopsys: dw-mipi-dsi: Add dw_mipi_dsi_get_bridge() helper
  drm/bridge: synopsys: dw-mipi-dsi: Add input bus format negotiation
    support
  drm/bridge: synopsys: dw-mipi-dsi: Force input bus flags
  drm/bridge: synopsys: dw-mipi-dsi: Add mode fixup support
  drm/bridge: synopsys: dw-mipi-dsi: Use pixel clock rate to calculate
    lbcc
  drm/bridge: synopsys: dw-mipi-dsi: Set minimum lane byte clock cycles
    for HSA and HBP
  drm/bridge: synopsys: dw-mipi-dsi: Disable HSTX and LPRX timeout check
  dt-bindings: display: bridge: Document Freescale i.MX93 MIPI DSI
  drm/bridge: imx: Add i.MX93 MIPI DSI support

 .../display/bridge/fsl,imx93-mipi-dsi.yaml    | 115 +++
 drivers/gpu/drm/bridge/imx/Kconfig            |  10 +
 drivers/gpu/drm/bridge/imx/Makefile           |   1 +
 drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c   | 934 ++++++++++++++++++
 drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c |  91 +-
 include/drm/bridge/dw_mipi_dsi.h              |  16 +
 6 files changed, 1163 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/fsl,imx93-mipi-dsi.yaml
 create mode 100644 drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c

Comments

Alexander Stein July 18, 2023, 7:39 a.m. UTC | #1
Hi,

Am Montag, 17. Juli 2023, 08:18:22 CEST schrieb Liu Ying:
> Hi,
> 
> This series aims to add MIPI DSI support for Freescale i.MX93 SoC.
> 
> There is a Synopsys DesignWare MIPI DSI host controller and a Synopsys
> Designware MIPI DPHY embedded in i.MX93.  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.
> 
> Note that since this series touches the dw-mipi-dsi driver, tests are
> needed to be done for meson, rockchip and stm.
> 
> Patch 1 ~ 7 do preparation work for adding i.MX93 MIPI DSI DRM bridge
> driver.
> 
> Patch 8 adds DT-binding documentation for i.MX93 MIPI DSI.
> 
> Patch 9 adds i.MX93 MIPI DSI DRM bridge.
> 
> Liu Ying (9):
>   drm/bridge: synopsys: dw-mipi-dsi: Add dw_mipi_dsi_get_bridge() helper
>   drm/bridge: synopsys: dw-mipi-dsi: Add input bus format negotiation
>     support
>   drm/bridge: synopsys: dw-mipi-dsi: Force input bus flags
>   drm/bridge: synopsys: dw-mipi-dsi: Add mode fixup support
>   drm/bridge: synopsys: dw-mipi-dsi: Use pixel clock rate to calculate
>     lbcc
>   drm/bridge: synopsys: dw-mipi-dsi: Set minimum lane byte clock cycles
>     for HSA and HBP
>   drm/bridge: synopsys: dw-mipi-dsi: Disable HSTX and LPRX timeout check
>   dt-bindings: display: bridge: Document Freescale i.MX93 MIPI DSI
>   drm/bridge: imx: Add i.MX93 MIPI DSI support
> 
>  .../display/bridge/fsl,imx93-mipi-dsi.yaml    | 115 +++
>  drivers/gpu/drm/bridge/imx/Kconfig            |  10 +
>  drivers/gpu/drm/bridge/imx/Makefile           |   1 +
>  drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c   | 934 ++++++++++++++++++
>  drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c |  91 +-
>  include/drm/bridge/dw_mipi_dsi.h              |  16 +
>  6 files changed, 1163 insertions(+), 4 deletions(-)
>  create mode 100644
> Documentation/devicetree/bindings/display/bridge/fsl,imx93-mipi-dsi.yaml
> create mode 100644 drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c

Thanks for posting this patch series. I was trying to use this driver on our 
TQMa93xxLA platform where the DSI signals are connected to a TC9595 (driver 
tc358767) DSI-to-DP bridge.
Unfortunately this bridge requires the DSI signals to be in LP-11 upon reset 
and while in idle, otherwise not even DP AUX channel is functional.
Apparently DSI is currently not in LP-11. But reading the RM I have no idea 
how to configure the DSI host to achieve that. Do you have additional 
information which might help me here?
Also could you provide your DT configuration?

Thanks and best regards,
Alexander
Liu Ying July 18, 2023, 8:52 a.m. UTC | #2
On Tuesday, July 18, 2023 3:40 PM Alexander Stein <alexander.stein@ew.tq-group.com> wrote:
>
> Hi,

Hi,

>
> Am Montag, 17. Juli 2023, 08:18:22 CEST schrieb Liu Ying:
> > Hi,
> >
> > This series aims to add MIPI DSI support for Freescale i.MX93 SoC.
> >
> > There is a Synopsys DesignWare MIPI DSI host controller and a Synopsys
> > Designware MIPI DPHY embedded in i.MX93.  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.
> >
> > Note that since this series touches the dw-mipi-dsi driver, tests are
> > needed to be done for meson, rockchip and stm.
> >
> > Patch 1 ~ 7 do preparation work for adding i.MX93 MIPI DSI DRM bridge
> > driver.
> >
> > Patch 8 adds DT-binding documentation for i.MX93 MIPI DSI.
> >
> > Patch 9 adds i.MX93 MIPI DSI DRM bridge.
> >
> > Liu Ying (9):
> >   drm/bridge: synopsys: dw-mipi-dsi: Add dw_mipi_dsi_get_bridge() helper
> >   drm/bridge: synopsys: dw-mipi-dsi: Add input bus format negotiation
> >     support
> >   drm/bridge: synopsys: dw-mipi-dsi: Force input bus flags
> >   drm/bridge: synopsys: dw-mipi-dsi: Add mode fixup support
> >   drm/bridge: synopsys: dw-mipi-dsi: Use pixel clock rate to calculate
> >     lbcc
> >   drm/bridge: synopsys: dw-mipi-dsi: Set minimum lane byte clock cycles
> >     for HSA and HBP
> >   drm/bridge: synopsys: dw-mipi-dsi: Disable HSTX and LPRX timeout check
> >   dt-bindings: display: bridge: Document Freescale i.MX93 MIPI DSI
> >   drm/bridge: imx: Add i.MX93 MIPI DSI support
> >
> >  .../display/bridge/fsl,imx93-mipi-dsi.yaml    | 115 +++
> >  drivers/gpu/drm/bridge/imx/Kconfig            |  10 +
> >  drivers/gpu/drm/bridge/imx/Makefile           |   1 +
> >  drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c   | 934
> ++++++++++++++++++
> >  drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c |  91 +-
> >  include/drm/bridge/dw_mipi_dsi.h              |  16 +
> >  6 files changed, 1163 insertions(+), 4 deletions(-)
> >  create mode 100644
> > Documentation/devicetree/bindings/display/bridge/fsl,imx93-mipi-
> dsi.yaml
> > create mode 100644 drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c
>
> Thanks for posting this patch series. I was trying to use this driver on our
> TQMa93xxLA platform where the DSI signals are connected to a TC9595
> (driver
> tc358767) DSI-to-DP bridge.

Thanks for trying to test/use this driver.

I don't have TC9595 bridge to test, unfortunately.

> Unfortunately this bridge requires the DSI signals to be in LP-11 upon reset
> and while in idle, otherwise not even DP AUX channel is functional.
> Apparently DSI is currently not in LP-11. But reading the RM I have no idea
> how to configure the DSI host to achieve that. Do you have additional
> information which might help me here?

Hmm, probably no.
But, I tested ADV7535 DSI to HDMI bridge or RM67191 DSI panel on
i.MX93 11x11 EVK with this series, which works.

> Also could you provide your DT configuration?

For media blk-ctrl, dsi and lcdif, see:
https://pastebin.mozilla.org/aP8tFrPM

For adv7535 display pipeline, see:
https://pastebin.mozilla.org/89zwvr9Y

Note assigned-clock-rates is needed in lcdif DT node to suggest video pll rate.

Regards,
Liu Ying

>
> Thanks and best regards,
> Alexander
> --
> TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht München, HRB 105018
> Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> http://www.t/
> q-
> group.com%2F&data=05%7C01%7Cvictor.liu%40nxp.com%7Cbf329d1b4d704
> 801a94f08db876225f6%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%
> 7C638252627845769696%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
> wMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7
> C%7C&sdata=Q4wBL6Ji0wcMVIJpuR1gNAqBtFgUiJAwA5QvesFoGLc%3D&rese
> rved=0
>
Alexander Stein July 18, 2023, 9:31 a.m. UTC | #3
Hi,

Am Dienstag, 18. Juli 2023, 11:00:25 CEST schrieb Ying Liu:
> On Tuesday, July 18, 2023 3:49 PM Alexander Stein <alexander.stein@ew.tq-
group.com> wrote:
> > Hi,
> 
> Hi,
> 
> > thanks for the patch.
> 
> Thanks for your review.
> 
> > Am Montag, 17. Juli 2023, 08:18:31 CEST schrieb Liu Ying:
> > > 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.
> > > 
> > > Signed-off-by: Liu Ying <victor.liu@nxp.com>
> > > ---
> > > 
> > >  drivers/gpu/drm/bridge/imx/Kconfig          |  10 +
> > >  drivers/gpu/drm/bridge/imx/Makefile         |   1 +
> > >  drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c | 934
> > 
> > ++++++++++++++++++++
> > 
> > >  3 files changed, 945 insertions(+)
> > >  create mode 100644 drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c
> > > 
> > > diff --git a/drivers/gpu/drm/bridge/imx/Kconfig
> > > b/drivers/gpu/drm/bridge/imx/Kconfig index
> > 
> > 9fae28db6aa7..5182298c7182
> > 
> > > 100644
> > > --- a/drivers/gpu/drm/bridge/imx/Kconfig
> > > +++ b/drivers/gpu/drm/bridge/imx/Kconfig
> > > @@ -49,4 +49,14 @@ config DRM_IMX8QXP_PIXEL_LINK_TO_DPI
> > > 
> > >       Choose this to enable pixel link to display pixel
> > 
> > interface(PXL2DPI)
> > 
> > >       found in Freescale i.MX8qxp processor.
> > > 
> > > +config DRM_IMX93_MIPI_DSI
> > > +   tristate "Freescale i.MX93 specific extensions for Synopsys DW MIPI
> > 
> > DSI"
> > 
> > > +   depends on OF
> > > +   depends on COMMON_CLK
> > > +   select DRM_DW_MIPI_DSI
> > > +   select GENERIC_PHY_MIPI_DPHY
> > > +   help
> > > +     Choose this to enable MIPI DSI controller found in Freescale
> > 
> > i.MX93
> > 
> > > +     processor.
> > > +
> > > 
> > >  endif # ARCH_MXC || COMPILE_TEST
> > > 
> > > diff --git a/drivers/gpu/drm/bridge/imx/Makefile
> > > b/drivers/gpu/drm/bridge/imx/Makefile index
> > 
> > 8e2ebf3399a1..2b0c2e44aa1b
> > 
> > > 100644
> > > --- a/drivers/gpu/drm/bridge/imx/Makefile
> > > +++ b/drivers/gpu/drm/bridge/imx/Makefile
> > > @@ -4,3 +4,4 @@ obj-$(CONFIG_DRM_IMX8QXP_LDB) += imx8qxp-ldb.o
> > > 
> > >  obj-$(CONFIG_DRM_IMX8QXP_PIXEL_COMBINER) += imx8qxp-pixel-
> > 
> > combiner.o
> > 
> > >  obj-$(CONFIG_DRM_IMX8QXP_PIXEL_LINK) += imx8qxp-pixel-link.o
> > >  obj-$(CONFIG_DRM_IMX8QXP_PIXEL_LINK_TO_DPI) += imx8qxp-pxl2dpi.o
> > > 
> > > +obj-$(CONFIG_DRM_IMX93_MIPI_DSI) += imx93-mipi-dsi.o
> > > diff --git a/drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c
> > > b/drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c new file mode 100644
> > > index 000000000000..77f59e3407a0
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c
> > > 
> > > [snip]
> > > 
> > > +static int imx93_dsi_probe(struct platform_device *pdev)
> > > +{
> > > +   struct device *dev = &pdev->dev;
> > > +   struct device_node *np = dev->of_node;
> > > +   struct imx93_dsi *dsi;
> > > +   int ret;
> > > +
> > > +   dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
> > > +   if (!dsi)
> > > +           return -ENOMEM;
> > > +
> > > +   dsi->regmap = syscon_regmap_lookup_by_phandle(np, "fsl,media-
> > 
> > blk-
> > ctrl");
> > 
> > > +   if (IS_ERR(dsi->regmap)) {
> > > +           ret = PTR_ERR(dsi->regmap);
> > 
> > > +           DRM_DEV_ERROR(dev, "failed to get block ctrl regmap:
> > %d\n", ret);
> > 
> > Could you use dev_err_probe here instead?
> 
> Maybe, it's better to keep using DRM_DEV_ERROR to achieve consistent
> error log format across the driver which is implemented in drm_dev_printk().
> I see other DRM drivers do the same.

I see your point. On the other hand the benefit of dev_err_probe() is that the 
message of deferred probe can be seen in /sys/kernel/debug/devices_deferred.
Your check against EPROBE_DEFER will hide the message if something is not 
correct.

Maybe a to be introduced DRM_DEV_ERROR_PROBE might be useful.

Best regards,
Alexander

> > > +           return ret;
> > > +   }
> > > +
> > > +   dsi->clk_pixel = devm_clk_get(dev, "pix");
> > > +   if (IS_ERR(dsi->clk_pixel)) {
> > > +           ret = PTR_ERR(dsi->clk_pixel);
> > > +           if (ret != -EPROBE_DEFER)
> > 
> > > +                   DRM_DEV_ERROR(dev, "failed to get pixel clock:
> > %d\n", ret);
> > 
> > Could you use dev_err_probe here instead?
> 
> Ditto.
> 
> > > +           return ret;
> > > +   }
> > > +
> > > +   dsi->clk_cfg = devm_clk_get(dev, "phy_cfg");
> > > +   if (IS_ERR(dsi->clk_cfg)) {
> > > +           ret = PTR_ERR(dsi->clk_cfg);
> > > +           if (ret != -EPROBE_DEFER)
> > 
> > > +                   DRM_DEV_ERROR(dev, "failed to get phy cfg clock:
> > %d\n", ret);
> > 
> > > +           return ret;
> > > +   }
> > > +
> > > +   dsi->clk_ref = devm_clk_get(dev, "phy_ref");
> > > +   if (IS_ERR(dsi->clk_ref)) {
> > > +           ret = PTR_ERR(dsi->clk_ref);
> > > +           if (ret != -EPROBE_DEFER)
> > 
> > > +                   DRM_DEV_ERROR(dev, "failed to get phy ref clock:
> > %d\n", ret);
> > 
> > Could you use dev_err_probe here instead?
> 
> Ditto.
> 
> > > +           return ret;
> > > +   }
> > > +
> > > +   dsi->ref_clk_rate = clk_get_rate(dsi->clk_ref);
> > > +   if (dsi->ref_clk_rate < REF_CLK_RATE_MIN ||
> > > +       dsi->ref_clk_rate > REF_CLK_RATE_MAX) {
> > > +           DRM_DEV_ERROR(dev, "invalid phy ref clock rate %lu\n",
> > > +                         dsi->ref_clk_rate);
> > > +           return -EINVAL;
> > > +   }
> > > +   DRM_DEV_DEBUG_DRIVER(dev, "phy ref clock rate: %lu\n", dsi-
> > >
> > >ref_clk_rate);
> > >
> > > +
> > > +   dsi->dev = dev;
> > > +   dsi->pdata.max_data_lanes = 4;
> > > +   dsi->pdata.mode_valid = imx93_dsi_mode_valid;
> > > +   dsi->pdata.mode_fixup = imx93_dsi_mode_fixup;
> > > +   dsi->pdata.get_input_bus_fmts = imx93_dsi_get_input_bus_fmts;
> > > +   dsi->pdata.phy_ops = &imx93_dsi_phy_ops;
> > > +   dsi->pdata.host_ops = &imx93_dsi_host_ops;
> > > +   dsi->pdata.priv_data = dsi;
> > > +   platform_set_drvdata(pdev, dsi);
> > > +
> > > +   dsi->dmd = dw_mipi_dsi_probe(pdev, &dsi->pdata);
> > > +   if (IS_ERR(dsi->dmd)) {
> > > +           ret = PTR_ERR(dsi->dmd);
> > > +           if (ret != -EPROBE_DEFER)
> > 
> > > +                   DRM_DEV_ERROR(dev, "failed to probe dw_mipi_dsi:
> > %d\n", ret);
> > 
> > Could you use dev_err_probe here instead?
> 
> Ditto.
> 
> Regards,
> Liu Ying
> 
> > Best regards,
> > Alexander
> > 
> > > +           return ret;
> > > +   }
> > > +
> > > +   return 0;
> > > +}
> > > +
> > > +static void imx93_dsi_remove(struct platform_device *pdev)
> > > +{
> > > +   struct imx93_dsi *dsi = platform_get_drvdata(pdev);
> > > +
> > > +   dw_mipi_dsi_remove(dsi->dmd);
> > > +}
> > > +
> > > +static const struct of_device_id imx93_dsi_dt_ids[] = {
> > > +   { .compatible = "fsl,imx93-mipi-dsi", },
> > > +   { /* sentinel */ }
> > > +};
> > > +MODULE_DEVICE_TABLE(of, imx93_dsi_dt_ids);
> > > +
> > > +static struct platform_driver imx93_dsi_driver = {
> > > +   .probe  = imx93_dsi_probe,
> > > +   .remove_new = imx93_dsi_remove,
> > > +   .driver = {
> > > +           .of_match_table = imx93_dsi_dt_ids,
> > > +           .name = "imx93_mipi_dsi",
> > > +   },
> > > +};
> > > +module_platform_driver(imx93_dsi_driver);
> > > +
> > > +MODULE_DESCRIPTION("Freescale i.MX93 MIPI DSI driver");
> > > +MODULE_AUTHOR("Liu Ying <victor.liu@nxp.com>");
> > > +MODULE_LICENSE("GPL");
> > 
> > --
> > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> > Amtsgericht München, HRB 105018
> > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> > http://www.t/
> > q-
> > group.com%2F&data=05%7C01%7Cvictor.liu%40nxp.com%7Cc42417f9a9484
> > 3ead2b808db876380d3%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
> > %7C638252633665634690%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLj
> > AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%
> > 7C%7C&sdata=GUWOZgHFFp0nKImw2aIAsaqMv9KtgI6%2BD%2BaOdDhJ%2B
> > tU%3D&reserved=0
Liu Ying July 18, 2023, 9:55 a.m. UTC | #4
On Tuesday, July 18, 2023 5:31 PM Alexander Stein <alexander.stein@ew.tq-group.com> wrote:
>
> Hi,

Hi,

>
> Am Dienstag, 18. Juli 2023, 11:00:25 CEST schrieb Ying Liu:
> > On Tuesday, July 18, 2023 3:49 PM Alexander Stein
> <alexander.stein@ew.tq-
> group.com> wrote:
> > > Hi,
> >
> > Hi,
> >
> > > thanks for the patch.
> >
> > Thanks for your review.
> >
> > > Am Montag, 17. Juli 2023, 08:18:31 CEST schrieb Liu Ying:
> > > > 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.
> > > >
> > > > Signed-off-by: Liu Ying <victor.liu@nxp.com>
> > > > ---
> > > >
> > > >  drivers/gpu/drm/bridge/imx/Kconfig          |  10 +
> > > >  drivers/gpu/drm/bridge/imx/Makefile         |   1 +
> > > >  drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c | 934
> > >
> > > ++++++++++++++++++++
> > >
> > > >  3 files changed, 945 insertions(+)
> > > >  create mode 100644 drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c
> > > >
> > > > diff --git a/drivers/gpu/drm/bridge/imx/Kconfig
> > > > b/drivers/gpu/drm/bridge/imx/Kconfig index
> > >
> > > 9fae28db6aa7..5182298c7182
> > >
> > > > 100644
> > > > --- a/drivers/gpu/drm/bridge/imx/Kconfig
> > > > +++ b/drivers/gpu/drm/bridge/imx/Kconfig
> > > > @@ -49,4 +49,14 @@ config DRM_IMX8QXP_PIXEL_LINK_TO_DPI
> > > >
> > > >       Choose this to enable pixel link to display pixel
> > >
> > > interface(PXL2DPI)
> > >
> > > >       found in Freescale i.MX8qxp processor.
> > > >
> > > > +config DRM_IMX93_MIPI_DSI
> > > > +   tristate "Freescale i.MX93 specific extensions for Synopsys DW MIPI
> > >
> > > DSI"
> > >
> > > > +   depends on OF
> > > > +   depends on COMMON_CLK
> > > > +   select DRM_DW_MIPI_DSI
> > > > +   select GENERIC_PHY_MIPI_DPHY
> > > > +   help
> > > > +     Choose this to enable MIPI DSI controller found in Freescale
> > >
> > > i.MX93
> > >
> > > > +     processor.
> > > > +
> > > >
> > > >  endif # ARCH_MXC || COMPILE_TEST
> > > >
> > > > diff --git a/drivers/gpu/drm/bridge/imx/Makefile
> > > > b/drivers/gpu/drm/bridge/imx/Makefile index
> > >
> > > 8e2ebf3399a1..2b0c2e44aa1b
> > >
> > > > 100644
> > > > --- a/drivers/gpu/drm/bridge/imx/Makefile
> > > > +++ b/drivers/gpu/drm/bridge/imx/Makefile
> > > > @@ -4,3 +4,4 @@ obj-$(CONFIG_DRM_IMX8QXP_LDB) += imx8qxp-
> ldb.o
> > > >
> > > >  obj-$(CONFIG_DRM_IMX8QXP_PIXEL_COMBINER) += imx8qxp-pixel-
> > >
> > > combiner.o
> > >
> > > >  obj-$(CONFIG_DRM_IMX8QXP_PIXEL_LINK) += imx8qxp-pixel-link.o
> > > >  obj-$(CONFIG_DRM_IMX8QXP_PIXEL_LINK_TO_DPI) += imx8qxp-
> pxl2dpi.o
> > > >
> > > > +obj-$(CONFIG_DRM_IMX93_MIPI_DSI) += imx93-mipi-dsi.o
> > > > diff --git a/drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c
> > > > b/drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c new file mode 100644
> > > > index 000000000000..77f59e3407a0
> > > > --- /dev/null
> > > > +++ b/drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c
> > > >
> > > > [snip]
> > > >
> > > > +static int imx93_dsi_probe(struct platform_device *pdev)
> > > > +{
> > > > +   struct device *dev = &pdev->dev;
> > > > +   struct device_node *np = dev->of_node;
> > > > +   struct imx93_dsi *dsi;
> > > > +   int ret;
> > > > +
> > > > +   dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
> > > > +   if (!dsi)
> > > > +           return -ENOMEM;
> > > > +
> > > > +   dsi->regmap = syscon_regmap_lookup_by_phandle(np, "fsl,media-
> > >
> > > blk-
> > > ctrl");
> > >
> > > > +   if (IS_ERR(dsi->regmap)) {
> > > > +           ret = PTR_ERR(dsi->regmap);
> > >
> > > > +           DRM_DEV_ERROR(dev, "failed to get block ctrl regmap:
> > > %d\n", ret);
> > >
> > > Could you use dev_err_probe here instead?
> >
> > Maybe, it's better to keep using DRM_DEV_ERROR to achieve consistent
> > error log format across the driver which is implemented in
> drm_dev_printk().
> > I see other DRM drivers do the same.
>
> I see your point. On the other hand the benefit of dev_err_probe() is that the
> message of deferred probe can be seen in
> /sys/kernel/debug/devices_deferred.
> Your check against EPROBE_DEFER will hide the message if something is not
> correct.
>
> Maybe a to be introduced DRM_DEV_ERROR_PROBE might be useful.

Maybe.  I assume this is not a must-have for this series - maybe someone may
come along to introduce it in future.

Regards,
Liu Ying

>
> Best regards,
> Alexander
>
> > > > +           return ret;
> > > > +   }
> > > > +
> > > > +   dsi->clk_pixel = devm_clk_get(dev, "pix");
> > > > +   if (IS_ERR(dsi->clk_pixel)) {
> > > > +           ret = PTR_ERR(dsi->clk_pixel);
> > > > +           if (ret != -EPROBE_DEFER)
> > >
> > > > +                   DRM_DEV_ERROR(dev, "failed to get pixel clock:
> > > %d\n", ret);
> > >
> > > Could you use dev_err_probe here instead?
> >
> > Ditto.
> >
> > > > +           return ret;
> > > > +   }
> > > > +
> > > > +   dsi->clk_cfg = devm_clk_get(dev, "phy_cfg");
> > > > +   if (IS_ERR(dsi->clk_cfg)) {
> > > > +           ret = PTR_ERR(dsi->clk_cfg);
> > > > +           if (ret != -EPROBE_DEFER)
> > >
> > > > +                   DRM_DEV_ERROR(dev, "failed to get phy cfg clock:
> > > %d\n", ret);
> > >
> > > > +           return ret;
> > > > +   }
> > > > +
> > > > +   dsi->clk_ref = devm_clk_get(dev, "phy_ref");
> > > > +   if (IS_ERR(dsi->clk_ref)) {
> > > > +           ret = PTR_ERR(dsi->clk_ref);
> > > > +           if (ret != -EPROBE_DEFER)
> > >
> > > > +                   DRM_DEV_ERROR(dev, "failed to get phy ref clock:
> > > %d\n", ret);
> > >
> > > Could you use dev_err_probe here instead?
> >
> > Ditto.
> >
> > > > +           return ret;
> > > > +   }
> > > > +
> > > > +   dsi->ref_clk_rate = clk_get_rate(dsi->clk_ref);
> > > > +   if (dsi->ref_clk_rate < REF_CLK_RATE_MIN ||
> > > > +       dsi->ref_clk_rate > REF_CLK_RATE_MAX) {
> > > > +           DRM_DEV_ERROR(dev, "invalid phy ref clock rate %lu\n",
> > > > +                         dsi->ref_clk_rate);
> > > > +           return -EINVAL;
> > > > +   }
> > > > +   DRM_DEV_DEBUG_DRIVER(dev, "phy ref clock rate: %lu\n", dsi-
> > > >
> > > >ref_clk_rate);
> > > >
> > > > +
> > > > +   dsi->dev = dev;
> > > > +   dsi->pdata.max_data_lanes = 4;
> > > > +   dsi->pdata.mode_valid = imx93_dsi_mode_valid;
> > > > +   dsi->pdata.mode_fixup = imx93_dsi_mode_fixup;
> > > > +   dsi->pdata.get_input_bus_fmts = imx93_dsi_get_input_bus_fmts;
> > > > +   dsi->pdata.phy_ops = &imx93_dsi_phy_ops;
> > > > +   dsi->pdata.host_ops = &imx93_dsi_host_ops;
> > > > +   dsi->pdata.priv_data = dsi;
> > > > +   platform_set_drvdata(pdev, dsi);
> > > > +
> > > > +   dsi->dmd = dw_mipi_dsi_probe(pdev, &dsi->pdata);
> > > > +   if (IS_ERR(dsi->dmd)) {
> > > > +           ret = PTR_ERR(dsi->dmd);
> > > > +           if (ret != -EPROBE_DEFER)
> > >
> > > > +                   DRM_DEV_ERROR(dev, "failed to probe dw_mipi_dsi:
> > > %d\n", ret);
> > >
> > > Could you use dev_err_probe here instead?
> >
> > Ditto.
> >
> > Regards,
> > Liu Ying
> >
> > > Best regards,
> > > Alexander
> > >
> > > > +           return ret;
> > > > +   }
> > > > +
> > > > +   return 0;
> > > > +}
> > > > +
> > > > +static void imx93_dsi_remove(struct platform_device *pdev)
> > > > +{
> > > > +   struct imx93_dsi *dsi = platform_get_drvdata(pdev);
> > > > +
> > > > +   dw_mipi_dsi_remove(dsi->dmd);
> > > > +}
> > > > +
> > > > +static const struct of_device_id imx93_dsi_dt_ids[] = {
> > > > +   { .compatible = "fsl,imx93-mipi-dsi", },
> > > > +   { /* sentinel */ }
> > > > +};
> > > > +MODULE_DEVICE_TABLE(of, imx93_dsi_dt_ids);
> > > > +
> > > > +static struct platform_driver imx93_dsi_driver = {
> > > > +   .probe  = imx93_dsi_probe,
> > > > +   .remove_new = imx93_dsi_remove,
> > > > +   .driver = {
> > > > +           .of_match_table = imx93_dsi_dt_ids,
> > > > +           .name = "imx93_mipi_dsi",
> > > > +   },
> > > > +};
> > > > +module_platform_driver(imx93_dsi_driver);
> > > > +
> > > > +MODULE_DESCRIPTION("Freescale i.MX93 MIPI DSI driver");
> > > > +MODULE_AUTHOR("Liu Ying <victor.liu@nxp.com>");
> > > > +MODULE_LICENSE("GPL");
> > >
> > > --
> > > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> > > Amtsgericht München, HRB 105018
> > > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> > >
> http://www.t/
> %2F&data=05%7C01%7Cvictor.liu%40nxp.com%7Cefbbc19ba3414f89448308
> db8771b5df%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638252
> 694676247773%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJ
> QIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdat
> a=BCL6xs5i8jZZDFC2ONKMv5%2B1o0mpuD0ocxC3BnUa2To%3D&reserved=0
> > > q-
> > >
> group.com%2F&data=05%7C01%7Cvictor.liu%40nxp.com%7Cc42417f9a9484
> > >
> 3ead2b808db876380d3%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
> > > %7C638252633665634690%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
> wLj
> > >
> AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%
> > >
> 7C%7C&sdata=GUWOZgHFFp0nKImw2aIAsaqMv9KtgI6%2BD%2BaOdDhJ%2B
> > > tU%3D&reserved=0
>
>
> --
> TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht München, HRB 105018
> Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> http://www.t/
> q-
> group.com%2F&data=05%7C01%7Cvictor.liu%40nxp.com%7Cefbbc19ba3414
> f89448308db8771b5df%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%
> 7C638252694676247773%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
> wMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7
> C%7C&sdata=%2FCtQd5%2BYCDCjOKmsbiO2LSeE1hqQsrmhepmSGalydhc%3
> D&reserved=0
>