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

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

[Cristian Ciocaltea]

The first patch in the series implements 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 second patch is needed for RK3576 in order to fixup the timer base
setup according to the actual reference clock rate, which differs
slightly from RK3588.

The following three patches setup platform data with the new information
expected by the HDMI QP transmitter library, while improving the error
handling in the probe path.

Please note the CEC helpers were affected by a resource deallocation
issue which could crash the kernel and freeze the system under certain
test conditions.  This has been already fixed in v6.17-rc1 via commit
19920ab98e17 ("drm/display: hdmi-cec-helper: Fix adapter
unregistration").

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
Changes in v4:
- Fixed the bisect-related issues reported by Daniel by implementing
  the following operations in dw_hdmi_qp_bind():
  * Disable CEC support when the related IRQ is not available
  * Set ref_clk_rate to vendor default in case it was not provided by
    the platform driver
  * In both scenarios, also print a warning message to highlight the
    need for fixing the platform driver
- Simplified dw_hdmi_qp_cec_init() a bit
  * Removed the now obsolete cec->irq validation test
  * Removed the superfluous error checking and logging around
    devm_request_threaded_irq() call (it already handles all that)
- Collected R-b tags from Daniel
- Rebased series onto next-20250903
- Link to v3: https://lore.kernel.org/r/20250825-rk3588-hdmi-cec-v3-0-95324fb22592@collabora.com

Changes in v3:
- Fixup PATCH 1 according to the recent upstream commit 02bb63d1a593
  ("drm/bridge: Make dp/hdmi_audio_* callback keep the same paramter
  order with get_modes") which changed the signature of ->hdmi_cec_init()
  callback of struct drm_bridge_funcs; while at it, also update the
  copyright section
- Updated cover letter to indicate that the CEC helpers fix is already
  available since v6.17-rc1
- Rebased series onto next-20250825
- Link to v2: https://lore.kernel.org/r/20250710-rk3588-hdmi-cec-v2-0-f5884be34bc1@collabora.com

Changes in v2:
- Collected R-b tag from Dmitry
- Restructured the generic bridge patches to not depend on the
  platform-specific changes and updated cover letter accordingly (Heiko)
- Replaced the loop searching for "ref" clock with clk_get() (Maxime)
- Added new patch "drm/rockchip: dw_hdmi_qp: Improve error handling with
  dev_err_probe()"
- Link to v1: https://lore.kernel.org/r/20250704-rk3588-hdmi-cec-v1-0-2bd8de8700cd@collabora.com

---
Cristian Ciocaltea (6):
      drm/bridge: dw-hdmi-qp: Add CEC support
      drm/bridge: dw-hdmi-qp: Fixup timer base setup
      drm/rockchip: dw_hdmi_qp: Improve error handling with dev_err_probe()
      drm/rockchip: dw_hdmi_qp: Provide CEC IRQ in dw_hdmi_qp_plat_data
      drm/rockchip: dw_hdmi_qp: Provide ref clock rate in dw_hdmi_qp_plat_data
      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   | 224 ++++++++++++++++++++++++-
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.h   |  14 ++
 drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c |  77 +++++----
 include/drm/bridge/dw_hdmi_qp.h                |   2 +
 6 files changed, 284 insertions(+), 42 deletions(-)
---
base-commit: 5d50cf9f7cf20a17ac469c20a2e07c29c1f6aab7
change-id: 20250703-rk3588-hdmi-cec-cea8f523df48

[PATCH v4 1/6] 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.

Also note struct dw_hdmi_qp_plat_data has been extended to include the
CEC IRQ number to be provided by the platform driver.

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>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.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 | 212 +++++++++++++++++++++++++++
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.h |  14 ++
 include/drm/bridge/dw_hdmi_qp.h              |   1 +
 4 files changed, 235 insertions(+)

diff --git a/drivers/gpu/drm/bridge/synopsys/Kconfig b/drivers/gpu/drm/bridge/synopsys/Kconfig
index 2c5e532410de9ef024f13d44502c4fcb5f36ba66..a46df7583bcf907a38e34a1babb02ce8c8be69be 100644
--- a/drivers/gpu/drm/bridge/synopsys/Kconfig
+++ b/drivers/gpu/drm/bridge/synopsys/Kconfig
@@ -61,6 +61,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 39332c57f2c54296f39e27612544f4fbf923863f..fc98953672b6fb388d05201e280d24b8f214498a 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,179 @@ 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_bridge *bridge,
+			       struct drm_connector *connector)
+{
+	struct dw_hdmi_qp *hdmi = dw_hdmi_qp_from_bridge(bridge);
+	struct dw_hdmi_qp_cec *cec = hdmi->cec;
+
+	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);
+
+	return 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);
+}
+
+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 +1171,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 +1289,22 @@ 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
+	if (plat_data->cec_irq) {
+		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;
+	} else {
+		dev_warn(dev, "Disabled CEC support due to missing IRQ\n");
+	}
+#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
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.51.0

[PATCH v4 2/6] 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 that
shall be provided by the platform driver.  Otherwise fallback to the
vendor default.

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 | 12 +++++++++---
 include/drm/bridge/dw_hdmi_qp.h              |  1 +
 2 files changed, 10 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 fc98953672b6fb388d05201e280d24b8f214498a..4ba7b339eff62592aa748429a3bfca82494679d1 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;
@@ -1210,13 +1211,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 */
@@ -1262,6 +1261,13 @@ 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;
 
+	if (plat_data->ref_clk_rate) {
+		hdmi->ref_clk_rate = plat_data->ref_clk_rate;
+	} else {
+		hdmi->ref_clk_rate = 428571429;
+		dev_warn(dev, "Set ref_clk_rate to vendor default\n");
+	}
+
 	dw_hdmi_qp_init_hw(hdmi);
 
 	ret = devm_request_threaded_irq(dev, plat_data->main_irq,
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.51.0

Re: [PATCH v4 2/6] drm/bridge: dw-hdmi-qp: Fixup timer base setup

[Heiko Stübner]

Am Mittwoch, 3. September 2025, 20:51:00 Mitteleuropäische Sommerzeit schrieb 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 that
> shall be provided by the platform driver.  Otherwise fallback to the
> vendor default.
> 
> While at it, also drop the unnecessary empty lines in
> dw_hdmi_qp_init_hw().
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>

This _does_ look ok to me, but as that touches the main bridge, could
we get a 2nd set of eyes?

Thanks
Heiko

> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 12 +++++++++---
>  include/drm/bridge/dw_hdmi_qp.h              |  1 +
>  2 files changed, 10 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 fc98953672b6fb388d05201e280d24b8f214498a..4ba7b339eff62592aa748429a3bfca82494679d1 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;
> @@ -1210,13 +1211,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 */
> @@ -1262,6 +1261,13 @@ 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;
>  
> +	if (plat_data->ref_clk_rate) {
> +		hdmi->ref_clk_rate = plat_data->ref_clk_rate;
> +	} else {
> +		hdmi->ref_clk_rate = 428571429;
> +		dev_warn(dev, "Set ref_clk_rate to vendor default\n");
> +	}
> +
>  	dw_hdmi_qp_init_hw(hdmi);
>  
>  	ret = devm_request_threaded_irq(dev, plat_data->main_irq,
> 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,
> 
> 

Re: [PATCH v4 2/6] drm/bridge: dw-hdmi-qp: Fixup timer base setup

[Daniel Stone]

On Wed, 15 Oct 2025 at 16:44, Heiko Stübner <heiko@sntech.de> wrote:
> Am Mittwoch, 3. September 2025, 20:51:00 Mitteleuropäische Sommerzeit schrieb 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 that
> > shall be provided by the platform driver.  Otherwise fallback to the
> > vendor default.
> >
> > While at it, also drop the unnecessary empty lines in
> > dw_hdmi_qp_init_hw().
> >
> > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>
> This _does_ look ok to me, but as that touches the main bridge, could
> we get a 2nd set of eyes?

Sure can.

Reviewed-by: Daniel Stone <daniels@collabora.com>

Cheers,
Daniel

[PATCH v4 3/6] drm/rockchip: dw_hdmi_qp: Improve error handling with dev_err_probe()

[Cristian Ciocaltea]

The error handling in dw_hdmi_qp_rockchip_bind() is quite inconsistent,
i.e. in some cases the error code is not included in the message, while
in some other cases there is no check for -EPROBE_DEFER.

Since this is part of the probe path, address the aforementioned issues
by switching to dev_err_probe(), which also reduces the code a bit.

Reviewed-by: Daniel Stone <daniels@collabora.com>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 62 ++++++++++----------------
 1 file changed, 24 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
index ed6e8f036f4b3d76425725c130394cedf039acd0..a775d89f20fc20e9103ecbac0dcf3db10ba9984f 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
@@ -455,10 +455,8 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 		return -ENODEV;
 
 	if (!cfg->ctrl_ops || !cfg->ctrl_ops->io_init ||
-	    !cfg->ctrl_ops->irq_callback || !cfg->ctrl_ops->hardirq_callback) {
-		dev_err(dev, "Missing platform ctrl ops\n");
-		return -ENODEV;
-	}
+	    !cfg->ctrl_ops->irq_callback || !cfg->ctrl_ops->hardirq_callback)
+		return dev_err_probe(dev, -ENODEV, "Missing platform ctrl ops\n");
 
 	hdmi->ctrl_ops = cfg->ctrl_ops;
 	hdmi->dev = &pdev->dev;
@@ -471,10 +469,9 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 			break;
 		}
 	}
-	if (hdmi->port_id < 0) {
-		dev_err(hdmi->dev, "Failed to match HDMI port ID\n");
-		return hdmi->port_id;
-	}
+	if (hdmi->port_id < 0)
+		return dev_err_probe(hdmi->dev, hdmi->port_id,
+				     "Failed to match HDMI port ID\n");
 
 	plat_data.phy_ops = cfg->phy_ops;
 	plat_data.phy_data = hdmi;
@@ -495,39 +492,30 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 
 	hdmi->regmap = syscon_regmap_lookup_by_phandle(dev->of_node,
 						       "rockchip,grf");
-	if (IS_ERR(hdmi->regmap)) {
-		dev_err(hdmi->dev, "Unable to get rockchip,grf\n");
-		return PTR_ERR(hdmi->regmap);
-	}
+	if (IS_ERR(hdmi->regmap))
+		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->regmap),
+				     "Unable to get rockchip,grf\n");
 
 	hdmi->vo_regmap = syscon_regmap_lookup_by_phandle(dev->of_node,
 							  "rockchip,vo-grf");
-	if (IS_ERR(hdmi->vo_regmap)) {
-		dev_err(hdmi->dev, "Unable to get rockchip,vo-grf\n");
-		return PTR_ERR(hdmi->vo_regmap);
-	}
+	if (IS_ERR(hdmi->vo_regmap))
+		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->vo_regmap),
+				     "Unable to get rockchip,vo-grf\n");
 
 	ret = devm_clk_bulk_get_all_enabled(hdmi->dev, &clks);
-	if (ret < 0) {
-		dev_err(hdmi->dev, "Failed to get clocks: %d\n", ret);
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(hdmi->dev, ret, "Failed to get clocks\n");
 
 	hdmi->enable_gpio = devm_gpiod_get_optional(hdmi->dev, "enable",
 						    GPIOD_OUT_HIGH);
-	if (IS_ERR(hdmi->enable_gpio)) {
-		ret = PTR_ERR(hdmi->enable_gpio);
-		dev_err(hdmi->dev, "Failed to request enable GPIO: %d\n", ret);
-		return ret;
-	}
+	if (IS_ERR(hdmi->enable_gpio))
+		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->enable_gpio),
+				     "Failed to request enable GPIO\n");
 
 	hdmi->phy = devm_of_phy_get_by_index(dev, dev->of_node, 0);
-	if (IS_ERR(hdmi->phy)) {
-		ret = PTR_ERR(hdmi->phy);
-		if (ret != -EPROBE_DEFER)
-			dev_err(hdmi->dev, "failed to get phy: %d\n", ret);
-		return ret;
-	}
+	if (IS_ERR(hdmi->phy))
+		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->phy),
+				     "Failed to get phy\n");
 
 	cfg->ctrl_ops->io_init(hdmi);
 
@@ -556,17 +544,15 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 
 	hdmi->hdmi = dw_hdmi_qp_bind(pdev, encoder, &plat_data);
 	if (IS_ERR(hdmi->hdmi)) {
-		ret = PTR_ERR(hdmi->hdmi);
 		drm_encoder_cleanup(encoder);
-		return ret;
+		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->hdmi),
+				     "Failed to bind dw-hdmi-qp");
 	}
 
 	connector = drm_bridge_connector_init(drm, encoder);
-	if (IS_ERR(connector)) {
-		ret = PTR_ERR(connector);
-		dev_err(hdmi->dev, "failed to init bridge connector: %d\n", ret);
-		return ret;
-	}
+	if (IS_ERR(connector))
+		return dev_err_probe(hdmi->dev, PTR_ERR(connector),
+				     "Failed to init bridge connector\n");
 
 	return drm_connector_attach_encoder(connector, encoder);
 }

-- 
2.51.0

[PATCH v4 4/6] drm/rockchip: dw_hdmi_qp: Provide CEC IRQ in dw_hdmi_qp_plat_data

[Cristian Ciocaltea]

In order to support the CEC interface of the DesignWare HDMI QP IP
block, setup platform data to include the required IRQ number.

Reviewed-by: Daniel Stone <daniels@collabora.com>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
index a775d89f20fc20e9103ecbac0dcf3db10ba9984f..9191a74a568fb38c2b2ff7ead1e703b3af9addc9 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
@@ -525,6 +525,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;

-- 
2.51.0

[PATCH v4 5/6] 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, setup platform data to include the required reference clock
rate.

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

Reviewed-by: Daniel Stone <daniels@collabora.com>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 11 ++++++++++-
 1 file changed, 10 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 9191a74a568fb38c2b2ff7ead1e703b3af9addc9..931343b072adc05877db9ae867e31a3cd1134e6c 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
@@ -429,14 +429,15 @@ 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;
 	struct rockchip_hdmi_qp *hdmi;
 	struct resource *res;
 	struct clk_bulk_data *clks;
+	struct clk *ref_clk;
 	int ret, irq, i;
 
 	if (!pdev->dev.of_node)
@@ -506,6 +507,14 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 	if (ret < 0)
 		return dev_err_probe(hdmi->dev, ret, "Failed to get clocks\n");
 
+	ref_clk = clk_get(hdmi->dev, "ref");
+	if (IS_ERR(ref_clk))
+		return dev_err_probe(hdmi->dev, PTR_ERR(ref_clk),
+				     "Failed to get ref clock\n");
+
+	plat_data.ref_clk_rate = clk_get_rate(ref_clk);
+	clk_put(ref_clk);
+
 	hdmi->enable_gpio = devm_gpiod_get_optional(hdmi->dev, "enable",
 						    GPIOD_OUT_HIGH);
 	if (IS_ERR(hdmi->enable_gpio))

-- 
2.51.0

[PATCH v4 6/6] 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 acb6807d3461384929e84f4c939fcd00c4b509ae..346ef79c1ddd0a317f0b9a8056c680c29a4e0baf 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -966,6 +966,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.51.0

Re: [PATCH v4 0/6] Add HDMI CEC support to Rockchip RK3588/RK3576 SoCs

[Cristian Ciocaltea]

Hello Heiko,

On 9/3/25 9:50 PM, Cristian Ciocaltea wrote:
> The first patch in the series implements 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 second patch is needed for RK3576 in order to fixup the timer base
> setup according to the actual reference clock rate, which differs
> slightly from RK3588.
> 
> The following three patches setup platform data with the new information
> expected by the HDMI QP transmitter library, while improving the error
> handling in the probe path.
> 
> Please note the CEC helpers were affected by a resource deallocation
> issue which could crash the kernel and freeze the system under certain
> test conditions.  This has been already fixed in v6.17-rc1 via commit
> 19920ab98e17 ("drm/display: hdmi-cec-helper: Fix adapter
> unregistration").
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
> Changes in v4:
> - Fixed the bisect-related issues reported by Daniel by implementing
>   the following operations in dw_hdmi_qp_bind():
>   * Disable CEC support when the related IRQ is not available
>   * Set ref_clk_rate to vendor default in case it was not provided by
>     the platform driver
>   * In both scenarios, also print a warning message to highlight the
>     need for fixing the platform driver
> - Simplified dw_hdmi_qp_cec_init() a bit
>   * Removed the now obsolete cec->irq validation test
>   * Removed the superfluous error checking and logging around
>     devm_request_threaded_irq() call (it already handles all that)
> - Collected R-b tags from Daniel
> - Rebased series onto next-20250903

I forgot to mention that luckily there are no conflicts with the patches
introducing the hw-specific bitfield operations in next-20250903, which this
revision is based on.

I verified the series still applies cleanly onto drm-misc-next, while commit
ad24f6e10a5f ("drm/rockchip: dw_hdmi_qp: switch to FIELD_PREP_WM16 macro")
responsible for the macro conversion can be further cherry-picked without
issues on top of all that.  The resulting file content of
drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c is identical to what's
expected after applying this patch set onto next-20250903.

Regards,
Cristian

Re: [PATCH v4 0/6] Add HDMI CEC support to Rockchip RK3588/RK3576 SoCs

[Cristian Ciocaltea]

Hi,

On 9/3/25 10:52 PM, Cristian Ciocaltea wrote:
> Hello Heiko,
> 
> On 9/3/25 9:50 PM, Cristian Ciocaltea wrote:
>> The first patch in the series implements 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 second patch is needed for RK3576 in order to fixup the timer base
>> setup according to the actual reference clock rate, which differs
>> slightly from RK3588.
>>
>> The following three patches setup platform data with the new information
>> expected by the HDMI QP transmitter library, while improving the error
>> handling in the probe path.
>>
>> Please note the CEC helpers were affected by a resource deallocation
>> issue which could crash the kernel and freeze the system under certain
>> test conditions.  This has been already fixed in v6.17-rc1 via commit
>> 19920ab98e17 ("drm/display: hdmi-cec-helper: Fix adapter
>> unregistration").
>>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>> Changes in v4:
>> - Fixed the bisect-related issues reported by Daniel by implementing
>>   the following operations in dw_hdmi_qp_bind():
>>   * Disable CEC support when the related IRQ is not available
>>   * Set ref_clk_rate to vendor default in case it was not provided by
>>     the platform driver
>>   * In both scenarios, also print a warning message to highlight the
>>     need for fixing the platform driver
>> - Simplified dw_hdmi_qp_cec_init() a bit
>>   * Removed the now obsolete cec->irq validation test
>>   * Removed the superfluous error checking and logging around
>>     devm_request_threaded_irq() call (it already handles all that)
>> - Collected R-b tags from Daniel
>> - Rebased series onto next-20250903
> 
> I forgot to mention that luckily there are no conflicts with the patches
> introducing the hw-specific bitfield operations in next-20250903, which this
> revision is based on.
> 
> I verified the series still applies cleanly onto drm-misc-next, while commit
> ad24f6e10a5f ("drm/rockchip: dw_hdmi_qp: switch to FIELD_PREP_WM16 macro")
> responsible for the macro conversion can be further cherry-picked without
> issues on top of all that.  The resulting file content of
> drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c is identical to what's
> expected after applying this patch set onto next-20250903.

Could we get this merged, please? This would unblock some other patchsets
conflicting with it.

I've just checked and it applies cleanly on top of v6.18-rc1. Also seems to work
fine, at least I haven't noticed any regressions so far.

Thanks,
Cristian

Re: [PATCH v4 0/6] Add HDMI CEC support to Rockchip RK3588/RK3576 SoCs

[Dmitry Baryshkov]

On Wed, Sep 03, 2025 at 09:50:58PM +0300, Cristian Ciocaltea wrote:
> The first patch in the series implements 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 second patch is needed for RK3576 in order to fixup the timer base
> setup according to the actual reference clock rate, which differs
> slightly from RK3588.
> 
> The following three patches setup platform data with the new information
> expected by the HDMI QP transmitter library, while improving the error
> handling in the probe path.
> 
> Please note the CEC helpers were affected by a resource deallocation
> issue which could crash the kernel and freeze the system under certain
> test conditions.  This has been already fixed in v6.17-rc1 via commit
> 19920ab98e17 ("drm/display: hdmi-cec-helper: Fix adapter
> unregistration").
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

Cristian, I'm sorry for almost off-topic, but as you are working on this
driver: would it be possible to support HDMI (vendor-specific) and SPD
InfoFrames in the dw-hdmi-qp driver?

-- 
With best wishes
Dmitry

Re: [PATCH v4 0/6] Add HDMI CEC support to Rockchip RK3588/RK3576 SoCs

[Cristian Ciocaltea]

Hi Dmitry,

On 9/5/25 2:48 AM, Dmitry Baryshkov wrote:
> On Wed, Sep 03, 2025 at 09:50:58PM +0300, Cristian Ciocaltea wrote:
>> The first patch in the series implements 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 second patch is needed for RK3576 in order to fixup the timer base
>> setup according to the actual reference clock rate, which differs
>> slightly from RK3588.
>>
>> The following three patches setup platform data with the new information
>> expected by the HDMI QP transmitter library, while improving the error
>> handling in the probe path.
>>
>> Please note the CEC helpers were affected by a resource deallocation
>> issue which could crash the kernel and freeze the system under certain
>> test conditions.  This has been already fixed in v6.17-rc1 via commit
>> 19920ab98e17 ("drm/display: hdmi-cec-helper: Fix adapter
>> unregistration").
>>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> 
> Cristian, I'm sorry for almost off-topic, but as you are working on this
> driver: would it be possible to support HDMI (vendor-specific) and SPD
> InfoFrames in the dw-hdmi-qp driver?

Sure, no worries.  I'll be on leave for the next two weeks, but I can handle
it on my return.

Regards,
Cristian

Re: [PATCH v4 0/6] Add HDMI CEC support to Rockchip RK3588/RK3576 SoCs

[Dmitry Baryshkov]

On Fri, Sep 05, 2025 at 09:32:36AM +0300, Cristian Ciocaltea wrote:
> Hi Dmitry,
> 
> On 9/5/25 2:48 AM, Dmitry Baryshkov wrote:
> > On Wed, Sep 03, 2025 at 09:50:58PM +0300, Cristian Ciocaltea wrote:
> >> The first patch in the series implements 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 second patch is needed for RK3576 in order to fixup the timer base
> >> setup according to the actual reference clock rate, which differs
> >> slightly from RK3588.
> >>
> >> The following three patches setup platform data with the new information
> >> expected by the HDMI QP transmitter library, while improving the error
> >> handling in the probe path.
> >>
> >> Please note the CEC helpers were affected by a resource deallocation
> >> issue which could crash the kernel and freeze the system under certain
> >> test conditions.  This has been already fixed in v6.17-rc1 via commit
> >> 19920ab98e17 ("drm/display: hdmi-cec-helper: Fix adapter
> >> unregistration").
> >>
> >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> > 
> > Cristian, I'm sorry for almost off-topic, but as you are working on this
> > driver: would it be possible to support HDMI (vendor-specific) and SPD
> > InfoFrames in the dw-hdmi-qp driver?
> 
> Sure, no worries.  I'll be on leave for the next two weeks, but I can handle
> it on my return.

Nice, thanks!

-- 
With best wishes
Dmitry

Re: (subset) [PATCH v4 0/6] Add HDMI CEC support to Rockchip RK3588/RK3576 SoCs

[Heiko Stuebner]

On Wed, 03 Sep 2025 21:50:58 +0300, Cristian Ciocaltea wrote:
> The first patch in the series implements 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 second patch is needed for RK3576 in order to fixup the timer base
> setup according to the actual reference clock rate, which differs
> slightly from RK3588.
> 
> [...]

Applied, thanks!

[1/6] drm/bridge: dw-hdmi-qp: Add CEC support
      commit: e4a2d54a2f1a9c9a1971651832c8f0ad9d3782c4
[2/6] drm/bridge: dw-hdmi-qp: Fixup timer base setup
      commit: f7a1de0d86221000dc0699a8b48ad3a848e766d9
[3/6] drm/rockchip: dw_hdmi_qp: Improve error handling with dev_err_probe()
      commit: b6736a4ea3fa68524074a18334f344a34a05bee8
[4/6] drm/rockchip: dw_hdmi_qp: Provide CEC IRQ in dw_hdmi_qp_plat_data
      commit: 9baa02327adf2b1e919e95af23036102cfd0133f
[5/6] drm/rockchip: dw_hdmi_qp: Provide ref clock rate in dw_hdmi_qp_plat_data
      commit: 33ea4d520fbda505e2cfe5b36ebf522de1f3f5e9

Best regards,
-- 
Heiko Stuebner <heiko@sntech.de>

Re: (subset) [PATCH v4 0/6] Add HDMI CEC support to Rockchip RK3588/RK3576 SoCs

[Heiko Stuebner]

On Wed, 03 Sep 2025 21:50:58 +0300, Cristian Ciocaltea wrote:
> The first patch in the series implements 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 second patch is needed for RK3576 in order to fixup the timer base
> setup according to the actual reference clock rate, which differs
> slightly from RK3588.
> 
> [...]

Applied, thanks!

[6/6] arm64: defconfig: Enable DW HDMI QP CEC support
      commit: ae753d769a5f63cddd483ec50067ec61eef5489d

Best regards,
-- 
Heiko Stuebner <heiko@sntech.de>

Re: [PATCH v4 0/6] Add HDMI CEC support to Rockchip RK3588/RK3576 SoCs

[Mark Brown]

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

On Wed, Sep 03, 2025 at 09:50:58PM +0300, Cristian Ciocaltea wrote:
> The first patch in the series implements 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.

For the past couple of weeks we've been seeing various instability with
the graphics drivers on the Rock 5B in -next, the most common system is
that we get faults in code that looks suspiciously relevant to this
series:

<6>[   17.353368] rockchip-drm display-subsystem: bound fdd90000.vop (ops vop2_component_ops [rockchipdrm])
<6>[   17.355237] dwhdmiqp-rockchip fde80000.hdmi: registered DesignWare HDMI QP I2C bus driver
/ # <1>[   17.357803] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000098

...

<4>[   17.372390] Hardware name: Radxa ROCK 5B (DT)

...

<4>[   17.382082] Call trace:
<4>[   17.382317]  drm_bridge_connector_hdmi_cec_init+0x8/0x38 [drm_display_helper] (P)
<4>[   17.383003]  drm_bridge_connector_init+0x658/0x678 [drm_display_helper]
<4>[   17.383612]  dw_hdmi_qp_rockchip_bind+0x35c/0x4d8 [rockchipdrm]
<4>[   17.384159]  component_bind_all+0x118/0x248
<4>[   17.384550]  rockchip_drm_bind+0xb4/0x20c [rockchipdrm]
<4>[   17.385034]  try_to_bring_up_aggregate_device+0x164/0x1d0
<4>[   17.385528]  component_master_add_with_match+0xc4/0x104
<4>[   17.386008]  rockchip_drm_platform_probe+0x1f8/0x31c [rockchipdrm]

(from today's -next, 

Unfortunately we haven't managed to point at a specific commit, it looks
like this might be triggered by multiple serieses interacting with each
other.  I'm not sure what other information might be useful here?

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

Re: [PATCH v4 0/6] Add HDMI CEC support to Rockchip RK3588/RK3576 SoCs

[Cristian Ciocaltea]

Hi Mark,

On 10/28/25 2:38 PM, Mark Brown wrote:
> On Wed, Sep 03, 2025 at 09:50:58PM +0300, Cristian Ciocaltea wrote:
>> The first patch in the series implements 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.
> 
> For the past couple of weeks we've been seeing various instability with
> the graphics drivers on the Rock 5B in -next, the most common system is
> that we get faults in code that looks suspiciously relevant to this
> series:
> 
> <6>[   17.353368] rockchip-drm display-subsystem: bound fdd90000.vop (ops vop2_component_ops [rockchipdrm])
> <6>[   17.355237] dwhdmiqp-rockchip fde80000.hdmi: registered DesignWare HDMI QP I2C bus driver
> / # <1>[   17.357803] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000098
> 
> ...
> 
> <4>[   17.372390] Hardware name: Radxa ROCK 5B (DT)
> 
> ...
> 
> <4>[   17.382082] Call trace:
> <4>[   17.382317]  drm_bridge_connector_hdmi_cec_init+0x8/0x38 [drm_display_helper] (P)
> <4>[   17.383003]  drm_bridge_connector_init+0x658/0x678 [drm_display_helper]
> <4>[   17.383612]  dw_hdmi_qp_rockchip_bind+0x35c/0x4d8 [rockchipdrm]
> <4>[   17.384159]  component_bind_all+0x118/0x248
> <4>[   17.384550]  rockchip_drm_bind+0xb4/0x20c [rockchipdrm]
> <4>[   17.385034]  try_to_bring_up_aggregate_device+0x164/0x1d0
> <4>[   17.385528]  component_master_add_with_match+0xc4/0x104
> <4>[   17.386008]  rockchip_drm_platform_probe+0x1f8/0x31c [rockchipdrm]
> 
> (from today's -next, 
> 
> Unfortunately we haven't managed to point at a specific commit, it looks
> like this might be triggered by multiple serieses interacting with each
> other.  I'm not sure what other information might be useful here?

Thanks for reporting, I will investigate.

Regards,
Cristian

Re: [PATCH v4 0/6] Add HDMI CEC support to Rockchip RK3588/RK3576 SoCs

[Cristian Ciocaltea]

On 10/28/25 2:42 PM, Cristian Ciocaltea wrote:
> Hi Mark,
> 
> On 10/28/25 2:38 PM, Mark Brown wrote:
>> On Wed, Sep 03, 2025 at 09:50:58PM +0300, Cristian Ciocaltea wrote:
>>> The first patch in the series implements 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.
>>
>> For the past couple of weeks we've been seeing various instability with
>> the graphics drivers on the Rock 5B in -next, the most common system is
>> that we get faults in code that looks suspiciously relevant to this
>> series:
>>
>> <6>[   17.353368] rockchip-drm display-subsystem: bound fdd90000.vop (ops vop2_component_ops [rockchipdrm])
>> <6>[   17.355237] dwhdmiqp-rockchip fde80000.hdmi: registered DesignWare HDMI QP I2C bus driver
>> / # <1>[   17.357803] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000098
>>
>> ...
>>
>> <4>[   17.372390] Hardware name: Radxa ROCK 5B (DT)
>>
>> ...
>>
>> <4>[   17.382082] Call trace:
>> <4>[   17.382317]  drm_bridge_connector_hdmi_cec_init+0x8/0x38 [drm_display_helper] (P)
>> <4>[   17.383003]  drm_bridge_connector_init+0x658/0x678 [drm_display_helper]
>> <4>[   17.383612]  dw_hdmi_qp_rockchip_bind+0x35c/0x4d8 [rockchipdrm]
>> <4>[   17.384159]  component_bind_all+0x118/0x248
>> <4>[   17.384550]  rockchip_drm_bind+0xb4/0x20c [rockchipdrm]
>> <4>[   17.385034]  try_to_bring_up_aggregate_device+0x164/0x1d0
>> <4>[   17.385528]  component_master_add_with_match+0xc4/0x104
>> <4>[   17.386008]  rockchip_drm_platform_probe+0x1f8/0x31c [rockchipdrm]
>>
>> (from today's -next, 
>>
>> Unfortunately we haven't managed to point at a specific commit, it looks
>> like this might be triggered by multiple serieses interacting with each
>> other.  I'm not sure what other information might be useful here?
> 
> Thanks for reporting, I will investigate.

This seems to have been already addressed:

https://lore.kernel.org/all/20251017-drm-bridge-alloc-getput-bridge-connector-fix-hdmi_cec-v2-0-667abf6d47c0@bootlin.com/
> Regards,
> Cristian

Re: [PATCH v4 0/6] Add HDMI CEC support to Rockchip RK3588/RK3576 SoCs

[Mark Brown]

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

On Tue, Oct 28, 2025 at 04:57:03PM +0200, Cristian Ciocaltea wrote:

> This seems to have been already addressed:

> https://lore.kernel.org/all/20251017-drm-bridge-alloc-getput-bridge-connector-fix-hdmi_cec-v2-0-667abf6d47c0@bootlin.com/

Ah, good.  Hopefully that lands soon, do you have any idea what's
holding it up?

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

Re: [PATCH v4 0/6] Add HDMI CEC support to Rockchip RK3588/RK3576 SoCs

[Cristian Ciocaltea]

On 10/28/25 5:08 PM, Mark Brown wrote:
> On Tue, Oct 28, 2025 at 04:57:03PM +0200, Cristian Ciocaltea wrote:
> 
>> This seems to have been already addressed:
> 
>> https://lore.kernel.org/all/20251017-drm-bridge-alloc-getput-bridge-connector-fix-hdmi_cec-v2-0-667abf6d47c0@bootlin.com/
> 
> Ah, good.  Hopefully that lands soon, do you have any idea what's
> holding it up?

I guess it's just the complexity, since that is part of a larger work concerning DRM bridges:

https://lore.kernel.org/all/20250926-drm-bridge-alloc-getput-bridge-connector-v2-1-138b4bb70576@bootlin.com/

Re: [PATCH v4 0/6] Add HDMI CEC support to Rockchip RK3588/RK3576 SoCs

[Mark Brown]

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

On Tue, Oct 28, 2025 at 05:24:42PM +0200, Cristian Ciocaltea wrote:
> On 10/28/25 5:08 PM, Mark Brown wrote:
> > On Tue, Oct 28, 2025 at 04:57:03PM +0200, Cristian Ciocaltea wrote:

> >> This seems to have been already addressed:

> >> https://lore.kernel.org/all/20251017-drm-bridge-alloc-getput-bridge-connector-fix-hdmi_cec-v2-0-667abf6d47c0@bootlin.com/

> > Ah, good.  Hopefully that lands soon, do you have any idea what's
> > holding it up?

> I guess it's just the complexity, since that is part of a larger work concerning DRM bridges:

> https://lore.kernel.org/all/20250926-drm-bridge-alloc-getput-bridge-connector-v2-1-138b4bb70576@bootlin.com/

Oh dear, I'll perhaps follow up there - sometimes this breaks boot
entirely depending on which way we hit the issue so at least getting the
revert in would be good.

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