[PATCH v5 00/10] Add HDMI 2.0 support to DW HDMI QP TX

[PATCH v5 00/10] Add HDMI 2.0 support to DW HDMI QP TX

[Cristian Ciocaltea]

Enable HDMI 2.0 display modes (e.g. 4K@60Hz) on the Synopsys DW HDMI QP
TX controller, as found in Rockchip RK3576 & RK3588 SoCs, by adding SCDC
management for high TMDS clock ratio and scrambling.

Since SCDC state is lost on sink disconnects, the bridge driver needs to
trigger a CRTC reset during connector detection.  To support this, the
series introduces an atomic-aware detect_ctx bridge hook and a
drm_bridge_detect_ctx() helper (patches 1-4), then uses them to
implement the SCDC scrambling feature (patch 5).

Patches 6-8 are minor cleanups in the dw-hdmi-qp and Rockchip platform
drivers.  Patches 9-10 improve HPD handling by deferring IRQ
registration until the connector is fully initialized and restricting
HPD events to the affected connector.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
Changes in v5:
- Added new patches: 1/10, 3/10, 6/10, 7/10, 8/10
- Removed redundant no-op error check in drm_bridge_helper_reset_crtc()
  (patch 1)
- Removed the EDEADLK retry loop from the bridge .detect_ctx() callback,
  as that's already handled in the drm_bridge_detect_ctx() helper or by
  the caller when ctx is provided (patch 2)
- Refactored drm_bridge_detect() to delegate to drm_bridge_detect_ctx()
  and added a WARN_ON for unexpected negative return values (patch 2)
- Split the bridge-connector .detect_ctx() switch into a preparatory
  patch to use cached connector status in .get_modes() (patch 3)
- Improved error handling in SCDC scrambling setup: roll back high TMDS
  clock ratio on scrambling failure, reset scramb_enabled flag on
  set_scramb failure, and add SCDC version read/write error checks
  (patch 5)
- Annotated scramb_enabled with READ_ONCE/WRITE_ONCE for cross-context
  access between modeset paths and the scrambling work item (patch 5)
- Renamed SCDC_MIN_SOURCE_VERSION to SCDC_MAX_SOURCE_VERSION (patch 5)
- Rate limited i2c error messages (patch 6)
- Added missing newlines in dev_err_probe() messages (patch 7)
- Replaced indirect device pointer accesses with local dev variable in
  bind() (patch 8)
- Split the HPD connector restriction (formerly patch 4/4): register HPD
  IRQ after connector setup first (patch 9), then restrict HPD event to
  the affected connector (patch 10); also collected R-b from Heiko
- Rebased onto latest drm-misc-next
- Link to v4: https://lore.kernel.org/r/20260303-dw-hdmi-qp-scramb-v4-0-317d3b8bd219@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 (10):
      drm/bridge: Remove redundant error check in drm_bridge_helper_reset_crtc()
      drm/bridge: Add detect_ctx hook and drm_bridge_detect_ctx() helper
      drm/bridge-connector: Use cached connector status in .get_modes()
      drm/bridge-connector: Switch to .detect_ctx() for connector detection
      drm/bridge: dw-hdmi-qp: Add HDMI 2.0 SCDC scrambling and high TMDS clock ratio support
      drm/bridge: dw-hdmi-qp: Rate limit i2c read error messages
      drm/rockchip: dw_hdmi_qp: Add missing newlines in dev_err_probe() messages
      drm/rockchip: dw_hdmi_qp: Use local dev variable consistently in bind()
      drm/rockchip: dw_hdmi_qp: Register HPD IRQ after connector setup
      drm/rockchip: dw_hdmi_qp: Restrict HPD event to the affected connector

 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c   | 192 ++++++++++++++++++++++---
 drivers/gpu/drm/display/drm_bridge_connector.c |  75 +++++-----
 drivers/gpu/drm/drm_bridge.c                   |  66 ++++++++-
 drivers/gpu/drm/drm_bridge_helper.c            |   2 -
 drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c |  68 ++++-----
 include/drm/drm_bridge.h                       |  43 +++++-
 6 files changed, 344 insertions(+), 102 deletions(-)
---
base-commit: 03af6c3afc4893988ceed54531f5dde4bebd6024
change-id: 20251203-dw-hdmi-qp-scramb-cdbd8b57ccf9

[PATCH v5 01/10] drm/bridge: Remove redundant error check in drm_bridge_helper_reset_crtc()

[Cristian Ciocaltea]

Remove the no-op error check after drm_atomic_helper_reset_crtc() since
the goto target is the immediately following label and the return value
is already propagated correctly without it.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/drm_bridge_helper.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_bridge_helper.c b/drivers/gpu/drm/drm_bridge_helper.c
index 420f29cf3e54..0a3c8fee66b3 100644
--- a/drivers/gpu/drm/drm_bridge_helper.c
+++ b/drivers/gpu/drm/drm_bridge_helper.c
@@ -50,8 +50,6 @@ int drm_bridge_helper_reset_crtc(struct drm_bridge *bridge,
 
 	crtc = connector->state->crtc;
 	ret = drm_atomic_helper_reset_crtc(crtc, ctx);
-	if (ret)
-		goto out;
 
 out:
 	drm_modeset_unlock(&dev->mode_config.connection_mutex);

-- 
2.53.0

Re: [PATCH v5 01/10] drm/bridge: Remove redundant error check in drm_bridge_helper_reset_crtc()

[Dmitry Baryshkov]

On Sun, Apr 26, 2026 at 03:20:13AM +0300, Cristian Ciocaltea wrote:
> Remove the no-op error check after drm_atomic_helper_reset_crtc() since
> the goto target is the immediately following label and the return value
> is already propagated correctly without it.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/gpu/drm/drm_bridge_helper.c | 2 --
>  1 file changed, 2 deletions(-)
> 

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


-- 
With best wishes
Dmitry

Claude review: drm/bridge: Remove redundant error check in drm_bridge_helper_reset_crtc()

[Claude Code Review Bot]

Patch Review

Straightforward cleanup. The existing code is:

```c
	ret = drm_atomic_helper_reset_crtc(crtc, ctx);
	if (ret)
		goto out;

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

The `goto out` on error jumps to the immediately following label, making the check a no-op. The removal is correct — `ret` is returned either way.

**Reviewed-by worthy. No issues.**

---

---
Generated by Claude Code Patch Reviewer

[PATCH v5 02/10] drm/bridge: Add detect_ctx hook and drm_bridge_detect_ctx() helper

[Cristian Ciocaltea]

Add an atomic-aware .detect_ctx() callback to drm_bridge_funcs and a
drm_bridge_detect_ctx() helper that accepts an optional
drm_modeset_acquire_ctx.

This enables bridge drivers to perform operations requiring modeset
locking during connector detection, such as SCDC management for HDMI
2.0.  When both ->detect_ctx and ->detect are defined, the former takes
precedence.  When ctx is NULL, locking is managed internally with EDEADLK
retry.

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 | 66 ++++++++++++++++++++++++++++++++++++++++----
 include/drm/drm_bridge.h     | 43 +++++++++++++++++++++++++----
 2 files changed, 98 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 986e4c79a4e0..a5c6c2ea4ba6 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -1361,9 +1361,9 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_check);
  * @connector: attached connector
  *
  * If the bridge supports output detection, as reported by the
- * DRM_BRIDGE_OP_DETECT bridge ops flag, call &drm_bridge_funcs.detect for the
- * bridge and return the connection status. Otherwise return
- * connector_status_unknown.
+ * 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.
  *
  * RETURNS:
  * The detection status on success, or connector_status_unknown if the bridge
@@ -1372,12 +1372,68 @@ EXPORT_SYMBOL(drm_atomic_bridge_chain_check);
 enum drm_connector_status
 drm_bridge_detect(struct drm_bridge *bridge, struct drm_connector *connector)
 {
+	return drm_bridge_detect_ctx(bridge, connector, NULL);
+}
+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.
+ *
+ * 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)
+{
+	struct drm_modeset_acquire_ctx br_ctx;
+	int ret;
+
 	if (!(bridge->ops & DRM_BRIDGE_OP_DETECT))
 		return connector_status_unknown;
 
-	return bridge->funcs->detect(bridge, connector);
+	if (!bridge->funcs->detect_ctx)
+		return bridge->funcs->detect(bridge, connector);
+
+	if (ctx) {
+		ret = bridge->funcs->detect_ctx(bridge, connector, ctx);
+		if (ret == -EDEADLK)
+			return ret;
+
+		goto out;
+	}
+
+	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;
+	}
+
+	drm_modeset_drop_locks(&br_ctx);
+	drm_modeset_acquire_fini(&br_ctx);
+out:
+	if (WARN_ON(ret < 0))
+		ret = connector_status_unknown;
+
+	return ret;
 }
-EXPORT_SYMBOL_GPL(drm_bridge_detect);
+EXPORT_SYMBOL_GPL(drm_bridge_detect_ctx);
 
 /**
  * drm_bridge_get_modes - fill all modes currently valid for the sink into the
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index d6cd0f5af045..6c751b96c3c9 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -535,10 +535,12 @@ struct drm_bridge_funcs {
 	 *
 	 * 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 non-atomic version of detect_ctx() callback, and is
+	 * optional. If both are implemented, it is ignored. If none is
+	 * 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.
 	 *
 	 * RETURNS:
 	 *
@@ -547,6 +549,32 @@ 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 is the atomic version of detect() callback, and is optional.
+	 * If both are implemented, it takes precedence. If none is 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.
+	 *
+	 * 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:
 	 *
@@ -1002,8 +1030,8 @@ struct drm_bridge_timings {
 enum drm_bridge_ops {
 	/**
 	 * @DRM_BRIDGE_OP_DETECT: The bridge can detect displays connected to
-	 * its output. Bridges that set this flag shall implement the
-	 * &drm_bridge_funcs->detect callback.
+	 * its output. Bridges that set this flag shall implement either the
+	 * &drm_bridge_funcs->detect or &drm_bridge_funcs->detect_ctx callbacks.
 	 */
 	DRM_BRIDGE_OP_DETECT = BIT(0),
 	/**
@@ -1565,6 +1593,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.53.0

Re: [PATCH v5 02/10] drm/bridge: Add detect_ctx hook and drm_bridge_detect_ctx() helper

[Heiko Stuebner]

Am Sonntag, 26. April 2026, 02:20:14 Mitteleuropäische Sommerzeit schrieb Cristian Ciocaltea:
> Add an atomic-aware .detect_ctx() callback to drm_bridge_funcs and a
> drm_bridge_detect_ctx() helper that accepts an optional
> drm_modeset_acquire_ctx.
> 
> This enables bridge drivers to perform operations requiring modeset
> locking during connector detection, such as SCDC management for HDMI
> 2.0.  When both ->detect_ctx and ->detect are defined, the former takes
> precedence.  When ctx is NULL, locking is managed internally with EDEADLK
> retry.
> 
> 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>

while this looks sane to me, my drm insights are spotty, so I can
only provide the lighter

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

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

[Claude Code Review Bot]

Patch Review

This is the core infrastructure patch. Key observations:

**Return type change:** `drm_bridge_detect_ctx()` returns `int` rather than `enum drm_connector_status`, allowing `-EDEADLK` to propagate. This is a reasonable approach since connector status values are positive. The `WARN_ON(ret < 0)` fallback at the end catches unexpected negative returns:

```c
out:
	if (WARN_ON(ret < 0))
		ret = connector_status_unknown;

	return ret;
```

This is good defensive coding. However, note that when `ctx != NULL` and `ret == -EDEADLK`, the function returns early (correct), but for any *other* negative return from `detect_ctx` with a caller-provided ctx, it falls through to the `WARN_ON` and converts to `connector_status_unknown`. That seems intentional and fine.

**Locking in the NULL-ctx path:** When `ctx` is NULL, the function acquires `connection_mutex` internally. This matches what the DRM core does for the connector `.detect_ctx` path. The EDEADLK retry loop is correctly structured.

**Delegation from drm_bridge_detect():** The existing `drm_bridge_detect()` now simply delegates to `drm_bridge_detect_ctx(bridge, connector, NULL)`. This means old callers that only set `.detect` go through an extra function call but the behavior is preserved since `!bridge->funcs->detect_ctx` returns `bridge->funcs->detect(bridge, connector)` directly.

**One subtlety:** When `ctx` is NULL and `detect_ctx` is implemented, the helper acquires `connection_mutex` internally. But when going through the old `drm_bridge_detect()` → `drm_bridge_detect_ctx(NULL)` path for a bridge that only has `.detect`, no locking is acquired — which matches the previous behavior. Good.

**Documentation** is clear and correctly describes the precedence rules (`detect_ctx` over `detect`).

**No issues found.**

---

---
Generated by Claude Code Patch Reviewer

[PATCH v5 03/10] drm/bridge-connector: Use cached connector status in .get_modes()

[Cristian Ciocaltea]

Replace the active drm_bridge_connector_detect() call in get_modes()
with a read of the already-cached connector->status.

The .get_modes() callback is only invoked from
drm_helper_probe_single_connector_modes(), which has already retrieved
the connector status.  Calling detect again is redundant and triggers a
duplicate hotplug event.  This is also a prerequisite for switching to
the .detect_ctx() hook, which requires a drm_modeset_acquire_ctx not
available in the .get_modes() path.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/display/drm_bridge_connector.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index 39cc18f78eda..575c09e9cb73 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -294,12 +294,10 @@ static const struct drm_connector_funcs drm_bridge_connector_funcs = {
 static int drm_bridge_connector_get_modes_edid(struct drm_connector *connector,
 					       struct drm_bridge *bridge)
 {
-	enum drm_connector_status status;
 	const struct drm_edid *drm_edid;
 	int n;
 
-	status = drm_bridge_connector_detect(connector, false);
-	if (status != connector_status_connected)
+	if (connector->status != connector_status_connected)
 		goto no_edid;
 
 	drm_edid = drm_bridge_edid_read(bridge, connector);

-- 
2.53.0

Re: [PATCH v5 03/10] drm/bridge-connector: Use cached connector status in .get_modes()

[Dmitry Baryshkov]

On Sun, Apr 26, 2026 at 03:20:15AM +0300, Cristian Ciocaltea wrote:
> Replace the active drm_bridge_connector_detect() call in get_modes()
> with a read of the already-cached connector->status.
> 
> The .get_modes() callback is only invoked from
> drm_helper_probe_single_connector_modes(), which has already retrieved
> the connector status.  Calling detect again is redundant and triggers a
> duplicate hotplug event.  This is also a prerequisite for switching to
> the .detect_ctx() hook, which requires a drm_modeset_acquire_ctx not
> available in the .get_modes() path.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/gpu/drm/display/drm_bridge_connector.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 

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


-- 
With best wishes
Dmitry

Claude review: drm/bridge-connector: Use cached connector status in .get_modes()

[Claude Code Review Bot]

Patch Review

```c
-	status = drm_bridge_connector_detect(connector, false);
-	if (status != connector_status_connected)
+	if (connector->status != connector_status_connected)
		goto no_edid;
```

The commit message correctly explains that `.get_modes()` is only called from `drm_helper_probe_single_connector_modes()`, which has already run detection and cached the result in `connector->status`. This eliminates a redundant detect call and is a prerequisite for patch 4 since `detect_ctx` requires a `drm_modeset_acquire_ctx` not available in the `get_modes` path.

**Correct and well-motivated. No issues.**

---

---
Generated by Claude Code Patch Reviewer

[PATCH v5 04/10] drm/bridge-connector: Switch to .detect_ctx() for connector detection

[Cristian Ciocaltea]

Use the atomic .detect_ctx() connector helper hook and invoke
drm_bridge_detect_ctx() to propagate the modeset acquire context to
bridge drivers.

This enables bridge drivers to perform modeset operations during
detection, which is needed for managing SCDC state lost on sink
disconnects in HDMI 2.0 scenarios.

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 | 71 ++++++++++++++------------
 1 file changed, 37 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index 575c09e9cb73..ca2ba7630f6b 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)
 {
@@ -382,6 +384,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.53.0

Re: [PATCH v5 04/10] drm/bridge-connector: Switch to .detect_ctx() for connector detection

[Dmitry Baryshkov]

On Sun, Apr 26, 2026 at 03:20:16AM +0300, Cristian Ciocaltea wrote:
> Use the atomic .detect_ctx() connector helper hook and invoke
> drm_bridge_detect_ctx() to propagate the modeset acquire context to
> bridge drivers.
> 
> This enables bridge drivers to perform modeset operations during
> detection, which is needed for managing SCDC state lost on sink
> disconnects in HDMI 2.0 scenarios.
> 
> 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 | 71 ++++++++++++++------------
>  1 file changed, 37 insertions(+), 34 deletions(-)
> 

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


-- 
With best wishes
Dmitry

Claude review: drm/bridge-connector: Switch to .detect_ctx() for connector detection

[Claude Code Review Bot]

Patch Review

This patch removes the old `.detect` callback from `drm_bridge_connector_funcs` and replaces it with a `.detect_ctx` helper hook:

```c
static int drm_bridge_connector_detect_ctx(struct drm_connector *connector,
					   struct drm_modeset_acquire_ctx *ctx,
					   bool force)
```

The implementation correctly calls `drm_bridge_detect_ctx()` instead of `detect->funcs->detect()` directly, which routes through the proper fallback logic in patch 2. The error propagation (`if (ret < 0) return ret`) allows EDEADLK to bubble up to the DRM core's retry loop.

**One observation:** With patch 4, the old `drm_bridge_connector_detect()` is completely removed. Any external caller that was using the `connector->funcs->detect` path will now get NULL. The DRM core's `drm_helper_probe_single_connector_modes()` prefers `.detect_ctx` over `.detect` when available, so this should be fine. The `.force` callback still exists and isn't changed, which is correct.

**No issues found.**

---

---
Generated by Claude Code Patch Reviewer

[PATCH v5 05/10] drm/bridge: dw-hdmi-qp: Add HDMI 2.0 SCDC scrambling and high TMDS clock ratio support

[Cristian Ciocaltea]

Enable HDMI 2.0 display modes (e.g. 4K@60Hz) by adding SCDC management
for the high TMDS clock ratio and scrambling, required when the TMDS
character rate exceeds the 340 MHz HDMI 1.4b limit.

A periodic work item monitors the sink's scrambling status to recover
from sink-side resets.  On hotplug detect, if SCDC scrambling state is
out of sync with the driver, trigger a CRTC reset to re-establish the
link.

Reject modes requiring TMDS rates above 600 MHz, as those fall in the
HDMI 2.1 FRL domain which is not supported. In no_hpd configurations,
further restrict to 340 MHz since SCDC requires a connected sink.

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 | 188 ++++++++++++++++++++++++---
 1 file changed, 172 insertions(+), 16 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..c482a8e7da25 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,7 +42,9 @@
 #define DDC_SEGMENT_ADDR	0x30
 
 #define HDMI14_MAX_TMDSCLK	340000000
+#define HDMI20_MAX_TMDSRATE	600000000
 
+#define SCDC_MAX_SOURCE_VERSION	0x1
 #define SCRAMB_POLL_DELAY_MS	3000
 
 /*
@@ -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,124 @@ 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 int dw_hdmi_qp_set_scramb(struct dw_hdmi_qp *hdmi)
+{
+	bool done;
+
+	dev_dbg(hdmi->dev, "set scrambling\n");
+
+	done = drm_scdc_set_high_tmds_clock_ratio(hdmi->curr_conn, true);
+	if (!done)
+		return -EIO;
+
+	done = drm_scdc_set_scrambling(hdmi->curr_conn, true);
+	if (!done) {
+		drm_scdc_set_high_tmds_clock_ratio(hdmi->curr_conn, false);
+		return -EIO;
+	}
+
+	schedule_delayed_work(&hdmi->scramb_work,
+			      msecs_to_jiffies(SCRAMB_POLL_DELAY_MS));
+	return 0;
+}
+
+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 (READ_ONCE(hdmi->scramb_enabled) &&
+	    !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)
+{
+	int ret;
+	u8 ver;
+
+	if (!dw_hdmi_qp_supports_scrambling(&hdmi->curr_conn->display_info))
+		return;
+
+	ret = drm_scdc_readb(hdmi->bridge.ddc, SCDC_SINK_VERSION, &ver);
+	if (ret) {
+		dev_err(hdmi->dev, "Failed to read SCDC_SINK_VERSION: %d\n", ret);
+		return;
+	}
+
+	ret = drm_scdc_writeb(hdmi->bridge.ddc, SCDC_SOURCE_VERSION,
+			      min_t(u8, ver, SCDC_MAX_SOURCE_VERSION));
+	if (ret) {
+		dev_err(hdmi->dev, "Failed to write SCDC_SOURCE_VERSION: %d\n", ret);
+		return;
+	}
+
+	WRITE_ONCE(hdmi->scramb_enabled, true);
+
+	ret = dw_hdmi_qp_set_scramb(hdmi);
+	if (ret) {
+		hdmi->scramb_enabled = false;
+		return;
+	}
+
+	dw_hdmi_qp_write(hdmi, 1, SCRAMB_CONFIG0);
+
+	/* 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");
+
+	WRITE_ONCE(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 +887,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,14 +897,49 @@ 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");
+
+	return drm_bridge_helper_reset_crtc(&hdmi->bridge, ctx);
+}
+
+static int dw_hdmi_qp_bridge_detect_ctx(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;
+	int ret;
 
 	if (hdmi->no_hpd) {
 		drm_edid = drm_edid_read_ddc(connector, bridge->ddc);
@@ -808,7 +949,20 @@ 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) {
+		ret = dw_hdmi_qp_reset_crtc(hdmi, connector, ctx);
+		if (ret == -EDEADLK)
+			return ret;
+		if (ret < 0)
+			status = connector_status_unknown;
+	}
+
+	return status;
 }
 
 static const struct drm_edid *
@@ -832,12 +986,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 +1351,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_ctx,
 	.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 +1441,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.53.0

Re: [PATCH v5 05/10] drm/bridge: dw-hdmi-qp: Add HDMI 2.0 SCDC scrambling and high TMDS clock ratio support

[Heiko Stuebner]

Am Sonntag, 26. April 2026, 02:20:17 Mitteleuropäische Sommerzeit schrieb Cristian Ciocaltea:
> Enable HDMI 2.0 display modes (e.g. 4K@60Hz) by adding SCDC management
> for the high TMDS clock ratio and scrambling, required when the TMDS
> character rate exceeds the 340 MHz HDMI 1.4b limit.
> 
> A periodic work item monitors the sink's scrambling status to recover
> from sink-side resets.  On hotplug detect, if SCDC scrambling state is
> out of sync with the driver, trigger a CRTC reset to re-establish the
> link.
> 
> Reject modes requiring TMDS rates above 600 MHz, as those fall in the
> HDMI 2.1 FRL domain which is not supported. In no_hpd configurations,
> further restrict to 340 MHz since SCDC requires a connected sink.
> 
> 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 | 188 ++++++++++++++++++++++++---
>  1 file changed, 172 insertions(+), 16 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..c482a8e7da25 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.

I have no real clue about the inner workings of hdmi, but that line
does make me curious, because no part of the patch authorship does
mention Amazon ;-) .

Also I think "or it's affiliates" might be way too broad, because this
can be essentially everyone. So noone in the future will know who to
ask on copyright questions.

Similarly when _somebody_ comes forward with "I hold a copyright on this"
no-one could verify this claim as well.

I somehow expect _one_ entity being specified, not possibly hundreds.


Heiko

Re: [PATCH v5 05/10] drm/bridge: dw-hdmi-qp: Add HDMI 2.0 SCDC scrambling and high TMDS clock ratio support

[Cristian Ciocaltea]

Hello Heiko,

Thanks for the quick reviews!

On 4/27/26 1:49 PM, Heiko Stuebner wrote:
> Am Sonntag, 26. April 2026, 02:20:17 Mitteleuropäische Sommerzeit schrieb Cristian Ciocaltea:
>> Enable HDMI 2.0 display modes (e.g. 4K@60Hz) by adding SCDC management
>> for the high TMDS clock ratio and scrambling, required when the TMDS
>> character rate exceeds the 340 MHz HDMI 1.4b limit.
>>
>> A periodic work item monitors the sink's scrambling status to recover
>> from sink-side resets.  On hotplug detect, if SCDC scrambling state is
>> out of sync with the driver, trigger a CRTC reset to re-establish the
>> link.
>>
>> Reject modes requiring TMDS rates above 600 MHz, as those fall in the
>> HDMI 2.1 FRL domain which is not supported. In no_hpd configurations,
>> further restrict to 340 MHz since SCDC requires a connected sink.
>>
>> 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 | 188 ++++++++++++++++++++++++---
>>  1 file changed, 172 insertions(+), 16 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..c482a8e7da25 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.
> 
> I have no real clue about the inner workings of hdmi, but that line
> does make me curious, because no part of the patch authorship does
> mention Amazon ;-) .

That was intentional: although the code was written by Collabora, the copyright
belongs to Amazon.

> Also I think "or it's affiliates" might be way too broad, because this
> can be essentially everyone. So noone in the future will know who to
> ask on copyright questions.
> 
> Similarly when _somebody_ comes forward with "I hold a copyright on this"
> no-one could verify this claim as well.
> 
> I somehow expect _one_ entity being specified, not possibly hundreds.

Hmm, I followed the wording already used elsewhere in the kernel tree:

$ git grep 'Copyright.*Amazon.*affiliates' | wc -l
93

There appears to be only a single instance that omits “affiliates”:

$ git grep 'Copyright.*Amazon' | grep -v affiliates
drivers/power/supply/max14656_charger_detector.c: * Copyright (C) Amazon Technologies Inc. All rights reserved.

Which was added nearly ten years ago, so I’m not sure it makes sense to adopt
that form now.

Actually, this seems to be fairly common among other vendors as well:

$ git grep 'Copyright.*affiliates' | wc -l
1014

Regards,
Cristian

Re: [PATCH v5 05/10] drm/bridge: dw-hdmi-qp: Add HDMI 2.0 SCDC scrambling and high TMDS clock ratio support

[Heiko Stuebner]

Am Montag, 27. April 2026, 18:39:44 Mitteleuropäische Sommerzeit schrieb Cristian Ciocaltea:
> Hello Heiko,
> 
> Thanks for the quick reviews!
> 
> On 4/27/26 1:49 PM, Heiko Stuebner wrote:
> > Am Sonntag, 26. April 2026, 02:20:17 Mitteleuropäische Sommerzeit schrieb Cristian Ciocaltea:
> >> Enable HDMI 2.0 display modes (e.g. 4K@60Hz) by adding SCDC management
> >> for the high TMDS clock ratio and scrambling, required when the TMDS
> >> character rate exceeds the 340 MHz HDMI 1.4b limit.
> >>
> >> A periodic work item monitors the sink's scrambling status to recover
> >> from sink-side resets.  On hotplug detect, if SCDC scrambling state is
> >> out of sync with the driver, trigger a CRTC reset to re-establish the
> >> link.
> >>
> >> Reject modes requiring TMDS rates above 600 MHz, as those fall in the
> >> HDMI 2.1 FRL domain which is not supported. In no_hpd configurations,
> >> further restrict to 340 MHz since SCDC requires a connected sink.
> >>
> >> 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 | 188 ++++++++++++++++++++++++---
> >>  1 file changed, 172 insertions(+), 16 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..c482a8e7da25 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.
> > 
> > I have no real clue about the inner workings of hdmi, but that line
> > does make me curious, because no part of the patch authorship does
> > mention Amazon ;-) .
> 
> That was intentional: although the code was written by Collabora, the copyright
> belongs to Amazon.

They selected a nice chip then :-D


> > Also I think "or it's affiliates" might be way too broad, because this
> > can be essentially everyone. So noone in the future will know who to
> > ask on copyright questions.
> > 
> > Similarly when _somebody_ comes forward with "I hold a copyright on this"
> > no-one could verify this claim as well.
> > 
> > I somehow expect _one_ entity being specified, not possibly hundreds.
> 
> Hmm, I followed the wording already used elsewhere in the kernel tree:
> 
> $ git grep 'Copyright.*Amazon.*affiliates' | wc -l
> 93
> 
> There appears to be only a single instance that omits “affiliates”:
> 
> $ git grep 'Copyright.*Amazon' | grep -v affiliates
> drivers/power/supply/max14656_charger_detector.c: * Copyright (C) Amazon Technologies Inc. All rights reserved.
> 
> Which was added nearly ten years ago, so I’m not sure it makes sense to adopt
> that form now.
> 
> Actually, this seems to be fairly common among other vendors as well:
> 
> $ git grep 'Copyright.*affiliates' | wc -l
> 1014

okay then ... if that is already used a lot, and nobody else complained
then it should be fine I guess :-) .

It just sounded strange to me when reading that earlier.


Thanks a lot for digging those stats up
Heiko

Re: [PATCH v5 05/10] drm/bridge: dw-hdmi-qp: Add HDMI 2.0 SCDC scrambling and high TMDS clock ratio support

[Heiko Stuebner]

Am Sonntag, 26. April 2026, 02:20:17 Mitteleuropäische Sommerzeit schrieb Cristian Ciocaltea:
> Enable HDMI 2.0 display modes (e.g. 4K@60Hz) by adding SCDC management
> for the high TMDS clock ratio and scrambling, required when the TMDS
> character rate exceeds the 340 MHz HDMI 1.4b limit.
> 
> A periodic work item monitors the sink's scrambling status to recover
> from sink-side resets.  On hotplug detect, if SCDC scrambling state is
> out of sync with the driver, trigger a CRTC reset to re-establish the
> link.
> 
> Reject modes requiring TMDS rates above 600 MHz, as those fall in the
> HDMI 2.1 FRL domain which is not supported. In no_hpd configurations,
> further restrict to 340 MHz since SCDC requires a connected sink.
> 
> 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>

The change looks fine to my semi-trained eye, so even with my copyright
concern addressed, I can only provide an

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

Re: [PATCH v5 05/10] drm/bridge: dw-hdmi-qp: Add HDMI 2.0 SCDC scrambling and high TMDS clock ratio support

[Dmitry Baryshkov]

On Sun, Apr 26, 2026 at 03:20:17AM +0300, Cristian Ciocaltea wrote:
> Enable HDMI 2.0 display modes (e.g. 4K@60Hz) by adding SCDC management
> for the high TMDS clock ratio and scrambling, required when the TMDS
> character rate exceeds the 340 MHz HDMI 1.4b limit.
> 
> A periodic work item monitors the sink's scrambling status to recover
> from sink-side resets.  On hotplug detect, if SCDC scrambling state is
> out of sync with the driver, trigger a CRTC reset to re-establish the
> link.
> 
> Reject modes requiring TMDS rates above 600 MHz, as those fall in the
> HDMI 2.1 FRL domain which is not supported. In no_hpd configurations,
> further restrict to 340 MHz since SCDC requires a connected sink.
> 
> 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 | 188 ++++++++++++++++++++++++---
>  1 file changed, 172 insertions(+), 16 deletions(-)

My main issue with this patch (sorry) is that this adds yet another copy
of SCDC-related helpers into the driver which already OP_HDMI and other
helpers.

> @@ -39,7 +42,9 @@
>  #define DDC_SEGMENT_ADDR	0x30
>  
>  #define HDMI14_MAX_TMDSCLK	340000000
> +#define HDMI20_MAX_TMDSRATE	600000000
>  
> +#define SCDC_MAX_SOURCE_VERSION	0x1
>  #define SCRAMB_POLL_DELAY_MS	3000
>  
>  /*
> @@ -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;

s/scramb/scrambler/

Can we move those two to the drm_connector_hdmi

> +
>  	struct regmap *regm;
>  	int main_irq;
>  
> @@ -749,28 +759,124 @@ 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 int dw_hdmi_qp_set_scramb(struct dw_hdmi_qp *hdmi)
> +{
> +	bool done;
> +
> +	dev_dbg(hdmi->dev, "set scrambling\n");
> +
> +	done = drm_scdc_set_high_tmds_clock_ratio(hdmi->curr_conn, true);
> +	if (!done)
> +		return -EIO;
> +
> +	done = drm_scdc_set_scrambling(hdmi->curr_conn, true);
> +	if (!done) {
> +		drm_scdc_set_high_tmds_clock_ratio(hdmi->curr_conn, false);
> +		return -EIO;
> +	}
> +
> +	schedule_delayed_work(&hdmi->scramb_work,
> +			      msecs_to_jiffies(SCRAMB_POLL_DELAY_MS));
> +	return 0;
> +}
> +
> +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 (READ_ONCE(hdmi->scramb_enabled) &&
> +	    !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)
> +{
> +	int ret;
> +	u8 ver;
> +
> +	if (!dw_hdmi_qp_supports_scrambling(&hdmi->curr_conn->display_info))
> +		return;
> +
> +	ret = drm_scdc_readb(hdmi->bridge.ddc, SCDC_SINK_VERSION, &ver);
> +	if (ret) {
> +		dev_err(hdmi->dev, "Failed to read SCDC_SINK_VERSION: %d\n", ret);
> +		return;
> +	}
> +
> +	ret = drm_scdc_writeb(hdmi->bridge.ddc, SCDC_SOURCE_VERSION,
> +			      min_t(u8, ver, SCDC_MAX_SOURCE_VERSION));
> +	if (ret) {
> +		dev_err(hdmi->dev, "Failed to write SCDC_SOURCE_VERSION: %d\n", ret);
> +		return;
> +	}
> +
> +	WRITE_ONCE(hdmi->scramb_enabled, true);
> +
> +	ret = dw_hdmi_qp_set_scramb(hdmi);
> +	if (ret) {
> +		hdmi->scramb_enabled = false;
> +		return;
> +	}
> +
> +	dw_hdmi_qp_write(hdmi, 1, SCRAMB_CONFIG0);
> +
> +	/* 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");
> +
> +	WRITE_ONCE(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);
> +	}
> +}

All of these feel like generic helpers.

> +
>  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 +887,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,14 +897,49 @@ 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;

Also please check the high TMDS clock ration bit.

> +
> +	drm_atomic_helper_connector_hdmi_hotplug(connector,
> +						 connector_status_connected);

I don't see a forced hotplug event in the existing drivers. Why is it
necessary? This function is being called from the detect() path.

> +	/*
> +	 * 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");
> +
> +	return drm_bridge_helper_reset_crtc(&hdmi->bridge, ctx);
> +}
> +
> +static int dw_hdmi_qp_bridge_detect_ctx(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;
> +	int ret;
>  
>  	if (hdmi->no_hpd) {
>  		drm_edid = drm_edid_read_ddc(connector, bridge->ddc);
> @@ -808,7 +949,20 @@ 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) {
> +		ret = dw_hdmi_qp_reset_crtc(hdmi, connector, ctx);
> +		if (ret == -EDEADLK)
> +			return ret;
> +		if (ret < 0)
> +			status = connector_status_unknown;

Ideally this should go to the drm_atomic_helper_connector_hdmi_update().
And once it goes, I don't think we'd need the detect_ctx() callback for
bridges.

> +	}
> +
> +	return status;
>  }
>  
>  static const struct drm_edid *
> @@ -832,12 +986,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 +1351,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_ctx,
>  	.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 +1441,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.53.0
> 

-- 
With best wishes
Dmitry

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

[Claude Code Review Bot]

Patch Review

This is the main feature patch and the most complex. Several aspects to examine:

**Struct additions:**
```c
	struct drm_connector *curr_conn;
	struct delayed_work scramb_work;
	bool scramb_enabled;
```

**Concurrency concern with `curr_conn`:** The `curr_conn` pointer is set in `atomic_enable` and cleared in `atomic_disable`. It's used without locking in the delayed work item (`dw_hdmi_qp_scramb_work`) and in `detect_ctx`. There's a potential race:

1. `scramb_work` fires and reads `hdmi->curr_conn`
2. Concurrently, `atomic_disable` runs and sets `hdmi->curr_conn = NULL`
3. The work dereferences a NULL or stale pointer

The `cancel_delayed_work_sync()` in `dw_hdmi_qp_disable_scramb()` should prevent this because it's called before `hdmi->curr_conn = NULL`:

```c
	WRITE_ONCE(hdmi->scramb_enabled, false);
	cancel_delayed_work_sync(&hdmi->scramb_work);
	...
	// (in atomic_disable, after disable_scramb returns)
	hdmi->curr_conn = NULL;
```

The `WRITE_ONCE(false)` + `cancel_delayed_work_sync()` ensures the work won't re-schedule and any in-flight execution completes before `curr_conn` is cleared. This looks correct.

**However**, `curr_conn` is also accessed in `detect_ctx` which runs from the connector detect path (HPD work). There's no synchronization between `detect_ctx` reading `hdmi->scramb_enabled` / `hdmi->curr_conn` and `atomic_disable` clearing them. If a detect fires while a modeset is tearing down, `detect_ctx` could see `scramb_enabled == true` but `curr_conn` could be in-flight being set to NULL. In practice, the connection_mutex should serialize these paths, but it might be worth adding a comment or a `READ_ONCE` for `curr_conn` reads as well, similar to what was done for `scramb_enabled`.

**SCDC rollback logic in `dw_hdmi_qp_set_scramb`:**
```c
	done = drm_scdc_set_high_tmds_clock_ratio(hdmi->curr_conn, true);
	if (!done)
		return -EIO;

	done = drm_scdc_set_scrambling(hdmi->curr_conn, true);
	if (!done) {
		drm_scdc_set_high_tmds_clock_ratio(hdmi->curr_conn, false);
		return -EIO;
	}
```
Good — if scrambling fails, the high TMDS clock ratio is rolled back. The v5 changelog mentions this was added in response to review feedback.

**In `dw_hdmi_qp_enable_scramb`:**
```c
	WRITE_ONCE(hdmi->scramb_enabled, true);

	ret = dw_hdmi_qp_set_scramb(hdmi);
	if (ret) {
		hdmi->scramb_enabled = false;
		return;
	}
```

Minor: The set uses `WRITE_ONCE` but the reset on failure uses a plain store. Since the work item reads with `READ_ONCE`, the reset should also use `WRITE_ONCE` for consistency. Though in practice, at this point the work hasn't been scheduled yet (it's scheduled inside `set_scramb` which failed), so it's not a live bug.

**`dw_hdmi_qp_reset_crtc` in detect_ctx:**
```c
	if (!!(config & SCDC_SCRAMBLING_ENABLE) == hdmi->scramb_enabled)
		return 0;
```
This compares the sink's current SCDC scrambling state with the driver's expectation. If they mismatch (e.g., sink was power-cycled), trigger a CRTC reset. This is the key logic for recovering from sink-side resets.

**`dw_hdmi_qp_bridge_tmds_char_rate_valid` changes:**
```c
	if (hdmi->no_hpd && rate > HDMI14_MAX_TMDSCLK) {
		...
		return MODE_CLOCK_HIGH;
	}

	if (rate > HDMI20_MAX_TMDSRATE) {
		...
		return MODE_CLOCK_HIGH;
	}
```
This correctly resolves the TODO that was in the original code: no_hpd mode is limited to 340 MHz (no SCDC without a connected sink), and the overall cap is 600 MHz (HDMI 2.0 TMDS limit, above which FRL is needed).

**`INIT_DELAYED_WORK` placement:** It's initialized in `dw_hdmi_qp_bind()` before `hdmi->dev` is assigned:
```c
	INIT_DELAYED_WORK(&hdmi->scramb_work, dw_hdmi_qp_scramb_work);

	hdmi->dev = dev;
```
This is fine — `INIT_DELAYED_WORK` only sets up the work struct metadata and the callback pointer, it doesn't use `hdmi->dev`.

**Overall for patch 5:** The design is solid. The main nit is the inconsistent `WRITE_ONCE` on the `scramb_enabled = false` failure path, and it would be good to have a brief comment about the locking assumptions for `curr_conn` access in `detect_ctx` vs `atomic_disable`.

---

---
Generated by Claude Code Patch Reviewer

[PATCH v5 06/10] drm/bridge: dw-hdmi-qp: Rate limit i2c read error messages

[Cristian Ciocaltea]

During EDID reads, repeated i2c errors can flood the kernel log:

[   25.361716] dwhdmiqp-rockchip fde80000.hdmi: i2c read error
[   25.363376] dwhdmiqp-rockchip fde80000.hdmi: i2c read error
...
[   25.368671] dwhdmiqp-rockchip fde80000.hdmi: i2c read error
[   25.369440] dwhdmiqp-rockchip fde80000.hdmi: failed to get edid

Switch to dev_err_ratelimited() in dw_hdmi_qp_i2c_read() to reduce log
spam while still reporting the condition.

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

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
index c482a8e7da25..36fbd3831bc4 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
@@ -573,7 +573,7 @@ static int dw_hdmi_qp_i2c_read(struct dw_hdmi_qp *hdmi,
 				dev_dbg_ratelimited(hdmi->dev,
 						    "i2c read timed out\n");
 			else
-				dev_err(hdmi->dev, "i2c read timed out\n");
+				dev_err_ratelimited(hdmi->dev, "i2c read timed out\n");
 			dw_hdmi_qp_write(hdmi, 0x01, I2CM_CONTROL0);
 			return -EAGAIN;
 		}
@@ -584,7 +584,7 @@ static int dw_hdmi_qp_i2c_read(struct dw_hdmi_qp *hdmi,
 				dev_dbg_ratelimited(hdmi->dev,
 						    "i2c read error\n");
 			else
-				dev_err(hdmi->dev, "i2c read error\n");
+				dev_err_ratelimited(hdmi->dev, "i2c read error\n");
 			dw_hdmi_qp_write(hdmi, 0x01, I2CM_CONTROL0);
 			return -EIO;
 		}

-- 
2.53.0

Re: [PATCH v5 06/10] drm/bridge: dw-hdmi-qp: Rate limit i2c read error messages

[Heiko Stuebner]

Am Sonntag, 26. April 2026, 02:20:18 Mitteleuropäische Sommerzeit schrieb Cristian Ciocaltea:
> During EDID reads, repeated i2c errors can flood the kernel log:
> 
> [   25.361716] dwhdmiqp-rockchip fde80000.hdmi: i2c read error
> [   25.363376] dwhdmiqp-rockchip fde80000.hdmi: i2c read error
> ...
> [   25.368671] dwhdmiqp-rockchip fde80000.hdmi: i2c read error
> [   25.369440] dwhdmiqp-rockchip fde80000.hdmi: failed to get edid
> 
> Switch to dev_err_ratelimited() in dw_hdmi_qp_i2c_read() to reduce log
> spam while still reporting the condition.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

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

Claude review: drm/bridge: dw-hdmi-qp: Rate limit i2c read error messages

[Claude Code Review Bot]

Patch Review

```c
-			dev_err(hdmi->dev, "i2c read timed out\n");
+			dev_err_ratelimited(hdmi->dev, "i2c read timed out\n");
```

Straightforward and well-motivated — the log snippet in the commit message shows the spam. Applied to both the timeout and NACK error paths. Both already had `dev_dbg_ratelimited` for the `no_hpd` case, so this makes the non-no_hpd path consistent.

**No issues.**

---

---
Generated by Claude Code Patch Reviewer

[PATCH v5 07/10] drm/rockchip: dw_hdmi_qp: Add missing newlines in dev_err_probe() messages

[Cristian Ciocaltea]

Add the missing trailing newlines to a couple of dev_err_probe() calls
in dw_hdmi_qp_rockchip_bind().

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

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
index c78db7f8ab6c..d9333ad8996b 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
@@ -589,14 +589,14 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 	drm_encoder_helper_add(encoder, &dw_hdmi_qp_rockchip_encoder_helper_funcs);
 	ret = drmm_encoder_init(drm, encoder, NULL, DRM_MODE_ENCODER_TMDS, NULL);
 	if (ret)
-		return dev_err_probe(hdmi->dev, ret, "Failed to init encoder");
+		return dev_err_probe(hdmi->dev, ret, "Failed to init encoder\n");
 
 	platform_set_drvdata(pdev, hdmi);
 
 	hdmi->hdmi = dw_hdmi_qp_bind(pdev, encoder, &plat_data);
 	if (IS_ERR(hdmi->hdmi))
 		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->hdmi),
-				     "Failed to bind dw-hdmi-qp");
+				     "Failed to bind dw-hdmi-qp\n");
 
 	connector = drm_bridge_connector_init(drm, encoder);
 	if (IS_ERR(connector))

-- 
2.53.0

Re: [PATCH v5 07/10] drm/rockchip: dw_hdmi_qp: Add missing newlines in dev_err_probe() messages

[Heiko Stuebner]

Am Sonntag, 26. April 2026, 02:20:19 Mitteleuropäische Sommerzeit schrieb Cristian Ciocaltea:
> Add the missing trailing newlines to a couple of dev_err_probe() calls
> in dw_hdmi_qp_rockchip_bind().
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

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

Re: [PATCH v5 07/10] drm/rockchip: dw_hdmi_qp: Add missing newlines in dev_err_probe() messages

[Dmitry Baryshkov]

On Sun, Apr 26, 2026 at 03:20:19AM +0300, Cristian Ciocaltea wrote:
> Add the missing trailing newlines to a couple of dev_err_probe() calls
> in dw_hdmi_qp_rockchip_bind().
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Missing Fixes tag.

> 
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> index c78db7f8ab6c..d9333ad8996b 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> @@ -589,14 +589,14 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>  	drm_encoder_helper_add(encoder, &dw_hdmi_qp_rockchip_encoder_helper_funcs);
>  	ret = drmm_encoder_init(drm, encoder, NULL, DRM_MODE_ENCODER_TMDS, NULL);
>  	if (ret)
> -		return dev_err_probe(hdmi->dev, ret, "Failed to init encoder");
> +		return dev_err_probe(hdmi->dev, ret, "Failed to init encoder\n");
>  
>  	platform_set_drvdata(pdev, hdmi);
>  
>  	hdmi->hdmi = dw_hdmi_qp_bind(pdev, encoder, &plat_data);
>  	if (IS_ERR(hdmi->hdmi))
>  		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->hdmi),
> -				     "Failed to bind dw-hdmi-qp");
> +				     "Failed to bind dw-hdmi-qp\n");
>  
>  	connector = drm_bridge_connector_init(drm, encoder);
>  	if (IS_ERR(connector))
> 
> -- 
> 2.53.0
> 

-- 
With best wishes
Dmitry

Claude review: drm/rockchip: dw_hdmi_qp: Add missing newlines in dev_err_probe() messages

[Claude Code Review Bot]

Patch Review

```c
-		return dev_err_probe(hdmi->dev, ret, "Failed to init encoder");
+		return dev_err_probe(hdmi->dev, ret, "Failed to init encoder\n");
```

Trivial fix for two `dev_err_probe()` calls missing trailing `\n`. Note that `dev_err_probe()` does not automatically add newlines (unlike some other kernel logging APIs), so this is correct.

**No issues.**

---

---
Generated by Claude Code Patch Reviewer

[PATCH v5 08/10] drm/rockchip: dw_hdmi_qp: Use local dev variable consistently in bind()

[Cristian Ciocaltea]

Replace indirect struct device accesses via hdmi->dev and pdev->dev with
the local dev parameter already available in dw_hdmi_qp_rockchip_bind(),
for consistency and readability.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 33 +++++++++++++-------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
index d9333ad8996b..618d2aaa5c7d 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
@@ -475,7 +475,7 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 	struct clk *ref_clk;
 	int ret, irq, i;
 
-	if (!pdev->dev.of_node)
+	if (!dev->of_node)
 		return -ENODEV;
 
 	hdmi = drmm_kzalloc(drm, sizeof(*hdmi), GFP_KERNEL);
@@ -495,7 +495,7 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 		return dev_err_probe(dev, -ENODEV, "Missing platform ctrl ops\n");
 
 	hdmi->ctrl_ops = cfg->ctrl_ops;
-	hdmi->dev = &pdev->dev;
+	hdmi->dev = dev;
 	hdmi->port_id = -ENODEV;
 
 	/* Identify port ID by matching base IO address */
@@ -506,7 +506,7 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 		}
 	}
 	if (hdmi->port_id < 0)
-		return dev_err_probe(hdmi->dev, hdmi->port_id,
+		return dev_err_probe(dev, hdmi->port_id,
 				     "Failed to match HDMI port ID\n");
 
 	plat_data.phy_ops = cfg->phy_ops;
@@ -530,37 +530,36 @@ 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))
-		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->regmap),
+		return dev_err_probe(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))
-		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->vo_regmap),
+		return dev_err_probe(dev, PTR_ERR(hdmi->vo_regmap),
 				     "Unable to get rockchip,vo-grf\n");
 
-	ret = devm_clk_bulk_get_all_enabled(hdmi->dev, &clks);
+	ret = devm_clk_bulk_get_all_enabled(dev, &clks);
 	if (ret < 0)
-		return dev_err_probe(hdmi->dev, ret, "Failed to get clocks\n");
+		return dev_err_probe(dev, ret, "Failed to get clocks\n");
 
-	ref_clk = clk_get(hdmi->dev, "ref");
+	ref_clk = clk_get(dev, "ref");
 	if (IS_ERR(ref_clk))
-		return dev_err_probe(hdmi->dev, PTR_ERR(ref_clk),
+		return dev_err_probe(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->frl_enable_gpio = devm_gpiod_get_optional(hdmi->dev, "frl-enable",
+	hdmi->frl_enable_gpio = devm_gpiod_get_optional(dev, "frl-enable",
 							GPIOD_OUT_LOW);
 	if (IS_ERR(hdmi->frl_enable_gpio))
-		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->frl_enable_gpio),
+		return dev_err_probe(dev, PTR_ERR(hdmi->frl_enable_gpio),
 				     "Failed to request FRL enable GPIO\n");
 
 	hdmi->phy = devm_of_phy_get_by_index(dev, dev->of_node, 0);
 	if (IS_ERR(hdmi->phy))
-		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->phy),
-				     "Failed to get phy\n");
+		return dev_err_probe(dev, PTR_ERR(hdmi->phy), "Failed to get phy\n");
 
 	cfg->ctrl_ops->io_init(hdmi);
 
@@ -578,7 +577,7 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 	if (irq < 0)
 		return irq;
 
-	ret = devm_request_threaded_irq(hdmi->dev, irq,
+	ret = devm_request_threaded_irq(dev, irq,
 					cfg->ctrl_ops->hardirq_callback,
 					cfg->ctrl_ops->irq_callback,
 					IRQF_SHARED, "dw-hdmi-qp-hpd",
@@ -589,18 +588,18 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 	drm_encoder_helper_add(encoder, &dw_hdmi_qp_rockchip_encoder_helper_funcs);
 	ret = drmm_encoder_init(drm, encoder, NULL, DRM_MODE_ENCODER_TMDS, NULL);
 	if (ret)
-		return dev_err_probe(hdmi->dev, ret, "Failed to init encoder\n");
+		return dev_err_probe(dev, ret, "Failed to init encoder\n");
 
 	platform_set_drvdata(pdev, hdmi);
 
 	hdmi->hdmi = dw_hdmi_qp_bind(pdev, encoder, &plat_data);
 	if (IS_ERR(hdmi->hdmi))
-		return dev_err_probe(hdmi->dev, PTR_ERR(hdmi->hdmi),
+		return dev_err_probe(dev, PTR_ERR(hdmi->hdmi),
 				     "Failed to bind dw-hdmi-qp\n");
 
 	connector = drm_bridge_connector_init(drm, encoder);
 	if (IS_ERR(connector))
-		return dev_err_probe(hdmi->dev, PTR_ERR(connector),
+		return dev_err_probe(dev, PTR_ERR(connector),
 				     "Failed to init bridge connector\n");
 
 	return drm_connector_attach_encoder(connector, encoder);

-- 
2.53.0

Re: [PATCH v5 08/10] drm/rockchip: dw_hdmi_qp: Use local dev variable consistently in bind()

[Heiko Stuebner]

Am Sonntag, 26. April 2026, 02:20:20 Mitteleuropäische Sommerzeit schrieb Cristian Ciocaltea:
> Replace indirect struct device accesses via hdmi->dev and pdev->dev with
> the local dev parameter already available in dw_hdmi_qp_rockchip_bind(),
> for consistency and readability.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

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

Claude review: drm/rockchip: dw_hdmi_qp: Use local dev variable consistently in bind()

[Claude Code Review Bot]

Patch Review

This replaces `hdmi->dev`, `pdev->dev`, etc. with the `dev` parameter already available in `dw_hdmi_qp_rockchip_bind()`. All three expressions (`dev`, `hdmi->dev`, `&pdev->dev`) refer to the same `struct device`, so the change is purely cosmetic. The ordering is correct — `hdmi->dev` is set to `dev` earlier in the function, so they're interchangeable.

**No issues. Clean mechanical cleanup.**

---

---
Generated by Claude Code Patch Reviewer

[PATCH v5 09/10] drm/rockchip: dw_hdmi_qp: Register HPD IRQ after connector setup

[Cristian Ciocaltea]

Move devm_request_threaded_irq() to the end of bind(), after
drm_bridge_connector_init() and drm_connector_attach_encoder(), to
ensure all DRM resources are ready before HPD interrupts can fire.

While at it, add error handling for drm_connector_attach_encoder().

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

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
index 618d2aaa5c7d..fbbe26f8730c 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
@@ -577,14 +577,6 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 	if (irq < 0)
 		return irq;
 
-	ret = devm_request_threaded_irq(dev, irq,
-					cfg->ctrl_ops->hardirq_callback,
-					cfg->ctrl_ops->irq_callback,
-					IRQF_SHARED, "dw-hdmi-qp-hpd",
-					hdmi);
-	if (ret)
-		return ret;
-
 	drm_encoder_helper_add(encoder, &dw_hdmi_qp_rockchip_encoder_helper_funcs);
 	ret = drmm_encoder_init(drm, encoder, NULL, DRM_MODE_ENCODER_TMDS, NULL);
 	if (ret)
@@ -602,7 +594,15 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 		return dev_err_probe(dev, PTR_ERR(connector),
 				     "Failed to init bridge connector\n");
 
-	return drm_connector_attach_encoder(connector, encoder);
+	ret = drm_connector_attach_encoder(connector, encoder);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to attach connector\n");
+
+	return devm_request_threaded_irq(dev, 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.53.0

Re: [PATCH v5 09/10] drm/rockchip: dw_hdmi_qp: Register HPD IRQ after connector setup

[Dmitry Baryshkov]

On Sun, Apr 26, 2026 at 03:20:21AM +0300, Cristian Ciocaltea wrote:
> Move devm_request_threaded_irq() to the end of bind(), after
> drm_bridge_connector_init() and drm_connector_attach_encoder(), to
> ensure all DRM resources are ready before HPD interrupts can fire.
> 
> While at it, add error handling for drm_connector_attach_encoder().
> 
> Tested-by: Diederik de Haas <diederik@cknow-tech.com>
> Tested-by: Maud Spierings <maud_spierings@hotmail.com>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> index 618d2aaa5c7d..fbbe26f8730c 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> @@ -577,14 +577,6 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>  	if (irq < 0)
>  		return irq;
>  
> -	ret = devm_request_threaded_irq(dev, irq,
> -					cfg->ctrl_ops->hardirq_callback,
> -					cfg->ctrl_ops->irq_callback,
> -					IRQF_SHARED, "dw-hdmi-qp-hpd",
> -					hdmi);
> -	if (ret)
> -		return ret;
> -
>  	drm_encoder_helper_add(encoder, &dw_hdmi_qp_rockchip_encoder_helper_funcs);
>  	ret = drmm_encoder_init(drm, encoder, NULL, DRM_MODE_ENCODER_TMDS, NULL);
>  	if (ret)
> @@ -602,7 +594,15 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>  		return dev_err_probe(dev, PTR_ERR(connector),
>  				     "Failed to init bridge connector\n");
>  
> -	return drm_connector_attach_encoder(connector, encoder);
> +	ret = drm_connector_attach_encoder(connector, encoder);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to attach connector\n");
> +
> +	return devm_request_threaded_irq(dev, irq,
> +					 cfg->ctrl_ops->hardirq_callback,
> +					 cfg->ctrl_ops->irq_callback,
> +					 IRQF_SHARED, "dw-hdmi-qp-hpd",
> +					 hdmi);

And if requesting of the IRQ fails, you have a lot to unwind. It might
be better to request IRQ early enough with the IRQF_NO_AUTOEN flag and
then enable_irq() here.

>  }
>  
>  static void dw_hdmi_qp_rockchip_unbind(struct device *dev,
> 
> -- 
> 2.53.0
> 
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

-- 
With best wishes
Dmitry

Claude review: drm/rockchip: dw_hdmi_qp: Register HPD IRQ after connector setup

[Claude Code Review Bot]

Patch Review

Moves `devm_request_threaded_irq()` to after `drm_bridge_connector_init()` and `drm_connector_attach_encoder()`. This prevents HPD interrupts from firing before the DRM connector is fully wired up.

Also adds error handling for `drm_connector_attach_encoder()` which was previously unchecked (the return value was just passed through as the function's return, but without an error message):

```c
+	ret = drm_connector_attach_encoder(connector, encoder);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to attach connector\n");
```

The IRQ is now the last thing registered, which means it will be the first thing torn down by `devm` on unbind — correct ordering.

Has R-b from Heiko. **No issues.**

---

---
Generated by Claude Code Patch Reviewer

[PATCH v5 10/10] drm/rockchip: dw_hdmi_qp: Restrict HPD event to the affected connector

[Cristian Ciocaltea]

Switch from drm_helper_hpd_irq_event(), which polls all connectors, to
drm_connector_helper_hpd_irq_event(), which runs the detect cycle only
on the affected connector.

This avoids unnecessary work and redundant detect calls on unrelated
connectors.

Tested-by: Diederik de Haas <diederik@cknow-tech.com>
Tested-by: Maud Spierings <maud_spierings@hotmail.com>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
index fbbe26f8730c..d6338195a5b7 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
@@ -93,6 +93,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;
@@ -252,11 +253,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;
 
-	if (drm) {
-		changed = drm_helper_hpd_irq_event(drm);
+	if (hdmi->connector) {
+		changed = drm_connector_helper_hpd_irq_event(hdmi->connector);
 		if (changed)
 			dev_dbg(hdmi->dev, "connector status changed\n");
 	}
@@ -467,7 +467,6 @@ 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;
@@ -589,12 +588,12 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 		return dev_err_probe(dev, PTR_ERR(hdmi->hdmi),
 				     "Failed to bind dw-hdmi-qp\n");
 
-	connector = drm_bridge_connector_init(drm, encoder);
-	if (IS_ERR(connector))
-		return dev_err_probe(dev, PTR_ERR(connector),
+	hdmi->connector = drm_bridge_connector_init(drm, encoder);
+	if (IS_ERR(hdmi->connector))
+		return dev_err_probe(dev, PTR_ERR(hdmi->connector),
 				     "Failed to init bridge connector\n");
 
-	ret = drm_connector_attach_encoder(connector, encoder);
+	ret = drm_connector_attach_encoder(hdmi->connector, encoder);
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to attach connector\n");
 
@@ -612,6 +611,8 @@ static void dw_hdmi_qp_rockchip_unbind(struct device *dev,
 	struct rockchip_hdmi_qp *hdmi = dev_get_drvdata(dev);
 
 	cancel_delayed_work_sync(&hdmi->hpd_work);
+
+	hdmi->connector = NULL;
 }
 
 static const struct component_ops dw_hdmi_qp_rockchip_ops = {
@@ -646,8 +647,8 @@ static int __maybe_unused dw_hdmi_qp_rockchip_resume(struct device *dev)
 
 	dw_hdmi_qp_resume(dev, hdmi->hdmi);
 
-	if (hdmi->encoder.encoder.dev)
-		drm_helper_hpd_irq_event(hdmi->encoder.encoder.dev);
+	if (hdmi->connector)
+		drm_connector_helper_hpd_irq_event(hdmi->connector);
 
 	return 0;
 }

-- 
2.53.0

Re: [PATCH v5 10/10] drm/rockchip: dw_hdmi_qp: Restrict HPD event to the affected connector

[Dmitry Baryshkov]

On Sun, Apr 26, 2026 at 03:20:22AM +0300, Cristian Ciocaltea wrote:
> Switch from drm_helper_hpd_irq_event(), which polls all connectors, to
> drm_connector_helper_hpd_irq_event(), which runs the detect cycle only
> on the affected connector.
> 
> This avoids unnecessary work and redundant detect calls on unrelated
> connectors.

This also doesn't feel correct. The dw_hdmi_qp is a bridge. As such it
should be using drm_bridge_hpd_notify() rather than any of the mentioned
functions. Would it be better to add the a corresponding wrapper to the
dw_hdmi_qp API and call one there? Also consider implementing the
.hpd_enable() and .hpd_disable() callbacks for the bridge, which would
remove the necessity to call enable the IRQ in the bind function.

> 
> Tested-by: Diederik de Haas <diederik@cknow-tech.com>
> Tested-by: Maud Spierings <maud_spierings@hotmail.com>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 

-- 
With best wishes
Dmitry

Claude review: drm/rockchip: dw_hdmi_qp: Restrict HPD event to the affected connector

[Claude Code Review Bot]

Patch Review

Switches from `drm_helper_hpd_irq_event()` (polls all connectors) to `drm_connector_helper_hpd_irq_event()` (targets a specific connector). Adds `struct drm_connector *connector` to `rockchip_hdmi_qp` to track the relevant connector.

```c
-	if (drm) {
-		changed = drm_helper_hpd_irq_event(drm);
+	if (hdmi->connector) {
+		changed = drm_connector_helper_hpd_irq_event(hdmi->connector);
```

The `hdmi->connector` is set in `bind()` (after patch 9 ensures the connector is fully initialized) and cleared in `unbind()`. The NULL check before use in the HPD work and resume path is correct.

The `connector = NULL` in unbind runs after `cancel_delayed_work_sync(&hdmi->hpd_work)`, so no racing work item will see a stale pointer.

Has R-b from Heiko. **No issues.**

---

## SUMMARY

The series is well-organized and at v5 has addressed most review feedback. The only substantive point I'd raise:

1. **Patch 5:** The `hdmi->scramb_enabled = false` on the failure path in `dw_hdmi_qp_enable_scramb()` should use `WRITE_ONCE` for consistency with the `READ_ONCE` in the scramb_work, even though it's not a live race at that point.

2. **Patch 5:** Consider documenting the locking assumption that protects `curr_conn` from concurrent access between `detect_ctx` (which reads it) and `atomic_disable` (which clears it). The connection_mutex should serialize these, but an explicit comment would help future readers.

These are minor nits; the series looks good for merge.

---
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: 26
Reviewed: 2026-04-28T15:25:19.150012

---

This is a well-structured v5 series by Cristian Ciocaltea that adds HDMI 2.0 support (4K@60Hz) to the Synopsys DW HDMI QP TX controller on Rockchip RK3576/RK3588 SoCs. The core contribution is SCDC scrambling and high TMDS clock ratio management for rates above 340 MHz.

The series is logically ordered: patches 1-4 introduce the infrastructure (atomic-aware `detect_ctx` bridge hook), patch 5 implements the SCDC feature, patches 6-8 are minor cleanups, and patches 9-10 improve HPD handling. The series has been through 5 revisions with good review feedback incorporated, including tested-by tags from two testers and R-b from Heiko on the later patches.

The overall design is sound. Key concerns are around the concurrency model for `curr_conn` and `scramb_enabled` in patch 5 (discussed below), and a minor behavioral subtlety in the `detect_ctx` fallback path.

**Overall: Good shape. A few points to discuss on patch 5's concurrency, but nothing blocking.**

---

---
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>

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-25T07:36:46.176176

---

This is a well-structured 4-patch series adding HDMI 2.0 support (high TMDS clock ratio and scrambling) to the Synopsys DW HDMI QP TX controller, primarily targeting RK3576/RK3588. The approach is sound: introduce an atomic `detect_ctx` bridge callback (patch 1), wire it through bridge-connector (patch 2), use it in dw-hdmi-qp for SCDC management with CRTC reset on scrambling state mismatch (patch 3), and optimize HPD to per-connector events (patch 4).

The series is at v4/RESEND, has Tested-by tags from two testers, and the code is generally clean. A few issues worth addressing:

---
Generated by Claude Code Patch Reviewer

[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

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