Re: [PATCH v10 45/69] drm/vc4: hdmi: Convert to common HDMI 2.0 scrambling infrastructure

Thank you for your contribution! Sashiko AI review found 2 potential issue(=
s) to consider:

New issues:
- [Medium] Spurious CRTC reset at boot breaking fastboot / seamless display=
 takeover.

Pre-existing issues:
- [High] Atomic Context Violation due to sleeping functions in encoder call=
backs.
--

--- Patch [45]: [PATCH v10 45/69] drm/vc4: hdmi: Convert to common HDMI 2.0=
 scrambling infrastructure ---
commit 1688ee272699db4262a5b9b8449734034531bbd5
Author: Cristian Ciocaltea <[email protected]>

drm/vc4: hdmi: Convert to common HDMI 2.0 scrambling infrastructure

Replace driver local scrambling logic with the newly introduced common
HDMI 2.0 scrambling support.

> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdm=
i.c
> index d599957fee3ac..ec0c1f4224d75 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c

[ ... ]

> @@ -534,6 +409,14 @@ static int vc4_hdmi_connector_init(struct drm_device=
 *dev,
> =20
>  	drm_connector_helper_add(connector, &vc4_hdmi_connector_helper_funcs);
> =20
> +	/*
> +	 * Since we don't know the state of the controller and its
> +	 * display (if any), let's assume it's always enabled.
> +	 * drm_connector_hdmi_disable_scrambling() will thus run at boot,
> +	 * make sure it's disabled, and avoid any inconsistency.
> +	 */
> +	connector->hdmi.scrambler_enabled =3D drm_connector_hdmi_scrambler_supp=
orted(connector);

[Severity: Medium]
Will this cause an unnecessary full modeset and visual flicker at boot,
breaking fastboot seamless display takeover?

By unconditionally forcing the scrambler_enabled state to true here, when
vc4_hdmi_connector_detect_ctx() handles the initial detection, it calls
drm_atomic_helper_connector_hdmi_hotplug() which in turn invokes
drm_connector_hdmi_sync_scdc().

If the sink does not have scrambling currently active (for example, because
the bootloader set a 1080p mode that does not require it), the sync_scdc
helper will detect a mismatch between this spoofed driver state and the
actual sink state, forcing a CRTC reset.

[ ... ]

> @@ -890,7 +724,7 @@ static void vc4_hdmi_encoder_post_crtc_disable(struct=
 drm_encoder *encoder,
>  		spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags);
>  	}
> =20
> -	vc4_hdmi_disable_scrambling(encoder);
> +	drm_connector_hdmi_disable_scrambling(&vc4_hdmi->connector);

[Severity: High]
This isn't a bug introduced by this patch, but can this cause scheduling
while atomic warnings and potential deadlocks?

The encoder post_crtc_disable callback executes in atomic context during
display commits where sleeping is forbidden. However, this calls
drm_connector_hdmi_disable_scrambling(), which performs blocking operations
such as cancel_delayed_work_sync() and I2C transfers.

[ ... ]

> @@ -1666,7 +1501,9 @@ static void vc4_hdmi_encoder_post_crtc_enable(struc=
t drm_encoder *encoder,
>  	}
> =20
>  	vc4_hdmi_recenter_fifo(vc4_hdmi);
> -	vc4_hdmi_enable_scrambling(encoder);
> +
> +	conn_state =3D drm_atomic_get_new_connector_state(state, connector);
> +	drm_connector_hdmi_enable_scrambling(connector, conn_state);

[Severity: High]
This is a pre-existing issue, but similar to the disable path above, does
calling drm_connector_hdmi_enable_scrambling() here violate the atomic
context of the post_crtc_enable callback?

The generic scrambler setup performs blocking I2C operations, which can lead
to system instability or scheduling while atomic warnings when called from
this context.

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260731-dw-hdmi-qp=
[email protected]?part=3D45