Hi Maxime,
On 5/22/25 7:16 PM, Maxime Ripard wrote:
> Hi,
>
> On Mon, May 19, 2025 at 01:55:10PM +0300, Cristian Ciocaltea wrote:
>> On 5/19/25 11:42 AM, Maxime Ripard wrote:
>>> Hi,
>>>
>>> On Fri, Apr 25, 2025 at 01:27:14PM +0300, Cristian Ciocaltea wrote:
>>>> Provide a test to verify that if both driver and screen support RGB and
>>>> YUV420 formats, drm_atomic_helper_connector_hdmi_check() cannot succeed
>>>> when trying to set a mode unsupported by the display.
>>>>
>>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocal...@collabora.com>
>>>> ---
>>>> drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 66
>>>> ++++++++++++++++++++++
>>>> 1 file changed, 66 insertions(+)
>>>>
>>>> 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
>>>> d79084cfb516b69c4244098c0767d604ad02f2c3..6337a1c52b86810c638f446c4995e7ee63dbc084
>>>> 100644
>>>> --- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
>>>> +++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
>>>> @@ -1622,6 +1622,71 @@ static void
>>>> drm_test_check_driver_unsupported_fallback_yuv420(struct kunit *test
>>>> drm_modeset_acquire_fini(&ctx);
>>>> }
>>>>
>>>> +/*
>>>> + * Test that if a driver and screen supports RGB and YUV420 formats, but
>>>> the
>>>> + * chosen mode cannot be supported by the screen, we end up with
>>>> unsuccessful
>>>> + * fallback attempts.
>>>> + */
>>>> +static void drm_test_check_display_unsupported_fallback_rgb_yuv420(struct
>>>> kunit *test)
>>>> +{
>>>> + struct drm_atomic_helper_connector_hdmi_priv *priv;
>>>> + struct drm_modeset_acquire_ctx ctx;
>>>> + struct drm_crtc_state *crtc_state;
>>>> + struct drm_atomic_state *state;
>>>> + struct drm_display_info *info;
>>>> + struct drm_display_mode *preferred, *unsupported_mode;
>>>> + struct drm_connector *conn;
>>>> + struct drm_device *drm;
>>>> + struct drm_crtc *crtc;
>>>> + int ret;
>>>> +
>>>> + priv = drm_kunit_helper_connector_hdmi_init_with_edid_funcs(test,
>>>> + BIT(HDMI_COLORSPACE_RGB) |
>>>> + BIT(HDMI_COLORSPACE_YUV420),
>>>> + 10,
>>>> + &dummy_connector_hdmi_funcs,
>>>> + test_edid_hdmi_4k_rgb_yuv420_dc_max_340mhz);
>>>> + KUNIT_ASSERT_NOT_NULL(test, priv);
>>>> +
>>>> + drm = &priv->drm;
>>>> + crtc = priv->crtc;
>>>> + conn = &priv->connector;
>>>> + info = &conn->display_info;
>>>> + KUNIT_ASSERT_TRUE(test, info->is_hdmi);
>>>> + KUNIT_ASSERT_TRUE(test, conn->ycbcr_420_allowed);
>>>> +
>>>> + preferred = find_preferred_mode(conn);
>>>> + KUNIT_ASSERT_NOT_NULL(test, preferred);
>>>> +
>>>> + unsupported_mode = drm_kunit_display_mode_from_cea_vic(test, drm, 96);
>>>> + KUNIT_ASSERT_NOT_NULL(test, unsupported_mode);
>>>
>>> I'm not sure what this one is supposed to test. If the mode is
>>> unsupported by the screen, it will be for both YUV and RGB, right? So
>>> what are we testing here?
>>
>> That would be the case suggested at [1]:
>>
>> "We still need to do the same with a driver that supports both, but the
>> monitor doesn't."
>>
>> Should we drop it?
>
> Ah, I see. I meant that we should normally end up with YUV420 (so mode
> is supported by the monitor, but the resolution is too high for RGB and
> we should pick YUV instead), but the monitor doesn't support it and thus
> we fail.
If I get it right, to verify this scenario we'd need a new test EDID
that basically advertises an RGB mode which is not actually supported by
the screen, i.e. the mode requires a TMDS rate which exceeds the maximum
supported by the display for any of the available color depths.
But that's not something we should encounter in practice, is it? I mean
the EDID would be wrong, as it doesn't match the hardware capabilities.
Regardless, assuming this is solely for the purpose of testing the
framework, I will try to come up with a test case. The only problem
is generating the EDID - which is a really annoying process, as I've
already mentioned a while ago [1].
Hence I'm wondering if we could move on without it for the moment (I'll
get back to it ASAP).
[1]
https://lore.kernel.org/all/8b0a8a7a-456a-487f-853e-5ec3e1112...@collabora.com/
