Re: [PATCH i-g-t][V3] tests/kms_hdr: Add visual HDR verification subtests

["Sharma, Swati2" <swati2.sharma@intel.com>]

Hi Alex,

On 12-05-2026 06:06 am, Alex Hung wrote:
> Hi,
>
> Does anyone have any concerns if I merge this patch?
>
> On 5/7/26 15:13, Alex Hung wrote:
>> From: Wayne Lin <wayne.lin@amd.com>
>>
>> The new subtests display an HDR test pattern under a given metadata
>> mode and pause for user confirmation, enabling manual inspection of
>> HDR output quality on a connected HDR panel.
>>
>> New subtests:
>> kms_hdr --run-subtest static-swap-smpte2084 
>> --interactive-debug=smpte2084
>> kms_hdr --run-subtest static-swap-traditional-sdr 
>> --interactive-debug=traditional-sdr
>>
>> Co-developed-by: Alex Hung <alex.hung@amd.com>
>> Signed-off-by: Alex Hung <alex.hung@amd.com>
Is Co-developed redundant?
>> Signed-off-by: Wayne Lin <wayne.lin@amd.com>
>> Assisted-by: Copilot:Claude-Sonnet-4.6
>> ---
>>   tests/kms_hdr.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 89 insertions(+)
>>
>> diff --git a/tests/kms_hdr.c b/tests/kms_hdr.c
>> index 876c1c03a..c13e985e1 100644
>> --- a/tests/kms_hdr.c
>> +++ b/tests/kms_hdr.c
>> @@ -67,6 +67,14 @@
>>    *
>>    * @swap:                    swapping static HDR metadata
>>    * @toggle:                  entering and exiting HDR mode
>> + *
>> + * SUBTEST: static-swap-smpte2084
>> + * Description: Show a visual HDR pattern with SMPTE ST 2084 
>> metadata and
>> + * require user confirmation.
>> + *
>> + * SUBTEST: static-swap-traditional-sdr
>> + * Description: Show a visual HDR pattern with traditional SDR gamma 
>> metadata
>> + * and require user confirmation.
>>    */
>>
>>   IGT_TEST_DESCRIPTION("Test HDR metadata interfaces and bpc switch");
>> @@ -647,6 +655,79 @@ static void test_hdr(data_t *data, uint32_t flags)
>>       }
>>   }
>>
>> +static void test_hdr_visual(data_t *data,
>> +                 void (*fill_metadata)(struct hdr_output_metadata *),
>> +                 const char *mode_name) 
>> +{
>> +    igt_display_t *display = &data->display;
>> +    igt_output_t *output;
>> +    igt_fb_t afb;
>> +    int afb_id;
>> +    bool found = false;
>> +    struct hdr_output_metadata hdr;
>> +
>> +    igt_display_reset(display);
>> +
>> +    for_each_connected_output(display, output) {
>> +        igt_crtc_t *crtc;
>> +
>> +        if (!has_max_bpc(output) || !igt_output_supports_hdr(output)) {
>> +            igt_info("%s: Doesn't support IGT_CONNECTOR_MAX_BPC or 
>> IGT_CONNECTOR_HDR_OUTPUT_METADATA.\n",
>> +                 igt_output_name(output));
>> +            continue;
>> +        }
>> +
>> +        if (!igt_is_panel_hdr(data->fd, output)) {
>> +            igt_info("Panel attached via %s connector is non-HDR\n", 
>> igt_output_name(output));
>> +            continue;
>> +        }
>> +
>> +        if (igt_get_output_max_bpc(output) < 10) {
>> +            igt_info("%s: Doesn't support 10 bpc.\n", 
>> igt_output_name(output));
>> +            continue;
>> +        }
>> +
>> +        for_each_crtc(display, crtc) {
>> +            igt_output_set_crtc(output, crtc);
>> +            if (!igt_crtc_connector_valid(crtc, output))
>> +                continue;
>> +
>> +            prepare_test(data, output, crtc);
>> +
>> +            afb_id = igt_create_fb(data->fd, 512, 512,
>> +                           DRM_FORMAT_XRGB2101010,
>> +                           DRM_FORMAT_MOD_LINEAR, &afb);
>> +            igt_assert(afb_id);
>> +
>> +            draw_hdr_pattern(&afb);
>> +
>> +            igt_plane_set_fb(data->primary, &afb);
>> +            igt_plane_set_size(data->primary, data->w, data->h);
>> +            fill_metadata(&hdr);
>> +            igt_hdr_set_metadata(data->output, &hdr);
>> +            igt_output_set_prop_value(data->output, 
>> IGT_CONNECTOR_MAX_BPC, 10);
>> +            igt_display_commit_atomic(display,
>> +                          DRM_MODE_ATOMIC_ALLOW_MODESET,
>> +                          NULL);
>> +
>> +            igt_info("Displaying %s HDR pattern. Press a key to 
>> continue.\n", mode_name);
>> +            igt_debug_wait_for_keypress(mode_name);
>> +
>> +            igt_hdr_set_metadata(data->output, NULL);
>> +            igt_output_set_prop_value(data->output, 
>> IGT_CONNECTOR_MAX_BPC, 8);
>> +            igt_display_commit_atomic(display, 
>> DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
>> +
>> +            test_fini(data);
>> +            igt_remove_fb(data->fd, &afb);
>> +
>> +            found = true;
>> +            break;
>> +        }
>> +    }
>> +
>> +    igt_require_f(found, "No HDR-capable connector found.\n");
>> +}
>> +
>>   int igt_main()
>>   {
>>       data_t data = {};
>> @@ -698,6 +779,14 @@ int igt_main()
>>       igt_subtest_with_dynamic("invalid-hdr")
>>           test_hdr(&data, TEST_INVALID_HDR);
>>
>> +    igt_describe("Show HDR pattern with SMPTE ST 2084 metadata and 
>> require user confirmation");
>> +    igt_subtest("static-swap-smpte2084")
>> +        test_hdr_visual(&data, igt_hdr_fill_st2084, "smpte2084");
>> +
>> +    igt_describe("Show HDR pattern with traditional SDR gamma 
>> metadata and require user confirmation");
>> +    igt_subtest("static-swap-traditional-sdr")
>> +        test_hdr_visual(&data, igt_hdr_fill_sdr, "traditional-sdr");
igt_debug_wait_for_keypress() is a no-op in CI — these subtests just 
duplicate static-swap with no added validation.
Instead, add igt_debug_interactive_mode_check() calls inside the 
existing test_static_swap() after each metadata commit. This is CI-safe 
(no-op without terminal) and provides Y/n visual verdict when run with 
--interactive-debug=all. See tests/intel/kms_dsc.c:90-92,216 for 
reference. No new subtests or functions needed.
>> +
>>       igt_fixture() {
>>           igt_display_fini(&data.display);
>>           drm_close_driver(data.fd);
>> -- 
>> 2.43.0
>>
>