[PATCH v4 0/4] Add HDMI 2.0 support to DW HDMI QP TX

[PATCH v4 0/4] Add HDMI 2.0 support to DW HDMI QP TX

[Cristian Ciocaltea]

This patch series provides the missing support for high TMDS clock ratio
and scrambling to DW HDMI QP TX library, required for handling HDMI 2.0
display modes on RK3576 & RK3588 SoCs.

In order to allow addressing the SCDC status lost on sink disconnects,
it adds an atomic variant of the drm_bridge_funcs.detect() callback and
a new drm_bridge_detect_ctx() helper, which is further used in
drm_bridge_connector to switch to using the ->detect_ctx() hook.

Furthermore, it optimizes HPD event handling in dw_hdmi_qp Rockchip
platform driver to run the detect cycle on the affected connector only.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
Changes in v4:
- Fixed conflicts while rebasing onto latest drm-misc-next
- Link to v3: https://lore.kernel.org/r/20260119-dw-hdmi-qp-scramb-v3-0-bd8611730fc1@collabora.com

Changes in v3:
- Used drm_bridge_helper_reset_crtc() helper to reset the display
  pipeline and got rid of some boilerplate code (Maxime)
- Rebased onto latest drm-misc-next
- Link to v2: https://lore.kernel.org/r/20260113-dw-hdmi-qp-scramb-v2-0-ae7b2c58d24d@collabora.com

Changes in v2:
- Collected Tested-by tags from Diederik and Maud
- Rebased onto latest drm-misc-next
- Ensured the recently introduced 'no-hpd' support for dealing with
  unconnected/repurposed/broken HPD pin is limited to HDMI 1.4 rates
- Link to v1: https://lore.kernel.org/r/20251203-dw-hdmi-qp-scramb-v1-0-836fe7401a69@collabora.com

---
Cristian Ciocaltea (4):
      drm/bridge: Add ->detect_ctx hook and drm_bridge_detect_ctx()
      drm/bridge-connector: Switch to using ->detect_ctx hook
      drm/bridge: dw-hdmi-qp: Add high TMDS clock ratio and scrambling support
      drm/rockchip: dw_hdmi_qp: Do not send HPD events for all connectors

 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c   | 167 ++++++++++++++++++++++---
 drivers/gpu/drm/display/drm_bridge_connector.c |  73 +++++------
 drivers/gpu/drm/drm_bridge.c                   |  58 +++++++++
 drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c |  44 +++----
 include/drm/drm_bridge.h                       |  30 +++++
 5 files changed, 296 insertions(+), 76 deletions(-)
---
base-commit: 0dd7a23da214279cfb77b0eb03c38614867524d7
change-id: 20251203-dw-hdmi-qp-scramb-cdbd8b57ccf9

[PATCH v4 1/4] drm/bridge: Add ->detect_ctx hook and drm_bridge_detect_ctx()

[Cristian Ciocaltea]

Add an atomic variant of the ->detect callback and a new helper to call
the hook while passing an optional drm_modeset_acquire_ctx reference.

When both ->detect_ctx and ->detect are defined, the latter is ignored.
If acquire_ctx is unset, the function takes care of the locking,
while also handling EDEADLK.

Tested-by: Diederik de Haas <diederik@cknow-tech.com>
Tested-by: Maud Spierings <maud_spierings@hotmail.com>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/drm_bridge.c | 58 ++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_bridge.h     | 30 +++++++++++++++++++++++
 2 files changed, 88 insertions(+)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index f8b0333a0a3b..1247573c2100 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -1345,6 +1345,64 @@ drm_bridge_detect(struct drm_bridge *bridge, struct drm_connector *connector)
 }
 EXPORT_SYMBOL_GPL(drm_bridge_detect);
 
+/**
+ * drm_bridge_detect_ctx - check if anything is attached to the bridge output
+ * @bridge: bridge control structure
+ * @connector: attached connector
+ * @ctx: acquire_ctx, or NULL to let this function handle locking
+ *
+ * If the bridge supports output detection, as reported by the
+ * DRM_BRIDGE_OP_DETECT bridge ops flag, call &drm_bridge_funcs.detect_ctx
+ * or &drm_bridge_funcs.detect for the bridge and return the connection status.
+ * Otherwise return connector_status_unknown.
+ *
+ * When both @ctx and &drm_bridge_funcs.detect_ctx are not set, this helper
+ * function is equivalent to drm_bridge_detect() above.
+ *
+ * RETURNS:
+ * The detection status on success, or connector_status_unknown if the bridge
+ * doesn't support output detection.
+ * If @ctx is set, it might also return -EDEADLK.
+ */
+int drm_bridge_detect_ctx(struct drm_bridge *bridge,
+			  struct drm_connector *connector,
+			  struct drm_modeset_acquire_ctx *ctx)
+{
+	if (!(bridge->ops & DRM_BRIDGE_OP_DETECT))
+		return connector_status_unknown;
+
+	if (bridge->funcs->detect_ctx) {
+		struct drm_modeset_acquire_ctx br_ctx;
+		int ret;
+
+		if (ctx)
+			return bridge->funcs->detect_ctx(bridge, connector, ctx);
+
+		drm_modeset_acquire_init(&br_ctx, 0);
+retry:
+		ret = drm_modeset_lock(&connector->dev->mode_config.connection_mutex,
+				       &br_ctx);
+		if (!ret)
+			ret = bridge->funcs->detect_ctx(bridge, connector, &br_ctx);
+
+		if (ret == -EDEADLK) {
+			drm_modeset_backoff(&br_ctx);
+			goto retry;
+		}
+
+		if (ret < 0)
+			ret = connector_status_unknown;
+
+		drm_modeset_drop_locks(&br_ctx);
+		drm_modeset_acquire_fini(&br_ctx);
+
+		return ret;
+	}
+
+	return bridge->funcs->detect(bridge, connector);
+}
+EXPORT_SYMBOL_GPL(drm_bridge_detect_ctx);
+
 /**
  * drm_bridge_get_modes - fill all modes currently valid for the sink into the
  * @connector
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 4f19f7064ee3..31ef1496fe63 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -547,6 +547,33 @@ struct drm_bridge_funcs {
 	enum drm_connector_status (*detect)(struct drm_bridge *bridge,
 					    struct drm_connector *connector);
 
+	/**
+	 * @detect_ctx:
+	 *
+	 * Check if anything is attached to the bridge output.
+	 *
+	 * This callback is optional, if not implemented the bridge will be
+	 * considered as always having a component attached to its output.
+	 * Bridges that implement this callback shall set the
+	 * DRM_BRIDGE_OP_DETECT flag in their &drm_bridge->ops.
+	 *
+	 * This is the atomic version of &drm_bridge_funcs.detect.
+	 *
+	 * To avoid races against concurrent connector state updates, the
+	 * helper libraries always call this with ctx set to a valid context,
+	 * and &drm_mode_config.connection_mutex will always be locked with
+	 * the ctx parameter set to this ctx. This allows taking additional
+	 * locks as required.
+	 *
+	 * RETURNS:
+	 *
+	 * &drm_connector_status indicating the bridge output status,
+	 * or the error code returned by drm_modeset_lock(), -EDEADLK.
+	 */
+	int (*detect_ctx)(struct drm_bridge *bridge,
+			  struct drm_connector *connector,
+			  struct drm_modeset_acquire_ctx *ctx);
+
 	/**
 	 * @get_modes:
 	 *
@@ -1542,6 +1569,9 @@ drm_atomic_helper_bridge_propagate_bus_fmt(struct drm_bridge *bridge,
 
 enum drm_connector_status
 drm_bridge_detect(struct drm_bridge *bridge, struct drm_connector *connector);
+int drm_bridge_detect_ctx(struct drm_bridge *bridge,
+			  struct drm_connector *connector,
+			  struct drm_modeset_acquire_ctx *ctx);
 int drm_bridge_get_modes(struct drm_bridge *bridge,
 			 struct drm_connector *connector);
 const struct drm_edid *drm_bridge_edid_read(struct drm_bridge *bridge,

-- 
2.52.0

Claude review: drm/bridge: Add ->detect_ctx hook and drm_bridge_detect_ctx()

[Claude Code Review Bot]

Patch Review

This patch adds the atomic variant of `drm_bridge_funcs.detect()` and a new `drm_bridge_detect_ctx()` helper. The implementation is clean and follows the established patterns from the connector-level `detect_ctx`.

**Looks good.** A few observations:

The fallback path when `detect_ctx` is NULL but `detect` exists is correct:
```c
return bridge->funcs->detect(bridge, connector);
```

The self-managed locking path (ctx == NULL) properly handles EDEADLK:
```c
drm_modeset_acquire_init(&br_ctx, 0);
retry:
    ret = drm_modeset_lock(&connector->dev->mode_config.connection_mutex, &br_ctx);
    if (!ret)
        ret = bridge->funcs->detect_ctx(bridge, connector, &br_ctx);
    if (ret == -EDEADLK) {
        drm_modeset_backoff(&br_ctx);
        goto retry;
    }
```

The conversion of negative errors to `connector_status_unknown` is appropriate:
```c
if (ret < 0)
    ret = connector_status_unknown;
```

**Minor doc nit:** The doc says "If @ctx is set, it might also return -EDEADLK" — but when ctx is passed through directly to `detect_ctx`, the callback could return *any* negative error, not just -EDEADLK. The caller would see all of them. This is handled correctly in practice (patch 2's `drm_bridge_connector_detect_ctx` checks `ret < 0`), but the doc could be more precise.

---
Generated by Claude Code Patch Reviewer

[PATCH v4 2/4] drm/bridge-connector: Switch to using ->detect_ctx hook

[Cristian Ciocaltea]

In preparation to allow bridge drivers relying on the HDMI connector
framework to provide HDMI 2.0 support, make use of the atomic version of
drm_connector_funcs.detect() hook and invoke the newly introduced
drm_bridge_detect_ctx() helper.

In particular, this is going to be used for triggering an empty modeset
in drm_bridge_funcs.detect_ctx() callback, in order to manage SCDC
status lost on sink disconnects.

Tested-by: Diederik de Haas <diederik@cknow-tech.com>
Tested-by: Maud Spierings <maud_spierings@hotmail.com>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/display/drm_bridge_connector.c | 73 ++++++++++++++------------
 1 file changed, 38 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index f686aa5c0ed9..626f15aba5d5 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -208,39 +208,6 @@ static void drm_bridge_connector_disable_hpd(struct drm_connector *connector)
  * Bridge Connector Functions
  */
 
-static enum drm_connector_status
-drm_bridge_connector_detect(struct drm_connector *connector, bool force)
-{
-	struct drm_bridge_connector *bridge_connector =
-		to_drm_bridge_connector(connector);
-	struct drm_bridge *detect = bridge_connector->bridge_detect;
-	struct drm_bridge *hdmi = bridge_connector->bridge_hdmi;
-	enum drm_connector_status status;
-
-	if (detect) {
-		status = detect->funcs->detect(detect, connector);
-
-		if (hdmi)
-			drm_atomic_helper_connector_hdmi_hotplug(connector, status);
-
-		drm_bridge_connector_hpd_notify(connector, status);
-	} else {
-		switch (connector->connector_type) {
-		case DRM_MODE_CONNECTOR_DPI:
-		case DRM_MODE_CONNECTOR_LVDS:
-		case DRM_MODE_CONNECTOR_DSI:
-		case DRM_MODE_CONNECTOR_eDP:
-			status = connector_status_connected;
-			break;
-		default:
-			status = connector_status_unknown;
-			break;
-		}
-	}
-
-	return status;
-}
-
 static void drm_bridge_connector_force(struct drm_connector *connector)
 {
 	struct drm_bridge_connector *bridge_connector =
@@ -278,7 +245,6 @@ static void drm_bridge_connector_reset(struct drm_connector *connector)
 
 static const struct drm_connector_funcs drm_bridge_connector_funcs = {
 	.reset = drm_bridge_connector_reset,
-	.detect = drm_bridge_connector_detect,
 	.force = drm_bridge_connector_force,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
@@ -291,6 +257,42 @@ static const struct drm_connector_funcs drm_bridge_connector_funcs = {
  * Bridge Connector Helper Functions
  */
 
+static int drm_bridge_connector_detect_ctx(struct drm_connector *connector,
+					   struct drm_modeset_acquire_ctx *ctx,
+					   bool force)
+{
+	struct drm_bridge_connector *bridge_connector =
+		to_drm_bridge_connector(connector);
+	struct drm_bridge *detect = bridge_connector->bridge_detect;
+	struct drm_bridge *hdmi = bridge_connector->bridge_hdmi;
+	int ret;
+
+	if (detect) {
+		ret = drm_bridge_detect_ctx(detect, connector, ctx);
+		if (ret < 0)
+			return ret;
+
+		if (hdmi)
+			drm_atomic_helper_connector_hdmi_hotplug(connector, ret);
+
+		drm_bridge_connector_hpd_notify(connector, ret);
+	} else {
+		switch (connector->connector_type) {
+		case DRM_MODE_CONNECTOR_DPI:
+		case DRM_MODE_CONNECTOR_LVDS:
+		case DRM_MODE_CONNECTOR_DSI:
+		case DRM_MODE_CONNECTOR_eDP:
+			ret = connector_status_connected;
+			break;
+		default:
+			ret = connector_status_unknown;
+			break;
+		}
+	}
+
+	return ret;
+}
+
 static int drm_bridge_connector_get_modes_edid(struct drm_connector *connector,
 					       struct drm_bridge *bridge)
 {
@@ -298,7 +300,7 @@ static int drm_bridge_connector_get_modes_edid(struct drm_connector *connector,
 	const struct drm_edid *drm_edid;
 	int n;
 
-	status = drm_bridge_connector_detect(connector, false);
+	status = drm_bridge_connector_detect_ctx(connector, NULL, false);
 	if (status != connector_status_connected)
 		goto no_edid;
 
@@ -384,6 +386,7 @@ static int drm_bridge_connector_atomic_check(struct drm_connector *connector,
 
 static const struct drm_connector_helper_funcs drm_bridge_connector_helper_funcs = {
 	.get_modes = drm_bridge_connector_get_modes,
+	.detect_ctx = drm_bridge_connector_detect_ctx,
 	.mode_valid = drm_bridge_connector_mode_valid,
 	.enable_hpd = drm_bridge_connector_enable_hpd,
 	.disable_hpd = drm_bridge_connector_disable_hpd,

-- 
2.52.0

Claude review: drm/bridge-connector: Switch to using ->detect_ctx hook

[Claude Code Review Bot]

Patch Review

This patch removes `.detect` from `drm_bridge_connector_funcs` and adds `.detect_ctx` to the helper funcs. The conversion is straightforward.

**Concern about removing .detect:** The old `drm_bridge_connector_detect` was assigned to `drm_connector_funcs.detect`. By removing it entirely, any code path that calls `connector->funcs->detect()` directly (rather than going through the helper framework) will now get a NULL function pointer. The helper framework (`drm_helper_probe_detect_ctx`) checks `detect_ctx` first, so that path works. But `drm_mode_getconnector()` (the ioctl handler) may have a direct `.detect` call path that should be verified. If the ioctl handler also goes through `drm_helper_probe_detect`, this is fine.

The `drm_bridge_connector_get_modes_edid` change is correct:
```c
-	status = drm_bridge_connector_detect(connector, false);
+	status = drm_bridge_connector_detect_ctx(connector, NULL, false);
```
When ctx is NULL, `drm_bridge_detect_ctx` manages its own locking and never returns negative values, so assigning to the `enum drm_connector_status status` variable is safe.

**Minor observation:** The `force` parameter from `detect_ctx(connector, ctx, force)` is received but never forwarded to `drm_bridge_detect_ctx`. This is consistent with the bridge-level detect hooks not having a `force` parameter, but it means bridges can never distinguish forced vs. polled detection.

---
Generated by Claude Code Patch Reviewer

[PATCH v4 3/4] drm/bridge: dw-hdmi-qp: Add high TMDS clock ratio and scrambling support

[Cristian Ciocaltea]

Add support for HDMI 2.0 display modes, e.g. 4K@60Hz, by permitting TMDS
character rates above the 340 MHz limit of HDMI 1.4b.

Hence, provide the required SCDC management, including the high TMDS
clock ratio and scrambling setup, and filter out the HDMI 2.1 modes.

Tested-by: Diederik de Haas <diederik@cknow-tech.com>
Tested-by: Maud Spierings <maud_spierings@hotmail.com>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 167 ++++++++++++++++++++++++---
 1 file changed, 150 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
index d649a1cf07f5..e40f16a364ed 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
@@ -2,6 +2,7 @@
 /*
  * Copyright (c) 2021-2022 Rockchip Electronics Co., Ltd.
  * Copyright (c) 2024 Collabora Ltd.
+ * Copyright (c) 2025 Amazon.com, Inc. or its affiliates.
  *
  * Author: Algea Cao <algea.cao@rock-chips.com>
  * Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
@@ -21,9 +22,11 @@
 #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/display/drm_scdc_helper.h>
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge.h>
+#include <drm/drm_bridge_helper.h>
 #include <drm/drm_connector.h>
 #include <drm/drm_edid.h>
 #include <drm/drm_modes.h>
@@ -39,8 +42,10 @@
 #define DDC_SEGMENT_ADDR	0x30
 
 #define HDMI14_MAX_TMDSCLK	340000000
+#define HDMI20_MAX_TMDSRATE	600000000
 
-#define SCRAMB_POLL_DELAY_MS	3000
+#define SCDC_MIN_SOURCE_VERSION	0x1
+#define SCRAMB_POLL_DELAY_MS	5000
 
 /*
  * Unless otherwise noted, entries in this table are 100% optimization.
@@ -164,6 +169,11 @@ struct dw_hdmi_qp {
 	} phy;
 
 	unsigned long ref_clk_rate;
+
+	struct drm_connector *curr_conn;
+	struct delayed_work scramb_work;
+	bool scramb_enabled;
+
 	struct regmap *regm;
 	int main_irq;
 
@@ -749,28 +759,98 @@ static struct i2c_adapter *dw_hdmi_qp_i2c_adapter(struct dw_hdmi_qp *hdmi)
 	return adap;
 }
 
+static bool dw_hdmi_qp_supports_scrambling(struct drm_display_info *display)
+{
+	if (!display->is_hdmi)
+		return false;
+
+	return display->hdmi.scdc.supported &&
+		display->hdmi.scdc.scrambling.supported;
+}
+
+static void dw_hdmi_qp_set_scramb(struct dw_hdmi_qp *hdmi)
+{
+	dev_dbg(hdmi->dev, "set scrambling\n");
+
+	drm_scdc_set_high_tmds_clock_ratio(hdmi->curr_conn, true);
+	drm_scdc_set_scrambling(hdmi->curr_conn, true);
+
+	schedule_delayed_work(&hdmi->scramb_work,
+			      msecs_to_jiffies(SCRAMB_POLL_DELAY_MS));
+}
+
+static void dw_hdmi_qp_scramb_work(struct work_struct *work)
+{
+	struct dw_hdmi_qp *hdmi = container_of(to_delayed_work(work),
+					       struct dw_hdmi_qp,
+					       scramb_work);
+	if (!drm_scdc_get_scrambling_status(hdmi->curr_conn))
+		dw_hdmi_qp_set_scramb(hdmi);
+}
+
+static void dw_hdmi_qp_enable_scramb(struct dw_hdmi_qp *hdmi)
+{
+	u8 ver;
+
+	if (!dw_hdmi_qp_supports_scrambling(&hdmi->curr_conn->display_info))
+		return;
+
+	drm_scdc_readb(hdmi->bridge.ddc, SCDC_SINK_VERSION, &ver);
+	drm_scdc_writeb(hdmi->bridge.ddc, SCDC_SOURCE_VERSION,
+			min_t(u8, ver, SCDC_MIN_SOURCE_VERSION));
+
+	dw_hdmi_qp_set_scramb(hdmi);
+	dw_hdmi_qp_write(hdmi, 1, SCRAMB_CONFIG0);
+
+	hdmi->scramb_enabled = true;
+
+	/* Wait at least 1 ms before resuming TMDS transmission */
+	usleep_range(1000, 5000);
+}
+
+static void dw_hdmi_qp_disable_scramb(struct dw_hdmi_qp *hdmi)
+{
+	if (!hdmi->scramb_enabled)
+		return;
+
+	dev_dbg(hdmi->dev, "disable scrambling\n");
+
+	hdmi->scramb_enabled = false;
+	cancel_delayed_work_sync(&hdmi->scramb_work);
+
+	dw_hdmi_qp_write(hdmi, 0, SCRAMB_CONFIG0);
+
+	if (hdmi->curr_conn->status == connector_status_connected) {
+		drm_scdc_set_scrambling(hdmi->curr_conn, false);
+		drm_scdc_set_high_tmds_clock_ratio(hdmi->curr_conn, false);
+	}
+}
+
 static void dw_hdmi_qp_bridge_atomic_enable(struct drm_bridge *bridge,
 					    struct drm_atomic_state *state)
 {
 	struct dw_hdmi_qp *hdmi = bridge->driver_private;
 	struct drm_connector_state *conn_state;
-	struct drm_connector *connector;
 	unsigned int op_mode;
 
-	connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
-	if (WARN_ON(!connector))
+	hdmi->curr_conn = drm_atomic_get_new_connector_for_encoder(state,
+								   bridge->encoder);
+	if (WARN_ON(!hdmi->curr_conn))
 		return;
 
-	conn_state = drm_atomic_get_new_connector_state(state, connector);
+	conn_state = drm_atomic_get_new_connector_state(state, hdmi->curr_conn);
 	if (WARN_ON(!conn_state))
 		return;
 
-	if (connector->display_info.is_hdmi) {
+	if (hdmi->curr_conn->display_info.is_hdmi) {
 		dev_dbg(hdmi->dev, "%s mode=HDMI %s rate=%llu bpc=%u\n", __func__,
 			drm_hdmi_connector_get_output_format_name(conn_state->hdmi.output_format),
 			conn_state->hdmi.tmds_char_rate, conn_state->hdmi.output_bpc);
 		op_mode = 0;
 		hdmi->tmds_char_rate = conn_state->hdmi.tmds_char_rate;
+
+		if (conn_state->hdmi.tmds_char_rate > HDMI14_MAX_TMDSCLK)
+			dw_hdmi_qp_enable_scramb(hdmi);
 	} else {
 		dev_dbg(hdmi->dev, "%s mode=DVI\n", __func__);
 		op_mode = OPMODE_DVI;
@@ -781,7 +861,7 @@ static void dw_hdmi_qp_bridge_atomic_enable(struct drm_bridge *bridge,
 	dw_hdmi_qp_mod(hdmi, HDCP2_BYPASS, HDCP2_BYPASS, HDCP2LOGIC_CONFIG0);
 	dw_hdmi_qp_mod(hdmi, op_mode, OPMODE_DVI, LINK_CONFIG0);
 
-	drm_atomic_helper_connector_hdmi_update_infoframes(connector, state);
+	drm_atomic_helper_connector_hdmi_update_infoframes(hdmi->curr_conn, state);
 }
 
 static void dw_hdmi_qp_bridge_atomic_disable(struct drm_bridge *bridge,
@@ -791,13 +871,56 @@ static void dw_hdmi_qp_bridge_atomic_disable(struct drm_bridge *bridge,
 
 	hdmi->tmds_char_rate = 0;
 
+	dw_hdmi_qp_disable_scramb(hdmi);
+
+	hdmi->curr_conn = NULL;
 	hdmi->phy.ops->disable(hdmi, hdmi->phy.data);
 }
 
-static enum drm_connector_status
-dw_hdmi_qp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *connector)
+static int dw_hdmi_qp_reset_crtc(struct dw_hdmi_qp *hdmi,
+				 struct drm_connector *connector,
+				 struct drm_modeset_acquire_ctx *ctx)
+{
+	u8 config;
+	int ret;
+
+	ret = drm_scdc_readb(hdmi->bridge.ddc, SCDC_TMDS_CONFIG, &config);
+	if (ret < 0) {
+		dev_err(hdmi->dev, "Failed to read TMDS config: %d\n", ret);
+		return ret;
+	}
+
+	if (!!(config & SCDC_SCRAMBLING_ENABLE) == hdmi->scramb_enabled)
+		return 0;
+
+	drm_atomic_helper_connector_hdmi_hotplug(connector,
+						 connector_status_connected);
+	/*
+	 * Conform to HDMI 2.0 spec by ensuring scrambled data is not sent
+	 * before configuring the sink scrambling, as well as suspending any
+	 * TMDS transmission while changing the TMDS clock rate in the sink.
+	 */
+
+	dev_dbg(hdmi->dev, "resetting crtc\n");
+
+retry:
+	ret = drm_bridge_helper_reset_crtc(&hdmi->bridge, ctx);
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(ctx);
+		goto retry;
+	} else if (ret) {
+		dev_err(hdmi->dev, "Failed to reset crtc: %d\n", ret);
+	}
+
+	return ret;
+}
+
+static int dw_hdmi_qp_bridge_detect(struct drm_bridge *bridge,
+				    struct drm_connector *connector,
+				    struct drm_modeset_acquire_ctx *ctx)
 {
 	struct dw_hdmi_qp *hdmi = bridge->driver_private;
+	enum drm_connector_status status;
 	const struct drm_edid *drm_edid;
 
 	if (hdmi->no_hpd) {
@@ -808,7 +931,15 @@ dw_hdmi_qp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *connec
 			return connector_status_disconnected;
 	}
 
-	return hdmi->phy.ops->read_hpd(hdmi, hdmi->phy.data);
+	status = hdmi->phy.ops->read_hpd(hdmi, hdmi->phy.data);
+
+	dev_dbg(hdmi->dev, "%s status=%d scramb=%d\n", __func__,
+		status, hdmi->scramb_enabled);
+
+	if (status == connector_status_connected && hdmi->scramb_enabled)
+		dw_hdmi_qp_reset_crtc(hdmi, connector, ctx);
+
+	return status;
 }
 
 static const struct drm_edid *
@@ -832,12 +963,12 @@ dw_hdmi_qp_bridge_tmds_char_rate_valid(const struct drm_bridge *bridge,
 {
 	struct dw_hdmi_qp *hdmi = bridge->driver_private;
 
-	/*
-	 * TODO: when hdmi->no_hpd is 1 we must not support modes that
-	 * require scrambling, including every mode with a clock above
-	 * HDMI14_MAX_TMDSCLK.
-	 */
-	if (rate > HDMI14_MAX_TMDSCLK) {
+	if (hdmi->no_hpd && rate > HDMI14_MAX_TMDSCLK) {
+		dev_dbg(hdmi->dev, "Unsupported TMDS char rate in no_hpd mode: %lld\n", rate);
+		return MODE_CLOCK_HIGH;
+	}
+
+	if (rate > HDMI20_MAX_TMDSRATE) {
 		dev_dbg(hdmi->dev, "Unsupported TMDS char rate: %lld\n", rate);
 		return MODE_CLOCK_HIGH;
 	}
@@ -1197,7 +1328,7 @@ static const struct drm_bridge_funcs dw_hdmi_qp_bridge_funcs = {
 	.atomic_reset = drm_atomic_helper_bridge_reset,
 	.atomic_enable = dw_hdmi_qp_bridge_atomic_enable,
 	.atomic_disable = dw_hdmi_qp_bridge_atomic_disable,
-	.detect = dw_hdmi_qp_bridge_detect,
+	.detect_ctx = dw_hdmi_qp_bridge_detect,
 	.edid_read = dw_hdmi_qp_bridge_edid_read,
 	.hdmi_tmds_char_rate_valid = dw_hdmi_qp_bridge_tmds_char_rate_valid,
 	.hdmi_clear_avi_infoframe = dw_hdmi_qp_bridge_clear_avi_infoframe,
@@ -1287,6 +1418,8 @@ struct dw_hdmi_qp *dw_hdmi_qp_bind(struct platform_device *pdev,
 	if (IS_ERR(hdmi))
 		return ERR_CAST(hdmi);
 
+	INIT_DELAYED_WORK(&hdmi->scramb_work, dw_hdmi_qp_scramb_work);
+
 	hdmi->dev = dev;
 
 	regs = devm_platform_ioremap_resource(pdev, 0);

-- 
2.52.0

Claude review: drm/bridge: dw-hdmi-qp: Add high TMDS clock ratio and scrambling support

[Claude Code Review Bot]

Patch Review

This is the core patch. It has important functionality but also a significant locking concern.

**Locking issue in `dw_hdmi_qp_reset_crtc`:** This function is called from `dw_hdmi_qp_bridge_detect` (the `detect_ctx` callback), which is invoked with `connection_mutex` already held via `ctx`. Inside, it calls:

```c
retry:
    ret = drm_bridge_helper_reset_crtc(&hdmi->bridge, ctx);
    if (ret == -EDEADLK) {
        drm_modeset_backoff(ctx);
        goto retry;
    }
```

`drm_bridge_helper_reset_crtc` acquires `connection_mutex` with `ctx` (returns 0 due to `-EALREADY` handling in `modeset_lock()` at `drm_modeset_lock.c:321`), but then **unconditionally releases** it at the end:

```c
out:
    drm_modeset_unlock(&dev->mode_config.connection_mutex);
```

`drm_modeset_unlock` does `list_del_init(&lock->head)` + `ww_mutex_unlock(&lock->mutex)`, which fully releases the lock — even though the caller's framework (`drm_helper_probe_detect_ctx`) still expects it to be held. This is a locking protocol violation. After `drm_bridge_helper_reset_crtc` returns, `connection_mutex` is no longer held, creating a window where concurrent threads can modify connector state.

Additionally, calling `drm_modeset_backoff(ctx)` on a context that the function does not own is incorrect. `drm_modeset_backoff` drops **all** locks held by `ctx`, not just the contended one. The EDEADLK should be propagated back to the framework's retry loop rather than handled internally. The correct approach would be:

```c
ret = drm_bridge_helper_reset_crtc(&hdmi->bridge, ctx);
if (ret)
    return ret;  /* Let caller handle EDEADLK and other errors */
```

This would also require `dw_hdmi_qp_bridge_detect` to propagate the error:
```c
if (status == connector_status_connected && hdmi->scramb_enabled) {
    ret = dw_hdmi_qp_reset_crtc(hdmi, connector, ctx);
    if (ret)
        return ret;
}
```

However, even with this fix, `drm_bridge_helper_reset_crtc` still unlocks `connection_mutex` at line 57 after the EALREADY re-lock. This seems like `drm_bridge_helper_reset_crtc` is designed to be called **without** `connection_mutex` already held. The detect_ctx callback may need to be restructured to release `connection_mutex` before calling reset, or `drm_bridge_helper_reset_crtc` needs a variant that doesn't manage `connection_mutex` itself.

**Scrambling work queue safety:** The `scramb_work` delayed work accesses `hdmi->curr_conn`:

```c
static void dw_hdmi_qp_scramb_work(struct work_struct *work)
{
    ...
    if (!drm_scdc_get_scrambling_status(hdmi->curr_conn))
        dw_hdmi_qp_set_scramb(hdmi);
}
```

The lifecycle looks correct — `cancel_delayed_work_sync` in `dw_hdmi_qp_disable_scramb` runs before `curr_conn` is set to NULL in `atomic_disable`. However, consider: if the sink disconnects and HPD triggers detect_ctx (which resets the CRTC, triggering atomic_disable), the `cancel_delayed_work_sync` could block on a work item that is in the middle of writing SCDC. This should be fine functionally but might cause a delay.

**`SCDC_MIN_SOURCE_VERSION` naming:** The constant is used as:
```c
drm_scdc_writeb(hdmi->bridge.ddc, SCDC_SOURCE_VERSION,
                min_t(u8, ver, SCDC_MIN_SOURCE_VERSION));
```
The name `MIN` is misleading — this is the **maximum** source version the driver supports. `SCDC_SOURCE_VERSION` or `SCDC_MAX_SOURCE_VERSION` would be clearer.

**`SCRAMB_POLL_DELAY_MS` change from 3000 to 5000:** Not explained in the commit message. What motivated this change? It increases the window during which scrambling could be desynchronized from the sink.

**Pre-existing `drm_edid` leak in `no_hpd` path** (not introduced by this patch but worth noting):
```c
if (hdmi->no_hpd) {
    drm_edid = drm_edid_read_ddc(connector, bridge->ddc);
    if (drm_edid)
        return connector_status_connected;  /* drm_edid leaked */
```

**Ignored return value:** `dw_hdmi_qp_bridge_detect` ignores the return value of `dw_hdmi_qp_reset_crtc`:
```c
if (status == connector_status_connected && hdmi->scramb_enabled)
    dw_hdmi_qp_reset_crtc(hdmi, connector, ctx);

return status;
```
If the reset fails, the function still reports connected status. Errors should either be propagated or explicitly documented as intentionally ignored.

---
Generated by Claude Code Patch Reviewer

[PATCH v4 4/4] drm/rockchip: dw_hdmi_qp: Do not send HPD events for all connectors

[Cristian Ciocaltea]

In order to optimize the HPD event handling and run the detect cycle on
the affected connector only, make use of
drm_connector_helper_hpd_irq_event() instead of
drm_helper_hpd_irq_event().

Additionally, move devm_request_threaded_irq() after bridge connector
initialization.

Tested-by: Diederik de Haas <diederik@cknow-tech.com>
Tested-by: Maud Spierings <maud_spierings@hotmail.com>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 44 ++++++++++++--------------
 1 file changed, 20 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
index 1a09bcc96c3e..65dfaee15178 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
@@ -92,6 +92,7 @@ struct rockchip_hdmi_qp {
 	struct regmap *regmap;
 	struct regmap *vo_regmap;
 	struct rockchip_encoder encoder;
+	struct drm_connector *connector;
 	struct dw_hdmi_qp *hdmi;
 	struct phy *phy;
 	struct gpio_desc *frl_enable_gpio;
@@ -251,14 +252,10 @@ static void dw_hdmi_qp_rk3588_hpd_work(struct work_struct *work)
 	struct rockchip_hdmi_qp *hdmi = container_of(work,
 						     struct rockchip_hdmi_qp,
 						     hpd_work.work);
-	struct drm_device *drm = hdmi->encoder.encoder.dev;
-	bool changed;
+	bool changed = drm_connector_helper_hpd_irq_event(hdmi->connector);
 
-	if (drm) {
-		changed = drm_helper_hpd_irq_event(drm);
-		if (changed)
-			dev_dbg(hdmi->dev, "connector status changed\n");
-	}
+	if (changed)
+		dev_dbg(hdmi->dev, "connector status changed\n");
 }
 
 static irqreturn_t dw_hdmi_qp_rk3576_hardirq(int irq, void *dev_id)
@@ -466,13 +463,12 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 	struct dw_hdmi_qp_plat_data plat_data = {};
 	const struct rockchip_hdmi_qp_cfg *cfg;
 	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;
+	int ret, hpd_irq, i;
 
 	if (!pdev->dev.of_node)
 		return -ENODEV;
@@ -573,17 +569,9 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 	if (plat_data.cec_irq < 0)
 		return plat_data.cec_irq;
 
-	irq = platform_get_irq_byname(pdev, "hpd");
-	if (irq < 0)
-		return irq;
-
-	ret = devm_request_threaded_irq(hdmi->dev, irq,
-					cfg->ctrl_ops->hardirq_callback,
-					cfg->ctrl_ops->irq_callback,
-					IRQF_SHARED, "dw-hdmi-qp-hpd",
-					hdmi);
-	if (ret)
-		return ret;
+	hpd_irq = platform_get_irq_byname(pdev, "hpd");
+	if (hpd_irq < 0)
+		return hpd_irq;
 
 	drm_encoder_helper_add(encoder, &dw_hdmi_qp_rockchip_encoder_helper_funcs);
 	drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_TMDS);
@@ -597,12 +585,20 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 				     "Failed to bind dw-hdmi-qp");
 	}
 
-	connector = drm_bridge_connector_init(drm, encoder);
-	if (IS_ERR(connector))
-		return dev_err_probe(hdmi->dev, PTR_ERR(connector),
+	hdmi->connector = drm_bridge_connector_init(drm, encoder);
+	if (IS_ERR(hdmi->connector))
+		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->connector),
 				     "Failed to init bridge connector\n");
 
-	return drm_connector_attach_encoder(connector, encoder);
+	ret = drm_connector_attach_encoder(hdmi->connector, encoder);
+	if (ret)
+		return ret;
+
+	return devm_request_threaded_irq(hdmi->dev, hpd_irq,
+					 cfg->ctrl_ops->hardirq_callback,
+					 cfg->ctrl_ops->irq_callback,
+					 IRQF_SHARED, "dw-hdmi-qp-hpd",
+					 hdmi);
 }
 
 static void dw_hdmi_qp_rockchip_unbind(struct device *dev,

-- 
2.52.0

Claude review: drm/rockchip: dw_hdmi_qp: Do not send HPD events for all connectors

[Claude Code Review Bot]

Patch Review

This patch switches from `drm_helper_hpd_irq_event()` (which probes all connectors) to `drm_connector_helper_hpd_irq_event()` (which probes only the specific connector). The IRQ registration is also moved after bridge connector initialization so `hdmi->connector` is available.

**Looks good.** The reordering is correct:
1. `drm_bridge_connector_init` → sets `hdmi->connector`
2. `drm_connector_attach_encoder` → attaches it
3. `devm_request_threaded_irq` → now HPD IRQ can safely access `hdmi->connector`

The rename of `irq` to `hpd_irq` is a nice clarity improvement and avoids shadowing.

**One observation:** The old code had a `if (drm)` NULL check before calling `drm_helper_hpd_irq_event`. The new code unconditionally calls `drm_connector_helper_hpd_irq_event(hdmi->connector)`. Since the IRQ is only registered after the connector is initialized, `hdmi->connector` should never be NULL when the work runs, so this is fine.

---
Generated by Claude Code Patch Reviewer

Claude review: Add HDMI 2.0 support to DW HDMI QP TX

[Claude Code Review Bot]

Overall Series Review

Subject: Add HDMI 2.0 support to DW HDMI QP TX
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
Patches: 5
Reviewed: 2026-03-03T12:42:54.286151

---

This 4-patch series adds HDMI 2.0 support (modes above 340 MHz TMDS, like 4K@60Hz) to the Synopsys DesignWare HDMI QP TX bridge driver used on Rockchip RK3576/RK3588. The approach is sound: introduce an atomic `detect_ctx` bridge hook so the detect path can hold modeset locks and trigger a CRTC reset when SCDC scrambling state is lost on sink disconnect/reconnect.

The series is well-structured with clean separation between the framework plumbing (patches 1-2) and the driver-specific changes (patches 3-4). There is one significant locking concern in patch 3 regarding `drm_bridge_helper_reset_crtc` being called while `connection_mutex` is already held by the same acquire context, and the EDEADLK handling calling `drm_modeset_backoff` on a context the function doesn't own.

---
Generated by Claude Code Patch Reviewer

[PATCH RESEND v4 0/4] Add HDMI 2.0 support to DW HDMI QP TX

[Cristian Ciocaltea]

This patch series provides the missing support for high TMDS clock ratio
and scrambling to DW HDMI QP TX library, required for handling HDMI 2.0
display modes on RK3576 & RK3588 SoCs.

In order to allow addressing the SCDC status lost on sink disconnects,
it adds an atomic variant of the drm_bridge_funcs.detect() callback and
a new drm_bridge_detect_ctx() helper, which is further used in
drm_bridge_connector to switch to using the ->detect_ctx() hook.

Furthermore, it optimizes HPD event handling in dw_hdmi_qp Rockchip
platform driver to run the detect cycle on the affected connector only.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
Changes in v4:
- Fixed conflicts while rebasing onto latest drm-misc-next
- Link to v3: https://lore.kernel.org/r/20260119-dw-hdmi-qp-scramb-v3-0-bd8611730fc1@collabora.com

Changes in v3:
- Used drm_bridge_helper_reset_crtc() helper to reset the display
  pipeline and got rid of some boilerplate code (Maxime)
- Rebased onto latest drm-misc-next
- Link to v2: https://lore.kernel.org/r/20260113-dw-hdmi-qp-scramb-v2-0-ae7b2c58d24d@collabora.com

Changes in v2:
- Collected Tested-by tags from Diederik and Maud
- Rebased onto latest drm-misc-next
- Ensured the recently introduced 'no-hpd' support for dealing with
  unconnected/repurposed/broken HPD pin is limited to HDMI 1.4 rates
- Link to v1: https://lore.kernel.org/r/20251203-dw-hdmi-qp-scramb-v1-0-836fe7401a69@collabora.com

---
Cristian Ciocaltea (4):
      drm/bridge: Add ->detect_ctx hook and drm_bridge_detect_ctx()
      drm/bridge-connector: Switch to using ->detect_ctx hook
      drm/bridge: dw-hdmi-qp: Add high TMDS clock ratio and scrambling support
      drm/rockchip: dw_hdmi_qp: Do not send HPD events for all connectors

 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c   | 167 ++++++++++++++++++++++---
 drivers/gpu/drm/display/drm_bridge_connector.c |  73 +++++------
 drivers/gpu/drm/drm_bridge.c                   |  58 +++++++++
 drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c |  44 +++----
 include/drm/drm_bridge.h                       |  30 +++++
 5 files changed, 296 insertions(+), 76 deletions(-)
---
base-commit: 0dd7a23da214279cfb77b0eb03c38614867524d7
change-id: 20251203-dw-hdmi-qp-scramb-cdbd8b57ccf9
-- 
Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

[PATCH RESEND v4 3/4] drm/bridge: dw-hdmi-qp: Add high TMDS clock ratio and scrambling support

[Cristian Ciocaltea]

Add support for HDMI 2.0 display modes, e.g. 4K@60Hz, by permitting TMDS
character rates above the 340 MHz limit of HDMI 1.4b.

Hence, provide the required SCDC management, including the high TMDS
clock ratio and scrambling setup, and filter out the HDMI 2.1 modes.

Tested-by: Diederik de Haas <diederik@cknow-tech.com>
Tested-by: Maud Spierings <maud_spierings@hotmail.com>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 167 ++++++++++++++++++++++++---
 1 file changed, 150 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
index d649a1cf07f5..e40f16a364ed 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
@@ -2,6 +2,7 @@
 /*
  * Copyright (c) 2021-2022 Rockchip Electronics Co., Ltd.
  * Copyright (c) 2024 Collabora Ltd.
+ * Copyright (c) 2025 Amazon.com, Inc. or its affiliates.
  *
  * Author: Algea Cao <algea.cao@rock-chips.com>
  * Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
@@ -21,9 +22,11 @@
 #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/display/drm_scdc_helper.h>
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge.h>
+#include <drm/drm_bridge_helper.h>
 #include <drm/drm_connector.h>
 #include <drm/drm_edid.h>
 #include <drm/drm_modes.h>
@@ -39,8 +42,10 @@
 #define DDC_SEGMENT_ADDR	0x30
 
 #define HDMI14_MAX_TMDSCLK	340000000
+#define HDMI20_MAX_TMDSRATE	600000000
 
-#define SCRAMB_POLL_DELAY_MS	3000
+#define SCDC_MIN_SOURCE_VERSION	0x1
+#define SCRAMB_POLL_DELAY_MS	5000
 
 /*
  * Unless otherwise noted, entries in this table are 100% optimization.
@@ -164,6 +169,11 @@ struct dw_hdmi_qp {
 	} phy;
 
 	unsigned long ref_clk_rate;
+
+	struct drm_connector *curr_conn;
+	struct delayed_work scramb_work;
+	bool scramb_enabled;
+
 	struct regmap *regm;
 	int main_irq;
 
@@ -749,28 +759,98 @@ static struct i2c_adapter *dw_hdmi_qp_i2c_adapter(struct dw_hdmi_qp *hdmi)
 	return adap;
 }
 
+static bool dw_hdmi_qp_supports_scrambling(struct drm_display_info *display)
+{
+	if (!display->is_hdmi)
+		return false;
+
+	return display->hdmi.scdc.supported &&
+		display->hdmi.scdc.scrambling.supported;
+}
+
+static void dw_hdmi_qp_set_scramb(struct dw_hdmi_qp *hdmi)
+{
+	dev_dbg(hdmi->dev, "set scrambling\n");
+
+	drm_scdc_set_high_tmds_clock_ratio(hdmi->curr_conn, true);
+	drm_scdc_set_scrambling(hdmi->curr_conn, true);
+
+	schedule_delayed_work(&hdmi->scramb_work,
+			      msecs_to_jiffies(SCRAMB_POLL_DELAY_MS));
+}
+
+static void dw_hdmi_qp_scramb_work(struct work_struct *work)
+{
+	struct dw_hdmi_qp *hdmi = container_of(to_delayed_work(work),
+					       struct dw_hdmi_qp,
+					       scramb_work);
+	if (!drm_scdc_get_scrambling_status(hdmi->curr_conn))
+		dw_hdmi_qp_set_scramb(hdmi);
+}
+
+static void dw_hdmi_qp_enable_scramb(struct dw_hdmi_qp *hdmi)
+{
+	u8 ver;
+
+	if (!dw_hdmi_qp_supports_scrambling(&hdmi->curr_conn->display_info))
+		return;
+
+	drm_scdc_readb(hdmi->bridge.ddc, SCDC_SINK_VERSION, &ver);
+	drm_scdc_writeb(hdmi->bridge.ddc, SCDC_SOURCE_VERSION,
+			min_t(u8, ver, SCDC_MIN_SOURCE_VERSION));
+
+	dw_hdmi_qp_set_scramb(hdmi);
+	dw_hdmi_qp_write(hdmi, 1, SCRAMB_CONFIG0);
+
+	hdmi->scramb_enabled = true;
+
+	/* Wait at least 1 ms before resuming TMDS transmission */
+	usleep_range(1000, 5000);
+}
+
+static void dw_hdmi_qp_disable_scramb(struct dw_hdmi_qp *hdmi)
+{
+	if (!hdmi->scramb_enabled)
+		return;
+
+	dev_dbg(hdmi->dev, "disable scrambling\n");
+
+	hdmi->scramb_enabled = false;
+	cancel_delayed_work_sync(&hdmi->scramb_work);
+
+	dw_hdmi_qp_write(hdmi, 0, SCRAMB_CONFIG0);
+
+	if (hdmi->curr_conn->status == connector_status_connected) {
+		drm_scdc_set_scrambling(hdmi->curr_conn, false);
+		drm_scdc_set_high_tmds_clock_ratio(hdmi->curr_conn, false);
+	}
+}
+
 static void dw_hdmi_qp_bridge_atomic_enable(struct drm_bridge *bridge,
 					    struct drm_atomic_state *state)
 {
 	struct dw_hdmi_qp *hdmi = bridge->driver_private;
 	struct drm_connector_state *conn_state;
-	struct drm_connector *connector;
 	unsigned int op_mode;
 
-	connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
-	if (WARN_ON(!connector))
+	hdmi->curr_conn = drm_atomic_get_new_connector_for_encoder(state,
+								   bridge->encoder);
+	if (WARN_ON(!hdmi->curr_conn))
 		return;
 
-	conn_state = drm_atomic_get_new_connector_state(state, connector);
+	conn_state = drm_atomic_get_new_connector_state(state, hdmi->curr_conn);
 	if (WARN_ON(!conn_state))
 		return;
 
-	if (connector->display_info.is_hdmi) {
+	if (hdmi->curr_conn->display_info.is_hdmi) {
 		dev_dbg(hdmi->dev, "%s mode=HDMI %s rate=%llu bpc=%u\n", __func__,
 			drm_hdmi_connector_get_output_format_name(conn_state->hdmi.output_format),
 			conn_state->hdmi.tmds_char_rate, conn_state->hdmi.output_bpc);
 		op_mode = 0;
 		hdmi->tmds_char_rate = conn_state->hdmi.tmds_char_rate;
+
+		if (conn_state->hdmi.tmds_char_rate > HDMI14_MAX_TMDSCLK)
+			dw_hdmi_qp_enable_scramb(hdmi);
 	} else {
 		dev_dbg(hdmi->dev, "%s mode=DVI\n", __func__);
 		op_mode = OPMODE_DVI;
@@ -781,7 +861,7 @@ static void dw_hdmi_qp_bridge_atomic_enable(struct drm_bridge *bridge,
 	dw_hdmi_qp_mod(hdmi, HDCP2_BYPASS, HDCP2_BYPASS, HDCP2LOGIC_CONFIG0);
 	dw_hdmi_qp_mod(hdmi, op_mode, OPMODE_DVI, LINK_CONFIG0);
 
-	drm_atomic_helper_connector_hdmi_update_infoframes(connector, state);
+	drm_atomic_helper_connector_hdmi_update_infoframes(hdmi->curr_conn, state);
 }
 
 static void dw_hdmi_qp_bridge_atomic_disable(struct drm_bridge *bridge,
@@ -791,13 +871,56 @@ static void dw_hdmi_qp_bridge_atomic_disable(struct drm_bridge *bridge,
 
 	hdmi->tmds_char_rate = 0;
 
+	dw_hdmi_qp_disable_scramb(hdmi);
+
+	hdmi->curr_conn = NULL;
 	hdmi->phy.ops->disable(hdmi, hdmi->phy.data);
 }
 
-static enum drm_connector_status
-dw_hdmi_qp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *connector)
+static int dw_hdmi_qp_reset_crtc(struct dw_hdmi_qp *hdmi,
+				 struct drm_connector *connector,
+				 struct drm_modeset_acquire_ctx *ctx)
+{
+	u8 config;
+	int ret;
+
+	ret = drm_scdc_readb(hdmi->bridge.ddc, SCDC_TMDS_CONFIG, &config);
+	if (ret < 0) {
+		dev_err(hdmi->dev, "Failed to read TMDS config: %d\n", ret);
+		return ret;
+	}
+
+	if (!!(config & SCDC_SCRAMBLING_ENABLE) == hdmi->scramb_enabled)
+		return 0;
+
+	drm_atomic_helper_connector_hdmi_hotplug(connector,
+						 connector_status_connected);
+	/*
+	 * Conform to HDMI 2.0 spec by ensuring scrambled data is not sent
+	 * before configuring the sink scrambling, as well as suspending any
+	 * TMDS transmission while changing the TMDS clock rate in the sink.
+	 */
+
+	dev_dbg(hdmi->dev, "resetting crtc\n");
+
+retry:
+	ret = drm_bridge_helper_reset_crtc(&hdmi->bridge, ctx);
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(ctx);
+		goto retry;
+	} else if (ret) {
+		dev_err(hdmi->dev, "Failed to reset crtc: %d\n", ret);
+	}
+
+	return ret;
+}
+
+static int dw_hdmi_qp_bridge_detect(struct drm_bridge *bridge,
+				    struct drm_connector *connector,
+				    struct drm_modeset_acquire_ctx *ctx)
 {
 	struct dw_hdmi_qp *hdmi = bridge->driver_private;
+	enum drm_connector_status status;
 	const struct drm_edid *drm_edid;
 
 	if (hdmi->no_hpd) {
@@ -808,7 +931,15 @@ dw_hdmi_qp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *connec
 			return connector_status_disconnected;
 	}
 
-	return hdmi->phy.ops->read_hpd(hdmi, hdmi->phy.data);
+	status = hdmi->phy.ops->read_hpd(hdmi, hdmi->phy.data);
+
+	dev_dbg(hdmi->dev, "%s status=%d scramb=%d\n", __func__,
+		status, hdmi->scramb_enabled);
+
+	if (status == connector_status_connected && hdmi->scramb_enabled)
+		dw_hdmi_qp_reset_crtc(hdmi, connector, ctx);
+
+	return status;
 }
 
 static const struct drm_edid *
@@ -832,12 +963,12 @@ dw_hdmi_qp_bridge_tmds_char_rate_valid(const struct drm_bridge *bridge,
 {
 	struct dw_hdmi_qp *hdmi = bridge->driver_private;
 
-	/*
-	 * TODO: when hdmi->no_hpd is 1 we must not support modes that
-	 * require scrambling, including every mode with a clock above
-	 * HDMI14_MAX_TMDSCLK.
-	 */
-	if (rate > HDMI14_MAX_TMDSCLK) {
+	if (hdmi->no_hpd && rate > HDMI14_MAX_TMDSCLK) {
+		dev_dbg(hdmi->dev, "Unsupported TMDS char rate in no_hpd mode: %lld\n", rate);
+		return MODE_CLOCK_HIGH;
+	}
+
+	if (rate > HDMI20_MAX_TMDSRATE) {
 		dev_dbg(hdmi->dev, "Unsupported TMDS char rate: %lld\n", rate);
 		return MODE_CLOCK_HIGH;
 	}
@@ -1197,7 +1328,7 @@ static const struct drm_bridge_funcs dw_hdmi_qp_bridge_funcs = {
 	.atomic_reset = drm_atomic_helper_bridge_reset,
 	.atomic_enable = dw_hdmi_qp_bridge_atomic_enable,
 	.atomic_disable = dw_hdmi_qp_bridge_atomic_disable,
-	.detect = dw_hdmi_qp_bridge_detect,
+	.detect_ctx = dw_hdmi_qp_bridge_detect,
 	.edid_read = dw_hdmi_qp_bridge_edid_read,
 	.hdmi_tmds_char_rate_valid = dw_hdmi_qp_bridge_tmds_char_rate_valid,
 	.hdmi_clear_avi_infoframe = dw_hdmi_qp_bridge_clear_avi_infoframe,
@@ -1287,6 +1418,8 @@ struct dw_hdmi_qp *dw_hdmi_qp_bind(struct platform_device *pdev,
 	if (IS_ERR(hdmi))
 		return ERR_CAST(hdmi);
 
+	INIT_DELAYED_WORK(&hdmi->scramb_work, dw_hdmi_qp_scramb_work);
+
 	hdmi->dev = dev;
 
 	regs = devm_platform_ioremap_resource(pdev, 0);

-- 
2.52.0

Claude review: drm/bridge: dw-hdmi-qp: Add high TMDS clock ratio and scrambling support

[Claude Code Review Bot]

Patch Review

**This is the core patch. Several observations:**

1. **Pre-existing EDID leak in `no_hpd` detect path (line 927-931):**
   ```c
   drm_edid = drm_edid_read_ddc(connector, bridge->ddc);
   if (drm_edid)
       return connector_status_connected;
   ```
   The `drm_edid` is never freed. This is a pre-existing bug, but since this patch changes the function signature and touches surrounding code, it would be good to fix it here with a `drm_edid_free(drm_edid)` before the return.

2. **`curr_conn` lifetime concerns:** `hdmi->curr_conn` is set in `atomic_enable` and cleared in `atomic_disable`. The `scramb_work` delayed work accesses `hdmi->curr_conn` (line 910 via `dw_hdmi_qp_scramb_work` → `dw_hdmi_qp_set_scramb`). While `cancel_delayed_work_sync` is called in `disable_scramb`, there's a window: if the work fires and calls `dw_hdmi_qp_set_scramb()` which calls `schedule_delayed_work()` *after* the cancel but before `scramb_enabled = false` — actually no, `cancel_delayed_work_sync` waits for completion, and `scramb_enabled` is set to false before cancel. Wait, looking at the order in `dw_hdmi_qp_disable_scramb()`:

   ```c
   hdmi->scramb_enabled = false;
   cancel_delayed_work_sync(&hdmi->scramb_work);
   ```

   The work function `dw_hdmi_qp_scramb_work` doesn't check `scramb_enabled`, so if the work is currently executing when `disable_scramb` sets `scramb_enabled = false`, the work could re-schedule itself via `dw_hdmi_qp_set_scramb()` → `schedule_delayed_work()` *before* `cancel_delayed_work_sync` completes. `cancel_delayed_work_sync` will wait for the currently-running work to finish, but if the work re-schedules itself, the cancellation might not catch the newly scheduled instance. This is a potential race. Consider checking `scramb_enabled` in `dw_hdmi_qp_scramb_work()` before re-scheduling, or using a different cancellation pattern.

3. **`SCDC_MIN_SOURCE_VERSION` naming:** The constant is named `SCDC_MIN_SOURCE_VERSION` with value `0x1`, but it's used as the maximum version the source supports (line 922-923: `min_t(u8, ver, SCDC_MIN_SOURCE_VERSION)`). The `min_t` clamps the source version to at most `0x1`. The naming is confusing — something like `SCDC_MAX_SOURCE_VERSION` would be clearer.

4. **Error handling in `dw_hdmi_qp_set_scramb` (lines 795-799):** Neither `drm_scdc_set_high_tmds_clock_ratio()` nor `drm_scdc_set_scrambling()` have their return values checked. If these I2C writes fail (e.g., due to a flaky connection), scrambling is assumed to be configured but it isn't. At minimum, log a warning on failure.

5. **`dw_hdmi_qp_reset_crtc` error propagation:** When called from `dw_hdmi_qp_bridge_detect()` (line 940), the return value of `dw_hdmi_qp_reset_crtc()` is ignored. If the CRTC reset fails, the function still returns `connector_status_connected`. This seems intentional (detection succeeded, reset is best-effort), but worth a comment.

6. **`dw_hdmi_qp_enable_scramb` called before PHY init:** In `atomic_enable`, `dw_hdmi_qp_enable_scramb()` is called (line 851) *before* `hdmi->phy.ops->init()` (line 859). The `enable_scramb` function does I2C writes to the sink via SCDC and writes to hardware register `SCRAMB_CONFIG0`, then sleeps 1ms. The PHY init happens after. Is this ordering correct? The HDMI 2.0 spec requires the source to stop TMDS transmission, configure scrambling, then resume — so doing scramb setup before PHY init (which presumably starts TMDS) seems correct.

7. **`HDMI20_MAX_TMDSRATE` value of 600 MHz:** HDMI 2.0 specifies a maximum of 600 MHz TMDS character rate (corresponding to 18 Gbps). This is correct.

---
Generated by Claude Code Patch Reviewer