[PATCH v2 0/2] Add StarFive JH7110 SoC DSI support

[PATCH v2 0/2] Add StarFive JH7110 SoC DSI support

[Shengyang Chen]

This series is the series that attempts to support
the CDNS DSI driver used to converts DPI to DSI.
CDNS DSI is embedded in StarFive JH7110 SoC.
The series has been tested on the VisionFive 2 board.


changes since v1:
- Rebased on tag v6.7.

patch 1:
- Changed the 'starfive,cdns-dsi' to 'starfve,jh7110-dsi'.
- Changed the compatible enum alphabetical order.
- Restrict other variants.
- Drop 'dsi_' prefix.

patch 2:
- Optimize the calculation process.
- Drop useless definition.

v1: https://patchwork.kernel.org/project/dri-devel/cover/20231127113436.57361-1-shengyang.chen@starfivetech.com/

Keith Zhao (2):
  dt-bindings: display: bridge: cdns: Add properties to support StarFive
    JH7110 SoC
  gpu: drm: bridge: cadence: Add a driver and platform ops for StarFive
    JH7110 SoC

 .../bindings/display/bridge/cdns,dsi.yaml     |  44 +++-
 MAINTAINERS                                   |   8 +
 drivers/gpu/drm/bridge/cadence/Kconfig        |   7 +
 drivers/gpu/drm/bridge/cadence/Makefile       |   1 +
 .../gpu/drm/bridge/cadence/cdns-dsi-core.c    |  29 ++-
 .../gpu/drm/bridge/cadence/cdns-dsi-core.h    |  21 ++
 .../gpu/drm/bridge/cadence/cdns-dsi-jh7110.c  | 193 ++++++++++++++++++
 .../gpu/drm/bridge/cadence/cdns-dsi-jh7110.h  |  16 ++
 8 files changed, 316 insertions(+), 3 deletions(-)
 create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-dsi-jh7110.c
 create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-dsi-jh7110.h

-- 
2.17.1

[PATCH v2 1/2] dt-bindings: display: bridge: cdns: Add properties to support StarFive JH7110 SoC

[Shengyang Chen]

From: Keith Zhao <keith.zhao@starfivetech.com>

Add properties in CDNS DSI yaml file to match with
CDNS DSI module in StarFive JH7110 SoC.

Signed-off-by: Keith Zhao <keith.zhao@starfivetech.com>
---
 .../bindings/display/bridge/cdns,dsi.yaml     | 44 ++++++++++++++++++-
 1 file changed, 42 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml
index 23060324d16e..da091e105794 100644
--- a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml
@@ -16,6 +16,7 @@ properties:
   compatible:
     enum:
       - cdns,dsi
+      - starfve,jh7110-dsi
       - ti,j721e-dsi
 
   reg:
@@ -27,14 +28,20 @@ properties:
           Register block for wrapper settings registers in case of TI J7 SoCs.
 
   clocks:
+    minItems: 2
     items:
       - description: PSM clock, used by the IP
       - description: sys clock, used by the IP
+      - description: apb clock, used by the IP
+      - description: txesc clock, used by the IP
 
   clock-names:
+    minItems: 2
     items:
       - const: dsi_p_clk
       - const: dsi_sys_clk
+      - const: apb
+      - const: txesc
 
   phys:
     maxItems: 1
@@ -46,10 +53,21 @@ properties:
     maxItems: 1
 
   resets:
-    maxItems: 1
+    minItems: 1
+    items:
+      - description: dsi sys reset line
+      - description: dsi dpi reset line
+      - description: dsi apb reset line
+      - description: dsi txesc reset line
+      - description: dsi txbytehs reset line
 
   reset-names:
-    const: dsi_p_rst
+    items:
+      - const: dsi_p_rst
+      - const: dpi
+      - const: apb
+      - const: txesc
+      - const: txbytehs
 
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
@@ -90,6 +108,28 @@ allOf:
         reg:
           maxItems: 1
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: starfive,jh7110-dsi
+    then:
+      properties:
+        clocks:
+          minItems: 4
+          maxItems: 4
+        resets:
+          minItems: 5
+          maxItems: 5
+      required:
+        - reset-names
+    else:
+      properties:
+        clocks:
+          maxItems: 2
+        resets:
+          maxItems: 1
+
 required:
   - compatible
   - reg
-- 
2.17.1

[PATCH v2 2/2] gpu: drm: bridge: cadence: Add a driver and platform ops for StarFive JH7110 SoC

[Shengyang Chen]

From: Keith Zhao <keith.zhao@starfivetech.com>

In order to fit CDNS DSI module in StarFive JH7110 SoC,
The mainly modification is followed:

1.Add driver for StarFive JH7110 SoC to drive its CDNS DSI module.
2.Add platform ops in cdns-dsi.c for StarFive JH7110 SoC probing.

Signed-off-by: Keith Zhao <keith.zhao@starfivetech.com>
---
 MAINTAINERS                                   |   8 +
 drivers/gpu/drm/bridge/cadence/Kconfig        |   7 +
 drivers/gpu/drm/bridge/cadence/Makefile       |   1 +
 .../gpu/drm/bridge/cadence/cdns-dsi-core.c    |  29 ++-
 .../gpu/drm/bridge/cadence/cdns-dsi-core.h    |  21 ++
 .../gpu/drm/bridge/cadence/cdns-dsi-jh7110.c  | 193 ++++++++++++++++++
 .../gpu/drm/bridge/cadence/cdns-dsi-jh7110.h  |  16 ++
 7 files changed, 274 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-dsi-jh7110.c
 create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-dsi-jh7110.h

diff --git a/MAINTAINERS b/MAINTAINERS
index d32920dd943f..a220ea04f5c4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6847,6 +6847,14 @@ F:	Documentation/devicetree/bindings/display/solomon,ssd-common.yaml
 F:	Documentation/devicetree/bindings/display/solomon,ssd13*.yaml
 F:	drivers/gpu/drm/solomon/ssd130x*
 
+DRM DRIVERS FOR STARFIVE
+M:	Keith Zhao <keith.zhao@starfivetech.com>
+M:	Shengyang Chen <shengyang.chen@starfivetech.com>
+L:	dri-devel@lists.freedesktop.org
+S:	Maintained
+T:	git git://anongit.freedesktop.org/drm/drm-misc
+F:	drivers/gpu/drm/bridge/cadence/cdns-dsi-jh7110*
+
 DRM DRIVER FOR ST-ERICSSON MCDE
 M:	Linus Walleij <linus.walleij@linaro.org>
 S:	Maintained
diff --git a/drivers/gpu/drm/bridge/cadence/Kconfig b/drivers/gpu/drm/bridge/cadence/Kconfig
index cced81633ddc..ad9f572f4720 100644
--- a/drivers/gpu/drm/bridge/cadence/Kconfig
+++ b/drivers/gpu/drm/bridge/cadence/Kconfig
@@ -19,6 +19,13 @@ config DRM_CDNS_DSI_J721E
 	help
 	  Support J721E Cadence DSI wrapper. The wrapper manages
 	  the routing of the DSS DPI signal to the Cadence DSI.
+
+config DRM_CDNS_DSI_JH7110
+	bool "JH7110 SOC Cadence DSI support"
+	default n
+	help
+	  Cadence DPI to DSI bridge which is embedded in the
+	  StarFive JH7110 SoC.
 endif
 
 config DRM_CDNS_MHDP8546
diff --git a/drivers/gpu/drm/bridge/cadence/Makefile b/drivers/gpu/drm/bridge/cadence/Makefile
index c95fd5b81d13..87f603a9f4ad 100644
--- a/drivers/gpu/drm/bridge/cadence/Makefile
+++ b/drivers/gpu/drm/bridge/cadence/Makefile
@@ -2,6 +2,7 @@
 obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o
 cdns-dsi-y := cdns-dsi-core.o
 cdns-dsi-$(CONFIG_DRM_CDNS_DSI_J721E) += cdns-dsi-j721e.o
+cdns-dsi-$(CONFIG_DRM_CDNS_DSI_JH7110) += cdns-dsi-jh7110.o
 obj-$(CONFIG_DRM_CDNS_MHDP8546) += cdns-mhdp8546.o
 cdns-mhdp8546-y := cdns-mhdp8546-core.o cdns-mhdp8546-hdcp.o
 cdns-mhdp8546-$(CONFIG_DRM_CDNS_MHDP8546_J721E) += cdns-mhdp8546-j721e.o
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
index 7457d38622b0..a8f976f9eeed 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
@@ -27,6 +27,10 @@
 #include "cdns-dsi-j721e.h"
 #endif
 
+#ifdef CONFIG_DRM_CDNS_DSI_JH7110
+#include "cdns-dsi-jh7110.h"
+#endif
+
 #define IP_CONF				0x0
 #define SP_HS_FIFO_DEPTH(x)		(((x) & GENMASK(30, 26)) >> 26)
 #define SP_LP_FIFO_DEPTH(x)		(((x) & GENMASK(25, 21)) >> 21)
@@ -552,6 +556,10 @@ static int cdns_dsi_adjust_phy_config(struct cdns_dsi *dsi,
 	/* data rate was in bytes/sec, convert to bits/sec. */
 	phy_cfg->hs_clk_rate = dlane_bps * 8;
 
+	if (dsi->platform_ops && dsi->platform_ops->update)
+		adj_dsi_htotal = dsi->platform_ops->update(dsi, dsi_cfg, phy_cfg,
+							   dpi_hz, dpi_htotal, dsi_htotal);
+
 	dsi_hfp_ext = adj_dsi_htotal - dsi_htotal;
 	dsi_cfg->hfp += dsi_hfp_ext;
 	dsi_cfg->htotal = dsi_htotal + dsi_hfp_ext;
@@ -683,7 +691,7 @@ static void cdns_dsi_bridge_post_disable(struct drm_bridge *bridge)
 	pm_runtime_put(dsi->base.dev);
 }
 
-static void cdns_dsi_hs_init(struct cdns_dsi *dsi)
+void cdns_dsi_hs_init(struct cdns_dsi *dsi)
 {
 	struct cdns_dsi_output *output = &dsi->output;
 	u32 status;
@@ -1026,6 +1034,14 @@ static ssize_t cdns_dsi_transfer(struct mipi_dsi_host *host,
 
 	cdns_dsi_init_link(dsi);
 
+	/*
+	 * on JH7110 SoC , when transfer dsi command ,
+	 * cdns_dsi_hs_init is needed.
+	 * or the final ret will be error value.
+	 */
+	if (dsi->platform_ops && dsi->platform_ops->transfer)
+		dsi->platform_ops->transfer(dsi);
+
 	ret = mipi_dsi_create_packet(&packet, msg);
 	if (ret)
 		goto out;
@@ -1142,6 +1158,9 @@ static int __maybe_unused cdns_dsi_resume(struct device *dev)
 	clk_prepare_enable(dsi->dsi_p_clk);
 	clk_prepare_enable(dsi->dsi_sys_clk);
 
+	if (dsi->platform_ops && dsi->platform_ops->resume)
+		dsi->platform_ops->resume(dsi);
+
 	return 0;
 }
 
@@ -1152,6 +1171,10 @@ static int __maybe_unused cdns_dsi_suspend(struct device *dev)
 	clk_disable_unprepare(dsi->dsi_sys_clk);
 	clk_disable_unprepare(dsi->dsi_p_clk);
 	reset_control_assert(dsi->dsi_p_rst);
+
+	if (dsi->platform_ops && dsi->platform_ops->suspend)
+		dsi->platform_ops->suspend(dsi);
+
 	dsi->link_initialized = false;
 	return 0;
 }
@@ -1294,6 +1317,10 @@ static const struct of_device_id cdns_dsi_of_match[] = {
 #ifdef CONFIG_DRM_CDNS_DSI_J721E
 	{ .compatible = "ti,j721e-dsi", .data = &dsi_ti_j721e_ops, },
 #endif
+#ifdef CONFIG_DRM_CDNS_DSI_JH7110
+	{ .compatible = "starfive,jh7110-dsi", .data = &dsi_ti_jh7110_ops, },
+#endif
+
 	{ },
 };
 MODULE_DEVICE_TABLE(of, cdns_dsi_of_match);
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.h b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.h
index ca7ea2da635c..f4491009c977 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.h
+++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.h
@@ -53,12 +53,22 @@ struct cdns_dsi;
  * @deinit: Called in the CDNS DSI remove
  * @enable: Called at the beginning of CDNS DSI bridge enable
  * @disable: Called at the end of CDNS DSI bridge disable
+ * @resume: Called at the resume of CDNS DSI
+ * @suspend: Called at the suspend of CDNS DSI
+ * @update: Called at the middle of CDNS DSI bridge enable
  */
 struct cdns_dsi_platform_ops {
 	int (*init)(struct cdns_dsi *dsi);
 	void (*deinit)(struct cdns_dsi *dsi);
 	void (*enable)(struct cdns_dsi *dsi);
 	void (*disable)(struct cdns_dsi *dsi);
+	void (*resume)(struct cdns_dsi *dsi);
+	void (*suspend)(struct cdns_dsi *dsi);
+	int (*update)(struct cdns_dsi *dsi, struct cdns_dsi_cfg *dsi_cfg,
+		      struct phy_configure_opts_mipi_dphy *phy_cfg,
+		      unsigned long dpi_hz, unsigned long dpi_htotal,
+		      unsigned long dsi_htotal);
+	void (*transfer)(struct cdns_dsi *dsi);
 };
 
 struct cdns_dsi {
@@ -79,6 +89,17 @@ struct cdns_dsi {
 	bool link_initialized;
 	bool phy_initialized;
 	struct phy *dphy;
+
+#ifdef CONFIG_DRM_CDNS_DSI_JH7110
+	struct clk *apb_clk;
+	struct clk *txesc_clk;
+	struct reset_control *dpi_rst;
+	struct reset_control *apb_rst;
+	struct reset_control *txesc_rst;
+	struct reset_control *txbytehs_rst;
+#endif
 };
 
+void cdns_dsi_hs_init(struct cdns_dsi *dsi);
+
 #endif /* !__CDNS_DSI_H__ */
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-jh7110.c b/drivers/gpu/drm/bridge/cadence/cdns-dsi-jh7110.c
new file mode 100644
index 000000000000..654d083f49e1
--- /dev/null
+++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-jh7110.c
@@ -0,0 +1,193 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * JH7110 SoC Cadence DSI wrapper
+ *
+ * Copyright (C) 2023 StarFive Technology Co., Ltd.
+ */
+
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/reset.h>
+
+#include "cdns-dsi-jh7110.h"
+
+static int cdns_dsi_clock_enable(struct cdns_dsi *dsi, struct device *dev)
+{
+	int ret;
+
+	ret = clk_prepare_enable(dsi->apb_clk);
+	if (ret) {
+		dev_err(dev, "Failed to prepare/enable apb_clk\n");
+		return ret;
+	}
+
+	ret = clk_prepare_enable(dsi->txesc_clk);
+	if (ret) {
+		dev_err(dev, "Failed to prepare/enable txesc_clk\n");
+		return ret;
+	}
+
+	return ret;
+}
+
+static void cdns_dsi_clock_disable(struct cdns_dsi *dsi)
+{
+	clk_disable_unprepare(dsi->apb_clk);
+	clk_disable_unprepare(dsi->txesc_clk);
+}
+
+static int cdns_dsi_resets_deassert(struct cdns_dsi *dsi, struct device *dev)
+{
+	int ret;
+
+	ret = reset_control_deassert(dsi->apb_rst);
+	if (ret < 0) {
+		dev_err(dev, "failed to deassert apb_rst\n");
+		return ret;
+	}
+
+	ret = reset_control_deassert(dsi->txesc_rst);
+	if (ret < 0) {
+		dev_err(dev, "failed to deassert txesc_rst\n");
+		return ret;
+	}
+
+	ret = reset_control_deassert(dsi->dpi_rst);
+	if (ret < 0) {
+		dev_err(dev, "failed to deassert dpi_rst\n");
+		return ret;
+	}
+
+	return ret;
+}
+
+static int cdns_dsi_resets_assert(struct cdns_dsi *dsi, struct device *dev)
+{
+	int ret;
+
+	ret = reset_control_assert(dsi->apb_rst);
+	if (ret < 0) {
+		dev_err(dev, "failed to assert apb_rst\n");
+		return ret;
+	}
+
+	ret = reset_control_assert(dsi->txesc_rst);
+	if (ret < 0) {
+		dev_err(dev, "failed to assert txesc_rst\n");
+		return ret;
+	}
+
+	ret = reset_control_assert(dsi->dpi_rst);
+	if (ret < 0) {
+		dev_err(dev, "failed to assert dpi_rst\n");
+		return ret;
+	}
+
+	return ret;
+}
+
+static int cdns_dsi_get_clock(struct device *dev, struct cdns_dsi *dsi)
+{
+	dsi->apb_clk = devm_clk_get(dev, "apb");
+	if (IS_ERR(dsi->apb_clk))
+		return PTR_ERR(dsi->apb_clk);
+
+	dsi->txesc_clk = devm_clk_get(dev, "txesc");
+	if (IS_ERR(dsi->txesc_clk))
+		return PTR_ERR(dsi->txesc_clk);
+
+	return 0;
+}
+
+static int cdns_dsi_get_reset(struct device *dev, struct cdns_dsi *dsi)
+{
+	dsi->dpi_rst = devm_reset_control_get(dev, "dpi");
+	if (IS_ERR(dsi->dpi_rst))
+		return PTR_ERR(dsi->dpi_rst);
+
+	dsi->apb_rst = devm_reset_control_get(dev, "apb");
+	if (IS_ERR(dsi->apb_rst))
+		return PTR_ERR(dsi->apb_rst);
+
+	dsi->txesc_rst = devm_reset_control_get(dev, "txesc");
+	if (IS_ERR(dsi->txesc_rst))
+		return PTR_ERR(dsi->txesc_rst);
+
+	dsi->txbytehs_rst = devm_reset_control_get(dev, "txbytehs");
+	if (IS_ERR(dsi->txbytehs_rst))
+		return PTR_ERR(dsi->txbytehs_rst);
+
+	return 0;
+}
+
+static int cdns_dsi_jh7110_init(struct cdns_dsi *dsi)
+{
+	int ret;
+
+	ret = cdns_dsi_get_clock(dsi->base.dev, dsi);
+	if (ret)
+		return ret;
+
+	return cdns_dsi_get_reset(dsi->base.dev, dsi);
+}
+
+static void cdns_dsi_jh7110_resume(struct cdns_dsi *dsi)
+{
+	int ret;
+
+	ret = cdns_dsi_clock_enable(dsi, dsi->base.dev);
+	if (ret) {
+		dev_err(dsi->base.dev, "failed to enable clock\n");
+		return;
+	}
+	ret = cdns_dsi_resets_deassert(dsi, dsi->base.dev);
+	if (ret < 0) {
+		dev_err(dsi->base.dev, "failed to deassert reset\n");
+		return;
+	}
+}
+
+static void cdns_dsi_jh7110_suspend(struct cdns_dsi *dsi)
+{
+	int ret;
+
+	ret = cdns_dsi_resets_assert(dsi, dsi->base.dev);
+	if (ret < 0) {
+		dev_err(dsi->base.dev, "failed to deassert reset\n");
+		return;
+	}
+
+	cdns_dsi_clock_disable(dsi);
+}
+
+static int cdns_dsi_jh7110_update(struct cdns_dsi *dsi, struct cdns_dsi_cfg *dsi_cfg,
+				  struct phy_configure_opts_mipi_dphy *phy_cfg,
+				  unsigned long dpi_hz, unsigned long dpi_htotal,
+				  unsigned long dsi_htotal)
+{
+	unsigned long long dlane_bps;
+	unsigned long adj_dsi_htotal;
+	unsigned int lanes = dsi->output.dev->lanes;
+
+	phy_cfg->hs_clk_rate = phy_cfg->hs_clk_rate - (phy_cfg->hs_clk_rate % 10000000);
+	phy_cfg->hs_clk_rate = phy_cfg->hs_clk_rate + 40000000;
+	dlane_bps = phy_cfg->hs_clk_rate * lanes * dpi_htotal / 8;
+	adj_dsi_htotal = dlane_bps / dpi_hz;
+
+	return adj_dsi_htotal;
+}
+
+static void jh7110_cdns_dsi_hs_init(struct cdns_dsi *dsi)
+{
+	cdns_dsi_hs_init(dsi);
+	reset_control_deassert(dsi->txbytehs_rst);
+}
+
+const struct cdns_dsi_platform_ops dsi_ti_jh7110_ops = {
+	.init = cdns_dsi_jh7110_init,
+	.resume = cdns_dsi_jh7110_resume,
+	.suspend = cdns_dsi_jh7110_suspend,
+	.update = cdns_dsi_jh7110_update,
+	.transfer = jh7110_cdns_dsi_hs_init,
+};
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-jh7110.h b/drivers/gpu/drm/bridge/cadence/cdns-dsi-jh7110.h
new file mode 100644
index 000000000000..15d6a766b502
--- /dev/null
+++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-jh7110.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * JH7110 Cadence DSI
+ *
+ * Copyright (C) 2022-2023 StarFive Technology Co., Ltd.
+ * Author: keith.zhao <keith.zhao@starfivetech.com>
+ */
+
+#ifndef __CDNS_DSI_JH7110_H__
+#define __CDNS_DSI_JH7110_H__
+
+#include "cdns-dsi-core.h"
+
+extern const struct cdns_dsi_platform_ops dsi_ti_jh7110_ops;
+
+#endif /* !__CDNS_DSI_JH7110_H__ */
-- 
2.17.1

Re: [PATCH v2 2/2] gpu: drm: bridge: cadence: Add a driver and platform ops for StarFive JH7110 SoC

[Philipp Zabel]

On Di, 2024-01-09 at 15:25 +0800, Shengyang Chen wrote:
> +static int cdns_dsi_get_reset(struct device *dev, struct cdns_dsi *dsi)
> +{
> +	dsi->dpi_rst = devm_reset_control_get(dev, "dpi");
> +	if (IS_ERR(dsi->dpi_rst))
> +		return PTR_ERR(dsi->dpi_rst);

Please use devm_reset_control_get_exclusive() directly.

Also, consider using devm_reset_control_bulk_get_exclusive() instead,
to control "dpi"/"apb"/"txesc" resets together - if the hardware can
handle deasserting in reversed order.

> +
> +	dsi->apb_rst = devm_reset_control_get(dev, "apb");
> +	if (IS_ERR(dsi->apb_rst))
> +		return PTR_ERR(dsi->apb_rst);
> +
> +	dsi->txesc_rst = devm_reset_control_get(dev, "txesc");
> +	if (IS_ERR(dsi->txesc_rst))
> +		return PTR_ERR(dsi->txesc_rst);
> +
> +	dsi->txbytehs_rst = devm_reset_control_get(dev, "txbytehs");
> +	if (IS_ERR(dsi->txbytehs_rst))
> +		return PTR_ERR(dsi->txbytehs_rst);
> +
> +	return 0;
> +}

regards
Philipp

Re: [PATCH v2 1/2] dt-bindings: display: bridge: cdns: Add properties to support StarFive JH7110 SoC

[Krzysztof Kozlowski]

On 09/01/2024 08:25, Shengyang Chen wrote:
> From: Keith Zhao <keith.zhao@starfivetech.com>
> 
> Add properties in CDNS DSI yaml file to match with
> CDNS DSI module in StarFive JH7110 SoC.

Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597

Subject: Make it concise, like: "Add StarFive JH7110 SoC display bridge
or something".

> 
> Signed-off-by: Keith Zhao <keith.zhao@starfivetech.com>
> ---
>  .../bindings/display/bridge/cdns,dsi.yaml     | 44 ++++++++++++++++++-
>  1 file changed, 42 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml
> index 23060324d16e..da091e105794 100644
> --- a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml
> @@ -16,6 +16,7 @@ properties:
>    compatible:
>      enum:
>        - cdns,dsi
> +      - starfve,jh7110-dsi
>        - ti,j721e-dsi
>  
>    reg:
> @@ -27,14 +28,20 @@ properties:
>            Register block for wrapper settings registers in case of TI J7 SoCs.
>  
>    clocks:
> +    minItems: 2
>      items:
>        - description: PSM clock, used by the IP
>        - description: sys clock, used by the IP
> +      - description: apb clock, used by the IP
> +      - description: txesc clock, used by the IP
>  
>    clock-names:
> +    minItems: 2
>      items:
>        - const: dsi_p_clk
>        - const: dsi_sys_clk
> +      - const: apb
> +      - const: txesc
>  
>    phys:
>      maxItems: 1
> @@ -46,10 +53,21 @@ properties:
>      maxItems: 1
>  
>    resets:
> -    maxItems: 1
> +    minItems: 1
> +    items:
> +      - description: dsi sys reset line
> +      - description: dsi dpi reset line
> +      - description: dsi apb reset line
> +      - description: dsi txesc reset line
> +      - description: dsi txbytehs reset line
>  
>    reset-names:
> -    const: dsi_p_rst
> +    items:
> +      - const: dsi_p_rst
> +      - const: dpi
> +      - const: apb
> +      - const: txesc
> +      - const: txbytehs
>  
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
> @@ -90,6 +108,28 @@ allOf:
>          reg:
>            maxItems: 1
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: starfive,jh7110-dsi
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 4
> +          maxItems: 4
> +        resets:
> +          minItems: 5
> +          maxItems: 5

Missing constraints for xxx-names.

> +      required:
> +        - reset-names
> +    else:
> +      properties:
> +        clocks:
> +          maxItems: 2
> +        resets:
> +          maxItems: 1

Same problem.

Best regards,
Krzysztof

RE: [PATCH v2 2/2] gpu: drm: bridge: cadence: Add a driver and platform ops for StarFive JH7110 SoC

[Shengyang Chen]

Hi, Philipp

Thanks for review and comment.

> -----Original Message-----
> From: Philipp Zabel <p.zabel@pengutronix.de>
> Sent: 2024年1月9日 17:08
> To: Shengyang Chen <shengyang.chen@starfivetech.com>;
> devicetree@vger.kernel.org; dri-devel@lists.freedesktop.org
> Cc: andrzej.hajda@intel.com; neil.armstrong@linaro.org; rfoss@kernel.org;
> Laurent.pinchart@ideasonboard.com; jonas@kwiboo.se;
> jernej.skrabec@gmail.com; maarten.lankhorst@linux.intel.com;
> mripard@kernel.org; tzimmermann@suse.de; airlied@gmail.com;
> daniel@ffwll.ch; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> conor+dt@kernel.org; tomi.valkeinen@ideasonboard.com; r-ravikumar@ti.com;
> aford173@gmail.com; rdunlap@infradead.org;
> u.kleine-koenig@pengutronix.de; bbrezillon@kernel.org; Changhuang Liang
> <changhuang.liang@starfivetech.com>; Keith Zhao
> <keith.zhao@starfivetech.com>; Jack Zhu <jack.zhu@starfivetech.com>;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v2 2/2] gpu: drm: bridge: cadence: Add a driver and
> platform ops for StarFive JH7110 SoC
> 
> On Di, 2024-01-09 at 15:25 +0800, Shengyang Chen wrote:
> > +static int cdns_dsi_get_reset(struct device *dev, struct cdns_dsi
> > +*dsi) {
> > +	dsi->dpi_rst = devm_reset_control_get(dev, "dpi");
> > +	if (IS_ERR(dsi->dpi_rst))
> > +		return PTR_ERR(dsi->dpi_rst);
> 
> Please use devm_reset_control_get_exclusive() directly.
> 
> Also, consider using devm_reset_control_bulk_get_exclusive() instead, to
> control "dpi"/"apb"/"txesc" resets together - if the hardware can handle
> deasserting in reversed order.
> 

ok, will follow up this issue.

> > +
> > +	dsi->apb_rst = devm_reset_control_get(dev, "apb");
> > +	if (IS_ERR(dsi->apb_rst))
> > +		return PTR_ERR(dsi->apb_rst);
> > +
> > +	dsi->txesc_rst = devm_reset_control_get(dev, "txesc");
> > +	if (IS_ERR(dsi->txesc_rst))
> > +		return PTR_ERR(dsi->txesc_rst);
> > +
> > +	dsi->txbytehs_rst = devm_reset_control_get(dev, "txbytehs");
> > +	if (IS_ERR(dsi->txbytehs_rst))
> > +		return PTR_ERR(dsi->txbytehs_rst);
> > +
> > +	return 0;
> > +}
> 
> regards
> Philipp


thanks.

Best Regards,
Shengyang

RE: [PATCH v2 1/2] dt-bindings: display: bridge: cdns: Add properties to support StarFive JH7110 SoC

[Shengyang Chen]

Hi, Krzysztof

Thanks for review and comment.

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: 2024年1月11日 5:06
> To: Shengyang Chen <shengyang.chen@starfivetech.com>;
> devicetree@vger.kernel.org; dri-devel@lists.freedesktop.org
> Cc: andrzej.hajda@intel.com; neil.armstrong@linaro.org; rfoss@kernel.org;
> Laurent.pinchart@ideasonboard.com; jonas@kwiboo.se;
> jernej.skrabec@gmail.com; maarten.lankhorst@linux.intel.com;
> mripard@kernel.org; tzimmermann@suse.de; airlied@gmail.com;
> daniel@ffwll.ch; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> conor+dt@kernel.org; p.zabel@pengutronix.de;
> tomi.valkeinen@ideasonboard.com; r-ravikumar@ti.com;
> aford173@gmail.com; rdunlap@infradead.org;
> u.kleine-koenig@pengutronix.de; bbrezillon@kernel.org; Changhuang Liang
> <changhuang.liang@starfivetech.com>; Keith Zhao
> <keith.zhao@starfivetech.com>; Jack Zhu <jack.zhu@starfivetech.com>;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v2 1/2] dt-bindings: display: bridge: cdns: Add properties to
> support StarFive JH7110 SoC
> 
> On 09/01/2024 08:25, Shengyang Chen wrote:
> > From: Keith Zhao <keith.zhao@starfivetech.com>
> >
> > Add properties in CDNS DSI yaml file to match with CDNS DSI module in
> > StarFive JH7110 SoC.
> 
> Please wrap commit message according to Linux coding style / submission
> process (neither too early nor over the limit):
> https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submi
> tting-patches.rst#L597
> 
> Subject: Make it concise, like: "Add StarFive JH7110 SoC display bridge or
> something".
> 

ok, will follow up this issue.
thanks for suggestion.

> >
> > Signed-off-by: Keith Zhao <keith.zhao@starfivetech.com>
> > ---
> >  .../bindings/display/bridge/cdns,dsi.yaml     | 44 ++++++++++++++++++-
> >  1 file changed, 42 insertions(+), 2 deletions(-)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml
> > b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml
> > index 23060324d16e..da091e105794 100644
> > --- a/Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml
> > +++ b/Documentation/devicetree/bindings/display/bridge/cdns,dsi.yaml
> > @@ -16,6 +16,7 @@ properties:
> >    compatible:
> >      enum:
> >        - cdns,dsi
> > +      - starfve,jh7110-dsi
> >        - ti,j721e-dsi
> >
> >    reg:
> > @@ -27,14 +28,20 @@ properties:
> >            Register block for wrapper settings registers in case of TI J7
> SoCs.
> >
> >    clocks:
> > +    minItems: 2
> >      items:
> >        - description: PSM clock, used by the IP
> >        - description: sys clock, used by the IP
> > +      - description: apb clock, used by the IP
> > +      - description: txesc clock, used by the IP
> >
> >    clock-names:
> > +    minItems: 2
> >      items:
> >        - const: dsi_p_clk
> >        - const: dsi_sys_clk
> > +      - const: apb
> > +      - const: txesc
> >
> >    phys:
> >      maxItems: 1
> > @@ -46,10 +53,21 @@ properties:
> >      maxItems: 1
> >
> >    resets:
> > -    maxItems: 1
> > +    minItems: 1
> > +    items:
> > +      - description: dsi sys reset line
> > +      - description: dsi dpi reset line
> > +      - description: dsi apb reset line
> > +      - description: dsi txesc reset line
> > +      - description: dsi txbytehs reset line
> >
> >    reset-names:
> > -    const: dsi_p_rst
> > +    items:
> > +      - const: dsi_p_rst
> > +      - const: dpi
> > +      - const: apb
> > +      - const: txesc
> > +      - const: txbytehs
> >
> >    ports:
> >      $ref: /schemas/graph.yaml#/properties/ports
> > @@ -90,6 +108,28 @@ allOf:
> >          reg:
> >            maxItems: 1
> >
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: starfive,jh7110-dsi
> > +    then:
> > +      properties:
> > +        clocks:
> > +          minItems: 4
> > +          maxItems: 4
> > +        resets:
> > +          minItems: 5
> > +          maxItems: 5
> 
> Missing constraints for xxx-names.
> 

ok, will fix it

> > +      required:
> > +        - reset-names
> > +    else:
> > +      properties:
> > +        clocks:
> > +          maxItems: 2
> > +        resets:
> > +          maxItems: 1
> 
> Same problem.
> 

ok, will fix it

> Best regards,
> Krzysztof

thanks.

Best Regards,
Shengyang