Re: [PATCH v8 19/39] drm/bridge: dw-hdmi-qp: Add HDMI 2.0 SCDC scrambling support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] State tracking bug in atomic disable skips PHY shutdown on initial modesets.
--

commit 572f752e8a57b7a37e457765a056b711271ae975
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
Subject: drm/bridge: dw-hdmi-qp: Add HDMI 2.0 SCDC scrambling support

Enable HDMI 2.0 display modes (e.g. 4K@60Hz) by implementing SCDC
scrambling and high TMDS clock ratio management for TMDS character
rates exceeding the 340 MHz HDMI 1.4b limit. Reject modes requiring
TMDS rates above 600 MHz since those require HDMI 2.1 FRL.

> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
> index a0613f0d2ebcf..07b274f76e53e 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c

[ ... ]

> @@ -752,26 +751,33 @@ static void dw_hdmi_qp_bridge_atomic_enable(struct drm_bridge *bridge,
>  {
>  	struct dw_hdmi_qp *hdmi = bridge->driver_private;
>  	struct drm_connector_state *conn_state;
> -	struct drm_connector *connector;
>  	unsigned int op_mode;
> +	int ret;
>  
> -	connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
> -	if (WARN_ON(!connector))
> +	hdmi->curr_conn = drm_atomic_get_new_connector_for_encoder(state,
> +								   bridge->encoder);

[ ... ]

> @@ -787,8 +793,14 @@ static void dw_hdmi_qp_bridge_atomic_disable(struct drm_bridge *bridge,
>  {
>  	struct dw_hdmi_qp *hdmi = bridge->driver_private;
>  
> +	if (!hdmi->curr_conn)
> +		return;
> +

[Severity: Medium]
Will this early return skip the PHY disable sequence during initial modesets?

If the DRM core determines the encoder is active on boot and needs to be
disabled for a new modeset, it calls dw_hdmi_qp_bridge_atomic_disable().
Because dw_hdmi_qp_bridge_atomic_enable() hasn't run yet in this boot cycle,
hdmi->curr_conn will be NULL.

This causes the early return to trigger, skipping the hdmi->phy.ops->disable()
call below. Could this leave the hardware in an inconsistent state, causing PLL
lock failures or display glitches when atomic_enable() tries to re-initialize
an already running PHY with new parameters?

To avoid caching unreferenced pointers like curr_conn across state changes,
should the old connector be extracted directly from the atomic state using
drm_atomic_get_old_connector_for_encoder() instead?

>  	hdmi->tmds_char_rate = 0;
>  
> +	drm_connector_hdmi_disable_scrambling(hdmi->curr_conn);
> +
> +	hdmi->curr_conn = NULL;
>  	hdmi->phy.ops->disable(hdmi, hdmi->phy.data);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-dw-hdmi-qp-scramb-v8-0-d79890d00b6a@collabora.com?part=19