[igt-dev] [PATCH i-g-t v2 0/2] Add HDR negative tests

[igt-dev] [PATCH i-g-t v2 0/2] Add HDR negative tests

[Swati Sharma]

In this series, 2 test cases are introduced.
1. test for invalid metadata
2. test to enforce HDR on non-HDR panel

Swati Sharma (2):
  tests/kms_hdr: Add negative test for invalid metadata sizes
  tests/kms_hdr: Add negative test to enforce HDR on non-HDR panel

 tests/kms_hdr.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 51 insertions(+), 2 deletions(-)

-- 
2.25.1

[igt-dev] [PATCH i-g-t v2 1/2] tests/kms_hdr: Add negative test for invalid metadata sizes

[Swati Sharma]

Negative test is added in which HDR output metadata prop is
set with invalid sizes.

v2: -Fixed nitpicks (Kamil)
    -Add cleanup (Kamil)

Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
---
 tests/kms_hdr.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/tests/kms_hdr.c b/tests/kms_hdr.c
index 655c3e14f..63b03089b 100644
--- a/tests/kms_hdr.c
+++ b/tests/kms_hdr.c
@@ -50,6 +50,7 @@ enum {
 	TEST_DPMS = 1 << 1,
 	TEST_SUSPEND = 1 << 2,
 	TEST_SWAP = 1 << 3,
+	TEST_INVALID_METADATA_SIZES = 1 << 4,
 };
 
 /* BPC connector state. */
@@ -327,6 +328,18 @@ static void set_hdr_output_metadata(data_t *data,
 				     meta ? sizeof(*meta) : 0);
 }
 
+/* Sets the HDR output metadata prop with invalid size. */
+static int set_invalid_hdr_output_metadata(data_t *data,
+					   struct hdr_output_metadata const *meta,
+					   size_t length)
+{
+	igt_output_replace_prop_blob(data->output,
+				     IGT_CONNECTOR_HDR_OUTPUT_METADATA, meta,
+				     meta ? length : 0);
+
+	return igt_display_try_commit_atomic(&data->display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
+}
+
 /* Converts a double to 861-G spec FP format. */
 static uint16_t calc_hdr_float(double val)
 {
@@ -518,6 +531,22 @@ static void test_static_swap(data_t *data, enum pipe pipe, igt_output_t *output)
 	igt_remove_fb(data->fd, &afb);
 }
 
+static void test_invalid_metadata_sizes(data_t *data, igt_output_t *output)
+{
+	struct hdr_output_metadata hdr;
+	size_t metadata_size = sizeof(hdr);
+
+	fill_hdr_output_metadata_st2048(&hdr);
+
+	igt_assert_eq(set_invalid_hdr_output_metadata(data, &hdr, 1), -EINVAL);
+	igt_assert_eq(set_invalid_hdr_output_metadata(data, &hdr, metadata_size + 1), -EINVAL);
+	igt_assert_eq(set_invalid_hdr_output_metadata(data, &hdr, metadata_size - 1), -EINVAL);
+	igt_assert_eq(set_invalid_hdr_output_metadata(data, &hdr, metadata_size * 2), -EINVAL);
+
+	test_fini(data);
+	igt_remove_fb(data->fd, &afb);
+}
+
 /* Returns true if an output supports HDR metadata property. */
 static bool has_hdr(igt_output_t *output)
 {
@@ -565,6 +594,8 @@ static void test_hdr(data_t *data, uint32_t flags)
 						test_static_toggle(data, pipe, output, flags);
 					if (flags & TEST_SWAP)
 						test_static_swap(data, pipe, output);
+					if (flags & TEST_INVALID_METADATA_SIZES)
+						test_invalid_metadata_sizes(data, output);
 				}
 
 				/* One pipe is enough */
@@ -613,6 +644,10 @@ igt_main
 	igt_subtest_with_dynamic("static-swap")
 		test_hdr(&data, TEST_SWAP);
 
+	igt_describe("Tests invalid HDR metadata sizes");
+	igt_subtest_with_dynamic("invalid-metadata-sizes")
+		test_hdr(&data, TEST_INVALID_METADATA_SIZES);
+
 	igt_fixture {
 		igt_display_fini(&data.display);
 		close(data.fd);
-- 
2.25.1

[igt-dev] [PATCH i-g-t v2 2/2] tests/kms_hdr: Add negative test to enforce HDR on non-HDR panel

[Swati Sharma]

Negative test is added to ensure HDR is not enabled on
a non-HDR panel.
Commit/modeset won't fail. However, in dmesg we can find

[drm:drm_hdmi_infoframe_set_hdr_metadata [drm_display_helper]]
EOTF Not Supported

v2: -fixed nitpicks (Kamil)

Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
---
 tests/kms_hdr.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/tests/kms_hdr.c b/tests/kms_hdr.c
index 63b03089b..901b1da78 100644
--- a/tests/kms_hdr.c
+++ b/tests/kms_hdr.c
@@ -51,6 +51,7 @@ enum {
 	TEST_SUSPEND = 1 << 2,
 	TEST_SWAP = 1 << 3,
 	TEST_INVALID_METADATA_SIZES = 1 << 4,
+	TEST_INVALID_HDR = 1 << 5,
 };
 
 /* BPC connector state. */
@@ -414,6 +415,10 @@ static void test_static_toggle(data_t *data, enum pipe pipe,
 	set_hdr_output_metadata(data, &hdr);
 	igt_output_set_prop_value(data->output, IGT_CONNECTOR_MAX_BPC, 10);
 	igt_display_commit_atomic(display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
+	if (flags & TEST_INVALID_HDR) {
+		igt_assert_eq(system("dmesg|tail -n 1000|grep -E \"EOTF Not Supported\""), 0);
+		goto cleanup;
+	}
 	igt_assert_output_bpc_equal(data->fd, pipe, output->name, 10);
 
 	/* Verify that the CRC are equal after DPMS or suspend. */
@@ -429,6 +434,7 @@ static void test_static_toggle(data_t *data, enum pipe pipe,
 
 	igt_assert_crc_equal(&ref_crc, &new_crc);
 
+cleanup:
 	test_fini(data);
 	igt_remove_fb(data->fd, &afb);
 }
@@ -568,7 +574,11 @@ static void test_hdr(data_t *data, uint32_t flags)
 		if (!has_max_bpc(output) || !has_hdr(output))
 			continue;
 
-		if (!is_panel_hdr(data, output))
+		/* For negative test, panel should be non-hdr. */
+		if ((flags & TEST_INVALID_HDR) && is_panel_hdr(data, output))
+			continue;
+
+		if ((flags & ~TEST_INVALID_HDR) && !is_panel_hdr(data, output))
 			continue;
 
 		if (igt_get_output_max_bpc(data->fd, output->name) < 10)
@@ -590,7 +600,7 @@ static void test_hdr(data_t *data, uint32_t flags)
 
 				igt_dynamic_f("pipe-%s-%s",
 					      kmstest_pipe_name(pipe), output->name) {
-					if (flags & TEST_NONE || flags & TEST_DPMS || flags & TEST_SUSPEND)
+					if (flags & (TEST_NONE | TEST_DPMS | TEST_SUSPEND | TEST_INVALID_HDR))
 						test_static_toggle(data, pipe, output, flags);
 					if (flags & TEST_SWAP)
 						test_static_swap(data, pipe, output);
@@ -648,6 +658,10 @@ igt_main
 	igt_subtest_with_dynamic("invalid-metadata-sizes")
 		test_hdr(&data, TEST_INVALID_METADATA_SIZES);
 
+	igt_describe("Test to ensure HDR is not enabled on non-HDR panel");
+	igt_subtest_with_dynamic("invalid-hdr")
+		test_hdr(&data, TEST_INVALID_HDR);
+
 	igt_fixture {
 		igt_display_fini(&data.display);
 		close(data.fd);
-- 
2.25.1

[igt-dev] ✗ Fi.CI.BUILD: failure for Add HDR negative tests (rev3)

[Patchwork]

== Series Details ==

Series: Add HDR negative tests (rev3)
URL   : https://patchwork.freedesktop.org/series/112242/
State : failure

== Summary ==

IGT patchset build failed on latest successful build
165df656261863684067cd53f95c3a301e67fa24 tests/gem_barrier_race: Extend support over compute engines

[267/1037] Linking target lib/tests/igt_no_exit.
[268/1037] Linking target lib/tests/igt_runnercomms_packets.
[269/1037] Linking target lib/tests/igt_segfault.
[270/1037] Linking target lib/tests/igt_simulation.
[271/1037] Linking target lib/tests/igt_stats.
[272/1037] Linking target lib/tests/igt_types.
[273/1037] Linking target lib/tests/igt_subtest_group.
[274/1037] Linking target lib/tests/igt_thread.
[275/1037] Linking target lib/tests/i915_perf_data_alignment.
[276/1037] Linking target lib/tests/igt_audio.
[277/1037] Linking target lib/tests/bad_subtest_type.
[278/1037] Linking target lib/tests/igt_no_subtest.
[279/1037] Linking target lib/tests/igt_simple_test_subtests.
[280/1037] Linking target lib/tests/igt_timeout.
[281/1037] Linking target tests/core_auth.
[282/1037] Linking target tests/core_getclient.
[283/1037] Linking target tests/core_getstats.
[284/1037] Linking target tests/core_getversion.
[285/1037] Linking target tests/core_setmaster.
[286/1037] Linking target tests/core_setmaster_vs_auth.
[287/1037] Linking target tests/debugfs_test.
[288/1037] Linking target tests/dmabuf.
[289/1037] Linking target tests/dmabuf_sync_file.
[290/1037] Linking target tests/device_reset.
[291/1037] Linking target tests/drm_buddy.
[292/1037] Linking target tests/drm_mm.
[293/1037] Linking target tests/drm_read.
[294/1037] Linking target tests/fbdev.
[295/1037] Linking target tests/feature_discovery.
[296/1037] Linking target tests/kms_3d.
[297/1037] Linking target tests/kms_atomic.
[298/1037] Linking target tests/kms_addfb_basic.
[299/1037] Linking target tests/kms_async_flips.
[300/1037] Linking target tests/kms_atomic_interruptible.
[301/1037] Linking target tests/kms_atomic_transition.
[302/1037] Linking target tests/kms_bw.
[303/1037] Linking target tests/kms_concurrent.
[304/1037] Linking target tests/kms_content_protection.
[305/1037] Linking target tests/kms_cursor_crc.
[306/1037] Linking target tests/kms_cursor_edge_walk.
[307/1037] Linking target tests/kms_cursor_legacy.
[308/1037] Compiling C object 'tests/59830eb@@kms_hdr@exe/kms_hdr.c.o'.
FAILED: tests/59830eb@@kms_hdr@exe/kms_hdr.c.o 
cc -Itests/59830eb@@kms_hdr@exe -Itests -I../../../usr/src/igt-gpu-tools/tests -I../../../usr/src/igt-gpu-tools/include/drm-uapi -I../../../usr/src/igt-gpu-tools/include/linux-uapi -Ilib -I../../../usr/src/igt-gpu-tools/lib -I../../../usr/src/igt-gpu-tools/lib/stubs/syscalls -I. -I../../../usr/src/igt-gpu-tools/ -I/usr/include/cairo -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/uuid -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/libdrm -I/usr/include/x86_64-linux-gnu -I/usr/include/valgrind -I/usr/include -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -std=gnu11 -O2 -g -D_GNU_SOURCE -include config.h -D_FORTIFY_SOURCE=2 -Wbad-function-cast -Wdeclaration-after-statement -Wformat=2 -Wimplicit-fallthrough=0 -Wlogical-op -Wmissing-declarations -Wmissing-format-attribute -Wmissing-noreturn -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wredundant-decls -Wshadow -Wstrict-prototypes -Wuninitialized -Wunused -Wno-clobbered -Wno-maybe-uninitialized -Wno-missing-field-initializers -Wno-pointer-arith -Wno-address-of-packed-member -Wno-sign-compare -Wno-type-limits -Wno-unused-parameter -Wno-unused-result -Werror=address -Werror=array-bounds -Werror=implicit -Werror=init-self -Werror=int-to-pointer-cast -Werror=main -Werror=missing-braces -Werror=nonnull -Werror=pointer-to-int-cast -Werror=return-type -Werror=sequence-point -Werror=trigraphs -Werror=write-strings -fno-builtin-malloc -fno-builtin-calloc -pthread -MD -MQ 'tests/59830eb@@kms_hdr@exe/kms_hdr.c.o' -MF 'tests/59830eb@@kms_hdr@exe/kms_hdr.c.o.d' -o 'tests/59830eb@@kms_hdr@exe/kms_hdr.c.o' -c ../../../usr/src/igt-gpu-tools/tests/kms_hdr.c
../../../usr/src/igt-gpu-tools/tests/kms_hdr.c: In function ‘test_invalid_metadata_sizes’:
../../../usr/src/igt-gpu-tools/tests/kms_hdr.c:553:27: error: ‘afb’ undeclared (first use in this function)
  553 |  igt_remove_fb(data->fd, &afb);
      |                           ^~~
../../../usr/src/igt-gpu-tools/tests/kms_hdr.c:553:27: note: each undeclared identifier is reported only once for each function it appears in
ninja: build stopped: subcommand failed.

[igt-dev] [PATCH i-g-t v2 1/2] tests/kms_hdr: Add negative test for invalid metadata sizes

[Swati Sharma]

Negative test is added in which HDR output metadata prop is
set with invalid sizes.

v2: -Fixed nitpicks (Kamil)
    -Add cleanup (Kamil)

Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
---
 tests/kms_hdr.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/tests/kms_hdr.c b/tests/kms_hdr.c
index 655c3e14f..46b28b960 100644
--- a/tests/kms_hdr.c
+++ b/tests/kms_hdr.c
@@ -50,6 +50,7 @@ enum {
 	TEST_DPMS = 1 << 1,
 	TEST_SUSPEND = 1 << 2,
 	TEST_SWAP = 1 << 3,
+	TEST_INVALID_METADATA_SIZES = 1 << 4,
 };
 
 /* BPC connector state. */
@@ -327,6 +328,18 @@ static void set_hdr_output_metadata(data_t *data,
 				     meta ? sizeof(*meta) : 0);
 }
 
+/* Sets the HDR output metadata prop with invalid size. */
+static int set_invalid_hdr_output_metadata(data_t *data,
+					   struct hdr_output_metadata const *meta,
+					   size_t length)
+{
+	igt_output_replace_prop_blob(data->output,
+				     IGT_CONNECTOR_HDR_OUTPUT_METADATA, meta,
+				     meta ? length : 0);
+
+	return igt_display_try_commit_atomic(&data->display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
+}
+
 /* Converts a double to 861-G spec FP format. */
 static uint16_t calc_hdr_float(double val)
 {
@@ -518,6 +531,21 @@ static void test_static_swap(data_t *data, enum pipe pipe, igt_output_t *output)
 	igt_remove_fb(data->fd, &afb);
 }
 
+static void test_invalid_metadata_sizes(data_t *data, igt_output_t *output)
+{
+	struct hdr_output_metadata hdr;
+	size_t metadata_size = sizeof(hdr);
+
+	fill_hdr_output_metadata_st2048(&hdr);
+
+	igt_assert_eq(set_invalid_hdr_output_metadata(data, &hdr, 1), -EINVAL);
+	igt_assert_eq(set_invalid_hdr_output_metadata(data, &hdr, metadata_size + 1), -EINVAL);
+	igt_assert_eq(set_invalid_hdr_output_metadata(data, &hdr, metadata_size - 1), -EINVAL);
+	igt_assert_eq(set_invalid_hdr_output_metadata(data, &hdr, metadata_size * 2), -EINVAL);
+
+	test_fini(data);
+}
+
 /* Returns true if an output supports HDR metadata property. */
 static bool has_hdr(igt_output_t *output)
 {
@@ -565,6 +593,8 @@ static void test_hdr(data_t *data, uint32_t flags)
 						test_static_toggle(data, pipe, output, flags);
 					if (flags & TEST_SWAP)
 						test_static_swap(data, pipe, output);
+					if (flags & TEST_INVALID_METADATA_SIZES)
+						test_invalid_metadata_sizes(data, output);
 				}
 
 				/* One pipe is enough */
@@ -613,6 +643,10 @@ igt_main
 	igt_subtest_with_dynamic("static-swap")
 		test_hdr(&data, TEST_SWAP);
 
+	igt_describe("Tests invalid HDR metadata sizes");
+	igt_subtest_with_dynamic("invalid-metadata-sizes")
+		test_hdr(&data, TEST_INVALID_METADATA_SIZES);
+
 	igt_fixture {
 		igt_display_fini(&data.display);
 		close(data.fd);
-- 
2.25.1

Re: [igt-dev] [PATCH i-g-t v2 1/2] tests/kms_hdr: Add negative test for invalid metadata sizes

[Kamil Konieczny]

Hi Swati,

On 2023-02-22 at 16:18:05 +0530, Swati Sharma wrote:
> Negative test is added in which HDR output metadata prop is
> set with invalid sizes.
> 
> v2: -Fixed nitpicks (Kamil)
>     -Add cleanup (Kamil)
> 
> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
> Reviewed-by: Uma Shankar <uma.shankar@intel.com>

Lgtm,

Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

> ---
>  tests/kms_hdr.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/tests/kms_hdr.c b/tests/kms_hdr.c
> index 655c3e14f..63b03089b 100644
> --- a/tests/kms_hdr.c
> +++ b/tests/kms_hdr.c
> @@ -50,6 +50,7 @@ enum {
>  	TEST_DPMS = 1 << 1,
>  	TEST_SUSPEND = 1 << 2,
>  	TEST_SWAP = 1 << 3,
> +	TEST_INVALID_METADATA_SIZES = 1 << 4,
>  };
>  
>  /* BPC connector state. */
> @@ -327,6 +328,18 @@ static void set_hdr_output_metadata(data_t *data,
>  				     meta ? sizeof(*meta) : 0);
>  }
>  
> +/* Sets the HDR output metadata prop with invalid size. */
> +static int set_invalid_hdr_output_metadata(data_t *data,
> +					   struct hdr_output_metadata const *meta,
> +					   size_t length)
> +{
> +	igt_output_replace_prop_blob(data->output,
> +				     IGT_CONNECTOR_HDR_OUTPUT_METADATA, meta,
> +				     meta ? length : 0);
> +
> +	return igt_display_try_commit_atomic(&data->display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> +}
> +
>  /* Converts a double to 861-G spec FP format. */
>  static uint16_t calc_hdr_float(double val)
>  {
> @@ -518,6 +531,22 @@ static void test_static_swap(data_t *data, enum pipe pipe, igt_output_t *output)
>  	igt_remove_fb(data->fd, &afb);
>  }
>  
> +static void test_invalid_metadata_sizes(data_t *data, igt_output_t *output)
> +{
> +	struct hdr_output_metadata hdr;
> +	size_t metadata_size = sizeof(hdr);
> +
> +	fill_hdr_output_metadata_st2048(&hdr);
> +
> +	igt_assert_eq(set_invalid_hdr_output_metadata(data, &hdr, 1), -EINVAL);
> +	igt_assert_eq(set_invalid_hdr_output_metadata(data, &hdr, metadata_size + 1), -EINVAL);
> +	igt_assert_eq(set_invalid_hdr_output_metadata(data, &hdr, metadata_size - 1), -EINVAL);
> +	igt_assert_eq(set_invalid_hdr_output_metadata(data, &hdr, metadata_size * 2), -EINVAL);
> +
> +	test_fini(data);
> +	igt_remove_fb(data->fd, &afb);
> +}
> +
>  /* Returns true if an output supports HDR metadata property. */
>  static bool has_hdr(igt_output_t *output)
>  {
> @@ -565,6 +594,8 @@ static void test_hdr(data_t *data, uint32_t flags)
>  						test_static_toggle(data, pipe, output, flags);
>  					if (flags & TEST_SWAP)
>  						test_static_swap(data, pipe, output);
> +					if (flags & TEST_INVALID_METADATA_SIZES)
> +						test_invalid_metadata_sizes(data, output);
>  				}
>  
>  				/* One pipe is enough */
> @@ -613,6 +644,10 @@ igt_main
>  	igt_subtest_with_dynamic("static-swap")
>  		test_hdr(&data, TEST_SWAP);
>  
> +	igt_describe("Tests invalid HDR metadata sizes");
> +	igt_subtest_with_dynamic("invalid-metadata-sizes")
> +		test_hdr(&data, TEST_INVALID_METADATA_SIZES);
> +
>  	igt_fixture {
>  		igt_display_fini(&data.display);
>  		close(data.fd);
> -- 
> 2.25.1
> 

Re: [igt-dev] [PATCH i-g-t v2 2/2] tests/kms_hdr: Add negative test to enforce HDR on non-HDR panel

[Kamil Konieczny]

On 2023-02-22 at 16:18:06 +0530, Swati Sharma wrote:
> Negative test is added to ensure HDR is not enabled on
> a non-HDR panel.
> Commit/modeset won't fail. However, in dmesg we can find
> 
> [drm:drm_hdmi_infoframe_set_hdr_metadata [drm_display_helper]]
> EOTF Not Supported
> 
> v2: -fixed nitpicks (Kamil)
> 
> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
> Reviewed-by: Uma Shankar <uma.shankar@intel.com>

Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

> ---
>  tests/kms_hdr.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/kms_hdr.c b/tests/kms_hdr.c
> index 63b03089b..901b1da78 100644
> --- a/tests/kms_hdr.c
> +++ b/tests/kms_hdr.c
> @@ -51,6 +51,7 @@ enum {
>  	TEST_SUSPEND = 1 << 2,
>  	TEST_SWAP = 1 << 3,
>  	TEST_INVALID_METADATA_SIZES = 1 << 4,
> +	TEST_INVALID_HDR = 1 << 5,
>  };
>  
>  /* BPC connector state. */
> @@ -414,6 +415,10 @@ static void test_static_toggle(data_t *data, enum pipe pipe,
>  	set_hdr_output_metadata(data, &hdr);
>  	igt_output_set_prop_value(data->output, IGT_CONNECTOR_MAX_BPC, 10);
>  	igt_display_commit_atomic(display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> +	if (flags & TEST_INVALID_HDR) {
> +		igt_assert_eq(system("dmesg|tail -n 1000|grep -E \"EOTF Not Supported\""), 0);
> +		goto cleanup;
> +	}
>  	igt_assert_output_bpc_equal(data->fd, pipe, output->name, 10);
>  
>  	/* Verify that the CRC are equal after DPMS or suspend. */
> @@ -429,6 +434,7 @@ static void test_static_toggle(data_t *data, enum pipe pipe,
>  
>  	igt_assert_crc_equal(&ref_crc, &new_crc);
>  
> +cleanup:
>  	test_fini(data);
>  	igt_remove_fb(data->fd, &afb);
>  }
> @@ -568,7 +574,11 @@ static void test_hdr(data_t *data, uint32_t flags)
>  		if (!has_max_bpc(output) || !has_hdr(output))
>  			continue;
>  
> -		if (!is_panel_hdr(data, output))
> +		/* For negative test, panel should be non-hdr. */
> +		if ((flags & TEST_INVALID_HDR) && is_panel_hdr(data, output))
> +			continue;
> +
> +		if ((flags & ~TEST_INVALID_HDR) && !is_panel_hdr(data, output))
>  			continue;
>  
>  		if (igt_get_output_max_bpc(data->fd, output->name) < 10)
> @@ -590,7 +600,7 @@ static void test_hdr(data_t *data, uint32_t flags)
>  
>  				igt_dynamic_f("pipe-%s-%s",
>  					      kmstest_pipe_name(pipe), output->name) {
> -					if (flags & TEST_NONE || flags & TEST_DPMS || flags & TEST_SUSPEND)
> +					if (flags & (TEST_NONE | TEST_DPMS | TEST_SUSPEND | TEST_INVALID_HDR))
>  						test_static_toggle(data, pipe, output, flags);
>  					if (flags & TEST_SWAP)
>  						test_static_swap(data, pipe, output);
> @@ -648,6 +658,10 @@ igt_main
>  	igt_subtest_with_dynamic("invalid-metadata-sizes")
>  		test_hdr(&data, TEST_INVALID_METADATA_SIZES);
>  
> +	igt_describe("Test to ensure HDR is not enabled on non-HDR panel");
> +	igt_subtest_with_dynamic("invalid-hdr")
> +		test_hdr(&data, TEST_INVALID_HDR);
> +
>  	igt_fixture {
>  		igt_display_fini(&data.display);
>  		close(data.fd);
> -- 
> 2.25.1
> 

Re: [igt-dev] [PATCH i-g-t v2 2/2] tests/kms_hdr: Add negative test to enforce HDR on non-HDR panel

[Swati Sharma]

Thanks for the reviews! Patch pushed.

On 22-Feb-23 8:05 PM, Kamil Konieczny wrote:
> On 2023-02-22 at 16:18:06 +0530, Swati Sharma wrote:
>> Negative test is added to ensure HDR is not enabled on
>> a non-HDR panel.
>> Commit/modeset won't fail. However, in dmesg we can find
>>
>> [drm:drm_hdmi_infoframe_set_hdr_metadata [drm_display_helper]]
>> EOTF Not Supported
>>
>> v2: -fixed nitpicks (Kamil)
>>
>> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>> Reviewed-by: Uma Shankar <uma.shankar@intel.com>
> 
> Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> 
>> ---
>>   tests/kms_hdr.c | 18 ++++++++++++++++--
>>   1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/kms_hdr.c b/tests/kms_hdr.c
>> index 63b03089b..901b1da78 100644
>> --- a/tests/kms_hdr.c
>> +++ b/tests/kms_hdr.c
>> @@ -51,6 +51,7 @@ enum {
>>   	TEST_SUSPEND = 1 << 2,
>>   	TEST_SWAP = 1 << 3,
>>   	TEST_INVALID_METADATA_SIZES = 1 << 4,
>> +	TEST_INVALID_HDR = 1 << 5,
>>   };
>>   
>>   /* BPC connector state. */
>> @@ -414,6 +415,10 @@ static void test_static_toggle(data_t *data, enum pipe pipe,
>>   	set_hdr_output_metadata(data, &hdr);
>>   	igt_output_set_prop_value(data->output, IGT_CONNECTOR_MAX_BPC, 10);
>>   	igt_display_commit_atomic(display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
>> +	if (flags & TEST_INVALID_HDR) {
>> +		igt_assert_eq(system("dmesg|tail -n 1000|grep -E \"EOTF Not Supported\""), 0);
>> +		goto cleanup;
>> +	}
>>   	igt_assert_output_bpc_equal(data->fd, pipe, output->name, 10);
>>   
>>   	/* Verify that the CRC are equal after DPMS or suspend. */
>> @@ -429,6 +434,7 @@ static void test_static_toggle(data_t *data, enum pipe pipe,
>>   
>>   	igt_assert_crc_equal(&ref_crc, &new_crc);
>>   
>> +cleanup:
>>   	test_fini(data);
>>   	igt_remove_fb(data->fd, &afb);
>>   }
>> @@ -568,7 +574,11 @@ static void test_hdr(data_t *data, uint32_t flags)
>>   		if (!has_max_bpc(output) || !has_hdr(output))
>>   			continue;
>>   
>> -		if (!is_panel_hdr(data, output))
>> +		/* For negative test, panel should be non-hdr. */
>> +		if ((flags & TEST_INVALID_HDR) && is_panel_hdr(data, output))
>> +			continue;
>> +
>> +		if ((flags & ~TEST_INVALID_HDR) && !is_panel_hdr(data, output))
>>   			continue;
>>   
>>   		if (igt_get_output_max_bpc(data->fd, output->name) < 10)
>> @@ -590,7 +600,7 @@ static void test_hdr(data_t *data, uint32_t flags)
>>   
>>   				igt_dynamic_f("pipe-%s-%s",
>>   					      kmstest_pipe_name(pipe), output->name) {
>> -					if (flags & TEST_NONE || flags & TEST_DPMS || flags & TEST_SUSPEND)
>> +					if (flags & (TEST_NONE | TEST_DPMS | TEST_SUSPEND | TEST_INVALID_HDR))
>>   						test_static_toggle(data, pipe, output, flags);
>>   					if (flags & TEST_SWAP)
>>   						test_static_swap(data, pipe, output);
>> @@ -648,6 +658,10 @@ igt_main
>>   	igt_subtest_with_dynamic("invalid-metadata-sizes")
>>   		test_hdr(&data, TEST_INVALID_METADATA_SIZES);
>>   
>> +	igt_describe("Test to ensure HDR is not enabled on non-HDR panel");
>> +	igt_subtest_with_dynamic("invalid-hdr")
>> +		test_hdr(&data, TEST_INVALID_HDR);
>> +
>>   	igt_fixture {
>>   		igt_display_fini(&data.display);
>>   		close(data.fd);
>> -- 
>> 2.25.1
>>

-- 
~Swati Sharma

Re: [igt-dev] [PATCH i-g-t v2 1/2] tests/kms_hdr: Add negative test for invalid metadata sizes

[Swati Sharma]

Thanks for the reviews! Patch pushed.

On 22-Feb-23 8:05 PM, Kamil Konieczny wrote:
> Hi Swati,
> 
> On 2023-02-22 at 16:18:05 +0530, Swati Sharma wrote:
>> Negative test is added in which HDR output metadata prop is
>> set with invalid sizes.
>>
>> v2: -Fixed nitpicks (Kamil)
>>      -Add cleanup (Kamil)
>>
>> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>> Reviewed-by: Uma Shankar <uma.shankar@intel.com>
> 
> Lgtm,
> 
> Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> 
>> ---
>>   tests/kms_hdr.c | 35 +++++++++++++++++++++++++++++++++++
>>   1 file changed, 35 insertions(+)
>>
>> diff --git a/tests/kms_hdr.c b/tests/kms_hdr.c
>> index 655c3e14f..63b03089b 100644
>> --- a/tests/kms_hdr.c
>> +++ b/tests/kms_hdr.c
>> @@ -50,6 +50,7 @@ enum {
>>   	TEST_DPMS = 1 << 1,
>>   	TEST_SUSPEND = 1 << 2,
>>   	TEST_SWAP = 1 << 3,
>> +	TEST_INVALID_METADATA_SIZES = 1 << 4,
>>   };
>>   
>>   /* BPC connector state. */
>> @@ -327,6 +328,18 @@ static void set_hdr_output_metadata(data_t *data,
>>   				     meta ? sizeof(*meta) : 0);
>>   }
>>   
>> +/* Sets the HDR output metadata prop with invalid size. */
>> +static int set_invalid_hdr_output_metadata(data_t *data,
>> +					   struct hdr_output_metadata const *meta,
>> +					   size_t length)
>> +{
>> +	igt_output_replace_prop_blob(data->output,
>> +				     IGT_CONNECTOR_HDR_OUTPUT_METADATA, meta,
>> +				     meta ? length : 0);
>> +
>> +	return igt_display_try_commit_atomic(&data->display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
>> +}
>> +
>>   /* Converts a double to 861-G spec FP format. */
>>   static uint16_t calc_hdr_float(double val)
>>   {
>> @@ -518,6 +531,22 @@ static void test_static_swap(data_t *data, enum pipe pipe, igt_output_t *output)
>>   	igt_remove_fb(data->fd, &afb);
>>   }
>>   
>> +static void test_invalid_metadata_sizes(data_t *data, igt_output_t *output)
>> +{
>> +	struct hdr_output_metadata hdr;
>> +	size_t metadata_size = sizeof(hdr);
>> +
>> +	fill_hdr_output_metadata_st2048(&hdr);
>> +
>> +	igt_assert_eq(set_invalid_hdr_output_metadata(data, &hdr, 1), -EINVAL);
>> +	igt_assert_eq(set_invalid_hdr_output_metadata(data, &hdr, metadata_size + 1), -EINVAL);
>> +	igt_assert_eq(set_invalid_hdr_output_metadata(data, &hdr, metadata_size - 1), -EINVAL);
>> +	igt_assert_eq(set_invalid_hdr_output_metadata(data, &hdr, metadata_size * 2), -EINVAL);
>> +
>> +	test_fini(data);
>> +	igt_remove_fb(data->fd, &afb);
>> +}
>> +
>>   /* Returns true if an output supports HDR metadata property. */
>>   static bool has_hdr(igt_output_t *output)
>>   {
>> @@ -565,6 +594,8 @@ static void test_hdr(data_t *data, uint32_t flags)
>>   						test_static_toggle(data, pipe, output, flags);
>>   					if (flags & TEST_SWAP)
>>   						test_static_swap(data, pipe, output);
>> +					if (flags & TEST_INVALID_METADATA_SIZES)
>> +						test_invalid_metadata_sizes(data, output);
>>   				}
>>   
>>   				/* One pipe is enough */
>> @@ -613,6 +644,10 @@ igt_main
>>   	igt_subtest_with_dynamic("static-swap")
>>   		test_hdr(&data, TEST_SWAP);
>>   
>> +	igt_describe("Tests invalid HDR metadata sizes");
>> +	igt_subtest_with_dynamic("invalid-metadata-sizes")
>> +		test_hdr(&data, TEST_INVALID_METADATA_SIZES);
>> +
>>   	igt_fixture {
>>   		igt_display_fini(&data.display);
>>   		close(data.fd);
>> -- 
>> 2.25.1
>>

-- 
~Swati Sharma