Re: [PATCH 22/25] drm/tests: hdmi: Verify SCDC resync on hotplug

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] drm/tests: hdmi: missing scrambler enable in
  sync_scdc_test_init()
--

commit ad6b966d9b68c01d91b4a1ff13528d634475ed1e
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>

drm/tests: hdmi: Verify SCDC resync on hotplug

This commit adds KUnit tests for SCDC scrambling resynchronization across
drm_connector_hdmi_sync_scdc(), drm_atomic_helper_connector_hdmi_hotplug(),
and drm_atomic_helper_connector_hdmi_force_ctx().

> diff --git a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
> index 46654d8fa45cf..f17ccbf52bb18 100644
> --- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
> +++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
[ ... ]
> @@ -315,6 +316,8 @@ static void test_encoder_atomic_enable(struct drm_encoder *encoder,
>  		encoder_to_priv(encoder);
>  	int ret;
>  
> +	priv->encoder_enable_count++;
> +
>  	ret = drm_atomic_helper_connector_hdmi_update_infoframes(&priv->connector, state);
>  	if (ret)
>  		priv->hdmi_update_failures++;
> @@ -4789,12 +4792,321 @@ static struct kunit_suite drm_atomic_helper_connector_hdmi_scrambling_test_suite
[ ... ]
> +static struct drm_atomic_helper_connector_hdmi_priv *
> +sync_scdc_test_init(struct kunit *test,
> +		    const struct drm_connector_hdmi_funcs *hdmi_funcs,
> +		    u8 vic, struct drm_modeset_acquire_ctx *ctx)
> +{
> +	struct drm_atomic_helper_connector_hdmi_priv *priv;
> +	struct drm_display_mode *mode;
> +	int ret;
> +
> +	priv = drm_kunit_helper_connector_hdmi_init_with_scdc(test, hdmi_funcs,
> +				test_edid_hdmi_4k_rgb_yuv420_dc_max_600mhz);
> +	KUNIT_ASSERT_NOT_NULL(test, priv);
> +
> +	drm_encoder_helper_add(&priv->encoder, &test_encoder_helper_funcs);
> +
> +	mode = drm_kunit_display_mode_from_cea_vic(test, &priv->drm, vic);
> +	KUNIT_ASSERT_NOT_NULL(test, mode);
> +
> +	drm_modeset_acquire_init(ctx, 0);
> +
> +retry_conn_enable:
> +	ret = drm_kunit_helper_enable_crtc_connector(test, &priv->drm,
> +						     priv->crtc,
> +						     &priv->connector,
> +						     mode, ctx);
> +	if (ret == -EDEADLK) {
> +		ret = drm_modeset_backoff(ctx);
> +		if (!ret)
> +			goto retry_conn_enable;
> +	}
> +	KUNIT_ASSERT_EQ(test, ret, 0);
> +	KUNIT_ASSERT_EQ(test, priv->encoder_enable_count, 1);
> +
> +	return priv;
> +}

[Severity: Medium]
Does sync_scdc_test_init() leave priv->connector.hdmi.scrambler_enabled as
false when enabling a mode that requires scrambling?

Enabling the CRTC sets priv->connector.state->hdmi.scrambler_needed during
atomic check and invokes test_encoder_atomic_enable(), which updates
infoframes but never calls drm_connector_hdmi_enable_scrambling().
drm_connector_hdmi_sync_scdc() checks scrambler_enabled before inspecting
SCDC state:

drivers/gpu/drm/display/drm_hdmi_helper.c:drm_connector_hdmi_sync_scdc() {
	...
	if (!drm_connector_hdmi_scrambler_supported(connector) ||
	    !READ_ONCE(hdmi->scrambler_enabled))
		return 0;
	...
}

Because scrambler_enabled remains false, drm_connector_hdmi_sync_scdc()
returns 0 immediately without reading SCDC_TMDS_CONFIG over DDC or resetting
the CRTC, causing the subtests that expect a CRTC reset or DDC error to fail
their KUnit assertions and the remaining subtests to pass without exercising
SCDC reads.  Calling drm_connector_hdmi_enable_scrambling() from
test_encoder_atomic_enable() (or in sync_scdc_test_init() after enabling the
CRTC) would populate scrambler_enabled as expected.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114-drm-conn-hdmi-cleanup-v1-0-20e8a33a2286@collabora.com?part=22