[PATCH V2 0/2] Add Xilinx DSI-Tx subsystem DRM driver

[PATCH V2 0/2] Add Xilinx DSI-Tx subsystem DRM driver

[Venkateshwar Rao Gannavarapu]

MIPI DSI TX subsystem allows you to quickly create systems based on the
MIPI protocol. It interfaces between the video processing subsystems and
MIPI-based displays. An internal high-speed physical layer design, D-PHY,
is provided to allow direct connection to display peripherals.

The subsystem consists of the following sub-blocks:
- MIPI D-PHY
- MIPI DSI TX Controller
- AXI Crossbar

Please refer pg238 [1].

The DSI TX Controller receives stream of image data through an input stream
interface. At design time, this subsystem can be configured to number of lanes
and pixel format.

This patch series adds the dt-binding and DRM driver for Xilinx DSI-Tx soft IP.

Changes in V2:
    - Rebased on 5.19 kernel
    - Moved mode_set functionality to atomic_enable as its deprecrated
    - Mask fixes
    - Replaced panel calls with bridge API's
    - Added mandatory atomic operations
    - Removed unnecessary logging
    - Added ARCH_ZYNQMP dependency in Kconfig
    - Fixed YAML warnings
    - Cleanup

Venkateshwar Rao Gannavarapu (2):
  dt-bindings: display: xlnx: Add DSI 2.0 Tx subsystem documentation
  drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver

 .../bindings/display/xlnx/xlnx,dsi-tx.yaml         | 101 +++++
 drivers/gpu/drm/xlnx/Kconfig                       |  12 +
 drivers/gpu/drm/xlnx/Makefile                      |   1 +
 drivers/gpu/drm/xlnx/xlnx_dsi.c                    | 429 +++++++++++++++++++++
 4 files changed, 543 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml
 create mode 100644 drivers/gpu/drm/xlnx/xlnx_dsi.c

-- 
1.8.3.1

[PATCH V2 0/2] Add Xilinx DSI-Tx subsystem DRM driver

[Venkateshwar Rao Gannavarapu]

MIPI DSI TX subsystem allows you to quickly create systems based on the
MIPI protocol. It interfaces between the video processing subsystems and
MIPI-based displays. An internal high-speed physical layer design, D-PHY,
is provided to allow direct connection to display peripherals.

The subsystem consists of the following sub-blocks:
- MIPI D-PHY
- MIPI DSI TX Controller
- AXI Crossbar

Please refer pg238 [1].

The DSI TX Controller receives stream of image data through an input stream
interface. At design time, this subsystem can be configured to number of lanes
and pixel format.

This patch series adds the dt-binding and DRM driver for Xilinx DSI-Tx soft IP.

Changes in V2:
    - Rebased on 5.19 kernel
    - Moved mode_set functionality to atomic_enable as its deprecrated
    - Mask fixes
    - Replaced panel calls with bridge API's
    - Added mandatory atomic operations
    - Removed unnecessary logging
    - Added ARCH_ZYNQMP dependency in Kconfig
    - Fixed YAML warnings
    - Cleanup

Venkateshwar Rao Gannavarapu (2):
  dt-bindings: display: xlnx: Add DSI 2.0 Tx subsystem documentation
  drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver

 .../bindings/display/xlnx/xlnx,dsi-tx.yaml         | 101 +++++
 drivers/gpu/drm/xlnx/Kconfig                       |  12 +
 drivers/gpu/drm/xlnx/Makefile                      |   1 +
 drivers/gpu/drm/xlnx/xlnx_dsi.c                    | 429 +++++++++++++++++++++
 4 files changed, 543 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml
 create mode 100644 drivers/gpu/drm/xlnx/xlnx_dsi.c

-- 
1.8.3.1

[PATCH V2 1/2] dt-bindings: display: xlnx: Add DSI 2.0 Tx subsystem documentation

[Venkateshwar Rao Gannavarapu]

This patch adds dt binding for Xilinx DSI-TX subsystem.

The Xilinx MIPI DSI (Display serial interface) Transmitter Subsystem
implements the Mobile Industry Processor Interface (MIPI) based display
interface. It supports the interface with the programmable logic (FPGA).

Signed-off-by: Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>
---
 .../bindings/display/xlnx/xlnx,dsi-tx.yaml         | 101 +++++++++++++++++++++
 1 file changed, 101 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml

diff --git a/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml b/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml
new file mode 100644
index 0000000..644934d
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml
@@ -0,0 +1,101 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/xlnx/xlnx,dsi-tx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx DSI Transmitter subsystem Device Tree Bindings
+
+maintainers:
+  - Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>
+
+description: |
+  The Xilinx DSI Transmitter Subsystem implements the Mobile Industry
+  Processor Interface based display interface. It supports the interface
+  with the programmable logic (FPGA).
+
+  For more details refer to PG238 Xilinx MIPI DSI-V2.0 Tx Subsystem.
+
+properties:
+  compatible:
+    const: xlnx,dsi-tx-v2.0
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: AXI Lite CPU clock
+      - description: D-PHY clock
+
+  clock-names:
+    items:
+      - const: s_axis_aclk
+      - const: dphy_clk_200M
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/properties/port
+        description:
+          This port should be the input endpoint.
+
+      port@1:
+        $ref: /schemas/graph.yaml#/properties/port
+        description:
+          This port should be the output endpoint.
+
+required:
+  - "#address-cells"
+  - "#size-cells"
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    dsi0: dsi_tx@80020000 {
+        compatible = "xlnx,dsi-tx-v2.0";
+        reg = <0x80020000 0x20000>;
+        clocks = <&misc_clk_0>, <&misc_clk_1>;
+        clock-names = "s_axis_aclk", "dphy_clk_200M";
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        ports {
+              #address-cells = <1>;
+              #size-cells = <0>;
+
+              port@0 {
+                    reg = <0>;
+                    mipi_dsi_in: endpoint {
+                        remote-endpoint = <&pl_disp>;
+                    };
+              };
+
+              port@1 {
+                    reg = <1>;
+                    mipi_dsi_out: endpoint {
+                        remote-endpoint = <&panel_in>;
+                    };
+              };
+        };
+
+        panel@0 {
+              compatible = "auo,b101uan01";
+              reg = <0>;
+              port {
+                    panel_in: endpoint {
+                        remote-endpoint = <&mipi_dsi_out>;
+                    };
+              };
+        };
+    };
+
+...
--
1.8.3.1

[PATCH V2 1/2] dt-bindings: display: xlnx: Add DSI 2.0 Tx subsystem documentation

[Venkateshwar Rao Gannavarapu]

This patch adds dt binding for Xilinx DSI-TX subsystem.

The Xilinx MIPI DSI (Display serial interface) Transmitter Subsystem
implements the Mobile Industry Processor Interface (MIPI) based display
interface. It supports the interface with the programmable logic (FPGA).

Signed-off-by: Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>
---
 .../bindings/display/xlnx/xlnx,dsi-tx.yaml         | 101 +++++++++++++++++++++
 1 file changed, 101 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml

diff --git a/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml b/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml
new file mode 100644
index 0000000..644934d
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml
@@ -0,0 +1,101 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/xlnx/xlnx,dsi-tx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx DSI Transmitter subsystem Device Tree Bindings
+
+maintainers:
+  - Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>
+
+description: |
+  The Xilinx DSI Transmitter Subsystem implements the Mobile Industry
+  Processor Interface based display interface. It supports the interface
+  with the programmable logic (FPGA).
+
+  For more details refer to PG238 Xilinx MIPI DSI-V2.0 Tx Subsystem.
+
+properties:
+  compatible:
+    const: xlnx,dsi-tx-v2.0
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: AXI Lite CPU clock
+      - description: D-PHY clock
+
+  clock-names:
+    items:
+      - const: s_axis_aclk
+      - const: dphy_clk_200M
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/properties/port
+        description:
+          This port should be the input endpoint.
+
+      port@1:
+        $ref: /schemas/graph.yaml#/properties/port
+        description:
+          This port should be the output endpoint.
+
+required:
+  - "#address-cells"
+  - "#size-cells"
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    dsi0: dsi_tx@80020000 {
+        compatible = "xlnx,dsi-tx-v2.0";
+        reg = <0x80020000 0x20000>;
+        clocks = <&misc_clk_0>, <&misc_clk_1>;
+        clock-names = "s_axis_aclk", "dphy_clk_200M";
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        ports {
+              #address-cells = <1>;
+              #size-cells = <0>;
+
+              port@0 {
+                    reg = <0>;
+                    mipi_dsi_in: endpoint {
+                        remote-endpoint = <&pl_disp>;
+                    };
+              };
+
+              port@1 {
+                    reg = <1>;
+                    mipi_dsi_out: endpoint {
+                        remote-endpoint = <&panel_in>;
+                    };
+              };
+        };
+
+        panel@0 {
+              compatible = "auo,b101uan01";
+              reg = <0>;
+              port {
+                    panel_in: endpoint {
+                        remote-endpoint = <&mipi_dsi_out>;
+                    };
+              };
+        };
+    };
+
+...
--
1.8.3.1

[PATCH V2 2/2] drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver

[Venkateshwar Rao Gannavarapu]

The Xilinx MIPI DSI Tx Subsystem soft IP is used to display video
data from AXI-4 stream interface.

It supports upto 4 lanes, optional register interface for the DPHY
and multiple RGB color formats.
This is a MIPI-DSI host driver and provides DSI bus for panels.
This driver also helps to communicate with its panel using panel
framework.

Signed-off-by: Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>
---
 drivers/gpu/drm/xlnx/Kconfig    |  12 ++
 drivers/gpu/drm/xlnx/Makefile   |   1 +
 drivers/gpu/drm/xlnx/xlnx_dsi.c | 429 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 442 insertions(+)
 create mode 100644 drivers/gpu/drm/xlnx/xlnx_dsi.c

diff --git a/drivers/gpu/drm/xlnx/Kconfig b/drivers/gpu/drm/xlnx/Kconfig
index f9cf93c..a75bd76 100644
--- a/drivers/gpu/drm/xlnx/Kconfig
+++ b/drivers/gpu/drm/xlnx/Kconfig
@@ -1,3 +1,15 @@
+config DRM_XLNX_DSI
+	tristate "Xilinx DRM DSI Subsystem Driver"
+	depends on ARCH_ZYNQMP || COMPILE_TEST
+	depends on DRM && OF
+	select DRM_KMS_HELPER
+	select DRM_MIPI_DSI
+	select DRM_PANEL_BRIDGE
+	help
+	  DRM bridge driver for Xilinx programmable DSI subsystem controller.
+	  choose this option if you hava a Xilinx MIPI-DSI Tx subsytem in
+	  video pipeline.
+
 config DRM_ZYNQMP_DPSUB
 	tristate "ZynqMP DisplayPort Controller Driver"
 	depends on ARCH_ZYNQMP || COMPILE_TEST
diff --git a/drivers/gpu/drm/xlnx/Makefile b/drivers/gpu/drm/xlnx/Makefile
index 51c24b7..f90849b 100644
--- a/drivers/gpu/drm/xlnx/Makefile
+++ b/drivers/gpu/drm/xlnx/Makefile
@@ -1,2 +1,3 @@
+obj-$(CONFIG_DRM_XLNX_DSI) += xlnx_dsi.o
 zynqmp-dpsub-y := zynqmp_disp.o zynqmp_dpsub.o zynqmp_dp.o
 obj-$(CONFIG_DRM_ZYNQMP_DPSUB) += zynqmp-dpsub.o
diff --git a/drivers/gpu/drm/xlnx/xlnx_dsi.c b/drivers/gpu/drm/xlnx/xlnx_dsi.c
new file mode 100644
index 0000000..39d8947
--- /dev/null
+++ b/drivers/gpu/drm/xlnx/xlnx_dsi.c
@@ -0,0 +1,429 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx FPGA MIPI DSI Tx Controller driver.
+ *
+ * Copyright (C) 2022 Xilinx, Inc.
+ *
+ * Author: Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_of.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_print.h>
+
+/* DSI Tx IP registers */
+#define XDSI_CCR			0x00
+#define XDSI_CCR_COREENB		BIT(0)
+#define XDSI_CCR_SOFTRST		BIT(1)
+#define XDSI_CCR_CRREADY		BIT(2)
+#define XDSI_CCR_CMDMODE		BIT(3)
+#define XDSI_CCR_DFIFORST		BIT(4)
+#define XDSI_CCR_CMDFIFORST		BIT(5)
+#define XDSI_PCR			0x04
+#define XDSI_PCR_LANES_MASK		3
+#define XDSI_PCR_VIDEOMODE(x)		(((x) & 0x3) << 3)
+#define XDSI_PCR_VIDEOMODE_MASK	GENMASK(4, 3)
+#define XDSI_PCR_VIDEOMODE_SHIFT	3
+#define XDSI_PCR_BLLPTYPE(x)		((x) << 5)
+#define XDSI_PCR_BLLPMODE(x)		((x) << 6)
+#define XDSI_PCR_PIXELFORMAT_MASK	GENMASK(12, 11)
+#define XDSI_PCR_PIXELFORMAT_SHIFT	11
+#define XDSI_PCR_EOTPENABLE(x)		((x) << 13)
+#define XDSI_GIER			0x20
+#define XDSI_ISR			0x24
+#define XDSI_IER			0x28
+#define XDSI_STR			0x2C
+#define XDSI_STR_RDY_SHPKT		BIT(6)
+#define XDSI_STR_RDY_LNGPKT		BIT(7)
+#define XDSI_STR_DFIFO_FULL		BIT(8)
+#define XDSI_STR_DFIFO_EMPTY		BIT(9)
+#define XDSI_STR_WAITFR_DATA		BIT(10)
+#define XDSI_STR_CMD_EXE_PGS		BIT(11)
+#define XDSI_STR_CCMD_PROC		BIT(12)
+#define XDSI_CMD			0x30
+#define XDSI_CMD_QUEUE_PACKET(x)	((x) & GENMASK(23, 0))
+#define XDSI_DFR			0x34
+#define XDSI_TIME1			0x50
+#define XDSI_TIME1_BLLP_BURST(x)	((x) & GENMASK(15, 0))
+#define XDSI_TIME1_HSA(x)		(((x) & GENMASK(15, 0)) << 16)
+#define XDSI_TIME2			0x54
+#define XDSI_TIME2_VACT(x)		((x) & GENMASK(15, 0))
+#define XDSI_TIME2_HACT(x)		(((x) & GENMASK(15, 0)) << 16)
+#define XDSI_HACT_MULTIPLIER		GENMASK(1, 0)
+#define XDSI_TIME3			0x58
+#define XDSI_TIME3_HFP(x)		((x) & GENMASK(15, 0))
+#define XDSI_TIME3_HBP(x)		(((x) & GENMASK(15, 0)) << 16)
+#define XDSI_TIME4			0x5c
+#define XDSI_TIME4_VFP(x)		((x) & GENMASK(7, 0))
+#define XDSI_TIME4_VBP(x)		(((x) & GENMASK(7, 0)) << 8)
+#define XDSI_TIME4_VSA(x)		(((x) & GENMASK(7, 0)) << 16)
+#define XDSI_NUM_DATA_T		4
+
+/**
+ * struct xlnx_dsi - Xilinx DSI-TX core
+ * @bridge: DRM bridge structure
+ * @dsi_host: DSI host device
+ * @next_bridge: bridge structure
+ * @dev: device structure
+ * @clks: clock source structure
+ * @iomem: Base address of DSI subsystem
+ * @mode_flags: DSI operation mode related flags
+ * @lanes: number of active data lanes supported by DSI controller
+ * @mul_factor: multiplication factor for HACT timing
+ * @format: pixel format for video mode of DSI controller
+ * @device_found: Flag to indicate device presence
+ */
+struct xlnx_dsi {
+	struct drm_bridge bridge;
+	struct mipi_dsi_host dsi_host;
+	struct drm_bridge *next_bridge;
+	struct device *dev;
+	struct clk_bulk_data *clks;
+	void __iomem *iomem;
+	unsigned long mode_flags;
+	u32 lanes;
+	u32 mul_factor;
+	enum mipi_dsi_pixel_format format;
+	bool device_found;
+};
+
+static const struct clk_bulk_data xdsi_clks[] = {
+	{ .id = "s_axis_aclk" },
+	{ .id = "dphy_clk_200M" },
+};
+
+static inline struct xlnx_dsi *host_to_dsi(struct mipi_dsi_host *host)
+{
+	return container_of(host, struct xlnx_dsi, dsi_host);
+}
+
+static inline struct xlnx_dsi *bridge_to_dsi(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct xlnx_dsi, bridge);
+}
+
+static inline void xlnx_dsi_write(struct xlnx_dsi *dsi, int offset, u32 val)
+{
+	writel(val, dsi->iomem + offset);
+}
+
+static inline u32 xlnx_dsi_read(struct xlnx_dsi *dsi, int offset)
+{
+	return readl(dsi->iomem + offset);
+}
+
+static int xlnx_dsi_host_attach(struct mipi_dsi_host *host,
+				struct mipi_dsi_device *device)
+{
+	struct xlnx_dsi *dsi = host_to_dsi(host);
+	struct device *dev = host->dev;
+
+	dsi->mode_flags = device->mode_flags;
+
+	if (dsi->lanes != device->lanes) {
+		dev_err(dsi->dev, "Mismatch of lanes. panel = %d, DSI = %d\n",
+			device->lanes, dsi->lanes);
+		return -EINVAL;
+	}
+
+	if (dsi->format != device->format) {
+		dev_err(dsi->dev, "Mismatch of format. panel = %d, DSI = %d\n",
+			device->format, dsi->format);
+		return -EINVAL;
+	}
+
+	if (!dsi->device_found) {
+		dsi->next_bridge = devm_drm_of_get_bridge(dev,
+							  dev->of_node, 0, 0);
+		if (IS_ERR(dsi->next_bridge))
+			return PTR_ERR(dsi->next_bridge);
+		drm_bridge_add(&dsi->bridge);
+		dsi->device_found = true;
+	}
+
+	return 0;
+}
+
+static int xlnx_dsi_host_detach(struct mipi_dsi_host *host,
+				struct mipi_dsi_device *device)
+{
+	struct xlnx_dsi *dsi = host_to_dsi(host);
+
+	drm_bridge_remove(&dsi->bridge);
+	return 0;
+}
+
+static const struct mipi_dsi_host_ops xlnx_dsi_ops = {
+	.attach = xlnx_dsi_host_attach,
+	.detach	= xlnx_dsi_host_detach,
+};
+
+static void
+xlnx_dsi_bridge_disable(struct drm_bridge *bridge,
+			struct drm_bridge_state *old_bridge_state)
+{
+	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
+	u32 reg = xlnx_dsi_read(dsi, XDSI_CCR);
+
+	reg &= ~XDSI_CCR_COREENB;
+	xlnx_dsi_write(dsi, XDSI_CCR, reg);
+}
+
+static void
+xlnx_dsi_bridge_enable(struct drm_bridge *bridge,
+		       struct drm_bridge_state *old_bridge_state)
+{
+	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
+	struct drm_atomic_state *state = old_bridge_state->base.state;
+	struct drm_connector *connector;
+	struct drm_crtc *crtc;
+	const struct drm_crtc_state *crtc_state;
+	const struct drm_display_mode *mode;
+	u32 reg, video_mode;
+
+	connector = drm_atomic_get_new_connector_for_encoder(state,
+							     bridge->encoder);
+	crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
+	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+	mode = &crtc_state->adjusted_mode;
+
+	reg = xlnx_dsi_read(dsi, XDSI_PCR);
+	video_mode = (reg & XDSI_PCR_VIDEOMODE_MASK) >> XDSI_PCR_VIDEOMODE_SHIFT;
+
+	if (!video_mode && (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)) {
+		reg = XDSI_TIME1_HSA(mode->hsync_end -
+				     mode->hsync_start);
+		xlnx_dsi_write(dsi, XDSI_TIME1, reg);
+	}
+
+	reg = XDSI_TIME4_VFP(mode->vsync_start - mode->vdisplay) |
+		XDSI_TIME4_VBP(mode->vtotal - mode->vsync_end) |
+		XDSI_TIME4_VSA(mode->vsync_end - mode->vsync_start);
+	xlnx_dsi_write(dsi, XDSI_TIME4, reg);
+
+	reg = XDSI_TIME3_HFP(mode->hsync_start - mode->hdisplay) |
+		XDSI_TIME3_HBP(mode->htotal - mode->hsync_end);
+	xlnx_dsi_write(dsi, XDSI_TIME3, reg);
+
+	reg = XDSI_TIME2_HACT(mode->hdisplay * dsi->mul_factor / 100) |
+		XDSI_TIME2_VACT(mode->vdisplay);
+	xlnx_dsi_write(dsi->iomem, XDSI_TIME2, reg);
+
+	xlnx_dsi_write(dsi, XDSI_PCR, XDSI_PCR_VIDEOMODE(BIT(0)));
+
+	/* Enable Core */
+	reg = xlnx_dsi_read(dsi, XDSI_CCR);
+	reg |= XDSI_CCR_COREENB;
+	xlnx_dsi_write(dsi, XDSI_CCR, reg);
+}
+
+#define MAX_INPUT_SEL_FORMATS   3
+static u32
+*xlnx_dsi_bridge_atomic_get_input_bus_fmts(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)
+{
+	u32 *input_fmts;
+	unsigned int i = 0;
+
+	*num_input_fmts = 0;
+	input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts), GFP_KERNEL);
+	if (!input_fmts)
+		return NULL;
+
+	switch (output_fmt) {
+	case MEDIA_BUS_FMT_FIXED:
+		input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
+		break;
+	case MEDIA_BUS_FMT_RGB666_1X18:
+		input_fmts[i++] = MEDIA_BUS_FMT_RGB666_1X18;
+		break;
+	case MEDIA_BUS_FMT_RGB565_1X16:
+		input_fmts[i++] = MEDIA_BUS_FMT_RGB565_1X16;
+		break;
+	default: /* define */
+	}
+
+	*num_input_fmts = i;
+	if (*num_input_fmts == 0) {
+		kfree(input_fmts);
+		input_fmts = NULL;
+	}
+
+	return input_fmts;
+}
+
+static int xlnx_dsi_bridge_attach(struct drm_bridge *bridge,
+				  enum drm_bridge_attach_flags flags)
+{
+	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
+
+	if (!dsi->next_bridge)
+		return 0;
+
+	/* Attach the next bridge */
+	return drm_bridge_attach(bridge->encoder, dsi->next_bridge, bridge,
+				 flags);
+}
+
+static void xlnx_dsi_bridge_detach(struct drm_bridge *bridge)
+{
+	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
+
+	drm_of_panel_bridge_remove(dsi->dev->of_node, 1, 0);
+}
+
+static enum drm_mode_status
+xlnx_dsi_bridge_mode_valid(struct drm_bridge *bridge,
+			   const struct drm_display_info *info,
+			   const struct drm_display_mode *mode)
+{
+	if ((mode->hdisplay & XDSI_HACT_MULTIPLIER) != 0)
+		return MODE_BAD_WIDTH;
+
+	return MODE_OK;
+}
+
+static const struct drm_bridge_funcs xlnx_dsi_bridge_funcs = {
+	.atomic_duplicate_state		= drm_atomic_helper_bridge_duplicate_state,
+	.atomic_destroy_state		= drm_atomic_helper_bridge_destroy_state,
+	.atomic_reset			= drm_atomic_helper_bridge_reset,
+	.atomic_disable                 = xlnx_dsi_bridge_disable,
+	.atomic_enable			= xlnx_dsi_bridge_enable,
+	.atomic_get_input_bus_fmts      = xlnx_dsi_bridge_atomic_get_input_bus_fmts,
+	.attach				= xlnx_dsi_bridge_attach,
+	.detach				= xlnx_dsi_bridge_detach,
+	.mode_valid			= xlnx_dsi_bridge_mode_valid,
+};
+
+static int xlnx_dsi_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	struct xlnx_dsi *dsi;
+	int ret;
+	const int xdsi_mul_fact[XDSI_NUM_DATA_T] = {300, 225, 225, 200};
+	u32 reg;
+
+	dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
+	if (!dsi)
+		return -ENOMEM;
+
+	dsi->dev = dev;
+	dsi->clks = devm_kmemdup(dev, xdsi_clks, sizeof(xdsi_clks),
+				 GFP_KERNEL);
+	if (!dsi->clks)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	dsi->iomem = devm_ioremap_resource(dev, res);
+	if (IS_ERR(dsi->iomem))
+		return PTR_ERR(dsi->iomem);
+
+	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(xdsi_clks), dsi->clks);
+	if (ret)
+		return ret;
+
+	ret = clk_bulk_prepare_enable(ARRAY_SIZE(xdsi_clks), dsi->clks);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, dsi);
+	dsi->dsi_host.ops = &xlnx_dsi_ops;
+	dsi->dsi_host.dev = dev;
+
+	ret = mipi_dsi_host_register(&dsi->dsi_host);
+	if (ret) {
+		dev_err(dev, "Failed to register MIPI host: %d\n", ret);
+		goto err_clk_put;
+	}
+
+	dsi->bridge.driver_private = dsi;
+	dsi->bridge.funcs = &xlnx_dsi_bridge_funcs;
+	dsi->bridge.of_node = pdev->dev.of_node;
+
+	reg = xlnx_dsi_read(dsi, XDSI_PCR);
+	dsi->lanes = reg & XDSI_PCR_LANES_MASK;
+	dsi->format = (reg & XDSI_PCR_PIXELFORMAT_MASK) >>
+		XDSI_PCR_PIXELFORMAT_SHIFT;
+
+	if (dsi->lanes > 4 || dsi->lanes < 1) {
+		dev_err(dsi->dev, "%d invalid lanes\n",	dsi->lanes);
+		return -EINVAL;
+	}
+
+	if (dsi->format > MIPI_DSI_FMT_RGB565) {
+		dev_err(dsi->dev, "Invalid xlnx,dsi-data-type string\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * Used as a multiplication factor for HACT based on used
+	 * DSI data type.
+	 *
+	 * e.g. for RGB666_L datatype and 1920x1080 resolution,
+	 * the Hact (WC) would be as follows -
+	 * 1920 pixels * 18 bits per pixel / 8 bits per byte
+	 * = 1920 pixels * 2.25 bytes per pixel = 4320 bytes.
+	 *
+	 * Data Type - Multiplication factor
+	 * RGB888    - 3
+	 * RGB666_L  - 2.25
+-	 * RGB666_P  - 2.25
+	 * RGB565    - 2
+	 *
+	 * Since the multiplication factor is a floating number,
+	 * a 100x multiplication factor is used.
+	 */
+	dsi->mul_factor = xdsi_mul_fact[dsi->format];
+
+	dev_dbg(dsi->dev, "DSI controller num lanes = %d\n", dsi->lanes);
+	dev_dbg(dsi->dev, "DSI controller format = %d\n", dsi->format);
+
+err_clk_put:
+	clk_bulk_disable_unprepare(ARRAY_SIZE(xdsi_clks), dsi->clks);
+
+	return ret;
+}
+
+static int xlnx_dsi_remove(struct platform_device *pdev)
+{
+	struct xlnx_dsi *dsi = platform_get_drvdata(pdev);
+
+	mipi_dsi_host_unregister(&dsi->dsi_host);
+	clk_bulk_disable_unprepare(ARRAY_SIZE(xdsi_clks), dsi->clks);
+
+	return 0;
+}
+
+static const struct of_device_id xlnx_dsi_of_match[] = {
+	{ .compatible = "xlnx,dsi-tx-v2.0"},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, xlnx_dsi_of_match);
+
+static struct platform_driver dsi_driver = {
+	.probe = xlnx_dsi_probe,
+	.remove = xlnx_dsi_remove,
+	.driver = {
+		.name = "xlnx-dsi",
+		.of_match_table = xlnx_dsi_of_match,
+	},
+};
+
+module_platform_driver(dsi_driver);
+
+MODULE_AUTHOR("Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>");
+MODULE_DESCRIPTION("Xilinx MIPI DSI host controller driver");
+MODULE_LICENSE("GPL");
--
1.8.3.1

[PATCH V2 2/2] drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver

[Venkateshwar Rao Gannavarapu]

The Xilinx MIPI DSI Tx Subsystem soft IP is used to display video
data from AXI-4 stream interface.

It supports upto 4 lanes, optional register interface for the DPHY
and multiple RGB color formats.
This is a MIPI-DSI host driver and provides DSI bus for panels.
This driver also helps to communicate with its panel using panel
framework.

Signed-off-by: Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>
---
 drivers/gpu/drm/xlnx/Kconfig    |  12 ++
 drivers/gpu/drm/xlnx/Makefile   |   1 +
 drivers/gpu/drm/xlnx/xlnx_dsi.c | 429 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 442 insertions(+)
 create mode 100644 drivers/gpu/drm/xlnx/xlnx_dsi.c

diff --git a/drivers/gpu/drm/xlnx/Kconfig b/drivers/gpu/drm/xlnx/Kconfig
index f9cf93c..a75bd76 100644
--- a/drivers/gpu/drm/xlnx/Kconfig
+++ b/drivers/gpu/drm/xlnx/Kconfig
@@ -1,3 +1,15 @@
+config DRM_XLNX_DSI
+	tristate "Xilinx DRM DSI Subsystem Driver"
+	depends on ARCH_ZYNQMP || COMPILE_TEST
+	depends on DRM && OF
+	select DRM_KMS_HELPER
+	select DRM_MIPI_DSI
+	select DRM_PANEL_BRIDGE
+	help
+	  DRM bridge driver for Xilinx programmable DSI subsystem controller.
+	  choose this option if you hava a Xilinx MIPI-DSI Tx subsytem in
+	  video pipeline.
+
 config DRM_ZYNQMP_DPSUB
 	tristate "ZynqMP DisplayPort Controller Driver"
 	depends on ARCH_ZYNQMP || COMPILE_TEST
diff --git a/drivers/gpu/drm/xlnx/Makefile b/drivers/gpu/drm/xlnx/Makefile
index 51c24b7..f90849b 100644
--- a/drivers/gpu/drm/xlnx/Makefile
+++ b/drivers/gpu/drm/xlnx/Makefile
@@ -1,2 +1,3 @@
+obj-$(CONFIG_DRM_XLNX_DSI) += xlnx_dsi.o
 zynqmp-dpsub-y := zynqmp_disp.o zynqmp_dpsub.o zynqmp_dp.o
 obj-$(CONFIG_DRM_ZYNQMP_DPSUB) += zynqmp-dpsub.o
diff --git a/drivers/gpu/drm/xlnx/xlnx_dsi.c b/drivers/gpu/drm/xlnx/xlnx_dsi.c
new file mode 100644
index 0000000..39d8947
--- /dev/null
+++ b/drivers/gpu/drm/xlnx/xlnx_dsi.c
@@ -0,0 +1,429 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx FPGA MIPI DSI Tx Controller driver.
+ *
+ * Copyright (C) 2022 Xilinx, Inc.
+ *
+ * Author: Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_of.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_print.h>
+
+/* DSI Tx IP registers */
+#define XDSI_CCR			0x00
+#define XDSI_CCR_COREENB		BIT(0)
+#define XDSI_CCR_SOFTRST		BIT(1)
+#define XDSI_CCR_CRREADY		BIT(2)
+#define XDSI_CCR_CMDMODE		BIT(3)
+#define XDSI_CCR_DFIFORST		BIT(4)
+#define XDSI_CCR_CMDFIFORST		BIT(5)
+#define XDSI_PCR			0x04
+#define XDSI_PCR_LANES_MASK		3
+#define XDSI_PCR_VIDEOMODE(x)		(((x) & 0x3) << 3)
+#define XDSI_PCR_VIDEOMODE_MASK	GENMASK(4, 3)
+#define XDSI_PCR_VIDEOMODE_SHIFT	3
+#define XDSI_PCR_BLLPTYPE(x)		((x) << 5)
+#define XDSI_PCR_BLLPMODE(x)		((x) << 6)
+#define XDSI_PCR_PIXELFORMAT_MASK	GENMASK(12, 11)
+#define XDSI_PCR_PIXELFORMAT_SHIFT	11
+#define XDSI_PCR_EOTPENABLE(x)		((x) << 13)
+#define XDSI_GIER			0x20
+#define XDSI_ISR			0x24
+#define XDSI_IER			0x28
+#define XDSI_STR			0x2C
+#define XDSI_STR_RDY_SHPKT		BIT(6)
+#define XDSI_STR_RDY_LNGPKT		BIT(7)
+#define XDSI_STR_DFIFO_FULL		BIT(8)
+#define XDSI_STR_DFIFO_EMPTY		BIT(9)
+#define XDSI_STR_WAITFR_DATA		BIT(10)
+#define XDSI_STR_CMD_EXE_PGS		BIT(11)
+#define XDSI_STR_CCMD_PROC		BIT(12)
+#define XDSI_CMD			0x30
+#define XDSI_CMD_QUEUE_PACKET(x)	((x) & GENMASK(23, 0))
+#define XDSI_DFR			0x34
+#define XDSI_TIME1			0x50
+#define XDSI_TIME1_BLLP_BURST(x)	((x) & GENMASK(15, 0))
+#define XDSI_TIME1_HSA(x)		(((x) & GENMASK(15, 0)) << 16)
+#define XDSI_TIME2			0x54
+#define XDSI_TIME2_VACT(x)		((x) & GENMASK(15, 0))
+#define XDSI_TIME2_HACT(x)		(((x) & GENMASK(15, 0)) << 16)
+#define XDSI_HACT_MULTIPLIER		GENMASK(1, 0)
+#define XDSI_TIME3			0x58
+#define XDSI_TIME3_HFP(x)		((x) & GENMASK(15, 0))
+#define XDSI_TIME3_HBP(x)		(((x) & GENMASK(15, 0)) << 16)
+#define XDSI_TIME4			0x5c
+#define XDSI_TIME4_VFP(x)		((x) & GENMASK(7, 0))
+#define XDSI_TIME4_VBP(x)		(((x) & GENMASK(7, 0)) << 8)
+#define XDSI_TIME4_VSA(x)		(((x) & GENMASK(7, 0)) << 16)
+#define XDSI_NUM_DATA_T		4
+
+/**
+ * struct xlnx_dsi - Xilinx DSI-TX core
+ * @bridge: DRM bridge structure
+ * @dsi_host: DSI host device
+ * @next_bridge: bridge structure
+ * @dev: device structure
+ * @clks: clock source structure
+ * @iomem: Base address of DSI subsystem
+ * @mode_flags: DSI operation mode related flags
+ * @lanes: number of active data lanes supported by DSI controller
+ * @mul_factor: multiplication factor for HACT timing
+ * @format: pixel format for video mode of DSI controller
+ * @device_found: Flag to indicate device presence
+ */
+struct xlnx_dsi {
+	struct drm_bridge bridge;
+	struct mipi_dsi_host dsi_host;
+	struct drm_bridge *next_bridge;
+	struct device *dev;
+	struct clk_bulk_data *clks;
+	void __iomem *iomem;
+	unsigned long mode_flags;
+	u32 lanes;
+	u32 mul_factor;
+	enum mipi_dsi_pixel_format format;
+	bool device_found;
+};
+
+static const struct clk_bulk_data xdsi_clks[] = {
+	{ .id = "s_axis_aclk" },
+	{ .id = "dphy_clk_200M" },
+};
+
+static inline struct xlnx_dsi *host_to_dsi(struct mipi_dsi_host *host)
+{
+	return container_of(host, struct xlnx_dsi, dsi_host);
+}
+
+static inline struct xlnx_dsi *bridge_to_dsi(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct xlnx_dsi, bridge);
+}
+
+static inline void xlnx_dsi_write(struct xlnx_dsi *dsi, int offset, u32 val)
+{
+	writel(val, dsi->iomem + offset);
+}
+
+static inline u32 xlnx_dsi_read(struct xlnx_dsi *dsi, int offset)
+{
+	return readl(dsi->iomem + offset);
+}
+
+static int xlnx_dsi_host_attach(struct mipi_dsi_host *host,
+				struct mipi_dsi_device *device)
+{
+	struct xlnx_dsi *dsi = host_to_dsi(host);
+	struct device *dev = host->dev;
+
+	dsi->mode_flags = device->mode_flags;
+
+	if (dsi->lanes != device->lanes) {
+		dev_err(dsi->dev, "Mismatch of lanes. panel = %d, DSI = %d\n",
+			device->lanes, dsi->lanes);
+		return -EINVAL;
+	}
+
+	if (dsi->format != device->format) {
+		dev_err(dsi->dev, "Mismatch of format. panel = %d, DSI = %d\n",
+			device->format, dsi->format);
+		return -EINVAL;
+	}
+
+	if (!dsi->device_found) {
+		dsi->next_bridge = devm_drm_of_get_bridge(dev,
+							  dev->of_node, 0, 0);
+		if (IS_ERR(dsi->next_bridge))
+			return PTR_ERR(dsi->next_bridge);
+		drm_bridge_add(&dsi->bridge);
+		dsi->device_found = true;
+	}
+
+	return 0;
+}
+
+static int xlnx_dsi_host_detach(struct mipi_dsi_host *host,
+				struct mipi_dsi_device *device)
+{
+	struct xlnx_dsi *dsi = host_to_dsi(host);
+
+	drm_bridge_remove(&dsi->bridge);
+	return 0;
+}
+
+static const struct mipi_dsi_host_ops xlnx_dsi_ops = {
+	.attach = xlnx_dsi_host_attach,
+	.detach	= xlnx_dsi_host_detach,
+};
+
+static void
+xlnx_dsi_bridge_disable(struct drm_bridge *bridge,
+			struct drm_bridge_state *old_bridge_state)
+{
+	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
+	u32 reg = xlnx_dsi_read(dsi, XDSI_CCR);
+
+	reg &= ~XDSI_CCR_COREENB;
+	xlnx_dsi_write(dsi, XDSI_CCR, reg);
+}
+
+static void
+xlnx_dsi_bridge_enable(struct drm_bridge *bridge,
+		       struct drm_bridge_state *old_bridge_state)
+{
+	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
+	struct drm_atomic_state *state = old_bridge_state->base.state;
+	struct drm_connector *connector;
+	struct drm_crtc *crtc;
+	const struct drm_crtc_state *crtc_state;
+	const struct drm_display_mode *mode;
+	u32 reg, video_mode;
+
+	connector = drm_atomic_get_new_connector_for_encoder(state,
+							     bridge->encoder);
+	crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
+	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+	mode = &crtc_state->adjusted_mode;
+
+	reg = xlnx_dsi_read(dsi, XDSI_PCR);
+	video_mode = (reg & XDSI_PCR_VIDEOMODE_MASK) >> XDSI_PCR_VIDEOMODE_SHIFT;
+
+	if (!video_mode && (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)) {
+		reg = XDSI_TIME1_HSA(mode->hsync_end -
+				     mode->hsync_start);
+		xlnx_dsi_write(dsi, XDSI_TIME1, reg);
+	}
+
+	reg = XDSI_TIME4_VFP(mode->vsync_start - mode->vdisplay) |
+		XDSI_TIME4_VBP(mode->vtotal - mode->vsync_end) |
+		XDSI_TIME4_VSA(mode->vsync_end - mode->vsync_start);
+	xlnx_dsi_write(dsi, XDSI_TIME4, reg);
+
+	reg = XDSI_TIME3_HFP(mode->hsync_start - mode->hdisplay) |
+		XDSI_TIME3_HBP(mode->htotal - mode->hsync_end);
+	xlnx_dsi_write(dsi, XDSI_TIME3, reg);
+
+	reg = XDSI_TIME2_HACT(mode->hdisplay * dsi->mul_factor / 100) |
+		XDSI_TIME2_VACT(mode->vdisplay);
+	xlnx_dsi_write(dsi->iomem, XDSI_TIME2, reg);
+
+	xlnx_dsi_write(dsi, XDSI_PCR, XDSI_PCR_VIDEOMODE(BIT(0)));
+
+	/* Enable Core */
+	reg = xlnx_dsi_read(dsi, XDSI_CCR);
+	reg |= XDSI_CCR_COREENB;
+	xlnx_dsi_write(dsi, XDSI_CCR, reg);
+}
+
+#define MAX_INPUT_SEL_FORMATS   3
+static u32
+*xlnx_dsi_bridge_atomic_get_input_bus_fmts(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)
+{
+	u32 *input_fmts;
+	unsigned int i = 0;
+
+	*num_input_fmts = 0;
+	input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts), GFP_KERNEL);
+	if (!input_fmts)
+		return NULL;
+
+	switch (output_fmt) {
+	case MEDIA_BUS_FMT_FIXED:
+		input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
+		break;
+	case MEDIA_BUS_FMT_RGB666_1X18:
+		input_fmts[i++] = MEDIA_BUS_FMT_RGB666_1X18;
+		break;
+	case MEDIA_BUS_FMT_RGB565_1X16:
+		input_fmts[i++] = MEDIA_BUS_FMT_RGB565_1X16;
+		break;
+	default: /* define */
+	}
+
+	*num_input_fmts = i;
+	if (*num_input_fmts == 0) {
+		kfree(input_fmts);
+		input_fmts = NULL;
+	}
+
+	return input_fmts;
+}
+
+static int xlnx_dsi_bridge_attach(struct drm_bridge *bridge,
+				  enum drm_bridge_attach_flags flags)
+{
+	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
+
+	if (!dsi->next_bridge)
+		return 0;
+
+	/* Attach the next bridge */
+	return drm_bridge_attach(bridge->encoder, dsi->next_bridge, bridge,
+				 flags);
+}
+
+static void xlnx_dsi_bridge_detach(struct drm_bridge *bridge)
+{
+	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
+
+	drm_of_panel_bridge_remove(dsi->dev->of_node, 1, 0);
+}
+
+static enum drm_mode_status
+xlnx_dsi_bridge_mode_valid(struct drm_bridge *bridge,
+			   const struct drm_display_info *info,
+			   const struct drm_display_mode *mode)
+{
+	if ((mode->hdisplay & XDSI_HACT_MULTIPLIER) != 0)
+		return MODE_BAD_WIDTH;
+
+	return MODE_OK;
+}
+
+static const struct drm_bridge_funcs xlnx_dsi_bridge_funcs = {
+	.atomic_duplicate_state		= drm_atomic_helper_bridge_duplicate_state,
+	.atomic_destroy_state		= drm_atomic_helper_bridge_destroy_state,
+	.atomic_reset			= drm_atomic_helper_bridge_reset,
+	.atomic_disable                 = xlnx_dsi_bridge_disable,
+	.atomic_enable			= xlnx_dsi_bridge_enable,
+	.atomic_get_input_bus_fmts      = xlnx_dsi_bridge_atomic_get_input_bus_fmts,
+	.attach				= xlnx_dsi_bridge_attach,
+	.detach				= xlnx_dsi_bridge_detach,
+	.mode_valid			= xlnx_dsi_bridge_mode_valid,
+};
+
+static int xlnx_dsi_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	struct xlnx_dsi *dsi;
+	int ret;
+	const int xdsi_mul_fact[XDSI_NUM_DATA_T] = {300, 225, 225, 200};
+	u32 reg;
+
+	dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
+	if (!dsi)
+		return -ENOMEM;
+
+	dsi->dev = dev;
+	dsi->clks = devm_kmemdup(dev, xdsi_clks, sizeof(xdsi_clks),
+				 GFP_KERNEL);
+	if (!dsi->clks)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	dsi->iomem = devm_ioremap_resource(dev, res);
+	if (IS_ERR(dsi->iomem))
+		return PTR_ERR(dsi->iomem);
+
+	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(xdsi_clks), dsi->clks);
+	if (ret)
+		return ret;
+
+	ret = clk_bulk_prepare_enable(ARRAY_SIZE(xdsi_clks), dsi->clks);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, dsi);
+	dsi->dsi_host.ops = &xlnx_dsi_ops;
+	dsi->dsi_host.dev = dev;
+
+	ret = mipi_dsi_host_register(&dsi->dsi_host);
+	if (ret) {
+		dev_err(dev, "Failed to register MIPI host: %d\n", ret);
+		goto err_clk_put;
+	}
+
+	dsi->bridge.driver_private = dsi;
+	dsi->bridge.funcs = &xlnx_dsi_bridge_funcs;
+	dsi->bridge.of_node = pdev->dev.of_node;
+
+	reg = xlnx_dsi_read(dsi, XDSI_PCR);
+	dsi->lanes = reg & XDSI_PCR_LANES_MASK;
+	dsi->format = (reg & XDSI_PCR_PIXELFORMAT_MASK) >>
+		XDSI_PCR_PIXELFORMAT_SHIFT;
+
+	if (dsi->lanes > 4 || dsi->lanes < 1) {
+		dev_err(dsi->dev, "%d invalid lanes\n",	dsi->lanes);
+		return -EINVAL;
+	}
+
+	if (dsi->format > MIPI_DSI_FMT_RGB565) {
+		dev_err(dsi->dev, "Invalid xlnx,dsi-data-type string\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * Used as a multiplication factor for HACT based on used
+	 * DSI data type.
+	 *
+	 * e.g. for RGB666_L datatype and 1920x1080 resolution,
+	 * the Hact (WC) would be as follows -
+	 * 1920 pixels * 18 bits per pixel / 8 bits per byte
+	 * = 1920 pixels * 2.25 bytes per pixel = 4320 bytes.
+	 *
+	 * Data Type - Multiplication factor
+	 * RGB888    - 3
+	 * RGB666_L  - 2.25
+-	 * RGB666_P  - 2.25
+	 * RGB565    - 2
+	 *
+	 * Since the multiplication factor is a floating number,
+	 * a 100x multiplication factor is used.
+	 */
+	dsi->mul_factor = xdsi_mul_fact[dsi->format];
+
+	dev_dbg(dsi->dev, "DSI controller num lanes = %d\n", dsi->lanes);
+	dev_dbg(dsi->dev, "DSI controller format = %d\n", dsi->format);
+
+err_clk_put:
+	clk_bulk_disable_unprepare(ARRAY_SIZE(xdsi_clks), dsi->clks);
+
+	return ret;
+}
+
+static int xlnx_dsi_remove(struct platform_device *pdev)
+{
+	struct xlnx_dsi *dsi = platform_get_drvdata(pdev);
+
+	mipi_dsi_host_unregister(&dsi->dsi_host);
+	clk_bulk_disable_unprepare(ARRAY_SIZE(xdsi_clks), dsi->clks);
+
+	return 0;
+}
+
+static const struct of_device_id xlnx_dsi_of_match[] = {
+	{ .compatible = "xlnx,dsi-tx-v2.0"},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, xlnx_dsi_of_match);
+
+static struct platform_driver dsi_driver = {
+	.probe = xlnx_dsi_probe,
+	.remove = xlnx_dsi_remove,
+	.driver = {
+		.name = "xlnx-dsi",
+		.of_match_table = xlnx_dsi_of_match,
+	},
+};
+
+module_platform_driver(dsi_driver);
+
+MODULE_AUTHOR("Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>");
+MODULE_DESCRIPTION("Xilinx MIPI DSI host controller driver");
+MODULE_LICENSE("GPL");
--
1.8.3.1

Re: [PATCH V2 2/2] drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver

[Randy Dunlap]

On 6/16/22 07:17, Venkateshwar Rao Gannavarapu wrote:
> diff --git a/drivers/gpu/drm/xlnx/Kconfig b/drivers/gpu/drm/xlnx/Kconfig
> index f9cf93c..a75bd76 100644
> --- a/drivers/gpu/drm/xlnx/Kconfig
> +++ b/drivers/gpu/drm/xlnx/Kconfig
> @@ -1,3 +1,15 @@
> +config DRM_XLNX_DSI
> +	tristate "Xilinx DRM DSI Subsystem Driver"
> +	depends on ARCH_ZYNQMP || COMPILE_TEST
> +	depends on DRM && OF
> +	select DRM_KMS_HELPER
> +	select DRM_MIPI_DSI
> +	select DRM_PANEL_BRIDGE
> +	help
> +	  DRM bridge driver for Xilinx programmable DSI subsystem controller.
> +	  choose this option if you hava a Xilinx MIPI-DSI Tx subsytem in

	  Choose

> +	  video pipeline.

-- 
~Randy

Re: [PATCH V2 2/2] drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver

[Randy Dunlap]

On 6/16/22 07:17, Venkateshwar Rao Gannavarapu wrote:
> diff --git a/drivers/gpu/drm/xlnx/Kconfig b/drivers/gpu/drm/xlnx/Kconfig
> index f9cf93c..a75bd76 100644
> --- a/drivers/gpu/drm/xlnx/Kconfig
> +++ b/drivers/gpu/drm/xlnx/Kconfig
> @@ -1,3 +1,15 @@
> +config DRM_XLNX_DSI
> +	tristate "Xilinx DRM DSI Subsystem Driver"
> +	depends on ARCH_ZYNQMP || COMPILE_TEST
> +	depends on DRM && OF
> +	select DRM_KMS_HELPER
> +	select DRM_MIPI_DSI
> +	select DRM_PANEL_BRIDGE
> +	help
> +	  DRM bridge driver for Xilinx programmable DSI subsystem controller.
> +	  choose this option if you hava a Xilinx MIPI-DSI Tx subsytem in

	  Choose

> +	  video pipeline.

-- 
~Randy

Re: [PATCH V2 1/2] dt-bindings: display: xlnx: Add DSI 2.0 Tx subsystem documentation

[Laurent Pinchart]

Hi GVRao,

Thank you for the patch.

On Thu, Jun 16, 2022 at 07:47:35PM +0530, Venkateshwar Rao Gannavarapu wrote:
> This patch adds dt binding for Xilinx DSI-TX subsystem.
> 
> The Xilinx MIPI DSI (Display serial interface) Transmitter Subsystem
> implements the Mobile Industry Processor Interface (MIPI) based display
> interface. It supports the interface with the programmable logic (FPGA).
> 
> Signed-off-by: Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>
> ---
>  .../bindings/display/xlnx/xlnx,dsi-tx.yaml         | 101 +++++++++++++++++++++
>  1 file changed, 101 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml b/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml
> new file mode 100644
> index 0000000..644934d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml
> @@ -0,0 +1,101 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/xlnx/xlnx,dsi-tx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Xilinx DSI Transmitter subsystem Device Tree Bindings
> +
> +maintainers:
> +  - Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>
> +
> +description: |
> +  The Xilinx DSI Transmitter Subsystem implements the Mobile Industry
> +  Processor Interface based display interface. It supports the interface
> +  with the programmable logic (FPGA).
> +
> +  For more details refer to PG238 Xilinx MIPI DSI-V2.0 Tx Subsystem.
> +
> +properties:
> +  compatible:
> +    const: xlnx,dsi-tx-v2.0
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: AXI Lite CPU clock
> +      - description: D-PHY clock
> +
> +  clock-names:
> +    items:
> +      - const: s_axis_aclk
> +      - const: dphy_clk_200M
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description:
> +          This port should be the input endpoint.

Port and endpoint are two different things. You can just write "Input
port of the DSI encoder".

> +
> +      port@1:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description:
> +          This port should be the output endpoint.

And here, "Output port of the DSI encoder".

> +
> +required:
> +  - "#address-cells"
> +  - "#size-cells"

I think those should be listed in the properties above, with fixed
values.

> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    dsi0: dsi_tx@80020000 {
> +        compatible = "xlnx,dsi-tx-v2.0";
> +        reg = <0x80020000 0x20000>;
> +        clocks = <&misc_clk_0>, <&misc_clk_1>;
> +        clock-names = "s_axis_aclk", "dphy_clk_200M";
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        ports {
> +              #address-cells = <1>;
> +              #size-cells = <0>;
> +
> +              port@0 {
> +                    reg = <0>;
> +                    mipi_dsi_in: endpoint {
> +                        remote-endpoint = <&pl_disp>;
> +                    };
> +              };
> +
> +              port@1 {
> +                    reg = <1>;
> +                    mipi_dsi_out: endpoint {
> +                        remote-endpoint = <&panel_in>;
> +                    };
> +              };
> +        };
> +
> +        panel@0 {
> +              compatible = "auo,b101uan01";
> +              reg = <0>;
> +              port {
> +                    panel_in: endpoint {
> +                        remote-endpoint = <&mipi_dsi_out>;
> +                    };
> +              };
> +        };

Does this example validate (with `make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml`)
without listing the panel node in the properties ?

> +    };
> +
> +...

-- 
Regards,

Laurent Pinchart

Re: [PATCH V2 1/2] dt-bindings: display: xlnx: Add DSI 2.0 Tx subsystem documentation

[Laurent Pinchart]

Hi GVRao,

Thank you for the patch.

On Thu, Jun 16, 2022 at 07:47:35PM +0530, Venkateshwar Rao Gannavarapu wrote:
> This patch adds dt binding for Xilinx DSI-TX subsystem.
> 
> The Xilinx MIPI DSI (Display serial interface) Transmitter Subsystem
> implements the Mobile Industry Processor Interface (MIPI) based display
> interface. It supports the interface with the programmable logic (FPGA).
> 
> Signed-off-by: Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>
> ---
>  .../bindings/display/xlnx/xlnx,dsi-tx.yaml         | 101 +++++++++++++++++++++
>  1 file changed, 101 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml b/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml
> new file mode 100644
> index 0000000..644934d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml
> @@ -0,0 +1,101 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/xlnx/xlnx,dsi-tx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Xilinx DSI Transmitter subsystem Device Tree Bindings
> +
> +maintainers:
> +  - Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>
> +
> +description: |
> +  The Xilinx DSI Transmitter Subsystem implements the Mobile Industry
> +  Processor Interface based display interface. It supports the interface
> +  with the programmable logic (FPGA).
> +
> +  For more details refer to PG238 Xilinx MIPI DSI-V2.0 Tx Subsystem.
> +
> +properties:
> +  compatible:
> +    const: xlnx,dsi-tx-v2.0
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: AXI Lite CPU clock
> +      - description: D-PHY clock
> +
> +  clock-names:
> +    items:
> +      - const: s_axis_aclk
> +      - const: dphy_clk_200M
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description:
> +          This port should be the input endpoint.

Port and endpoint are two different things. You can just write "Input
port of the DSI encoder".

> +
> +      port@1:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description:
> +          This port should be the output endpoint.

And here, "Output port of the DSI encoder".

> +
> +required:
> +  - "#address-cells"
> +  - "#size-cells"

I think those should be listed in the properties above, with fixed
values.

> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    dsi0: dsi_tx@80020000 {
> +        compatible = "xlnx,dsi-tx-v2.0";
> +        reg = <0x80020000 0x20000>;
> +        clocks = <&misc_clk_0>, <&misc_clk_1>;
> +        clock-names = "s_axis_aclk", "dphy_clk_200M";
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        ports {
> +              #address-cells = <1>;
> +              #size-cells = <0>;
> +
> +              port@0 {
> +                    reg = <0>;
> +                    mipi_dsi_in: endpoint {
> +                        remote-endpoint = <&pl_disp>;
> +                    };
> +              };
> +
> +              port@1 {
> +                    reg = <1>;
> +                    mipi_dsi_out: endpoint {
> +                        remote-endpoint = <&panel_in>;
> +                    };
> +              };
> +        };
> +
> +        panel@0 {
> +              compatible = "auo,b101uan01";
> +              reg = <0>;
> +              port {
> +                    panel_in: endpoint {
> +                        remote-endpoint = <&mipi_dsi_out>;
> +                    };
> +              };
> +        };

Does this example validate (with `make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml`)
without listing the panel node in the properties ?

> +    };
> +
> +...

-- 
Regards,

Laurent Pinchart

Re: [PATCH V2 2/2] drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver

[Laurent Pinchart]

Hi GVRao,

Thank you for the patch.

On Thu, Jun 16, 2022 at 07:47:36PM +0530, Venkateshwar Rao Gannavarapu wrote:
> The Xilinx MIPI DSI Tx Subsystem soft IP is used to display video
> data from AXI-4 stream interface.
> 
> It supports upto 4 lanes, optional register interface for the DPHY
> and multiple RGB color formats.
> This is a MIPI-DSI host driver and provides DSI bus for panels.
> This driver also helps to communicate with its panel using panel
> framework.
> 
> Signed-off-by: Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>
> ---
>  drivers/gpu/drm/xlnx/Kconfig    |  12 ++
>  drivers/gpu/drm/xlnx/Makefile   |   1 +
>  drivers/gpu/drm/xlnx/xlnx_dsi.c | 429 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 442 insertions(+)
>  create mode 100644 drivers/gpu/drm/xlnx/xlnx_dsi.c
> 
> diff --git a/drivers/gpu/drm/xlnx/Kconfig b/drivers/gpu/drm/xlnx/Kconfig
> index f9cf93c..a75bd76 100644
> --- a/drivers/gpu/drm/xlnx/Kconfig
> +++ b/drivers/gpu/drm/xlnx/Kconfig
> @@ -1,3 +1,15 @@
> +config DRM_XLNX_DSI
> +	tristate "Xilinx DRM DSI Subsystem Driver"
> +	depends on ARCH_ZYNQMP || COMPILE_TEST
> +	depends on DRM && OF
> +	select DRM_KMS_HELPER
> +	select DRM_MIPI_DSI
> +	select DRM_PANEL_BRIDGE

You can drop DRM_PANEL_BRIDGE, the driver doesn't need it.

> +	help
> +	  DRM bridge driver for Xilinx programmable DSI subsystem controller.
> +	  choose this option if you hava a Xilinx MIPI-DSI Tx subsytem in

s/choose/Choose/

> +	  video pipeline.
> +
>  config DRM_ZYNQMP_DPSUB
>  	tristate "ZynqMP DisplayPort Controller Driver"
>  	depends on ARCH_ZYNQMP || COMPILE_TEST
> diff --git a/drivers/gpu/drm/xlnx/Makefile b/drivers/gpu/drm/xlnx/Makefile
> index 51c24b7..f90849b 100644
> --- a/drivers/gpu/drm/xlnx/Makefile
> +++ b/drivers/gpu/drm/xlnx/Makefile
> @@ -1,2 +1,3 @@
> +obj-$(CONFIG_DRM_XLNX_DSI) += xlnx_dsi.o
>  zynqmp-dpsub-y := zynqmp_disp.o zynqmp_dpsub.o zynqmp_dp.o
>  obj-$(CONFIG_DRM_ZYNQMP_DPSUB) += zynqmp-dpsub.o
> diff --git a/drivers/gpu/drm/xlnx/xlnx_dsi.c b/drivers/gpu/drm/xlnx/xlnx_dsi.c
> new file mode 100644
> index 0000000..39d8947
> --- /dev/null
> +++ b/drivers/gpu/drm/xlnx/xlnx_dsi.c
> @@ -0,0 +1,429 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Xilinx FPGA MIPI DSI Tx Controller driver.
> + *
> + * Copyright (C) 2022 Xilinx, Inc.
> + *
> + * Author: Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>

I'd add at least platform_device.h to avoid depending on indirect
inclusion of headers.

> +
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_bridge.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_of.h>
> +#include <drm/drm_panel.h>

Not needed.

> +#include <drm/drm_print.h>
> +
> +/* DSI Tx IP registers */
> +#define XDSI_CCR			0x00
> +#define XDSI_CCR_COREENB		BIT(0)
> +#define XDSI_CCR_SOFTRST		BIT(1)
> +#define XDSI_CCR_CRREADY		BIT(2)
> +#define XDSI_CCR_CMDMODE		BIT(3)
> +#define XDSI_CCR_DFIFORST		BIT(4)
> +#define XDSI_CCR_CMDFIFORST		BIT(5)
> +#define XDSI_PCR			0x04
> +#define XDSI_PCR_LANES_MASK		3
> +#define XDSI_PCR_VIDEOMODE(x)		(((x) & 0x3) << 3)
> +#define XDSI_PCR_VIDEOMODE_MASK	GENMASK(4, 3)
> +#define XDSI_PCR_VIDEOMODE_SHIFT	3
> +#define XDSI_PCR_BLLPTYPE(x)		((x) << 5)
> +#define XDSI_PCR_BLLPMODE(x)		((x) << 6)
> +#define XDSI_PCR_PIXELFORMAT_MASK	GENMASK(12, 11)
> +#define XDSI_PCR_PIXELFORMAT_SHIFT	11
> +#define XDSI_PCR_EOTPENABLE(x)		((x) << 13)
> +#define XDSI_GIER			0x20
> +#define XDSI_ISR			0x24
> +#define XDSI_IER			0x28
> +#define XDSI_STR			0x2C
> +#define XDSI_STR_RDY_SHPKT		BIT(6)
> +#define XDSI_STR_RDY_LNGPKT		BIT(7)
> +#define XDSI_STR_DFIFO_FULL		BIT(8)
> +#define XDSI_STR_DFIFO_EMPTY		BIT(9)
> +#define XDSI_STR_WAITFR_DATA		BIT(10)
> +#define XDSI_STR_CMD_EXE_PGS		BIT(11)
> +#define XDSI_STR_CCMD_PROC		BIT(12)
> +#define XDSI_CMD			0x30
> +#define XDSI_CMD_QUEUE_PACKET(x)	((x) & GENMASK(23, 0))
> +#define XDSI_DFR			0x34
> +#define XDSI_TIME1			0x50
> +#define XDSI_TIME1_BLLP_BURST(x)	((x) & GENMASK(15, 0))
> +#define XDSI_TIME1_HSA(x)		(((x) & GENMASK(15, 0)) << 16)
> +#define XDSI_TIME2			0x54
> +#define XDSI_TIME2_VACT(x)		((x) & GENMASK(15, 0))
> +#define XDSI_TIME2_HACT(x)		(((x) & GENMASK(15, 0)) << 16)
> +#define XDSI_HACT_MULTIPLIER		GENMASK(1, 0)
> +#define XDSI_TIME3			0x58
> +#define XDSI_TIME3_HFP(x)		((x) & GENMASK(15, 0))
> +#define XDSI_TIME3_HBP(x)		(((x) & GENMASK(15, 0)) << 16)
> +#define XDSI_TIME4			0x5c
> +#define XDSI_TIME4_VFP(x)		((x) & GENMASK(7, 0))
> +#define XDSI_TIME4_VBP(x)		(((x) & GENMASK(7, 0)) << 8)
> +#define XDSI_TIME4_VSA(x)		(((x) & GENMASK(7, 0)) << 16)
> +#define XDSI_NUM_DATA_T		4
> +
> +/**
> + * struct xlnx_dsi - Xilinx DSI-TX core
> + * @bridge: DRM bridge structure
> + * @dsi_host: DSI host device
> + * @next_bridge: bridge structure
> + * @dev: device structure
> + * @clks: clock source structure
> + * @iomem: Base address of DSI subsystem
> + * @mode_flags: DSI operation mode related flags
> + * @lanes: number of active data lanes supported by DSI controller
> + * @mul_factor: multiplication factor for HACT timing
> + * @format: pixel format for video mode of DSI controller
> + * @device_found: Flag to indicate device presence
> + */
> +struct xlnx_dsi {
> +	struct drm_bridge bridge;
> +	struct mipi_dsi_host dsi_host;
> +	struct drm_bridge *next_bridge;
> +	struct device *dev;
> +	struct clk_bulk_data *clks;
> +	void __iomem *iomem;
> +	unsigned long mode_flags;
> +	u32 lanes;
> +	u32 mul_factor;
> +	enum mipi_dsi_pixel_format format;
> +	bool device_found;
> +};
> +
> +static const struct clk_bulk_data xdsi_clks[] = {
> +	{ .id = "s_axis_aclk" },
> +	{ .id = "dphy_clk_200M" },
> +};
> +
> +static inline struct xlnx_dsi *host_to_dsi(struct mipi_dsi_host *host)
> +{
> +	return container_of(host, struct xlnx_dsi, dsi_host);
> +}
> +
> +static inline struct xlnx_dsi *bridge_to_dsi(struct drm_bridge *bridge)
> +{
> +	return container_of(bridge, struct xlnx_dsi, bridge);
> +}
> +
> +static inline void xlnx_dsi_write(struct xlnx_dsi *dsi, int offset, u32 val)
> +{
> +	writel(val, dsi->iomem + offset);
> +}
> +
> +static inline u32 xlnx_dsi_read(struct xlnx_dsi *dsi, int offset)
> +{
> +	return readl(dsi->iomem + offset);
> +}
> +
> +static int xlnx_dsi_host_attach(struct mipi_dsi_host *host,
> +				struct mipi_dsi_device *device)
> +{
> +	struct xlnx_dsi *dsi = host_to_dsi(host);
> +	struct device *dev = host->dev;
> +
> +	dsi->mode_flags = device->mode_flags;
> +
> +	if (dsi->lanes != device->lanes) {
> +		dev_err(dsi->dev, "Mismatch of lanes. panel = %d, DSI = %d\n",

It's not necessarily a panel. You can write

		dev_err(dsi->dev, "Mismatch of lanes, host: %u != device: %u\n",
			dsi->lanes, device->lanes);

> +			device->lanes, dsi->lanes);
> +		return -EINVAL;
> +	}
> +
> +	if (dsi->format != device->format) {
> +		dev_err(dsi->dev, "Mismatch of format. panel = %d, DSI = %d\n",
> +			device->format, dsi->format);

Same here.

> +		return -EINVAL;
> +	}
> +
> +	if (!dsi->device_found) {
> +		dsi->next_bridge = devm_drm_of_get_bridge(dev,

As dev is the same as dsi->dev, I'd use dev->dsi here and drop the local
variable.

> +							  dev->of_node, 0, 0);
> +		if (IS_ERR(dsi->next_bridge))
> +			return PTR_ERR(dsi->next_bridge);
> +		drm_bridge_add(&dsi->bridge);
> +		dsi->device_found = true;

I don't think the device_found flag mechanism is right. You remove the
bridge in xlnx_dsi_host_detach(), so it should be added back here,
shouldn't it ?

The next question would then be what to do with the next bridge acquired
with devm_drm_of_get_bridge(). It looks like we have a lifetime
management issue here, and I don't think it's fair to ask you to fix the
bridge/panel core to get this driver merged, so I'm fine keeping this
for now even if it's incorrect. I'd like feedback from others though.

> +	}
> +
> +	return 0;
> +}
> +
> +static int xlnx_dsi_host_detach(struct mipi_dsi_host *host,
> +				struct mipi_dsi_device *device)
> +{
> +	struct xlnx_dsi *dsi = host_to_dsi(host);
> +
> +	drm_bridge_remove(&dsi->bridge);
> +	return 0;
> +}
> +
> +static const struct mipi_dsi_host_ops xlnx_dsi_ops = {
> +	.attach = xlnx_dsi_host_attach,
> +	.detach	= xlnx_dsi_host_detach,
> +};
> +
> +static void
> +xlnx_dsi_bridge_disable(struct drm_bridge *bridge,
> +			struct drm_bridge_state *old_bridge_state)
> +{
> +	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
> +	u32 reg = xlnx_dsi_read(dsi, XDSI_CCR);
> +
> +	reg &= ~XDSI_CCR_COREENB;
> +	xlnx_dsi_write(dsi, XDSI_CCR, reg);
> +}
> +
> +static void
> +xlnx_dsi_bridge_enable(struct drm_bridge *bridge,
> +		       struct drm_bridge_state *old_bridge_state)
> +{
> +	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
> +	struct drm_atomic_state *state = old_bridge_state->base.state;
> +	struct drm_connector *connector;
> +	struct drm_crtc *crtc;
> +	const struct drm_crtc_state *crtc_state;
> +	const struct drm_display_mode *mode;
> +	u32 reg, video_mode;
> +
> +	connector = drm_atomic_get_new_connector_for_encoder(state,
> +							     bridge->encoder);
> +	crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
> +	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
> +	mode = &crtc_state->adjusted_mode;
> +
> +	reg = xlnx_dsi_read(dsi, XDSI_PCR);
> +	video_mode = (reg & XDSI_PCR_VIDEOMODE_MASK) >> XDSI_PCR_VIDEOMODE_SHIFT;
> +
> +	if (!video_mode && (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)) {
> +		reg = XDSI_TIME1_HSA(mode->hsync_end -
> +				     mode->hsync_start);

This holds on a single line.

> +		xlnx_dsi_write(dsi, XDSI_TIME1, reg);
> +	}
> +
> +	reg = XDSI_TIME4_VFP(mode->vsync_start - mode->vdisplay) |
> +		XDSI_TIME4_VBP(mode->vtotal - mode->vsync_end) |
> +		XDSI_TIME4_VSA(mode->vsync_end - mode->vsync_start);
> +	xlnx_dsi_write(dsi, XDSI_TIME4, reg);
> +
> +	reg = XDSI_TIME3_HFP(mode->hsync_start - mode->hdisplay) |
> +		XDSI_TIME3_HBP(mode->htotal - mode->hsync_end);
> +	xlnx_dsi_write(dsi, XDSI_TIME3, reg);
> +
> +	reg = XDSI_TIME2_HACT(mode->hdisplay * dsi->mul_factor / 100) |
> +		XDSI_TIME2_VACT(mode->vdisplay);
> +	xlnx_dsi_write(dsi->iomem, XDSI_TIME2, reg);
> +
> +	xlnx_dsi_write(dsi, XDSI_PCR, XDSI_PCR_VIDEOMODE(BIT(0)));
> +
> +	/* Enable Core */
> +	reg = xlnx_dsi_read(dsi, XDSI_CCR);
> +	reg |= XDSI_CCR_COREENB;
> +	xlnx_dsi_write(dsi, XDSI_CCR, reg);
> +}
> +
> +#define MAX_INPUT_SEL_FORMATS   3
> +static u32
> +*xlnx_dsi_bridge_atomic_get_input_bus_fmts(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)
> +{
> +	u32 *input_fmts;
> +	unsigned int i = 0;
> +
> +	*num_input_fmts = 0;
> +	input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts), GFP_KERNEL);
> +	if (!input_fmts)
> +		return NULL;
> +
> +	switch (output_fmt) {
> +	case MEDIA_BUS_FMT_FIXED:
> +		input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
> +		break;
> +	case MEDIA_BUS_FMT_RGB666_1X18:
> +		input_fmts[i++] = MEDIA_BUS_FMT_RGB666_1X18;
> +		break;
> +	case MEDIA_BUS_FMT_RGB565_1X16:
> +		input_fmts[i++] = MEDIA_BUS_FMT_RGB565_1X16;
> +		break;
> +	default: /* define */
> +	}

As the cases are mutually exclusive, i will always be equal to 1. You
can drop the MAX_INPUT_SEL_FORMATS macro, the i variable, allocate a
single entry for input_fmts, and set inputs_fmts[0].

> +
> +	*num_input_fmts = i;
> +	if (*num_input_fmts == 0) {
> +		kfree(input_fmts);
> +		input_fmts = NULL;
> +	}
> +
> +	return input_fmts;
> +}
> +
> +static int xlnx_dsi_bridge_attach(struct drm_bridge *bridge,
> +				  enum drm_bridge_attach_flags flags)
> +{
> +	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
> +
> +	if (!dsi->next_bridge)

Can this ever happen ?

> +		return 0;
> +
> +	/* Attach the next bridge */
> +	return drm_bridge_attach(bridge->encoder, dsi->next_bridge, bridge,
> +				 flags);
> +}
> +
> +static void xlnx_dsi_bridge_detach(struct drm_bridge *bridge)
> +{
> +	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
> +
> +	drm_of_panel_bridge_remove(dsi->dev->of_node, 1, 0);

This doesn't look right. Is it a leftover ? You should instead call
drm_bridge_detach() on the next bridge.

> +}
> +
> +static enum drm_mode_status
> +xlnx_dsi_bridge_mode_valid(struct drm_bridge *bridge,
> +			   const struct drm_display_info *info,
> +			   const struct drm_display_mode *mode)
> +{
> +	if ((mode->hdisplay & XDSI_HACT_MULTIPLIER) != 0)
> +		return MODE_BAD_WIDTH;
> +
> +	return MODE_OK;
> +}
> +
> +static const struct drm_bridge_funcs xlnx_dsi_bridge_funcs = {
> +	.atomic_duplicate_state		= drm_atomic_helper_bridge_duplicate_state,
> +	.atomic_destroy_state		= drm_atomic_helper_bridge_destroy_state,
> +	.atomic_reset			= drm_atomic_helper_bridge_reset,
> +	.atomic_disable                 = xlnx_dsi_bridge_disable,
> +	.atomic_enable			= xlnx_dsi_bridge_enable,
> +	.atomic_get_input_bus_fmts      = xlnx_dsi_bridge_atomic_get_input_bus_fmts,
> +	.attach				= xlnx_dsi_bridge_attach,
> +	.detach				= xlnx_dsi_bridge_detach,
> +	.mode_valid			= xlnx_dsi_bridge_mode_valid,
> +};
> +
> +static int xlnx_dsi_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct resource *res;
> +	struct xlnx_dsi *dsi;
> +	int ret;
> +	const int xdsi_mul_fact[XDSI_NUM_DATA_T] = {300, 225, 225, 200};

static const, and you can move it as the first variable of the function.

> +	u32 reg;
> +
> +	dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
> +	if (!dsi)
> +		return -ENOMEM;
> +
> +	dsi->dev = dev;
> +	dsi->clks = devm_kmemdup(dev, xdsi_clks, sizeof(xdsi_clks),
> +				 GFP_KERNEL);
> +	if (!dsi->clks)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	dsi->iomem = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(dsi->iomem))
> +		return PTR_ERR(dsi->iomem);
> +
> +	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(xdsi_clks), dsi->clks);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_bulk_prepare_enable(ARRAY_SIZE(xdsi_clks), dsi->clks);
> +	if (ret)
> +		return ret;

Shouldn't this be done in the .atomic_enable() handler instead, possible
through runtime PM ?

> +
> +	platform_set_drvdata(pdev, dsi);
> +	dsi->dsi_host.ops = &xlnx_dsi_ops;
> +	dsi->dsi_host.dev = dev;
> +
> +	ret = mipi_dsi_host_register(&dsi->dsi_host);
> +	if (ret) {
> +		dev_err(dev, "Failed to register MIPI host: %d\n", ret);
> +		goto err_clk_put;
> +	}
> +
> +	dsi->bridge.driver_private = dsi;
> +	dsi->bridge.funcs = &xlnx_dsi_bridge_funcs;
> +	dsi->bridge.of_node = pdev->dev.of_node;
> +
> +	reg = xlnx_dsi_read(dsi, XDSI_PCR);
> +	dsi->lanes = reg & XDSI_PCR_LANES_MASK;
> +	dsi->format = (reg & XDSI_PCR_PIXELFORMAT_MASK) >>
> +		XDSI_PCR_PIXELFORMAT_SHIFT;

OK, you'll need to enable clocks for this too, so runtime PM could be a
good option.

> +
> +	if (dsi->lanes > 4 || dsi->lanes < 1) {
> +		dev_err(dsi->dev, "%d invalid lanes\n",	dsi->lanes);

dsi->lanes is unsigned, so %u.

> +		return -EINVAL;

This leaves the clocks enabled.

> +	}
> +
> +	if (dsi->format > MIPI_DSI_FMT_RGB565) {
> +		dev_err(dsi->dev, "Invalid xlnx,dsi-data-type string\n");

This is read from a register, not parsed from DT, so the message isn't
valid.

> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * Used as a multiplication factor for HACT based on used
> +	 * DSI data type.
> +	 *
> +	 * e.g. for RGB666_L datatype and 1920x1080 resolution,
> +	 * the Hact (WC) would be as follows -
> +	 * 1920 pixels * 18 bits per pixel / 8 bits per byte
> +	 * = 1920 pixels * 2.25 bytes per pixel = 4320 bytes.
> +	 *
> +	 * Data Type - Multiplication factor
> +	 * RGB888    - 3
> +	 * RGB666_L  - 2.25
> +-	 * RGB666_P  - 2.25
> +	 * RGB565    - 2
> +	 *
> +	 * Since the multiplication factor is a floating number,
> +	 * a 100x multiplication factor is used.
> +	 */
> +	dsi->mul_factor = xdsi_mul_fact[dsi->format];
> +
> +	dev_dbg(dsi->dev, "DSI controller num lanes = %d\n", dsi->lanes);
> +	dev_dbg(dsi->dev, "DSI controller format = %d\n", dsi->format);

You could combine both messages into a single one.

Are you missing a return 0 here ?

> +
> +err_clk_put:
> +	clk_bulk_disable_unprepare(ARRAY_SIZE(xdsi_clks), dsi->clks);
> +
> +	return ret;
> +}
> +
> +static int xlnx_dsi_remove(struct platform_device *pdev)
> +{
> +	struct xlnx_dsi *dsi = platform_get_drvdata(pdev);
> +
> +	mipi_dsi_host_unregister(&dsi->dsi_host);
> +	clk_bulk_disable_unprepare(ARRAY_SIZE(xdsi_clks), dsi->clks);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id xlnx_dsi_of_match[] = {
> +	{ .compatible = "xlnx,dsi-tx-v2.0"},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, xlnx_dsi_of_match);
> +
> +static struct platform_driver dsi_driver = {
> +	.probe = xlnx_dsi_probe,
> +	.remove = xlnx_dsi_remove,
> +	.driver = {
> +		.name = "xlnx-dsi",
> +		.of_match_table = xlnx_dsi_of_match,
> +	},
> +};
> +
> +module_platform_driver(dsi_driver);
> +
> +MODULE_AUTHOR("Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>");
> +MODULE_DESCRIPTION("Xilinx MIPI DSI host controller driver");
> +MODULE_LICENSE("GPL");

-- 
Regards,

Laurent Pinchart

Re: [PATCH V2 2/2] drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver

[Laurent Pinchart]

Hi GVRao,

Thank you for the patch.

On Thu, Jun 16, 2022 at 07:47:36PM +0530, Venkateshwar Rao Gannavarapu wrote:
> The Xilinx MIPI DSI Tx Subsystem soft IP is used to display video
> data from AXI-4 stream interface.
> 
> It supports upto 4 lanes, optional register interface for the DPHY
> and multiple RGB color formats.
> This is a MIPI-DSI host driver and provides DSI bus for panels.
> This driver also helps to communicate with its panel using panel
> framework.
> 
> Signed-off-by: Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>
> ---
>  drivers/gpu/drm/xlnx/Kconfig    |  12 ++
>  drivers/gpu/drm/xlnx/Makefile   |   1 +
>  drivers/gpu/drm/xlnx/xlnx_dsi.c | 429 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 442 insertions(+)
>  create mode 100644 drivers/gpu/drm/xlnx/xlnx_dsi.c
> 
> diff --git a/drivers/gpu/drm/xlnx/Kconfig b/drivers/gpu/drm/xlnx/Kconfig
> index f9cf93c..a75bd76 100644
> --- a/drivers/gpu/drm/xlnx/Kconfig
> +++ b/drivers/gpu/drm/xlnx/Kconfig
> @@ -1,3 +1,15 @@
> +config DRM_XLNX_DSI
> +	tristate "Xilinx DRM DSI Subsystem Driver"
> +	depends on ARCH_ZYNQMP || COMPILE_TEST
> +	depends on DRM && OF
> +	select DRM_KMS_HELPER
> +	select DRM_MIPI_DSI
> +	select DRM_PANEL_BRIDGE

You can drop DRM_PANEL_BRIDGE, the driver doesn't need it.

> +	help
> +	  DRM bridge driver for Xilinx programmable DSI subsystem controller.
> +	  choose this option if you hava a Xilinx MIPI-DSI Tx subsytem in

s/choose/Choose/

> +	  video pipeline.
> +
>  config DRM_ZYNQMP_DPSUB
>  	tristate "ZynqMP DisplayPort Controller Driver"
>  	depends on ARCH_ZYNQMP || COMPILE_TEST
> diff --git a/drivers/gpu/drm/xlnx/Makefile b/drivers/gpu/drm/xlnx/Makefile
> index 51c24b7..f90849b 100644
> --- a/drivers/gpu/drm/xlnx/Makefile
> +++ b/drivers/gpu/drm/xlnx/Makefile
> @@ -1,2 +1,3 @@
> +obj-$(CONFIG_DRM_XLNX_DSI) += xlnx_dsi.o
>  zynqmp-dpsub-y := zynqmp_disp.o zynqmp_dpsub.o zynqmp_dp.o
>  obj-$(CONFIG_DRM_ZYNQMP_DPSUB) += zynqmp-dpsub.o
> diff --git a/drivers/gpu/drm/xlnx/xlnx_dsi.c b/drivers/gpu/drm/xlnx/xlnx_dsi.c
> new file mode 100644
> index 0000000..39d8947
> --- /dev/null
> +++ b/drivers/gpu/drm/xlnx/xlnx_dsi.c
> @@ -0,0 +1,429 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Xilinx FPGA MIPI DSI Tx Controller driver.
> + *
> + * Copyright (C) 2022 Xilinx, Inc.
> + *
> + * Author: Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>

I'd add at least platform_device.h to avoid depending on indirect
inclusion of headers.

> +
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_bridge.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_of.h>
> +#include <drm/drm_panel.h>

Not needed.

> +#include <drm/drm_print.h>
> +
> +/* DSI Tx IP registers */
> +#define XDSI_CCR			0x00
> +#define XDSI_CCR_COREENB		BIT(0)
> +#define XDSI_CCR_SOFTRST		BIT(1)
> +#define XDSI_CCR_CRREADY		BIT(2)
> +#define XDSI_CCR_CMDMODE		BIT(3)
> +#define XDSI_CCR_DFIFORST		BIT(4)
> +#define XDSI_CCR_CMDFIFORST		BIT(5)
> +#define XDSI_PCR			0x04
> +#define XDSI_PCR_LANES_MASK		3
> +#define XDSI_PCR_VIDEOMODE(x)		(((x) & 0x3) << 3)
> +#define XDSI_PCR_VIDEOMODE_MASK	GENMASK(4, 3)
> +#define XDSI_PCR_VIDEOMODE_SHIFT	3
> +#define XDSI_PCR_BLLPTYPE(x)		((x) << 5)
> +#define XDSI_PCR_BLLPMODE(x)		((x) << 6)
> +#define XDSI_PCR_PIXELFORMAT_MASK	GENMASK(12, 11)
> +#define XDSI_PCR_PIXELFORMAT_SHIFT	11
> +#define XDSI_PCR_EOTPENABLE(x)		((x) << 13)
> +#define XDSI_GIER			0x20
> +#define XDSI_ISR			0x24
> +#define XDSI_IER			0x28
> +#define XDSI_STR			0x2C
> +#define XDSI_STR_RDY_SHPKT		BIT(6)
> +#define XDSI_STR_RDY_LNGPKT		BIT(7)
> +#define XDSI_STR_DFIFO_FULL		BIT(8)
> +#define XDSI_STR_DFIFO_EMPTY		BIT(9)
> +#define XDSI_STR_WAITFR_DATA		BIT(10)
> +#define XDSI_STR_CMD_EXE_PGS		BIT(11)
> +#define XDSI_STR_CCMD_PROC		BIT(12)
> +#define XDSI_CMD			0x30
> +#define XDSI_CMD_QUEUE_PACKET(x)	((x) & GENMASK(23, 0))
> +#define XDSI_DFR			0x34
> +#define XDSI_TIME1			0x50
> +#define XDSI_TIME1_BLLP_BURST(x)	((x) & GENMASK(15, 0))
> +#define XDSI_TIME1_HSA(x)		(((x) & GENMASK(15, 0)) << 16)
> +#define XDSI_TIME2			0x54
> +#define XDSI_TIME2_VACT(x)		((x) & GENMASK(15, 0))
> +#define XDSI_TIME2_HACT(x)		(((x) & GENMASK(15, 0)) << 16)
> +#define XDSI_HACT_MULTIPLIER		GENMASK(1, 0)
> +#define XDSI_TIME3			0x58
> +#define XDSI_TIME3_HFP(x)		((x) & GENMASK(15, 0))
> +#define XDSI_TIME3_HBP(x)		(((x) & GENMASK(15, 0)) << 16)
> +#define XDSI_TIME4			0x5c
> +#define XDSI_TIME4_VFP(x)		((x) & GENMASK(7, 0))
> +#define XDSI_TIME4_VBP(x)		(((x) & GENMASK(7, 0)) << 8)
> +#define XDSI_TIME4_VSA(x)		(((x) & GENMASK(7, 0)) << 16)
> +#define XDSI_NUM_DATA_T		4
> +
> +/**
> + * struct xlnx_dsi - Xilinx DSI-TX core
> + * @bridge: DRM bridge structure
> + * @dsi_host: DSI host device
> + * @next_bridge: bridge structure
> + * @dev: device structure
> + * @clks: clock source structure
> + * @iomem: Base address of DSI subsystem
> + * @mode_flags: DSI operation mode related flags
> + * @lanes: number of active data lanes supported by DSI controller
> + * @mul_factor: multiplication factor for HACT timing
> + * @format: pixel format for video mode of DSI controller
> + * @device_found: Flag to indicate device presence
> + */
> +struct xlnx_dsi {
> +	struct drm_bridge bridge;
> +	struct mipi_dsi_host dsi_host;
> +	struct drm_bridge *next_bridge;
> +	struct device *dev;
> +	struct clk_bulk_data *clks;
> +	void __iomem *iomem;
> +	unsigned long mode_flags;
> +	u32 lanes;
> +	u32 mul_factor;
> +	enum mipi_dsi_pixel_format format;
> +	bool device_found;
> +};
> +
> +static const struct clk_bulk_data xdsi_clks[] = {
> +	{ .id = "s_axis_aclk" },
> +	{ .id = "dphy_clk_200M" },
> +};
> +
> +static inline struct xlnx_dsi *host_to_dsi(struct mipi_dsi_host *host)
> +{
> +	return container_of(host, struct xlnx_dsi, dsi_host);
> +}
> +
> +static inline struct xlnx_dsi *bridge_to_dsi(struct drm_bridge *bridge)
> +{
> +	return container_of(bridge, struct xlnx_dsi, bridge);
> +}
> +
> +static inline void xlnx_dsi_write(struct xlnx_dsi *dsi, int offset, u32 val)
> +{
> +	writel(val, dsi->iomem + offset);
> +}
> +
> +static inline u32 xlnx_dsi_read(struct xlnx_dsi *dsi, int offset)
> +{
> +	return readl(dsi->iomem + offset);
> +}
> +
> +static int xlnx_dsi_host_attach(struct mipi_dsi_host *host,
> +				struct mipi_dsi_device *device)
> +{
> +	struct xlnx_dsi *dsi = host_to_dsi(host);
> +	struct device *dev = host->dev;
> +
> +	dsi->mode_flags = device->mode_flags;
> +
> +	if (dsi->lanes != device->lanes) {
> +		dev_err(dsi->dev, "Mismatch of lanes. panel = %d, DSI = %d\n",

It's not necessarily a panel. You can write

		dev_err(dsi->dev, "Mismatch of lanes, host: %u != device: %u\n",
			dsi->lanes, device->lanes);

> +			device->lanes, dsi->lanes);
> +		return -EINVAL;
> +	}
> +
> +	if (dsi->format != device->format) {
> +		dev_err(dsi->dev, "Mismatch of format. panel = %d, DSI = %d\n",
> +			device->format, dsi->format);

Same here.

> +		return -EINVAL;
> +	}
> +
> +	if (!dsi->device_found) {
> +		dsi->next_bridge = devm_drm_of_get_bridge(dev,

As dev is the same as dsi->dev, I'd use dev->dsi here and drop the local
variable.

> +							  dev->of_node, 0, 0);
> +		if (IS_ERR(dsi->next_bridge))
> +			return PTR_ERR(dsi->next_bridge);
> +		drm_bridge_add(&dsi->bridge);
> +		dsi->device_found = true;

I don't think the device_found flag mechanism is right. You remove the
bridge in xlnx_dsi_host_detach(), so it should be added back here,
shouldn't it ?

The next question would then be what to do with the next bridge acquired
with devm_drm_of_get_bridge(). It looks like we have a lifetime
management issue here, and I don't think it's fair to ask you to fix the
bridge/panel core to get this driver merged, so I'm fine keeping this
for now even if it's incorrect. I'd like feedback from others though.

> +	}
> +
> +	return 0;
> +}
> +
> +static int xlnx_dsi_host_detach(struct mipi_dsi_host *host,
> +				struct mipi_dsi_device *device)
> +{
> +	struct xlnx_dsi *dsi = host_to_dsi(host);
> +
> +	drm_bridge_remove(&dsi->bridge);
> +	return 0;
> +}
> +
> +static const struct mipi_dsi_host_ops xlnx_dsi_ops = {
> +	.attach = xlnx_dsi_host_attach,
> +	.detach	= xlnx_dsi_host_detach,
> +};
> +
> +static void
> +xlnx_dsi_bridge_disable(struct drm_bridge *bridge,
> +			struct drm_bridge_state *old_bridge_state)
> +{
> +	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
> +	u32 reg = xlnx_dsi_read(dsi, XDSI_CCR);
> +
> +	reg &= ~XDSI_CCR_COREENB;
> +	xlnx_dsi_write(dsi, XDSI_CCR, reg);
> +}
> +
> +static void
> +xlnx_dsi_bridge_enable(struct drm_bridge *bridge,
> +		       struct drm_bridge_state *old_bridge_state)
> +{
> +	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
> +	struct drm_atomic_state *state = old_bridge_state->base.state;
> +	struct drm_connector *connector;
> +	struct drm_crtc *crtc;
> +	const struct drm_crtc_state *crtc_state;
> +	const struct drm_display_mode *mode;
> +	u32 reg, video_mode;
> +
> +	connector = drm_atomic_get_new_connector_for_encoder(state,
> +							     bridge->encoder);
> +	crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
> +	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
> +	mode = &crtc_state->adjusted_mode;
> +
> +	reg = xlnx_dsi_read(dsi, XDSI_PCR);
> +	video_mode = (reg & XDSI_PCR_VIDEOMODE_MASK) >> XDSI_PCR_VIDEOMODE_SHIFT;
> +
> +	if (!video_mode && (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)) {
> +		reg = XDSI_TIME1_HSA(mode->hsync_end -
> +				     mode->hsync_start);

This holds on a single line.

> +		xlnx_dsi_write(dsi, XDSI_TIME1, reg);
> +	}
> +
> +	reg = XDSI_TIME4_VFP(mode->vsync_start - mode->vdisplay) |
> +		XDSI_TIME4_VBP(mode->vtotal - mode->vsync_end) |
> +		XDSI_TIME4_VSA(mode->vsync_end - mode->vsync_start);
> +	xlnx_dsi_write(dsi, XDSI_TIME4, reg);
> +
> +	reg = XDSI_TIME3_HFP(mode->hsync_start - mode->hdisplay) |
> +		XDSI_TIME3_HBP(mode->htotal - mode->hsync_end);
> +	xlnx_dsi_write(dsi, XDSI_TIME3, reg);
> +
> +	reg = XDSI_TIME2_HACT(mode->hdisplay * dsi->mul_factor / 100) |
> +		XDSI_TIME2_VACT(mode->vdisplay);
> +	xlnx_dsi_write(dsi->iomem, XDSI_TIME2, reg);
> +
> +	xlnx_dsi_write(dsi, XDSI_PCR, XDSI_PCR_VIDEOMODE(BIT(0)));
> +
> +	/* Enable Core */
> +	reg = xlnx_dsi_read(dsi, XDSI_CCR);
> +	reg |= XDSI_CCR_COREENB;
> +	xlnx_dsi_write(dsi, XDSI_CCR, reg);
> +}
> +
> +#define MAX_INPUT_SEL_FORMATS   3
> +static u32
> +*xlnx_dsi_bridge_atomic_get_input_bus_fmts(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)
> +{
> +	u32 *input_fmts;
> +	unsigned int i = 0;
> +
> +	*num_input_fmts = 0;
> +	input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts), GFP_KERNEL);
> +	if (!input_fmts)
> +		return NULL;
> +
> +	switch (output_fmt) {
> +	case MEDIA_BUS_FMT_FIXED:
> +		input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
> +		break;
> +	case MEDIA_BUS_FMT_RGB666_1X18:
> +		input_fmts[i++] = MEDIA_BUS_FMT_RGB666_1X18;
> +		break;
> +	case MEDIA_BUS_FMT_RGB565_1X16:
> +		input_fmts[i++] = MEDIA_BUS_FMT_RGB565_1X16;
> +		break;
> +	default: /* define */
> +	}

As the cases are mutually exclusive, i will always be equal to 1. You
can drop the MAX_INPUT_SEL_FORMATS macro, the i variable, allocate a
single entry for input_fmts, and set inputs_fmts[0].

> +
> +	*num_input_fmts = i;
> +	if (*num_input_fmts == 0) {
> +		kfree(input_fmts);
> +		input_fmts = NULL;
> +	}
> +
> +	return input_fmts;
> +}
> +
> +static int xlnx_dsi_bridge_attach(struct drm_bridge *bridge,
> +				  enum drm_bridge_attach_flags flags)
> +{
> +	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
> +
> +	if (!dsi->next_bridge)

Can this ever happen ?

> +		return 0;
> +
> +	/* Attach the next bridge */
> +	return drm_bridge_attach(bridge->encoder, dsi->next_bridge, bridge,
> +				 flags);
> +}
> +
> +static void xlnx_dsi_bridge_detach(struct drm_bridge *bridge)
> +{
> +	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
> +
> +	drm_of_panel_bridge_remove(dsi->dev->of_node, 1, 0);

This doesn't look right. Is it a leftover ? You should instead call
drm_bridge_detach() on the next bridge.

> +}
> +
> +static enum drm_mode_status
> +xlnx_dsi_bridge_mode_valid(struct drm_bridge *bridge,
> +			   const struct drm_display_info *info,
> +			   const struct drm_display_mode *mode)
> +{
> +	if ((mode->hdisplay & XDSI_HACT_MULTIPLIER) != 0)
> +		return MODE_BAD_WIDTH;
> +
> +	return MODE_OK;
> +}
> +
> +static const struct drm_bridge_funcs xlnx_dsi_bridge_funcs = {
> +	.atomic_duplicate_state		= drm_atomic_helper_bridge_duplicate_state,
> +	.atomic_destroy_state		= drm_atomic_helper_bridge_destroy_state,
> +	.atomic_reset			= drm_atomic_helper_bridge_reset,
> +	.atomic_disable                 = xlnx_dsi_bridge_disable,
> +	.atomic_enable			= xlnx_dsi_bridge_enable,
> +	.atomic_get_input_bus_fmts      = xlnx_dsi_bridge_atomic_get_input_bus_fmts,
> +	.attach				= xlnx_dsi_bridge_attach,
> +	.detach				= xlnx_dsi_bridge_detach,
> +	.mode_valid			= xlnx_dsi_bridge_mode_valid,
> +};
> +
> +static int xlnx_dsi_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct resource *res;
> +	struct xlnx_dsi *dsi;
> +	int ret;
> +	const int xdsi_mul_fact[XDSI_NUM_DATA_T] = {300, 225, 225, 200};

static const, and you can move it as the first variable of the function.

> +	u32 reg;
> +
> +	dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
> +	if (!dsi)
> +		return -ENOMEM;
> +
> +	dsi->dev = dev;
> +	dsi->clks = devm_kmemdup(dev, xdsi_clks, sizeof(xdsi_clks),
> +				 GFP_KERNEL);
> +	if (!dsi->clks)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	dsi->iomem = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(dsi->iomem))
> +		return PTR_ERR(dsi->iomem);
> +
> +	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(xdsi_clks), dsi->clks);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_bulk_prepare_enable(ARRAY_SIZE(xdsi_clks), dsi->clks);
> +	if (ret)
> +		return ret;

Shouldn't this be done in the .atomic_enable() handler instead, possible
through runtime PM ?

> +
> +	platform_set_drvdata(pdev, dsi);
> +	dsi->dsi_host.ops = &xlnx_dsi_ops;
> +	dsi->dsi_host.dev = dev;
> +
> +	ret = mipi_dsi_host_register(&dsi->dsi_host);
> +	if (ret) {
> +		dev_err(dev, "Failed to register MIPI host: %d\n", ret);
> +		goto err_clk_put;
> +	}
> +
> +	dsi->bridge.driver_private = dsi;
> +	dsi->bridge.funcs = &xlnx_dsi_bridge_funcs;
> +	dsi->bridge.of_node = pdev->dev.of_node;
> +
> +	reg = xlnx_dsi_read(dsi, XDSI_PCR);
> +	dsi->lanes = reg & XDSI_PCR_LANES_MASK;
> +	dsi->format = (reg & XDSI_PCR_PIXELFORMAT_MASK) >>
> +		XDSI_PCR_PIXELFORMAT_SHIFT;

OK, you'll need to enable clocks for this too, so runtime PM could be a
good option.

> +
> +	if (dsi->lanes > 4 || dsi->lanes < 1) {
> +		dev_err(dsi->dev, "%d invalid lanes\n",	dsi->lanes);

dsi->lanes is unsigned, so %u.

> +		return -EINVAL;

This leaves the clocks enabled.

> +	}
> +
> +	if (dsi->format > MIPI_DSI_FMT_RGB565) {
> +		dev_err(dsi->dev, "Invalid xlnx,dsi-data-type string\n");

This is read from a register, not parsed from DT, so the message isn't
valid.

> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * Used as a multiplication factor for HACT based on used
> +	 * DSI data type.
> +	 *
> +	 * e.g. for RGB666_L datatype and 1920x1080 resolution,
> +	 * the Hact (WC) would be as follows -
> +	 * 1920 pixels * 18 bits per pixel / 8 bits per byte
> +	 * = 1920 pixels * 2.25 bytes per pixel = 4320 bytes.
> +	 *
> +	 * Data Type - Multiplication factor
> +	 * RGB888    - 3
> +	 * RGB666_L  - 2.25
> +-	 * RGB666_P  - 2.25
> +	 * RGB565    - 2
> +	 *
> +	 * Since the multiplication factor is a floating number,
> +	 * a 100x multiplication factor is used.
> +	 */
> +	dsi->mul_factor = xdsi_mul_fact[dsi->format];
> +
> +	dev_dbg(dsi->dev, "DSI controller num lanes = %d\n", dsi->lanes);
> +	dev_dbg(dsi->dev, "DSI controller format = %d\n", dsi->format);

You could combine both messages into a single one.

Are you missing a return 0 here ?

> +
> +err_clk_put:
> +	clk_bulk_disable_unprepare(ARRAY_SIZE(xdsi_clks), dsi->clks);
> +
> +	return ret;
> +}
> +
> +static int xlnx_dsi_remove(struct platform_device *pdev)
> +{
> +	struct xlnx_dsi *dsi = platform_get_drvdata(pdev);
> +
> +	mipi_dsi_host_unregister(&dsi->dsi_host);
> +	clk_bulk_disable_unprepare(ARRAY_SIZE(xdsi_clks), dsi->clks);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id xlnx_dsi_of_match[] = {
> +	{ .compatible = "xlnx,dsi-tx-v2.0"},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, xlnx_dsi_of_match);
> +
> +static struct platform_driver dsi_driver = {
> +	.probe = xlnx_dsi_probe,
> +	.remove = xlnx_dsi_remove,
> +	.driver = {
> +		.name = "xlnx-dsi",
> +		.of_match_table = xlnx_dsi_of_match,
> +	},
> +};
> +
> +module_platform_driver(dsi_driver);
> +
> +MODULE_AUTHOR("Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>");
> +MODULE_DESCRIPTION("Xilinx MIPI DSI host controller driver");
> +MODULE_LICENSE("GPL");

-- 
Regards,

Laurent Pinchart

Re: [PATCH V2 2/2] drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver

[kernel test robot]

Hi Venkateshwar,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm/drm-next]
[also build test ERROR on linus/master v5.19-rc3 next-20220624]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Venkateshwar-Rao-Gannavarapu/Add-Xilinx-DSI-Tx-subsystem-DRM-driver/20220616-222008
base:   git://anongit.freedesktop.org/drm/drm drm-next
config: mips-randconfig-c004-20220626
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project b0d6dd3905db145853c7c744ac92d49b00b1fa20)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install mips cross compiling tool for clang build
        # apt-get install binutils-mips-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/28aa62ffdc1901029bf75961166f4ebba948b9b7
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Venkateshwar-Rao-Gannavarapu/Add-Xilinx-DSI-Tx-subsystem-DRM-driver/20220616-222008
        git checkout 28aa62ffdc1901029bf75961166f4ebba948b9b7
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/gpu/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/xlnx/xlnx_dsi.c:255:10: error: label at end of compound statement: expected statement
           default: /* define */
                   ^
                    ;
   1 error generated.


vim +255 drivers/gpu/drm/xlnx/xlnx_dsi.c

   227	
   228	#define MAX_INPUT_SEL_FORMATS   3
   229	static u32
   230	*xlnx_dsi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
   231						   struct drm_bridge_state *bridge_state,
   232						   struct drm_crtc_state *crtc_state,
   233						   struct drm_connector_state *conn_state,
   234						   u32 output_fmt,
   235						   unsigned int *num_input_fmts)
   236	{
   237		u32 *input_fmts;
   238		unsigned int i = 0;
   239	
   240		*num_input_fmts = 0;
   241		input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts), GFP_KERNEL);
   242		if (!input_fmts)
   243			return NULL;
   244	
   245		switch (output_fmt) {
   246		case MEDIA_BUS_FMT_FIXED:
   247			input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
   248			break;
   249		case MEDIA_BUS_FMT_RGB666_1X18:
   250			input_fmts[i++] = MEDIA_BUS_FMT_RGB666_1X18;
   251			break;
   252		case MEDIA_BUS_FMT_RGB565_1X16:
   253			input_fmts[i++] = MEDIA_BUS_FMT_RGB565_1X16;
   254			break;
 > 255		default: /* define */
   256		}
   257	
   258		*num_input_fmts = i;
   259		if (*num_input_fmts == 0) {
   260			kfree(input_fmts);
   261			input_fmts = NULL;
   262		}
   263	
   264		return input_fmts;
   265	}
   266	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH V2 2/2] drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver

[kernel test robot]

Hi Venkateshwar,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm/drm-next]
[also build test ERROR on linus/master v5.19-rc3 next-20220624]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Venkateshwar-Rao-Gannavarapu/Add-Xilinx-DSI-Tx-subsystem-DRM-driver/20220616-222008
base:   git://anongit.freedesktop.org/drm/drm drm-next
config: mips-randconfig-c004-20220626
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project b0d6dd3905db145853c7c744ac92d49b00b1fa20)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install mips cross compiling tool for clang build
        # apt-get install binutils-mips-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/28aa62ffdc1901029bf75961166f4ebba948b9b7
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Venkateshwar-Rao-Gannavarapu/Add-Xilinx-DSI-Tx-subsystem-DRM-driver/20220616-222008
        git checkout 28aa62ffdc1901029bf75961166f4ebba948b9b7
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/gpu/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/xlnx/xlnx_dsi.c:255:10: error: label at end of compound statement: expected statement
           default: /* define */
                   ^
                    ;
   1 error generated.


vim +255 drivers/gpu/drm/xlnx/xlnx_dsi.c

   227	
   228	#define MAX_INPUT_SEL_FORMATS   3
   229	static u32
   230	*xlnx_dsi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
   231						   struct drm_bridge_state *bridge_state,
   232						   struct drm_crtc_state *crtc_state,
   233						   struct drm_connector_state *conn_state,
   234						   u32 output_fmt,
   235						   unsigned int *num_input_fmts)
   236	{
   237		u32 *input_fmts;
   238		unsigned int i = 0;
   239	
   240		*num_input_fmts = 0;
   241		input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts), GFP_KERNEL);
   242		if (!input_fmts)
   243			return NULL;
   244	
   245		switch (output_fmt) {
   246		case MEDIA_BUS_FMT_FIXED:
   247			input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
   248			break;
   249		case MEDIA_BUS_FMT_RGB666_1X18:
   250			input_fmts[i++] = MEDIA_BUS_FMT_RGB666_1X18;
   251			break;
   252		case MEDIA_BUS_FMT_RGB565_1X16:
   253			input_fmts[i++] = MEDIA_BUS_FMT_RGB565_1X16;
   254			break;
 > 255		default: /* define */
   256		}
   257	
   258		*num_input_fmts = i;
   259		if (*num_input_fmts == 0) {
   260			kfree(input_fmts);
   261			input_fmts = NULL;
   262		}
   263	
   264		return input_fmts;
   265	}
   266	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH V2 2/2] drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver

[kernel test robot]

Hi Venkateshwar,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm/drm-next]
[also build test WARNING on linus/master v5.19-rc2 next-20220617]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Venkateshwar-Rao-Gannavarapu/Add-Xilinx-DSI-Tx-subsystem-DRM-driver/20220616-222008
base:   git://anongit.freedesktop.org/drm/drm drm-next
config: riscv-randconfig-s031-20220619 (https://download.01.org/0day-ci/archive/20220619/202206191404.Z1X0BOIH-lkp@intel.com/config)
compiler: riscv32-linux-gcc (GCC) 11.3.0
reproduce:
         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
         chmod +x ~/bin/make.cross
         # apt-get install sparse
         # sparse version: v0.6.4-30-g92122700-dirty
         # https://github.com/intel-lab-lkp/linux/commit/28aa62ffdc1901029bf75961166f4ebba948b9b7
         git remote add linux-review https://github.com/intel-lab-lkp/linux
         git fetch --no-tags linux-review Venkateshwar-Rao-Gannavarapu/Add-Xilinx-DSI-Tx-subsystem-DRM-driver/20220616-222008
         git checkout 28aa62ffdc1901029bf75961166f4ebba948b9b7
         # save the config file
         mkdir build_dir && cp config build_dir/.config
         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=riscv SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <yujie.liu@intel.com>


sparse warnings: (new ones prefixed by >>)

 >> drivers/gpu/drm/xlnx/xlnx_dsi.c:218:27: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct xlnx_dsi *dsi @@     got void [noderef] __iomem *iomem @@
    drivers/gpu/drm/xlnx/xlnx_dsi.c:218:27: sparse:     expected struct xlnx_dsi *dsi
    drivers/gpu/drm/xlnx/xlnx_dsi.c:218:27: sparse:     got void [noderef] __iomem *iomem

 >> drivers/gpu/drm/xlnx/xlnx_dsi.c:256:9: sparse: sparse: statement expected after case label

vim +256 drivers/gpu/drm/xlnx/xlnx_dsi.c

28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  179
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  180  static void
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  181  xlnx_dsi_bridge_enable(struct drm_bridge *bridge,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  182  		       struct drm_bridge_state *old_bridge_state)
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  183  {
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  184  	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  185  	struct drm_atomic_state *state = old_bridge_state->base.state;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  186  	struct drm_connector *connector;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  187  	struct drm_crtc *crtc;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  188  	const struct drm_crtc_state *crtc_state;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  189  	const struct drm_display_mode *mode;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  190  	u32 reg, video_mode;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  191
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  192  	connector = drm_atomic_get_new_connector_for_encoder(state,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  193  							     bridge->encoder);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  194  	crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  195  	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  196  	mode = &crtc_state->adjusted_mode;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  197
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  198  	reg = xlnx_dsi_read(dsi, XDSI_PCR);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  199  	video_mode = (reg & XDSI_PCR_VIDEOMODE_MASK) >> XDSI_PCR_VIDEOMODE_SHIFT;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  200
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  201  	if (!video_mode && (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)) {
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  202  		reg = XDSI_TIME1_HSA(mode->hsync_end -
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  203  				     mode->hsync_start);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  204  		xlnx_dsi_write(dsi, XDSI_TIME1, reg);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  205  	}
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  206
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  207  	reg = XDSI_TIME4_VFP(mode->vsync_start - mode->vdisplay) |
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  208  		XDSI_TIME4_VBP(mode->vtotal - mode->vsync_end) |
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  209  		XDSI_TIME4_VSA(mode->vsync_end - mode->vsync_start);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  210  	xlnx_dsi_write(dsi, XDSI_TIME4, reg);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  211
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  212  	reg = XDSI_TIME3_HFP(mode->hsync_start - mode->hdisplay) |
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  213  		XDSI_TIME3_HBP(mode->htotal - mode->hsync_end);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  214  	xlnx_dsi_write(dsi, XDSI_TIME3, reg);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  215
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  216  	reg = XDSI_TIME2_HACT(mode->hdisplay * dsi->mul_factor / 100) |
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  217  		XDSI_TIME2_VACT(mode->vdisplay);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16 @218  	xlnx_dsi_write(dsi->iomem, XDSI_TIME2, reg);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  219
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  220  	xlnx_dsi_write(dsi, XDSI_PCR, XDSI_PCR_VIDEOMODE(BIT(0)));
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  221
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  222  	/* Enable Core */
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  223  	reg = xlnx_dsi_read(dsi, XDSI_CCR);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  224  	reg |= XDSI_CCR_COREENB;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  225  	xlnx_dsi_write(dsi, XDSI_CCR, reg);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  226  }
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  227
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  228  #define MAX_INPUT_SEL_FORMATS   3
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  229  static u32
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  230  *xlnx_dsi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  231  					   struct drm_bridge_state *bridge_state,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  232  					   struct drm_crtc_state *crtc_state,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  233  					   struct drm_connector_state *conn_state,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  234  					   u32 output_fmt,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  235  					   unsigned int *num_input_fmts)
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  236  {
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  237  	u32 *input_fmts;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  238  	unsigned int i = 0;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  239
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  240  	*num_input_fmts = 0;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  241  	input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts), GFP_KERNEL);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  242  	if (!input_fmts)
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  243  		return NULL;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  244
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  245  	switch (output_fmt) {
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  246  	case MEDIA_BUS_FMT_FIXED:
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  247  		input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  248  		break;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  249  	case MEDIA_BUS_FMT_RGB666_1X18:
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  250  		input_fmts[i++] = MEDIA_BUS_FMT_RGB666_1X18;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  251  		break;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  252  	case MEDIA_BUS_FMT_RGB565_1X16:
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  253  		input_fmts[i++] = MEDIA_BUS_FMT_RGB565_1X16;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  254  		break;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  255  	default: /* define */
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16 @256  	}
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  257
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  258  	*num_input_fmts = i;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  259  	if (*num_input_fmts == 0) {
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  260  		kfree(input_fmts);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  261  		input_fmts = NULL;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  262  	}
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  263
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  264  	return input_fmts;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  265  }
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  266

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH V2 2/2] drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver

[kernel test robot]

Hi Venkateshwar,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm/drm-next]
[also build test WARNING on linus/master v5.19-rc2 next-20220617]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Venkateshwar-Rao-Gannavarapu/Add-Xilinx-DSI-Tx-subsystem-DRM-driver/20220616-222008
base:   git://anongit.freedesktop.org/drm/drm drm-next
config: riscv-randconfig-s031-20220619 (https://download.01.org/0day-ci/archive/20220619/202206191404.Z1X0BOIH-lkp@intel.com/config)
compiler: riscv32-linux-gcc (GCC) 11.3.0
reproduce:
         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
         chmod +x ~/bin/make.cross
         # apt-get install sparse
         # sparse version: v0.6.4-30-g92122700-dirty
         # https://github.com/intel-lab-lkp/linux/commit/28aa62ffdc1901029bf75961166f4ebba948b9b7
         git remote add linux-review https://github.com/intel-lab-lkp/linux
         git fetch --no-tags linux-review Venkateshwar-Rao-Gannavarapu/Add-Xilinx-DSI-Tx-subsystem-DRM-driver/20220616-222008
         git checkout 28aa62ffdc1901029bf75961166f4ebba948b9b7
         # save the config file
         mkdir build_dir && cp config build_dir/.config
         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=riscv SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <yujie.liu@intel.com>


sparse warnings: (new ones prefixed by >>)

 >> drivers/gpu/drm/xlnx/xlnx_dsi.c:218:27: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct xlnx_dsi *dsi @@     got void [noderef] __iomem *iomem @@
    drivers/gpu/drm/xlnx/xlnx_dsi.c:218:27: sparse:     expected struct xlnx_dsi *dsi
    drivers/gpu/drm/xlnx/xlnx_dsi.c:218:27: sparse:     got void [noderef] __iomem *iomem

 >> drivers/gpu/drm/xlnx/xlnx_dsi.c:256:9: sparse: sparse: statement expected after case label

vim +256 drivers/gpu/drm/xlnx/xlnx_dsi.c

28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  179
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  180  static void
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  181  xlnx_dsi_bridge_enable(struct drm_bridge *bridge,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  182  		       struct drm_bridge_state *old_bridge_state)
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  183  {
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  184  	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  185  	struct drm_atomic_state *state = old_bridge_state->base.state;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  186  	struct drm_connector *connector;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  187  	struct drm_crtc *crtc;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  188  	const struct drm_crtc_state *crtc_state;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  189  	const struct drm_display_mode *mode;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  190  	u32 reg, video_mode;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  191
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  192  	connector = drm_atomic_get_new_connector_for_encoder(state,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  193  							     bridge->encoder);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  194  	crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  195  	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  196  	mode = &crtc_state->adjusted_mode;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  197
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  198  	reg = xlnx_dsi_read(dsi, XDSI_PCR);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  199  	video_mode = (reg & XDSI_PCR_VIDEOMODE_MASK) >> XDSI_PCR_VIDEOMODE_SHIFT;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  200
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  201  	if (!video_mode && (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)) {
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  202  		reg = XDSI_TIME1_HSA(mode->hsync_end -
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  203  				     mode->hsync_start);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  204  		xlnx_dsi_write(dsi, XDSI_TIME1, reg);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  205  	}
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  206
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  207  	reg = XDSI_TIME4_VFP(mode->vsync_start - mode->vdisplay) |
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  208  		XDSI_TIME4_VBP(mode->vtotal - mode->vsync_end) |
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  209  		XDSI_TIME4_VSA(mode->vsync_end - mode->vsync_start);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  210  	xlnx_dsi_write(dsi, XDSI_TIME4, reg);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  211
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  212  	reg = XDSI_TIME3_HFP(mode->hsync_start - mode->hdisplay) |
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  213  		XDSI_TIME3_HBP(mode->htotal - mode->hsync_end);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  214  	xlnx_dsi_write(dsi, XDSI_TIME3, reg);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  215
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  216  	reg = XDSI_TIME2_HACT(mode->hdisplay * dsi->mul_factor / 100) |
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  217  		XDSI_TIME2_VACT(mode->vdisplay);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16 @218  	xlnx_dsi_write(dsi->iomem, XDSI_TIME2, reg);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  219
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  220  	xlnx_dsi_write(dsi, XDSI_PCR, XDSI_PCR_VIDEOMODE(BIT(0)));
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  221
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  222  	/* Enable Core */
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  223  	reg = xlnx_dsi_read(dsi, XDSI_CCR);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  224  	reg |= XDSI_CCR_COREENB;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  225  	xlnx_dsi_write(dsi, XDSI_CCR, reg);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  226  }
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  227
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  228  #define MAX_INPUT_SEL_FORMATS   3
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  229  static u32
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  230  *xlnx_dsi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  231  					   struct drm_bridge_state *bridge_state,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  232  					   struct drm_crtc_state *crtc_state,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  233  					   struct drm_connector_state *conn_state,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  234  					   u32 output_fmt,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  235  					   unsigned int *num_input_fmts)
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  236  {
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  237  	u32 *input_fmts;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  238  	unsigned int i = 0;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  239
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  240  	*num_input_fmts = 0;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  241  	input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts), GFP_KERNEL);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  242  	if (!input_fmts)
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  243  		return NULL;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  244
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  245  	switch (output_fmt) {
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  246  	case MEDIA_BUS_FMT_FIXED:
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  247  		input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  248  		break;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  249  	case MEDIA_BUS_FMT_RGB666_1X18:
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  250  		input_fmts[i++] = MEDIA_BUS_FMT_RGB666_1X18;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  251  		break;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  252  	case MEDIA_BUS_FMT_RGB565_1X16:
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  253  		input_fmts[i++] = MEDIA_BUS_FMT_RGB565_1X16;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  254  		break;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  255  	default: /* define */
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16 @256  	}
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  257
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  258  	*num_input_fmts = i;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  259  	if (*num_input_fmts == 0) {
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  260  		kfree(input_fmts);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  261  		input_fmts = NULL;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  262  	}
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  263
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  264  	return input_fmts;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  265  }
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  266

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH V2 2/2] drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 11166 bytes --]

:::::: 
:::::: Manual check reason: "low confidence static check warning: drivers/gpu/drm/xlnx/xlnx_dsi.c:256:9: sparse: sparse: statement expected after case label"
:::::: 

CC: kbuild-all(a)lists.01.org
BCC: lkp(a)intel.com
In-Reply-To: <1655389056-37044-3-git-send-email-venkateshwar.rao.gannavarapu@xilinx.com>
References: <1655389056-37044-3-git-send-email-venkateshwar.rao.gannavarapu@xilinx.com>
TO: Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>
TO: laurent.pinchart(a)ideasonboard.com
TO: sam(a)ravnborg.org
TO: dri-devel(a)lists.freedesktop.org
CC: airlied(a)linux.ie
CC: vgannava(a)xilinx.com
CC: Venkateshwar Rao Gannavarapu <venkateshwar.rao.gannavarapu@xilinx.com>
CC: linux-kernel(a)vger.kernel.org

Hi Venkateshwar,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm/drm-next]
[also build test WARNING on linus/master v5.19-rc2 next-20220617]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Venkateshwar-Rao-Gannavarapu/Add-Xilinx-DSI-Tx-subsystem-DRM-driver/20220616-222008
base:   git://anongit.freedesktop.org/drm/drm drm-next
:::::: branch date: 3 days ago
:::::: commit date: 3 days ago
config: riscv-randconfig-s031-20220619 (https://download.01.org/0day-ci/archive/20220619/202206191404.Z1X0BOIH-lkp(a)intel.com/config)
compiler: riscv32-linux-gcc (GCC) 11.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-30-g92122700-dirty
        # https://github.com/intel-lab-lkp/linux/commit/28aa62ffdc1901029bf75961166f4ebba948b9b7
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Venkateshwar-Rao-Gannavarapu/Add-Xilinx-DSI-Tx-subsystem-DRM-driver/20220616-222008
        git checkout 28aa62ffdc1901029bf75961166f4ebba948b9b7
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=riscv SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> drivers/gpu/drm/xlnx/xlnx_dsi.c:256:9: sparse: sparse: statement expected after case label
>> drivers/gpu/drm/xlnx/xlnx_dsi.c:218:27: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct xlnx_dsi *dsi @@     got void [noderef] __iomem *iomem @@
   drivers/gpu/drm/xlnx/xlnx_dsi.c:218:27: sparse:     expected struct xlnx_dsi *dsi
   drivers/gpu/drm/xlnx/xlnx_dsi.c:218:27: sparse:     got void [noderef] __iomem *iomem

vim +256 drivers/gpu/drm/xlnx/xlnx_dsi.c

28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  179  
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  180  static void
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  181  xlnx_dsi_bridge_enable(struct drm_bridge *bridge,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  182  		       struct drm_bridge_state *old_bridge_state)
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  183  {
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  184  	struct xlnx_dsi *dsi = bridge_to_dsi(bridge);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  185  	struct drm_atomic_state *state = old_bridge_state->base.state;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  186  	struct drm_connector *connector;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  187  	struct drm_crtc *crtc;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  188  	const struct drm_crtc_state *crtc_state;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  189  	const struct drm_display_mode *mode;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  190  	u32 reg, video_mode;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  191  
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  192  	connector = drm_atomic_get_new_connector_for_encoder(state,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  193  							     bridge->encoder);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  194  	crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  195  	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  196  	mode = &crtc_state->adjusted_mode;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  197  
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  198  	reg = xlnx_dsi_read(dsi, XDSI_PCR);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  199  	video_mode = (reg & XDSI_PCR_VIDEOMODE_MASK) >> XDSI_PCR_VIDEOMODE_SHIFT;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  200  
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  201  	if (!video_mode && (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)) {
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  202  		reg = XDSI_TIME1_HSA(mode->hsync_end -
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  203  				     mode->hsync_start);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  204  		xlnx_dsi_write(dsi, XDSI_TIME1, reg);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  205  	}
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  206  
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  207  	reg = XDSI_TIME4_VFP(mode->vsync_start - mode->vdisplay) |
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  208  		XDSI_TIME4_VBP(mode->vtotal - mode->vsync_end) |
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  209  		XDSI_TIME4_VSA(mode->vsync_end - mode->vsync_start);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  210  	xlnx_dsi_write(dsi, XDSI_TIME4, reg);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  211  
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  212  	reg = XDSI_TIME3_HFP(mode->hsync_start - mode->hdisplay) |
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  213  		XDSI_TIME3_HBP(mode->htotal - mode->hsync_end);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  214  	xlnx_dsi_write(dsi, XDSI_TIME3, reg);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  215  
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  216  	reg = XDSI_TIME2_HACT(mode->hdisplay * dsi->mul_factor / 100) |
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  217  		XDSI_TIME2_VACT(mode->vdisplay);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16 @218  	xlnx_dsi_write(dsi->iomem, XDSI_TIME2, reg);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  219  
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  220  	xlnx_dsi_write(dsi, XDSI_PCR, XDSI_PCR_VIDEOMODE(BIT(0)));
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  221  
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  222  	/* Enable Core */
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  223  	reg = xlnx_dsi_read(dsi, XDSI_CCR);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  224  	reg |= XDSI_CCR_COREENB;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  225  	xlnx_dsi_write(dsi, XDSI_CCR, reg);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  226  }
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  227  
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  228  #define MAX_INPUT_SEL_FORMATS   3
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  229  static u32
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  230  *xlnx_dsi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  231  					   struct drm_bridge_state *bridge_state,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  232  					   struct drm_crtc_state *crtc_state,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  233  					   struct drm_connector_state *conn_state,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  234  					   u32 output_fmt,
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  235  					   unsigned int *num_input_fmts)
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  236  {
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  237  	u32 *input_fmts;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  238  	unsigned int i = 0;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  239  
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  240  	*num_input_fmts = 0;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  241  	input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts), GFP_KERNEL);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  242  	if (!input_fmts)
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  243  		return NULL;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  244  
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  245  	switch (output_fmt) {
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  246  	case MEDIA_BUS_FMT_FIXED:
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  247  		input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  248  		break;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  249  	case MEDIA_BUS_FMT_RGB666_1X18:
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  250  		input_fmts[i++] = MEDIA_BUS_FMT_RGB666_1X18;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  251  		break;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  252  	case MEDIA_BUS_FMT_RGB565_1X16:
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  253  		input_fmts[i++] = MEDIA_BUS_FMT_RGB565_1X16;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  254  		break;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  255  	default: /* define */
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16 @256  	}
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  257  
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  258  	*num_input_fmts = i;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  259  	if (*num_input_fmts == 0) {
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  260  		kfree(input_fmts);
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  261  		input_fmts = NULL;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  262  	}
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  263  
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  264  	return input_fmts;
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  265  }
28aa62ffdc1901 Venkateshwar Rao Gannavarapu 2022-06-16  266  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp