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

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 <[email protected]>

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=23