Re: [PATCH v9 18/61] 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 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 <[email protected]>

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

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