[igt-dev] [PATCH i-g-t 6/7] tests/kms_hdr: Add subtest to validate HDR mode using luminance sensor

Shankar, Uma uma.shankar at intel.com
Tue Jan 14 15:03:52 UTC 2020



>-----Original Message-----
>From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of Petri Latvala
>Sent: Wednesday, January 8, 2020 2:59 PM
>To: Sharma, Swati2 <swati2.sharma at intel.com>
>Cc: igt-dev at lists.freedesktop.org
>Subject: Re: [igt-dev] [PATCH i-g-t 6/7] tests/kms_hdr: Add subtest to validate HDR
>mode using luminance sensor
>
>On Tue, Dec 31, 2019 at 06:51:57PM +0530, Swati Sharma wrote:
>> From: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
>>
>> Add subtest to validate HDR mode by setting the static metadata and
>> this involves display level verification for infoframes using a
>> luminance sensor. Therefore, luminance sensor is required for this
>> subtest, if sensor is not available test will skip.
>>
>> On intel driver, haven't validated this subtest but included this for
>> amd driver.
>>
>> Signed-off-by: Swati Sharma <swati2.sharma at intel.com>
>> ---
>>  tests/kms_hdr.c | 271
>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 271 insertions(+)
>>
>> diff --git a/tests/kms_hdr.c b/tests/kms_hdr.c index
>> dfd377b5..a63021b1 100644
>> --- a/tests/kms_hdr.c
>> +++ b/tests/kms_hdr.c
>> @@ -44,6 +44,7 @@ enum {
>>  	TEST_NONE = 0,
>>  	TEST_DPMS = 1 << 0,
>>  	TEST_SUSPEND = 1 << 1,
>> +	TEST_OUTPUT = 1 << 2,
>>  };
>>
>>  /* BPC connector state. */
>> @@ -61,6 +62,7 @@ typedef struct data {
>>  	igt_pipe_crc_t *pipe_crc;
>>  	drmModeModeInfo *mode;
>>  	enum pipe pipe_id;
>> +	int sensor_fd;
>>  	int fd;
>>  	int w;
>>  	int h;
>> @@ -165,6 +167,7 @@ static void prepare_test(data_t *data,
>> igt_output_t *output, enum pipe pipe)
>>
>>  	data->w = data->mode->hdisplay;
>>  	data->h = data->mode->vdisplay;
>> +	data->sensor_fd = -1;
>>  }
>>
>>  static bool igt_pipe_is_free(igt_display_t *display, enum pipe pipe)
>> @@ -393,6 +396,269 @@ static void test_static_toggle(data_t *data, igt_output_t
>*output,
>>  	}
>>  }
>>
>> +/*
>> + * Loads the sensor if unloaded. The sensor is a serial to USB
>> +interface that
>> + * prints the current measured luminance (nits) as a float, separated
>> +by a
>> + * newline. Uses baudrate 9600.
>> + */
>> +static void open_sensor(data_t *data) {
>> +	struct termios toptions;
>> +	int res;
>> +
>> +	/* Return if already loaded. */
>> +	if (data->sensor_fd >= 0)
>> +		return;
>> +
>> +	data->sensor_fd = open("/dev/ttyUSB0", O_RDWR | O_NOCTTY);
>
>What is the sensor device in question? How can you even know this device, if it exists,
>is the sensor?
>
>Connecting a usb-serial cable for a debug terminal is common enough to warrant not
>using ttyUSB0 blindly.

I agree, this is a hard coding and need to have better detection mechanism to identify
if a luminance sensor is actually present.

>
>> +	if (data->sensor_fd < 0) {
>> +		igt_info("Luminance sensor not found.\n");
>> +		return;
>> +	}
>> +
>> +	res = tcgetattr(data->sensor_fd, &toptions);
>> +	igt_assert_lte(0, res);
>> +
>> +	cfsetispeed(&toptions, B9600);
>> +	cfsetospeed(&toptions, B9600);
>> +
>> +	toptions.c_cflag &= ~(PARENB | CSTOPB | CSIZE);
>> +	toptions.c_cflag |= (CS8 | CREAD | CLOCAL);
>> +	toptions.c_lflag |= ICANON;
>> +
>> +	cfmakeraw(&toptions);
>> +
>> +	res = tcsetattr(data->sensor_fd, TCSANOW, &toptions);
>> +	igt_assert_lte(0, res);
>> +
>> +	res = tcsetattr(data->sensor_fd, TCSAFLUSH, &toptions);
>> +	igt_assert_lte(0, res);
>> +}
>> +
>> +/* Reads a string from the sensor. */ static void
>> +get_sensor_str(data_t *data, char *dst, int dst_bytes) {
>> +	char c = 0;
>> +	int i = 0;
>> +
>> +	igt_require(data->sensor_fd >= 0);
>> +	igt_set_timeout(3, "Waiting for sensor read\n");
>> +
>> +	dst_bytes -= 1;
>> +
>> +	while (c != '\n' && i < dst_bytes) {
>> +		int n = read(data->sensor_fd, &c, 1);
>> +		igt_assert_lte(0, n);
>> +
>> +		dst[i++] = c;
>> +	}
>> +
>> +	igt_reset_timeout();
>> +
>> +	if (dst_bytes > 0) {
>> +		dst[i] = 0;
>> +	}
>
>
>The buffer can be left non-nul-terminated if the device gives too much text.
>
>
>--
>Petri Latvala
>
>
>
>
>> +}
>> +
>> +/* Asserts that two given values in nits are equal within a given threshold. */
>> +static void assert_nits_equal(double n0, double n1, double threshold)
>> +{
>> +	double diff = fabs(n0 - n1);
>> +
>> +	igt_assert_f(diff <= threshold,
>> +		     "Nits not in threshold: | %.3f - %.3f | > %.3f\n",
>> +		     n0, n1, threshold);
>> +}
>> +
>> +/* Returns the current luminance reading from the sensor in cd/m^2. */
>> +static float get_sensor_nits(data_t *data)
>> +{
>> +	float nits = -1.0f;
>> +	char buf[32];
>> +
>> +	/* Sensor opening is deferred until we actually need it - here. */
>> +	open_sensor(data);
>> +
>> +	/* Flush old data from the buffer in case it's been a while. */
>> +	igt_require(data->sensor_fd >= 0);
>> +	tcflush(data->sensor_fd, TCIOFLUSH);
>> +
>> +	/* Read twice so we get a clean line. */
>> +	get_sensor_str(data, buf, ARRAY_SIZE(buf));
>> +	get_sensor_str(data, buf, ARRAY_SIZE(buf));
>> +
>> +	sscanf(buf, "%f", &nits);
>> +
>> +	return nits;
>> +}
>> +
>> +/* Logs the cursor sensor nits. */
>> +static void log_sensor_nits(double nits)
>> +{
>> +	igt_info("Sensor: %.3f nits\n", nits);
>> +}
>> +
>> +/*
>> + * Waits for the monitor to light-up to a given threshold, useful for
>> + * post-modeset measurement.
>> + */
>> +static void wait_for_threshold(data_t *data, double threshold)
>> +{
>> +	/*
>> +	 * If we read too quick the sensor might still be lit up.
>> +	 * Easy hack: just wait a second.
>> +	 */
>> +	sleep(1);
>> +
>> +	/* Poll sensor until lightup. */
>> +	igt_set_timeout(5, "Waiting for sensor read\n");
>> +
>> +	for (;;) {
>> +		double nits = get_sensor_nits(data);
>> +
>> +		if (nits >= threshold)
>> +			break;
>> +	}
>> +
>> +	igt_reset_timeout();
>> +}
>> +
>> +/* PQ Inverse, L normalized luminance to signal value V. */
>> +static double calc_pq_inverse(double l)
>> +{
>> +	double c1 = 0.8359375;
>> +	double c2 = 18.8515625;
>> +	double c3 = 18.6875;
>> +	double m1 = 0.1593017578125;
>> +	double m2 = 78.84375;
>> +	double lm = pow(l, m1);
>> +
>> +	return pow((c1 + c2 * lm) / (1.0 + c3 * lm), m2);
>> +}
>> +
>> +/* Fills the FB with a solid color given mapping to a light value in nits. */
>> +static void draw_light(igt_fb_t *fb, double nits)
>> +{
>> +	cairo_t *cr;
>> +	double l, v;
>> +	int x, y, w, h;
>> +
>> +	cr = igt_get_cairo_ctx(fb->fd, fb);
>> +
>> +	l = nits / 10000.0;
>> +	v = calc_pq_inverse(l);
>> +
>> +	/*
>> +	 * Draw a white rect in the bottom center of the screen for the sensor.
>> +	 * It's only 10% of the width and height of the screen since not every
>> +	 * monitor is capable of full HDR brightness for the whole screen.
>> +	 */
>> +	w = fb->width * 0.10;
>> +	h = fb->height * 0.10;
>> +	x = (fb->width - w) / 2;
>> +	y = (fb->height - h);
>> +
>> +	igt_paint_color(cr, 0, 0, fb->width, fb->height, 0.0, 0.0, 0.0);
>> +	igt_paint_color(cr, x, y, w, h, v, v, v);
>> +
>> +	igt_put_cairo_ctx(fb->fd, fb, cr);
>> +}
>> +
>> +/*
>> + * Note: This test is display specific in the sense that it requries
>> + * a display that is capable of going above SDR brightness levels.
>> + * Most HDR400 or higher certified displays should be capable of this.
>> + *
>> + * Some displays may require first limiting the output brightness
>> + * in the OSD for this to work.
>> + *
>> + * Requires the luminance sensor to be attached to the test machine,
>> + * if sensor isn't attached to the test machine, test will skip.
>> + */
>> +static void test_static_output(data_t *data, igt_output_t *output)
>> +{
>> +	igt_display_t *display = &data->display;
>> +	struct hdr_output_metadata hdr;
>> +	enum pipe pipe;
>> +	igt_fb_t afb;
>> +	int afb_id;
>> +	double lightup = 100.0;
>> +	double threshold = 15.0;
>> +	double nits_sdr0, nits_sdr1, nits_sdr2, nits_hdr;
>> +
>> +	for_each_pipe(display, pipe) {
>> +		if (!igt_pipe_connector_valid(pipe, output))
>> +			continue;
>> +
>> +		if (!igt_pipe_is_free(display, pipe))
>> +			continue;
>> +
>> +		prepare_test(data, output, pipe);
>> +
>> +		afb_id = igt_create_fb(data->fd, data->w, data->h,
>DRM_FORMAT_XRGB2101010, 0, &afb);
>> +		igt_assert(afb_id);
>> +
>> +		draw_light(&afb, 10000.0);
>> +
>> +		igt_info("Test SDR, 8bpc\n");
>> +		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);
>> +		if (is_amdgpu_device(data->fd))
>> +			assert_output_bpc(data, 8);
>> +
>> +		wait_for_threshold(data, lightup);
>> +		nits_sdr0 = get_sensor_nits(data);
>> +		log_sensor_nits(nits_sdr0);
>> +
>> +		igt_info("Test SDR, 10bpc\n");
>> +		igt_output_set_prop_value(data->output,
>IGT_CONNECTOR_MAX_BPC, 10);
>> +		igt_display_commit_atomic(display,
>DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
>> +		if (is_amdgpu_device(data->fd))
>> +			assert_output_bpc(data, 10);
>> +
>> +		wait_for_threshold(data, lightup);
>> +		nits_sdr1 = get_sensor_nits(data);
>> +		log_sensor_nits(nits_sdr1);
>> +
>> +		igt_info("Test HDR, 10bpc\n");
>> +		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);
>> +
>> +		wait_for_threshold(data, lightup);
>> +		nits_hdr = get_sensor_nits(data);
>> +		log_sensor_nits(nits_hdr);
>> +
>> +		igt_info("Exit HDR into SDR, 8bpc\n");
>> +		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);
>> +		if (is_amdgpu_device(data->fd))
>> +			assert_output_bpc(data, 8);
>> +
>> +		wait_for_threshold(data, lightup);
>> +		nits_sdr2 = get_sensor_nits(data);
>> +		log_sensor_nits(nits_sdr2);
>> +
>> +		/* Verify that all nit values were as expected. */
>> +		assert_nits_equal(nits_sdr0, nits_sdr1, threshold);
>> +		assert_nits_equal(nits_sdr0, nits_sdr2, threshold);
>> +
>> +		igt_assert_f(nits_hdr - nits_sdr0 > threshold * 2.0,
>> +		     "No measurable difference between SDR and HDR luminance: "
>> +		     "threshold=%.1f actual=%.1f\n",
>> +		     threshold * 2.0, nits_hdr - nits_sdr0);
>> +
>> +		test_fini(data);
>> +		igt_remove_fb(data->fd, &afb);
>> +
>> +		break;
>> +	}
>> +}
>> +
>>  /* Returns true if an output supports hdr metadata property */
>>  static bool has_hdr(igt_output_t *output)
>>  {
>> @@ -418,6 +684,8 @@ static void test_hdr(data_t *data, const char *test_name,
>uint32_t flags)
>>  		igt_info("HDR %s test execution on %s\n", test_name, output-
>>name);
>>  		if (flags & TEST_NONE || flags & TEST_DPMS || flags &
>TEST_SUSPEND)
>>  			test_static_toggle(data, output, flags);
>> +		if (flags & TEST_OUTPUT)
>> +			test_static_output(data, output);
>>  		valid_tests++;
>>  	}
>>
>> @@ -453,6 +721,9 @@ igt_main
>>  	igt_describe("Tests static toggle with suspend");
>>  	igt_subtest("static-toggle-suspend") test_hdr(&data, "static-toggle-suspend",
>TEST_SUSPEND);
>>
>> +	igt_describe("Tests HDR mode by setting the static metadata");
>> +	igt_subtest("static-output") test_hdr(&data, "static-output", TEST_OUTPUT);
>> +
>>  	igt_fixture {
>>  		igt_display_fini(&data.display);
>>  	}
>> --
>> 2.24.1
>>
>> _______________________________________________
>> igt-dev mailing list
>> igt-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/igt-dev
>_______________________________________________
>igt-dev mailing list
>igt-dev at lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/igt-dev


More information about the igt-dev mailing list