Re: [PATCH 1/3] drm/bridge/synopsys: Add MIPI DSI2 host controller bridge

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 06/11/2024 13:33, Heiko Stuebner wrote:
From: Heiko Stuebner <heiko.stuebner@xxxxxxxxx>

Add a Synopsys Designware MIPI DSI host DRM bridge driver for their
DSI2 host controller, based on the Rockchip version from the driver
rockchip/dw-mipi-dsi2.c in their vendor-kernel with phy & bridge APIs.

While the driver is heavily modelled after the previous IP, the register
set of this DSI2 controller is completely different and there are also
additional properties like the variable-width phy interface.

Signed-off-by: Heiko Stuebner <heiko.stuebner@xxxxxxxxx>
---
  drivers/gpu/drm/bridge/synopsys/Kconfig       |    6 +
  drivers/gpu/drm/bridge/synopsys/Makefile      |    1 +
  .../gpu/drm/bridge/synopsys/dw-mipi-dsi2.c    | 1034 +++++++++++++++++
  include/drm/bridge/dw_mipi_dsi2.h             |   94 ++
  4 files changed, 1135 insertions(+)
  create mode 100644 drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi2.c
  create mode 100644 include/drm/bridge/dw_mipi_dsi2.h

diff --git a/drivers/gpu/drm/bridge/synopsys/Kconfig b/drivers/gpu/drm/bridge/synopsys/Kconfig
index ca416dab156d..f3ab2f985f8c 100644
--- a/drivers/gpu/drm/bridge/synopsys/Kconfig
+++ b/drivers/gpu/drm/bridge/synopsys/Kconfig
@@ -59,3 +59,9 @@ config DRM_DW_MIPI_DSI
  	select DRM_KMS_HELPER
  	select DRM_MIPI_DSI
  	select DRM_PANEL_BRIDGE
+
+config DRM_DW_MIPI_DSI2
+	tristate
+	select DRM_KMS_HELPER
+	select DRM_MIPI_DSI
+	select DRM_PANEL_BRIDGE
diff --git a/drivers/gpu/drm/bridge/synopsys/Makefile b/drivers/gpu/drm/bridge/synopsys/Makefile
index 9869d9651ed1..9dc376d220ad 100644
--- a/drivers/gpu/drm/bridge/synopsys/Makefile
+++ b/drivers/gpu/drm/bridge/synopsys/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_DRM_DW_HDMI_CEC) += dw-hdmi-cec.o
  obj-$(CONFIG_DRM_DW_HDMI_QP) += dw-hdmi-qp.o
obj-$(CONFIG_DRM_DW_MIPI_DSI) += dw-mipi-dsi.o
+obj-$(CONFIG_DRM_DW_MIPI_DSI2) += dw-mipi-dsi2.o
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi2.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi2.c
new file mode 100644
index 000000000000..43738fe3cb93
--- /dev/null
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi2.c
@@ -0,0 +1,1034 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2024, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * Modified by Heiko Stuebner <heiko.stuebner@xxxxxxxxx>
+ * This generic Synopsys DesignWare MIPI DSI2 host driver is based on the
+ * Rockchip version from rockchip/dw-mipi-dsi2.c converted to use bridge APIs.
+ */
+
+#include <linux/clk.h>
+#include <linux/iopoll.h>
+#include <linux/media-bus-format.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+
+#include <video/mipi_display.h>
+
+#include <drm/bridge/dw_mipi_dsi2.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_of.h>
+#include <drm/drm_print.h>
+
+#define UPDATE(v, h, l)			(((v) << (l)) & GENMASK((h), (l)))

I'm not super fan of this macro, overall I thinkg you should switch to
regmap and make use of regmap_update_bits and drop dsi2_write/read wrappers
to readl/writel.

+
+#define DSI2_PWR_UP			0x000c
+#define RESET				0
+#define POWER_UP			BIT(0)
+#define CMD_TX_MODE(x)			UPDATE(x,  24,  24)
+#define DSI2_SOFT_RESET			0x0010
+#define SYS_RSTN			BIT(2)
+#define PHY_RSTN			BIT(1)
+#define IPI_RSTN			BIT(0)
+#define INT_ST_MAIN			0x0014
+#define DSI2_MODE_CTRL			0x0018
+#define DSI2_MODE_STATUS		0x001c
+#define DSI2_CORE_STATUS		0x0020
+#define PRI_RD_DATA_AVAIL		BIT(26)
+#define PRI_FIFOS_NOT_EMPTY		BIT(25)
+#define PRI_BUSY			BIT(24)
+#define CRI_RD_DATA_AVAIL		BIT(18)
+#define CRT_FIFOS_NOT_EMPTY		BIT(17)
+#define CRI_BUSY			BIT(16)
+#define IPI_FIFOS_NOT_EMPTY		BIT(9)
+#define IPI_BUSY			BIT(8)
+#define CORE_FIFOS_NOT_EMPTY		BIT(1)
+#define CORE_BUSY			BIT(0)
+#define MANUAL_MODE_CFG			0x0024
+#define MANUAL_MODE_EN			BIT(0)
+#define DSI2_TIMEOUT_HSTX_CFG		0x0048
+#define TO_HSTX(x)			UPDATE(x, 15, 0)
+#define DSI2_TIMEOUT_HSTXRDY_CFG	0x004c
+#define TO_HSTXRDY(x)			UPDATE(x, 15, 0)
+#define DSI2_TIMEOUT_LPRX_CFG		0x0050
+#define TO_LPRXRDY(x)			UPDATE(x, 15, 0)
+#define DSI2_TIMEOUT_LPTXRDY_CFG	0x0054
+#define TO_LPTXRDY(x)			UPDATE(x, 15, 0)
+#define DSI2_TIMEOUT_LPTXTRIG_CFG	0x0058
+#define TO_LPTXTRIG(x)			UPDATE(x, 15, 0)
+#define DSI2_TIMEOUT_LPTXULPS_CFG	0x005c
+#define TO_LPTXULPS(x)			UPDATE(x, 15, 0)
+#define DSI2_TIMEOUT_BTA_CFG		0x60
+#define TO_BTA(x)			UPDATE(x, 15, 0)
+

<snip>

+
+static struct dw_mipi_dsi2 *
+__dw_mipi_dsi2_probe(struct platform_device *pdev,
+		     const struct dw_mipi_dsi2_plat_data *plat_data)
+{
+	struct device *dev = &pdev->dev;
+	struct reset_control *apb_rst;
+	struct dw_mipi_dsi2 *dsi2;
+	int ret;
+
+	dsi2 = devm_kzalloc(dev, sizeof(*dsi2), GFP_KERNEL);
+	if (!dsi2)
+		return ERR_PTR(-ENOMEM);
+
+	dsi2->dev = dev;
+	dsi2->plat_data = plat_data;
+
+	if (!plat_data->phy_ops->init || !plat_data->phy_ops->get_lane_mbps ||
+	    !plat_data->phy_ops->get_timing)
+		return dev_err_ptr_probe(dev, -ENODEV, "Phy not properly configured\n");
+
+	if (!plat_data->base) {
+		dsi2->base = devm_platform_ioremap_resource(pdev, 0);
+		if (IS_ERR(dsi2->base))
+			return ERR_PTR(-ENODEV);
+	} else {
+		dsi2->base = plat_data->base;
+	}
+
+	dsi2->pclk = devm_clk_get(dev, "pclk");
+	if (IS_ERR(dsi2->pclk))
+		return dev_err_cast_probe(dev, dsi2->pclk, "Unable to get pclk\n");
+
+	dsi2->sys_clk = devm_clk_get(dev, "sys");
+	if (IS_ERR(dsi2->sys_clk))
+		return dev_err_cast_probe(dev, dsi2->sys_clk, "Unable to get sys_clk\n");
+
+	/*
+	 * Note that the reset was not defined in the initial device tree, so
+	 * we have to be prepared for it not being found.
+	 */
+	apb_rst = devm_reset_control_get_optional_exclusive(dev, "apb");
+	if (IS_ERR(apb_rst))
+		return dev_err_cast_probe(dev, apb_rst, "Unable to get reset control\n");
+
+	if (apb_rst) {
+		ret = clk_prepare_enable(dsi2->pclk);
+		if (ret) {
+			dev_err(dev, "%s: Failed to enable pclk\n", __func__);
+			return ERR_PTR(ret);
+		}
+
+		reset_control_assert(apb_rst);
+		usleep_range(10, 20);
+		reset_control_deassert(apb_rst);
+
+		clk_disable_unprepare(dsi2->pclk);
+	}
+
+	pm_runtime_enable(dev);
+
+	dsi2->dsi_host.ops = &dw_mipi_dsi2_host_ops;
+	dsi2->dsi_host.dev = dev;
+	ret = mipi_dsi_host_register(&dsi2->dsi_host);
+	if (ret) {
+		dev_err(dev, "Failed to register MIPI host: %d\n", ret);
+		pm_runtime_disable(dev);
+		return ERR_PTR(ret);
+	}
+
+	dsi2->bridge.driver_private = dsi2;
+	dsi2->bridge.funcs = &dw_mipi_dsi2_bridge_funcs;
+	dsi2->bridge.of_node = pdev->dev.of_node;
+
+	return dsi2;
+}
+
+static void __dw_mipi_dsi2_remove(struct dw_mipi_dsi2 *dsi2)
+{
+	mipi_dsi_host_unregister(&dsi2->dsi_host);
+
+	pm_runtime_disable(dsi2->dev);
+}
+
+/*
+ * Probe/remove API, used from platforms based on the DRM bridge API.
+ */
+struct dw_mipi_dsi2 *
+dw_mipi_dsi2_probe(struct platform_device *pdev,
+		   const struct dw_mipi_dsi2_plat_data *plat_data)
+{
+	return __dw_mipi_dsi2_probe(pdev, plat_data);
+}
+EXPORT_SYMBOL_GPL(dw_mipi_dsi2_probe);
+
+void dw_mipi_dsi2_remove(struct dw_mipi_dsi2 *dsi2)
+{
+	__dw_mipi_dsi2_remove(dsi2);
+}
+EXPORT_SYMBOL_GPL(dw_mipi_dsi2_remove);

Since it's not use yet, you should probably drop those since it's dead
code.

+
+/*
+ * Bind/unbind API, used from platforms based on the component framework.
+ */
+int dw_mipi_dsi2_bind(struct dw_mipi_dsi2 *dsi2, struct drm_encoder *encoder)
+{
+	return drm_bridge_attach(encoder, &dsi2->bridge, NULL, 0);
+}
+EXPORT_SYMBOL_GPL(dw_mipi_dsi2_bind);
+
+void dw_mipi_dsi2_unbind(struct dw_mipi_dsi2 *dsi2)
+{
+}
+EXPORT_SYMBOL_GPL(dw_mipi_dsi2_unbind);
+
+MODULE_AUTHOR("Guochun Huang <hero.huang@xxxxxxxxxxxxxx>");
+MODULE_AUTHOR("Heiko Stuebner <heiko.stuebner@xxxxxxxxx>");
+MODULE_DESCRIPTION("DW MIPI DSI2 host controller driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:dw-mipi-dsi2");
diff --git a/include/drm/bridge/dw_mipi_dsi2.h b/include/drm/bridge/dw_mipi_dsi2.h
new file mode 100644
index 000000000000..ef5479b35028
--- /dev/null
+++ b/include/drm/bridge/dw_mipi_dsi2.h
@@ -0,0 +1,94 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2024, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * Authors: Guochun Huang <hero.huang@xxxxxxxxxxxxxx>
+ *          Heiko Stuebner <heiko.stuebner@xxxxxxxxx>
+ */
+
+#ifndef __DW_MIPI_DSI2__
+#define __DW_MIPI_DSI2__
+
+#include <linux/types.h>
+
+#include <drm/drm_atomic.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_connector.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_modes.h>
+
+struct drm_display_mode;
+struct drm_encoder;
+struct dw_mipi_dsi2;
+struct mipi_dsi_device;
+struct platform_device;
+
+enum dw_mipi_dsi2_phy_type {
+	DW_MIPI_DSI2_DPHY,
+	DW_MIPI_DSI2_CPHY,
+};
+
+struct dw_mipi_dsi2_phy_iface {
+	int ppi_width;
+	enum dw_mipi_dsi2_phy_type phy_type;
+};
+
+struct dw_mipi_dsi2_phy_timing {
+	u32 data_hs2lp;
+	u32 data_lp2hs;
+};
+
+struct dw_mipi_dsi2_phy_ops {
+	int (*init)(void *priv_data);
+	void (*power_on)(void *priv_data);
+	void (*power_off)(void *priv_data);
+	void (*get_interface)(void *priv_data, struct dw_mipi_dsi2_phy_iface *iface);
+	int (*get_lane_mbps)(void *priv_data,
+			     const struct drm_display_mode *mode,
+			     unsigned long mode_flags, u32 lanes, u32 format,
+			     unsigned int *lane_mbps);
+	int (*get_timing)(void *priv_data, unsigned int lane_mbps,
+			  struct dw_mipi_dsi2_phy_timing *timing);
+	int (*get_esc_clk_rate)(void *priv_data, unsigned int *esc_clk_rate);
+};
+
+struct dw_mipi_dsi2_host_ops {
+	int (*attach)(void *priv_data,
+		      struct mipi_dsi_device *dsi);
+	int (*detach)(void *priv_data,
+		      struct mipi_dsi_device *dsi);
+};
+
+struct dw_mipi_dsi2_plat_data {
+	void __iomem *base;
+	unsigned int max_data_lanes;
+
+	enum drm_mode_status (*mode_valid)(void *priv_data,
+					   const struct drm_display_mode *mode,
+					   unsigned long mode_flags,
+					   u32 lanes, u32 format);
+
+	bool (*mode_fixup)(void *priv_data, const struct drm_display_mode *mode,
+			   struct drm_display_mode *adjusted_mode);
+
+	u32 *(*get_input_bus_fmts)(void *priv_data,
+				   struct drm_bridge *bridge,
+				   struct drm_bridge_state *bridge_state,
+				   struct drm_crtc_state *crtc_state,
+				   struct drm_connector_state *conn_state,
+				   u32 output_fmt,
+				   unsigned int *num_input_fmts);
+
+	const struct dw_mipi_dsi2_phy_ops *phy_ops;
+	const struct dw_mipi_dsi2_host_ops *host_ops;
+
+	void *priv_data;
+};
+
+struct dw_mipi_dsi2 *dw_mipi_dsi2_probe(struct platform_device *pdev,
+					const struct dw_mipi_dsi2_plat_data *plat_data);
+void dw_mipi_dsi2_remove(struct dw_mipi_dsi2 *dsi2);
+int dw_mipi_dsi2_bind(struct dw_mipi_dsi2 *dsi2, struct drm_encoder *encoder);
+void dw_mipi_dsi2_unbind(struct dw_mipi_dsi2 *dsi2);
+
+#endif /* __DW_MIPI_DSI2__ */

Overall the driver is very close to dw-mipi-dsi, si it's overall good!

Thanks,
Neil



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux