[PATCH i-g-t v2 1/2] tests/kms_hdr: Add brightness tests for hdr

Samala, Pranay pranay.samala at intel.com
Mon Sep 30 06:15:54 UTC 2024


Hi Santosh,

> -----Original Message-----
> From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of Santhosh
> Reddy Guddati
> Sent: Friday, September 27, 2024 3:51 PM
> To: igt-dev at lists.freedesktop.org
> Cc: Reddy Guddati, Santhosh <santhosh.reddy.guddati at intel.com>
> Subject: [PATCH i-g-t v2 1/2] tests/kms_hdr: Add brightness tests for hdr
> 
> Add subtests for Aux based backlight control on hdr enabled edp panels by
> changing brightness via sysfs.
> 
> v2: Add function to adjust brightness in range 0 to max, refactor code and store
> and restore brightness before and after test. (Pranay)
> 
> Signed-off-by: Santhosh Reddy Guddati <santhosh.reddy.guddati at intel.com>
> ---
>  tests/kms_hdr.c | 144
> ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 144 insertions(+)
> 
> diff --git a/tests/kms_hdr.c b/tests/kms_hdr.c index f123c6b36..115ed6ab5
> 100644
> --- a/tests/kms_hdr.c
> +++ b/tests/kms_hdr.c
> @@ -64,6 +64,18 @@
>   * Description: Tests static toggle with suspend
>   * Functionality: colorspace, static_hdr, suspend
>   *
> + * SUBTEST: brightness-with-hdr
> + * Description: Tests brightness with HDR
> + * Functionality: colorspace, static_hdr
> + *
> + * SUBTEST: brightness-with-dpms
> + * Description: Tests brightness with dpms
> + * Functionality: colorspace, static_hdr, dpms
> + *
> + * SUBTEST: brightness-with-suspend
> + * Description: Tests brightness with suspend
> + * Functionality: colorspace, static_hdr, suspend
> + *
>   * SUBTEST: static-%s
>   * Description: Tests %arg[1].
>   * Functionality: colorspace, static_hdr @@ -81,6 +93,9 @@
> IGT_TEST_DESCRIPTION("Test HDR metadata interfaces and bpc switch");
>  #define HDR_STATIC_METADATA_BLOCK       0x06
>  #define USE_EXTENDED_TAG		0x07
> 
> +/* Intel Backlight Path. */
> +#define INTEL_BACKLIGHT_PATH "/sys/class/backlight/intel_backlight"
> +
>  /* DRM HDR definitions. Not in the UAPI header, unfortunately. */  enum
> hdmi_metadata_type {
>  	HDMI_STATIC_METADATA_TYPE1 = 0,
> @@ -100,6 +115,9 @@ enum {
>  	TEST_SWAP = 1 << 3,
>  	TEST_INVALID_METADATA_SIZES = 1 << 4,
>  	TEST_INVALID_HDR = 1 << 5,
> +	TEST_BRIGHTNESS = 1 << 6,
> +	TEST_BRIGHTNESS_DPMS = 1 << 7,
> +	TEST_BRIGHTNESS_SUSPEND = 1 << 8,
>  };
> 
>  /* BPC connector state. */
> @@ -623,6 +641,112 @@ static bool has_hdr(igt_output_t *output)
>  	return igt_output_has_prop(output,
> IGT_CONNECTOR_HDR_OUTPUT_METADATA);
>  }
> 
> +/* Move adjust_brightness to lib/igt_sysfs to reuse in other tests. */

Mention TODO here if you are planning to do this in future

> +static void adjust_brightness(data_t *data, uint32_t flags) {
> +	char brightness_path[1024];
> +	int brightness_fd;
> +	char brightness_value[64];
> +
> +	snprintf(brightness_path, sizeof(brightness_path),
> +			"%s/%s", INTEL_BACKLIGHT_PATH, "brightness");

Alignment should match open parenthesis

> +
> +	/* Get current brightness */
> +	brightness_fd = open(brightness_path, O_RDONLY);
> +	igt_assert_f(brightness_fd >= 0, "Failed to open %s\n",
> +brightness_path);
> +
> +	igt_assert(read(brightness_fd, brightness_value,
> +sizeof(brightness_value)) > 0);
> +
> +	int current_brightness = atoi(brightness_value);
> +
> +	close(brightness_fd);
> +
> +	/* Get max brightness */
> +	snprintf(brightness_path, sizeof(brightness_path),
> +			"%s/%s", INTEL_BACKLIGHT_PATH, "max_brightness");

Alignment should match open parenthesis

> +
> +	brightness_fd = open(brightness_path, O_RDONLY);
> +	igt_assert_f(brightness_fd >= 0, "Failed to open %s\n",
> +brightness_path);
> +
> +	igt_assert(read(brightness_fd, brightness_value,
> +sizeof(brightness_value)) > 0);
> +
> +	int max_brightness = atoi(brightness_value);
> +
> +	close(brightness_fd);
> +
> +	for (int write_brightness = 0; write_brightness <= max_brightness;
> write_brightness += 50) {
> +		brightness_fd = open(brightness_path, O_WRONLY);

This brightness_path has the path of max_brightness path. 
Change the path if you were trying to change the current brightness. 

> +		igt_assert_f(brightness_fd >= 0, "Failed to open %s\n",
> +brightness_path);
> +
> +		snprintf(brightness_value, sizeof(brightness_value), "%d",
> write_brightness);
> +		igt_assert(write(brightness_fd, brightness_value,
> +strlen(brightness_value)) > 0);
> +
> +		close(brightness_fd);
> +
> +		/* Verify brightness adjustment */
> +		brightness_fd = open(brightness_path, O_RDONLY);

Same as above (path is wrong if trying to access current brightness)

> +		igt_assert(brightness_fd >= 0);
> +
> +		igt_assert(read(brightness_fd, brightness_value,
> +sizeof(brightness_value)) > 0);
> +
> +		int read_brightness = atoi(brightness_value);
> +
> +		igt_assert_eq(read_brightness, write_brightness);
> +
> +		close(brightness_fd);
> +	}
> +
> +	brightness_fd = open(brightness_path, O_WRONLY);
> +	igt_assert_f(brightness_fd >= 0, "Failed to open %s\n",
> +brightness_path);
> +
> +	snprintf(brightness_value, sizeof(brightness_value), "%d",
> current_brightness);
> +	igt_assert(write(brightness_fd, brightness_value,

Same as above (path is wrong if trying to access current brightness)

> +strlen(brightness_value)) > 0); }
> +
> +static void test_brightness_output(data_t *data, enum pipe pipe,
> +				    igt_output_t *output,

Alignment should match open parenthesis

> +				    uint32_t flags)
> +{
> +	igt_display_t *display = &data->display;
> +	igt_fb_t afb;
> +	int afb_id;
> +	struct hdr_output_metadata hdr;
> +
> +	/* 10-bit formats are slow, so limit the size. */
> +	afb_id = igt_create_fb(data->fd, 512, 512,
> +					       DRM_FORMAT_XRGB2101010,

Alignment should match open parenthesis

> DRM_FORMAT_MOD_LINEAR,
> +						   &afb);
> +	igt_assert(afb_id);
> +
> +	draw_hdr_pattern(&afb);
> +
> +	/* Start in SDR. */
> +	igt_plane_set_fb(data->primary, &afb);
> +	igt_plane_set_size(data->primary, data->w, data->h);
> +	igt_output_set_prop_value(data->output, IGT_CONNECTOR_MAX_BPC,
> 8);
> +	igt_display_commit_atomic(display,
> DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> +	igt_assert_output_bpc_equal(data->fd, pipe, output->name, 8);
> +
> +	/* Enter HDR, a modeset is allowed here. */
> +	fill_hdr_output_metadata_st2048(&hdr);
> +	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);
> +	igt_assert_output_bpc_equal(data->fd, pipe, output->name, 10);
> +
> +	adjust_brightness(data, flags);
> +
> +	/* Exit HDR and enter 8bpc, cleanup. */
> +	set_hdr_output_metadata(data, NULL);
> +	igt_output_set_prop_value(data->output, IGT_CONNECTOR_MAX_BPC,
> 8);
> +	igt_display_commit_atomic(display,
> DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> +	igt_assert_output_bpc_equal(data->fd, pipe, output->name, 8);
> +
> +	test_fini(data);
> +	igt_remove_fb(data->fd, &afb);
> +

Blank line is not needed before a close brace

> +}
> +
>  static void test_hdr(data_t *data, uint32_t flags)  {
>  	igt_display_t *display = &data->display; @@ -691,6 +815,14 @@ static
> void test_hdr(data_t *data, uint32_t flags)
>  					test_static_swap(data, pipe, output);
>  				if (flags & TEST_INVALID_METADATA_SIZES)
>  					test_invalid_metadata_sizes(data,
> output);
> +				if (flags & (TEST_BRIGHTNESS |
> TEST_BRIGHTNESS_DPMS |
> +
> 	TEST_BRIGHTNESS_SUSPEND)) {
> +					igt_require(output->config.connector-
> >connector_type ==
> +
> 	DRM_MODE_CONNECTOR_eDP);
> +					igt_require_f(is_intel_device(data->fd),
> +									"Only

Alignment should match open parenthesis

Regards,
Pranay Samala

> supported on Intel devices\n");
> +					test_brightness_output(data, pipe,
> output, flags);
> +				}
>  			}
> 
>  			/* One pipe is enough */
> @@ -734,6 +866,18 @@ igt_main
>  	igt_subtest_with_dynamic("static-toggle-suspend")
>  		test_hdr(&data, TEST_SUSPEND);
> 
> +	igt_describe("Tests brightness while entering and exiting HDR mode");
> +	igt_subtest_with_dynamic("brightness-with-hdr")
> +		test_hdr(&data, TEST_BRIGHTNESS);
> +
> +	igt_describe("Tests brightness with dpms");
> +	igt_subtest_with_dynamic("brightness-with-dpms")
> +		test_hdr(&data, TEST_BRIGHTNESS_DPMS);
> +
> +	igt_describe("Tests brightness with suspend");
> +	igt_subtest_with_dynamic("brightness-with-suspend")
> +		test_hdr(&data, TEST_BRIGHTNESS_SUSPEND);
> +
>  	igt_describe("Tests swapping static HDR metadata");
>  	igt_subtest_with_dynamic("static-swap")
>  		test_hdr(&data, TEST_SWAP);
> --
> 2.34.1



More information about the igt-dev mailing list