[PATCH v12 0/9] MIPS: JZ4780 and CI20 HDMI

[PATCH v12 0/9] MIPS: JZ4780 and CI20 HDMI

[H. Nikolaus Schaller]

PATCH V12 2022-01-31 13:26:54:
This version reworks how hdmi ddc power is controlled by connector and not
by ddc/hdmi bridge driver.

Also some patches of the previous version of this series have been removed
since they are already applied to mips-next/linux/next/v5.17-rc1.

Fixes and changes:

- repair interworking of dw-hdmi with connector-hdmi (by hns@goldelico.com)
- fix JZ_REG_LCD_OSDC setup for jz4780 (by hns@goldelico.com and paul@crapouillou.net)
- adjustments for ci20.dts to use connector gpio for +5v (suggested by several)
- to add control of "ddc-en-gpios" to hdmi-connector driver (by hns@goldelico.com)
- regulator code removed because we now use the "ddc-en-gpios" of the connector
  driver (suggested by paul@crapouillou.net)
- bindings: addition of "ddc-i2c-bus" and "hdmi-5v-supply" removed (suggested by robh+dt@kernel.org)

PATCH V11 2021-12-02 19:39:52:
- patch 4/8: change devm_regulator_get_optional to devm_regulator_get and
             remove NULL check (requested by broonie@kernel.org)
- patch 3/8: make hdmi-5v-supply required (requested by broonie@kernel.org)

PATCH V10 2021-11-30 22:26:41:
- patch 3/8: fix $id and $ref paths (found by robh@kernel.org)

PATCH V9 2021-11-24 22:29:14:
- patch 6/8: remove optional <0> for assigned-clocks and unintentionally included "unwedge" setup (found by paul@crapouillou.net)
- patch 4/8: some cosmetics
             make regulator enable/disable only if not NULL (found by paul@crapouillou.net)
             simplify/fix error handling and driver cleanup on remove (proposed by paul@crapouillou.net)
- patch 3/8: fix #include path in example (found by paul@crapouillou.net)
             fix missing "i" in unevaluatedProperties (found by robh@kernel.org)
             fix 4 spaces indentation for required: property (found by robh@kernel.org)

PATCH V8 2021-11-23 19:14:00:
- fix a bad editing result from patch 2/8 (found by paul@crapouillou.net)

PATCH V7 2021-11-23 18:46:23:
- changed gpio polarity of hdmi_power to 0 (suggested by paul@crapouillou.net)
- fixed LCD1 irq number (bug found by paul@crapouillou.net)
- removed "- 4" for calculating max_register (suggested by paul@crapouillou.net)
- use unevaluatedPropertes instead of additionalProperties (suggested by robh@kernel.org)
- moved and renamed ingenic,jz4780-hdmi.yaml (suggested by robh@kernel.org)
- adjusted assigned-clocks changes to upstream which added some for SSI (by hns@goldelico.com)
- rebased and tested with v5.16-rc2 + patch set drm/ingenic by paul@crapouillou.net (by hns@goldelico.com)

PATCH V6 2021-11-10 20:43:33:
- changed CONFIG_DRM_INGENIC_DW_HDMI to "m" (by hns@goldelico.com)
- made ingenic-dw-hdmi an independent platform driver which can be compiled as module
  and removed error patch fixes for IPU (suggested by paul@crapouillou.net)
- moved assigned-clocks from jz4780.dtsi to ci20.dts (suggested by paul@crapouillou.net)
- fixed reg property in jz4780.dtsi to cover all registers incl. gamma and vee (by hns@goldelico.com)
- added a base patch to calculate regmap size from DTS reg property (requested by paul@crapouillou.net)
- restored resetting all bits except one in LCDOSDC (requested by paul@crapouillou.net)
- clarified setting of cpos (suggested by paul@crapouillou.net)
- moved bindings definition for ddc-i2c-bus (suggested by paul@crapouillou.net)
- simplified mask definitions for JZ_LCD_DESSIZE (requested by paul@crapouillou.net)
- removed setting alpha premultiplication (suggested by paul@crapouillou.net)
- removed some comments (suggested by paul@crapouillou.net)

PATCH V5 2021-10-05 14:28:44:
- dropped mode_fixup and timings support in dw-hdmi as it is no longer needed in this V5 (by hns@goldelico.com)
- dropped "drm/ingenic: add some jz4780 specific features" (stimulated by paul@crapouillou.net)
- fixed typo in commit subject: "synopsis" -> "synopsys" (by hns@goldelico.com)
- swapped clocks in jz4780.dtsi to match synopsys,dw-hdmi.yaml (by hns@goldelico.com)
- improved, simplified, fixed, dtbschecked ingenic-jz4780-hdmi.yaml and made dependent of bridge/synopsys,dw-hdmi.yaml (based on suggestions by maxime@cerno.tech)
- fixed binding vs. driver&DTS use of hdmi-5v regulator (suggested by maxime@cerno.tech)
- dropped "drm/bridge: synopsis: Fix to properly handle HPD" - was a no longer needed workaround for a previous version
  (suggested by maxime@cerno.tech)

PATCH V4 2021-09-27 18:44:38:
- fix setting output_port = 1 (issue found by paul@crapouillou.net)
- ci20.dts: convert to use hdmi-connector (by hns@goldelico.com)
- add a hdmi-regulator to control +5V power (by hns@goldelico.com)
- added a fix to dw-hdmi to call drm_kms_helper_hotplug_event on plugin event detection (by hns@goldelico.com)
- always allocate extended descriptor but initialize only for jz4780 (by hns@goldelico.com)
- updated to work on top of "[PATCH v3 0/6] drm/ingenic: Various improvements v3" (by paul@crapouillou.net)
- rebased to v5.13-rc3

PATCH V3 2021-08-08 07:10:50:
This series adds HDMI support for JZ4780 and CI20 board (and fixes one IPU related issue in registration error path)
- [patch 1/8] switched from mode_fixup to atomic_check (suggested by robert.foss@linaro.org)
  - the call to the dw-hdmi specialization is still called mode_fixup
- [patch 3/8] diverse fixes for ingenic-drm-drv (suggested by paul@crapouillou.net)
  - factor out some non-HDMI features of the jz4780 into a separate patch
  - multiple fixes around max height
  - do not change regmap config but a copy on stack
  - define some constants
  - factor out fixing of drm_init error path for IPU into separate patch
  - use FIELD_PREP()
- [patch 8/8] conversion to component framework dropped (suggested by Laurent.pinchart@ideasonboard.com and paul@crapouillou.net)

PATCH V2 2021-08-05 16:08:05:
- code and commit messages revisited for checkpatch warnings
- rebased on v5.14-rc4
- include (failed, hence RFC 8/8) attempt to convert to component framework
  (was suggested by Paul Cercueil <paul@crapouillou.net> a while ago)

This series adds HDMI support for JZ4780 and CI20 board



H. Nikolaus Schaller (5):
  drm/ingenic: prepare ingenic drm for later addition of JZ4780
  drm/synopsys+ingenic: repair hot plug detection
  dw-hdmi/ingenic-dw-hdmi: repair interworking with hdmi-connector
  drm/bridge: display-connector: add ddc-en gpio support
  MIPS: DTS: CI20: fix how ddc power is enabled

Paul Boddie (3):
  drm/ingenic: Add support for JZ4780 and HDMI output
  drm/ingenic: Add dw-hdmi driver specialization for jz4780
  [RFC] drm/ingenic: add some more features specific to jz4780

Sam Ravnborg (1):
  dt-bindings: display: Add ingenic,jz4780-dw-hdmi DT Schema

 .../display/bridge/ingenic,jz4780-hdmi.yaml   |  83 ++++++++++++++
 arch/mips/boot/dts/ingenic/ci20.dts           |  15 +--
 drivers/gpu/drm/bridge/display-connector.c    |  17 +++
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c     |  13 ++-
 drivers/gpu/drm/ingenic/Kconfig               |   9 ++
 drivers/gpu/drm/ingenic/Makefile              |   1 +
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c     | 104 ++++++++++++++++-
 drivers/gpu/drm/ingenic/ingenic-drm.h         |  38 +++++++
 drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c     | 106 ++++++++++++++++++
 include/drm/bridge/dw_hdmi.h                  |   1 +
 10 files changed, 368 insertions(+), 19 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/ingenic,jz4780-hdmi.yaml
 create mode 100644 drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c

-- 
2.33.0

[PATCH v12 1/9] drm/ingenic: prepare ingenic drm for later addition of JZ4780

[H. Nikolaus Schaller]

This changes the way the regmap is allocated to prepare for the
later addition of the JZ4780 which has more registers and bits
than the others.

Therefore we make the regmap as big as the reg property in
the device tree tells.

Suggested-by: Paul Cercueil <paul@crapouillou.net>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index b4943a56be09b..9c60fc4605e4b 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -173,7 +173,6 @@ static const struct regmap_config ingenic_drm_regmap_config = {
 	.val_bits = 32,
 	.reg_stride = 4,
 
-	.max_register = JZ_REG_LCD_SIZE1,
 	.writeable_reg = ingenic_drm_writeable_reg,
 };
 
@@ -1011,6 +1010,8 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
 	struct ingenic_drm_bridge *ib;
 	struct drm_device *drm;
 	void __iomem *base;
+	struct resource *res;
+	struct regmap_config regmap_config;
 	long parent_rate;
 	unsigned int i, clone_mask = 0;
 	int ret, irq;
@@ -1056,14 +1057,16 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
 	drm->mode_config.funcs = &ingenic_drm_mode_config_funcs;
 	drm->mode_config.helper_private = &ingenic_drm_mode_config_helpers;
 
-	base = devm_platform_ioremap_resource(pdev, 0);
+	base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
 	if (IS_ERR(base)) {
 		dev_err(dev, "Failed to get memory resource\n");
 		return PTR_ERR(base);
 	}
 
+	regmap_config = ingenic_drm_regmap_config;
+	regmap_config.max_register = res->end - res->start;
 	priv->map = devm_regmap_init_mmio(dev, base,
-					  &ingenic_drm_regmap_config);
+					  &regmap_config);
 	if (IS_ERR(priv->map)) {
 		dev_err(dev, "Failed to create regmap\n");
 		return PTR_ERR(priv->map);
-- 
2.33.0

[PATCH v12 2/9] drm/ingenic: Add support for JZ4780 and HDMI output

[H. Nikolaus Schaller]

From: Paul Boddie <paul@boddie.org.uk>

Add support for the LCD controller present on JZ4780 SoCs.
This SoC uses 8-byte descriptors which extend the current
4-byte descriptors used for other Ingenic SoCs.

Tested on MIPS Creator CI20 board.

Signed-off-by: Paul Boddie <paul@boddie.org.uk>
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 61 ++++++++++++++++++++++-
 drivers/gpu/drm/ingenic/ingenic-drm.h     | 38 ++++++++++++++
 2 files changed, 98 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index 9c60fc4605e4b..ccdb9eedd9247 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -6,6 +6,7 @@
 
 #include "ingenic-drm.h"
 
+#include <linux/bitfield.h>
 #include <linux/component.h>
 #include <linux/clk.h>
 #include <linux/dma-mapping.h>
@@ -49,6 +50,11 @@ struct ingenic_dma_hwdesc {
 	u32 addr;
 	u32 id;
 	u32 cmd;
+	/* extended hw descriptor for jz4780 */
+	u32 offsize;
+	u32 pagewidth;
+	u32 cpos;
+	u32 dessize;
 } __aligned(16);
 
 struct ingenic_dma_hwdescs {
@@ -59,7 +65,9 @@ struct ingenic_dma_hwdescs {
 struct jz_soc_info {
 	bool needs_dev_clk;
 	bool has_osd;
+	bool has_alpha;
 	bool map_noncoherent;
+	bool use_extended_hwdesc;
 	unsigned int max_width, max_height;
 	const u32 *formats_f0, *formats_f1;
 	unsigned int num_formats_f0, num_formats_f1;
@@ -446,6 +454,9 @@ static int ingenic_drm_plane_atomic_check(struct drm_plane *plane,
 	if (!crtc)
 		return 0;
 
+	if (plane == &priv->f0)
+		return -EINVAL;
+
 	crtc_state = drm_atomic_get_existing_crtc_state(state,
 							crtc);
 	if (WARN_ON(!crtc_state))
@@ -662,6 +673,33 @@ static void ingenic_drm_plane_atomic_update(struct drm_plane *plane,
 		hwdesc->cmd = JZ_LCD_CMD_EOF_IRQ | (width * height * cpp / 4);
 		hwdesc->next = dma_hwdesc_addr(priv, next_id);
 
+		if (priv->soc_info->use_extended_hwdesc) {
+			hwdesc->cmd |= JZ_LCD_CMD_FRM_ENABLE;
+
+			/* Extended 8-byte descriptor */
+			hwdesc->cpos = 0;
+			hwdesc->offsize = 0;
+			hwdesc->pagewidth = 0;
+
+			switch (newstate->fb->format->format) {
+			case DRM_FORMAT_XRGB1555:
+				hwdesc->cpos |= JZ_LCD_CPOS_RGB555;
+				fallthrough;
+			case DRM_FORMAT_RGB565:
+				hwdesc->cpos |= JZ_LCD_CPOS_BPP_15_16;
+				break;
+			case DRM_FORMAT_XRGB8888:
+				hwdesc->cpos |= JZ_LCD_CPOS_BPP_18_24;
+				break;
+			}
+			hwdesc->cpos |= (JZ_LCD_CPOS_COEFFICIENT_1 <<
+					 JZ_LCD_CPOS_COEFFICIENT_OFFSET);
+			hwdesc->dessize =
+				(0xff << JZ_LCD_DESSIZE_ALPHA_OFFSET) |
+				FIELD_PREP(JZ_LCD_DESSIZE_HEIGHT_MASK, height - 1) |
+				FIELD_PREP(JZ_LCD_DESSIZE_WIDTH_MASK, width - 1);
+		}
+
 		if (drm_atomic_crtc_needs_modeset(crtc_state)) {
 			fourcc = newstate->fb->format->format;
 
@@ -693,6 +731,9 @@ static void ingenic_drm_encoder_atomic_mode_set(struct drm_encoder *encoder,
 		    | JZ_LCD_CFG_SPL_DISABLE | JZ_LCD_CFG_REV_DISABLE;
 	}
 
+	if (priv->soc_info->use_extended_hwdesc)
+		cfg |= JZ_LCD_CFG_DESCRIPTOR_8;
+
 	if (mode->flags & DRM_MODE_FLAG_NHSYNC)
 		cfg |= JZ_LCD_CFG_HSYNC_ACTIVE_LOW;
 	if (mode->flags & DRM_MODE_FLAG_NVSYNC)
@@ -1015,6 +1056,7 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
 	long parent_rate;
 	unsigned int i, clone_mask = 0;
 	int ret, irq;
+	u32 osdc = 0;
 
 	soc_info = of_device_get_match_data(dev);
 	if (!soc_info) {
@@ -1272,7 +1314,10 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
 
 	/* Enable OSD if available */
 	if (soc_info->has_osd)
-		regmap_write(priv->map, JZ_REG_LCD_OSDC, JZ_LCD_OSDC_OSDEN);
+		osdc |= JZ_LCD_OSDC_OSDEN;
+	if (soc_info->has_alpha)
+		osdc |= JZ_LCD_OSDC_ALPHAEN;
+	regmap_write(priv->map, JZ_REG_LCD_OSDC, osdc);
 
 	mutex_init(&priv->clk_mutex);
 	priv->clock_nb.notifier_call = ingenic_drm_update_pixclk;
@@ -1468,10 +1513,24 @@ static const struct jz_soc_info jz4770_soc_info = {
 	.num_formats_f0 = ARRAY_SIZE(jz4770_formats_f0),
 };
 
+static const struct jz_soc_info jz4780_soc_info = {
+	.needs_dev_clk = true,
+	.has_osd = true,
+	.has_alpha = true,
+	.use_extended_hwdesc = true,
+	.max_width = 4096,
+	.max_height = 2048,
+	.formats_f1 = jz4770_formats_f1,
+	.num_formats_f1 = ARRAY_SIZE(jz4770_formats_f1),
+	.formats_f0 = jz4770_formats_f0,
+	.num_formats_f0 = ARRAY_SIZE(jz4770_formats_f0),
+};
+
 static const struct of_device_id ingenic_drm_of_match[] = {
 	{ .compatible = "ingenic,jz4740-lcd", .data = &jz4740_soc_info },
 	{ .compatible = "ingenic,jz4725b-lcd", .data = &jz4725b_soc_info },
 	{ .compatible = "ingenic,jz4770-lcd", .data = &jz4770_soc_info },
+	{ .compatible = "ingenic,jz4780-lcd", .data = &jz4780_soc_info },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, ingenic_drm_of_match);
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.h b/drivers/gpu/drm/ingenic/ingenic-drm.h
index 22654ac1dde1c..cb1d09b625881 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm.h
+++ b/drivers/gpu/drm/ingenic/ingenic-drm.h
@@ -44,8 +44,11 @@
 #define JZ_REG_LCD_XYP1				0x124
 #define JZ_REG_LCD_SIZE0			0x128
 #define JZ_REG_LCD_SIZE1			0x12c
+#define JZ_REG_LCD_PCFG				0x2c0
 
 #define JZ_LCD_CFG_SLCD				BIT(31)
+#define JZ_LCD_CFG_DESCRIPTOR_8			BIT(28)
+#define JZ_LCD_CFG_RECOVER_FIFO_UNDERRUN	BIT(25)
 #define JZ_LCD_CFG_PS_DISABLE			BIT(23)
 #define JZ_LCD_CFG_CLS_DISABLE			BIT(22)
 #define JZ_LCD_CFG_SPL_DISABLE			BIT(21)
@@ -63,6 +66,7 @@
 #define JZ_LCD_CFG_DE_ACTIVE_LOW		BIT(9)
 #define JZ_LCD_CFG_VSYNC_ACTIVE_LOW		BIT(8)
 #define JZ_LCD_CFG_18_BIT			BIT(7)
+#define JZ_LCD_CFG_24_BIT			BIT(6)
 #define JZ_LCD_CFG_PDW				(BIT(5) | BIT(4))
 
 #define JZ_LCD_CFG_MODE_GENERIC_16BIT		0
@@ -132,6 +136,7 @@
 #define JZ_LCD_CMD_SOF_IRQ			BIT(31)
 #define JZ_LCD_CMD_EOF_IRQ			BIT(30)
 #define JZ_LCD_CMD_ENABLE_PAL			BIT(28)
+#define JZ_LCD_CMD_FRM_ENABLE			BIT(26)
 
 #define JZ_LCD_SYNC_MASK			0x3ff
 
@@ -153,6 +158,7 @@
 #define JZ_LCD_RGBC_EVEN_BGR			(0x5 << 0)
 
 #define JZ_LCD_OSDC_OSDEN			BIT(0)
+#define JZ_LCD_OSDC_ALPHAEN			BIT(2)
 #define JZ_LCD_OSDC_F0EN			BIT(3)
 #define JZ_LCD_OSDC_F1EN			BIT(4)
 
@@ -176,6 +182,38 @@
 #define JZ_LCD_SIZE01_WIDTH_LSB			0
 #define JZ_LCD_SIZE01_HEIGHT_LSB		16
 
+#define JZ_LCD_DESSIZE_ALPHA_OFFSET		24
+#define JZ_LCD_DESSIZE_HEIGHT_MASK		GENMASK(23, 12)
+#define JZ_LCD_DESSIZE_WIDTH_MASK		GENMASK(11, 0)
+
+#define JZ_LCD_CPOS_BPP_15_16			(4 << 27)
+#define JZ_LCD_CPOS_BPP_18_24			(5 << 27)
+#define JZ_LCD_CPOS_BPP_30			(7 << 27)
+#define JZ_LCD_CPOS_RGB555			BIT(30)
+#define JZ_LCD_CPOS_PREMULTIPLY_LCD		BIT(26)
+#define JZ_LCD_CPOS_COEFFICIENT_OFFSET		24
+#define JZ_LCD_CPOS_COEFFICIENT_0		0
+#define JZ_LCD_CPOS_COEFFICIENT_1		1
+#define JZ_LCD_CPOS_COEFFICIENT_ALPHA1		2
+#define JZ_LCD_CPOS_COEFFICIENT_1_ALPHA1	3
+
+#define JZ_LCD_RGBC_RGB_PADDING			BIT(15)
+#define JZ_LCD_RGBC_RGB_PADDING_FIRST		BIT(14)
+#define JZ_LCD_RGBC_422				BIT(8)
+#define JZ_LCD_RGBC_RGB_FORMAT_ENABLE		BIT(7)
+
+#define JZ_LCD_PCFG_PRI_MODE			BIT(31)
+#define JZ_LCD_PCFG_HP_BST_4			(0 << 28)
+#define JZ_LCD_PCFG_HP_BST_8			(1 << 28)
+#define JZ_LCD_PCFG_HP_BST_16			(2 << 28)
+#define JZ_LCD_PCFG_HP_BST_32			(3 << 28)
+#define JZ_LCD_PCFG_HP_BST_64			(4 << 28)
+#define JZ_LCD_PCFG_HP_BST_16_CONT		(5 << 28)
+#define JZ_LCD_PCFG_HP_BST_DISABLE		(7 << 28)
+#define JZ_LCD_PCFG_THRESHOLD2_OFFSET		18
+#define JZ_LCD_PCFG_THRESHOLD1_OFFSET		9
+#define JZ_LCD_PCFG_THRESHOLD0_OFFSET		0
+
 struct device;
 struct drm_plane;
 struct drm_plane_state;
-- 
2.33.0

[PATCH v12 3/9] dt-bindings: display: Add ingenic,jz4780-dw-hdmi DT Schema

[H. Nikolaus Schaller]

From: Sam Ravnborg <sam@ravnborg.org>

Add DT bindings for the hdmi driver for the Ingenic JZ4780 SoC.
Based on .txt binding from Zubair Lutfullah Kakakhel

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
---
 .../display/bridge/ingenic,jz4780-hdmi.yaml   | 83 +++++++++++++++++++
 1 file changed, 83 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/ingenic,jz4780-hdmi.yaml

diff --git a/Documentation/devicetree/bindings/display/bridge/ingenic,jz4780-hdmi.yaml b/Documentation/devicetree/bindings/display/bridge/ingenic,jz4780-hdmi.yaml
new file mode 100644
index 0000000000000..5a2767308c0ab
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/ingenic,jz4780-hdmi.yaml
@@ -0,0 +1,83 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/ingenic,jz4780-hdmi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Bindings for Ingenic JZ4780 HDMI Transmitter
+
+maintainers:
+  - H. Nikolaus Schaller <hns@goldelico.com>
+
+description: |
+  The HDMI Transmitter in the Ingenic JZ4780 is a Synopsys DesignWare HDMI 1.4
+  TX controller IP with accompanying PHY IP.
+
+allOf:
+  - $ref: synopsys,dw-hdmi.yaml#
+
+properties:
+  compatible:
+    const: ingenic,jz4780-dw-hdmi
+
+  reg-io-width:
+    const: 4
+
+  clocks:
+    maxItems: 2
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/properties/port
+        description: Input from LCD controller output.
+
+      port@1:
+        $ref: /schemas/graph.yaml#/properties/port
+        description: Link to the HDMI connector.
+
+required:
+  - compatible
+  - clocks
+  - clock-names
+  - ports
+  - reg-io-width
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/ingenic,jz4780-cgu.h>
+
+    hdmi: hdmi@10180000 {
+        compatible = "ingenic,jz4780-dw-hdmi";
+        reg = <0x10180000 0x8000>;
+        reg-io-width = <4>;
+        ddc-i2c-bus = <&i2c4>;
+        interrupt-parent = <&intc>;
+        interrupts = <3>;
+        clocks = <&cgu JZ4780_CLK_AHB0>, <&cgu JZ4780_CLK_HDMI>;
+        clock-names = "iahb", "isfr";
+        hdmi-5v-supply = <&hdmi_power>;
+
+        ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+            hdmi_in: port@0 {
+                reg = <0>;
+                dw_hdmi_in: endpoint {
+                    remote-endpoint = <&jz4780_lcd_out>;
+                };
+            };
+            hdmi_out: port@1 {
+                reg = <1>;
+                dw_hdmi_out: endpoint {
+                    remote-endpoint = <&hdmi_con>;
+                };
+            };
+        };
+    };
+
+...
-- 
2.33.0

[PATCH v12 4/9] drm/ingenic: Add dw-hdmi driver specialization for jz4780

[H. Nikolaus Schaller]

From: Paul Boddie <paul@boddie.org.uk>

A specialisation of the generic Synopsys HDMI driver is employed for
JZ4780 HDMI support. This requires a new driver, plus device tree and
configuration modifications.

Here we add Kconfig DRM_INGENIC_DW_HDMI, Makefile and driver code.

Signed-off-by: Paul Boddie <paul@boddie.org.uk>
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/gpu/drm/ingenic/Kconfig           |   9 ++
 drivers/gpu/drm/ingenic/Makefile          |   1 +
 drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c | 104 ++++++++++++++++++++++
 3 files changed, 114 insertions(+)
 create mode 100644 drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c

diff --git a/drivers/gpu/drm/ingenic/Kconfig b/drivers/gpu/drm/ingenic/Kconfig
index 001f59fb06d56..ba4a650869cd8 100644
--- a/drivers/gpu/drm/ingenic/Kconfig
+++ b/drivers/gpu/drm/ingenic/Kconfig
@@ -24,4 +24,13 @@ config DRM_INGENIC_IPU
 
 	  The Image Processing Unit (IPU) will appear as a second primary plane.
 
+config DRM_INGENIC_DW_HDMI
+	tristate "Ingenic specific support for Synopsys DW HDMI"
+	depends on MACH_JZ4780
+	select DRM_DW_HDMI
+	help
+	  Choose this option to enable Synopsys DesignWare HDMI based driver.
+	  If you want to enable HDMI on Ingenic JZ4780 based SoC, you should
+	  select this option..
+
 endif
diff --git a/drivers/gpu/drm/ingenic/Makefile b/drivers/gpu/drm/ingenic/Makefile
index d313326bdddbb..f10cc1c5a5f22 100644
--- a/drivers/gpu/drm/ingenic/Makefile
+++ b/drivers/gpu/drm/ingenic/Makefile
@@ -1,3 +1,4 @@
 obj-$(CONFIG_DRM_INGENIC) += ingenic-drm.o
 ingenic-drm-y = ingenic-drm-drv.o
 ingenic-drm-$(CONFIG_DRM_INGENIC_IPU) += ingenic-ipu.o
+obj-$(CONFIG_DRM_INGENIC_DW_HDMI) += ingenic-dw-hdmi.o
diff --git a/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c
new file mode 100644
index 0000000000000..34e986dd606cf
--- /dev/null
+++ b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2011-2013 Freescale Semiconductor, Inc.
+ * Copyright (C) 2019, 2020 Paul Boddie <paul@boddie.org.uk>
+ *
+ * Derived from dw_hdmi-imx.c with i.MX portions removed.
+ * Probe and remove operations derived from rcar_dw_hdmi.c.
+ */
+
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+#include <drm/bridge/dw_hdmi.h>
+#include <drm/drm_of.h>
+#include <drm/drm_print.h>
+
+static const struct dw_hdmi_mpll_config ingenic_mpll_cfg[] = {
+	{ 45250000,  { { 0x01e0, 0x0000 }, { 0x21e1, 0x0000 }, { 0x41e2, 0x0000 } } },
+	{ 92500000,  { { 0x0140, 0x0005 }, { 0x2141, 0x0005 }, { 0x4142, 0x0005 } } },
+	{ 148500000, { { 0x00a0, 0x000a }, { 0x20a1, 0x000a }, { 0x40a2, 0x000a } } },
+	{ 216000000, { { 0x00a0, 0x000a }, { 0x2001, 0x000f }, { 0x4002, 0x000f } } },
+	{ ~0UL,      { { 0x0000, 0x0000 }, { 0x0000, 0x0000 }, { 0x0000, 0x0000 } } }
+};
+
+static const struct dw_hdmi_curr_ctrl ingenic_cur_ctr[] = {
+	/*pixelclk     bpp8    bpp10   bpp12 */
+	{ 54000000,  { 0x091c, 0x091c, 0x06dc } },
+	{ 58400000,  { 0x091c, 0x06dc, 0x06dc } },
+	{ 72000000,  { 0x06dc, 0x06dc, 0x091c } },
+	{ 74250000,  { 0x06dc, 0x0b5c, 0x091c } },
+	{ 118800000, { 0x091c, 0x091c, 0x06dc } },
+	{ 216000000, { 0x06dc, 0x0b5c, 0x091c } },
+	{ ~0UL,      { 0x0000, 0x0000, 0x0000 } },
+};
+
+/*
+ * Resistance term 133Ohm Cfg
+ * PREEMP config 0.00
+ * TX/CK level 10
+ */
+static const struct dw_hdmi_phy_config ingenic_phy_config[] = {
+	/*pixelclk   symbol   term   vlev */
+	{ 216000000, 0x800d, 0x0005, 0x01ad},
+	{ ~0UL,      0x0000, 0x0000, 0x0000}
+};
+
+static enum drm_mode_status
+ingenic_dw_hdmi_mode_valid(struct dw_hdmi *hdmi, void *data,
+			   const struct drm_display_info *info,
+			   const struct drm_display_mode *mode)
+{
+	if (mode->clock < 13500)
+		return MODE_CLOCK_LOW;
+	/* FIXME: Hardware is capable of 270MHz, but setup data is missing. */
+	if (mode->clock > 216000)
+		return MODE_CLOCK_HIGH;
+
+	return MODE_OK;
+}
+
+static struct dw_hdmi_plat_data ingenic_dw_hdmi_plat_data = {
+	.mpll_cfg   = ingenic_mpll_cfg,
+	.cur_ctr    = ingenic_cur_ctr,
+	.phy_config = ingenic_phy_config,
+	.mode_valid = ingenic_dw_hdmi_mode_valid,
+	.output_port	= 1,
+};
+
+static const struct of_device_id ingenic_dw_hdmi_dt_ids[] = {
+	{ .compatible = "ingenic,jz4780-dw-hdmi" },
+	{ /* Sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, ingenic_dw_hdmi_dt_ids);
+
+static void ingenic_dw_hdmi_cleanup(void *data)
+{
+	struct dw_hdmi *hdmi = (struct dw_hdmi *)data;
+
+	dw_hdmi_remove(hdmi);
+}
+
+static int ingenic_dw_hdmi_probe(struct platform_device *pdev)
+{
+	struct dw_hdmi *hdmi;
+
+	hdmi = dw_hdmi_probe(pdev, &ingenic_dw_hdmi_plat_data);
+	if (IS_ERR(hdmi))
+		return PTR_ERR(hdmi);
+
+	return devm_add_action_or_reset(&pdev->dev, ingenic_dw_hdmi_cleanup, hdmi);
+}
+
+static struct platform_driver ingenic_dw_hdmi_driver = {
+	.probe  = ingenic_dw_hdmi_probe,
+	.driver = {
+		.name = "dw-hdmi-ingenic",
+		.of_match_table = ingenic_dw_hdmi_dt_ids,
+	},
+};
+module_platform_driver(ingenic_dw_hdmi_driver);
+
+MODULE_DESCRIPTION("JZ4780 Specific DW-HDMI Driver Extension");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:dwhdmi-ingenic");
-- 
2.33.0

[PATCH v12 5/9] drm/synopsys+ingenic: repair hot plug detection

[H. Nikolaus Schaller]

so that it calls drm_kms_helper_hotplug_event().

We need to set .poll_enabled but that struct component
can only be accessed in the core code. Hence we add a public
setter function.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 9 +++++++++
 drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c | 2 ++
 include/drm/bridge/dw_hdmi.h              | 1 +
 3 files changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 54d8fdad395f5..52e7cd2e020d3 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -3216,6 +3216,15 @@ static int dw_hdmi_parse_dt(struct dw_hdmi *hdmi)
 	return 0;
 }
 
+void dw_hdmi_enable_poll(struct dw_hdmi *hdmi, bool enable)
+{
+	if (hdmi->bridge.dev)
+		hdmi->bridge.dev->mode_config.poll_enabled = enable;
+	else
+		dev_warn(hdmi->dev, "no hdmi->bridge.dev");
+}
+EXPORT_SYMBOL_GPL(dw_hdmi_enable_poll);
+
 struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
 			      const struct dw_hdmi_plat_data *plat_data)
 {
diff --git a/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c
index 34e986dd606cf..90547a28dc5c7 100644
--- a/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c
+++ b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c
@@ -55,6 +55,8 @@ ingenic_dw_hdmi_mode_valid(struct dw_hdmi *hdmi, void *data,
 	if (mode->clock > 216000)
 		return MODE_CLOCK_HIGH;
 
+	dw_hdmi_enable_poll(hdmi, true);
+
 	return MODE_OK;
 }
 
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index 2a1f85f9a8a3f..963960794b40e 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -196,5 +196,6 @@ enum drm_connector_status dw_hdmi_phy_read_hpd(struct dw_hdmi *hdmi,
 void dw_hdmi_phy_update_hpd(struct dw_hdmi *hdmi, void *data,
 			    bool force, bool disabled, bool rxsense);
 void dw_hdmi_phy_setup_hpd(struct dw_hdmi *hdmi, void *data);
+void dw_hdmi_enable_poll(struct dw_hdmi *hdmi, bool enable);
 
 #endif /* __IMX_HDMI_H__ */
-- 
2.33.0

[PATCH v12 6/9] dw-hdmi/ingenic-dw-hdmi: repair interworking with hdmi-connector

[H. Nikolaus Schaller]

Commit 7cd70656d1285b ("drm/bridge: display-connector: implement bus fmts callbacks")

introduced a new mechanism to negotiate bus formats between hdmi connector
and the synopsys hdmi driver inside the jz4780.

By this, the dw-hdmi is no longer the only bridge and sets up a list
of formats in dw_hdmi_bridge_atomic_get_output_bus_fmts().

This includes MEDIA_BUS_FMT_UYVY8_1X16 which is chosen for the jz4780 but only
produces a black screen.

This fix is based on the observation that max_bpc = 0 when running this
function while info->bpc = 8. Since the formats checks before this always test
for max_bpc >= info->pbc indirectly my assumption is that we must check it
here as well.

Adding the proposed patch makes the CI20/jz4780 panel work again in
MEDIA_BUS_FMT_RGB888_1X24 mode.

Fixes: 7cd70656d1285b ("drm/bridge: display-connector: implement bus fmts callbacks")
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 52e7cd2e020d3..34703a15ee4ff 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2620,10 +2620,10 @@ static u32 *dw_hdmi_bridge_atomic_get_output_bus_fmts(struct drm_bridge *bridge,
 		output_fmts[i++] = MEDIA_BUS_FMT_RGB101010_1X30;
 	}
 
-	if (info->color_formats & DRM_COLOR_FORMAT_YCRCB422)
+	if (max_bpc >= info->bpc && info->color_formats & DRM_COLOR_FORMAT_YCRCB422)
 		output_fmts[i++] = MEDIA_BUS_FMT_UYVY8_1X16;
 
-	if (info->color_formats & DRM_COLOR_FORMAT_YCRCB444)
+	if (max_bpc >= info->bpc && info->color_formats & DRM_COLOR_FORMAT_YCRCB444)
 		output_fmts[i++] = MEDIA_BUS_FMT_YUV8_1X24;
 
 	/* Default 8bit RGB fallback */
-- 
2.33.0

[PATCH v12 7/9] drm/bridge: display-connector: add ddc-en gpio support

[H. Nikolaus Schaller]

"hdmi-connector.yaml" bindings defines an optional property
"ddc-en-gpios" for a single gpio to enable DDC operation.

Usually this controls +5V power on the HDMI connector.
This +5V may also be needed for HPD.

This was not reflected in code.

Now, the driver activates the ddc gpio after probe and
deactivates after remove so it is "almost on".

But only if this driver is loaded (and not e.g. blacklisted
as module).

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/gpu/drm/bridge/display-connector.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c
index d24f5b90feabf..555395e301096 100644
--- a/drivers/gpu/drm/bridge/display-connector.c
+++ b/drivers/gpu/drm/bridge/display-connector.c
@@ -24,6 +24,7 @@ struct display_connector {
 	int			hpd_irq;
 
 	struct regulator	*dp_pwr;
+	struct gpio_desc	*ddc_en;
 };
 
 static inline struct display_connector *
@@ -345,6 +346,19 @@ static int display_connector_probe(struct platform_device *pdev)
 		}
 	}
 
+	/* enable DDC */
+	if (type == DRM_MODE_CONNECTOR_HDMIA) {
+		conn->ddc_en = devm_gpiod_get_optional(&pdev->dev, "ddc-en",
+						       GPIOD_OUT_HIGH);
+
+		if (IS_ERR(conn->ddc_en)) {
+			dev_err(&pdev->dev, "Couldn't get ddc-en gpio\n");
+			return PTR_ERR(conn->ddc_en);
+		}
+
+		gpiod_set_value(conn->ddc_en, 1);
+	}
+
 	conn->bridge.funcs = &display_connector_bridge_funcs;
 	conn->bridge.of_node = pdev->dev.of_node;
 
@@ -373,6 +387,9 @@ static int display_connector_remove(struct platform_device *pdev)
 {
 	struct display_connector *conn = platform_get_drvdata(pdev);
 
+	if (conn->ddc_en)
+		gpiod_set_value(conn->ddc_en, 0);
+
 	if (conn->dp_pwr)
 		regulator_disable(conn->dp_pwr);
 
-- 
2.33.0

[PATCH v12 8/9] MIPS: DTS: CI20: fix how ddc power is enabled

[H. Nikolaus Schaller]

Originally we proposed a new hdmi-5v-supply regulator reference
for CI20 device tree but that was superseded by a better idea to use
the already defined "ddc-en-gpios" property of the "hdmi-connector".

Since "MIPS: DTS: CI20: Add DT nodes for HDMI setup" has already
been applied to v5.17-rc1, we add this on top.

Fixes: ae1b8d2c2de9 ("MIPS: DTS: CI20: Add DT nodes for HDMI setup")
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 arch/mips/boot/dts/ingenic/ci20.dts | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/arch/mips/boot/dts/ingenic/ci20.dts b/arch/mips/boot/dts/ingenic/ci20.dts
index 3e336b3dbb109..ab6e3dc0bc1d0 100644
--- a/arch/mips/boot/dts/ingenic/ci20.dts
+++ b/arch/mips/boot/dts/ingenic/ci20.dts
@@ -83,6 +83,8 @@ hdmi_out: connector {
 		label = "HDMI OUT";
 		type = "a";
 
+		ddc-en-gpios = <&gpa 25 GPIO_ACTIVE_HIGH>;
+
 		port {
 			hdmi_con: endpoint {
 				remote-endpoint = <&dw_hdmi_out>;
@@ -114,17 +116,6 @@ otg_power: fixedregulator@2 {
 		gpio = <&gpf 14 GPIO_ACTIVE_LOW>;
 		enable-active-high;
 	};
-
-	hdmi_power: fixedregulator@3 {
-		compatible = "regulator-fixed";
-
-		regulator-name = "hdmi_power";
-		regulator-min-microvolt = <5000000>;
-		regulator-max-microvolt = <5000000>;
-
-		gpio = <&gpa 25 0>;
-		enable-active-high;
-	};
 };
 
 &ext {
@@ -576,8 +567,6 @@ &hdmi {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pins_hdmi_ddc>;
 
-	hdmi-5v-supply = <&hdmi_power>;
-
 	ports {
 		#address-cells = <1>;
 		#size-cells = <0>;
-- 
2.33.0

Re: [PATCH v12 3/9] dt-bindings: display: Add ingenic,jz4780-dw-hdmi DT Schema

[Rob Herring]

On Mon, 31 Jan 2022 13:26:49 +0100, H. Nikolaus Schaller wrote:
> From: Sam Ravnborg <sam@ravnborg.org>
> 
> Add DT bindings for the hdmi driver for the Ingenic JZ4780 SoC.
> Based on .txt binding from Zubair Lutfullah Kakakhel
> 
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: devicetree@vger.kernel.org
> ---
>  .../display/bridge/ingenic,jz4780-hdmi.yaml   | 83 +++++++++++++++++++
>  1 file changed, 83 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/ingenic,jz4780-hdmi.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v12 3/9] dt-bindings: display: Add ingenic,jz4780-dw-hdmi DT Schema

[Paul Cercueil]

Hi Nikolaus,

Le lun., janv. 31 2022 at 13:26:49 +0100, H. Nikolaus Schaller 
<hns@goldelico.com> a écrit :
> From: Sam Ravnborg <sam@ravnborg.org>
> 
> Add DT bindings for the hdmi driver for the Ingenic JZ4780 SoC.
> Based on .txt binding from Zubair Lutfullah Kakakhel
> 
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: devicetree@vger.kernel.org
> ---
>  .../display/bridge/ingenic,jz4780-hdmi.yaml   | 83 
> +++++++++++++++++++
>  1 file changed, 83 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/bridge/ingenic,jz4780-hdmi.yaml
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/bridge/ingenic,jz4780-hdmi.yaml 
> b/Documentation/devicetree/bindings/display/bridge/ingenic,jz4780-hdmi.yaml
> new file mode 100644
> index 0000000000000..5a2767308c0ab
> --- /dev/null
> +++ 
> b/Documentation/devicetree/bindings/display/bridge/ingenic,jz4780-hdmi.yaml
> @@ -0,0 +1,83 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: 
> http://devicetree.org/schemas/display/bridge/ingenic,jz4780-hdmi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Bindings for Ingenic JZ4780 HDMI Transmitter
> +
> +maintainers:
> +  - H. Nikolaus Schaller <hns@goldelico.com>
> +
> +description: |
> +  The HDMI Transmitter in the Ingenic JZ4780 is a Synopsys 
> DesignWare HDMI 1.4
> +  TX controller IP with accompanying PHY IP.
> +
> +allOf:
> +  - $ref: synopsys,dw-hdmi.yaml#
> +
> +properties:
> +  compatible:
> +    const: ingenic,jz4780-dw-hdmi
> +
> +  reg-io-width:
> +    const: 4
> +
> +  clocks:
> +    maxItems: 2
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description: Input from LCD controller output.
> +
> +      port@1:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description: Link to the HDMI connector.
> +
> +required:
> +  - compatible
> +  - clocks
> +  - clock-names
> +  - ports
> +  - reg-io-width
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/ingenic,jz4780-cgu.h>
> +
> +    hdmi: hdmi@10180000 {
> +        compatible = "ingenic,jz4780-dw-hdmi";
> +        reg = <0x10180000 0x8000>;
> +        reg-io-width = <4>;
> +        ddc-i2c-bus = <&i2c4>;
> +        interrupt-parent = <&intc>;
> +        interrupts = <3>;
> +        clocks = <&cgu JZ4780_CLK_AHB0>, <&cgu JZ4780_CLK_HDMI>;
> +        clock-names = "iahb", "isfr";
> +        hdmi-5v-supply = <&hdmi_power>;

Where is this property defined?

Cheers,
-Paul

> +
> +        ports {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            hdmi_in: port@0 {
> +                reg = <0>;
> +                dw_hdmi_in: endpoint {
> +                    remote-endpoint = <&jz4780_lcd_out>;
> +                };
> +            };
> +            hdmi_out: port@1 {
> +                reg = <1>;
> +                dw_hdmi_out: endpoint {
> +                    remote-endpoint = <&hdmi_con>;
> +                };
> +            };
> +        };
> +    };
> +
> +...
> --
> 2.33.0
> 

Re: [PATCH v12 4/9] drm/ingenic: Add dw-hdmi driver specialization for jz4780

[Paul Cercueil]

Hi Nikolaus,

Le lun., janv. 31 2022 at 13:26:50 +0100, H. Nikolaus Schaller 
<hns@goldelico.com> a écrit :
> From: Paul Boddie <paul@boddie.org.uk>
> 
> A specialisation of the generic Synopsys HDMI driver is employed for
> JZ4780 HDMI support. This requires a new driver, plus device tree and
> configuration modifications.
> 
> Here we add Kconfig DRM_INGENIC_DW_HDMI, Makefile and driver code.
> 
> Signed-off-by: Paul Boddie <paul@boddie.org.uk>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
>  drivers/gpu/drm/ingenic/Kconfig           |   9 ++
>  drivers/gpu/drm/ingenic/Makefile          |   1 +
>  drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c | 104 
> ++++++++++++++++++++++
>  3 files changed, 114 insertions(+)
>  create mode 100644 drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c
> 
> diff --git a/drivers/gpu/drm/ingenic/Kconfig 
> b/drivers/gpu/drm/ingenic/Kconfig
> index 001f59fb06d56..ba4a650869cd8 100644
> --- a/drivers/gpu/drm/ingenic/Kconfig
> +++ b/drivers/gpu/drm/ingenic/Kconfig
> @@ -24,4 +24,13 @@ config DRM_INGENIC_IPU
> 
>  	  The Image Processing Unit (IPU) will appear as a second primary 
> plane.
> 
> +config DRM_INGENIC_DW_HDMI
> +	tristate "Ingenic specific support for Synopsys DW HDMI"
> +	depends on MACH_JZ4780
> +	select DRM_DW_HDMI
> +	help
> +	  Choose this option to enable Synopsys DesignWare HDMI based 
> driver.
> +	  If you want to enable HDMI on Ingenic JZ4780 based SoC, you should
> +	  select this option..

One dot is enough.

> +
>  endif
> diff --git a/drivers/gpu/drm/ingenic/Makefile 
> b/drivers/gpu/drm/ingenic/Makefile
> index d313326bdddbb..f10cc1c5a5f22 100644
> --- a/drivers/gpu/drm/ingenic/Makefile
> +++ b/drivers/gpu/drm/ingenic/Makefile
> @@ -1,3 +1,4 @@
>  obj-$(CONFIG_DRM_INGENIC) += ingenic-drm.o
>  ingenic-drm-y = ingenic-drm-drv.o
>  ingenic-drm-$(CONFIG_DRM_INGENIC_IPU) += ingenic-ipu.o
> +obj-$(CONFIG_DRM_INGENIC_DW_HDMI) += ingenic-dw-hdmi.o
> diff --git a/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c 
> b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c
> new file mode 100644
> index 0000000000000..34e986dd606cf
> --- /dev/null
> +++ b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c
> @@ -0,0 +1,104 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (C) 2011-2013 Freescale Semiconductor, Inc.
> + * Copyright (C) 2019, 2020 Paul Boddie <paul@boddie.org.uk>
> + *
> + * Derived from dw_hdmi-imx.c with i.MX portions removed.
> + * Probe and remove operations derived from rcar_dw_hdmi.c.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +
> +#include <drm/bridge/dw_hdmi.h>
> +#include <drm/drm_of.h>
> +#include <drm/drm_print.h>
> +
> +static const struct dw_hdmi_mpll_config ingenic_mpll_cfg[] = {
> +	{ 45250000,  { { 0x01e0, 0x0000 }, { 0x21e1, 0x0000 }, { 0x41e2, 
> 0x0000 } } },
> +	{ 92500000,  { { 0x0140, 0x0005 }, { 0x2141, 0x0005 }, { 0x4142, 
> 0x0005 } } },
> +	{ 148500000, { { 0x00a0, 0x000a }, { 0x20a1, 0x000a }, { 0x40a2, 
> 0x000a } } },
> +	{ 216000000, { { 0x00a0, 0x000a }, { 0x2001, 0x000f }, { 0x4002, 
> 0x000f } } },
> +	{ ~0UL,      { { 0x0000, 0x0000 }, { 0x0000, 0x0000 }, { 0x0000, 
> 0x0000 } } }
> +};
> +
> +static const struct dw_hdmi_curr_ctrl ingenic_cur_ctr[] = {
> +	/*pixelclk     bpp8    bpp10   bpp12 */
> +	{ 54000000,  { 0x091c, 0x091c, 0x06dc } },
> +	{ 58400000,  { 0x091c, 0x06dc, 0x06dc } },
> +	{ 72000000,  { 0x06dc, 0x06dc, 0x091c } },
> +	{ 74250000,  { 0x06dc, 0x0b5c, 0x091c } },
> +	{ 118800000, { 0x091c, 0x091c, 0x06dc } },
> +	{ 216000000, { 0x06dc, 0x0b5c, 0x091c } },
> +	{ ~0UL,      { 0x0000, 0x0000, 0x0000 } },
> +};
> +
> +/*
> + * Resistance term 133Ohm Cfg
> + * PREEMP config 0.00
> + * TX/CK level 10
> + */
> +static const struct dw_hdmi_phy_config ingenic_phy_config[] = {
> +	/*pixelclk   symbol   term   vlev */
> +	{ 216000000, 0x800d, 0x0005, 0x01ad},
> +	{ ~0UL,      0x0000, 0x0000, 0x0000}
> +};
> +
> +static enum drm_mode_status
> +ingenic_dw_hdmi_mode_valid(struct dw_hdmi *hdmi, void *data,
> +			   const struct drm_display_info *info,
> +			   const struct drm_display_mode *mode)
> +{
> +	if (mode->clock < 13500)
> +		return MODE_CLOCK_LOW;
> +	/* FIXME: Hardware is capable of 270MHz, but setup data is missing. 
> */
> +	if (mode->clock > 216000)
> +		return MODE_CLOCK_HIGH;
> +
> +	return MODE_OK;
> +}
> +
> +static struct dw_hdmi_plat_data ingenic_dw_hdmi_plat_data = {
> +	.mpll_cfg   = ingenic_mpll_cfg,
> +	.cur_ctr    = ingenic_cur_ctr,
> +	.phy_config = ingenic_phy_config,
> +	.mode_valid = ingenic_dw_hdmi_mode_valid,
> +	.output_port	= 1,
> +};
> +
> +static const struct of_device_id ingenic_dw_hdmi_dt_ids[] = {
> +	{ .compatible = "ingenic,jz4780-dw-hdmi" },
> +	{ /* Sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, ingenic_dw_hdmi_dt_ids);
> +
> +static void ingenic_dw_hdmi_cleanup(void *data)
> +{
> +	struct dw_hdmi *hdmi = (struct dw_hdmi *)data;
> +
> +	dw_hdmi_remove(hdmi);
> +}
> +
> +static int ingenic_dw_hdmi_probe(struct platform_device *pdev)
> +{
> +	struct dw_hdmi *hdmi;
> +
> +	hdmi = dw_hdmi_probe(pdev, &ingenic_dw_hdmi_plat_data);
> +	if (IS_ERR(hdmi))
> +		return PTR_ERR(hdmi);
> +
> +	return devm_add_action_or_reset(&pdev->dev, 
> ingenic_dw_hdmi_cleanup, hdmi);

Nitpick, but your probe function is so simple, you could just have a 
.remove callback instead of registering a devm action. Then you can 
just return PTR_ERR_OR_ZERO(hdmi).

Cheers,
-Paul

> +}
> +
> +static struct platform_driver ingenic_dw_hdmi_driver = {
> +	.probe  = ingenic_dw_hdmi_probe,
> +	.driver = {
> +		.name = "dw-hdmi-ingenic",
> +		.of_match_table = ingenic_dw_hdmi_dt_ids,
> +	},
> +};
> +module_platform_driver(ingenic_dw_hdmi_driver);
> +
> +MODULE_DESCRIPTION("JZ4780 Specific DW-HDMI Driver Extension");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:dwhdmi-ingenic");
> --
> 2.33.0
> 

Re: [PATCH v12 2/9] drm/ingenic: Add support for JZ4780 and HDMI output

[Paul Cercueil]

Hi Nikolaus,

Le lun., janv. 31 2022 at 13:26:48 +0100, H. Nikolaus Schaller 
<hns@goldelico.com> a écrit :
> From: Paul Boddie <paul@boddie.org.uk>
> 
> Add support for the LCD controller present on JZ4780 SoCs.
> This SoC uses 8-byte descriptors which extend the current
> 4-byte descriptors used for other Ingenic SoCs.
> 
> Tested on MIPS Creator CI20 board.
> 
> Signed-off-by: Paul Boddie <paul@boddie.org.uk>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
>  drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 61 
> ++++++++++++++++++++++-
>  drivers/gpu/drm/ingenic/ingenic-drm.h     | 38 ++++++++++++++
>  2 files changed, 98 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c 
> b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> index 9c60fc4605e4b..ccdb9eedd9247 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> @@ -6,6 +6,7 @@
> 
>  #include "ingenic-drm.h"
> 
> +#include <linux/bitfield.h>
>  #include <linux/component.h>
>  #include <linux/clk.h>
>  #include <linux/dma-mapping.h>
> @@ -49,6 +50,11 @@ struct ingenic_dma_hwdesc {
>  	u32 addr;
>  	u32 id;
>  	u32 cmd;
> +	/* extended hw descriptor for jz4780 */
> +	u32 offsize;
> +	u32 pagewidth;
> +	u32 cpos;
> +	u32 dessize;
>  } __aligned(16);
> 
>  struct ingenic_dma_hwdescs {
> @@ -59,7 +65,9 @@ struct ingenic_dma_hwdescs {
>  struct jz_soc_info {
>  	bool needs_dev_clk;
>  	bool has_osd;
> +	bool has_alpha;
>  	bool map_noncoherent;
> +	bool use_extended_hwdesc;
>  	unsigned int max_width, max_height;
>  	const u32 *formats_f0, *formats_f1;
>  	unsigned int num_formats_f0, num_formats_f1;
> @@ -446,6 +454,9 @@ static int ingenic_drm_plane_atomic_check(struct 
> drm_plane *plane,
>  	if (!crtc)
>  		return 0;
> 
> +	if (plane == &priv->f0)
> +		return -EINVAL;

This will break JZ4725B -> JZ4770 SoCs, the f0 plane is perfectly 
usable there.

Cheers,
-Paul

> +
>  	crtc_state = drm_atomic_get_existing_crtc_state(state,
>  							crtc);
>  	if (WARN_ON(!crtc_state))
> @@ -662,6 +673,33 @@ static void 
> ingenic_drm_plane_atomic_update(struct drm_plane *plane,
>  		hwdesc->cmd = JZ_LCD_CMD_EOF_IRQ | (width * height * cpp / 4);
>  		hwdesc->next = dma_hwdesc_addr(priv, next_id);
> 
> +		if (priv->soc_info->use_extended_hwdesc) {
> +			hwdesc->cmd |= JZ_LCD_CMD_FRM_ENABLE;
> +
> +			/* Extended 8-byte descriptor */
> +			hwdesc->cpos = 0;
> +			hwdesc->offsize = 0;
> +			hwdesc->pagewidth = 0;
> +
> +			switch (newstate->fb->format->format) {
> +			case DRM_FORMAT_XRGB1555:
> +				hwdesc->cpos |= JZ_LCD_CPOS_RGB555;
> +				fallthrough;
> +			case DRM_FORMAT_RGB565:
> +				hwdesc->cpos |= JZ_LCD_CPOS_BPP_15_16;
> +				break;
> +			case DRM_FORMAT_XRGB8888:
> +				hwdesc->cpos |= JZ_LCD_CPOS_BPP_18_24;
> +				break;
> +			}
> +			hwdesc->cpos |= (JZ_LCD_CPOS_COEFFICIENT_1 <<
> +					 JZ_LCD_CPOS_COEFFICIENT_OFFSET);
> +			hwdesc->dessize =
> +				(0xff << JZ_LCD_DESSIZE_ALPHA_OFFSET) |
> +				FIELD_PREP(JZ_LCD_DESSIZE_HEIGHT_MASK, height - 1) |
> +				FIELD_PREP(JZ_LCD_DESSIZE_WIDTH_MASK, width - 1);
> +		}
> +
>  		if (drm_atomic_crtc_needs_modeset(crtc_state)) {
>  			fourcc = newstate->fb->format->format;
> 
> @@ -693,6 +731,9 @@ static void 
> ingenic_drm_encoder_atomic_mode_set(struct drm_encoder *encoder,
>  		    | JZ_LCD_CFG_SPL_DISABLE | JZ_LCD_CFG_REV_DISABLE;
>  	}
> 
> +	if (priv->soc_info->use_extended_hwdesc)
> +		cfg |= JZ_LCD_CFG_DESCRIPTOR_8;
> +
>  	if (mode->flags & DRM_MODE_FLAG_NHSYNC)
>  		cfg |= JZ_LCD_CFG_HSYNC_ACTIVE_LOW;
>  	if (mode->flags & DRM_MODE_FLAG_NVSYNC)
> @@ -1015,6 +1056,7 @@ static int ingenic_drm_bind(struct device *dev, 
> bool has_components)
>  	long parent_rate;
>  	unsigned int i, clone_mask = 0;
>  	int ret, irq;
> +	u32 osdc = 0;
> 
>  	soc_info = of_device_get_match_data(dev);
>  	if (!soc_info) {
> @@ -1272,7 +1314,10 @@ static int ingenic_drm_bind(struct device 
> *dev, bool has_components)
> 
>  	/* Enable OSD if available */
>  	if (soc_info->has_osd)
> -		regmap_write(priv->map, JZ_REG_LCD_OSDC, JZ_LCD_OSDC_OSDEN);
> +		osdc |= JZ_LCD_OSDC_OSDEN;
> +	if (soc_info->has_alpha)
> +		osdc |= JZ_LCD_OSDC_ALPHAEN;
> +	regmap_write(priv->map, JZ_REG_LCD_OSDC, osdc);
> 
>  	mutex_init(&priv->clk_mutex);
>  	priv->clock_nb.notifier_call = ingenic_drm_update_pixclk;
> @@ -1468,10 +1513,24 @@ static const struct jz_soc_info 
> jz4770_soc_info = {
>  	.num_formats_f0 = ARRAY_SIZE(jz4770_formats_f0),
>  };
> 
> +static const struct jz_soc_info jz4780_soc_info = {
> +	.needs_dev_clk = true,
> +	.has_osd = true,
> +	.has_alpha = true,
> +	.use_extended_hwdesc = true,
> +	.max_width = 4096,
> +	.max_height = 2048,
> +	.formats_f1 = jz4770_formats_f1,
> +	.num_formats_f1 = ARRAY_SIZE(jz4770_formats_f1),
> +	.formats_f0 = jz4770_formats_f0,
> +	.num_formats_f0 = ARRAY_SIZE(jz4770_formats_f0),
> +};
> +
>  static const struct of_device_id ingenic_drm_of_match[] = {
>  	{ .compatible = "ingenic,jz4740-lcd", .data = &jz4740_soc_info },
>  	{ .compatible = "ingenic,jz4725b-lcd", .data = &jz4725b_soc_info },
>  	{ .compatible = "ingenic,jz4770-lcd", .data = &jz4770_soc_info },
> +	{ .compatible = "ingenic,jz4780-lcd", .data = &jz4780_soc_info },
>  	{ /* sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(of, ingenic_drm_of_match);
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.h 
> b/drivers/gpu/drm/ingenic/ingenic-drm.h
> index 22654ac1dde1c..cb1d09b625881 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm.h
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm.h
> @@ -44,8 +44,11 @@
>  #define JZ_REG_LCD_XYP1				0x124
>  #define JZ_REG_LCD_SIZE0			0x128
>  #define JZ_REG_LCD_SIZE1			0x12c
> +#define JZ_REG_LCD_PCFG				0x2c0
> 
>  #define JZ_LCD_CFG_SLCD				BIT(31)
> +#define JZ_LCD_CFG_DESCRIPTOR_8			BIT(28)
> +#define JZ_LCD_CFG_RECOVER_FIFO_UNDERRUN	BIT(25)
>  #define JZ_LCD_CFG_PS_DISABLE			BIT(23)
>  #define JZ_LCD_CFG_CLS_DISABLE			BIT(22)
>  #define JZ_LCD_CFG_SPL_DISABLE			BIT(21)
> @@ -63,6 +66,7 @@
>  #define JZ_LCD_CFG_DE_ACTIVE_LOW		BIT(9)
>  #define JZ_LCD_CFG_VSYNC_ACTIVE_LOW		BIT(8)
>  #define JZ_LCD_CFG_18_BIT			BIT(7)
> +#define JZ_LCD_CFG_24_BIT			BIT(6)
>  #define JZ_LCD_CFG_PDW				(BIT(5) | BIT(4))
> 
>  #define JZ_LCD_CFG_MODE_GENERIC_16BIT		0
> @@ -132,6 +136,7 @@
>  #define JZ_LCD_CMD_SOF_IRQ			BIT(31)
>  #define JZ_LCD_CMD_EOF_IRQ			BIT(30)
>  #define JZ_LCD_CMD_ENABLE_PAL			BIT(28)
> +#define JZ_LCD_CMD_FRM_ENABLE			BIT(26)
> 
>  #define JZ_LCD_SYNC_MASK			0x3ff
> 
> @@ -153,6 +158,7 @@
>  #define JZ_LCD_RGBC_EVEN_BGR			(0x5 << 0)
> 
>  #define JZ_LCD_OSDC_OSDEN			BIT(0)
> +#define JZ_LCD_OSDC_ALPHAEN			BIT(2)
>  #define JZ_LCD_OSDC_F0EN			BIT(3)
>  #define JZ_LCD_OSDC_F1EN			BIT(4)
> 
> @@ -176,6 +182,38 @@
>  #define JZ_LCD_SIZE01_WIDTH_LSB			0
>  #define JZ_LCD_SIZE01_HEIGHT_LSB		16
> 
> +#define JZ_LCD_DESSIZE_ALPHA_OFFSET		24
> +#define JZ_LCD_DESSIZE_HEIGHT_MASK		GENMASK(23, 12)
> +#define JZ_LCD_DESSIZE_WIDTH_MASK		GENMASK(11, 0)
> +
> +#define JZ_LCD_CPOS_BPP_15_16			(4 << 27)
> +#define JZ_LCD_CPOS_BPP_18_24			(5 << 27)
> +#define JZ_LCD_CPOS_BPP_30			(7 << 27)
> +#define JZ_LCD_CPOS_RGB555			BIT(30)
> +#define JZ_LCD_CPOS_PREMULTIPLY_LCD		BIT(26)
> +#define JZ_LCD_CPOS_COEFFICIENT_OFFSET		24
> +#define JZ_LCD_CPOS_COEFFICIENT_0		0
> +#define JZ_LCD_CPOS_COEFFICIENT_1		1
> +#define JZ_LCD_CPOS_COEFFICIENT_ALPHA1		2
> +#define JZ_LCD_CPOS_COEFFICIENT_1_ALPHA1	3
> +
> +#define JZ_LCD_RGBC_RGB_PADDING			BIT(15)
> +#define JZ_LCD_RGBC_RGB_PADDING_FIRST		BIT(14)
> +#define JZ_LCD_RGBC_422				BIT(8)
> +#define JZ_LCD_RGBC_RGB_FORMAT_ENABLE		BIT(7)
> +
> +#define JZ_LCD_PCFG_PRI_MODE			BIT(31)
> +#define JZ_LCD_PCFG_HP_BST_4			(0 << 28)
> +#define JZ_LCD_PCFG_HP_BST_8			(1 << 28)
> +#define JZ_LCD_PCFG_HP_BST_16			(2 << 28)
> +#define JZ_LCD_PCFG_HP_BST_32			(3 << 28)
> +#define JZ_LCD_PCFG_HP_BST_64			(4 << 28)
> +#define JZ_LCD_PCFG_HP_BST_16_CONT		(5 << 28)
> +#define JZ_LCD_PCFG_HP_BST_DISABLE		(7 << 28)
> +#define JZ_LCD_PCFG_THRESHOLD2_OFFSET		18
> +#define JZ_LCD_PCFG_THRESHOLD1_OFFSET		9
> +#define JZ_LCD_PCFG_THRESHOLD0_OFFSET		0
> +
>  struct device;
>  struct drm_plane;
>  struct drm_plane_state;
> --
> 2.33.0
> 

Re: [PATCH v12 7/9] drm/bridge: display-connector: add ddc-en gpio support

[Paul Cercueil]

Hi Nikolaus,

Le lun., janv. 31 2022 at 13:26:53 +0100, H. Nikolaus Schaller 
<hns@goldelico.com> a écrit :
> "hdmi-connector.yaml" bindings defines an optional property
> "ddc-en-gpios" for a single gpio to enable DDC operation.
> 
> Usually this controls +5V power on the HDMI connector.
> This +5V may also be needed for HPD.
> 
> This was not reflected in code.
> 
> Now, the driver activates the ddc gpio after probe and
> deactivates after remove so it is "almost on".
> 
> But only if this driver is loaded (and not e.g. blacklisted
> as module).
> 
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
>  drivers/gpu/drm/bridge/display-connector.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/display-connector.c 
> b/drivers/gpu/drm/bridge/display-connector.c
> index d24f5b90feabf..555395e301096 100644
> --- a/drivers/gpu/drm/bridge/display-connector.c
> +++ b/drivers/gpu/drm/bridge/display-connector.c
> @@ -24,6 +24,7 @@ struct display_connector {
>  	int			hpd_irq;
> 
>  	struct regulator	*dp_pwr;
> +	struct gpio_desc	*ddc_en;
>  };
> 
>  static inline struct display_connector *
> @@ -345,6 +346,19 @@ static int display_connector_probe(struct 
> platform_device *pdev)
>  		}
>  	}
> 
> +	/* enable DDC */
> +	if (type == DRM_MODE_CONNECTOR_HDMIA) {
> +		conn->ddc_en = devm_gpiod_get_optional(&pdev->dev, "ddc-en",
> +						       GPIOD_OUT_HIGH);
> +
> +		if (IS_ERR(conn->ddc_en)) {
> +			dev_err(&pdev->dev, "Couldn't get ddc-en gpio\n");
> +			return PTR_ERR(conn->ddc_en);
> +		}
> +
> +		gpiod_set_value(conn->ddc_en, 1);

You already requested the gpio with the GPIOD_OUT_HIGH flag, so this 
can be removed.


> +	}
> +
>  	conn->bridge.funcs = &display_connector_bridge_funcs;
>  	conn->bridge.of_node = pdev->dev.of_node;
> 
> @@ -373,6 +387,9 @@ static int display_connector_remove(struct 
> platform_device *pdev)
>  {
>  	struct display_connector *conn = platform_get_drvdata(pdev);
> 
> +	if (conn->ddc_en)
> +		gpiod_set_value(conn->ddc_en, 0);

Note that gpiod_set_value() already does the null-check internally. I 
actually do prefer your solution, so this is fine with me, but 
maintainers may have a different opinion.

Cheers,
-Paul

> +
>  	if (conn->dp_pwr)
>  		regulator_disable(conn->dp_pwr);
> 
> --
> 2.33.0
> 

Re: [PATCH v12 8/9] MIPS: DTS: CI20: fix how ddc power is enabled

[Paul Cercueil]

Le lun., janv. 31 2022 at 13:26:54 +0100, H. Nikolaus Schaller 
<hns@goldelico.com> a écrit :
> Originally we proposed a new hdmi-5v-supply regulator reference
> for CI20 device tree but that was superseded by a better idea to use
> the already defined "ddc-en-gpios" property of the "hdmi-connector".
> 
> Since "MIPS: DTS: CI20: Add DT nodes for HDMI setup" has already
> been applied to v5.17-rc1, we add this on top.
> 
> Fixes: ae1b8d2c2de9 ("MIPS: DTS: CI20: Add DT nodes for HDMI setup")
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>

Reviewed-by: Paul Cercueil <paul@crapouillou.net>

Cheers,
-Paul

> ---
>  arch/mips/boot/dts/ingenic/ci20.dts | 15 ++-------------
>  1 file changed, 2 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/mips/boot/dts/ingenic/ci20.dts 
> b/arch/mips/boot/dts/ingenic/ci20.dts
> index 3e336b3dbb109..ab6e3dc0bc1d0 100644
> --- a/arch/mips/boot/dts/ingenic/ci20.dts
> +++ b/arch/mips/boot/dts/ingenic/ci20.dts
> @@ -83,6 +83,8 @@ hdmi_out: connector {
>  		label = "HDMI OUT";
>  		type = "a";
> 
> +		ddc-en-gpios = <&gpa 25 GPIO_ACTIVE_HIGH>;
> +
>  		port {
>  			hdmi_con: endpoint {
>  				remote-endpoint = <&dw_hdmi_out>;
> @@ -114,17 +116,6 @@ otg_power: fixedregulator@2 {
>  		gpio = <&gpf 14 GPIO_ACTIVE_LOW>;
>  		enable-active-high;
>  	};
> -
> -	hdmi_power: fixedregulator@3 {
> -		compatible = "regulator-fixed";
> -
> -		regulator-name = "hdmi_power";
> -		regulator-min-microvolt = <5000000>;
> -		regulator-max-microvolt = <5000000>;
> -
> -		gpio = <&gpa 25 0>;
> -		enable-active-high;
> -	};
>  };
> 
>  &ext {
> @@ -576,8 +567,6 @@ &hdmi {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pins_hdmi_ddc>;
> 
> -	hdmi-5v-supply = <&hdmi_power>;
> -
>  	ports {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> --
> 2.33.0
> 

Re: [PATCH v12 4/9] drm/ingenic: Add dw-hdmi driver specialization for jz4780

[H. Nikolaus Schaller]

Hi Paul,

> Am 02.02.2022 um 11:16 schrieb Paul Cercueil <paul@crapouillou.net>:
> 
> Hi Nikolaus,
> 
> Le lun., janv. 31 2022 at 13:26:50 +0100, H. Nikolaus Schaller <hns@goldelico.com> a écrit :
>> From: Paul Boddie <paul@boddie.org.uk>
>> A specialisation of the generic Synopsys HDMI driver is employed for
>> JZ4780 HDMI support. This requires a new driver, plus device tree and
>> configuration modifications.
>> Here we add Kconfig DRM_INGENIC_DW_HDMI, Makefile and driver code.
>> Signed-off-by: Paul Boddie <paul@boddie.org.uk>
>> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> ---
>> drivers/gpu/drm/ingenic/Kconfig           |   9 ++
>> drivers/gpu/drm/ingenic/Makefile          |   1 +
>> drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c | 104 ++++++++++++++++++++++
>> 3 files changed, 114 insertions(+)
>> create mode 100644 drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c
>> diff --git a/drivers/gpu/drm/ingenic/Kconfig b/drivers/gpu/drm/ingenic/Kconfig
>> index 001f59fb06d56..ba4a650869cd8 100644
>> --- a/drivers/gpu/drm/ingenic/Kconfig
>> +++ b/drivers/gpu/drm/ingenic/Kconfig
>> @@ -24,4 +24,13 @@ config DRM_INGENIC_IPU
>> 	  The Image Processing Unit (IPU) will appear as a second primary plane.
>> +config DRM_INGENIC_DW_HDMI
>> +	tristate "Ingenic specific support for Synopsys DW HDMI"
>> +	depends on MACH_JZ4780
>> +	select DRM_DW_HDMI
>> +	help
>> +	  Choose this option to enable Synopsys DesignWare HDMI based driver.
>> +	  If you want to enable HDMI on Ingenic JZ4780 based SoC, you should
>> +	  select this option..
> 
> One dot is enough.

Ok, will fix.

> 
>> +
>> endif
>> diff --git a/drivers/gpu/drm/ingenic/Makefile b/drivers/gpu/drm/ingenic/Makefile
>> index d313326bdddbb..f10cc1c5a5f22 100644
>> --- a/drivers/gpu/drm/ingenic/Makefile
>> +++ b/drivers/gpu/drm/ingenic/Makefile
>> @@ -1,3 +1,4 @@
>> obj-$(CONFIG_DRM_INGENIC) += ingenic-drm.o
>> ingenic-drm-y = ingenic-drm-drv.o
>> ingenic-drm-$(CONFIG_DRM_INGENIC_IPU) += ingenic-ipu.o
>> +obj-$(CONFIG_DRM_INGENIC_DW_HDMI) += ingenic-dw-hdmi.o
>> diff --git a/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c
>> new file mode 100644
>> index 0000000000000..34e986dd606cf
>> --- /dev/null
>> +++ b/drivers/gpu/drm/ingenic/ingenic-dw-hdmi.c
>> @@ -0,0 +1,104 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (C) 2011-2013 Freescale Semiconductor, Inc.
>> + * Copyright (C) 2019, 2020 Paul Boddie <paul@boddie.org.uk>
>> + *
>> + * Derived from dw_hdmi-imx.c with i.MX portions removed.
>> + * Probe and remove operations derived from rcar_dw_hdmi.c.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +
>> +#include <drm/bridge/dw_hdmi.h>
>> +#include <drm/drm_of.h>
>> +#include <drm/drm_print.h>
>> +
>> +static const struct dw_hdmi_mpll_config ingenic_mpll_cfg[] = {
>> +	{ 45250000,  { { 0x01e0, 0x0000 }, { 0x21e1, 0x0000 }, { 0x41e2, 0x0000 } } },
>> +	{ 92500000,  { { 0x0140, 0x0005 }, { 0x2141, 0x0005 }, { 0x4142, 0x0005 } } },
>> +	{ 148500000, { { 0x00a0, 0x000a }, { 0x20a1, 0x000a }, { 0x40a2, 0x000a } } },
>> +	{ 216000000, { { 0x00a0, 0x000a }, { 0x2001, 0x000f }, { 0x4002, 0x000f } } },
>> +	{ ~0UL,      { { 0x0000, 0x0000 }, { 0x0000, 0x0000 }, { 0x0000, 0x0000 } } }
>> +};
>> +
>> +static const struct dw_hdmi_curr_ctrl ingenic_cur_ctr[] = {
>> +	/*pixelclk     bpp8    bpp10   bpp12 */
>> +	{ 54000000,  { 0x091c, 0x091c, 0x06dc } },
>> +	{ 58400000,  { 0x091c, 0x06dc, 0x06dc } },
>> +	{ 72000000,  { 0x06dc, 0x06dc, 0x091c } },
>> +	{ 74250000,  { 0x06dc, 0x0b5c, 0x091c } },
>> +	{ 118800000, { 0x091c, 0x091c, 0x06dc } },
>> +	{ 216000000, { 0x06dc, 0x0b5c, 0x091c } },
>> +	{ ~0UL,      { 0x0000, 0x0000, 0x0000 } },
>> +};
>> +
>> +/*
>> + * Resistance term 133Ohm Cfg
>> + * PREEMP config 0.00
>> + * TX/CK level 10
>> + */
>> +static const struct dw_hdmi_phy_config ingenic_phy_config[] = {
>> +	/*pixelclk   symbol   term   vlev */
>> +	{ 216000000, 0x800d, 0x0005, 0x01ad},
>> +	{ ~0UL,      0x0000, 0x0000, 0x0000}
>> +};
>> +
>> +static enum drm_mode_status
>> +ingenic_dw_hdmi_mode_valid(struct dw_hdmi *hdmi, void *data,
>> +			   const struct drm_display_info *info,
>> +			   const struct drm_display_mode *mode)
>> +{
>> +	if (mode->clock < 13500)
>> +		return MODE_CLOCK_LOW;
>> +	/* FIXME: Hardware is capable of 270MHz, but setup data is missing. */
>> +	if (mode->clock > 216000)
>> +		return MODE_CLOCK_HIGH;
>> +
>> +	return MODE_OK;
>> +}
>> +
>> +static struct dw_hdmi_plat_data ingenic_dw_hdmi_plat_data = {
>> +	.mpll_cfg   = ingenic_mpll_cfg,
>> +	.cur_ctr    = ingenic_cur_ctr,
>> +	.phy_config = ingenic_phy_config,
>> +	.mode_valid = ingenic_dw_hdmi_mode_valid,
>> +	.output_port	= 1,
>> +};
>> +
>> +static const struct of_device_id ingenic_dw_hdmi_dt_ids[] = {
>> +	{ .compatible = "ingenic,jz4780-dw-hdmi" },
>> +	{ /* Sentinel */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, ingenic_dw_hdmi_dt_ids);
>> +
>> +static void ingenic_dw_hdmi_cleanup(void *data)
>> +{
>> +	struct dw_hdmi *hdmi = (struct dw_hdmi *)data;
>> +
>> +	dw_hdmi_remove(hdmi);
>> +}
>> +
>> +static int ingenic_dw_hdmi_probe(struct platform_device *pdev)
>> +{
>> +	struct dw_hdmi *hdmi;
>> +
>> +	hdmi = dw_hdmi_probe(pdev, &ingenic_dw_hdmi_plat_data);
>> +	if (IS_ERR(hdmi))
>> +		return PTR_ERR(hdmi);
>> +
>> +	return devm_add_action_or_reset(&pdev->dev, ingenic_dw_hdmi_cleanup, hdmi);
> 
> Nitpick, but your probe function is so simple, you could just have a .remove callback

Well, we did have one in v5 or so (haven't looked back) and it was your suggestion to
add an action handler. Well at that point it was a little more to do.

> instead of registering a devm action. Then you can just return PTR_ERR_OR_ZERO(hdmi).
> 
> Cheers,
> -Paul

Re: [PATCH v12 7/9] drm/bridge: display-connector: add ddc-en gpio support

[H. Nikolaus Schaller]

Hi Paul,

> Am 02.02.2022 um 11:32 schrieb Paul Cercueil <paul@crapouillou.net>:
> 
> Hi Nikolaus,
> 
> Le lun., janv. 31 2022 at 13:26:53 +0100, H. Nikolaus Schaller <hns@goldelico.com> a écrit :
>> "hdmi-connector.yaml" bindings defines an optional property
>> "ddc-en-gpios" for a single gpio to enable DDC operation.
>> Usually this controls +5V power on the HDMI connector.
>> This +5V may also be needed for HPD.
>> This was not reflected in code.
>> Now, the driver activates the ddc gpio after probe and
>> deactivates after remove so it is "almost on".
>> But only if this driver is loaded (and not e.g. blacklisted
>> as module).
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> ---
>> drivers/gpu/drm/bridge/display-connector.c | 17 +++++++++++++++++
>> 1 file changed, 17 insertions(+)
>> diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c
>> index d24f5b90feabf..555395e301096 100644
>> --- a/drivers/gpu/drm/bridge/display-connector.c
>> +++ b/drivers/gpu/drm/bridge/display-connector.c
>> @@ -24,6 +24,7 @@ struct display_connector {
>> 	int			hpd_irq;
>> 	struct regulator	*dp_pwr;
>> +	struct gpio_desc	*ddc_en;
>> };
>> static inline struct display_connector *
>> @@ -345,6 +346,19 @@ static int display_connector_probe(struct platform_device *pdev)
>> 		}
>> 	}
>> +	/* enable DDC */
>> +	if (type == DRM_MODE_CONNECTOR_HDMIA) {
>> +		conn->ddc_en = devm_gpiod_get_optional(&pdev->dev, "ddc-en",
>> +						       GPIOD_OUT_HIGH);
>> +
>> +		if (IS_ERR(conn->ddc_en)) {
>> +			dev_err(&pdev->dev, "Couldn't get ddc-en gpio\n");
>> +			return PTR_ERR(conn->ddc_en);
>> +		}
>> +
>> +		gpiod_set_value(conn->ddc_en, 1);
> 
> You already requested the gpio with the GPIOD_OUT_HIGH flag, so this can be removed.

Ah, ok!

> 
> 
>> +	}
>> +
>> 	conn->bridge.funcs = &display_connector_bridge_funcs;
>> 	conn->bridge.of_node = pdev->dev.of_node;
>> @@ -373,6 +387,9 @@ static int display_connector_remove(struct platform_device *pdev)
>> {
>> 	struct display_connector *conn = platform_get_drvdata(pdev);
>> +	if (conn->ddc_en)
>> +		gpiod_set_value(conn->ddc_en, 0);
> 
> Note that gpiod_set_value() already does the null-check internally.

Indeed.

> I actually do prefer your solution, so this is fine with me, but maintainers may have a different opinion.

I am fine with any of them. Just need to know which one to take (and test).

BR,
Nikolaus

> 
> Cheers,
> -Paul
> 
>> +
>> 	if (conn->dp_pwr)
>> 		regulator_disable(conn->dp_pwr);
>> --
>> 2.33.0
> 
> 

Re: [PATCH v12 2/9] drm/ingenic: Add support for JZ4780 and HDMI output

[H. Nikolaus Schaller]

Hi Paul,
thanks for the reviews. Looks as if we are close to making a goal.

> Am 02.02.2022 um 11:23 schrieb Paul Cercueil <paul@crapouillou.net>:
> 
> Hi Nikolaus,
> 
> Le lun., janv. 31 2022 at 13:26:48 +0100, H. Nikolaus Schaller <hns@goldelico.com> a écrit :
>> From: Paul Boddie <paul@boddie.org.uk>
>> Add support for the LCD controller present on JZ4780 SoCs.
>> This SoC uses 8-byte descriptors which extend the current
>> 4-byte descriptors used for other Ingenic SoCs.
>> Tested on MIPS Creator CI20 board.
>> Signed-off-by: Paul Boddie <paul@boddie.org.uk>
>> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> ---
>> drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 61 ++++++++++++++++++++++-
>> drivers/gpu/drm/ingenic/ingenic-drm.h     | 38 ++++++++++++++
>> 2 files changed, 98 insertions(+), 1 deletion(-)
>> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>> index 9c60fc4605e4b..ccdb9eedd9247 100644
>> --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>> +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>> @@ -6,6 +6,7 @@
>> #include "ingenic-drm.h"
>> +#include <linux/bitfield.h>
>> #include <linux/component.h>
>> #include <linux/clk.h>
>> #include <linux/dma-mapping.h>
>> @@ -49,6 +50,11 @@ struct ingenic_dma_hwdesc {
>> 	u32 addr;
>> 	u32 id;
>> 	u32 cmd;
>> +	/* extended hw descriptor for jz4780 */
>> +	u32 offsize;
>> +	u32 pagewidth;
>> +	u32 cpos;
>> +	u32 dessize;
>> } __aligned(16);
>> struct ingenic_dma_hwdescs {
>> @@ -59,7 +65,9 @@ struct ingenic_dma_hwdescs {
>> struct jz_soc_info {
>> 	bool needs_dev_clk;
>> 	bool has_osd;
>> +	bool has_alpha;
>> 	bool map_noncoherent;
>> +	bool use_extended_hwdesc;
>> 	unsigned int max_width, max_height;
>> 	const u32 *formats_f0, *formats_f1;
>> 	unsigned int num_formats_f0, num_formats_f1;
>> @@ -446,6 +454,9 @@ static int ingenic_drm_plane_atomic_check(struct drm_plane *plane,
>> 	if (!crtc)
>> 		return 0;
>> +	if (plane == &priv->f0)
>> +		return -EINVAL;
> 
> This will break JZ4725B -> JZ4770 SoCs, the f0 plane is perfectly usable there.

Hm. I think it was your request/proposal to add this [1]?

What I have forgotten is why the f0 plane should not be usable for jz4780.

BR and thanks,
Nikolaus

[1] end of https://patchwork.kernel.org/project/dri-devel/patch/2c7d0aa7d3ef480ebb996d37c27cbaa6f722728b.1633436959.git.hns@goldelico.com/#24578683


> 
> Cheers,
> -Paul
> 
>> +
>> 	crtc_state = drm_atomic_get_existing_crtc_state(state,
>> 							crtc);
>> 	if (WARN_ON(!crtc_state))
>> @@ -662,6 +673,33 @@ static void ingenic_drm_plane_atomic_update(struct drm_plane *plane,
>> 		hwdesc->cmd = JZ_LCD_CMD_EOF_IRQ | (width * height * cpp / 4);
>> 		hwdesc->next = dma_hwdesc_addr(priv, next_id);
>> +		if (priv->soc_info->use_extended_hwdesc) {
>> +			hwdesc->cmd |= JZ_LCD_CMD_FRM_ENABLE;
>> +
>> +			/* Extended 8-byte descriptor */
>> +			hwdesc->cpos = 0;
>> +			hwdesc->offsize = 0;
>> +			hwdesc->pagewidth = 0;
>> +
>> +			switch (newstate->fb->format->format) {
>> +			case DRM_FORMAT_XRGB1555:
>> +				hwdesc->cpos |= JZ_LCD_CPOS_RGB555;
>> +				fallthrough;
>> +			case DRM_FORMAT_RGB565:
>> +				hwdesc->cpos |= JZ_LCD_CPOS_BPP_15_16;
>> +				break;
>> +			case DRM_FORMAT_XRGB8888:
>> +				hwdesc->cpos |= JZ_LCD_CPOS_BPP_18_24;
>> +				break;
>> +			}
>> +			hwdesc->cpos |= (JZ_LCD_CPOS_COEFFICIENT_1 <<
>> +					 JZ_LCD_CPOS_COEFFICIENT_OFFSET);
>> +			hwdesc->dessize =
>> +				(0xff << JZ_LCD_DESSIZE_ALPHA_OFFSET) |
>> +				FIELD_PREP(JZ_LCD_DESSIZE_HEIGHT_MASK, height - 1) |
>> +				FIELD_PREP(JZ_LCD_DESSIZE_WIDTH_MASK, width - 1);
>> +		}
>> +
>> 		if (drm_atomic_crtc_needs_modeset(crtc_state)) {
>> 			fourcc = newstate->fb->format->format;
>> @@ -693,6 +731,9 @@ static void ingenic_drm_encoder_atomic_mode_set(struct drm_encoder *encoder,
>> 		    | JZ_LCD_CFG_SPL_DISABLE | JZ_LCD_CFG_REV_DISABLE;
>> 	}
>> +	if (priv->soc_info->use_extended_hwdesc)
>> +		cfg |= JZ_LCD_CFG_DESCRIPTOR_8;
>> +
>> 	if (mode->flags & DRM_MODE_FLAG_NHSYNC)
>> 		cfg |= JZ_LCD_CFG_HSYNC_ACTIVE_LOW;
>> 	if (mode->flags & DRM_MODE_FLAG_NVSYNC)
>> @@ -1015,6 +1056,7 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
>> 	long parent_rate;
>> 	unsigned int i, clone_mask = 0;
>> 	int ret, irq;
>> +	u32 osdc = 0;
>> 	soc_info = of_device_get_match_data(dev);
>> 	if (!soc_info) {
>> @@ -1272,7 +1314,10 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
>> 	/* Enable OSD if available */
>> 	if (soc_info->has_osd)
>> -		regmap_write(priv->map, JZ_REG_LCD_OSDC, JZ_LCD_OSDC_OSDEN);
>> +		osdc |= JZ_LCD_OSDC_OSDEN;
>> +	if (soc_info->has_alpha)
>> +		osdc |= JZ_LCD_OSDC_ALPHAEN;
>> +	regmap_write(priv->map, JZ_REG_LCD_OSDC, osdc);
>> 	mutex_init(&priv->clk_mutex);
>> 	priv->clock_nb.notifier_call = ingenic_drm_update_pixclk;
>> @@ -1468,10 +1513,24 @@ static const struct jz_soc_info jz4770_soc_info = {
>> 	.num_formats_f0 = ARRAY_SIZE(jz4770_formats_f0),
>> };
>> +static const struct jz_soc_info jz4780_soc_info = {
>> +	.needs_dev_clk = true,
>> +	.has_osd = true,
>> +	.has_alpha = true,
>> +	.use_extended_hwdesc = true,
>> +	.max_width = 4096,
>> +	.max_height = 2048,
>> +	.formats_f1 = jz4770_formats_f1,
>> +	.num_formats_f1 = ARRAY_SIZE(jz4770_formats_f1),
>> +	.formats_f0 = jz4770_formats_f0,
>> +	.num_formats_f0 = ARRAY_SIZE(jz4770_formats_f0),
>> +};
>> +
>> static const struct of_device_id ingenic_drm_of_match[] = {
>> 	{ .compatible = "ingenic,jz4740-lcd", .data = &jz4740_soc_info },
>> 	{ .compatible = "ingenic,jz4725b-lcd", .data = &jz4725b_soc_info },
>> 	{ .compatible = "ingenic,jz4770-lcd", .data = &jz4770_soc_info },
>> +	{ .compatible = "ingenic,jz4780-lcd", .data = &jz4780_soc_info },
>> 	{ /* sentinel */ },
>> };
>> MODULE_DEVICE_TABLE(of, ingenic_drm_of_match);
>> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.h b/drivers/gpu/drm/ingenic/ingenic-drm.h
>> index 22654ac1dde1c..cb1d09b625881 100644
>> --- a/drivers/gpu/drm/ingenic/ingenic-drm.h
>> +++ b/drivers/gpu/drm/ingenic/ingenic-drm.h
>> @@ -44,8 +44,11 @@
>> #define JZ_REG_LCD_XYP1				0x124
>> #define JZ_REG_LCD_SIZE0			0x128
>> #define JZ_REG_LCD_SIZE1			0x12c
>> +#define JZ_REG_LCD_PCFG				0x2c0
>> #define JZ_LCD_CFG_SLCD				BIT(31)
>> +#define JZ_LCD_CFG_DESCRIPTOR_8			BIT(28)
>> +#define JZ_LCD_CFG_RECOVER_FIFO_UNDERRUN	BIT(25)
>> #define JZ_LCD_CFG_PS_DISABLE			BIT(23)
>> #define JZ_LCD_CFG_CLS_DISABLE			BIT(22)
>> #define JZ_LCD_CFG_SPL_DISABLE			BIT(21)
>> @@ -63,6 +66,7 @@
>> #define JZ_LCD_CFG_DE_ACTIVE_LOW		BIT(9)
>> #define JZ_LCD_CFG_VSYNC_ACTIVE_LOW		BIT(8)
>> #define JZ_LCD_CFG_18_BIT			BIT(7)
>> +#define JZ_LCD_CFG_24_BIT			BIT(6)
>> #define JZ_LCD_CFG_PDW				(BIT(5) | BIT(4))
>> #define JZ_LCD_CFG_MODE_GENERIC_16BIT		0
>> @@ -132,6 +136,7 @@
>> #define JZ_LCD_CMD_SOF_IRQ			BIT(31)
>> #define JZ_LCD_CMD_EOF_IRQ			BIT(30)
>> #define JZ_LCD_CMD_ENABLE_PAL			BIT(28)
>> +#define JZ_LCD_CMD_FRM_ENABLE			BIT(26)
>> #define JZ_LCD_SYNC_MASK			0x3ff
>> @@ -153,6 +158,7 @@
>> #define JZ_LCD_RGBC_EVEN_BGR			(0x5 << 0)
>> #define JZ_LCD_OSDC_OSDEN			BIT(0)
>> +#define JZ_LCD_OSDC_ALPHAEN			BIT(2)
>> #define JZ_LCD_OSDC_F0EN			BIT(3)
>> #define JZ_LCD_OSDC_F1EN			BIT(4)
>> @@ -176,6 +182,38 @@
>> #define JZ_LCD_SIZE01_WIDTH_LSB			0
>> #define JZ_LCD_SIZE01_HEIGHT_LSB		16
>> +#define JZ_LCD_DESSIZE_ALPHA_OFFSET		24
>> +#define JZ_LCD_DESSIZE_HEIGHT_MASK		GENMASK(23, 12)
>> +#define JZ_LCD_DESSIZE_WIDTH_MASK		GENMASK(11, 0)
>> +
>> +#define JZ_LCD_CPOS_BPP_15_16			(4 << 27)
>> +#define JZ_LCD_CPOS_BPP_18_24			(5 << 27)
>> +#define JZ_LCD_CPOS_BPP_30			(7 << 27)
>> +#define JZ_LCD_CPOS_RGB555			BIT(30)
>> +#define JZ_LCD_CPOS_PREMULTIPLY_LCD		BIT(26)
>> +#define JZ_LCD_CPOS_COEFFICIENT_OFFSET		24
>> +#define JZ_LCD_CPOS_COEFFICIENT_0		0
>> +#define JZ_LCD_CPOS_COEFFICIENT_1		1
>> +#define JZ_LCD_CPOS_COEFFICIENT_ALPHA1		2
>> +#define JZ_LCD_CPOS_COEFFICIENT_1_ALPHA1	3
>> +
>> +#define JZ_LCD_RGBC_RGB_PADDING			BIT(15)
>> +#define JZ_LCD_RGBC_RGB_PADDING_FIRST		BIT(14)
>> +#define JZ_LCD_RGBC_422				BIT(8)
>> +#define JZ_LCD_RGBC_RGB_FORMAT_ENABLE		BIT(7)
>> +
>> +#define JZ_LCD_PCFG_PRI_MODE			BIT(31)
>> +#define JZ_LCD_PCFG_HP_BST_4			(0 << 28)
>> +#define JZ_LCD_PCFG_HP_BST_8			(1 << 28)
>> +#define JZ_LCD_PCFG_HP_BST_16			(2 << 28)
>> +#define JZ_LCD_PCFG_HP_BST_32			(3 << 28)
>> +#define JZ_LCD_PCFG_HP_BST_64			(4 << 28)
>> +#define JZ_LCD_PCFG_HP_BST_16_CONT		(5 << 28)
>> +#define JZ_LCD_PCFG_HP_BST_DISABLE		(7 << 28)
>> +#define JZ_LCD_PCFG_THRESHOLD2_OFFSET		18
>> +#define JZ_LCD_PCFG_THRESHOLD1_OFFSET		9
>> +#define JZ_LCD_PCFG_THRESHOLD0_OFFSET		0
>> +
>> struct device;
>> struct drm_plane;
>> struct drm_plane_state;
>> --
>> 2.33.0
> 
> 

Re: [PATCH v12 3/9] dt-bindings: display: Add ingenic,jz4780-dw-hdmi DT Schema

[H. Nikolaus Schaller]

Hi Paul,

> Am 02.02.2022 um 10:59 schrieb Paul Cercueil <paul@crapouillou.net>:
> 
> Hi Nikolaus,
> 
> Le lun., janv. 31 2022 at 13:26:49 +0100, H. Nikolaus Schaller <hns@goldelico.com> a écrit :
>> From: Sam Ravnborg <sam@ravnborg.org>
>> Add DT bindings for the hdmi driver for the Ingenic JZ4780 SoC.
>> Based on .txt binding from Zubair Lutfullah Kakakhel
>> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> Cc: Rob Herring <robh@kernel.org>
>> Cc: devicetree@vger.kernel.org
>> ---
>> .../display/bridge/ingenic,jz4780-hdmi.yaml   | 83 +++++++++++++++++++
>> 1 file changed, 83 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/display/bridge/ingenic,jz4780-hdmi.yaml
>> diff --git a/Documentation/devicetree/bindings/display/bridge/ingenic,jz4780-hdmi.yaml b/Documentation/devicetree/bindings/display/bridge/ingenic,jz4780-hdmi.yaml
>> new file mode 100644
>> index 0000000000000..5a2767308c0ab
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/bridge/ingenic,jz4780-hdmi.yaml
>> @@ -0,0 +1,83 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/display/bridge/ingenic,jz4780-hdmi.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Bindings for Ingenic JZ4780 HDMI Transmitter
>> +
>> +maintainers:
>> +  - H. Nikolaus Schaller <hns@goldelico.com>
>> +
>> +description: |
>> +  The HDMI Transmitter in the Ingenic JZ4780 is a Synopsys DesignWare HDMI 1.4
>> +  TX controller IP with accompanying PHY IP.
>> +
>> +allOf:
>> +  - $ref: synopsys,dw-hdmi.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    const: ingenic,jz4780-dw-hdmi
>> +
>> +  reg-io-width:
>> +    const: 4
>> +
>> +  clocks:
>> +    maxItems: 2
>> +
>> +  ports:
>> +    $ref: /schemas/graph.yaml#/properties/ports
>> +
>> +    properties:
>> +      port@0:
>> +        $ref: /schemas/graph.yaml#/properties/port
>> +        description: Input from LCD controller output.
>> +
>> +      port@1:
>> +        $ref: /schemas/graph.yaml#/properties/port
>> +        description: Link to the HDMI connector.
>> +
>> +required:
>> +  - compatible
>> +  - clocks
>> +  - clock-names
>> +  - ports
>> +  - reg-io-width
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/ingenic,jz4780-cgu.h>
>> +
>> +    hdmi: hdmi@10180000 {
>> +        compatible = "ingenic,jz4780-dw-hdmi";
>> +        reg = <0x10180000 0x8000>;
>> +        reg-io-width = <4>;
>> +        ddc-i2c-bus = <&i2c4>;
>> +        interrupt-parent = <&intc>;
>> +        interrupts = <3>;
>> +        clocks = <&cgu JZ4780_CLK_AHB0>, <&cgu JZ4780_CLK_HDMI>;
>> +        clock-names = "iahb", "isfr";
>> +        hdmi-5v-supply = <&hdmi_power>;
> 
> Where is this property defined?

It was (not) is defined in v11... So forgot to remove it from the example.
HDMI power is now defined at the connector.

Thanks,
Nikolaus

> 
> Cheers,
> -Paul
> 
>> +
>> +        ports {
>> +            #address-cells = <1>;
>> +            #size-cells = <0>;
>> +            hdmi_in: port@0 {
>> +                reg = <0>;
>> +                dw_hdmi_in: endpoint {
>> +                    remote-endpoint = <&jz4780_lcd_out>;
>> +                };
>> +            };
>> +            hdmi_out: port@1 {
>> +                reg = <1>;
>> +                dw_hdmi_out: endpoint {
>> +                    remote-endpoint = <&hdmi_con>;
>> +                };
>> +            };
>> +        };
>> +    };
>> +
>> +...
>> --
>> 2.33.0
> 
> 

Re: [PATCH v12 2/9] drm/ingenic: Add support for JZ4780 and HDMI output

[Paul Cercueil]

Hi Nikolaus,

Le mer., févr. 2 2022 at 12:56:35 +0100, H. Nikolaus Schaller 
<hns@goldelico.com> a écrit :
> Hi Paul,
> thanks for the reviews. Looks as if we are close to making a goal.
> 
>>  Am 02.02.2022 um 11:23 schrieb Paul Cercueil <paul@crapouillou.net>:
>> 
>>  Hi Nikolaus,
>> 
>>  Le lun., janv. 31 2022 at 13:26:48 +0100, H. Nikolaus Schaller 
>> <hns@goldelico.com> a écrit :
>>>  From: Paul Boddie <paul@boddie.org.uk>
>>>  Add support for the LCD controller present on JZ4780 SoCs.
>>>  This SoC uses 8-byte descriptors which extend the current
>>>  4-byte descriptors used for other Ingenic SoCs.
>>>  Tested on MIPS Creator CI20 board.
>>>  Signed-off-by: Paul Boddie <paul@boddie.org.uk>
>>>  Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
>>>  Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>>>  ---
>>>  drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 61 
>>> ++++++++++++++++++++++-
>>>  drivers/gpu/drm/ingenic/ingenic-drm.h     | 38 ++++++++++++++
>>>  2 files changed, 98 insertions(+), 1 deletion(-)
>>>  diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c 
>>> b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>>>  index 9c60fc4605e4b..ccdb9eedd9247 100644
>>>  --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>>>  +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
>>>  @@ -6,6 +6,7 @@
>>>  #include "ingenic-drm.h"
>>>  +#include <linux/bitfield.h>
>>>  #include <linux/component.h>
>>>  #include <linux/clk.h>
>>>  #include <linux/dma-mapping.h>
>>>  @@ -49,6 +50,11 @@ struct ingenic_dma_hwdesc {
>>>  	u32 addr;
>>>  	u32 id;
>>>  	u32 cmd;
>>>  +	/* extended hw descriptor for jz4780 */
>>>  +	u32 offsize;
>>>  +	u32 pagewidth;
>>>  +	u32 cpos;
>>>  +	u32 dessize;
>>>  } __aligned(16);
>>>  struct ingenic_dma_hwdescs {
>>>  @@ -59,7 +65,9 @@ struct ingenic_dma_hwdescs {
>>>  struct jz_soc_info {
>>>  	bool needs_dev_clk;
>>>  	bool has_osd;
>>>  +	bool has_alpha;
>>>  	bool map_noncoherent;
>>>  +	bool use_extended_hwdesc;
>>>  	unsigned int max_width, max_height;
>>>  	const u32 *formats_f0, *formats_f1;
>>>  	unsigned int num_formats_f0, num_formats_f1;
>>>  @@ -446,6 +454,9 @@ static int 
>>> ingenic_drm_plane_atomic_check(struct drm_plane *plane,
>>>  	if (!crtc)
>>>  		return 0;
>>>  +	if (plane == &priv->f0)
>>>  +		return -EINVAL;
>> 
>>  This will break JZ4725B -> JZ4770 SoCs, the f0 plane is perfectly 
>> usable there.
> 
> Hm. I think it was your request/proposal to add this [1]?

Because otherwise with your current patchset the f0 plane does not work 
*on JZ4780*. It does work on older SoCs.

> What I have forgotten is why the f0 plane should not be usable for 
> jz4780.

We return an error here to prevent userspace from using the f0 plane 
until it's effectively working on the JZ4780.

Cheers,
-Paul

> BR and thanks,
> Nikolaus
> 
> [1] end of 
> https://patchwork.kernel.org/project/dri-devel/patch/2c7d0aa7d3ef480ebb996d37c27cbaa6f722728b.1633436959.git.hns@goldelico.com/#24578683
> 
> 
>> 
>>  Cheers,
>>  -Paul
>> 
>>>  +
>>>  	crtc_state = drm_atomic_get_existing_crtc_state(state,
>>>  							crtc);
>>>  	if (WARN_ON(!crtc_state))
>>>  @@ -662,6 +673,33 @@ static void 
>>> ingenic_drm_plane_atomic_update(struct drm_plane *plane,
>>>  		hwdesc->cmd = JZ_LCD_CMD_EOF_IRQ | (width * height * cpp / 4);
>>>  		hwdesc->next = dma_hwdesc_addr(priv, next_id);
>>>  +		if (priv->soc_info->use_extended_hwdesc) {
>>>  +			hwdesc->cmd |= JZ_LCD_CMD_FRM_ENABLE;
>>>  +
>>>  +			/* Extended 8-byte descriptor */
>>>  +			hwdesc->cpos = 0;
>>>  +			hwdesc->offsize = 0;
>>>  +			hwdesc->pagewidth = 0;
>>>  +
>>>  +			switch (newstate->fb->format->format) {
>>>  +			case DRM_FORMAT_XRGB1555:
>>>  +				hwdesc->cpos |= JZ_LCD_CPOS_RGB555;
>>>  +				fallthrough;
>>>  +			case DRM_FORMAT_RGB565:
>>>  +				hwdesc->cpos |= JZ_LCD_CPOS_BPP_15_16;
>>>  +				break;
>>>  +			case DRM_FORMAT_XRGB8888:
>>>  +				hwdesc->cpos |= JZ_LCD_CPOS_BPP_18_24;
>>>  +				break;
>>>  +			}
>>>  +			hwdesc->cpos |= (JZ_LCD_CPOS_COEFFICIENT_1 <<
>>>  +					 JZ_LCD_CPOS_COEFFICIENT_OFFSET);
>>>  +			hwdesc->dessize =
>>>  +				(0xff << JZ_LCD_DESSIZE_ALPHA_OFFSET) |
>>>  +				FIELD_PREP(JZ_LCD_DESSIZE_HEIGHT_MASK, height - 1) |
>>>  +				FIELD_PREP(JZ_LCD_DESSIZE_WIDTH_MASK, width - 1);
>>>  +		}
>>>  +
>>>  		if (drm_atomic_crtc_needs_modeset(crtc_state)) {
>>>  			fourcc = newstate->fb->format->format;
>>>  @@ -693,6 +731,9 @@ static void 
>>> ingenic_drm_encoder_atomic_mode_set(struct drm_encoder *encoder,
>>>  		    | JZ_LCD_CFG_SPL_DISABLE | JZ_LCD_CFG_REV_DISABLE;
>>>  	}
>>>  +	if (priv->soc_info->use_extended_hwdesc)
>>>  +		cfg |= JZ_LCD_CFG_DESCRIPTOR_8;
>>>  +
>>>  	if (mode->flags & DRM_MODE_FLAG_NHSYNC)
>>>  		cfg |= JZ_LCD_CFG_HSYNC_ACTIVE_LOW;
>>>  	if (mode->flags & DRM_MODE_FLAG_NVSYNC)
>>>  @@ -1015,6 +1056,7 @@ static int ingenic_drm_bind(struct device 
>>> *dev, bool has_components)
>>>  	long parent_rate;
>>>  	unsigned int i, clone_mask = 0;
>>>  	int ret, irq;
>>>  +	u32 osdc = 0;
>>>  	soc_info = of_device_get_match_data(dev);
>>>  	if (!soc_info) {
>>>  @@ -1272,7 +1314,10 @@ static int ingenic_drm_bind(struct device 
>>> *dev, bool has_components)
>>>  	/* Enable OSD if available */
>>>  	if (soc_info->has_osd)
>>>  -		regmap_write(priv->map, JZ_REG_LCD_OSDC, JZ_LCD_OSDC_OSDEN);
>>>  +		osdc |= JZ_LCD_OSDC_OSDEN;
>>>  +	if (soc_info->has_alpha)
>>>  +		osdc |= JZ_LCD_OSDC_ALPHAEN;
>>>  +	regmap_write(priv->map, JZ_REG_LCD_OSDC, osdc);
>>>  	mutex_init(&priv->clk_mutex);
>>>  	priv->clock_nb.notifier_call = ingenic_drm_update_pixclk;
>>>  @@ -1468,10 +1513,24 @@ static const struct jz_soc_info 
>>> jz4770_soc_info = {
>>>  	.num_formats_f0 = ARRAY_SIZE(jz4770_formats_f0),
>>>  };
>>>  +static const struct jz_soc_info jz4780_soc_info = {
>>>  +	.needs_dev_clk = true,
>>>  +	.has_osd = true,
>>>  +	.has_alpha = true,
>>>  +	.use_extended_hwdesc = true,
>>>  +	.max_width = 4096,
>>>  +	.max_height = 2048,
>>>  +	.formats_f1 = jz4770_formats_f1,
>>>  +	.num_formats_f1 = ARRAY_SIZE(jz4770_formats_f1),
>>>  +	.formats_f0 = jz4770_formats_f0,
>>>  +	.num_formats_f0 = ARRAY_SIZE(jz4770_formats_f0),
>>>  +};
>>>  +
>>>  static const struct of_device_id ingenic_drm_of_match[] = {
>>>  	{ .compatible = "ingenic,jz4740-lcd", .data = &jz4740_soc_info },
>>>  	{ .compatible = "ingenic,jz4725b-lcd", .data = &jz4725b_soc_info 
>>> },
>>>  	{ .compatible = "ingenic,jz4770-lcd", .data = &jz4770_soc_info },
>>>  +	{ .compatible = "ingenic,jz4780-lcd", .data = &jz4780_soc_info },
>>>  	{ /* sentinel */ },
>>>  };
>>>  MODULE_DEVICE_TABLE(of, ingenic_drm_of_match);
>>>  diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.h 
>>> b/drivers/gpu/drm/ingenic/ingenic-drm.h
>>>  index 22654ac1dde1c..cb1d09b625881 100644
>>>  --- a/drivers/gpu/drm/ingenic/ingenic-drm.h
>>>  +++ b/drivers/gpu/drm/ingenic/ingenic-drm.h
>>>  @@ -44,8 +44,11 @@
>>>  #define JZ_REG_LCD_XYP1				0x124
>>>  #define JZ_REG_LCD_SIZE0			0x128
>>>  #define JZ_REG_LCD_SIZE1			0x12c
>>>  +#define JZ_REG_LCD_PCFG				0x2c0
>>>  #define JZ_LCD_CFG_SLCD				BIT(31)
>>>  +#define JZ_LCD_CFG_DESCRIPTOR_8			BIT(28)
>>>  +#define JZ_LCD_CFG_RECOVER_FIFO_UNDERRUN	BIT(25)
>>>  #define JZ_LCD_CFG_PS_DISABLE			BIT(23)
>>>  #define JZ_LCD_CFG_CLS_DISABLE			BIT(22)
>>>  #define JZ_LCD_CFG_SPL_DISABLE			BIT(21)
>>>  @@ -63,6 +66,7 @@
>>>  #define JZ_LCD_CFG_DE_ACTIVE_LOW		BIT(9)
>>>  #define JZ_LCD_CFG_VSYNC_ACTIVE_LOW		BIT(8)
>>>  #define JZ_LCD_CFG_18_BIT			BIT(7)
>>>  +#define JZ_LCD_CFG_24_BIT			BIT(6)
>>>  #define JZ_LCD_CFG_PDW				(BIT(5) | BIT(4))
>>>  #define JZ_LCD_CFG_MODE_GENERIC_16BIT		0
>>>  @@ -132,6 +136,7 @@
>>>  #define JZ_LCD_CMD_SOF_IRQ			BIT(31)
>>>  #define JZ_LCD_CMD_EOF_IRQ			BIT(30)
>>>  #define JZ_LCD_CMD_ENABLE_PAL			BIT(28)
>>>  +#define JZ_LCD_CMD_FRM_ENABLE			BIT(26)
>>>  #define JZ_LCD_SYNC_MASK			0x3ff
>>>  @@ -153,6 +158,7 @@
>>>  #define JZ_LCD_RGBC_EVEN_BGR			(0x5 << 0)
>>>  #define JZ_LCD_OSDC_OSDEN			BIT(0)
>>>  +#define JZ_LCD_OSDC_ALPHAEN			BIT(2)
>>>  #define JZ_LCD_OSDC_F0EN			BIT(3)
>>>  #define JZ_LCD_OSDC_F1EN			BIT(4)
>>>  @@ -176,6 +182,38 @@
>>>  #define JZ_LCD_SIZE01_WIDTH_LSB			0
>>>  #define JZ_LCD_SIZE01_HEIGHT_LSB		16
>>>  +#define JZ_LCD_DESSIZE_ALPHA_OFFSET		24
>>>  +#define JZ_LCD_DESSIZE_HEIGHT_MASK		GENMASK(23, 12)
>>>  +#define JZ_LCD_DESSIZE_WIDTH_MASK		GENMASK(11, 0)
>>>  +
>>>  +#define JZ_LCD_CPOS_BPP_15_16			(4 << 27)
>>>  +#define JZ_LCD_CPOS_BPP_18_24			(5 << 27)
>>>  +#define JZ_LCD_CPOS_BPP_30			(7 << 27)
>>>  +#define JZ_LCD_CPOS_RGB555			BIT(30)
>>>  +#define JZ_LCD_CPOS_PREMULTIPLY_LCD		BIT(26)
>>>  +#define JZ_LCD_CPOS_COEFFICIENT_OFFSET		24
>>>  +#define JZ_LCD_CPOS_COEFFICIENT_0		0
>>>  +#define JZ_LCD_CPOS_COEFFICIENT_1		1
>>>  +#define JZ_LCD_CPOS_COEFFICIENT_ALPHA1		2
>>>  +#define JZ_LCD_CPOS_COEFFICIENT_1_ALPHA1	3
>>>  +
>>>  +#define JZ_LCD_RGBC_RGB_PADDING			BIT(15)
>>>  +#define JZ_LCD_RGBC_RGB_PADDING_FIRST		BIT(14)
>>>  +#define JZ_LCD_RGBC_422				BIT(8)
>>>  +#define JZ_LCD_RGBC_RGB_FORMAT_ENABLE		BIT(7)
>>>  +
>>>  +#define JZ_LCD_PCFG_PRI_MODE			BIT(31)
>>>  +#define JZ_LCD_PCFG_HP_BST_4			(0 << 28)
>>>  +#define JZ_LCD_PCFG_HP_BST_8			(1 << 28)
>>>  +#define JZ_LCD_PCFG_HP_BST_16			(2 << 28)
>>>  +#define JZ_LCD_PCFG_HP_BST_32			(3 << 28)
>>>  +#define JZ_LCD_PCFG_HP_BST_64			(4 << 28)
>>>  +#define JZ_LCD_PCFG_HP_BST_16_CONT		(5 << 28)
>>>  +#define JZ_LCD_PCFG_HP_BST_DISABLE		(7 << 28)
>>>  +#define JZ_LCD_PCFG_THRESHOLD2_OFFSET		18
>>>  +#define JZ_LCD_PCFG_THRESHOLD1_OFFSET		9
>>>  +#define JZ_LCD_PCFG_THRESHOLD0_OFFSET		0
>>>  +
>>>  struct device;
>>>  struct drm_plane;
>>>  struct drm_plane_state;
>>>  --
>>>  2.33.0
>> 
>> 
> 

Re: [PATCH v12 2/9] drm/ingenic: Add support for JZ4780 and HDMI output

[H. Nikolaus Schaller]

Hi Paul,

> Am 02.02.2022 um 13:06 schrieb Paul Cercueil <paul@crapouillou.net>:
> 
> Hi Nikolaus,
> 
>>>> @@ -446,6 +454,9 @@ static int ingenic_drm_plane_atomic_check(struct drm_plane *plane,
>>>> 	if (!crtc)
>>>> 		return 0;
>>>> +	if (plane == &priv->f0)
>>>> +		return -EINVAL;
>>> This will break JZ4725B -> JZ4770 SoCs, the f0 plane is perfectly usable there.
>> Hm. I think it was your request/proposal to add this [1]?
> 
> Because otherwise with your current patchset the f0 plane does not work *on JZ4780*.

Not that I am eager to fix that, but...
maybe it could be better to fix than having the check and -EINVAL depend on SoC compatible string
(or some new flag in soc_info. plane_f0_not_working)?

> It does work on older SoCs.
> 
>> What I have forgotten is why the f0 plane should not be usable for jz4780.
> 
> We return an error here to prevent userspace from using the f0 plane until it's effectively working on the JZ4780.

Well, what would be not working with that plane if user-space would try to use it?

> 
> Cheers,
> -Paul

BR and thanks,
Nikolaus

Re: [PATCH v12 2/9] drm/ingenic: Add support for JZ4780 and HDMI output

[Paul Cercueil]

Le mer., févr. 2 2022 at 13:17:14 +0100, H. Nikolaus Schaller 
<hns@goldelico.com> a écrit :
> Hi Paul,
> 
>>  Am 02.02.2022 um 13:06 schrieb Paul Cercueil <paul@crapouillou.net>:
>> 
>>  Hi Nikolaus,
>> 
>>>>>  @@ -446,6 +454,9 @@ static int 
>>>>> ingenic_drm_plane_atomic_check(struct drm_plane *plane,
>>>>>  	if (!crtc)
>>>>>  		return 0;
>>>>>  +	if (plane == &priv->f0)
>>>>>  +		return -EINVAL;
>>>>  This will break JZ4725B -> JZ4770 SoCs, the f0 plane is perfectly 
>>>> usable there.
>>>  Hm. I think it was your request/proposal to add this [1]?
>> 
>>  Because otherwise with your current patchset the f0 plane does not 
>> work *on JZ4780*.
> 
> Not that I am eager to fix that, but...
> maybe it could be better to fix than having the check and -EINVAL 
> depend on SoC compatible string
> (or some new flag in soc_info. plane_f0_not_working)?

Totally agree! A proper fix would be much better. A 
"plane_f0_not_working" in the meantime is OK with me.

Note that there are other things not working with your current 
implementation, for instance you cannot set the X/Y start position of 
the f1 plane, which means it's only really usable for fullscreen 
desktop/windows.

>>  It does work on older SoCs.
>> 
>>>  What I have forgotten is why the f0 plane should not be usable for 
>>> jz4780.
>> 
>>  We return an error here to prevent userspace from using the f0 
>> plane until it's effectively working on the JZ4780.
> 
> Well, what would be not working with that plane if user-space would 
> try to use it?

 From what I remember, it wouldn't show anything on screen, and after 
that trying to use the f1 plane wouldn't work either.

-Paul

Re: [PATCH v12 2/9] drm/ingenic: Add support for JZ4780 and HDMI output

[H. Nikolaus Schaller]

> Am 02.02.2022 um 13:28 schrieb Paul Cercueil <paul@crapouillou.net>:
> 
> 
> 
> Le mer., févr. 2 2022 at 13:17:14 +0100, H. Nikolaus Schaller <hns@goldelico.com> a écrit :
>> Hi Paul,
>>> Am 02.02.2022 um 13:06 schrieb Paul Cercueil <paul@crapouillou.net>:
>>> Hi Nikolaus,
>>>>>> @@ -446,6 +454,9 @@ static int ingenic_drm_plane_atomic_check(struct drm_plane *plane,
>>>>>> 	if (!crtc)
>>>>>> 		return 0;
>>>>>> +	if (plane == &priv->f0)
>>>>>> +		return -EINVAL;
>>>>> This will break JZ4725B -> JZ4770 SoCs, the f0 plane is perfectly usable there.
>>>> Hm. I think it was your request/proposal to add this [1]?
>>> Because otherwise with your current patchset the f0 plane does not work *on JZ4780*.
>> Not that I am eager to fix that, but...
>> maybe it could be better to fix than having the check and -EINVAL depend on SoC compatible string
>> (or some new flag in soc_info. plane_f0_not_working)?
> 
> Totally agree! A proper fix would be much better. A "plane_f0_not_working" in the meantime is OK with me.

Ok, then I'll prepare a v13 with plane_f0_not_working.

> 
> Note that there are other things not working with your current implementation, for instance you cannot set the X/Y start position of the f1 plane, which means it's only really usable for fullscreen desktop/windows.

Is setting x/y possible for the other SoC?

> 
>>> It does work on older SoCs.
>>>> What I have forgotten is why the f0 plane should not be usable for jz4780.
>>> We return an error here to prevent userspace from using the f0 plane until it's effectively working on the JZ4780.
>> Well, what would be not working with that plane if user-space would try to use it?
> 
> From what I remember, it wouldn't show anything on screen, and after that trying to use the f1 plane wouldn't work either.

Ok. That may become a big project to fix. So let's do step 1 first.

BR and thanks,
NIkolaus

Re: [PATCH v12 2/9] drm/ingenic: Add support for JZ4780 and HDMI output

[Paul Cercueil]

Le mer., févr. 2 2022 at 13:33:15 +0100, H. Nikolaus Schaller 
<hns@goldelico.com> a écrit :
> 
> 
>>  Am 02.02.2022 um 13:28 schrieb Paul Cercueil <paul@crapouillou.net>:
>> 
>> 
>> 
>>  Le mer., févr. 2 2022 at 13:17:14 +0100, H. Nikolaus Schaller 
>> <hns@goldelico.com> a écrit :
>>>  Hi Paul,
>>>>  Am 02.02.2022 um 13:06 schrieb Paul Cercueil 
>>>> <paul@crapouillou.net>:
>>>>  Hi Nikolaus,
>>>>>>>  @@ -446,6 +454,9 @@ static int 
>>>>>>> ingenic_drm_plane_atomic_check(struct drm_plane *plane,
>>>>>>>  	if (!crtc)
>>>>>>>  		return 0;
>>>>>>>  +	if (plane == &priv->f0)
>>>>>>>  +		return -EINVAL;
>>>>>>  This will break JZ4725B -> JZ4770 SoCs, the f0 plane is 
>>>>>> perfectly usable there.
>>>>>  Hm. I think it was your request/proposal to add this [1]?
>>>>  Because otherwise with your current patchset the f0 plane does 
>>>> not work *on JZ4780*.
>>>  Not that I am eager to fix that, but...
>>>  maybe it could be better to fix than having the check and -EINVAL 
>>> depend on SoC compatible string
>>>  (or some new flag in soc_info. plane_f0_not_working)?
>> 
>>  Totally agree! A proper fix would be much better. A 
>> "plane_f0_not_working" in the meantime is OK with me.
> 
> Ok, then I'll prepare a v13 with plane_f0_not_working.
> 
>> 
>>  Note that there are other things not working with your current 
>> implementation, for instance you cannot set the X/Y start position 
>> of the f1 plane, which means it's only really usable for fullscreen 
>> desktop/windows.
> 
> Is setting x/y possible for the other SoC?

Yes. They support different x/y positions, sizes, and pixel format for 
both f0, f1 and IPU planes.

-Paul

>> 
>>>>  It does work on older SoCs.
>>>>>  What I have forgotten is why the f0 plane should not be usable 
>>>>> for jz4780.
>>>>  We return an error here to prevent userspace from using the f0 
>>>> plane until it's effectively working on the JZ4780.
>>>  Well, what would be not working with that plane if user-space 
>>> would try to use it?
>> 
>>  From what I remember, it wouldn't show anything on screen, and 
>> after that trying to use the f1 plane wouldn't work either.
> 
> Ok. That may become a big project to fix. So let's do step 1 first.
> 
> BR and thanks,
> NIkolaus
> 

Re: [PATCH v12 2/9] drm/ingenic: Add support for JZ4780 and HDMI output

[H. Nikolaus Schaller]

> Am 02.02.2022 um 13:41 schrieb Paul Cercueil <paul@crapouillou.net>:
> 
> 
> 
> Le mer., févr. 2 2022 at 13:33:15 +0100, H. Nikolaus Schaller <hns@goldelico.com> a écrit :
>>> Am 02.02.2022 um 13:28 schrieb Paul Cercueil <paul@crapouillou.net>:
>>> Le mer., févr. 2 2022 at 13:17:14 +0100, H. Nikolaus Schaller <hns@goldelico.com> a écrit :
>>>> Hi Paul,
>>>>> Am 02.02.2022 um 13:06 schrieb Paul Cercueil <paul@crapouillou.net>:
>>>>> Hi Nikolaus,
>>>>>>>> @@ -446,6 +454,9 @@ static int ingenic_drm_plane_atomic_check(struct drm_plane *plane,
>>>>>>>> 	if (!crtc)
>>>>>>>> 		return 0;
>>>>>>>> +	if (plane == &priv->f0)
>>>>>>>> +		return -EINVAL;
>>>>>>> This will break JZ4725B -> JZ4770 SoCs, the f0 plane is perfectly usable there.
>>>>>> Hm. I think it was your request/proposal to add this [1]?
>>>>> Because otherwise with your current patchset the f0 plane does not work *on JZ4780*.
>>>> Not that I am eager to fix that, but...
>>>> maybe it could be better to fix than having the check and -EINVAL depend on SoC compatible string
>>>> (or some new flag in soc_info. plane_f0_not_working)?
>>> Totally agree! A proper fix would be much better. A "plane_f0_not_working" in the meantime is OK with me.
>> Ok, then I'll prepare a v13 with plane_f0_not_working.
>>> Note that there are other things not working with your current implementation, for instance you cannot set the X/Y start position of the f1 plane, which means it's only really usable for fullscreen desktop/windows.
>> Is setting x/y possible for the other SoC?
> 
> Yes. They support different x/y positions, sizes, and pixel format for both f0, f1 and IPU planes.

Hm. What I don't get is why the jz4780 doesn't support that equally well with existing code?
To me it looks mainly like an extended jz4740. But I have to admit that I did not study this deeply.

I am happy with a working desktop HDMI setup...

BR,
Nikolaus

Re: [PATCH v12 2/9] drm/ingenic: Add support for JZ4780 and HDMI output

[Paul Boddie]

On Wednesday, 2 February 2022 13:41:21 CET Paul Cercueil wrote:
> Le mer., févr. 2 2022 at 13:33:15 +0100, H. Nikolaus Schaller> 
<hns@goldelico.com> a écrit :
> >>  Am 02.02.2022 um 13:28 schrieb Paul Cercueil <paul@crapouillou.net>:
> >>  
> >>  Note that there are other things not working with your current
> >> 
> >> implementation, for instance you cannot set the X/Y start position
> >> of the f1 plane, which means it's only really usable for fullscreen
> >> desktop/windows.
> > 
> > Is setting x/y possible for the other SoC?
> 
> Yes. They support different x/y positions, sizes, and pixel format for
> both f0, f1 and IPU planes.

One thing worth noting about the JZ4780 is that a lot of the registers that 
might otherwise be used for the above purposes appear to be read-only, at 
least for the different fields concerned.

For example, those affecting ingenic_drm_plane_config:

Control Register (LCDCTRL) - specifically the BPP0 field
OSD Control Register (LCDOSDCTRL)
Foreground 0 XY Position Register (LCDXYP0)
Foreground 1 XY Position Register (LCDXYP1)
Foreground 0 Size Register (LCDSIZE0)
Foreground 1 Size Register (LCDSIZE1)

These require changes to the extended descriptor members instead, and I am 
fairly sure I mentioned the implications for pixel depth configuration 
previously. So, as far as I can tell, we would need to update the descriptors, 
not the registers, to support the operations mentioned above.

As for the f0 plane "not working", I am not aware of any limitation around 
using only f0 (assuming it corresponds to what the manual calls fg0) or only 
f1 (again, assuming fg1 in the manual) or both. My assumption was that for 
this particular driver, f0 was reserved for some kind of overlay and that f1 
was to be used for the normal non-overlay display for products where the OSD 
peripheral is provided.

From the definition of struct ingenic_drm:

        /*
         * f1 (aka. foreground1) is our primary plane, on top of which
         * f0 (aka. foreground0) can be overlayed. Z-order is fixed in
         * hardware and cannot be changed.
         */

So, as I understood it, the driver would configure f1 in the case of the 
JZ4780 for basic display support. Configuring f0 as an overlay should be 
entirely possible, but I imagine that it needs to change the descriptors, not 
the registers, to have a chance of actually working.

I hope this is somewhat useful information. I honestly don't know if, say, the 
JZ4770 has a similar arrangement with regard to configuration via descriptors, 
as opposed to registers, but I think it is an important distinction between 
devices in this particular family that needs to be accommodated in the driver, 
and we obviously want to determine how this might best be achieved.

Paul