[PATCH 0/5] Add HDMI CEC support to Rockchip RK3588/RK3576 SoCs

[PATCH 0/5] Add HDMI CEC support to Rockchip RK3588/RK3576 SoCs

[Cristian Ciocaltea]

The first two patches in the series implement the CEC capability of the
Synopsys DesignWare HDMI QP TX controller found in RK3588 & RK3576 Socs.

This is based on the downstream code, but rewritten on top of the CEC
helpers added recently to the DRM HDMI connector framework.

The next two patches are needed for RK3576 in order to fixup the timer
base setup according to the actual reference clock rate, which differs
slightly from RK3588.

Please note the CEC helpers are currently affected by a resource
deallocation issue which may crash the kernel and freeze the system
under certain test conditions.  I've already submitted a patch [1] that
seems to correct the problem.

[1] https://lore.kernel.org/all/20250703-hdmi-cec-helper-unreg-fix-v1-1-7e7b0eb578bb@collabora.com/

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
Cristian Ciocaltea (5):
      drm/rockchip: dw_hdmi_qp: Provide CEC IRQ in dw_hdmi_qp_plat_data
      drm/bridge: dw-hdmi-qp: Add CEC support
      drm/rockchip: dw_hdmi_qp: Provide ref clock rate in dw_hdmi_qp_plat_data
      drm/bridge: dw-hdmi-qp: Fixup timer base setup
      arm64: defconfig: Enable DW HDMI QP CEC support

 arch/arm64/configs/defconfig                   |   1 +
 drivers/gpu/drm/bridge/synopsys/Kconfig        |   8 +
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c   | 231 ++++++++++++++++++++++++-
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.h   |  14 ++
 drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c |  17 +-
 include/drm/bridge/dw_hdmi_qp.h                |   2 +
 6 files changed, 269 insertions(+), 4 deletions(-)
---
base-commit: 8d6c58332c7a8ba025fcfa76888b6c37dbce9633
change-id: 20250703-rk3588-hdmi-cec-cea8f523df48

[PATCH 1/5] drm/rockchip: dw_hdmi_qp: Provide CEC IRQ in dw_hdmi_qp_plat_data

[Cristian Ciocaltea]

In preparation to support the CEC interface of the DesignWare HDMI QP IP
block, extend the platform data to provide the required IRQ number.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 4 ++++
 include/drm/bridge/dw_hdmi_qp.h                | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
index 7d531b6f4c098c6c548788dad487ce4613a2f32b..126e556025961e8645f3567b4d7a1c73cc2f2e7f 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
@@ -539,6 +539,10 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 	if (plat_data.main_irq < 0)
 		return plat_data.main_irq;
 
+	plat_data.cec_irq = platform_get_irq_byname(pdev, "cec");
+	if (plat_data.cec_irq < 0)
+		return plat_data.cec_irq;
+
 	irq = platform_get_irq_byname(pdev, "hpd");
 	if (irq < 0)
 		return irq;
diff --git a/include/drm/bridge/dw_hdmi_qp.h b/include/drm/bridge/dw_hdmi_qp.h
index e9be6d507ad9cdc55f5c7d6d3ef37eba41f1ce74..b4a9b739734ec7b67013b683fe6017551aa19172 100644
--- a/include/drm/bridge/dw_hdmi_qp.h
+++ b/include/drm/bridge/dw_hdmi_qp.h
@@ -23,6 +23,7 @@ struct dw_hdmi_qp_plat_data {
 	const struct dw_hdmi_qp_phy_ops *phy_ops;
 	void *phy_data;
 	int main_irq;
+	int cec_irq;
 };
 
 struct dw_hdmi_qp *dw_hdmi_qp_bind(struct platform_device *pdev,

-- 
2.50.0

[PATCH 2/5] drm/bridge: dw-hdmi-qp: Add CEC support

[Cristian Ciocaltea]

Add support for the CEC interface of the Synopsys DesignWare HDMI QP TX
controller.

This is based on the downstream implementation, but rewritten on top of
the CEC helpers added recently to the DRM HDMI connector framework.

Co-developed-by: Algea Cao <algea.cao@rock-chips.com>
Signed-off-by: Algea Cao <algea.cao@rock-chips.com>
Co-developed-by: Derek Foreman <derek.foreman@collabora.com>
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/bridge/synopsys/Kconfig      |   8 +
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 220 +++++++++++++++++++++++++++
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.h |  14 ++
 3 files changed, 242 insertions(+)

diff --git a/drivers/gpu/drm/bridge/synopsys/Kconfig b/drivers/gpu/drm/bridge/synopsys/Kconfig
index f3ab2f985f8ca9dc1eeac3bda6b4a31d355cd51c..99878f051067e65fa3b97d8132be8cfa15980966 100644
--- a/drivers/gpu/drm/bridge/synopsys/Kconfig
+++ b/drivers/gpu/drm/bridge/synopsys/Kconfig
@@ -54,6 +54,14 @@ config DRM_DW_HDMI_QP
 	select DRM_KMS_HELPER
 	select REGMAP_MMIO
 
+config DRM_DW_HDMI_QP_CEC
+	bool "Synopsis Designware QP CEC interface"
+	depends on DRM_DW_HDMI_QP
+	select DRM_DISPLAY_HDMI_CEC_HELPER
+	help
+	  Support the CEC interface which is part of the Synopsys
+	  Designware HDMI QP block.
+
 config DRM_DW_MIPI_DSI
 	tristate
 	select DRM_KMS_HELPER
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
index 7ade80f02a94c91905c13b4a945c65da5681b183..cfe8171b2183874517f583f284f7728fe1613c91 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
@@ -18,6 +18,7 @@
 
 #include <drm/bridge/dw_hdmi_qp.h>
 #include <drm/display/drm_hdmi_helper.h>
+#include <drm/display/drm_hdmi_cec_helper.h>
 #include <drm/display/drm_hdmi_state_helper.h>
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
@@ -26,6 +27,8 @@
 #include <drm/drm_edid.h>
 #include <drm/drm_modes.h>
 
+#include <media/cec.h>
+
 #include <sound/hdmi-codec.h>
 
 #include "dw-hdmi-qp.h"
@@ -131,12 +134,28 @@ struct dw_hdmi_qp_i2c {
 	bool			is_segment;
 };
 
+#ifdef CONFIG_DRM_DW_HDMI_QP_CEC
+struct dw_hdmi_qp_cec {
+	struct drm_connector *connector;
+	int irq;
+	u32 addresses;
+	struct cec_msg rx_msg;
+	u8 tx_status;
+	bool tx_done;
+	bool rx_done;
+};
+#endif
+
 struct dw_hdmi_qp {
 	struct drm_bridge bridge;
 
 	struct device *dev;
 	struct dw_hdmi_qp_i2c *i2c;
 
+#ifdef CONFIG_DRM_DW_HDMI_QP_CEC
+	struct dw_hdmi_qp_cec *cec;
+#endif
+
 	struct {
 		const struct dw_hdmi_qp_phy_ops *ops;
 		void *data;
@@ -965,6 +984,191 @@ static int dw_hdmi_qp_bridge_write_infoframe(struct drm_bridge *bridge,
 	}
 }
 
+#ifdef CONFIG_DRM_DW_HDMI_QP_CEC
+static irqreturn_t dw_hdmi_qp_cec_hardirq(int irq, void *dev_id)
+{
+	struct dw_hdmi_qp *hdmi = dev_id;
+	struct dw_hdmi_qp_cec *cec = hdmi->cec;
+	irqreturn_t ret = IRQ_HANDLED;
+	u32 stat;
+
+	stat = dw_hdmi_qp_read(hdmi, CEC_INT_STATUS);
+	if (stat == 0)
+		return IRQ_NONE;
+
+	dw_hdmi_qp_write(hdmi, stat, CEC_INT_CLEAR);
+
+	if (stat & CEC_STAT_LINE_ERR) {
+		cec->tx_status = CEC_TX_STATUS_ERROR;
+		cec->tx_done = true;
+		ret = IRQ_WAKE_THREAD;
+	} else if (stat & CEC_STAT_DONE) {
+		cec->tx_status = CEC_TX_STATUS_OK;
+		cec->tx_done = true;
+		ret = IRQ_WAKE_THREAD;
+	} else if (stat & CEC_STAT_NACK) {
+		cec->tx_status = CEC_TX_STATUS_NACK;
+		cec->tx_done = true;
+		ret = IRQ_WAKE_THREAD;
+	}
+
+	if (stat & CEC_STAT_EOM) {
+		unsigned int len, i, val;
+
+		val = dw_hdmi_qp_read(hdmi, CEC_RX_COUNT_STATUS);
+		len = (val & 0xf) + 1;
+
+		if (len > sizeof(cec->rx_msg.msg))
+			len = sizeof(cec->rx_msg.msg);
+
+		for (i = 0; i < 4; i++) {
+			val = dw_hdmi_qp_read(hdmi, CEC_RX_DATA3_0 + i * 4);
+			cec->rx_msg.msg[i * 4] = val & 0xff;
+			cec->rx_msg.msg[i * 4 + 1] = (val >> 8) & 0xff;
+			cec->rx_msg.msg[i * 4 + 2] = (val >> 16) & 0xff;
+			cec->rx_msg.msg[i * 4 + 3] = (val >> 24) & 0xff;
+		}
+
+		dw_hdmi_qp_write(hdmi, 1, CEC_LOCK_CONTROL);
+
+		cec->rx_msg.len = len;
+		cec->rx_done = true;
+
+		ret = IRQ_WAKE_THREAD;
+	}
+
+	return ret;
+}
+
+static irqreturn_t dw_hdmi_qp_cec_thread(int irq, void *dev_id)
+{
+	struct dw_hdmi_qp *hdmi = dev_id;
+	struct dw_hdmi_qp_cec *cec = hdmi->cec;
+
+	if (cec->tx_done) {
+		cec->tx_done = false;
+		drm_connector_hdmi_cec_transmit_attempt_done(cec->connector,
+							     cec->tx_status);
+	}
+
+	if (cec->rx_done) {
+		cec->rx_done = false;
+		drm_connector_hdmi_cec_received_msg(cec->connector, &cec->rx_msg);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int dw_hdmi_qp_cec_init(struct drm_connector *connector,
+			       struct drm_bridge *bridge)
+{
+	struct dw_hdmi_qp *hdmi = dw_hdmi_qp_from_bridge(bridge);
+	struct dw_hdmi_qp_cec *cec = hdmi->cec;
+	int ret;
+
+	if (cec->irq < 0) {
+		dev_err(hdmi->dev, "Invalid cec irq: %d\n", cec->irq);
+		return -EINVAL;
+	}
+
+	cec->connector = connector;
+
+	dw_hdmi_qp_write(hdmi, 0, CEC_TX_COUNT);
+	dw_hdmi_qp_write(hdmi, ~0, CEC_INT_CLEAR);
+	dw_hdmi_qp_write(hdmi, 0, CEC_INT_MASK_N);
+
+	ret = devm_request_threaded_irq(hdmi->dev, cec->irq,
+					dw_hdmi_qp_cec_hardirq,
+					dw_hdmi_qp_cec_thread, IRQF_SHARED,
+					dev_name(hdmi->dev), hdmi);
+	if (ret < 0) {
+		dev_err(hdmi->dev, "Request cec irq thread failed: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int dw_hdmi_qp_cec_log_addr(struct drm_bridge *bridge, u8 logical_addr)
+{
+	struct dw_hdmi_qp *hdmi = dw_hdmi_qp_from_bridge(bridge);
+	struct dw_hdmi_qp_cec *cec = hdmi->cec;
+
+	if (logical_addr == CEC_LOG_ADDR_INVALID)
+		cec->addresses = 0;
+	else
+		cec->addresses |= BIT(logical_addr) | CEC_ADDR_BROADCAST;
+
+	dw_hdmi_qp_write(hdmi, cec->addresses, CEC_ADDR);
+
+	return 0;
+}
+
+static int dw_hdmi_qp_cec_enable(struct drm_bridge *bridge, bool enable)
+{
+	struct dw_hdmi_qp *hdmi = dw_hdmi_qp_from_bridge(bridge);
+	unsigned int irqs;
+	u32 swdisable;
+
+	if (!enable) {
+		dw_hdmi_qp_write(hdmi, 0, CEC_INT_MASK_N);
+		dw_hdmi_qp_write(hdmi, ~0, CEC_INT_CLEAR);
+
+		swdisable = dw_hdmi_qp_read(hdmi, GLOBAL_SWDISABLE);
+		swdisable = swdisable | CEC_SWDISABLE;
+		dw_hdmi_qp_write(hdmi, swdisable, GLOBAL_SWDISABLE);
+	} else {
+		swdisable = dw_hdmi_qp_read(hdmi, GLOBAL_SWDISABLE);
+		swdisable = swdisable & ~CEC_SWDISABLE;
+		dw_hdmi_qp_write(hdmi, swdisable, GLOBAL_SWDISABLE);
+
+		dw_hdmi_qp_write(hdmi, ~0, CEC_INT_CLEAR);
+		dw_hdmi_qp_write(hdmi, 1, CEC_LOCK_CONTROL);
+
+		dw_hdmi_qp_cec_log_addr(bridge, CEC_LOG_ADDR_INVALID);
+
+		irqs = CEC_STAT_LINE_ERR | CEC_STAT_NACK | CEC_STAT_EOM |
+		       CEC_STAT_DONE;
+		dw_hdmi_qp_write(hdmi, ~0, CEC_INT_CLEAR);
+		dw_hdmi_qp_write(hdmi, irqs, CEC_INT_MASK_N);
+	}
+
+	return 0;
+}
+
+static int dw_hdmi_qp_cec_transmit(struct drm_bridge *bridge, u8 attempts,
+				   u32 signal_free_time, struct cec_msg *msg)
+{
+	struct dw_hdmi_qp *hdmi = dw_hdmi_qp_from_bridge(bridge);
+	unsigned int i;
+	u32 val;
+
+	for (i = 0; i < msg->len; i++) {
+		if (!(i % 4))
+			val = msg->msg[i];
+		if ((i % 4) == 1)
+			val |= msg->msg[i] << 8;
+		if ((i % 4) == 2)
+			val |= msg->msg[i] << 16;
+		if ((i % 4) == 3)
+			val |= msg->msg[i] << 24;
+
+		if (i == (msg->len - 1) || (i % 4) == 3)
+			dw_hdmi_qp_write(hdmi, val, CEC_TX_DATA3_0 + (i / 4) * 4);
+	}
+
+	dw_hdmi_qp_write(hdmi, msg->len - 1, CEC_TX_COUNT);
+	dw_hdmi_qp_write(hdmi, CEC_CTRL_START, CEC_TX_CONTROL);
+
+	return 0;
+}
+#else
+#define dw_hdmi_qp_cec_init NULL
+#define dw_hdmi_qp_cec_enable NULL
+#define dw_hdmi_qp_cec_log_addr NULL
+#define dw_hdmi_qp_cec_transmit NULL
+#endif /* CONFIG_DRM_DW_HDMI_QP_CEC */
+
 static const struct drm_bridge_funcs dw_hdmi_qp_bridge_funcs = {
 	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
@@ -979,6 +1183,10 @@ static const struct drm_bridge_funcs dw_hdmi_qp_bridge_funcs = {
 	.hdmi_audio_startup = dw_hdmi_qp_audio_enable,
 	.hdmi_audio_shutdown = dw_hdmi_qp_audio_disable,
 	.hdmi_audio_prepare = dw_hdmi_qp_audio_prepare,
+	.hdmi_cec_init = dw_hdmi_qp_cec_init,
+	.hdmi_cec_enable = dw_hdmi_qp_cec_enable,
+	.hdmi_cec_log_addr = dw_hdmi_qp_cec_log_addr,
+	.hdmi_cec_transmit = dw_hdmi_qp_cec_transmit,
 };
 
 static irqreturn_t dw_hdmi_qp_main_hardirq(int irq, void *dev_id)
@@ -1093,6 +1301,18 @@ struct dw_hdmi_qp *dw_hdmi_qp_bind(struct platform_device *pdev,
 	hdmi->bridge.hdmi_audio_dev = dev;
 	hdmi->bridge.hdmi_audio_dai_port = 1;
 
+#ifdef CONFIG_DRM_DW_HDMI_QP_CEC
+	hdmi->bridge.ops |= DRM_BRIDGE_OP_HDMI_CEC_ADAPTER;
+	hdmi->bridge.hdmi_cec_dev = dev;
+	hdmi->bridge.hdmi_cec_adapter_name = dev_name(dev);
+
+	hdmi->cec = devm_kzalloc(hdmi->dev, sizeof(*hdmi->cec), GFP_KERNEL);
+	if (!hdmi->cec)
+		return ERR_PTR(-ENOMEM);
+
+	hdmi->cec->irq = plat_data->cec_irq;
+#endif
+
 	ret = devm_drm_bridge_add(dev, &hdmi->bridge);
 	if (ret)
 		return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.h
index 72987e6c468928f2b998099697a6f32726411557..91a15f82e32acc32eef58f11ec5ca958337ebb9a 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.h
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.h
@@ -488,9 +488,23 @@
 #define AUDPKT_VBIT_OVR0				0xf24
 /* CEC Registers */
 #define CEC_TX_CONTROL					0x1000
+#define CEC_CTRL_CLEAR					BIT(0)
+#define CEC_CTRL_START					BIT(0)
 #define CEC_STATUS					0x1004
+#define CEC_STAT_DONE					BIT(0)
+#define CEC_STAT_NACK					BIT(1)
+#define CEC_STAT_ARBLOST				BIT(2)
+#define CEC_STAT_LINE_ERR				BIT(3)
+#define CEC_STAT_RETRANS_FAIL				BIT(4)
+#define CEC_STAT_DISCARD				BIT(5)
+#define CEC_STAT_TX_BUSY				BIT(8)
+#define CEC_STAT_RX_BUSY				BIT(9)
+#define CEC_STAT_DRIVE_ERR				BIT(10)
+#define CEC_STAT_EOM					BIT(11)
+#define CEC_STAT_NOTIFY_ERR				BIT(12)
 #define CEC_CONFIG					0x1008
 #define CEC_ADDR					0x100c
+#define CEC_ADDR_BROADCAST				BIT(15)
 #define CEC_TX_COUNT					0x1020
 #define CEC_TX_DATA3_0					0x1024
 #define CEC_TX_DATA7_4					0x1028

-- 
2.50.0

[PATCH 3/5] drm/rockchip: dw_hdmi_qp: Provide ref clock rate in dw_hdmi_qp_plat_data

[Cristian Ciocaltea]

In order to support correct initialization of the timer base in the HDMI
QP IP block, extend the platform data to provide the necessary reference
clock rate.

While at it, ensure plat_data is zero-initialized in
dw_hdmi_qp_rockchip_bind().

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 13 ++++++++++++-
 include/drm/bridge/dw_hdmi_qp.h                |  1 +
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
index 126e556025961e8645f3567b4d7a1c73cc2f2e7f..8c1185490009c5f1bc658998a868f8b18dc479a3 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
@@ -431,8 +431,8 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 				    void *data)
 {
 	struct platform_device *pdev = to_platform_device(dev);
+	struct dw_hdmi_qp_plat_data plat_data = {};
 	const struct rockchip_hdmi_qp_cfg *cfg;
-	struct dw_hdmi_qp_plat_data plat_data;
 	struct drm_device *drm = data;
 	struct drm_connector *connector;
 	struct drm_encoder *encoder;
@@ -515,6 +515,17 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 		return ret;
 	}
 
+	for (i = 0; i < ret; i++) {
+		if (!strcmp(clks[i].id, "ref")) {
+			plat_data.ref_clk_rate = clk_get_rate(clks[i].clk);
+			break;
+		}
+	}
+	if (!plat_data.ref_clk_rate) {
+		dev_err(hdmi->dev, "Missing ref clock\n");
+		return -EINVAL;
+	}
+
 	hdmi->enable_gpio = devm_gpiod_get_optional(hdmi->dev, "enable",
 						    GPIOD_OUT_HIGH);
 	if (IS_ERR(hdmi->enable_gpio)) {
diff --git a/include/drm/bridge/dw_hdmi_qp.h b/include/drm/bridge/dw_hdmi_qp.h
index b4a9b739734ec7b67013b683fe6017551aa19172..76ecf31301997718604a05f70ce9eab8695e26b5 100644
--- a/include/drm/bridge/dw_hdmi_qp.h
+++ b/include/drm/bridge/dw_hdmi_qp.h
@@ -24,6 +24,7 @@ struct dw_hdmi_qp_plat_data {
 	void *phy_data;
 	int main_irq;
 	int cec_irq;
+	unsigned long ref_clk_rate;
 };
 
 struct dw_hdmi_qp *dw_hdmi_qp_bind(struct platform_device *pdev,

-- 
2.50.0

[PATCH 4/5] drm/bridge: dw-hdmi-qp: Fixup timer base setup

[Cristian Ciocaltea]

Currently the TIMER_BASE_CONFIG0 register gets initialized to a fixed
value as initially found in vendor driver code supporting the RK3588
SoC.  As a matter of fact the value matches the rate of the HDMI TX
reference clock, which is roughly 428.57 MHz.

However, on RK3576 SoC that rate is slightly lower, i.e. 396.00 MHz, and
the incorrect register configuration breaks CEC functionality.

Set the timer base according to the actual reference clock rate.  While
at it, also drop the unnecessary empty lines in dw_hdmi_qp_init_hw().

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
index cfe8171b2183874517f583f284f7728fe1613c91..5d252800168aa4f3c10f458631795de8912e29d4 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
@@ -161,6 +161,7 @@ struct dw_hdmi_qp {
 		void *data;
 	} phy;
 
+	unsigned long ref_clk_rate;
 	struct regmap *regm;
 
 	unsigned long tmds_char_rate;
@@ -1222,13 +1223,11 @@ static void dw_hdmi_qp_init_hw(struct dw_hdmi_qp *hdmi)
 {
 	dw_hdmi_qp_write(hdmi, 0, MAINUNIT_0_INT_MASK_N);
 	dw_hdmi_qp_write(hdmi, 0, MAINUNIT_1_INT_MASK_N);
-	dw_hdmi_qp_write(hdmi, 428571429, TIMER_BASE_CONFIG0);
+	dw_hdmi_qp_write(hdmi, hdmi->ref_clk_rate, TIMER_BASE_CONFIG0);
 
 	/* Software reset */
 	dw_hdmi_qp_write(hdmi, 0x01, I2CM_CONTROL0);
-
 	dw_hdmi_qp_write(hdmi, 0x085c085c, I2CM_FM_SCL_CONFIG0);
-
 	dw_hdmi_qp_mod(hdmi, 0, I2CM_FM_EN, I2CM_INTERFACE_CONTROL0);
 
 	/* Clear DONE and ERROR interrupts */
@@ -1254,6 +1253,11 @@ struct dw_hdmi_qp *dw_hdmi_qp_bind(struct platform_device *pdev,
 		return ERR_PTR(-ENODEV);
 	}
 
+	if (!plat_data->ref_clk_rate) {
+		dev_err(dev, "Missing ref_clk rate\n");
+		return ERR_PTR(-ENODEV);
+	}
+
 	hdmi = devm_drm_bridge_alloc(dev, struct dw_hdmi_qp, bridge,
 				     &dw_hdmi_qp_bridge_funcs);
 	if (IS_ERR(hdmi))
@@ -1273,6 +1277,7 @@ struct dw_hdmi_qp *dw_hdmi_qp_bind(struct platform_device *pdev,
 
 	hdmi->phy.ops = plat_data->phy_ops;
 	hdmi->phy.data = plat_data->phy_data;
+	hdmi->ref_clk_rate = plat_data->ref_clk_rate;
 
 	dw_hdmi_qp_init_hw(hdmi);
 

-- 
2.50.0

[PATCH 5/5] arm64: defconfig: Enable DW HDMI QP CEC support

[Cristian Ciocaltea]

Enable support for the CEC interface of the Synopsys DesignWare HDMI QP
IP block.

This is used by all boards based on RK3588 & RK3576 SoCs.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 739b19302865fe8edc54911213321d6f1278aae4..0363ba916e0ed7e7d9dc1169f447f3726580de18 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -958,6 +958,7 @@ CONFIG_DRM_CDNS_MHDP8546=m
 CONFIG_DRM_IMX8MP_DW_HDMI_BRIDGE=m
 CONFIG_DRM_DW_HDMI_AHB_AUDIO=m
 CONFIG_DRM_DW_HDMI_CEC=m
+CONFIG_DRM_DW_HDMI_QP_CEC=y
 CONFIG_DRM_IMX_DCSS=m
 CONFIG_DRM_V3D=m
 CONFIG_DRM_VC4=m

-- 
2.50.0

Re: [PATCH 1/5] drm/rockchip: dw_hdmi_qp: Provide CEC IRQ in dw_hdmi_qp_plat_data

[Heiko Stübner]

Hi Cristian,

Am Freitag, 4. Juli 2025, 16:23:22 Mitteleuropäische Sommerzeit schrieb Cristian Ciocaltea:
> In preparation to support the CEC interface of the DesignWare HDMI QP IP
> block, extend the platform data to provide the required IRQ number.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 4 ++++
>  include/drm/bridge/dw_hdmi_qp.h                | 1 +
>  2 files changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> index 7d531b6f4c098c6c548788dad487ce4613a2f32b..126e556025961e8645f3567b4d7a1c73cc2f2e7f 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> @@ -539,6 +539,10 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>  	if (plat_data.main_irq < 0)
>  		return plat_data.main_irq;
>  
> +	plat_data.cec_irq = platform_get_irq_byname(pdev, "cec");
> +	if (plat_data.cec_irq < 0)
> +		return plat_data.cec_irq;
> +
>  	irq = platform_get_irq_byname(pdev, "hpd");
>  	if (irq < 0)
>  		return irq;
> diff --git a/include/drm/bridge/dw_hdmi_qp.h b/include/drm/bridge/dw_hdmi_qp.h
> index e9be6d507ad9cdc55f5c7d6d3ef37eba41f1ce74..b4a9b739734ec7b67013b683fe6017551aa19172 100644
> --- a/include/drm/bridge/dw_hdmi_qp.h
> +++ b/include/drm/bridge/dw_hdmi_qp.h
> @@ -23,6 +23,7 @@ struct dw_hdmi_qp_plat_data {
>  	const struct dw_hdmi_qp_phy_ops *phy_ops;
>  	void *phy_data;
>  	int main_irq;
> +	int cec_irq;
>  };

from a structure point-of-view, I'd expect the series to be something like:

(1) drm/bridge: dw-hdmi-qp: Add CEC support
    -> including adding the cec_irq to dw_hdmi_qp_plat_data
(2) drm/bridge: dw-hdmi-qp: Fixup timer base setup
    -> including adding the ref_clk_rate to dw_hdmi_qp_plat_data
(3) drm/rockchip: dw_hdmi_qp: Provide CEC IRQ in dw_hdmi_qp_plat_data
(4) drm/rockchip: dw_hdmi_qp: Provide ref clock rate in dw_hdmi_qp_plat_data
(5) arm64: defconfig: Enable DW HDMI QP CEC support

The patches adding the generic functionality to the bridge should also
include the needed elements and not depend on platform-specific patches.


Heiko

Re: [PATCH 1/5] drm/rockchip: dw_hdmi_qp: Provide CEC IRQ in dw_hdmi_qp_plat_data

[Cristian Ciocaltea]

Hi Heiko,

On 7/4/25 5:37 PM, Heiko Stübner wrote:
> Hi Cristian,
> 
> Am Freitag, 4. Juli 2025, 16:23:22 Mitteleuropäische Sommerzeit schrieb Cristian Ciocaltea:
>> In preparation to support the CEC interface of the DesignWare HDMI QP IP
>> block, extend the platform data to provide the required IRQ number.
>>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>>  drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 4 ++++
>>  include/drm/bridge/dw_hdmi_qp.h                | 1 +
>>  2 files changed, 5 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
>> index 7d531b6f4c098c6c548788dad487ce4613a2f32b..126e556025961e8645f3567b4d7a1c73cc2f2e7f 100644
>> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
>> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
>> @@ -539,6 +539,10 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>>  	if (plat_data.main_irq < 0)
>>  		return plat_data.main_irq;
>>  
>> +	plat_data.cec_irq = platform_get_irq_byname(pdev, "cec");
>> +	if (plat_data.cec_irq < 0)
>> +		return plat_data.cec_irq;
>> +
>>  	irq = platform_get_irq_byname(pdev, "hpd");
>>  	if (irq < 0)
>>  		return irq;
>> diff --git a/include/drm/bridge/dw_hdmi_qp.h b/include/drm/bridge/dw_hdmi_qp.h
>> index e9be6d507ad9cdc55f5c7d6d3ef37eba41f1ce74..b4a9b739734ec7b67013b683fe6017551aa19172 100644
>> --- a/include/drm/bridge/dw_hdmi_qp.h
>> +++ b/include/drm/bridge/dw_hdmi_qp.h
>> @@ -23,6 +23,7 @@ struct dw_hdmi_qp_plat_data {
>>  	const struct dw_hdmi_qp_phy_ops *phy_ops;
>>  	void *phy_data;
>>  	int main_irq;
>> +	int cec_irq;
>>  };
> 
> from a structure point-of-view, I'd expect the series to be something like:
> 
> (1) drm/bridge: dw-hdmi-qp: Add CEC support
>     -> including adding the cec_irq to dw_hdmi_qp_plat_data
> (2) drm/bridge: dw-hdmi-qp: Fixup timer base setup
>     -> including adding the ref_clk_rate to dw_hdmi_qp_plat_data
> (3) drm/rockchip: dw_hdmi_qp: Provide CEC IRQ in dw_hdmi_qp_plat_data
> (4) drm/rockchip: dw_hdmi_qp: Provide ref clock rate in dw_hdmi_qp_plat_data
> (5) arm64: defconfig: Enable DW HDMI QP CEC support
> 
> The patches adding the generic functionality to the bridge should also
> include the needed elements and not depend on platform-specific patches.

You are right, that's a much better approach.

Thanks for the quick feedback!

Regards,
Cristian

Re: [PATCH 3/5] drm/rockchip: dw_hdmi_qp: Provide ref clock rate in dw_hdmi_qp_plat_data

[Maxime Ripard]

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

On Fri, Jul 04, 2025 at 05:23:24PM +0300, Cristian Ciocaltea wrote:
> In order to support correct initialization of the timer base in the HDMI
> QP IP block, extend the platform data to provide the necessary reference
> clock rate.
> 
> While at it, ensure plat_data is zero-initialized in
> dw_hdmi_qp_rockchip_bind().
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 13 ++++++++++++-
>  include/drm/bridge/dw_hdmi_qp.h                |  1 +
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> index 126e556025961e8645f3567b4d7a1c73cc2f2e7f..8c1185490009c5f1bc658998a868f8b18dc479a3 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> @@ -431,8 +431,8 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>  				    void *data)
>  {
>  	struct platform_device *pdev = to_platform_device(dev);
> +	struct dw_hdmi_qp_plat_data plat_data = {};
>  	const struct rockchip_hdmi_qp_cfg *cfg;
> -	struct dw_hdmi_qp_plat_data plat_data;
>  	struct drm_device *drm = data;
>  	struct drm_connector *connector;
>  	struct drm_encoder *encoder;
> @@ -515,6 +515,17 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>  		return ret;
>  	}
>  
> +	for (i = 0; i < ret; i++) {
> +		if (!strcmp(clks[i].id, "ref")) {
> +			plat_data.ref_clk_rate = clk_get_rate(clks[i].clk);
> +			break;
> +		}
> +	}
> +	if (!plat_data.ref_clk_rate) {
> +		dev_err(hdmi->dev, "Missing ref clock\n");
> +		return -EINVAL;
> +	}
> +

You'd be better off not using clk_bulk, or calling an additional clk_get
for the ref clock only.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: [PATCH 3/5] drm/rockchip: dw_hdmi_qp: Provide ref clock rate in dw_hdmi_qp_plat_data

[Cristian Ciocaltea]

Hi Maxime,

On 7/4/25 6:07 PM, Maxime Ripard wrote:
> On Fri, Jul 04, 2025 at 05:23:24PM +0300, Cristian Ciocaltea wrote:
>> In order to support correct initialization of the timer base in the HDMI
>> QP IP block, extend the platform data to provide the necessary reference
>> clock rate.
>>
>> While at it, ensure plat_data is zero-initialized in
>> dw_hdmi_qp_rockchip_bind().
>>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>>  drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 13 ++++++++++++-
>>  include/drm/bridge/dw_hdmi_qp.h                |  1 +
>>  2 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
>> index 126e556025961e8645f3567b4d7a1c73cc2f2e7f..8c1185490009c5f1bc658998a868f8b18dc479a3 100644
>> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
>> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
>> @@ -431,8 +431,8 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>>  				    void *data)
>>  {
>>  	struct platform_device *pdev = to_platform_device(dev);
>> +	struct dw_hdmi_qp_plat_data plat_data = {};
>>  	const struct rockchip_hdmi_qp_cfg *cfg;
>> -	struct dw_hdmi_qp_plat_data plat_data;
>>  	struct drm_device *drm = data;
>>  	struct drm_connector *connector;
>>  	struct drm_encoder *encoder;
>> @@ -515,6 +515,17 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>>  		return ret;
>>  	}
>>  
>> +	for (i = 0; i < ret; i++) {
>> +		if (!strcmp(clks[i].id, "ref")) {
>> +			plat_data.ref_clk_rate = clk_get_rate(clks[i].clk);
>> +			break;
>> +		}
>> +	}
>> +	if (!plat_data.ref_clk_rate) {
>> +		dev_err(hdmi->dev, "Missing ref clock\n");
>> +		return -EINVAL;
>> +	}
>> +
> 
> You'd be better off not using clk_bulk, or calling an additional clk_get
> for the ref clock only.

I'd keep clk_bulk as there are 5 more clocks in the list.  But replacing the
loop with just an extra clk_get would be a simpler approach, indeed, so
thanks for the hint!

Regards,
Cristian

Re: [PATCH 2/5] drm/bridge: dw-hdmi-qp: Add CEC support

[Dmitry Baryshkov]

On Fri, Jul 04, 2025 at 05:23:23PM +0300, Cristian Ciocaltea wrote:
> Add support for the CEC interface of the Synopsys DesignWare HDMI QP TX
> controller.
> 
> This is based on the downstream implementation, but rewritten on top of
> the CEC helpers added recently to the DRM HDMI connector framework.
> 
> Co-developed-by: Algea Cao <algea.cao@rock-chips.com>
> Signed-off-by: Algea Cao <algea.cao@rock-chips.com>
> Co-developed-by: Derek Foreman <derek.foreman@collabora.com>
> Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/gpu/drm/bridge/synopsys/Kconfig      |   8 +
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 220 +++++++++++++++++++++++++++
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.h |  14 ++
>  3 files changed, 242 insertions(+)

For the DRM CEC parts only:

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

-- 
With best wishes
Dmitry