Re: [PATCH] drm/amd/display: Try RGB before YCbCr 4:4:4 in stream validation

[Ivan Lipski]

Reviewed-by: Ivan Lipski <ivan.lipski@xxxxxxx>

On 9/11/26 11:13, Adrian Betschart wrote:
> amdgpu_dm_create_validate_stream_for_sink() walks encoding_order[] and
> uses the first encoding that validates. YCbCr 4:4:4 is listed before
> RGB, so an HDMI sink that advertises 4:4:4 gets YCbCr 4:4:4 whenever the
> "color format" property is left at AUTO, even though RGB fits the same
> link.
>
> That contradicts the documented AUTO behaviour for HDMI in enum
> drm_connector_color_format (RGB, falling back to YCbCr 4:2:0 only when
> the bandwidth is not available or the mode is 4:2:0-only), which the
> amdgpu implementation of the property also describes. It also leaves
> the "Broadcast RGB" property without effect on such sinks, since the
> quantization range it selects only applies to RGB output.
>
> Try RGB first. The mask still holds every encoding the sink supports,
> so a mode that cannot carry RGB falls back exactly as before.
>
> For reference, v7.2 picked RGB here unless YCbCr 4:4:4 was forced
> through debugfs, while earlier kernels picked YCbCr 4:4:4 for any HDMI
> sink that advertised it.
>
> Fixes: 0b0ff65d3ca1 ("drm/amd/display: Refactor stream validation")
> Suggested-by: Adolfo Rodrigues <adolfotregosa@xxxxxxxxx>
> Tested-by: Adolfo Rodrigues <adolfotregosa@xxxxxxxxx>
> Signed-off-by: Adrian Betschart <adrian.betschart@xxxxxxxxxxxx>
> Assisted-by: Claude Code:claude-fable-5-1
> ---
>   .../display/amdgpu_dm/amdgpu_dm_connector.c   |  2 +-
>   .../tests/amdgpu_dm_connector_test.c          | 34 +++++++++++++++++++
>   2 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> index 0b446133edd8..98d0b35ed4e4 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> @@ -2226,8 +2226,8 @@ amdgpu_dm_create_validate_stream_for_sink(struct drm_connector *connector,
>   	 * below gate which of these entries are actually attempted.
>   	 */
>   	static const enum dc_pixel_encoding encoding_order[] = {
> -		PIXEL_ENCODING_YCBCR444,
>   		PIXEL_ENCODING_RGB,
> +		PIXEL_ENCODING_YCBCR444,
>   		PIXEL_ENCODING_YCBCR422,
>   		PIXEL_ENCODING_YCBCR420,
>   	};
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_connector_test.c b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_connector_test.c
> index 6e70d72135dc..c064def8253b 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_connector_test.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_connector_test.c
> @@ -7175,6 +7175,39 @@ static void dm_test_create_validate_stream_prune_context(struct kunit *test)
>   								    NULL));
>   }
>   
> +static enum dc_pixel_encoding dm_test_cvs_first_encoding;
> +
> +static bool dm_test_cvs_record_first_encoding(struct timing_generator *tg,
> +					      const struct dc_crtc_timing *timing)
> +{
> +	if (dm_test_cvs_first_encoding == PIXEL_ENCODING_UNDEFINED)
> +		dm_test_cvs_first_encoding = timing->pixel_encoding;
> +
> +	return false;
> +}
> +
> +/**
> + * dm_test_create_validate_stream_hdmi_rgb_first - HDMI tries RGB before YCbCr 4:4:4
> + * @test: The KUnit test context
> + */
> +static void dm_test_create_validate_stream_hdmi_rgb_first(struct kunit *test)
> +{
> +	struct dm_test_cvs_dc *c = dm_test_cvs_dc_alloc(test);
> +
> +	c->link->connector_signal = SIGNAL_TYPE_HDMI_TYPE_A;
> +	c->aconnector->base.display_info.color_formats =
> +		BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444);
> +	c->tgfuncs->validate_timing = dm_test_cvs_record_first_encoding;
> +	dm_test_cvs_first_encoding = PIXEL_ENCODING_UNDEFINED;
> +
> +	KUNIT_EXPECT_NULL(test,
> +			  amdgpu_dm_create_validate_stream_for_sink(&c->aconnector->base,
> +								    c->mode,
> +								    c->dm_state,
> +								    NULL));
> +	KUNIT_EXPECT_EQ(test, (int)dm_test_cvs_first_encoding, (int)PIXEL_ENCODING_RGB);
> +}
> +
>   /* Further tests for amdgpu_dm_connector_mode_valid() */
>   
>   /**
> @@ -9268,6 +9301,7 @@ static struct kunit_case amdgpu_dm_connector_tests[] = {
>   	KUNIT_CASE(dm_test_create_validate_stream_force_ycbcr444),
>   	KUNIT_CASE(dm_test_create_validate_stream_prune_timing),
>   	KUNIT_CASE(dm_test_create_validate_stream_prune_context),
> +	KUNIT_CASE(dm_test_create_validate_stream_hdmi_rgb_first),
>   	/* amdgpu_dm_update_connector_after_detect */
>   	KUNIT_CASE(dm_test_update_after_detect_mst_noop),
>   	KUNIT_CASE(dm_test_update_after_detect_sink_unchanged),

--
Thanks,
Ivan Lipski