[i-g-t,v6,1/4] test: Add PSR2 selective update tests

Submitted by Souza, Jose on Feb. 8, 2019, 12:53 a.m.

Details

Message ID 20190208005305.6599-1-jose.souza@intel.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in IGT

Not browsing as part of any series.

Commit Message

Souza, Jose Feb. 8, 2019, 12:53 a.m.
This tests checks if hardware is able to do selective update when
screen changes.
PSR2 don't trigger interruptions and the 'PSR2 SU status' register
is not kept loaded all the times, so it is necessary keep polling
PSR status debugfs until those values are loaded.

Also from DEEP_SLEEP state HW will not do a seletive update, as
most of the memory/context is lost in deep sleep state hardware will
need to exit PSR mode then wait a configured number of frames to
activate PSR again to then start doing seletive updates, that is why
just one screen change is not enough to pass this tests.

When a selective update happens and the values are loaded and read
from debugfs it is compared with the expected value of seletive
update blocks, if matches the polling is stopped and the test passed
otherwise it will wait until it reachs a maximum number o screen
changes to fail the test.

v2: Using new SU blocks debugfs output

v3:
- removed the timerfd to fail the test, now failing based in a
maximum number of screen changes
- removing thread to read debugfs, read from main thread is enough
- improved commit message

v4:
- getting cairo context for frontbuffer test in prepare()
- droppoing poll(), using blocking timerfd instead

v5:
- Doing a modeset before trying to enable PSR2

v6:
- doing atomic commits to fix(legacy commit is taking more time in
recent kernels causing us to miss the SU when reading debugfs) and
speedup test
- fixed code to skip test when PSR2 is not possile

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 lib/igt_psr.c          |  29 ++++
 lib/igt_psr.h          |   1 +
 tests/Makefile.sources |   1 +
 tests/kms_psr2_su.c    | 294 +++++++++++++++++++++++++++++++++++++++++
 tests/meson.build      |   1 +
 5 files changed, 326 insertions(+)
 create mode 100644 tests/kms_psr2_su.c

Patch hide | download patch | download mbox

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index 6ad2c522..b5847bfd 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -178,3 +178,32 @@  bool psr_sink_support(int debugfs_fd, enum psr_mode mode)
 		 */
 		return strstr(buf, "Sink support: yes [0x03]");
 }
+
+#define PSR2_SU_BLOCK_STR_LOOKUP "PSR2 SU blocks:\n0\t"
+
+static bool
+psr2_read_last_num_su_blocks_val(int debugfs_fd, uint16_t *num_su_blocks)
+{
+	char buf[PSR_STATUS_MAX_LEN];
+	char *str;
+	int ret;
+
+	ret = igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
+				      sizeof(buf));
+	if (ret < 0)
+		return false;
+
+	str = strstr(buf, PSR2_SU_BLOCK_STR_LOOKUP);
+	if (!str)
+		return false;
+
+	str = &str[strlen(PSR2_SU_BLOCK_STR_LOOKUP)];
+	*num_su_blocks = (uint16_t)strtol(str, NULL, 10);
+
+	return true;
+}
+
+bool psr2_wait_su(int debugfs_fd, uint16_t *num_su_blocks)
+{
+	return igt_wait(psr2_read_last_num_su_blocks_val(debugfs_fd, num_su_blocks), 40, 1);
+}
diff --git a/lib/igt_psr.h b/lib/igt_psr.h
index 7e7017bf..49599cf8 100644
--- a/lib/igt_psr.h
+++ b/lib/igt_psr.h
@@ -40,5 +40,6 @@  bool psr_wait_update(int debugfs_fd, enum psr_mode mode);
 bool psr_enable(int debugfs_fd, enum psr_mode);
 bool psr_disable(int debugfs_fd);
 bool psr_sink_support(int debugfs_fd, enum psr_mode);
+bool psr2_wait_su(int debugfs_fd, uint16_t *num_su_blocks);
 
 #endif
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index a234fa5d..d2c4f9fe 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -62,6 +62,7 @@  TESTS_progs = \
 	kms_plane_scaling \
 	kms_properties \
 	kms_psr \
+	kms_psr2_su \
 	kms_pwrite_crc \
 	kms_rmfb \
 	kms_rotation_crc \
diff --git a/tests/kms_psr2_su.c b/tests/kms_psr2_su.c
new file mode 100644
index 00000000..2c75b85b
--- /dev/null
+++ b/tests/kms_psr2_su.c
@@ -0,0 +1,294 @@ 
+/*
+ * Copyright © 2019 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include "igt.h"
+#include "igt_sysfs.h"
+#include "igt_psr.h"
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/timerfd.h>
+#include "intel_bufmgr.h"
+
+IGT_TEST_DESCRIPTION("Test PSR2 selective update");
+
+#define SQUARE_SIZE 100
+/* each selective update block is 4 lines tall */
+#define EXPECTED_NUM_SU_BLOCKS ((SQUARE_SIZE / 4) + (SQUARE_SIZE % 4 ? 1 : 0))
+
+/*
+ * Minimum is 15 as the number of frames to active PSR2 could be configured
+ * to 15 frames plus a few more in case we miss a selective update between
+ * debugfs reads.
+ */
+#define MAX_SCREEN_CHANGES 20
+
+enum operations {
+	PAGE_FLIP,
+	FRONTBUFFER,
+	LAST
+};
+
+static const char *op_str(enum operations op)
+{
+	static const char * const name[] = {
+		[PAGE_FLIP] = "page_flip",
+		[FRONTBUFFER] = "frontbuffer"
+	};
+
+	return name[op];
+}
+
+typedef struct {
+	int drm_fd;
+	int debugfs_fd;
+	igt_display_t display;
+	drm_intel_bufmgr *bufmgr;
+	drmModeModeInfo *mode;
+	igt_output_t *output;
+	struct igt_fb fb[2];
+	enum operations op;
+	cairo_t *cr;
+	int change_screen_timerfd;
+	uint32_t screen_changes;
+} data_t;
+
+static void setup_output(data_t *data)
+{
+	igt_display_t *display = &data->display;
+	igt_output_t *output;
+	enum pipe pipe;
+
+	for_each_pipe_with_valid_output(display, pipe, output) {
+		drmModeConnectorPtr c = output->config.connector;
+
+		if (c->connector_type != DRM_MODE_CONNECTOR_eDP)
+			continue;
+
+		igt_output_set_pipe(output, pipe);
+		data->output = output;
+		data->mode = igt_output_get_mode(output);
+
+		return;
+	}
+}
+
+static void display_init(data_t *data)
+{
+	igt_display_require(&data->display, data->drm_fd);
+	setup_output(data);
+}
+
+static void display_fini(data_t *data)
+{
+	igt_display_fini(&data->display);
+}
+
+static void prepare(data_t *data)
+{
+	igt_plane_t *primary;
+
+	/* all green frame */
+	igt_create_color_fb(data->drm_fd,
+			    data->mode->hdisplay, data->mode->vdisplay,
+			    DRM_FORMAT_XRGB8888,
+			    LOCAL_DRM_FORMAT_MOD_NONE,
+			    0.0, 1.0, 0.0,
+			    &data->fb[0]);
+
+	if (data->op == PAGE_FLIP) {
+		cairo_t *cr;
+
+		igt_create_color_fb(data->drm_fd,
+				    data->mode->hdisplay, data->mode->vdisplay,
+				    DRM_FORMAT_XRGB8888,
+				    LOCAL_DRM_FORMAT_MOD_NONE,
+				    0.0, 1.0, 0.0,
+				    &data->fb[1]);
+
+		cr = igt_get_cairo_ctx(data->drm_fd, &data->fb[1]);
+		/* paint a white square */
+		igt_paint_color_alpha(cr, 0, 0, SQUARE_SIZE, SQUARE_SIZE,
+				      1.0, 1.0, 1.0, 1.0);
+		igt_put_cairo_ctx(data->drm_fd,  &data->fb[1], cr);
+	} else if (data->op == FRONTBUFFER) {
+		data->cr = igt_get_cairo_ctx(data->drm_fd, &data->fb[0]);
+	}
+
+	primary = igt_output_get_plane_type(data->output,
+					    DRM_PLANE_TYPE_PRIMARY);
+
+	igt_plane_set_fb(primary, &data->fb[0]);
+	igt_display_commit2(&data->display, COMMIT_ATOMIC);
+
+	data->screen_changes = 0;
+}
+
+static bool update_screen_and_test(data_t *data)
+{
+	uint16_t su_blocks;
+	bool ret = false;
+
+	data->screen_changes++;
+
+	switch (data->op) {
+	case PAGE_FLIP: {
+		igt_plane_t *primary;
+
+		primary = igt_output_get_plane_type(data->output,
+						    DRM_PLANE_TYPE_PRIMARY);
+
+		igt_plane_set_fb(primary, &data->fb[data->screen_changes & 1]);
+		igt_display_commit2(&data->display, COMMIT_ATOMIC);
+		break;
+	}
+	case FRONTBUFFER: {
+		drmModeClip clip;
+
+		clip.x1 = clip.y1 = 0;
+		clip.x2 = clip.y2 = SQUARE_SIZE;
+
+		if (data->screen_changes & 1) {
+			/* go back to all green frame with a square */
+			igt_paint_color_alpha(data->cr, 0, 0, SQUARE_SIZE,
+					      SQUARE_SIZE, 1.0, 1.0, 1.0, 1.0);
+		} else {
+			/* go back to all green frame */
+			igt_paint_color_alpha(data->cr, 0, 0, SQUARE_SIZE,
+					      SQUARE_SIZE, 0, 1.0, 0, 1.0);
+		}
+
+		drmModeDirtyFB(data->drm_fd, data->fb[0].fb_id, &clip, 1);
+		break;
+	}
+	default:
+		igt_assert_f(data->op, "Operation not handled\n");
+	}
+
+	if (psr2_wait_su(data->debugfs_fd, &su_blocks))
+		ret = su_blocks == EXPECTED_NUM_SU_BLOCKS;
+
+	return ret;
+}
+
+static void run(data_t *data)
+{
+	bool result = false;
+
+	igt_assert(psr_wait_entry(data->debugfs_fd, PSR_MODE_2));
+
+	while (data->screen_changes < MAX_SCREEN_CHANGES && !result) {
+		uint64_t exp;
+		int r;
+
+		r = read(data->change_screen_timerfd, &exp, sizeof(exp));
+		if (r == sizeof(uint64_t) && exp)
+			result = update_screen_and_test(data);
+	}
+
+	igt_debug("Screen changes: %u\n", data->screen_changes);
+	igt_assert_f(result,
+		     "No matching selective update blocks read from debugfs\n");
+}
+
+static void cleanup(data_t *data)
+{
+	igt_plane_t *primary;
+
+	primary = igt_output_get_plane_type(data->output,
+					    DRM_PLANE_TYPE_PRIMARY);
+	igt_plane_set_fb(primary, NULL);
+	igt_display_commit2(&data->display, COMMIT_ATOMIC);
+
+	if (data->op == PAGE_FLIP)
+		igt_remove_fb(data->drm_fd, &data->fb[1]);
+	else if (data->op == FRONTBUFFER)
+		igt_put_cairo_ctx(data->drm_fd, &data->fb[0], data->cr);
+
+	igt_remove_fb(data->drm_fd, &data->fb[0]);
+}
+
+int main(int argc, char *argv[])
+{
+	data_t data = {};
+
+	igt_subtest_init_parse_opts(&argc, argv, "", NULL,
+				    NULL, NULL, NULL);
+	igt_skip_on_simulation();
+
+	igt_fixture {
+		struct itimerspec interval;
+		int r;
+
+		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
+		data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
+		kmstest_set_vt_graphics_mode();
+
+		igt_require_f(psr_sink_support(data.debugfs_fd, PSR_MODE_2),
+			      "Sink does not support PSR2\n");
+
+		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
+		igt_assert(data.bufmgr);
+		drm_intel_bufmgr_gem_enable_reuse(data.bufmgr);
+
+		display_init(&data);
+
+		/* Test if PSR2 can be enabled */
+		igt_require_f(psr_enable(data.debugfs_fd, PSR_MODE_2),
+			      "Error enabling PSR2\n");
+		data.op = FRONTBUFFER;
+		prepare(&data);
+		r = psr_wait_entry(data.debugfs_fd, PSR_MODE_2);
+		cleanup(&data);
+		igt_require_f(r, "PSR2 can not be enabled\n");
+
+		/* blocking timerfd */
+		data.change_screen_timerfd = timerfd_create(CLOCK_MONOTONIC, 0);
+		igt_require(data.change_screen_timerfd != -1);
+		/* Changing screen at 30hz to support 30hz panels */
+		interval.it_value.tv_nsec = NSEC_PER_SEC / 30;
+		interval.it_value.tv_sec = 0;
+		interval.it_interval.tv_nsec = interval.it_value.tv_nsec;
+		interval.it_interval.tv_sec = interval.it_value.tv_sec;
+		r = timerfd_settime(data.change_screen_timerfd, 0, &interval, NULL);
+		igt_require_f(r != -1, "Error setting timerfd\n");
+	}
+
+	for (data.op = PAGE_FLIP; data.op < LAST; data.op++) {
+		igt_subtest_f("%s", op_str(data.op)) {
+			prepare(&data);
+			run(&data);
+			cleanup(&data);
+		}
+	}
+
+	igt_fixture {
+		close(data.debugfs_fd);
+		drm_intel_bufmgr_destroy(data.bufmgr);
+		display_fini(&data);
+	}
+
+	igt_exit();
+}
diff --git a/tests/meson.build b/tests/meson.build
index 0f12df26..ec980651 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -49,6 +49,7 @@  test_progs = [
 	'kms_plane_scaling',
 	'kms_properties',
 	'kms_psr',
+	'kms_psr2_su',
 	'kms_pwrite_crc',
 	'kms_rmfb',
 	'kms_rotation_crc',

Comments

On Thu, 2019-02-07 at 16:53 -0800, José Roberto de Souza wrote:
> This tests checks if hardware is able to do selective update when
> screen changes.
> PSR2 don't trigger interruptions and the 'PSR2 SU status' register
> is not kept loaded all the times, so it is necessary keep polling
> PSR status debugfs until those values are loaded.
> 
> Also from DEEP_SLEEP state HW will not do a seletive update, as
> most of the memory/context is lost in deep sleep state hardware will
> need to exit PSR mode then wait a configured number of frames to
> activate PSR again to then start doing seletive updates, that is why
> just one screen change is not enough to pass this tests.
> 
> When a selective update happens and the values are loaded and read
> from debugfs it is compared with the expected value of seletive
> update blocks, if matches the polling is stopped and the test passed
> otherwise it will wait until it reachs a maximum number o screen
> changes to fail the test.
> 
> v2: Using new SU blocks debugfs output
> 
> v3:
> - removed the timerfd to fail the test, now failing based in a
> maximum number of screen changes
> - removing thread to read debugfs, read from main thread is enough
> - improved commit message
> 
> v4:
> - getting cairo context for frontbuffer test in prepare()
> - droppoing poll(), using blocking timerfd instead
> 
> v5:
> - Doing a modeset before trying to enable PSR2
> 
> v6:
> - doing atomic commits to fix(legacy commit is taking more time in
> recent kernels causing us to miss the SU when reading debugfs) and
> speedup test
> - fixed code to skip test when PSR2 is not possile

Thanks for making these changes.

> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  lib/igt_psr.c          |  29 ++++
>  lib/igt_psr.h          |   1 +
>  tests/Makefile.sources |   1 +
>  tests/kms_psr2_su.c    | 294
> +++++++++++++++++++++++++++++++++++++++++
>  tests/meson.build      |   1 +
>  5 files changed, 326 insertions(+)
>  create mode 100644 tests/kms_psr2_su.c
> 
> diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> index 6ad2c522..b5847bfd 100644
> --- a/lib/igt_psr.c
> +++ b/lib/igt_psr.c
> @@ -178,3 +178,32 @@ bool psr_sink_support(int debugfs_fd, enum
> psr_mode mode)
>  		 */
>  		return strstr(buf, "Sink support: yes [0x03]");
>  }
> +
> +#define PSR2_SU_BLOCK_STR_LOOKUP "PSR2 SU blocks:\n0\t"
> +
> +static bool
> +psr2_read_last_num_su_blocks_val(int debugfs_fd, uint16_t
> *num_su_blocks)
> +{
> +	char buf[PSR_STATUS_MAX_LEN];
> +	char *str;
> +	int ret;
> +
> +	ret = igt_debugfs_simple_read(debugfs_fd,
> "i915_edp_psr_status", buf,
> +				      sizeof(buf));
> +	if (ret < 0)
> +		return false;
> +
> +	str = strstr(buf, PSR2_SU_BLOCK_STR_LOOKUP);
> +	if (!str)
> +		return false;
> +
> +	str = &str[strlen(PSR2_SU_BLOCK_STR_LOOKUP)];
> +	*num_su_blocks = (uint16_t)strtol(str, NULL, 10);
> +
> +	return true;
> +}
> +
> +bool psr2_wait_su(int debugfs_fd, uint16_t *num_su_blocks)
> +{
> +	return igt_wait(psr2_read_last_num_su_blocks_val(debugfs_fd,
> num_su_blocks), 40, 1);
> +}
> diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> index 7e7017bf..49599cf8 100644
> --- a/lib/igt_psr.h
> +++ b/lib/igt_psr.h
> @@ -40,5 +40,6 @@ bool psr_wait_update(int debugfs_fd, enum psr_mode
> mode);
>  bool psr_enable(int debugfs_fd, enum psr_mode);
>  bool psr_disable(int debugfs_fd);
>  bool psr_sink_support(int debugfs_fd, enum psr_mode);
> +bool psr2_wait_su(int debugfs_fd, uint16_t *num_su_blocks);
>  
>  #endif
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index a234fa5d..d2c4f9fe 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -62,6 +62,7 @@ TESTS_progs = \
>  	kms_plane_scaling \
>  	kms_properties \
>  	kms_psr \
> +	kms_psr2_su \
>  	kms_pwrite_crc \
>  	kms_rmfb \
>  	kms_rotation_crc \
> diff --git a/tests/kms_psr2_su.c b/tests/kms_psr2_su.c
> new file mode 100644
> index 00000000..2c75b85b
> --- /dev/null
> +++ b/tests/kms_psr2_su.c
> @@ -0,0 +1,294 @@
> +/*
> + * Copyright © 2019 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person
> obtaining a
> + * copy of this software and associated documentation files (the
> "Software"),
> + * to deal in the Software without restriction, including without
> limitation
> + * the rights to use, copy, modify, merge, publish, distribute,
> sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom
> the
> + * Software is furnished to do so, subject to the following
> conditions:
> + *
> + * The above copyright notice and this permission notice (including
> the next
> + * paragraph) shall be included in all copies or substantial
> portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO
> EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
> OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include "igt.h"
> +#include "igt_sysfs.h"
> +#include "igt_psr.h"
> +#include <errno.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <sys/timerfd.h>
> +#include "intel_bufmgr.h"
> +
> +IGT_TEST_DESCRIPTION("Test PSR2 selective update");
> +
> +#define SQUARE_SIZE 100
> +/* each selective update block is 4 lines tall */
> +#define EXPECTED_NUM_SU_BLOCKS ((SQUARE_SIZE / 4) + (SQUARE_SIZE % 4
> ? 1 : 0))
> +
> +/*
> + * Minimum is 15 as the number of frames to active PSR2 could be
> configured
> + * to 15 frames plus a few more in case we miss a selective update
> between
> + * debugfs reads.
> + */
> +#define MAX_SCREEN_CHANGES 20
> +
> +enum operations {
> +	PAGE_FLIP,
> +	FRONTBUFFER,
> +	LAST
Something like "INVALID" would have made it more obvious?


> +};
> +
> +static const char *op_str(enum operations op)
> +{
> +	static const char * const name[] = {
> +		[PAGE_FLIP] = "page_flip",
> +		[FRONTBUFFER] = "frontbuffer"
> +	};
> +
> +	return name[op];
> +}
> +
> +typedef struct {
> +	int drm_fd;
> +	int debugfs_fd;
> +	igt_display_t display;
> +	drm_intel_bufmgr *bufmgr;
> +	drmModeModeInfo *mode;
> +	igt_output_t *output;
> +	struct igt_fb fb[2];
> +	enum operations op;
> +	cairo_t *cr;
> +	int change_screen_timerfd;
> +	uint32_t screen_changes;
> +} data_t;
> +
> +static void setup_output(data_t *data)
> +{
> +	igt_display_t *display = &data->display;
> +	igt_output_t *output;
> +	enum pipe pipe;
> +
> +	for_each_pipe_with_valid_output(display, pipe, output) {
> +		drmModeConnectorPtr c = output->config.connector;
> +
> +		if (c->connector_type != DRM_MODE_CONNECTOR_eDP)
> +			continue;
> +
> +		igt_output_set_pipe(output, pipe);
> +		data->output = output;
> +		data->mode = igt_output_get_mode(output);
> +
> +		return;
> +	}
> +}
> +
> +static void display_init(data_t *data)
> +{
> +	igt_display_require(&data->display, data->drm_fd);
> +	setup_output(data);
> +}
> +
> +static void display_fini(data_t *data)
> +{
> +	igt_display_fini(&data->display);
> +}
> +
> +static void prepare(data_t *data)
> +{
> +	igt_plane_t *primary;
> +
> +	/* all green frame */
> +	igt_create_color_fb(data->drm_fd,
> +			    data->mode->hdisplay, data->mode->vdisplay,
> +			    DRM_FORMAT_XRGB8888,
> +			    LOCAL_DRM_FORMAT_MOD_NONE,
> +			    0.0, 1.0, 0.0,
> +			    &data->fb[0]);
> +
> +	if (data->op == PAGE_FLIP) {
> +		cairo_t *cr;
> +
> +		igt_create_color_fb(data->drm_fd,
> +				    data->mode->hdisplay, data->mode-
> >vdisplay,
> +				    DRM_FORMAT_XRGB8888,
> +				    LOCAL_DRM_FORMAT_MOD_NONE,
> +				    0.0, 1.0, 0.0,
> +				    &data->fb[1]);
> +
> +		cr = igt_get_cairo_ctx(data->drm_fd, &data->fb[1]);
> +		/* paint a white square */
> +		igt_paint_color_alpha(cr, 0, 0, SQUARE_SIZE,
> SQUARE_SIZE,
> +				      1.0, 1.0, 1.0, 1.0);
> +		igt_put_cairo_ctx(data->drm_fd,  &data->fb[1], cr);
> +	} else if (data->op == FRONTBUFFER) {
> +		data->cr = igt_get_cairo_ctx(data->drm_fd, &data-
> >fb[0]);
> +	}
> +
> +	primary = igt_output_get_plane_type(data->output,
> +					    DRM_PLANE_TYPE_PRIMARY);
> +
> +	igt_plane_set_fb(primary, &data->fb[0]);
> +	igt_display_commit2(&data->display, COMMIT_ATOMIC);
> +
> +	data->screen_changes = 0;
> +}
> +
> +static bool update_screen_and_test(data_t *data)
> +{
> +	uint16_t su_blocks;
> +	bool ret = false;
> +
> +	data->screen_changes++;
> +
> +	switch (data->op) {
> +	case PAGE_FLIP: {
> +		igt_plane_t *primary;
> +
> +		primary = igt_output_get_plane_type(data->output,
> +						    DRM_PLANE_TYPE_PRIM
> ARY);
> +
> +		igt_plane_set_fb(primary, &data->fb[data-
> >screen_changes & 1]);
> +		igt_display_commit2(&data->display, COMMIT_ATOMIC);
> +		break;
> +	}
> +	case FRONTBUFFER: {
> +		drmModeClip clip;
> +
> +		clip.x1 = clip.y1 = 0;
> +		clip.x2 = clip.y2 = SQUARE_SIZE;
> +
> +		if (data->screen_changes & 1) {
> +			/* go back to all green frame with a square */
> +			igt_paint_color_alpha(data->cr, 0, 0,
> SQUARE_SIZE,
> +					      SQUARE_SIZE, 1.0, 1.0,
> 1.0, 1.0);
> +		} else {
> +			/* go back to all green frame */
> +			igt_paint_color_alpha(data->cr, 0, 0,
> SQUARE_SIZE,
> +					      SQUARE_SIZE, 0, 1.0, 0,
> 1.0);
> +		}
> +
> +		drmModeDirtyFB(data->drm_fd, data->fb[0].fb_id, &clip,
> 1);
> +		break;
> +	}
> +	default:
> +		igt_assert_f(data->op, "Operation not handled\n");
> +	}
> +
> +	if (psr2_wait_su(data->debugfs_fd, &su_blocks))
> +		ret = su_blocks == EXPECTED_NUM_SU_BLOCKS;
> +
> +	return ret;
> +}
> +
> +static void run(data_t *data)
> +{
> +	bool result = false;
> +
> +	igt_assert(psr_wait_entry(data->debugfs_fd, PSR_MODE_2));
> +
> +	while (data->screen_changes < MAX_SCREEN_CHANGES && !result) {

nitpick: Updating data->screen_changes here would have made it clear
that we increment it every iteration.

With or without above changes, 
Reviewed-by: Dhinakaran Pandiyan <
dhinakaran.pandiyan@intel.com>
On ICL, 
Tested-by: Dhinakaran Pandiyan <
dhinakaran.pandiyan@intel.com>

> +		uint64_t exp;
> +		int r;
> +
> +		r = read(data->change_screen_timerfd, &exp,
> sizeof(exp));
> +		if (r == sizeof(uint64_t) && exp)
> +			result = update_screen_and_test(data);
> +	}
> +
> +	igt_debug("Screen changes: %u\n", data->screen_changes);
> +	igt_assert_f(result,
> +		     "No matching selective update blocks read from
> debugfs\n");
> +}
> +
> +static void cleanup(data_t *data)
> +{
> +	igt_plane_t *primary;
> +
> +	primary = igt_output_get_plane_type(data->output,
> +					    DRM_PLANE_TYPE_PRIMARY);
> +	igt_plane_set_fb(primary, NULL);
> +	igt_display_commit2(&data->display, COMMIT_ATOMIC);
> +
> +	if (data->op == PAGE_FLIP)
> +		igt_remove_fb(data->drm_fd, &data->fb[1]);
> +	else if (data->op == FRONTBUFFER)
> +		igt_put_cairo_ctx(data->drm_fd, &data->fb[0], data-
> >cr);
> +
> +	igt_remove_fb(data->drm_fd, &data->fb[0]);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	data_t data = {};
> +
> +	igt_subtest_init_parse_opts(&argc, argv, "", NULL,
> +				    NULL, NULL, NULL);
> +	igt_skip_on_simulation();
> +
> +	igt_fixture {
> +		struct itimerspec interval;
> +		int r;
> +
> +		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
> +		data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
> +		kmstest_set_vt_graphics_mode();
> +
> +		igt_require_f(psr_sink_support(data.debugfs_fd,
> PSR_MODE_2),
> +			      "Sink does not support PSR2\n");
> +
> +		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd,
> 4096);
> +		igt_assert(data.bufmgr);
> +		drm_intel_bufmgr_gem_enable_reuse(data.bufmgr);
> +
> +		display_init(&data);
> +
> +		/* Test if PSR2 can be enabled */
> +		igt_require_f(psr_enable(data.debugfs_fd, PSR_MODE_2),
> +			      "Error enabling PSR2\n");
> +		data.op = FRONTBUFFER;
> +		prepare(&data);
> +		r = psr_wait_entry(data.debugfs_fd, PSR_MODE_2);
I would rename this "r" to something more intuitive as assertions get
debug logged.



> +		cleanup(&data);
> +		igt_require_f(r, "PSR2 can not be enabled\n");
> +
> +		/* blocking timerfd */
> +		data.change_screen_timerfd =
> timerfd_create(CLOCK_MONOTONIC, 0);
> +		igt_require(data.change_screen_timerfd != -1);
> +		/* Changing screen at 30hz to support 30hz panels */
> +		interval.it_value.tv_nsec = NSEC_PER_SEC / 30;
> +		interval.it_value.tv_sec = 0;
> +		interval.it_interval.tv_nsec =
> interval.it_value.tv_nsec;
> +		interval.it_interval.tv_sec = interval.it_value.tv_sec;
> +		r = timerfd_settime(data.change_screen_timerfd, 0,
> &interval, NULL);
> +		igt_require_f(r != -1, "Error setting timerfd\n");
> +	}
> +
> +	for (data.op = PAGE_FLIP; data.op < LAST; data.op++) {
> +		igt_subtest_f("%s", op_str(data.op)) {
> +			prepare(&data);
> +			run(&data);
> +			cleanup(&data);
> +		}
> +	}
> +
> +	igt_fixture {
> +		close(data.debugfs_fd);
> +		drm_intel_bufmgr_destroy(data.bufmgr);
> +		display_fini(&data);
> +	}
> +
> +	igt_exit();
> +}
> diff --git a/tests/meson.build b/tests/meson.build
> index 0f12df26..ec980651 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -49,6 +49,7 @@ test_progs = [
>  	'kms_plane_scaling',
>  	'kms_properties',
>  	'kms_psr',
> +	'kms_psr2_su',
>  	'kms_pwrite_crc',
>  	'kms_rmfb',
>  	'kms_rotation_crc',
On Wed, 2019-02-13 at 16:21 -0800, Dhinakaran Pandiyan wrote:
> On Thu, 2019-02-07 at 16:53 -0800, José Roberto de Souza wrote:
> > This tests checks if hardware is able to do selective update when
> > screen changes.
> > PSR2 don't trigger interruptions and the 'PSR2 SU status' register
> > is not kept loaded all the times, so it is necessary keep polling
> > PSR status debugfs until those values are loaded.
> > 
> > Also from DEEP_SLEEP state HW will not do a seletive update, as
> > most of the memory/context is lost in deep sleep state hardware
> > will
> > need to exit PSR mode then wait a configured number of frames to
> > activate PSR again to then start doing seletive updates, that is
> > why
> > just one screen change is not enough to pass this tests.
> > 
> > When a selective update happens and the values are loaded and read
> > from debugfs it is compared with the expected value of seletive
> > update blocks, if matches the polling is stopped and the test
> > passed
> > otherwise it will wait until it reachs a maximum number o screen
> > changes to fail the test.
> > 
> > v2: Using new SU blocks debugfs output
> > 
> > v3:
> > - removed the timerfd to fail the test, now failing based in a
> > maximum number of screen changes
> > - removing thread to read debugfs, read from main thread is enough
> > - improved commit message
> > 
> > v4:
> > - getting cairo context for frontbuffer test in prepare()
> > - droppoing poll(), using blocking timerfd instead
> > 
> > v5:
> > - Doing a modeset before trying to enable PSR2
> > 
> > v6:
> > - doing atomic commits to fix(legacy commit is taking more time in
> > recent kernels causing us to miss the SU when reading debugfs) and
> > speedup test
> > - fixed code to skip test when PSR2 is not possile
> 
> Thanks for making these changes.
> 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  lib/igt_psr.c          |  29 ++++
> >  lib/igt_psr.h          |   1 +
> >  tests/Makefile.sources |   1 +
> >  tests/kms_psr2_su.c    | 294
> > +++++++++++++++++++++++++++++++++++++++++
> >  tests/meson.build      |   1 +
> >  5 files changed, 326 insertions(+)
> >  create mode 100644 tests/kms_psr2_su.c
> > 
> > diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> > index 6ad2c522..b5847bfd 100644
> > --- a/lib/igt_psr.c
> > +++ b/lib/igt_psr.c
> > @@ -178,3 +178,32 @@ bool psr_sink_support(int debugfs_fd, enum
> > psr_mode mode)
> >  		 */
> >  		return strstr(buf, "Sink support: yes [0x03]");
> >  }
> > +
> > +#define PSR2_SU_BLOCK_STR_LOOKUP "PSR2 SU blocks:\n0\t"
> > +
> > +static bool
> > +psr2_read_last_num_su_blocks_val(int debugfs_fd, uint16_t
> > *num_su_blocks)
> > +{
> > +	char buf[PSR_STATUS_MAX_LEN];
> > +	char *str;
> > +	int ret;
> > +
> > +	ret = igt_debugfs_simple_read(debugfs_fd,
> > "i915_edp_psr_status", buf,
> > +				      sizeof(buf));
> > +	if (ret < 0)
> > +		return false;
> > +
> > +	str = strstr(buf, PSR2_SU_BLOCK_STR_LOOKUP);
> > +	if (!str)
> > +		return false;
> > +
> > +	str = &str[strlen(PSR2_SU_BLOCK_STR_LOOKUP)];
> > +	*num_su_blocks = (uint16_t)strtol(str, NULL, 10);
> > +
> > +	return true;
> > +}
> > +
> > +bool psr2_wait_su(int debugfs_fd, uint16_t *num_su_blocks)
> > +{
> > +	return igt_wait(psr2_read_last_num_su_blocks_val(debugfs_fd,
> > num_su_blocks), 40, 1);
> > +}
> > diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> > index 7e7017bf..49599cf8 100644
> > --- a/lib/igt_psr.h
> > +++ b/lib/igt_psr.h
> > @@ -40,5 +40,6 @@ bool psr_wait_update(int debugfs_fd, enum
> > psr_mode
> > mode);
> >  bool psr_enable(int debugfs_fd, enum psr_mode);
> >  bool psr_disable(int debugfs_fd);
> >  bool psr_sink_support(int debugfs_fd, enum psr_mode);
> > +bool psr2_wait_su(int debugfs_fd, uint16_t *num_su_blocks);
> >  
> >  #endif
> > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > index a234fa5d..d2c4f9fe 100644
> > --- a/tests/Makefile.sources
> > +++ b/tests/Makefile.sources
> > @@ -62,6 +62,7 @@ TESTS_progs = \
> >  	kms_plane_scaling \
> >  	kms_properties \
> >  	kms_psr \
> > +	kms_psr2_su \
> >  	kms_pwrite_crc \
> >  	kms_rmfb \
> >  	kms_rotation_crc \
> > diff --git a/tests/kms_psr2_su.c b/tests/kms_psr2_su.c
> > new file mode 100644
> > index 00000000..2c75b85b
> > --- /dev/null
> > +++ b/tests/kms_psr2_su.c
> > @@ -0,0 +1,294 @@
> > +/*
> > + * Copyright © 2019 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person
> > obtaining a
> > + * copy of this software and associated documentation files (the
> > "Software"),
> > + * to deal in the Software without restriction, including without
> > limitation
> > + * the rights to use, copy, modify, merge, publish, distribute,
> > sublicense,
> > + * and/or sell copies of the Software, and to permit persons to
> > whom
> > the
> > + * Software is furnished to do so, subject to the following
> > conditions:
> > + *
> > + * The above copyright notice and this permission notice
> > (including
> > the next
> > + * paragraph) shall be included in all copies or substantial
> > portions of the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO
> > EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> > DAMAGES
> > OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> > OTHER DEALINGS
> > + * IN THE SOFTWARE.
> > + *
> > + */
> > +
> > +#include "igt.h"
> > +#include "igt_sysfs.h"
> > +#include "igt_psr.h"
> > +#include <errno.h>
> > +#include <stdbool.h>
> > +#include <stdio.h>
> > +#include <string.h>
> > +#include <sys/timerfd.h>
> > +#include "intel_bufmgr.h"
> > +
> > +IGT_TEST_DESCRIPTION("Test PSR2 selective update");
> > +
> > +#define SQUARE_SIZE 100
> > +/* each selective update block is 4 lines tall */
> > +#define EXPECTED_NUM_SU_BLOCKS ((SQUARE_SIZE / 4) + (SQUARE_SIZE %
> > 4
> > ? 1 : 0))
> > +
> > +/*
> > + * Minimum is 15 as the number of frames to active PSR2 could be
> > configured
> > + * to 15 frames plus a few more in case we miss a selective update
> > between
> > + * debugfs reads.
> > + */
> > +#define MAX_SCREEN_CHANGES 20
> > +
> > +enum operations {
> > +	PAGE_FLIP,
> > +	FRONTBUFFER,
> > +	LAST
> Something like "INVALID" would have made it more obvious?
> 
> 
> > +};
> > +
> > +static const char *op_str(enum operations op)
> > +{
> > +	static const char * const name[] = {
> > +		[PAGE_FLIP] = "page_flip",
> > +		[FRONTBUFFER] = "frontbuffer"
> > +	};
> > +
> > +	return name[op];
> > +}
> > +
> > +typedef struct {
> > +	int drm_fd;
> > +	int debugfs_fd;
> > +	igt_display_t display;
> > +	drm_intel_bufmgr *bufmgr;
> > +	drmModeModeInfo *mode;
> > +	igt_output_t *output;
> > +	struct igt_fb fb[2];
> > +	enum operations op;
> > +	cairo_t *cr;
> > +	int change_screen_timerfd;
> > +	uint32_t screen_changes;
> > +} data_t;
> > +
> > +static void setup_output(data_t *data)
> > +{
> > +	igt_display_t *display = &data->display;
> > +	igt_output_t *output;
> > +	enum pipe pipe;
> > +
> > +	for_each_pipe_with_valid_output(display, pipe, output) {
> > +		drmModeConnectorPtr c = output->config.connector;
> > +
> > +		if (c->connector_type != DRM_MODE_CONNECTOR_eDP)
> > +			continue;
> > +
> > +		igt_output_set_pipe(output, pipe);
> > +		data->output = output;
> > +		data->mode = igt_output_get_mode(output);
> > +
> > +		return;
> > +	}
> > +}
> > +
> > +static void display_init(data_t *data)
> > +{
> > +	igt_display_require(&data->display, data->drm_fd);
> > +	setup_output(data);
> > +}
> > +
> > +static void display_fini(data_t *data)
> > +{
> > +	igt_display_fini(&data->display);
> > +}
> > +
> > +static void prepare(data_t *data)
> > +{
> > +	igt_plane_t *primary;
> > +
> > +	/* all green frame */
> > +	igt_create_color_fb(data->drm_fd,
> > +			    data->mode->hdisplay, data->mode->vdisplay,
> > +			    DRM_FORMAT_XRGB8888,
> > +			    LOCAL_DRM_FORMAT_MOD_NONE,
> > +			    0.0, 1.0, 0.0,
> > +			    &data->fb[0]);
> > +
> > +	if (data->op == PAGE_FLIP) {
> > +		cairo_t *cr;
> > +
> > +		igt_create_color_fb(data->drm_fd,
> > +				    data->mode->hdisplay, data->mode-
> > > vdisplay,
> > +				    DRM_FORMAT_XRGB8888,
> > +				    LOCAL_DRM_FORMAT_MOD_NONE,
> > +				    0.0, 1.0, 0.0,
> > +				    &data->fb[1]);
> > +
> > +		cr = igt_get_cairo_ctx(data->drm_fd, &data->fb[1]);
> > +		/* paint a white square */
> > +		igt_paint_color_alpha(cr, 0, 0, SQUARE_SIZE,
> > SQUARE_SIZE,
> > +				      1.0, 1.0, 1.0, 1.0);
> > +		igt_put_cairo_ctx(data->drm_fd,  &data->fb[1], cr);
> > +	} else if (data->op == FRONTBUFFER) {
> > +		data->cr = igt_get_cairo_ctx(data->drm_fd, &data-
> > > fb[0]);
> > +	}
> > +
> > +	primary = igt_output_get_plane_type(data->output,
> > +					    DRM_PLANE_TYPE_PRIMARY);
> > +
> > +	igt_plane_set_fb(primary, &data->fb[0]);
> > +	igt_display_commit2(&data->display, COMMIT_ATOMIC);
> > +
> > +	data->screen_changes = 0;
> > +}
> > +
> > +static bool update_screen_and_test(data_t *data)
> > +{
> > +	uint16_t su_blocks;
> > +	bool ret = false;
> > +
> > +	data->screen_changes++;
> > +
> > +	switch (data->op) {
> > +	case PAGE_FLIP: {
> > +		igt_plane_t *primary;
> > +
> > +		primary = igt_output_get_plane_type(data->output,
> > +						    DRM_PLANE_TYPE_PRIM
> > ARY);
> > +
> > +		igt_plane_set_fb(primary, &data->fb[data-
> > > screen_changes & 1]);
> > +		igt_display_commit2(&data->display, COMMIT_ATOMIC);
> > +		break;
> > +	}
> > +	case FRONTBUFFER: {
> > +		drmModeClip clip;
> > +
> > +		clip.x1 = clip.y1 = 0;
> > +		clip.x2 = clip.y2 = SQUARE_SIZE;
> > +
> > +		if (data->screen_changes & 1) {
> > +			/* go back to all green frame with a square */
> > +			igt_paint_color_alpha(data->cr, 0, 0,
> > SQUARE_SIZE,
> > +					      SQUARE_SIZE, 1.0, 1.0,
> > 1.0, 1.0);
> > +		} else {
> > +			/* go back to all green frame */
> > +			igt_paint_color_alpha(data->cr, 0, 0,
> > SQUARE_SIZE,
> > +					      SQUARE_SIZE, 0, 1.0, 0,
> > 1.0);
> > +		}
> > +
> > +		drmModeDirtyFB(data->drm_fd, data->fb[0].fb_id, &clip,
> > 1);
> > +		break;
> > +	}
> > +	default:
> > +		igt_assert_f(data->op, "Operation not handled\n");
> > +	}
> > +
> > +	if (psr2_wait_su(data->debugfs_fd, &su_blocks))
> > +		ret = su_blocks == EXPECTED_NUM_SU_BLOCKS;
> > +
> > +	return ret;
> > +}
> > +
> > +static void run(data_t *data)
> > +{
> > +	bool result = false;
> > +
> > +	igt_assert(psr_wait_entry(data->debugfs_fd, PSR_MODE_2));
> > +
> > +	while (data->screen_changes < MAX_SCREEN_CHANGES && !result) {
> 
> nitpick: Updating data->screen_changes here would have made it clear
> that we increment it every iteration.
> 
> With or without above changes, 
> Reviewed-by: Dhinakaran Pandiyan <
> dhinakaran.pandiyan@intel.com>
> On ICL, 
> Tested-by: Dhinakaran Pandiyan <
> dhinakaran.pandiyan@intel.com>
> 

Thanks, I just pushed this, we can improve on top when adding more
operations.

> > +		uint64_t exp;
> > +		int r;
> > +
> > +		r = read(data->change_screen_timerfd, &exp,
> > sizeof(exp));
> > +		if (r == sizeof(uint64_t) && exp)
> > +			result = update_screen_and_test(data);
> > +	}
> > +
> > +	igt_debug("Screen changes: %u\n", data->screen_changes);
> > +	igt_assert_f(result,
> > +		     "No matching selective update blocks read from
> > debugfs\n");
> > +}
> > +
> > +static void cleanup(data_t *data)
> > +{
> > +	igt_plane_t *primary;
> > +
> > +	primary = igt_output_get_plane_type(data->output,
> > +					    DRM_PLANE_TYPE_PRIMARY);
> > +	igt_plane_set_fb(primary, NULL);
> > +	igt_display_commit2(&data->display, COMMIT_ATOMIC);
> > +
> > +	if (data->op == PAGE_FLIP)
> > +		igt_remove_fb(data->drm_fd, &data->fb[1]);
> > +	else if (data->op == FRONTBUFFER)
> > +		igt_put_cairo_ctx(data->drm_fd, &data->fb[0], data-
> > > cr);
> > +
> > +	igt_remove_fb(data->drm_fd, &data->fb[0]);
> > +}
> > +
> > +int main(int argc, char *argv[])
> > +{
> > +	data_t data = {};
> > +
> > +	igt_subtest_init_parse_opts(&argc, argv, "", NULL,
> > +				    NULL, NULL, NULL);
> > +	igt_skip_on_simulation();
> > +
> > +	igt_fixture {
> > +		struct itimerspec interval;
> > +		int r;
> > +
> > +		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
> > +		data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
> > +		kmstest_set_vt_graphics_mode();
> > +
> > +		igt_require_f(psr_sink_support(data.debugfs_fd,
> > PSR_MODE_2),
> > +			      "Sink does not support PSR2\n");
> > +
> > +		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd,
> > 4096);
> > +		igt_assert(data.bufmgr);
> > +		drm_intel_bufmgr_gem_enable_reuse(data.bufmgr);
> > +
> > +		display_init(&data);
> > +
> > +		/* Test if PSR2 can be enabled */
> > +		igt_require_f(psr_enable(data.debugfs_fd, PSR_MODE_2),
> > +			      "Error enabling PSR2\n");
> > +		data.op = FRONTBUFFER;
> > +		prepare(&data);
> > +		r = psr_wait_entry(data.debugfs_fd, PSR_MODE_2);
> I would rename this "r" to something more intuitive as assertions get
> debug logged.
> 
> 
> 
> > +		cleanup(&data);
> > +		igt_require_f(r, "PSR2 can not be enabled\n");
> > +
> > +		/* blocking timerfd */
> > +		data.change_screen_timerfd =
> > timerfd_create(CLOCK_MONOTONIC, 0);
> > +		igt_require(data.change_screen_timerfd != -1);
> > +		/* Changing screen at 30hz to support 30hz panels */
> > +		interval.it_value.tv_nsec = NSEC_PER_SEC / 30;
> > +		interval.it_value.tv_sec = 0;
> > +		interval.it_interval.tv_nsec =
> > interval.it_value.tv_nsec;
> > +		interval.it_interval.tv_sec = interval.it_value.tv_sec;
> > +		r = timerfd_settime(data.change_screen_timerfd, 0,
> > &interval, NULL);
> > +		igt_require_f(r != -1, "Error setting timerfd\n");
> > +	}
> > +
> > +	for (data.op = PAGE_FLIP; data.op < LAST; data.op++) {
> > +		igt_subtest_f("%s", op_str(data.op)) {
> > +			prepare(&data);
> > +			run(&data);
> > +			cleanup(&data);
> > +		}
> > +	}
> > +
> > +	igt_fixture {
> > +		close(data.debugfs_fd);
> > +		drm_intel_bufmgr_destroy(data.bufmgr);
> > +		display_fini(&data);
> > +	}
> > +
> > +	igt_exit();
> > +}
> > diff --git a/tests/meson.build b/tests/meson.build
> > index 0f12df26..ec980651 100644
> > --- a/tests/meson.build
> > +++ b/tests/meson.build
> > @@ -49,6 +49,7 @@ test_progs = [
> >  	'kms_plane_scaling',
> >  	'kms_properties',
> >  	'kms_psr',
> > +	'kms_psr2_su',
> >  	'kms_pwrite_crc',
> >  	'kms_rmfb',
> >  	'kms_rotation_crc',