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

Hi Sam,

On Wed, Mar 18, 2020 at 10:46:39PM +0100, Sam Ravnborg wrote:

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:

quoted

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 <redacted>
---
 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.

Missed that removal, dropped.

quoted

+
+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).

We don't do a drm_dev_alloc so no drm device here afaik tell.

quoted

+}
+
+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.

Fixed by rebasing on next-20200317. I'll send a new version after
letting this sit for a moment. It'd be so cool to get this queued up so
we get a working display stack on the imx8mq.

Cheers,
 -- Guido

quoted

+
+	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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel