diff mbox series

[v9,18/61] drm/display: hdmi-state-helper: Set HDMI scrambling requirement

Message ID 20260723-dw-hdmi-qp-scramb-v9-18-4fb12ea22ac9@collabora.com (mailing list archive)
State New
Headers
Series Add HDMI 2.0 support to DW HDMI QP TX |

Commit Message

Cristian Ciocaltea July 23, 2026, 1:35 a.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>
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 23, 2026, 1:50 a.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The HDMI atomic check framework fails to validate sink scrambling capabilities, allowing unsupported high-clock modes to pass atomic check and fail later during the commit phase.
--

--- Patch [18]: [PATCH v9 18/61] drm/display: hdmi-state-helper: Set HDMI scrambling requirement ---
commit 7fcaa4103f10e7da80a91d4e2de65346b900fcb1
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

drm/display: hdmi-state-helper: Set HDMI scrambling requirement
    
This patch sets drm_connector_hdmi_state.scrambler_needed when the computed
TMDS character rate exceeds the HDMI 1.3 maximum TMDS character rate.

> diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> index ad59318a3e85f..d112c15555178 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]
Could this introduce a regression where unsupported high-clock modes pass the
atomic check phase, only to fail later during atomic_commit?

If userspace requests a mode with a TMDS clock > 340 MHz on a legacy sink
that does not support scrambling, the hdmi_clock_valid() check bypasses the
maximum clock limit if info->max_tmds_clock == 0.

By setting scrambler_needed to true here without verifying if the sink
supports scrambling (e.g., via drm_scdc_sink_supports_scrambling()), the
validation successfully passes drm_atomic_helper_connector_hdmi_check().

However, later during atomic_commit, the attempt to enable scrambling fails:

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 the non-blocking KMS atomic_commit phase violates the atomic KMS
model and can leave the display hardware misconfigured. Should we validate sink
scrambling capabilities here in the atomic check phase instead?

> +
>  	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 ad59318a3e85..d112c1555517 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;
 }