U-Boot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] i2c: add HDMI DDC driver for MT8188 and MT8195
@ 2026-07-17 12:33 Julien Stephan
  2026-07-17 12:33 ` [PATCH 1/2] edid: add default EDID_ADDR macro Julien Stephan
  2026-07-17 12:33 ` [PATCH 2/2] i2c: mtk_mt8195_i2c_ddc: add new driver Julien Stephan
  0 siblings, 2 replies; 8+ messages in thread
From: Julien Stephan @ 2026-07-17 12:33 UTC (permalink / raw)
  To: u-boot
  Cc: GSS_MTK_Uboot_upstream, Tom Rini, Ilias Apalodimas, Simon Glass,
	Philipp Tomsich, Kever Yang, Heiko Schocher, Ryder Lee,
	Weijie Gao, Chunfeng Yun, Igor Belwon, David Lechner,
	Casey Connolly, Bastien Curutchet, Pavlo Yadvychuk, Justin Swartz,
	Johan Jonker, Julien Stephan

This series adds an I2C driver for the HDMI DDC (Display Data Channel)
found on MT8188 and MT8195 based SoCs, such as the Genio 510 and Genio
700 EVK. The DDC channel is used to read the sink EDID over the HDMI
connector.

While at it, a generic EDID_ADDR macro is added to edid.h for the
standard 0x50 EDID slave address.

The full HDMI support will be added later, as a separate series to ease
review.

Signed-off-by: Julien Stephan <jstephan@baylibre.com>
---
Julien Stephan (2):
      edid: add default EDID_ADDR macro
      i2c: mtk_mt8195_i2c_ddc: add new driver

 arch/arm/include/asm/arch-rockchip/edp_rk3288.h |   1 -
 drivers/i2c/Kconfig                             |  11 +
 drivers/i2c/Makefile                            |   1 +
 drivers/i2c/mtk_mt8195_i2c_ddc.c                | 398 ++++++++++++++++++++++++
 include/edid.h                                  |   3 +
 5 files changed, 413 insertions(+), 1 deletion(-)
---
base-commit: 78bfaf6281605509fd49262b0138cf1fb338ef62
change-id: 20260716-mt8188-add-ddc-i2c-driver-1a3d2f9e6d1b

Best regards,
--  
Julien Stephan <jstephan@baylibre.com>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2] edid: add default EDID_ADDR macro
  2026-07-17 12:33 [PATCH 0/2] i2c: add HDMI DDC driver for MT8188 and MT8195 Julien Stephan
@ 2026-07-17 12:33 ` Julien Stephan
  2026-07-22  5:10   ` Heiko Schocher via U-Boot
  2026-07-17 12:33 ` [PATCH 2/2] i2c: mtk_mt8195_i2c_ddc: add new driver Julien Stephan
  1 sibling, 1 reply; 8+ messages in thread
From: Julien Stephan @ 2026-07-17 12:33 UTC (permalink / raw)
  To: u-boot
  Cc: GSS_MTK_Uboot_upstream, Tom Rini, Ilias Apalodimas, Simon Glass,
	Philipp Tomsich, Kever Yang, Heiko Schocher, Ryder Lee,
	Weijie Gao, Chunfeng Yun, Igor Belwon, David Lechner,
	Casey Connolly, Bastien Curutchet, Pavlo Yadvychuk, Justin Swartz,
	Johan Jonker, Julien Stephan

EDID info is most of the time stored at address 0x50, so define a macro
for it in the common edid.h header.

Also drop the now-duplicate definition from the Rockchip edp_rk3288.h
header and rely on the common one.

Signed-off-by: Julien Stephan <jstephan@baylibre.com>
---
 arch/arm/include/asm/arch-rockchip/edp_rk3288.h | 1 -
 include/edid.h                                  | 3 +++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/arch-rockchip/edp_rk3288.h b/arch/arm/include/asm/arch-rockchip/edp_rk3288.h
index edacf102852..2332ae30953 100644
--- a/arch/arm/include/asm/arch-rockchip/edp_rk3288.h
+++ b/arch/arm/include/asm/arch-rockchip/edp_rk3288.h
@@ -527,7 +527,6 @@ check_member(rk3288_edp, pll_reg_5, 0xa00);
 #define PLL_LOCK_TIMEOUT 10
 #define DP_INIT_TRIES 10
 
-#define EDID_ADDR				0x50
 #define EDID_LENGTH				0x80
 #define EDID_HEADER				0x00
 #define EDID_EXTENSION_FLAG			0x7e
diff --git a/include/edid.h b/include/edid.h
index cee7c4c7637..77a5d258311 100644
--- a/include/edid.h
+++ b/include/edid.h
@@ -14,6 +14,9 @@
 
 #include <linux/types.h>
 
+/* Default EDID address */
+#define EDID_ADDR	0x50
+
 /* Size of the EDID data */
 #define EDID_SIZE	128
 #define EDID_EXT_SIZE	256

-- 
2.54.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] i2c: mtk_mt8195_i2c_ddc: add new driver
  2026-07-17 12:33 [PATCH 0/2] i2c: add HDMI DDC driver for MT8188 and MT8195 Julien Stephan
  2026-07-17 12:33 ` [PATCH 1/2] edid: add default EDID_ADDR macro Julien Stephan
@ 2026-07-17 12:33 ` Julien Stephan
  2026-07-22  5:20   ` Heiko Schocher via U-Boot
  1 sibling, 1 reply; 8+ messages in thread
From: Julien Stephan @ 2026-07-17 12:33 UTC (permalink / raw)
  To: u-boot
  Cc: GSS_MTK_Uboot_upstream, Tom Rini, Ilias Apalodimas, Simon Glass,
	Philipp Tomsich, Kever Yang, Heiko Schocher, Ryder Lee,
	Weijie Gao, Chunfeng Yun, Igor Belwon, David Lechner,
	Casey Connolly, Bastien Curutchet, Pavlo Yadvychuk, Justin Swartz,
	Johan Jonker, Julien Stephan

Add a new driver for HDMI DDC channel reading for MT8195 based SoCs.
The driver is based on the corresponding kernel driver.

Signed-off-by: Pavlo Yadvychuk <pyadvychuk@baylibre.com>
Signed-off-by: Julien Stephan <jstephan@baylibre.com>
---
 drivers/i2c/Kconfig              |  11 ++
 drivers/i2c/Makefile             |   1 +
 drivers/i2c/mtk_mt8195_i2c_ddc.c | 398 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 410 insertions(+)

diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
index ab5af17858c..9c563655945 100644
--- a/drivers/i2c/Kconfig
+++ b/drivers/i2c/Kconfig
@@ -232,6 +232,17 @@ config SYS_I2C_DAVINCI
 	help
 	  Say yes here to add support for Davinci and Keystone I2C controller
 
+config SYS_I2C_DDC_MTK
+	bool "MediaTek HDMI DDC driver"
+	depends on DM_I2C
+	depends on CLK
+	help
+	  This selects the MediaTek Inter-Integrated Circuit (I2C) bus driver
+	  used for HDMI DDC communication. The driver supports MT8188 and
+	  MT8195 compatible SoCs.
+	  If you want to use the MediaTek DDC interface, say Y here.
+	  If unsure, say N.
+
 config SYS_I2C_DW
 	bool "Designware I2C Controller"
 	help
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
index 2da649e97d3..8185753cd13 100644
--- a/drivers/i2c/Makefile
+++ b/drivers/i2c/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_SYS_I2C_AT91) += at91_i2c.o
 obj-$(CONFIG_SYS_I2C_CADENCE) += i2c-cdns.o
 obj-$(CONFIG_SYS_I2C_CA) += i2c-cortina.o
 obj-$(CONFIG_SYS_I2C_DAVINCI) += davinci_i2c.o
+obj-$(CONFIG_SYS_I2C_DDC_MTK) += mtk_mt8195_i2c_ddc.o
 obj-$(CONFIG_SYS_I2C_DW) += designware_i2c.o
 obj-$(CONFIG_SYS_I2C_DW_PCI) += designware_i2c_pci.o
 obj-$(CONFIG_SYS_I2C_FSL) += fsl_i2c.o
diff --git a/drivers/i2c/mtk_mt8195_i2c_ddc.c b/drivers/i2c/mtk_mt8195_i2c_ddc.c
new file mode 100644
index 00000000000..7b6e57929a9
--- /dev/null
+++ b/drivers/i2c/mtk_mt8195_i2c_ddc.c
@@ -0,0 +1,398 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2021 MediaTek Inc.
+ * Copyright (c) 2026 BayLibre, SAS
+ *
+ */
+
+#include <asm/io.h>
+#include <clk.h>
+#include <dm.h>
+#include <dm/device_compat.h>
+#include <edid.h>
+#include <i2c.h>
+#include <linux/delay.h>
+#include <time.h>
+
+#define DDC2_CLOCK 572 /* BIM=208M/(v*4) = 90Khz */
+#define DDC2_CLOCK_EDID 832 /* BIM=208M/(v*4) = 62.5Khz */
+
+#define SCDC_I2C_SLAVE_ADDRESS 0x54
+
+#define HDCP2X_DDCM_STATUS 0xC68
+
+#define SCDC_CTRL 0xC18
+
+#define CLEAR_FIFO 0x9
+
+#define CLOCK_SCL 0xA
+
+#define ENH_READ_NO_ACK 0x4
+
+#define DDC_CMD GENMASK(31, 28)
+#define DDC_CMD_SHIFT (28)
+#define DDC_CTRL 0xC10
+#define DDC_DATA_OUT GENMASK(23, 16)
+#define DDC_DATA_OUT_SHIFT (16)
+#define DDC_DELAY_CNT GENMASK(31, 16)
+#define DDC_DELAY_CNT_SHIFT (16)
+#define DDC_DIN_CNT_SHIFT (16)
+#define DDC_I2C_BUS_LOW BIT(11)
+#define DDC_I2C_IN_PROG BIT(13)
+#define DDC_I2C_NO_ACK BIT(10)
+#define DDC_OFFSET_SHIFT (8)
+#define DDC_SEGMENT GENMASK(15, 8)
+#define DDC_SEGMENT_SHIFT (8)
+
+#define HPD_DDC_CTRL 0xC08
+#define HPD_DDC_STATUS 0xC60
+
+#define SEQ_READ_NO_ACK 0x2
+#define SEQ_WRITE_REQ_ACK 0x7
+
+#define SI2C_CTRL 0xCAC
+#define SI2C_ADDR_READ (0xF4)
+#define SI2C_ADDR_SHIFT (16)
+#define SI2C_WDATA GENMASK(15, 8)
+#define SI2C_WDATA_SHIFT (8)
+#define SI2C_CONFIRM_READ BIT(2)
+#define SI2C_RD BIT(1)
+#define SI2C_WR BIT(0)
+
+#define HDCP2X_POL_CTRL 0xC54
+#define HDCP2X_DIS_POLL_EN BIT(16)
+
+struct mtk_hdmi_ddc {
+	struct udevice *udev;
+	struct clk clk;
+	void __iomem *regs;
+};
+
+enum sif_bit_t_hdmi {
+	SIF_8_BIT_HDMI, /* 8 bits data address */
+	SIF_16_BIT_HDMI, /* 16 bits data address */
+};
+
+static inline unsigned int mtk_ddc_read(struct mtk_hdmi_ddc *ddc,
+					unsigned int reg)
+{
+	return readl(ddc->regs + reg);
+}
+
+static inline void mtk_ddc_write(struct mtk_hdmi_ddc *ddc, unsigned int reg,
+				 unsigned int val)
+{
+	writel(val, ddc->regs + reg);
+}
+
+static inline void mtk_ddc_mask(struct mtk_hdmi_ddc *ddc, unsigned int reg,
+				unsigned int val, unsigned int mask)
+{
+	unsigned int tmp;
+
+	tmp = readl(ddc->regs + reg) & ~mask;
+	tmp |= (val & mask);
+	writel(tmp, ddc->regs + reg);
+}
+
+static void mtk_ddc_disable_hdcp_polling(struct mtk_hdmi_ddc *ddc)
+{
+	mtk_ddc_mask(ddc, HDCP2X_POL_CTRL, HDCP2X_DIS_POLL_EN,
+		     HDCP2X_DIS_POLL_EN);
+}
+
+static void ddc_wr_one(struct mtk_hdmi_ddc *ddc, unsigned int addr_id,
+		       unsigned int offset_id, unsigned char wr_data)
+{
+	if (mtk_ddc_read(ddc, HDCP2X_DDCM_STATUS) & DDC_I2C_BUS_LOW) {
+		mtk_ddc_mask(ddc, DDC_CTRL, (CLOCK_SCL << DDC_CMD_SHIFT),
+			     DDC_CMD);
+		udelay(300);
+	}
+	mtk_ddc_mask(ddc, HPD_DDC_CTRL, DDC2_CLOCK << DDC_DELAY_CNT_SHIFT,
+		     DDC_DELAY_CNT);
+	mtk_ddc_write(ddc, SI2C_CTRL, SI2C_ADDR_READ << SI2C_ADDR_SHIFT);
+	mtk_ddc_mask(ddc, SI2C_CTRL, wr_data << SI2C_WDATA_SHIFT, SI2C_WDATA);
+	mtk_ddc_mask(ddc, SI2C_CTRL, SI2C_WR, SI2C_WR);
+
+	mtk_ddc_write(ddc, DDC_CTRL,
+		      (SEQ_WRITE_REQ_ACK << DDC_CMD_SHIFT) +
+		      (1 << DDC_DIN_CNT_SHIFT) +
+		      (offset_id << DDC_OFFSET_SHIFT) + (addr_id << 1));
+
+	udelay(1250);
+
+	if ((mtk_ddc_read(ddc, HDCP2X_DDCM_STATUS) &
+	     (DDC_I2C_NO_ACK | DDC_I2C_BUS_LOW))) {
+		if (mtk_ddc_read(ddc, HDCP2X_DDCM_STATUS) & DDC_I2C_BUS_LOW) {
+			mtk_ddc_mask(ddc, DDC_CTRL,
+				     (CLOCK_SCL << DDC_CMD_SHIFT), DDC_CMD);
+			udelay(300);
+		}
+	}
+}
+
+static unsigned int
+ddcm_read_hdmi(struct mtk_hdmi_ddc *ddc, unsigned int u4_clk_div,
+	       unsigned char uc_dev, unsigned int u4_addr,
+	       unsigned char *puc_value, unsigned int u4_count)
+{
+	unsigned int i, temp_length, loop_counter;
+	unsigned int uc_read_count = 0, uc_idx;
+	unsigned long ddc_start_time, ddc_end_time, ddc_timeout;
+
+	if (!puc_value || !u4_count || !u4_clk_div)
+		return 0;
+
+	if (mtk_ddc_read(ddc, HDCP2X_DDCM_STATUS) & DDC_I2C_BUS_LOW) {
+		mtk_ddc_mask(ddc, DDC_CTRL, (CLOCK_SCL << DDC_CMD_SHIFT),
+			     DDC_CMD);
+		udelay(300);
+	}
+
+	mtk_ddc_mask(ddc, DDC_CTRL, (CLEAR_FIFO << DDC_CMD_SHIFT), DDC_CMD);
+
+	if (u4_count >= 16) {
+		temp_length = 16;
+		loop_counter = u4_count / 16 + ((u4_count % 16 == 0) ? 0 : 1);
+	} else {
+		temp_length = u4_count;
+		loop_counter = 1;
+	}
+
+	if (uc_dev >= EDID_ADDR && u4_clk_div < DDC2_CLOCK_EDID)
+		u4_clk_div = DDC2_CLOCK_EDID;
+
+	mtk_ddc_mask(ddc, HPD_DDC_CTRL, u4_clk_div << DDC_DELAY_CNT_SHIFT,
+		     DDC_DELAY_CNT);
+
+	for (i = 0; i < loop_counter; i++) {
+		if (i == (loop_counter - 1) && i != 0 && u4_count % 16)
+			temp_length = u4_count % 16;
+
+		/* EDID_ADDR(0x50) + 1 .. 0x53 select an EDID segment */
+		if (uc_dev > EDID_ADDR && uc_dev <= 0x53) {
+			mtk_ddc_mask(ddc, SCDC_CTRL,
+				     (uc_dev - EDID_ADDR)
+				     << DDC_SEGMENT_SHIFT,
+				     DDC_SEGMENT);
+			mtk_ddc_write(ddc, DDC_CTRL,
+				      (ENH_READ_NO_ACK << DDC_CMD_SHIFT) +
+				      (temp_length << DDC_DIN_CNT_SHIFT) +
+				      ((u4_addr + i * temp_length)
+				       << DDC_OFFSET_SHIFT) +
+				      (EDID_ADDR << 1));
+		} else {
+			mtk_ddc_write(ddc, DDC_CTRL,
+				      (SEQ_READ_NO_ACK << DDC_CMD_SHIFT) +
+				      (temp_length << DDC_DIN_CNT_SHIFT) +
+				      ((u4_addr + i * 16)
+				       << DDC_OFFSET_SHIFT) + (uc_dev << 1));
+		}
+		udelay(5500);
+		ddc_start_time = get_timer(0);
+		/* timeout in ms: about 1 ms per byte plus some margin */
+		ddc_timeout = temp_length + 5;
+		ddc_end_time = ddc_start_time + ddc_timeout;
+		while (1) {
+			if ((mtk_ddc_read(ddc, HPD_DDC_STATUS) &
+			     DDC_I2C_IN_PROG) == 0)
+				break;
+
+			if (time_after(get_timer(0), ddc_end_time)) {
+				dev_err(ddc->udev, "DDC transfer timeout\n");
+				return 0;
+			}
+			udelay(1500);
+		}
+		if ((mtk_ddc_read(ddc, HDCP2X_DDCM_STATUS) &
+		     (DDC_I2C_NO_ACK | DDC_I2C_BUS_LOW))) {
+			if (mtk_ddc_read(ddc, HDCP2X_DDCM_STATUS) &
+			    DDC_I2C_BUS_LOW) {
+				mtk_ddc_mask(ddc, DDC_CTRL,
+					     (CLOCK_SCL << DDC_CMD_SHIFT),
+					     DDC_CMD);
+				udelay(300);
+			}
+			return 0;
+		}
+
+		/* get the DDC data from the FIFO */
+		for (uc_idx = 0; uc_idx < temp_length; uc_idx++) {
+			/* latch the FIFO output */
+			mtk_ddc_write(ddc, SI2C_CTRL,
+				      (SI2C_ADDR_READ << SI2C_ADDR_SHIFT) +
+				      SI2C_RD);
+
+			/* read FIFO output value from DDC_STATUS */
+			puc_value[i * 16 + uc_idx] =
+				(mtk_ddc_read(ddc, HPD_DDC_STATUS) &
+				 DDC_DATA_OUT) >> DDC_DATA_OUT_SHIFT;
+
+			/* increment FIFO read pointer, un-latch the FIFO */
+			mtk_ddc_write(ddc, SI2C_CTRL,
+				      (SI2C_ADDR_READ << SI2C_ADDR_SHIFT) +
+				      SI2C_CONFIRM_READ);
+			/*
+			 * if the hdmi block was reset while reading, the DDC
+			 * speed falls back below DDC2_CLOCK: abort
+			 */
+			if (((mtk_ddc_read(ddc, HPD_DDC_CTRL) >> 16) &
+			     0xFFFF) < DDC2_CLOCK)
+				return 0;
+
+			uc_read_count = i * 16 + uc_idx + 1;
+		}
+	}
+
+	return uc_read_count;
+}
+
+static unsigned int vddc_read(struct mtk_hdmi_ddc *ddc,
+			      unsigned int u4_clk_div, unsigned char uc_dev,
+			      unsigned int u4_addr,
+			      enum sif_bit_t_hdmi uc_addr_type,
+			      unsigned char *puc_value, unsigned int u4_count)
+{
+	unsigned int u4_read_count = 0;
+
+	if (!puc_value || !u4_count || !u4_clk_div)
+		return 0;
+	if (uc_addr_type > SIF_16_BIT_HDMI)
+		return 0;
+	if (uc_addr_type == SIF_8_BIT_HDMI && u4_addr > 255)
+		return 0;
+	if (uc_addr_type == SIF_16_BIT_HDMI && u4_addr > 65535)
+		return 0;
+
+	if (uc_addr_type == SIF_8_BIT_HDMI)
+		u4_read_count = 255 - u4_addr + 1;
+	else if (uc_addr_type == SIF_16_BIT_HDMI)
+		u4_read_count = 65535 - u4_addr + 1;
+
+	u4_read_count = min(u4_read_count, u4_count);
+
+	return ddcm_read_hdmi(ddc, u4_clk_div, uc_dev, u4_addr, puc_value,
+			      u4_read_count);
+}
+
+static int fg_ddc_data_read(struct mtk_hdmi_ddc *ddc, unsigned char b_dev,
+			    unsigned char b_data_addr,
+			    unsigned int b_data_count, unsigned char *pr_data)
+{
+	mtk_ddc_disable_hdcp_polling(ddc);
+	if (vddc_read(ddc, DDC2_CLOCK, b_dev, b_data_addr, SIF_8_BIT_HDMI,
+		      pr_data, b_data_count) != b_data_count)
+		return -EREMOTEIO;
+
+	return 0;
+}
+
+static int fg_ddc_data_write(struct mtk_hdmi_ddc *ddc, unsigned char b_dev,
+			     unsigned char b_data_addr,
+			     unsigned int b_data_count,
+			     unsigned char *pr_data)
+{
+	unsigned int i;
+
+	mtk_ddc_disable_hdcp_polling(ddc);
+	for (i = 0; i < b_data_count; i++)
+		ddc_wr_one(ddc, b_dev, b_data_addr + i, *(pr_data + i));
+
+	return 0;
+}
+
+static int mtk_hdmi_ddc_xfer(struct udevice *dev, struct i2c_msg *msgs, int num)
+{
+	struct mtk_hdmi_ddc *ddc = dev_get_priv(dev);
+	unsigned char offset = 0;
+	int ret;
+	int i;
+
+	if (!msgs)
+		return -EINVAL;
+
+	if (!ddc || !ddc->regs)
+		return -EINVAL;
+
+	for (i = 0; i < num; i++) {
+		struct i2c_msg *msg = &msgs[i];
+
+		if (!msg->buf || !msg->len)
+			return -EINVAL;
+
+		if (msg->flags & I2C_M_RD) {
+			/*
+			 * The underlying DDC hardware always issues a write
+			 * request that assigns the read offset as part of the
+			 * read operation, so use the offset value stored on
+			 * the previous write request.
+			 */
+			ret = fg_ddc_data_read(ddc, msg->addr, offset,
+					       msg->len, &msg->buf[0]);
+		} else {
+			ret = fg_ddc_data_write(ddc, msg->addr, msg->buf[0],
+						msg->len - 1, &msg->buf[1]);
+
+			/*
+			 * store the offset requested by the EDID/SCDC
+			 * framework for use by subsequent read requests
+			 */
+			if ((msg->addr == EDID_ADDR ||
+			     msg->addr == SCDC_I2C_SLAVE_ADDRESS) &&
+			    msg->len == 1)
+				offset = msg->buf[0];
+		}
+
+		if (ret) {
+			dev_err(dev, "ddc transfer failed: %d\n", ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int mtk_hdmi_ddc_probe(struct udevice *dev)
+{
+	struct mtk_hdmi_ddc *ddc = dev_get_priv(dev);
+	int ret;
+
+	ddc->udev = dev;
+	/* the ddc node sits below the hdmi node, which holds the registers */
+	ddc->regs = dev_read_addr_ptr(dev->parent);
+	if (!ddc->regs)
+		return -EINVAL;
+
+	ret = clk_get_by_index(dev, 0, &ddc->clk);
+	if (ret) {
+		dev_err(dev, "failed to get ddc clk: %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_enable(&ddc->clk);
+	if (ret) {
+		dev_err(dev, "failed to enable ddc clk: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct dm_i2c_ops mtk_hdmi_ddc_ops = {
+	.xfer	= mtk_hdmi_ddc_xfer,
+};
+
+static const struct udevice_id mtk_hdmi_ddc_ids[] = {
+	{ .compatible = "mediatek,mt8195-hdmi-ddc", },
+	{ }
+};
+
+U_BOOT_DRIVER(mtk_i2c_ddc) = {
+	.name		= "mtk_i2c_ddc",
+	.id		= UCLASS_I2C,
+	.of_match	= mtk_hdmi_ddc_ids,
+	.probe		= mtk_hdmi_ddc_probe,
+	.priv_auto	= sizeof(struct mtk_hdmi_ddc),
+	.ops		= &mtk_hdmi_ddc_ops,
+};

-- 
2.54.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] edid: add default EDID_ADDR macro
  2026-07-17 12:33 ` [PATCH 1/2] edid: add default EDID_ADDR macro Julien Stephan
@ 2026-07-22  5:10   ` Heiko Schocher via U-Boot
  0 siblings, 0 replies; 8+ messages in thread
From: Heiko Schocher via U-Boot @ 2026-07-22  5:10 UTC (permalink / raw)
  To: Julien Stephan, u-boot
  Cc: GSS_MTK_Uboot_upstream, Tom Rini, Ilias Apalodimas, Simon Glass,
	Philipp Tomsich, Kever Yang, Ryder Lee, Weijie Gao, Chunfeng Yun,
	Igor Belwon, David Lechner, Casey Connolly, Bastien Curutchet,
	Pavlo Yadvychuk, Justin Swartz, Johan Jonker

Hello Julien,

(used the new u-boot ml address u-boot@lists.u-boot-project.org and
dropped the old denx one)

On 17.07.26 14:33, Julien Stephan wrote:
> EDID info is most of the time stored at address 0x50, so define a macro
> for it in the common edid.h header.
> 
> Also drop the now-duplicate definition from the Rockchip edp_rk3288.h
> header and rely on the common one.
> 
> Signed-off-by: Julien Stephan <jstephan@baylibre.com>
> ---
>   arch/arm/include/asm/arch-rockchip/edp_rk3288.h | 1 -
>   include/edid.h                                  | 3 +++
>   2 files changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Heiko Schocher <hs@nabladev.com>

bye,
Heiko
-- 
Nabla Software Engineering
HRB 40522 Augsburg
Phone: +49 821 45592596
E-Mail: office@nabladev.com
Geschäftsführer : Stefano Babic

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] i2c: mtk_mt8195_i2c_ddc: add new driver
  2026-07-17 12:33 ` [PATCH 2/2] i2c: mtk_mt8195_i2c_ddc: add new driver Julien Stephan
@ 2026-07-22  5:20   ` Heiko Schocher via U-Boot
  2026-07-27  8:49     ` Julien Stephan
  0 siblings, 1 reply; 8+ messages in thread
From: Heiko Schocher via U-Boot @ 2026-07-22  5:20 UTC (permalink / raw)
  To: Julien Stephan, u-boot
  Cc: GSS_MTK_Uboot_upstream, Tom Rini, Ilias Apalodimas, Simon Glass,
	Philipp Tomsich, Kever Yang, Ryder Lee, Weijie Gao, Chunfeng Yun,
	Igor Belwon, David Lechner, Casey Connolly, Bastien Curutchet,
	Pavlo Yadvychuk, Justin Swartz, Johan Jonker

Hello Julien,

(used the new u-boot ml address u-boot@lists.u-boot-project.org and
dropped the old denx one)

On 17.07.26 14:33, Julien Stephan wrote:
> Add a new driver for HDMI DDC channel reading for MT8195 based SoCs.
> The driver is based on the corresponding kernel driver.
> 
> Signed-off-by: Pavlo Yadvychuk <pyadvychuk@baylibre.com>
> Signed-off-by: Julien Stephan <jstephan@baylibre.com>
> ---
>   drivers/i2c/Kconfig              |  11 ++
>   drivers/i2c/Makefile             |   1 +
>   drivers/i2c/mtk_mt8195_i2c_ddc.c | 398 +++++++++++++++++++++++++++++++++++++++
>   3 files changed, 410 insertions(+)
> 
[...]
> diff --git a/drivers/i2c/mtk_mt8195_i2c_ddc.c b/drivers/i2c/mtk_mt8195_i2c_ddc.c
> new file mode 100644
> index 00000000000..7b6e57929a9
> --- /dev/null
> +++ b/drivers/i2c/mtk_mt8195_i2c_ddc.c
> @@ -0,0 +1,398 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2021 MediaTek Inc.
> + * Copyright (c) 2026 BayLibre, SAS
> + *
> + */
> +
> +#include <asm/io.h>
> +#include <clk.h>
> +#include <dm.h>
> +#include <dm/device_compat.h>
> +#include <edid.h>
> +#include <i2c.h>
> +#include <linux/delay.h>
> +#include <time.h>
> +
> +#define DDC2_CLOCK 572 /* BIM=208M/(v*4) = 90Khz */
> +#define DDC2_CLOCK_EDID 832 /* BIM=208M/(v*4) = 62.5Khz */

What is v ?

> +
> +#define SCDC_I2C_SLAVE_ADDRESS 0x54
> +
> +#define HDCP2X_DDCM_STATUS 0xC68
> +
> +#define SCDC_CTRL 0xC18
> +
> +#define CLEAR_FIFO 0x9
> +
> +#define CLOCK_SCL 0xA
> +
> +#define ENH_READ_NO_ACK 0x4
> +
> +#define DDC_CMD GENMASK(31, 28)
> +#define DDC_CMD_SHIFT (28)
> +#define DDC_CTRL 0xC10
> +#define DDC_DATA_OUT GENMASK(23, 16)
> +#define DDC_DATA_OUT_SHIFT (16)
> +#define DDC_DELAY_CNT GENMASK(31, 16)
> +#define DDC_DELAY_CNT_SHIFT (16)
> +#define DDC_DIN_CNT_SHIFT (16)
> +#define DDC_I2C_BUS_LOW BIT(11)
> +#define DDC_I2C_IN_PROG BIT(13)
> +#define DDC_I2C_NO_ACK BIT(10)
> +#define DDC_OFFSET_SHIFT (8)
> +#define DDC_SEGMENT GENMASK(15, 8)
> +#define DDC_SEGMENT_SHIFT (8)
> +
> +#define HPD_DDC_CTRL 0xC08
> +#define HPD_DDC_STATUS 0xC60
> +
> +#define SEQ_READ_NO_ACK 0x2
> +#define SEQ_WRITE_REQ_ACK 0x7
> +
> +#define SI2C_CTRL 0xCAC
> +#define SI2C_ADDR_READ (0xF4)
> +#define SI2C_ADDR_SHIFT (16)
> +#define SI2C_WDATA GENMASK(15, 8)
> +#define SI2C_WDATA_SHIFT (8)
> +#define SI2C_CONFIRM_READ BIT(2)
> +#define SI2C_RD BIT(1)
> +#define SI2C_WR BIT(0)
> +
> +#define HDCP2X_POL_CTRL 0xC54
> +#define HDCP2X_DIS_POLL_EN BIT(16)
> +
> +struct mtk_hdmi_ddc {
> +	struct udevice *udev;
> +	struct clk clk;
> +	void __iomem *regs;
> +};
> +
> +enum sif_bit_t_hdmi {
> +	SIF_8_BIT_HDMI, /* 8 bits data address */
> +	SIF_16_BIT_HDMI, /* 16 bits data address */
> +};
> +
> +static inline unsigned int mtk_ddc_read(struct mtk_hdmi_ddc *ddc,
> +					unsigned int reg)
> +{
> +	return readl(ddc->regs + reg);
> +}
> +
> +static inline void mtk_ddc_write(struct mtk_hdmi_ddc *ddc, unsigned int reg,
> +				 unsigned int val)
> +{
> +	writel(val, ddc->regs + reg);
> +}
> +
> +static inline void mtk_ddc_mask(struct mtk_hdmi_ddc *ddc, unsigned int reg,
> +				unsigned int val, unsigned int mask)
> +{
> +	unsigned int tmp;
> +
> +	tmp = readl(ddc->regs + reg) & ~mask;
> +	tmp |= (val & mask);
> +	writel(tmp, ddc->regs + reg);
> +}

May you want to use clrsetbits_* instead?

> +
> +static void mtk_ddc_disable_hdcp_polling(struct mtk_hdmi_ddc *ddc)
> +{
> +	mtk_ddc_mask(ddc, HDCP2X_POL_CTRL, HDCP2X_DIS_POLL_EN,
> +		     HDCP2X_DIS_POLL_EN);
> +}
> +
> +static void ddc_wr_one(struct mtk_hdmi_ddc *ddc, unsigned int addr_id,
> +		       unsigned int offset_id, unsigned char wr_data)
> +{
> +	if (mtk_ddc_read(ddc, HDCP2X_DDCM_STATUS) & DDC_I2C_BUS_LOW) {
> +		mtk_ddc_mask(ddc, DDC_CTRL, (CLOCK_SCL << DDC_CMD_SHIFT),
> +			     DDC_CMD);
> +		udelay(300);
> +	}
> +	mtk_ddc_mask(ddc, HPD_DDC_CTRL, DDC2_CLOCK << DDC_DELAY_CNT_SHIFT,
> +		     DDC_DELAY_CNT);
> +	mtk_ddc_write(ddc, SI2C_CTRL, SI2C_ADDR_READ << SI2C_ADDR_SHIFT);
> +	mtk_ddc_mask(ddc, SI2C_CTRL, wr_data << SI2C_WDATA_SHIFT, SI2C_WDATA);
> +	mtk_ddc_mask(ddc, SI2C_CTRL, SI2C_WR, SI2C_WR);
> +
> +	mtk_ddc_write(ddc, DDC_CTRL,
> +		      (SEQ_WRITE_REQ_ACK << DDC_CMD_SHIFT) +
> +		      (1 << DDC_DIN_CNT_SHIFT) +
> +		      (offset_id << DDC_OFFSET_SHIFT) + (addr_id << 1));
> +
> +	udelay(1250);
> +
> +	if ((mtk_ddc_read(ddc, HDCP2X_DDCM_STATUS) &
> +	     (DDC_I2C_NO_ACK | DDC_I2C_BUS_LOW))) {
> +		if (mtk_ddc_read(ddc, HDCP2X_DDCM_STATUS) & DDC_I2C_BUS_LOW) {
> +			mtk_ddc_mask(ddc, DDC_CTRL,
> +				     (CLOCK_SCL << DDC_CMD_SHIFT), DDC_CMD);
> +			udelay(300);
> +		}
> +	}
> +}

This magic delays... are they documented somewhere?

> +
> +static unsigned int
> +ddcm_read_hdmi(struct mtk_hdmi_ddc *ddc, unsigned int u4_clk_div,
> +	       unsigned char uc_dev, unsigned int u4_addr,
> +	       unsigned char *puc_value, unsigned int u4_count)
> +{
> +	unsigned int i, temp_length, loop_counter;
> +	unsigned int uc_read_count = 0, uc_idx;
> +	unsigned long ddc_start_time, ddc_end_time, ddc_timeout;
> +
> +	if (!puc_value || !u4_count || !u4_clk_div)
> +		return 0;
> +
> +	if (mtk_ddc_read(ddc, HDCP2X_DDCM_STATUS) & DDC_I2C_BUS_LOW) {
> +		mtk_ddc_mask(ddc, DDC_CTRL, (CLOCK_SCL << DDC_CMD_SHIFT),
> +			     DDC_CMD);
> +		udelay(300);
> +	}
> +
> +	mtk_ddc_mask(ddc, DDC_CTRL, (CLEAR_FIFO << DDC_CMD_SHIFT), DDC_CMD);
> +
> +	if (u4_count >= 16) {
> +		temp_length = 16;
> +		loop_counter = u4_count / 16 + ((u4_count % 16 == 0) ? 0 : 1);
> +	} else {
> +		temp_length = u4_count;
> +		loop_counter = 1;
> +	}
> +
> +	if (uc_dev >= EDID_ADDR && u4_clk_div < DDC2_CLOCK_EDID)
> +		u4_clk_div = DDC2_CLOCK_EDID;
> +
> +	mtk_ddc_mask(ddc, HPD_DDC_CTRL, u4_clk_div << DDC_DELAY_CNT_SHIFT,
> +		     DDC_DELAY_CNT);
> +
> +	for (i = 0; i < loop_counter; i++) {
> +		if (i == (loop_counter - 1) && i != 0 && u4_count % 16)
> +			temp_length = u4_count % 16;
> +
> +		/* EDID_ADDR(0x50) + 1 .. 0x53 select an EDID segment */
> +		if (uc_dev > EDID_ADDR && uc_dev <= 0x53) {
> +			mtk_ddc_mask(ddc, SCDC_CTRL,
> +				     (uc_dev - EDID_ADDR)
> +				     << DDC_SEGMENT_SHIFT,
> +				     DDC_SEGMENT);
> +			mtk_ddc_write(ddc, DDC_CTRL,
> +				      (ENH_READ_NO_ACK << DDC_CMD_SHIFT) +
> +				      (temp_length << DDC_DIN_CNT_SHIFT) +
> +				      ((u4_addr + i * temp_length)
> +				       << DDC_OFFSET_SHIFT) +
> +				      (EDID_ADDR << 1));
> +		} else {
> +			mtk_ddc_write(ddc, DDC_CTRL,
> +				      (SEQ_READ_NO_ACK << DDC_CMD_SHIFT) +
> +				      (temp_length << DDC_DIN_CNT_SHIFT) +
> +				      ((u4_addr + i * 16)
> +				       << DDC_OFFSET_SHIFT) + (uc_dev << 1));
> +		}
> +		udelay(5500);
> +		ddc_start_time = get_timer(0);
> +		/* timeout in ms: about 1 ms per byte plus some margin */
> +		ddc_timeout = temp_length + 5;
> +		ddc_end_time = ddc_start_time + ddc_timeout;
> +		while (1) {
> +			if ((mtk_ddc_read(ddc, HPD_DDC_STATUS) &
> +			     DDC_I2C_IN_PROG) == 0)
> +				break;
> +
> +			if (time_after(get_timer(0), ddc_end_time)) {
> +				dev_err(ddc->udev, "DDC transfer timeout\n");
> +				return 0;
> +			}
> +			udelay(1500);
> +		}
> +		if ((mtk_ddc_read(ddc, HDCP2X_DDCM_STATUS) &
> +		     (DDC_I2C_NO_ACK | DDC_I2C_BUS_LOW))) {
> +			if (mtk_ddc_read(ddc, HDCP2X_DDCM_STATUS) &
> +			    DDC_I2C_BUS_LOW) {
> +				mtk_ddc_mask(ddc, DDC_CTRL,
> +					     (CLOCK_SCL << DDC_CMD_SHIFT),
> +					     DDC_CMD);
> +				udelay(300);
> +			}
> +			return 0;
> +		}
> +
> +		/* get the DDC data from the FIFO */
> +		for (uc_idx = 0; uc_idx < temp_length; uc_idx++) {
> +			/* latch the FIFO output */
> +			mtk_ddc_write(ddc, SI2C_CTRL,
> +				      (SI2C_ADDR_READ << SI2C_ADDR_SHIFT) +
> +				      SI2C_RD);
> +
> +			/* read FIFO output value from DDC_STATUS */
> +			puc_value[i * 16 + uc_idx] =
> +				(mtk_ddc_read(ddc, HPD_DDC_STATUS) &
> +				 DDC_DATA_OUT) >> DDC_DATA_OUT_SHIFT;
> +
> +			/* increment FIFO read pointer, un-latch the FIFO */
> +			mtk_ddc_write(ddc, SI2C_CTRL,
> +				      (SI2C_ADDR_READ << SI2C_ADDR_SHIFT) +
> +				      SI2C_CONFIRM_READ);
> +			/*
> +			 * if the hdmi block was reset while reading, the DDC
> +			 * speed falls back below DDC2_CLOCK: abort
> +			 */
> +			if (((mtk_ddc_read(ddc, HPD_DDC_CTRL) >> 16) &
> +			     0xFFFF) < DDC2_CLOCK)
> +				return 0;
> +
> +			uc_read_count = i * 16 + uc_idx + 1;
> +		}
> +	}
> +
> +	return uc_read_count;
> +}
> +
> +static unsigned int vddc_read(struct mtk_hdmi_ddc *ddc,
> +			      unsigned int u4_clk_div, unsigned char uc_dev,
> +			      unsigned int u4_addr,
> +			      enum sif_bit_t_hdmi uc_addr_type,
> +			      unsigned char *puc_value, unsigned int u4_count)
> +{
> +	unsigned int u4_read_count = 0;
> +
> +	if (!puc_value || !u4_count || !u4_clk_div)
> +		return 0;
> +	if (uc_addr_type > SIF_16_BIT_HDMI)
> +		return 0;
> +	if (uc_addr_type == SIF_8_BIT_HDMI && u4_addr > 255)
> +		return 0;
> +	if (uc_addr_type == SIF_16_BIT_HDMI && u4_addr > 65535)
> +		return 0;
> +
> +	if (uc_addr_type == SIF_8_BIT_HDMI)
> +		u4_read_count = 255 - u4_addr + 1;
> +	else if (uc_addr_type == SIF_16_BIT_HDMI)
> +		u4_read_count = 65535 - u4_addr + 1;
> +
> +	u4_read_count = min(u4_read_count, u4_count);
> +
> +	return ddcm_read_hdmi(ddc, u4_clk_div, uc_dev, u4_addr, puc_value,
> +			      u4_read_count);
> +}
> +
> +static int fg_ddc_data_read(struct mtk_hdmi_ddc *ddc, unsigned char b_dev,
> +			    unsigned char b_data_addr,
> +			    unsigned int b_data_count, unsigned char *pr_data)
> +{
> +	mtk_ddc_disable_hdcp_polling(ddc);
> +	if (vddc_read(ddc, DDC2_CLOCK, b_dev, b_data_addr, SIF_8_BIT_HDMI,
> +		      pr_data, b_data_count) != b_data_count)
> +		return -EREMOTEIO;
> +
> +	return 0;
> +}
> +
> +static int fg_ddc_data_write(struct mtk_hdmi_ddc *ddc, unsigned char b_dev,
> +			     unsigned char b_data_addr,
> +			     unsigned int b_data_count,
> +			     unsigned char *pr_data)
> +{
> +	unsigned int i;
> +
> +	mtk_ddc_disable_hdcp_polling(ddc);
> +	for (i = 0; i < b_data_count; i++)
> +		ddc_wr_one(ddc, b_dev, b_data_addr + i, *(pr_data + i));
> +
> +	return 0;
> +}
> +
> +static int mtk_hdmi_ddc_xfer(struct udevice *dev, struct i2c_msg *msgs, int num)
> +{
> +	struct mtk_hdmi_ddc *ddc = dev_get_priv(dev);
> +	unsigned char offset = 0;
> +	int ret;
> +	int i;
> +
> +	if (!msgs)
> +		return -EINVAL;
> +
> +	if (!ddc || !ddc->regs)
> +		return -EINVAL;
> +
> +	for (i = 0; i < num; i++) {
> +		struct i2c_msg *msg = &msgs[i];
> +
> +		if (!msg->buf || !msg->len)
> +			return -EINVAL;
> +
> +		if (msg->flags & I2C_M_RD) {
> +			/*
> +			 * The underlying DDC hardware always issues a write
> +			 * request that assigns the read offset as part of the
> +			 * read operation, so use the offset value stored on
> +			 * the previous write request.
> +			 */
> +			ret = fg_ddc_data_read(ddc, msg->addr, offset,
> +					       msg->len, &msg->buf[0]);
> +		} else {
> +			ret = fg_ddc_data_write(ddc, msg->addr, msg->buf[0],
> +						msg->len - 1, &msg->buf[1]);
> +
> +			/*
> +			 * store the offset requested by the EDID/SCDC
> +			 * framework for use by subsequent read requests
> +			 */
> +			if ((msg->addr == EDID_ADDR ||
> +			     msg->addr == SCDC_I2C_SLAVE_ADDRESS) &&
> +			    msg->len == 1)
> +				offset = msg->buf[0];
> +		}
> +
> +		if (ret) {
> +			dev_err(dev, "ddc transfer failed: %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int mtk_hdmi_ddc_probe(struct udevice *dev)
> +{
> +	struct mtk_hdmi_ddc *ddc = dev_get_priv(dev);
> +	int ret;
> +
> +	ddc->udev = dev;
> +	/* the ddc node sits below the hdmi node, which holds the registers */
> +	ddc->regs = dev_read_addr_ptr(dev->parent);
> +	if (!ddc->regs)
> +		return -EINVAL;
> +
> +	ret = clk_get_by_index(dev, 0, &ddc->clk);
> +	if (ret) {
> +		dev_err(dev, "failed to get ddc clk: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = clk_enable(&ddc->clk);
> +	if (ret) {
> +		dev_err(dev, "failed to enable ddc clk: %d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct dm_i2c_ops mtk_hdmi_ddc_ops = {
> +	.xfer	= mtk_hdmi_ddc_xfer,
> +};
> +
> +static const struct udevice_id mtk_hdmi_ddc_ids[] = {
> +	{ .compatible = "mediatek,mt8195-hdmi-ddc", },
> +	{ }
> +};
> +
> +U_BOOT_DRIVER(mtk_i2c_ddc) = {
> +	.name		= "mtk_i2c_ddc",
> +	.id		= UCLASS_I2C,
> +	.of_match	= mtk_hdmi_ddc_ids,
> +	.probe		= mtk_hdmi_ddc_probe,
> +	.priv_auto	= sizeof(struct mtk_hdmi_ddc),
> +	.ops		= &mtk_hdmi_ddc_ops,
> +};
> 

beside the nitpicks

Reviewed-by: Heiko Schocher <hs@nabladev.com>

Thanks!

bye,
Heiko
-- 
Nabla Software Engineering
HRB 40522 Augsburg
Phone: +49 821 45592596
E-Mail: office@nabladev.com
Geschäftsführer : Stefano Babic

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] i2c: mtk_mt8195_i2c_ddc: add new driver
  2026-07-22  5:20   ` Heiko Schocher via U-Boot
@ 2026-07-27  8:49     ` Julien Stephan
  2026-07-27  9:01       ` Heiko Schocher via U-Boot
  0 siblings, 1 reply; 8+ messages in thread
From: Julien Stephan @ 2026-07-27  8:49 UTC (permalink / raw)
  To: Heiko Schocher
  Cc: u-boot, GSS_MTK_Uboot_upstream, Tom Rini, Ilias Apalodimas,
	Simon Glass, Philipp Tomsich, Kever Yang, Ryder Lee, Weijie Gao,
	Chunfeng Yun, Igor Belwon, David Lechner, Casey Connolly,
	Bastien Curutchet, Pavlo Yadvychuk, Justin Swartz, Johan Jonker

Le mer. 22 juil. 2026 à 07:20, Heiko Schocher <hs@nabladev.com> a écrit :
>
> Hello Julien,
>
> (used the new u-boot ml address u-boot@lists.u-boot-project.org and
> dropped the old denx one)
>
> On 17.07.26 14:33, Julien Stephan wrote:
> > Add a new driver for HDMI DDC channel reading for MT8195 based SoCs.
> > The driver is based on the corresponding kernel driver.
> >
> > Signed-off-by: Pavlo Yadvychuk <pyadvychuk@baylibre.com>
> > Signed-off-by: Julien Stephan <jstephan@baylibre.com>
> > ---
> >   drivers/i2c/Kconfig              |  11 ++
> >   drivers/i2c/Makefile             |   1 +
> >   drivers/i2c/mtk_mt8195_i2c_ddc.c | 398 +++++++++++++++++++++++++++++++++++++++
> >   3 files changed, 410 insertions(+)
> >
> [...]
> > diff --git a/drivers/i2c/mtk_mt8195_i2c_ddc.c b/drivers/i2c/mtk_mt8195_i2c_ddc.c
> > new file mode 100644
> > index 00000000000..7b6e57929a9
> > --- /dev/null
> > +++ b/drivers/i2c/mtk_mt8195_i2c_ddc.c
> > @@ -0,0 +1,398 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2021 MediaTek Inc.
> > + * Copyright (c) 2026 BayLibre, SAS
> > + *
> > + */
> > +
> > +#include <asm/io.h>
> > +#include <clk.h>
> > +#include <dm.h>
> > +#include <dm/device_compat.h>
> > +#include <edid.h>
> > +#include <i2c.h>
> > +#include <linux/delay.h>
> > +#include <time.h>
> > +
> > +#define DDC2_CLOCK 572 /* BIM=208M/(v*4) = 90Khz */
> > +#define DDC2_CLOCK_EDID 832 /* BIM=208M/(v*4) = 62.5Khz */
>
> What is v ?

Hi Heiko,

To be honest, I am not sure.. The code comes from a downstream driver
in MediaTek code base.
The driver is upstream in kernel, and contains the same comment
(defines was renamed during upstream process I guess, but meaning is
the same).
If you want, I can ask MediaTek to clarify this.

>
> > +
> > +#define SCDC_I2C_SLAVE_ADDRESS 0x54
> > +
> > +#define HDCP2X_DDCM_STATUS 0xC68
> > +
> > +#define SCDC_CTRL 0xC18
> > +
> > +#define CLEAR_FIFO 0x9
> > +
> > +#define CLOCK_SCL 0xA
> > +
> > +#define ENH_READ_NO_ACK 0x4
> > +
> > +#define DDC_CMD GENMASK(31, 28)
> > +#define DDC_CMD_SHIFT (28)
> > +#define DDC_CTRL 0xC10
> > +#define DDC_DATA_OUT GENMASK(23, 16)
> > +#define DDC_DATA_OUT_SHIFT (16)
> > +#define DDC_DELAY_CNT GENMASK(31, 16)
> > +#define DDC_DELAY_CNT_SHIFT (16)
> > +#define DDC_DIN_CNT_SHIFT (16)
> > +#define DDC_I2C_BUS_LOW BIT(11)
> > +#define DDC_I2C_IN_PROG BIT(13)
> > +#define DDC_I2C_NO_ACK BIT(10)
> > +#define DDC_OFFSET_SHIFT (8)
> > +#define DDC_SEGMENT GENMASK(15, 8)
> > +#define DDC_SEGMENT_SHIFT (8)
> > +
> > +#define HPD_DDC_CTRL 0xC08
> > +#define HPD_DDC_STATUS 0xC60
> > +
> > +#define SEQ_READ_NO_ACK 0x2
> > +#define SEQ_WRITE_REQ_ACK 0x7
> > +
> > +#define SI2C_CTRL 0xCAC
> > +#define SI2C_ADDR_READ (0xF4)
> > +#define SI2C_ADDR_SHIFT (16)
> > +#define SI2C_WDATA GENMASK(15, 8)
> > +#define SI2C_WDATA_SHIFT (8)
> > +#define SI2C_CONFIRM_READ BIT(2)
> > +#define SI2C_RD BIT(1)
> > +#define SI2C_WR BIT(0)
> > +
> > +#define HDCP2X_POL_CTRL 0xC54
> > +#define HDCP2X_DIS_POLL_EN BIT(16)
> > +
> > +struct mtk_hdmi_ddc {
> > +     struct udevice *udev;
> > +     struct clk clk;
> > +     void __iomem *regs;
> > +};
> > +
> > +enum sif_bit_t_hdmi {
> > +     SIF_8_BIT_HDMI, /* 8 bits data address */
> > +     SIF_16_BIT_HDMI, /* 16 bits data address */
> > +};
> > +
> > +static inline unsigned int mtk_ddc_read(struct mtk_hdmi_ddc *ddc,
> > +                                     unsigned int reg)
> > +{
> > +     return readl(ddc->regs + reg);
> > +}
> > +
> > +static inline void mtk_ddc_write(struct mtk_hdmi_ddc *ddc, unsigned int reg,
> > +                              unsigned int val)
> > +{
> > +     writel(val, ddc->regs + reg);
> > +}
> > +
> > +static inline void mtk_ddc_mask(struct mtk_hdmi_ddc *ddc, unsigned int reg,
> > +                             unsigned int val, unsigned int mask)
> > +{
> > +     unsigned int tmp;
> > +
> > +     tmp = readl(ddc->regs + reg) & ~mask;
> > +     tmp |= (val & mask);
> > +     writel(tmp, ddc->regs + reg);
> > +}
>
> May you want to use clrsetbits_* instead?

thank you, done in v2!

>
> > +
> > +static void mtk_ddc_disable_hdcp_polling(struct mtk_hdmi_ddc *ddc)
> > +{
> > +     mtk_ddc_mask(ddc, HDCP2X_POL_CTRL, HDCP2X_DIS_POLL_EN,
> > +                  HDCP2X_DIS_POLL_EN);
> > +}
> > +
> > +static void ddc_wr_one(struct mtk_hdmi_ddc *ddc, unsigned int addr_id,
> > +                    unsigned int offset_id, unsigned char wr_data)
> > +{
> > +     if (mtk_ddc_read(ddc, HDCP2X_DDCM_STATUS) & DDC_I2C_BUS_LOW) {
> > +             mtk_ddc_mask(ddc, DDC_CTRL, (CLOCK_SCL << DDC_CMD_SHIFT),
> > +                          DDC_CMD);
> > +             udelay(300);
> > +     }
> > +     mtk_ddc_mask(ddc, HPD_DDC_CTRL, DDC2_CLOCK << DDC_DELAY_CNT_SHIFT,
> > +                  DDC_DELAY_CNT);
> > +     mtk_ddc_write(ddc, SI2C_CTRL, SI2C_ADDR_READ << SI2C_ADDR_SHIFT);
> > +     mtk_ddc_mask(ddc, SI2C_CTRL, wr_data << SI2C_WDATA_SHIFT, SI2C_WDATA);
> > +     mtk_ddc_mask(ddc, SI2C_CTRL, SI2C_WR, SI2C_WR);
> > +
> > +     mtk_ddc_write(ddc, DDC_CTRL,
> > +                   (SEQ_WRITE_REQ_ACK << DDC_CMD_SHIFT) +
> > +                   (1 << DDC_DIN_CNT_SHIFT) +
> > +                   (offset_id << DDC_OFFSET_SHIFT) + (addr_id << 1));
> > +
> > +     udelay(1250);
> > +
> > +     if ((mtk_ddc_read(ddc, HDCP2X_DDCM_STATUS) &
> > +          (DDC_I2C_NO_ACK | DDC_I2C_BUS_LOW))) {
> > +             if (mtk_ddc_read(ddc, HDCP2X_DDCM_STATUS) & DDC_I2C_BUS_LOW) {
> > +                     mtk_ddc_mask(ddc, DDC_CTRL,
> > +                                  (CLOCK_SCL << DDC_CMD_SHIFT), DDC_CMD);
> > +                     udelay(300);
> > +             }
> > +     }
> > +}
>
> This magic delays... are they documented somewhere?
>

Same as before, not sure where they are coming from, but Kernel driver
uses the same values.

> > +
> > +static unsigned int
> > +ddcm_read_hdmi(struct mtk_hdmi_ddc *ddc, unsigned int u4_clk_div,
> > +            unsigned char uc_dev, unsigned int u4_addr,
> > +            unsigned char *puc_value, unsigned int u4_count)
> > +{
> > +     unsigned int i, temp_length, loop_counter;
> > +     unsigned int uc_read_count = 0, uc_idx;
> > +     unsigned long ddc_start_time, ddc_end_time, ddc_timeout;
> > +
> > +     if (!puc_value || !u4_count || !u4_clk_div)
> > +             return 0;
> > +
> > +     if (mtk_ddc_read(ddc, HDCP2X_DDCM_STATUS) & DDC_I2C_BUS_LOW) {
> > +             mtk_ddc_mask(ddc, DDC_CTRL, (CLOCK_SCL << DDC_CMD_SHIFT),
> > +                          DDC_CMD);
> > +             udelay(300);
> > +     }
> > +
> > +     mtk_ddc_mask(ddc, DDC_CTRL, (CLEAR_FIFO << DDC_CMD_SHIFT), DDC_CMD);
> > +
> > +     if (u4_count >= 16) {
> > +             temp_length = 16;
> > +             loop_counter = u4_count / 16 + ((u4_count % 16 == 0) ? 0 : 1);
> > +     } else {
> > +             temp_length = u4_count;
> > +             loop_counter = 1;
> > +     }
> > +
> > +     if (uc_dev >= EDID_ADDR && u4_clk_div < DDC2_CLOCK_EDID)
> > +             u4_clk_div = DDC2_CLOCK_EDID;
> > +
> > +     mtk_ddc_mask(ddc, HPD_DDC_CTRL, u4_clk_div << DDC_DELAY_CNT_SHIFT,
> > +                  DDC_DELAY_CNT);
> > +
> > +     for (i = 0; i < loop_counter; i++) {
> > +             if (i == (loop_counter - 1) && i != 0 && u4_count % 16)
> > +                     temp_length = u4_count % 16;
> > +
> > +             /* EDID_ADDR(0x50) + 1 .. 0x53 select an EDID segment */
> > +             if (uc_dev > EDID_ADDR && uc_dev <= 0x53) {
> > +                     mtk_ddc_mask(ddc, SCDC_CTRL,
> > +                                  (uc_dev - EDID_ADDR)
> > +                                  << DDC_SEGMENT_SHIFT,
> > +                                  DDC_SEGMENT);
> > +                     mtk_ddc_write(ddc, DDC_CTRL,
> > +                                   (ENH_READ_NO_ACK << DDC_CMD_SHIFT) +
> > +                                   (temp_length << DDC_DIN_CNT_SHIFT) +
> > +                                   ((u4_addr + i * temp_length)
> > +                                    << DDC_OFFSET_SHIFT) +
> > +                                   (EDID_ADDR << 1));
> > +             } else {
> > +                     mtk_ddc_write(ddc, DDC_CTRL,
> > +                                   (SEQ_READ_NO_ACK << DDC_CMD_SHIFT) +
> > +                                   (temp_length << DDC_DIN_CNT_SHIFT) +
> > +                                   ((u4_addr + i * 16)
> > +                                    << DDC_OFFSET_SHIFT) + (uc_dev << 1));
> > +             }
> > +             udelay(5500);
> > +             ddc_start_time = get_timer(0);
> > +             /* timeout in ms: about 1 ms per byte plus some margin */
> > +             ddc_timeout = temp_length + 5;
> > +             ddc_end_time = ddc_start_time + ddc_timeout;
> > +             while (1) {
> > +                     if ((mtk_ddc_read(ddc, HPD_DDC_STATUS) &
> > +                          DDC_I2C_IN_PROG) == 0)
> > +                             break;
> > +
> > +                     if (time_after(get_timer(0), ddc_end_time)) {
> > +                             dev_err(ddc->udev, "DDC transfer timeout\n");
> > +                             return 0;
> > +                     }
> > +                     udelay(1500);
> > +             }
> > +             if ((mtk_ddc_read(ddc, HDCP2X_DDCM_STATUS) &
> > +                  (DDC_I2C_NO_ACK | DDC_I2C_BUS_LOW))) {
> > +                     if (mtk_ddc_read(ddc, HDCP2X_DDCM_STATUS) &
> > +                         DDC_I2C_BUS_LOW) {
> > +                             mtk_ddc_mask(ddc, DDC_CTRL,
> > +                                          (CLOCK_SCL << DDC_CMD_SHIFT),
> > +                                          DDC_CMD);
> > +                             udelay(300);
> > +                     }
> > +                     return 0;
> > +             }
> > +
> > +             /* get the DDC data from the FIFO */
> > +             for (uc_idx = 0; uc_idx < temp_length; uc_idx++) {
> > +                     /* latch the FIFO output */
> > +                     mtk_ddc_write(ddc, SI2C_CTRL,
> > +                                   (SI2C_ADDR_READ << SI2C_ADDR_SHIFT) +
> > +                                   SI2C_RD);
> > +
> > +                     /* read FIFO output value from DDC_STATUS */
> > +                     puc_value[i * 16 + uc_idx] =
> > +                             (mtk_ddc_read(ddc, HPD_DDC_STATUS) &
> > +                              DDC_DATA_OUT) >> DDC_DATA_OUT_SHIFT;
> > +
> > +                     /* increment FIFO read pointer, un-latch the FIFO */
> > +                     mtk_ddc_write(ddc, SI2C_CTRL,
> > +                                   (SI2C_ADDR_READ << SI2C_ADDR_SHIFT) +
> > +                                   SI2C_CONFIRM_READ);
> > +                     /*
> > +                      * if the hdmi block was reset while reading, the DDC
> > +                      * speed falls back below DDC2_CLOCK: abort
> > +                      */
> > +                     if (((mtk_ddc_read(ddc, HPD_DDC_CTRL) >> 16) &
> > +                          0xFFFF) < DDC2_CLOCK)
> > +                             return 0;
> > +
> > +                     uc_read_count = i * 16 + uc_idx + 1;
> > +             }
> > +     }
> > +
> > +     return uc_read_count;
> > +}
> > +
> > +static unsigned int vddc_read(struct mtk_hdmi_ddc *ddc,
> > +                           unsigned int u4_clk_div, unsigned char uc_dev,
> > +                           unsigned int u4_addr,
> > +                           enum sif_bit_t_hdmi uc_addr_type,
> > +                           unsigned char *puc_value, unsigned int u4_count)
> > +{
> > +     unsigned int u4_read_count = 0;
> > +
> > +     if (!puc_value || !u4_count || !u4_clk_div)
> > +             return 0;
> > +     if (uc_addr_type > SIF_16_BIT_HDMI)
> > +             return 0;
> > +     if (uc_addr_type == SIF_8_BIT_HDMI && u4_addr > 255)
> > +             return 0;
> > +     if (uc_addr_type == SIF_16_BIT_HDMI && u4_addr > 65535)
> > +             return 0;
> > +
> > +     if (uc_addr_type == SIF_8_BIT_HDMI)
> > +             u4_read_count = 255 - u4_addr + 1;
> > +     else if (uc_addr_type == SIF_16_BIT_HDMI)
> > +             u4_read_count = 65535 - u4_addr + 1;
> > +
> > +     u4_read_count = min(u4_read_count, u4_count);
> > +
> > +     return ddcm_read_hdmi(ddc, u4_clk_div, uc_dev, u4_addr, puc_value,
> > +                           u4_read_count);
> > +}
> > +
> > +static int fg_ddc_data_read(struct mtk_hdmi_ddc *ddc, unsigned char b_dev,
> > +                         unsigned char b_data_addr,
> > +                         unsigned int b_data_count, unsigned char *pr_data)
> > +{
> > +     mtk_ddc_disable_hdcp_polling(ddc);
> > +     if (vddc_read(ddc, DDC2_CLOCK, b_dev, b_data_addr, SIF_8_BIT_HDMI,
> > +                   pr_data, b_data_count) != b_data_count)
> > +             return -EREMOTEIO;
> > +
> > +     return 0;
> > +}
> > +
> > +static int fg_ddc_data_write(struct mtk_hdmi_ddc *ddc, unsigned char b_dev,
> > +                          unsigned char b_data_addr,
> > +                          unsigned int b_data_count,
> > +                          unsigned char *pr_data)
> > +{
> > +     unsigned int i;
> > +
> > +     mtk_ddc_disable_hdcp_polling(ddc);
> > +     for (i = 0; i < b_data_count; i++)
> > +             ddc_wr_one(ddc, b_dev, b_data_addr + i, *(pr_data + i));
> > +
> > +     return 0;
> > +}
> > +
> > +static int mtk_hdmi_ddc_xfer(struct udevice *dev, struct i2c_msg *msgs, int num)
> > +{
> > +     struct mtk_hdmi_ddc *ddc = dev_get_priv(dev);
> > +     unsigned char offset = 0;
> > +     int ret;
> > +     int i;
> > +
> > +     if (!msgs)
> > +             return -EINVAL;
> > +
> > +     if (!ddc || !ddc->regs)
> > +             return -EINVAL;
> > +
> > +     for (i = 0; i < num; i++) {
> > +             struct i2c_msg *msg = &msgs[i];
> > +
> > +             if (!msg->buf || !msg->len)
> > +                     return -EINVAL;
> > +
> > +             if (msg->flags & I2C_M_RD) {
> > +                     /*
> > +                      * The underlying DDC hardware always issues a write
> > +                      * request that assigns the read offset as part of the
> > +                      * read operation, so use the offset value stored on
> > +                      * the previous write request.
> > +                      */
> > +                     ret = fg_ddc_data_read(ddc, msg->addr, offset,
> > +                                            msg->len, &msg->buf[0]);
> > +             } else {
> > +                     ret = fg_ddc_data_write(ddc, msg->addr, msg->buf[0],
> > +                                             msg->len - 1, &msg->buf[1]);
> > +
> > +                     /*
> > +                      * store the offset requested by the EDID/SCDC
> > +                      * framework for use by subsequent read requests
> > +                      */
> > +                     if ((msg->addr == EDID_ADDR ||
> > +                          msg->addr == SCDC_I2C_SLAVE_ADDRESS) &&
> > +                         msg->len == 1)
> > +                             offset = msg->buf[0];
> > +             }
> > +
> > +             if (ret) {
> > +                     dev_err(dev, "ddc transfer failed: %d\n", ret);
> > +                     return ret;
> > +             }
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static int mtk_hdmi_ddc_probe(struct udevice *dev)
> > +{
> > +     struct mtk_hdmi_ddc *ddc = dev_get_priv(dev);
> > +     int ret;
> > +
> > +     ddc->udev = dev;
> > +     /* the ddc node sits below the hdmi node, which holds the registers */
> > +     ddc->regs = dev_read_addr_ptr(dev->parent);
> > +     if (!ddc->regs)
> > +             return -EINVAL;
> > +
> > +     ret = clk_get_by_index(dev, 0, &ddc->clk);
> > +     if (ret) {
> > +             dev_err(dev, "failed to get ddc clk: %d\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     ret = clk_enable(&ddc->clk);
> > +     if (ret) {
> > +             dev_err(dev, "failed to enable ddc clk: %d\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static const struct dm_i2c_ops mtk_hdmi_ddc_ops = {
> > +     .xfer   = mtk_hdmi_ddc_xfer,
> > +};
> > +
> > +static const struct udevice_id mtk_hdmi_ddc_ids[] = {
> > +     { .compatible = "mediatek,mt8195-hdmi-ddc", },
> > +     { }
> > +};
> > +
> > +U_BOOT_DRIVER(mtk_i2c_ddc) = {
> > +     .name           = "mtk_i2c_ddc",
> > +     .id             = UCLASS_I2C,
> > +     .of_match       = mtk_hdmi_ddc_ids,
> > +     .probe          = mtk_hdmi_ddc_probe,
> > +     .priv_auto      = sizeof(struct mtk_hdmi_ddc),
> > +     .ops            = &mtk_hdmi_ddc_ops,
> > +};
> >
>
> beside the nitpicks
>
> Reviewed-by: Heiko Schocher <hs@nabladev.com>
>
> Thanks!
>
> bye,
> Heiko
> --
> Nabla Software Engineering
> HRB 40522 Augsburg
> Phone: +49 821 45592596
> E-Mail: office@nabladev.com
> Geschäftsführer : Stefano Babic

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] i2c: mtk_mt8195_i2c_ddc: add new driver
  2026-07-27  8:49     ` Julien Stephan
@ 2026-07-27  9:01       ` Heiko Schocher via U-Boot
  2026-07-27  9:18         ` Alex Sadovsky
  0 siblings, 1 reply; 8+ messages in thread
From: Heiko Schocher via U-Boot @ 2026-07-27  9:01 UTC (permalink / raw)
  To: Julien Stephan
  Cc: u-boot, GSS_MTK_Uboot_upstream, Tom Rini, Ilias Apalodimas,
	Simon Glass, Philipp Tomsich, Kever Yang, Ryder Lee, Weijie Gao,
	Chunfeng Yun, Igor Belwon, David Lechner, Casey Connolly,
	Bastien Curutchet, Pavlo Yadvychuk, Justin Swartz, Johan Jonker

Hello Julien,

On 27.07.26 10:49, Julien Stephan wrote:
> Le mer. 22 juil. 2026 à 07:20, Heiko Schocher <hs@nabladev.com> a écrit :
>>
>> Hello Julien,
>>
>> (used the new u-boot ml address u-boot@lists.u-boot-project.org and
>> dropped the old denx one)
>>
>> On 17.07.26 14:33, Julien Stephan wrote:
>>> Add a new driver for HDMI DDC channel reading for MT8195 based SoCs.
>>> The driver is based on the corresponding kernel driver.
>>>
>>> Signed-off-by: Pavlo Yadvychuk <pyadvychuk@baylibre.com>
>>> Signed-off-by: Julien Stephan <jstephan@baylibre.com>
>>> ---
>>>    drivers/i2c/Kconfig              |  11 ++
>>>    drivers/i2c/Makefile             |   1 +
>>>    drivers/i2c/mtk_mt8195_i2c_ddc.c | 398 +++++++++++++++++++++++++++++++++++++++
>>>    3 files changed, 410 insertions(+)
>>>
>> [...]
>>> diff --git a/drivers/i2c/mtk_mt8195_i2c_ddc.c b/drivers/i2c/mtk_mt8195_i2c_ddc.c
>>> new file mode 100644
>>> index 00000000000..7b6e57929a9
>>> --- /dev/null
>>> +++ b/drivers/i2c/mtk_mt8195_i2c_ddc.c
>>> @@ -0,0 +1,398 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (c) 2021 MediaTek Inc.
>>> + * Copyright (c) 2026 BayLibre, SAS
>>> + *
>>> + */
>>> +
>>> +#include <asm/io.h>
>>> +#include <clk.h>
>>> +#include <dm.h>
>>> +#include <dm/device_compat.h>
>>> +#include <edid.h>
>>> +#include <i2c.h>
>>> +#include <linux/delay.h>
>>> +#include <time.h>
>>> +
>>> +#define DDC2_CLOCK 572 /* BIM=208M/(v*4) = 90Khz */
>>> +#define DDC2_CLOCK_EDID 832 /* BIM=208M/(v*4) = 62.5Khz */
>>
>> What is v ?
> 
> Hi Heiko,
> 
> To be honest, I am not sure.. The code comes from a downstream driver
> in MediaTek code base.
> The driver is upstream in kernel, and contains the same comment
> (defines was renamed during upstream process I guess, but meaning is
> the same).

If it is acceppted in linux, I am fine with that!

> If you want, I can ask MediaTek to clarify this.

It would be nice to know... so if you get this info it would be cool,
thanks!

> 
>>
>>> +
>>> +#define SCDC_I2C_SLAVE_ADDRESS 0x54
>>> +
>>> +#define HDCP2X_DDCM_STATUS 0xC68
>>> +
>>> +#define SCDC_CTRL 0xC18
>>> +
>>> +#define CLEAR_FIFO 0x9
>>> +
>>> +#define CLOCK_SCL 0xA
>>> +
>>> +#define ENH_READ_NO_ACK 0x4
>>> +
>>> +#define DDC_CMD GENMASK(31, 28)
>>> +#define DDC_CMD_SHIFT (28)
>>> +#define DDC_CTRL 0xC10
>>> +#define DDC_DATA_OUT GENMASK(23, 16)
>>> +#define DDC_DATA_OUT_SHIFT (16)
>>> +#define DDC_DELAY_CNT GENMASK(31, 16)
>>> +#define DDC_DELAY_CNT_SHIFT (16)
>>> +#define DDC_DIN_CNT_SHIFT (16)
>>> +#define DDC_I2C_BUS_LOW BIT(11)
>>> +#define DDC_I2C_IN_PROG BIT(13)
>>> +#define DDC_I2C_NO_ACK BIT(10)
>>> +#define DDC_OFFSET_SHIFT (8)
>>> +#define DDC_SEGMENT GENMASK(15, 8)
>>> +#define DDC_SEGMENT_SHIFT (8)
>>> +
>>> +#define HPD_DDC_CTRL 0xC08
>>> +#define HPD_DDC_STATUS 0xC60
>>> +
>>> +#define SEQ_READ_NO_ACK 0x2
>>> +#define SEQ_WRITE_REQ_ACK 0x7
>>> +
>>> +#define SI2C_CTRL 0xCAC
>>> +#define SI2C_ADDR_READ (0xF4)
>>> +#define SI2C_ADDR_SHIFT (16)
>>> +#define SI2C_WDATA GENMASK(15, 8)
>>> +#define SI2C_WDATA_SHIFT (8)
>>> +#define SI2C_CONFIRM_READ BIT(2)
>>> +#define SI2C_RD BIT(1)
>>> +#define SI2C_WR BIT(0)
>>> +
>>> +#define HDCP2X_POL_CTRL 0xC54
>>> +#define HDCP2X_DIS_POLL_EN BIT(16)
>>> +
>>> +struct mtk_hdmi_ddc {
>>> +     struct udevice *udev;
>>> +     struct clk clk;
>>> +     void __iomem *regs;
>>> +};
>>> +
>>> +enum sif_bit_t_hdmi {
>>> +     SIF_8_BIT_HDMI, /* 8 bits data address */
>>> +     SIF_16_BIT_HDMI, /* 16 bits data address */
>>> +};
>>> +
>>> +static inline unsigned int mtk_ddc_read(struct mtk_hdmi_ddc *ddc,
>>> +                                     unsigned int reg)
>>> +{
>>> +     return readl(ddc->regs + reg);
>>> +}
>>> +
>>> +static inline void mtk_ddc_write(struct mtk_hdmi_ddc *ddc, unsigned int reg,
>>> +                              unsigned int val)
>>> +{
>>> +     writel(val, ddc->regs + reg);
>>> +}
>>> +
>>> +static inline void mtk_ddc_mask(struct mtk_hdmi_ddc *ddc, unsigned int reg,
>>> +                             unsigned int val, unsigned int mask)
>>> +{
>>> +     unsigned int tmp;
>>> +
>>> +     tmp = readl(ddc->regs + reg) & ~mask;
>>> +     tmp |= (val & mask);
>>> +     writel(tmp, ddc->regs + reg);
>>> +}
>>
>> May you want to use clrsetbits_* instead?
> 
> thank you, done in v2!
> 
>>
>>> +
>>> +static void mtk_ddc_disable_hdcp_polling(struct mtk_hdmi_ddc *ddc)
>>> +{
>>> +     mtk_ddc_mask(ddc, HDCP2X_POL_CTRL, HDCP2X_DIS_POLL_EN,
>>> +                  HDCP2X_DIS_POLL_EN);
>>> +}
>>> +
>>> +static void ddc_wr_one(struct mtk_hdmi_ddc *ddc, unsigned int addr_id,
>>> +                    unsigned int offset_id, unsigned char wr_data)
>>> +{
>>> +     if (mtk_ddc_read(ddc, HDCP2X_DDCM_STATUS) & DDC_I2C_BUS_LOW) {
>>> +             mtk_ddc_mask(ddc, DDC_CTRL, (CLOCK_SCL << DDC_CMD_SHIFT),
>>> +                          DDC_CMD);
>>> +             udelay(300);
>>> +     }
>>> +     mtk_ddc_mask(ddc, HPD_DDC_CTRL, DDC2_CLOCK << DDC_DELAY_CNT_SHIFT,
>>> +                  DDC_DELAY_CNT);
>>> +     mtk_ddc_write(ddc, SI2C_CTRL, SI2C_ADDR_READ << SI2C_ADDR_SHIFT);
>>> +     mtk_ddc_mask(ddc, SI2C_CTRL, wr_data << SI2C_WDATA_SHIFT, SI2C_WDATA);
>>> +     mtk_ddc_mask(ddc, SI2C_CTRL, SI2C_WR, SI2C_WR);
>>> +
>>> +     mtk_ddc_write(ddc, DDC_CTRL,
>>> +                   (SEQ_WRITE_REQ_ACK << DDC_CMD_SHIFT) +
>>> +                   (1 << DDC_DIN_CNT_SHIFT) +
>>> +                   (offset_id << DDC_OFFSET_SHIFT) + (addr_id << 1));
>>> +
>>> +     udelay(1250);
>>> +
>>> +     if ((mtk_ddc_read(ddc, HDCP2X_DDCM_STATUS) &
>>> +          (DDC_I2C_NO_ACK | DDC_I2C_BUS_LOW))) {
>>> +             if (mtk_ddc_read(ddc, HDCP2X_DDCM_STATUS) & DDC_I2C_BUS_LOW) {
>>> +                     mtk_ddc_mask(ddc, DDC_CTRL,
>>> +                                  (CLOCK_SCL << DDC_CMD_SHIFT), DDC_CMD);
>>> +                     udelay(300);
>>> +             }
>>> +     }
>>> +}
>>
>> This magic delays... are they documented somewhere?
>>
> 
> Same as before, not sure where they are coming from, but Kernel driver
> uses the same values.

Some comment as above!

bye,
Heiko
> 
>>> +
>>> +static unsigned int
>>> +ddcm_read_hdmi(struct mtk_hdmi_ddc *ddc, unsigned int u4_clk_div,
>>> +            unsigned char uc_dev, unsigned int u4_addr,
>>> +            unsigned char *puc_value, unsigned int u4_count)
>>> +{
>>> +     unsigned int i, temp_length, loop_counter;
>>> +     unsigned int uc_read_count = 0, uc_idx;
>>> +     unsigned long ddc_start_time, ddc_end_time, ddc_timeout;
>>> +
>>> +     if (!puc_value || !u4_count || !u4_clk_div)
>>> +             return 0;
>>> +
>>> +     if (mtk_ddc_read(ddc, HDCP2X_DDCM_STATUS) & DDC_I2C_BUS_LOW) {
>>> +             mtk_ddc_mask(ddc, DDC_CTRL, (CLOCK_SCL << DDC_CMD_SHIFT),
>>> +                          DDC_CMD);
>>> +             udelay(300);
>>> +     }
>>> +
>>> +     mtk_ddc_mask(ddc, DDC_CTRL, (CLEAR_FIFO << DDC_CMD_SHIFT), DDC_CMD);
>>> +
>>> +     if (u4_count >= 16) {
>>> +             temp_length = 16;
>>> +             loop_counter = u4_count / 16 + ((u4_count % 16 == 0) ? 0 : 1);
>>> +     } else {
>>> +             temp_length = u4_count;
>>> +             loop_counter = 1;
>>> +     }
>>> +
>>> +     if (uc_dev >= EDID_ADDR && u4_clk_div < DDC2_CLOCK_EDID)
>>> +             u4_clk_div = DDC2_CLOCK_EDID;
>>> +
>>> +     mtk_ddc_mask(ddc, HPD_DDC_CTRL, u4_clk_div << DDC_DELAY_CNT_SHIFT,
>>> +                  DDC_DELAY_CNT);
>>> +
>>> +     for (i = 0; i < loop_counter; i++) {
>>> +             if (i == (loop_counter - 1) && i != 0 && u4_count % 16)
>>> +                     temp_length = u4_count % 16;
>>> +
>>> +             /* EDID_ADDR(0x50) + 1 .. 0x53 select an EDID segment */
>>> +             if (uc_dev > EDID_ADDR && uc_dev <= 0x53) {
>>> +                     mtk_ddc_mask(ddc, SCDC_CTRL,
>>> +                                  (uc_dev - EDID_ADDR)
>>> +                                  << DDC_SEGMENT_SHIFT,
>>> +                                  DDC_SEGMENT);
>>> +                     mtk_ddc_write(ddc, DDC_CTRL,
>>> +                                   (ENH_READ_NO_ACK << DDC_CMD_SHIFT) +
>>> +                                   (temp_length << DDC_DIN_CNT_SHIFT) +
>>> +                                   ((u4_addr + i * temp_length)
>>> +                                    << DDC_OFFSET_SHIFT) +
>>> +                                   (EDID_ADDR << 1));
>>> +             } else {
>>> +                     mtk_ddc_write(ddc, DDC_CTRL,
>>> +                                   (SEQ_READ_NO_ACK << DDC_CMD_SHIFT) +
>>> +                                   (temp_length << DDC_DIN_CNT_SHIFT) +
>>> +                                   ((u4_addr + i * 16)
>>> +                                    << DDC_OFFSET_SHIFT) + (uc_dev << 1));
>>> +             }
>>> +             udelay(5500);
>>> +             ddc_start_time = get_timer(0);
>>> +             /* timeout in ms: about 1 ms per byte plus some margin */
>>> +             ddc_timeout = temp_length + 5;
>>> +             ddc_end_time = ddc_start_time + ddc_timeout;
>>> +             while (1) {
>>> +                     if ((mtk_ddc_read(ddc, HPD_DDC_STATUS) &
>>> +                          DDC_I2C_IN_PROG) == 0)
>>> +                             break;
>>> +
>>> +                     if (time_after(get_timer(0), ddc_end_time)) {
>>> +                             dev_err(ddc->udev, "DDC transfer timeout\n");
>>> +                             return 0;
>>> +                     }
>>> +                     udelay(1500);
>>> +             }
>>> +             if ((mtk_ddc_read(ddc, HDCP2X_DDCM_STATUS) &
>>> +                  (DDC_I2C_NO_ACK | DDC_I2C_BUS_LOW))) {
>>> +                     if (mtk_ddc_read(ddc, HDCP2X_DDCM_STATUS) &
>>> +                         DDC_I2C_BUS_LOW) {
>>> +                             mtk_ddc_mask(ddc, DDC_CTRL,
>>> +                                          (CLOCK_SCL << DDC_CMD_SHIFT),
>>> +                                          DDC_CMD);
>>> +                             udelay(300);
>>> +                     }
>>> +                     return 0;
>>> +             }
>>> +
>>> +             /* get the DDC data from the FIFO */
>>> +             for (uc_idx = 0; uc_idx < temp_length; uc_idx++) {
>>> +                     /* latch the FIFO output */
>>> +                     mtk_ddc_write(ddc, SI2C_CTRL,
>>> +                                   (SI2C_ADDR_READ << SI2C_ADDR_SHIFT) +
>>> +                                   SI2C_RD);
>>> +
>>> +                     /* read FIFO output value from DDC_STATUS */
>>> +                     puc_value[i * 16 + uc_idx] =
>>> +                             (mtk_ddc_read(ddc, HPD_DDC_STATUS) &
>>> +                              DDC_DATA_OUT) >> DDC_DATA_OUT_SHIFT;
>>> +
>>> +                     /* increment FIFO read pointer, un-latch the FIFO */
>>> +                     mtk_ddc_write(ddc, SI2C_CTRL,
>>> +                                   (SI2C_ADDR_READ << SI2C_ADDR_SHIFT) +
>>> +                                   SI2C_CONFIRM_READ);
>>> +                     /*
>>> +                      * if the hdmi block was reset while reading, the DDC
>>> +                      * speed falls back below DDC2_CLOCK: abort
>>> +                      */
>>> +                     if (((mtk_ddc_read(ddc, HPD_DDC_CTRL) >> 16) &
>>> +                          0xFFFF) < DDC2_CLOCK)
>>> +                             return 0;
>>> +
>>> +                     uc_read_count = i * 16 + uc_idx + 1;
>>> +             }
>>> +     }
>>> +
>>> +     return uc_read_count;
>>> +}
>>> +
>>> +static unsigned int vddc_read(struct mtk_hdmi_ddc *ddc,
>>> +                           unsigned int u4_clk_div, unsigned char uc_dev,
>>> +                           unsigned int u4_addr,
>>> +                           enum sif_bit_t_hdmi uc_addr_type,
>>> +                           unsigned char *puc_value, unsigned int u4_count)
>>> +{
>>> +     unsigned int u4_read_count = 0;
>>> +
>>> +     if (!puc_value || !u4_count || !u4_clk_div)
>>> +             return 0;
>>> +     if (uc_addr_type > SIF_16_BIT_HDMI)
>>> +             return 0;
>>> +     if (uc_addr_type == SIF_8_BIT_HDMI && u4_addr > 255)
>>> +             return 0;
>>> +     if (uc_addr_type == SIF_16_BIT_HDMI && u4_addr > 65535)
>>> +             return 0;
>>> +
>>> +     if (uc_addr_type == SIF_8_BIT_HDMI)
>>> +             u4_read_count = 255 - u4_addr + 1;
>>> +     else if (uc_addr_type == SIF_16_BIT_HDMI)
>>> +             u4_read_count = 65535 - u4_addr + 1;
>>> +
>>> +     u4_read_count = min(u4_read_count, u4_count);
>>> +
>>> +     return ddcm_read_hdmi(ddc, u4_clk_div, uc_dev, u4_addr, puc_value,
>>> +                           u4_read_count);
>>> +}
>>> +
>>> +static int fg_ddc_data_read(struct mtk_hdmi_ddc *ddc, unsigned char b_dev,
>>> +                         unsigned char b_data_addr,
>>> +                         unsigned int b_data_count, unsigned char *pr_data)
>>> +{
>>> +     mtk_ddc_disable_hdcp_polling(ddc);
>>> +     if (vddc_read(ddc, DDC2_CLOCK, b_dev, b_data_addr, SIF_8_BIT_HDMI,
>>> +                   pr_data, b_data_count) != b_data_count)
>>> +             return -EREMOTEIO;
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int fg_ddc_data_write(struct mtk_hdmi_ddc *ddc, unsigned char b_dev,
>>> +                          unsigned char b_data_addr,
>>> +                          unsigned int b_data_count,
>>> +                          unsigned char *pr_data)
>>> +{
>>> +     unsigned int i;
>>> +
>>> +     mtk_ddc_disable_hdcp_polling(ddc);
>>> +     for (i = 0; i < b_data_count; i++)
>>> +             ddc_wr_one(ddc, b_dev, b_data_addr + i, *(pr_data + i));
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int mtk_hdmi_ddc_xfer(struct udevice *dev, struct i2c_msg *msgs, int num)
>>> +{
>>> +     struct mtk_hdmi_ddc *ddc = dev_get_priv(dev);
>>> +     unsigned char offset = 0;
>>> +     int ret;
>>> +     int i;
>>> +
>>> +     if (!msgs)
>>> +             return -EINVAL;
>>> +
>>> +     if (!ddc || !ddc->regs)
>>> +             return -EINVAL;
>>> +
>>> +     for (i = 0; i < num; i++) {
>>> +             struct i2c_msg *msg = &msgs[i];
>>> +
>>> +             if (!msg->buf || !msg->len)
>>> +                     return -EINVAL;
>>> +
>>> +             if (msg->flags & I2C_M_RD) {
>>> +                     /*
>>> +                      * The underlying DDC hardware always issues a write
>>> +                      * request that assigns the read offset as part of the
>>> +                      * read operation, so use the offset value stored on
>>> +                      * the previous write request.
>>> +                      */
>>> +                     ret = fg_ddc_data_read(ddc, msg->addr, offset,
>>> +                                            msg->len, &msg->buf[0]);
>>> +             } else {
>>> +                     ret = fg_ddc_data_write(ddc, msg->addr, msg->buf[0],
>>> +                                             msg->len - 1, &msg->buf[1]);
>>> +
>>> +                     /*
>>> +                      * store the offset requested by the EDID/SCDC
>>> +                      * framework for use by subsequent read requests
>>> +                      */
>>> +                     if ((msg->addr == EDID_ADDR ||
>>> +                          msg->addr == SCDC_I2C_SLAVE_ADDRESS) &&
>>> +                         msg->len == 1)
>>> +                             offset = msg->buf[0];
>>> +             }
>>> +
>>> +             if (ret) {
>>> +                     dev_err(dev, "ddc transfer failed: %d\n", ret);
>>> +                     return ret;
>>> +             }
>>> +     }
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int mtk_hdmi_ddc_probe(struct udevice *dev)
>>> +{
>>> +     struct mtk_hdmi_ddc *ddc = dev_get_priv(dev);
>>> +     int ret;
>>> +
>>> +     ddc->udev = dev;
>>> +     /* the ddc node sits below the hdmi node, which holds the registers */
>>> +     ddc->regs = dev_read_addr_ptr(dev->parent);
>>> +     if (!ddc->regs)
>>> +             return -EINVAL;
>>> +
>>> +     ret = clk_get_by_index(dev, 0, &ddc->clk);
>>> +     if (ret) {
>>> +             dev_err(dev, "failed to get ddc clk: %d\n", ret);
>>> +             return ret;
>>> +     }
>>> +
>>> +     ret = clk_enable(&ddc->clk);
>>> +     if (ret) {
>>> +             dev_err(dev, "failed to enable ddc clk: %d\n", ret);
>>> +             return ret;
>>> +     }
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static const struct dm_i2c_ops mtk_hdmi_ddc_ops = {
>>> +     .xfer   = mtk_hdmi_ddc_xfer,
>>> +};
>>> +
>>> +static const struct udevice_id mtk_hdmi_ddc_ids[] = {
>>> +     { .compatible = "mediatek,mt8195-hdmi-ddc", },
>>> +     { }
>>> +};
>>> +
>>> +U_BOOT_DRIVER(mtk_i2c_ddc) = {
>>> +     .name           = "mtk_i2c_ddc",
>>> +     .id             = UCLASS_I2C,
>>> +     .of_match       = mtk_hdmi_ddc_ids,
>>> +     .probe          = mtk_hdmi_ddc_probe,
>>> +     .priv_auto      = sizeof(struct mtk_hdmi_ddc),
>>> +     .ops            = &mtk_hdmi_ddc_ops,
>>> +};
>>>
>>
>> beside the nitpicks
>>
>> Reviewed-by: Heiko Schocher <hs@nabladev.com>
>>
>> Thanks!
>>
>> bye,
>> Heiko
>> --
>> Nabla Software Engineering
>> HRB 40522 Augsburg
>> Phone: +49 821 45592596
>> E-Mail: office@nabladev.com
>> Geschäftsführer : Stefano Babic

-- 
Nabla Software Engineering
HRB 40522 Augsburg
Phone: +49 821 45592596
E-Mail: office@nabladev.com
Geschäftsführer : Stefano Babic

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] i2c: mtk_mt8195_i2c_ddc: add new driver
  2026-07-27  9:01       ` Heiko Schocher via U-Boot
@ 2026-07-27  9:18         ` Alex Sadovsky
  0 siblings, 0 replies; 8+ messages in thread
From: Alex Sadovsky @ 2026-07-27  9:18 UTC (permalink / raw)
  To: u-boot

Hi Heiko and Julien,
>> If you want, I can ask MediaTek to clarify this.
>
> It would be nice to know... so if you get this info it would be cool,
> thanks!

I'm pretty sure "v" is a divider value, yet there's still something interesting missing:
208M / (832 * 4) is exactly 62.5k but 208M / (572 * 4) is closer to 91k than 90k.
Either some nasty details are hidden, or DDC2_CLOCK should be defined as 577 or 578, or comment is slightly imprecise about desired frequency.

> +#define DDC2_CLOCK 572 /* BIM=208M/(v*4) = 90Khz */
> +#define DDC2_CLOCK_EDID 832 /* BIM=208M/(v*4) = 62.5Khz */

Cheers, Alex.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-07-27  9:19 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-17 12:33 [PATCH 0/2] i2c: add HDMI DDC driver for MT8188 and MT8195 Julien Stephan
2026-07-17 12:33 ` [PATCH 1/2] edid: add default EDID_ADDR macro Julien Stephan
2026-07-22  5:10   ` Heiko Schocher via U-Boot
2026-07-17 12:33 ` [PATCH 2/2] i2c: mtk_mt8195_i2c_ddc: add new driver Julien Stephan
2026-07-22  5:20   ` Heiko Schocher via U-Boot
2026-07-27  8:49     ` Julien Stephan
2026-07-27  9:01       ` Heiko Schocher via U-Boot
2026-07-27  9:18         ` Alex Sadovsky

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox