[v11,10/74] drm/display: hdmi: Add HDMI 2.0 scrambling management helpers

Message ID 20260901-dw-hdmi-qp-scramb-v11-10-bc12954a0688@collabora.com (mailing list archive)
State New
Headers
Series Add HDMI 2.0 support to DW HDMI QP TX |

Commit Message

Cristian Ciocaltea Sept. 1, 2026, 6:50 p.m. UTC
Add helpers to manage the full lifecycle of HDMI 2.0 SCDC scrambling on
both source and sink sides:

- drm_hdmi_mode_needs_scrambling(): checks whether a mode, bits per
  component and output format combination requires scrambling, which is
  needed when the resulting TMDS character rate exceeds the HDMI 1.3
  maximum of 340 MHz

- drm_connector_hdmi_enable_scrambling(): configures SCDC scrambling and
  high TMDS clock ratio and starts a periodic work item that monitors
  the sink's SCDC scrambling status, retrying setup when the sink loses
  state

- drm_connector_hdmi_disable_scrambling(): tears down scrambling on both
  sides and cancels the monitoring work

- drm_connector_hdmi_sync_scdc(): triggers a CRTC reset on reconnection
  to restore SCDC state lost during sink disconnects within an active
  display pipeline

Tested-by: Maud Spierings <maud_spierings@hotmail.com>
Tested-by: Diederik de Haas <diederik@cknow-tech.com>  # NanoPC-T6 LTS, Rock 5B
Reviewed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/display/drm_hdmi_helper.c | 297 ++++++++++++++++++++++++++++++
 include/drm/display/drm_hdmi_helper.h     |  15 ++
 2 files changed, 312 insertions(+)
  

Comments

sashiko-bot@kernel.org Sept. 1, 2026, 7:18 p.m. UTC | #1
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] HDMI scrambling lifecycle helpers introduce sleeping functions into atomic display paths.
- [Medium] Logic error in SCDC scrambling status monitor: the delayed work item stops scheduling itself, violating the intended periodic monitoring.
--

commit eedf8ebd00133222505fd96065a3aa6dbeefff8d
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

drm/display: hdmi: Add HDMI 2.0 scrambling management helpers

This commit introduces helpers to manage the full lifecycle of HDMI 2.0
SCDC scrambling on both source and sink sides. It includes configuration
of scrambling, high TMDS clock ratios, and a delayed work item to
monitor and restore the sink's scrambling status.

> diff --git a/drivers/gpu/drm/display/drm_hdmi_helper.c b/drivers/gpu/drm/display/drm_hdmi_helper.c
> index 5cb0b033b1713..bf1c7963b984b 100644
> --- a/drivers/gpu/drm/display/drm_hdmi_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdmi_helper.c
[ ... ]
> +static int drm_connector_hdmi_try_scrambling_setup(struct drm_connector *connector)
> +{
> +	bool done;
> +
> +	done = drm_scdc_set_high_tmds_clock_ratio(connector, true);
> +	if (!done)
> +		return -EIO;

[Severity: High]
Since drm_connector_hdmi_enable_scrambling() might be called from atomic
commit paths (like CRTC atomic enable), is it safe to call
drm_scdc_set_high_tmds_clock_ratio() here?

Looking at drm_scdc_set_high_tmds_clock_ratio() in drm_scdc_helper.c, it
uses a sleeping wait:

    usleep_range(1000, 2000);

Could this lead to sleeping in atomic context warnings or deadlocks if
reached during an atomic display update?

[Severity: Medium]
If drm_scdc_set_high_tmds_clock_ratio() or drm_scdc_set_scrambling() fail due
to a transient DDC error, we return -EIO early here and bypass the
schedule_delayed_work() call below.

Does this inadvertently stop the periodic monitoring from retrying the setup
later?

> +
> +	done = drm_scdc_set_scrambling(connector, true);
> +	if (!done)
> +		return -EIO;
> +
> +	if (READ_ONCE(connector->hdmi.scrambler_enabled))
> +		schedule_delayed_work(&connector->hdmi.scdc_work,
> +				      msecs_to_jiffies(DRM_HDMI_SCDC_POLL_DELAY_MS));
> +
> +	return 0;
> +}
> +
> +static void drm_connector_hdmi_scdc_work(struct work_struct *work)
> +{
[ ... ]
> +	if (READ_ONCE(connector->hdmi.scrambler_enabled) &&
> +	    !drm_scdc_get_scrambling_status(connector))
> +		drm_connector_hdmi_try_scrambling_setup(connector);
> +}

[Severity: Medium]
If the sink scrambling status is healthy (drm_scdc_get_scrambling_status()
returns true), this function exits without rescheduling itself.

Does this mean the periodic work item stops monitoring the SCDC status
entirely after the first successful check? The commit message mentions a
periodic work item that monitors the status and retries when lost, but it
appears the loop breaks as soon as the sink is in a good state.

[ ... ]
> +int drm_connector_hdmi_disable_scrambling(struct drm_connector *connector)
> +{
[ ... ]
> +	/*
> +	 * A driver may force scrambler_enabled at init to trigger a disable
> +	 * at boot (e.g. vc4), bypassing the enable path that sets up the work
> +	 * item. Only cancel it once it has actually been initialized.
> +	 */
> +	if (hdmi->scdc_work_initialized)
> +		cancel_delayed_work_sync(&hdmi->scdc_work);

[Severity: High]
If drm_connector_hdmi_disable_scrambling() is invoked from an atomic
context (e.g., during CRTC or encoder atomic disable callbacks), could
calling cancel_delayed_work_sync() cause a sleep in atomic context?

Since cancel_delayed_work_sync() can block waiting for the work to finish,
this might cause system instability if called while holding spinlocks or
from a non-preemptible path.

> +
> +	if (connector->ddc &&
> +	    connector->status == connector_status_connected &&
> +	    drm_scdc_sink_supports_scrambling(connector)) {
> +		drm_scdc_set_scrambling(connector, false);
> +		drm_scdc_set_high_tmds_clock_ratio(connector, false);
> +	}
> +
> +	return hdmi->funcs->scrambler_disable(connector);
> +}
  

Patch

diff --git a/drivers/gpu/drm/display/drm_hdmi_helper.c b/drivers/gpu/drm/display/drm_hdmi_helper.c
index 5cb0b033b171..bf1c7963b984 100644
--- a/drivers/gpu/drm/display/drm_hdmi_helper.c
+++ b/drivers/gpu/drm/display/drm_hdmi_helper.c
@@ -4,12 +4,20 @@ 
 #include <linux/module.h>
 
 #include <drm/display/drm_hdmi_helper.h>
+#include <drm/display/drm_scdc_helper.h>
+
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_connector.h>
+#include <drm/drm_crtc.h>
 #include <drm/drm_edid.h>
 #include <drm/drm_modes.h>
+#include <drm/drm_modeset_lock.h>
 #include <drm/drm_print.h>
 #include <drm/drm_property.h>
 
+#define DRM_HDMI_SCDC_POLL_DELAY_MS	1000
+
 static inline bool is_eotf_supported(u8 output_eotf, u8 sink_eotf)
 {
 	return sink_eotf & BIT(output_eotf);
@@ -426,3 +434,292 @@  drm_hdmi_acr_get_n_cts(unsigned long long tmds_char_rate,
 	*out_cts = cts;
 }
 EXPORT_SYMBOL(drm_hdmi_acr_get_n_cts);
+
+/**
+ * drm_hdmi_mode_needs_scrambling() - Check if an HDMI mode requires scrambling
+ * @mode: Display mode to check
+ * @bpc: Bits per color component
+ * @fmt: Output pixel format
+ *
+ * Computes the TMDS character rate for the given mode, bits per component and
+ * output format, and checks whether it exceeds the HDMI 1.3 maximum TMDS
+ * character rate of 340 MHz.
+ *
+ * Returns:
+ * true if the mode requires scrambling, false otherwise.
+ */
+bool drm_hdmi_mode_needs_scrambling(const struct drm_display_mode *mode,
+				    unsigned int bpc,
+				    enum drm_output_color_format fmt)
+{
+	unsigned long long tmds_char_rate;
+
+	tmds_char_rate = drm_hdmi_compute_mode_clock(mode, bpc, fmt);
+
+	return tmds_char_rate > HDMI_1_3_TMDS_CHAR_RATE_MAX_HZ;
+}
+EXPORT_SYMBOL(drm_hdmi_mode_needs_scrambling);
+
+static bool drm_scdc_sink_supports_scrambling(struct drm_connector *connector)
+{
+	const struct drm_display_info *info = &connector->display_info;
+
+	return info->is_hdmi &&
+	       info->hdmi.scdc.supported &&
+	       info->hdmi.scdc.scrambling.supported;
+}
+
+static int drm_connector_hdmi_try_scrambling_setup(struct drm_connector *connector)
+{
+	bool done;
+
+	done = drm_scdc_set_high_tmds_clock_ratio(connector, true);
+	if (!done)
+		return -EIO;
+
+	done = drm_scdc_set_scrambling(connector, true);
+	if (!done)
+		return -EIO;
+
+	if (READ_ONCE(connector->hdmi.scrambler_enabled))
+		schedule_delayed_work(&connector->hdmi.scdc_work,
+				      msecs_to_jiffies(DRM_HDMI_SCDC_POLL_DELAY_MS));
+
+	return 0;
+}
+
+static void drm_connector_hdmi_scdc_work(struct work_struct *work)
+{
+	struct drm_connector *connector;
+	struct drm_connector_hdmi *hdmi;
+
+	hdmi = container_of(to_delayed_work(work), struct drm_connector_hdmi,
+			    scdc_work);
+	connector = container_of(hdmi, struct drm_connector, hdmi);
+
+	/*
+	 * The connector, and hence this work item, may outlive the source
+	 * driver, e.g. when userspace keeps a DRM file descriptor open past
+	 * unbind. Never poke the DDC bus after the connector got unregistered,
+	 * as the underlying I2C adapter may already be gone.
+	 */
+	if (drm_connector_is_unregistered(connector))
+		return;
+
+	if (READ_ONCE(connector->hdmi.scrambler_enabled) &&
+	    !drm_scdc_get_scrambling_status(connector))
+		drm_connector_hdmi_try_scrambling_setup(connector);
+}
+
+/**
+ * drm_connector_hdmi_enable_scrambling() - enable scrambling and monitor SCDC status
+ * @connector: connector
+ * @conn_state: connector state
+ *
+ * Enables scrambling and high TMDS clock ratio on both source and sink sides.
+ * Additionally, use a delayed work item to monitor the scrambling status on
+ * the sink side and retry the operation, as some displays refuse to set the
+ * scrambling bit right away.
+ *
+ * The work item is cancelled by drm_connector_hdmi_disable_scrambling(), hence
+ * source drivers must ensure the display pipeline is shut down on unbind, e.g.
+ * via drm_atomic_helper_shutdown(). Otherwise the work may outlive the driver's
+ * devres-managed resources, since the connector itself is only cleaned up when
+ * the last reference to the DRM device is dropped.
+ *
+ * Returns:
+ * Zero if scrambling is set successfully, an error code otherwise.
+ */
+int drm_connector_hdmi_enable_scrambling(struct drm_connector *connector,
+					 const struct drm_connector_state *conn_state)
+{
+	struct drm_connector_hdmi *hdmi = &connector->hdmi;
+	struct drm_device *dev = connector->dev;
+	int ret;
+
+	if (!conn_state)
+		return -EINVAL;
+
+	if (!conn_state->hdmi.scrambler_needed)
+		return 0;
+
+	if (!drm_connector_hdmi_scrambler_supported(connector)) {
+		drm_dbg_kms(dev, "Source doesn't support scrambling.\n");
+		return -EINVAL;
+	}
+
+	if (!drm_scdc_sink_supports_scrambling(connector)) {
+		drm_dbg_kms(dev, "Sink doesn't support scrambling.\n");
+		return -EINVAL;
+	}
+
+	if (!connector->ddc)
+		return -EINVAL;
+
+	drm_dbg_kms(dev, "Enabling scrambling\n");
+
+	if (!hdmi->scdc_work_initialized) {
+		INIT_DELAYED_WORK(&hdmi->scdc_work,
+				  drm_connector_hdmi_scdc_work);
+		hdmi->scdc_work_initialized = true;
+	}
+
+	WRITE_ONCE(hdmi->scrambler_enabled, true);
+
+	ret = drm_connector_hdmi_try_scrambling_setup(connector);
+	if (!ret)
+		ret = hdmi->funcs->scrambler_enable(connector);
+
+	if (ret) {
+		WRITE_ONCE(hdmi->scrambler_enabled, false);
+		cancel_delayed_work_sync(&hdmi->scdc_work);
+
+		drm_scdc_set_scrambling(connector, false);
+		drm_scdc_set_high_tmds_clock_ratio(connector, false);
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL(drm_connector_hdmi_enable_scrambling);
+
+/**
+ * drm_connector_hdmi_disable_scrambling() - disable scrambling and SCDC monitoring
+ * @connector: connector
+ *
+ * Disables scrambling and high TMDS clock ratio on both source and sink sides.
+ * Also cancels the SCDC status monitoring work item, if it is still pending.
+ *
+ * Returns:
+ * Zero if scrambling is reset successfully, an error code otherwise.
+ */
+int drm_connector_hdmi_disable_scrambling(struct drm_connector *connector)
+{
+	struct drm_connector_hdmi *hdmi = &connector->hdmi;
+	struct drm_device *dev = connector->dev;
+
+	if (!READ_ONCE(hdmi->scrambler_enabled))
+		return 0;
+
+	drm_dbg_kms(dev, "Disabling scrambling\n");
+
+	WRITE_ONCE(hdmi->scrambler_enabled, false);
+
+	/*
+	 * A driver may force scrambler_enabled at init to trigger a disable
+	 * at boot (e.g. vc4), bypassing the enable path that sets up the work
+	 * item. Only cancel it once it has actually been initialized.
+	 */
+	if (hdmi->scdc_work_initialized)
+		cancel_delayed_work_sync(&hdmi->scdc_work);
+
+	if (connector->ddc &&
+	    connector->status == connector_status_connected &&
+	    drm_scdc_sink_supports_scrambling(connector)) {
+		drm_scdc_set_scrambling(connector, false);
+		drm_scdc_set_high_tmds_clock_ratio(connector, false);
+	}
+
+	return hdmi->funcs->scrambler_disable(connector);
+}
+EXPORT_SYMBOL(drm_connector_hdmi_disable_scrambling);
+
+/**
+ * drm_connector_hdmi_sync_scdc() - resync the sink-side SCDC upon reconnect
+ * @connector: connector
+ * @plugged: connector plugged status event
+ * @ctx: initialized lock acquisition context
+ *
+ * When receiving hotplug disconnect/reconnect event, while the display is
+ * still active (CRTC enabled), the SCDC status on the sink side is reset
+ * and must be explicitly restored.
+ *
+ * The typical solution for this is to trigger an empty modeset in
+ * drm_connector_helper_funcs.detect_ctx(), which is what this helper does
+ * by triggering a CRTC reset on reconnection.
+ *
+ * When making use of the HDMI connector framework, this is automatically
+ * triggered via drm_atomic_helper_connector_hdmi_hotplug().
+ *
+ * Returns:
+ * Zero on success, an error code otherwise, including -EDEADLK.
+ */
+int drm_connector_hdmi_sync_scdc(struct drm_connector *connector, bool plugged,
+				 struct drm_modeset_acquire_ctx *ctx)
+{
+	struct drm_connector_hdmi *hdmi = &connector->hdmi;
+	struct drm_device *dev = connector->dev;
+	struct drm_connector_state *conn_state;
+	struct drm_crtc *crtc;
+	u8 config;
+	int ret;
+
+	if (!connector || !plugged || !ctx)
+		return 0;
+
+	/*
+	 * This is normally part of .detect_ctx() call path, which already holds
+	 * connection_mutex through @ctx.  However, re-acquiring it with the
+	 * same context is a no-op and makes the helper safe under any caller.
+	 */
+	ret = drm_modeset_lock(&connector->dev->mode_config.connection_mutex, ctx);
+	if (ret)
+		return ret;
+
+	/* TODO: Also handle HDMI 2.1 FRL link training */
+
+	if (!drm_connector_hdmi_scrambler_supported(connector) ||
+	    !READ_ONCE(hdmi->scrambler_enabled))
+		return 0;
+
+	conn_state = connector->state;
+	if (!conn_state)
+		return 0;
+
+	crtc = conn_state->crtc;
+	if (!crtc)
+		return 0;
+
+	ret = drm_modeset_lock(&crtc->mutex, ctx);
+	if (ret)
+		return ret;
+
+	if (!crtc->state || !crtc->state->active)
+		return 0;
+
+	if (!connector->ddc || !drm_scdc_sink_supports_scrambling(connector))
+		return 0;
+
+	if (conn_state->commit &&
+	    !try_wait_for_completion(&conn_state->commit->hw_done))
+		return 0;
+
+	ret = drm_scdc_readb(connector->ddc, SCDC_TMDS_CONFIG, &config);
+	if (ret) {
+		drm_dbg_kms(dev, "Failed to read TMDS config: %d\n", ret);
+		return ret;
+	}
+
+	if (config & SCDC_SCRAMBLING_ENABLE)
+		return 0;
+
+	/*
+	 * HDMI 2.0 requires that one should not send scrambled data prior
+	 * to configuring the sink scrambling, and that TMDS clock/data
+	 * transmission should be suspended when changing the TMDS clock rate
+	 * in the sink.  So let's just do a full modeset here, even though some
+	 * sinks would be perfectly happy if were to just reconfigure the SCDC
+	 * settings on the fly.
+	 *
+	 * The disable/re-enable cycle triggered by the reset should call
+	 * drm_connector_hdmi_enable_scrambling() during re-enable, thus
+	 * properly configuring the sink before data transmission resumes.
+	 */
+	drm_dbg_kms(dev, "Resetting CRTC to restore SCDC status\n");
+
+	ret = drm_atomic_helper_reset_crtc(crtc, ctx);
+	if (ret && ret != -EDEADLK)
+		drm_dbg_kms(dev, "Failed to reset CRTC: %d\n", ret);
+
+	return ret;
+}
+EXPORT_SYMBOL(drm_connector_hdmi_sync_scdc);
diff --git a/include/drm/display/drm_hdmi_helper.h b/include/drm/display/drm_hdmi_helper.h
index 9c31ed90516b..2cdd6e3bc090 100644
--- a/include/drm/display/drm_hdmi_helper.h
+++ b/include/drm/display/drm_hdmi_helper.h
@@ -8,6 +8,7 @@ 
 struct drm_connector;
 struct drm_connector_state;
 struct drm_display_mode;
+struct drm_modeset_acquire_ctx;
 enum drm_output_color_format;
 
 void
@@ -35,4 +36,18 @@  drm_hdmi_acr_get_n_cts(unsigned long long tmds_char_rate,
 		       unsigned int *out_n,
 		       unsigned int *out_cts);
 
+bool
+drm_hdmi_mode_needs_scrambling(const struct drm_display_mode *mode,
+			       unsigned int bpc, enum drm_output_color_format fmt);
+
+int
+drm_connector_hdmi_enable_scrambling(struct drm_connector *connector,
+				     const struct drm_connector_state *conn_state);
+int
+drm_connector_hdmi_disable_scrambling(struct drm_connector *connector);
+
+int
+drm_connector_hdmi_sync_scdc(struct drm_connector *connector, bool plugged,
+			     struct drm_modeset_acquire_ctx *ctx);
+
 #endif