diff mbox series

[v11,24/74] drm/display: hdmi-state-helper: Set HDMI scrambling requirement

Message ID 20260901-dw-hdmi-qp-scramb-v11-24-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 
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 Sept. 1, 2026, 7:39 p.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing validation of sink SCDC scrambling capabilities during the atomic check phase when scrambling is required.
--

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

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

This commit sets the HDMI scrambling requirement in the HDMI state helper
when the computed TMDS character rate exceeds the HDMI 1.3 maximum limit.
Centralizing this clock-based requirement prevents drivers from needing
to open-code the threshold check.

> diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> index 1e269b203b987..7cdb7ca3dc12f 100644
> --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> @@ -596,6 +596,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 allow an unsupported configuration to pass the atomic check phase?

When a high TMDS clock is requested, this unconditionally sets
conn_state->hdmi.scrambler_needed to true without validating sink SCDC
capabilities. The configuration can pass hdmi_compute_clock() and
atomic_check because info->max_tmds_clock is permitted to exceed 340MHz
regardless of SCDC support.

Later during the hardware programming phase, it will fail:

drivers/gpu/drm/display/drm_hdmi_helper.c:drm_connector_hdmi_enable_scrambling() {
	...
	if (!drm_scdc_sink_supports_scrambling(connector)) {
		drm_dbg_kms(dev, "Sink doesn't support scrambling.\n");
		return -EINVAL;
	}
	...
}

Failing during atomic_enable violates KMS atomic guarantees. This sequence
can be triggered remotely if an untrusted or malformed EDID advertises a high
TMDS clock via HF-VSDB but clears the SCDC supported bit.

Should this validate drm_scdc_sink_supports_scrambling(connector) during the
atomic check phase and return an error if scrambling is required but SCDC is
unsupported by the sink?

> +
>  	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 1e269b203b98..7cdb7ca3dc12 100644
--- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
+++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
@@ -596,6 +596,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;
 }