Re: [PATCH 2/3] drm/display: hdmi: Use committed state to decide on SCDC resync

["Maíra Canal" <mcanal@igalia.com>]

Hi Cristian,

On 04/11/26 10:26, Cristian Ciocaltea wrote:
> drm_connector_hdmi_sync_scdc() uses the scrambler_enabled flag to decide
> whether the sink-side SCDC status is worth checking on a hotplug/detect
> event.  That flag tracks the source-side scrambler status, which is not
> necessarily in sync with what the committed mode actually requires.
> 
> In particular, a source driver may deliberately set scrambler_enabled at
> init to force a disable at boot and resync hardware left enabled by the
> firmware, as vc4 does.  If the pipeline doesn't get disabled afterwards,
> e.g. because no display was active at probe time, the flag remains
> stale.  A subsequent detect (e.g. an HPD event) then finds scrambling
> disabled on the sink while the flag claims otherwise, and triggers a
> CRTC reset.  This results in a one-time spurious flicker on an otherwise
> correctly configured non-scrambled mode.
> 
> Verify conn_state->hdmi.scrambler_needed instead, as it is computed
> during atomic check and reliably describes the requirements of the
> active mode.
> 
> Fixes: d653818324ed ("drm/display: hdmi: Add HDMI 2.0 scrambling management helpers")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/all/sashiko-outbox-170670@kernel.org/
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

Reviewed-by: Maíra Canal <mcanal@igalia.com>

Best regards,
- Maíra

> ---
>   drivers/gpu/drm/display/drm_hdmi_helper.c | 17 +++++++++++------
>   1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_hdmi_helper.c b/drivers/gpu/drm/display/drm_hdmi_helper.c
> index 872f7e32e8d2..974d5de6d380 100644
> --- a/drivers/gpu/drm/display/drm_hdmi_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdmi_helper.c
> @@ -653,7 +653,6 @@ EXPORT_SYMBOL(drm_connector_hdmi_disable_scrambling);
>   int drm_connector_hdmi_sync_scdc(struct drm_connector *connector, bool plugged,
>   				 struct drm_modeset_acquire_ctx *ctx)
>   {
> -	struct drm_connector_hdmi *hdmi;
>   	struct drm_connector_state *conn_state;
>   	struct drm_device *dev;
>   	struct drm_crtc *crtc;
> @@ -663,7 +662,6 @@ int drm_connector_hdmi_sync_scdc(struct drm_connector *connector, bool plugged,
>   	if (!connector || !plugged || !ctx)
>   		return 0;
>   
> -	hdmi = &connector->hdmi;
>   	dev = connector->dev;
>   
>   	/*
> @@ -677,14 +675,21 @@ int drm_connector_hdmi_sync_scdc(struct drm_connector *connector, bool plugged,
>   
>   	/* TODO: Also handle HDMI 2.1 FRL link training */
>   
> -	if (!drm_connector_hdmi_scrambler_supported(connector) ||
> -	    !READ_ONCE(hdmi->scrambler_enabled))
> -		return 0;
> -
>   	conn_state = connector->state;
>   	if (!conn_state)
>   		return 0;
>   
> +	/*
> +	 * Rely on the committed connector state rather than on the current
> +	 * scrambler status: a source driver may force scrambler_enabled at
> +	 * init to get a disable at boot (e.g. vc4), and that value would
> +	 * remain stale until the first disable actually happens, leading to
> +	 * a spurious CRTC reset whenever the active mode needs no scrambling.
> +	 */
> +	if (!drm_connector_hdmi_scrambler_supported(connector) ||
> +	    !conn_state->hdmi.scrambler_needed)
> +		return 0;
> +
>   	crtc = conn_state->crtc;
>   	if (!crtc)
>   		return 0;
>