diff mbox series

[v10,23/69] drm/display: hdmi-state-helper: Set HDMI scrambling requirement

Message ID 20260731-dw-hdmi-qp-scramb-v10-23-294364b2cf15@collabora.com (mailing list archive)
State New
Headers
Series Add HDMI 2.0 support to DW HDMI QP TX |

Commit Message

Cristian Ciocaltea July 31, 2026, 4:19 p.m. UTC 
Set drm_connector_hdmi_state.scrambler_needed when the computed TMDS
character rate exceeds the HDMI 1.3 maximum TMDS character rate.

HDMI 2.0 requires scrambling above 340 MHz.  Centralize that clock-based
requirement in the HDMI state helper so drivers do not need to open-code
the threshold check.

Acked-by: Maxime Ripard <mripard@kernel.org>
Tested-by: Maud Spierings <maud_spierings@hotmail.com>
Tested-by: Diederik de Haas <diederik@cknow-tech.com>  # NanoPC-T6 LTS, Rock 5B
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/display/drm_hdmi_state_helper.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

sashiko-bot@kernel.org July 31, 2026, 7:20 p.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The HDMI atomic check phase unconditionally sets `scrambler_needed` for TMDS character rates > 340 MHz without verifying if the source and sink actually support scrambling, which can cause an ungraceful failure during the atomic commit phase.
--

--- Patch [23]: [PATCH v10 23/69] drm/display: hdmi-state-helper: Set HDMI scrambling requirement ---
commit e6622cfc80adce83ef4209e099003b9cb90bec11
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

drm/display: hdmi-state-helper: Set HDMI scrambling requirement

This commit centralizes the HDMI scrambling requirement check in the HDMI state
helper. It sets drm_connector_hdmi_state.scrambler_needed to true when the
computed TMDS character rate exceeds the HDMI 1.3 maximum limit.

> diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> index 3377ea9361209..9544eb8659aa8 100644
> --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> @@ -594,6 +594,9 @@ hdmi_compute_clock(const struct drm_connector *connector,
>  
>  	conn_state->hdmi.tmds_char_rate = clock;
>  
> +	/* TODO: also check drm_display_info.hdmi.scdc.scrambling.low_rates */
> +	conn_state->hdmi.scrambler_needed = (clock > HDMI_1_3_TMDS_CHAR_RATE_MAX_HZ);
> +

[Severity: High]
Does this code allow an atomic_check to succeed even when the source or sink
does not actually support scrambling?

In the KMS atomic framework, hardware capabilities must be validated during the
atomic_check phase to prevent failures during atomic_commit. Here, we set
scrambler_needed to true for clocks > 340 MHz, but we don't seem to verify if
the sink or the source hardware actually supports scrambling.

If a malformed EDID advertises a high TMDS clock but lacks SCDC scrambling
support, or if a driver supports high clocks but fails to implement the
scrambler hooks, the atomic check phase will pass. Later, when
drm_connector_hdmi_enable_scrambling() is called during the commit phase, it
will evaluate scrambler_needed and return -EINVAL.

Failing during the atomic commit phase violates KMS rules and results in a
broken display pipeline. Should hdmi_compute_clock() or hdmi_clock_valid()
also verify drm_scdc_sink_supports_scrambling() and
drm_connector_hdmi_scrambler_supported() before succeeding?

>  	return 0;
>  }
diff mbox series

Patch

diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
index 3377ea936120..9544eb8659aa 100644
--- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
+++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
@@ -594,6 +594,9 @@  hdmi_compute_clock(const struct drm_connector *connector,
 
 	conn_state->hdmi.tmds_char_rate = clock;
 
+	/* TODO: also check drm_display_info.hdmi.scdc.scrambling.low_rates */
+	conn_state->hdmi.scrambler_needed = (clock > HDMI_1_3_TMDS_CHAR_RATE_MAX_HZ);
+
 	return 0;
 }