Re: [PATCH v9 2/2] drm/bridge: Add NWL MIPI DSI host controller support

Hi Guido.

Impressive and very detailed changelog in intro mail - nice.

On Wed, Mar 18, 2020 at 04:09:08PM +0100, Guido Günther wrote:

This adds initial support for the NWL MIPI DSI Host controller found on i.MX8 SoCs.

It adds support for the i.MX8MQ but the same IP can be found on e.g. the i.MX8QXP.

It has been tested on the Librem 5 devkit using mxsfb.

Signed-off-by: Guido Günther agx@sigxcpu.org Co-developed-by: Robert Chiras robert.chiras@nxp.com Signed-off-by: Robert Chiras robert.chiras@nxp.com Tested-by: Robert Chiras robert.chiras@nxp.com Tested-by: Martin Kepplinger martin.kepplinger@puri.sm

---

drivers/gpu/drm/bridge/Kconfig | 16 + drivers/gpu/drm/bridge/Makefile | 3 + drivers/gpu/drm/bridge/nwl-dsi.c | 1213 ++++++++++++++++++++++++++++++ drivers/gpu/drm/bridge/nwl-dsi.h | 144 ++++ 4 files changed, 1376 insertions(+) create mode 100644 drivers/gpu/drm/bridge/nwl-dsi.c create mode 100644 drivers/gpu/drm/bridge/nwl-dsi.h

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index 8397bf72d2f3..d41d93d24f16 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -55,6 +55,22 @@ config DRM_MEGACHIPS_STDPXXXX_GE_B850V3_FW to DP++. This is used with the i.MX6 imx-ldb driver. You are likely to say N here.

+config DRM_NWL_MIPI_DSI

• tristate "Northwest Logic MIPI DSI Host controller"
• depends on DRM
• depends on COMMON_CLK
• depends on OF && HAS_IOMEM
• select DRM_KMS_HELPER
• select DRM_MIPI_DSI
• select DRM_PANEL_BRIDGE
• select GENERIC_PHY_MIPI_DPHY
• select MFD_SYSCON
• select MULTIPLEXER
• select REGMAP_MMIO
• help

•  This enables the Northwest Logic MIPI DSI Host controller as
  

•  for example found on NXP's i.MX8 Processors.
  

config DRM_NXP_PTN3460 tristate "NXP PTN3460 DP/LVDS bridge" depends on OF diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile index 1eb5376c5d68..98581b3128a3 100644 --- a/drivers/gpu/drm/bridge/Makefile +++ b/drivers/gpu/drm/bridge/Makefile @@ -15,6 +15,9 @@ obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/ obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o +obj-$(CONFIG_DRM_NWL_MIPI_DSI) += nwl-dsi.o

obj-y += analogix/ obj-y += synopsys/

+header-test-y += nwl-dsi.h

Sorry - but header-test-y support was ripped out of the kernel again. So this line has no longer any effect.

+static void nwl_dsi_bridge_enable(struct drm_bridge *bridge) +{

• struct nwl_dsi *dsi = bridge_to_dsi(bridge);
• int ret;
• /* Step 5 from DSI reset-out instructions */
• ret = reset_control_deassert(dsi->rst_dpi);
• if (ret < 0)

• DRM_DEV_ERROR(dsi->dev, "Failed to deassert DPI: %d\n", ret);
  

I picked this for a general comment.

We have drm_err(drm, "...", ...) which is preferred over DRM_XXX They require a drm_device * that may not be available everywhere.

IMO not a showstopper, but should be trivial to fix (if adrm_device * is a avaiable).

+}

+static int nwl_dsi_bridge_attach(struct drm_bridge *bridge) +{

• struct nwl_dsi *dsi = bridge_to_dsi(bridge);
• struct drm_bridge *panel_bridge;
• struct drm_panel *panel;
• int ret;

This function now takes a flags argument. In other words - the driver will not build when applied to drm-misc-next.

• ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 1, 0, &panel,

• 			  &panel_bridge);
  

• if (ret)

• return ret;
  

• if (panel) {

• panel_bridge = drm_panel_bridge_add(panel);
  

• if (IS_ERR(panel_bridge))
  

• 	return PTR_ERR(panel_bridge);
  

• }
• dsi->panel_bridge = panel_bridge;
• if (!dsi->panel_bridge)

• return -EPROBE_DEFER;
  

• return drm_bridge_attach(bridge->encoder, dsi->panel_bridge, bridge);

+}

Sam