[igt-dev] [PATCH 1/2] tests/kms_hdr: Add test for output Colorspace
Pekka Paalanen
ppaalanen at gmail.com
Tue Dec 13 11:39:22 UTC 2022
On Mon, 12 Dec 2022 14:24:24 -0500
Harry Wentland <harry.wentland at amd.com> wrote:
> The drm_connector Colorspace property needs a test.
> This adds a Colorspace switch test, similar to the
> bpc switch.
>
> Currently this will only work on amdgpu. Other drivers
> will need to provide a debugfs to readback the currently
> set Colorspace in order to pass the test.
>
> Signed-off-by: Harry Wentland <harry.wentland at amd.com>
> Cc: Pekka Paalanen <ppaalanen at gmail.com>
> Cc: Sebastian Wick <sebastian.wick at redhat.com>
> Cc: Vitaly.Prosyak at amd.com
> Cc: Uma Shankar <uma.shankar at intel.com>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Cc: Joshua Ashton <joshua at froggi.es>
> Cc: Bhanuprakash Modem <bhanuprakash.modem at intel.com>
> Cc: Rodrigo Siqueira <Rodrigo.Siqueira at amd.com>
> Cc: Alex Hung <Alex.Hung at amd.com>
> ---
> lib/igt_kms.c | 51 +++++++++++++++++
> lib/igt_kms.h | 6 ++
> tests/kms_hdr.c | 143 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 200 insertions(+)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index b4a98ae176e1..15fe49b3f55c 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -640,6 +640,7 @@ const char * const igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
> [IGT_CONNECTOR_LINK_STATUS] = "link-status",
> [IGT_CONNECTOR_MAX_BPC] = "max bpc",
> [IGT_CONNECTOR_HDR_OUTPUT_METADATA] = "HDR_OUTPUT_METADATA",
> + [IGT_CONNECTOR_COLORSPACE] = "Colorspace",
> [IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS] = "WRITEBACK_PIXEL_FORMATS",
> [IGT_CONNECTOR_WRITEBACK_FB_ID] = "WRITEBACK_FB_ID",
> [IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR] = "WRITEBACK_OUT_FENCE_PTR",
> @@ -2272,6 +2273,10 @@ static void igt_output_reset(igt_output_t *output)
> igt_output_set_prop_value(output,
> IGT_CONNECTOR_HDR_OUTPUT_METADATA, 0);
>
> + if (igt_output_has_prop(output, IGT_CONNECTOR_COLORSPACE))
> + igt_output_set_prop_enum(output,
> + IGT_CONNECTOR_COLORSPACE, "Default");
> +
> if (igt_output_has_prop(output, IGT_CONNECTOR_WRITEBACK_FB_ID))
> igt_output_set_prop_value(output, IGT_CONNECTOR_WRITEBACK_FB_ID, 0);
> if (igt_output_has_prop(output, IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR)) {
> @@ -2296,6 +2301,7 @@ static void igt_output_reset(igt_output_t *output)
> * - %IGT_CONNECTOR_BROADCAST_RGB (if applicable)
> * %IGT_CONNECTOR_CONTENT_PROTECTION (if applicable)
> * %IGT_CONNECTOR_HDR_OUTPUT_METADATA (if applicable)
> + * %IGT_CONNECTOR_COLORSPACE (if applicable)
> * - %IGT_CONNECTOR_DITHERING_MODE (if applicable)
> * - igt_output_override_mode() to default.
> *
> @@ -5801,6 +5807,51 @@ bool igt_check_output_bpc_equal(int drmfd, enum pipe pipe,
> return (current == bpc);
> }
>
> +
> +/*
> + * igt_get_pipe_current_colorspace:
> + * @drmfd: A drm file descriptor
> + * @pipe: Display pipe
> + *
> + * Returns: The current colorspace from the crtc debugfs.
> + */
> +void igt_get_pipe_current_colorspace(int drmfd, enum pipe pipe,
> + char *colorspace, int size)
> +{
> + char debugfs_name[30];
> + int fd, res;
> +
> + fd = igt_debugfs_pipe_dir(drmfd, pipe, O_RDONLY);
> + igt_assert(fd >= 0);
> +
> + /* TODO add support for other drivers */
> + if (is_amdgpu_device(drmfd))
> + strcpy(debugfs_name, "amdgpu_current_colorspace");
> +
> + res = igt_debugfs_simple_read(fd, debugfs_name, colorspace, size);
> + igt_require(res > 0);
> +
> + close(fd);
> +}
> +
> +/*
> + * igt_assert_output_bpc_equal:
> + * @drmfd: A drm file descriptor
> + * @pipe: Display pipe
> + * @output_name: Name of the libdrm connector we're going to use
> + * @bpc: BPC to compare with max & current bpc
> + *
> + * Assert if crtc's current bpc is not matched with the requested one.
Hi,
forgot to rewrite the doc comment.
> + */
> +void igt_assert_output_colorspace_equal(int drmfd, enum pipe pipe,
> + const char *colorspace)
> +{
> + char buf[35];
> +
> + igt_get_pipe_current_colorspace(drmfd, pipe, buf, sizeof(buf));
> + igt_assert(strcmp(colorspace, buf) == 0);
> +}
> +
> /*
> * igt_max_bpc_constraint
> * @display: a pointer to an #igt_display_t structure
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 7a00d204545e..136292929fab 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -152,6 +152,7 @@ enum igt_atomic_connector_properties {
> IGT_CONNECTOR_LINK_STATUS,
> IGT_CONNECTOR_MAX_BPC,
> IGT_CONNECTOR_HDR_OUTPUT_METADATA,
> + IGT_CONNECTOR_COLORSPACE,
> IGT_CONNECTOR_WRITEBACK_PIXEL_FORMATS,
> IGT_CONNECTOR_WRITEBACK_FB_ID,
> IGT_CONNECTOR_WRITEBACK_OUT_FENCE_PTR,
> @@ -991,6 +992,11 @@ void igt_assert_output_bpc_equal(int drmfd, enum pipe pipe,
> bool igt_check_output_bpc_equal(int drmfd, enum pipe pipe,
> char *output_name, unsigned int bpc);
>
> +void igt_get_pipe_current_colorspace(int drmfd, enum pipe pipe,
> + char *colorspace, int size);
> +void igt_assert_output_colorspace_equal(int drmfd, enum pipe pipe,
> + const char *colorspace);
> +
> int sort_drm_modes_by_clk_dsc(const void *a, const void *b);
> int sort_drm_modes_by_clk_asc(const void *a, const void *b);
> int sort_drm_modes_by_res_dsc(const void *a, const void *b);
> diff --git a/tests/kms_hdr.c b/tests/kms_hdr.c
> index 655c3e14f7ef..b656dc7a7ed5 100644
> --- a/tests/kms_hdr.c
> +++ b/tests/kms_hdr.c
> @@ -244,6 +244,145 @@ static void test_bpc_switch(data_t *data, uint32_t flags)
> }
> }
>
> +/*
> + * List of all DP and HDMI colorspaces from drm_connector.c
> + *
> + * We purposely omit "Default" since driver behavior varies
> + * and there is no good way to test for it.
> + */
> +static const char *colorspaces[] = {
> + /* Standard Definition Colorimetry based on CEA 861 */
> + "SMPTE_170M_YCC",
> + "BT709_YCC",
> + /* Standard Definition Colorimetry based on IEC 61966-2-4 */
> + "XVYCC_601",
> + /* High Definition Colorimetry based on IEC 61966-2-4 */
> + "XVYCC_709",
> + /* Colorimetry based on IEC 61966-2-1/Amendment 1 */
> + "SYCC_601",
> + /* Colorimetry based on IEC 61966-2-5 [33] */
> + "opYCC_601",
> + /* Colorimetry based on IEC 61966-2-5 */
> + "opRGB",
> + /* Colorimetry based on ITU-R BT.2020 */
> + "BT2020_CYCC",
> + /* Colorimetry based on ITU-R BT.2020 */
> + "BT2020_RGB",
> + /* Colorimetry based on ITU-R BT.2020 */
> + "BT2020_YCC",
> + /* Added as part of Additional Colorimetry Extension in 861.G */
> + "DCI-P3_RGB_D65",
> + "DCI-P3_RGB_Theater",
> + /* For Default case, driver will set the colorspace */
> + "RGB_Wide_Gamut_Fixed_Point",
What default case?
> + /* Colorimetry based on scRGB (IEC 61966-2-2) */
> + "RGB_Wide_Gamut_Floating_Point",
> + "BT601_YCC",
> + NULL
> +};
> +
> +/* Prints the colorspace name on the framebuffer */
> +static void draw_colorspace(igt_fb_t *fb, const char* colorspace)
> +{
> + cairo_t *cr = igt_get_cairo_ctx(fb->fd, fb);
> +
> + igt_paint_color(cr, 0, 0, fb->width, 50, 1.0, 1.0, 1.0);
> + cairo_move_to(cr, fb->width / 2, 20);
> + cairo_set_font_size(cr, 12);
> + igt_cairo_printf_line(cr, align_hcenter, 0, "%s", colorspace);
> +
> + igt_put_cairo_ctx(cr);
> +}
> +
> +
> +static void test_colorspace_switch_on_output(data_t *data, enum pipe pipe,
> + igt_output_t *output,
> + uint32_t flags)
> +{
> + igt_display_t *display = &data->display;
> + // igt_crc_t ref_crc, new_crc;
> + igt_fb_t afb;
> + int afb_id, ret;
> +
> + /* 10-bit formats are slow, so limit the size. */
> + afb_id = igt_create_fb(data->fd, 512, 512, DRM_FORMAT_XRGB2101010, 0, &afb);
> + igt_assert(afb_id);
> +
> + draw_hdr_pattern(&afb);
> +
> + /* Plane may be required to fit fullscreen. Check it here and allow
> + * smaller plane size in following tests.
> + */
I don't understand the code here. Is there some kind of assumption that
the hardware is always capable of scaling a 512x512 FB to whatever CRTC
size is?
I would have expected to see an allocation of FB with CRTC size.
Thanks,
pq
> + igt_plane_set_fb(data->primary, &afb);
> + igt_plane_set_size(data->primary, data->w, data->h);
> + ret = igt_display_try_commit_atomic(display, DRM_MODE_ATOMIC_TEST_ONLY, NULL);
> + if (!ret) {
> + data->w = afb.width;
> + data->h = afb.height;
> + }
> +
> + /* require Colorspace prop */
> + igt_require(igt_output_has_prop(output, IGT_CONNECTOR_COLORSPACE));
> +
> + /* Get enum values for colorspace */
> + /* for each value set colorspace */
> + for (int i = 0; colorspaces[i]; i++) {
> + if (igt_output_try_prop_enum(output, IGT_CONNECTOR_COLORSPACE, colorspaces[i])) {
> + igt_info("Testing colorspace %s on %s\n",
> + colorspaces[i],
> + igt_output_name(output));
> + draw_colorspace(&afb, colorspaces[i]);
> + igt_output_set_prop_enum(output, IGT_CONNECTOR_COLORSPACE, colorspaces[i]);
> + igt_display_commit_atomic(display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> + /* get and check current colorspace */
> + igt_assert_output_colorspace_equal(data->fd, pipe, colorspaces[i]);
> + }
> + }
> +
> + igt_info("Testing bad colorspace on %s\n",
> + igt_output_name(output));
> + draw_colorspace(&afb, "bad");
> + igt_require(!igt_output_try_prop_enum(output, IGT_CONNECTOR_COLORSPACE, "bad"));
> +
> + /* revert back to "Default" */
> + igt_info("Testing colorspace %s on %s\n",
> + "Default",
> + igt_output_name(output));
> + draw_colorspace(&afb, "Default");
> + igt_output_set_prop_enum(output, IGT_CONNECTOR_COLORSPACE, "Default");
> + igt_display_commit_atomic(display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> +
> + test_fini(data);
> + igt_remove_fb(data->fd, &afb);
> +}
> +
> +static void test_colorspace_switch(data_t *data, uint32_t flags)
> +{
> + igt_display_t *display = &data->display;
> + igt_output_t *output;
> +
> + for_each_connected_output(display, output) {
> + enum pipe pipe;
> +
> + for_each_pipe(display, pipe) {
> + if (igt_pipe_connector_valid(pipe, output)) {
> + prepare_test(data, output, pipe);
> +
> + data->mode = igt_output_get_mode(output);
> + data->w = data->mode->hdisplay;
> + data->h = data->mode->vdisplay;
> +
> + igt_dynamic_f("pipe-%s-%s",
> + kmstest_pipe_name(pipe), output->name)
> + test_colorspace_switch_on_output(data, pipe, output, flags);
> +
> + /* One pipe is enough */
> + break;
> + }
> + }
> + }
> +}
> +
> static bool cta_block(const char *edid_ext)
> {
> /*
> @@ -599,6 +738,10 @@ igt_main
> igt_subtest_with_dynamic("bpc-switch-suspend")
> test_bpc_switch(&data, TEST_SUSPEND);
>
> + igt_describe("Tests switching Colorspace property, .i.e., the display colorimetry");
> + igt_subtest_with_dynamic("colorspace-switch")
> + test_colorspace_switch(&data, TEST_NONE);
> +
> igt_describe("Tests entering and exiting HDR mode");
> igt_subtest_with_dynamic("static-toggle")
> test_hdr(&data, TEST_NONE);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/igt-dev/attachments/20221213/f1cca41b/attachment.sig>
More information about the igt-dev
mailing list