[i-g-t,v2,7/9] tests/psr: Add the same test coverage that we have for PSR1 to PSR2

Submitted by Dhinakaran Pandiyan on Jan. 10, 2019, 11:52 p.m.

Details

Message ID 1b8b803eb063c3754a086f0796d19d696363f7f6.camel@intel.com
State New
Headers show
Series "Series without cover letter" ( rev: 2 ) in IGT

Not browsing as part of any series.

Commit Message

Dhinakaran Pandiyan Jan. 10, 2019, 11:52 p.m.
On Thu, 2019-01-03 at 06:36 -0800, José Roberto de Souza wrote:
> The main tests for PSR1 check if hardware tracking is detecting
> changes in planes when modifing it in different ways and now
> those tests will also run for PSR2 if supported by source and sink.
> 
> 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>
> ---
>  tests/kms_psr.c | 112 +++++++++++++++++++++++++++++++++++++++++++++-
> --
>  1 file changed, 105 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> index 855679b0..d4d38320 100644
> --- a/tests/kms_psr.c
> +++ b/tests/kms_psr.c
> @@ -60,6 +60,7 @@ typedef struct {
>  	int drm_fd;
>  	int debugfs_fd;
>  	enum operations op;
> +	bool op_psr2;
>  	uint32_t devid;
>  	uint32_t crtc_id;
>  	igt_display_t display;
> @@ -71,6 +72,7 @@ typedef struct {
>  	drmModeModeInfo *mode;
>  	igt_output_t *output;
>  	bool with_psr_disabled;
> +	bool supports_psr2;
>  } data_t;
>  
>  static void create_cursor_fb(data_t *data)
> @@ -194,12 +196,22 @@ static bool sink_support(data_t *data)
>  	return data->with_psr_disabled || psr_sink_support(data-
> >debugfs_fd);
>  }
>  
> -static bool psr_wait_entry_if_enabled(data_t *data)
> +static bool sink_support_psr2(data_t *data)
>  {
> -	if (data->with_psr_disabled)
> -		return true;
> +	return data->with_psr_disabled || psr2_sink_support(data-
> >debugfs_fd);
> +}
>  
> -	return psr_wait_entry(data->debugfs_fd);
> +static bool psr_wait_entry_if_enabled(data_t *data)
> +{
> +	if (!data->op_psr2) {
> +		if (data->with_psr_disabled)
> +			return true;
> +		return psr_wait_entry(data->debugfs_fd);
> +	} else {
> +		if (data->with_psr_disabled)
> +			return true;
> +		return psr2_wait_deep_sleep(data->debugfs_fd);
> +	}
>  }


Unless I am missing something, we can simplify all these changes by
pushing the PSR mode down through the call chain.

 
-static bool psr_active(int debugfs_fd, bool check_active)
+static bool psr_active(int debugfs_fd, enum psr_mode mode, bool
check_active)
 {
        bool active;
        char buf[PSR_STATUS_MAX_LEN];
+       char *s = mode == PSR_MODE_2 ? "DEEP_SLEEP" ? "SRDENT";
 
        igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
                                sizeof(buf));
        active = strstr(buf, "HW Enabled & Active bit: yes\n") &&
-               (strstr(buf, "SRDENT") || strstr(buf, "SLEEP"));
+                strstr(buf, s);
        return check_active ? active : !active;
 }
 
-bool psr_wait_entry(int debugfs_fd)
+bool psr_wait_entry(int debugfs_fd, int psr_mode)
 {
-       return igt_wait(psr_active(debugfs_fd, true), 500, 20);
+       return igt_wait(psr_active(debugfs_fd, true, psr_mode), 500,
20);
 }
 
-bool psr_wait_exit(int debugfs_fd)
+bool psr_wait_exit(int debugfs_fd, int psr_mode)
 {
        return igt_wait(psr_active(debugfs_fd, false), 40, 10);
 }


>  
>  static inline void manual(const char *expected)
> @@ -289,11 +301,16 @@ static void run_test(data_t *data)
>  		expected = "screen GREEN";
>  		break;
>  	}
> -	igt_assert(psr_wait_exit(data->debugfs_fd));
> +
> +	if (!data->op_psr2)
> +		igt_assert(psr_wait_exit(data->debugfs_fd));
> +	else
> +		igt_assert(psr2_wait_update(data->debugfs_fd));
>  	manual(expected);
>  }
>  
> -static void test_cleanup(data_t *data) {
> +static void test_cleanup(data_t *data)
> +{
>  	igt_plane_t *primary;
>  
>  	primary = igt_output_get_plane_type(data->output,
> @@ -304,6 +321,11 @@ static void test_cleanup(data_t *data) {
>  
>  	igt_remove_fb(data->drm_fd, &data->fb_green);
>  	igt_remove_fb(data->drm_fd, &data->fb_white);
> +
> +	/* switch to PSR1 again */
> +	if (data->op_psr2 && !data->with_psr_disabled)
> +		psr_enable(data->debugfs_fd);
> +	data->op_psr2 = false;
>  }
>  
>  static void setup_test_plane(data_t *data, int test_plane)
> @@ -311,6 +333,11 @@ static void setup_test_plane(data_t *data, int
> test_plane)
>  	uint32_t white_h, white_v;
>  	igt_plane_t *primary, *sprite, *cursor;
>  
> +	if (data->op_psr2 && !data->with_psr_disabled) {
> +		igt_require(data->supports_psr2);
> +		psr2_enable(data->debugfs_fd);
> +	}
> +
>  	igt_create_color_fb(data->drm_fd,
>  			    data->mode->hdisplay, data->mode->vdisplay,
>  			    DRM_FORMAT_XRGB8888,
> @@ -391,7 +418,7 @@ static int opt_handler(int opt, int opt_index,
> void *_data)
>  int main(int argc, char *argv[])
>  {
>  	const char *help_str =
> -	       "  --no-psr\tRun test without PSR.";
> +	       "  --no-psr\tRun test without PSR/PSR2.\n";
>  	static struct option long_options[] = {
>  		{"no-psr", 0, 0, 'n'},
>  		{ 0, 0, 0, 0 }
> @@ -414,6 +441,7 @@ int main(int argc, char *argv[])
>  
>  		igt_require_f(sink_support(&data),
>  			      "Sink does not support PSR\n");
> +		data.supports_psr2 = sink_support_psr2(&data);
>  
>  		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd,
> 4096);
>  		igt_assert(data.bufmgr);
> @@ -428,6 +456,13 @@ int main(int argc, char *argv[])
>  		test_cleanup(&data);
>  	}
>  
> +	igt_subtest("psr2_basic") {
> +		data.op_psr2 = true;
> +		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
> +		igt_assert(psr_wait_entry_if_enabled(&data));
> +		test_cleanup(&data);
> +	}
> +
>  	igt_subtest("no_drrs") {
>  		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
>  		igt_assert(psr_wait_entry_if_enabled(&data));
> @@ -435,6 +470,14 @@ int main(int argc, char *argv[])
>  		test_cleanup(&data);
>  	}
>  
> +	igt_subtest("psr2_no_drrs") {
> +		data.op_psr2 = true;
> +		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
> +		igt_assert(psr_wait_entry_if_enabled(&data));
> +		igt_assert(drrs_disabled(&data));
> +		test_cleanup(&data);
> +	}
> +
>  	for (op = PAGE_FLIP; op <= RENDER; op++) {
>  		igt_subtest_f("primary_%s", op_str(op)) {
>  			data.op = op;
> @@ -445,6 +488,17 @@ int main(int argc, char *argv[])
>  		}
>  	}
>  
> +	for (op = PAGE_FLIP; op <= RENDER; op++) {
> +		igt_subtest_f("psr2_primary_%s", op_str(op)) {
> +			data.op_psr2 = true;
> +			data.op = op;
> +			setup_test_plane(&data,
> DRM_PLANE_TYPE_PRIMARY);
> +			igt_assert(psr_wait_entry_if_enabled(&data));
> +			run_test(&data);
> +			test_cleanup(&data);
> +		}
> +	}
> +
>  	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
>  		igt_subtest_f("sprite_%s", op_str(op)) {
>  			data.op = op;
> @@ -455,6 +509,17 @@ int main(int argc, char *argv[])
>  		}
>  	}
>  
> +	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
> +		igt_subtest_f("psr2_sprite_%s", op_str(op)) {
> +			data.op_psr2 = true;
> +			data.op = op;
> +			setup_test_plane(&data,
> DRM_PLANE_TYPE_OVERLAY);
> +			igt_assert(psr_wait_entry_if_enabled(&data));
> +			run_test(&data);
> +			test_cleanup(&data);
> +		}
> +	}
> +
>  	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
>  		igt_subtest_f("cursor_%s", op_str(op)) {
>  			data.op = op;
> @@ -465,6 +530,17 @@ int main(int argc, char *argv[])
>  		}
>  	}
>  
> +	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
> +		igt_subtest_f("psr2_cursor_%s", op_str(op)) {
> +			data.op_psr2 = true;
> +			data.op = op;
> +			setup_test_plane(&data, DRM_PLANE_TYPE_CURSOR);
> +			igt_assert(psr_wait_entry_if_enabled(&data));
> +			run_test(&data);
> +			test_cleanup(&data);
> +		}
> +	}
> +
>  	igt_subtest_f("dpms") {
>  		data.op = RENDER;
>  		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
> @@ -474,6 +550,16 @@ int main(int argc, char *argv[])
>  		test_cleanup(&data);
>  	}
>  
> +	igt_subtest_f("psr2_dpms") {
> +		data.op_psr2 = true;
> +		data.op = RENDER;
> +		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
> +		igt_assert(psr_wait_entry_if_enabled(&data));
> +		dpms_off_on(&data);
> +		run_test(&data);
> +		test_cleanup(&data);
> +	}
> +
>  	igt_subtest_f("suspend") {
>  		data.op = PLANE_ONOFF;
>  		setup_test_plane(&data, DRM_PLANE_TYPE_CURSOR);
> @@ -485,6 +571,18 @@ int main(int argc, char *argv[])
>  		test_cleanup(&data);
>  	}
>  
> +	igt_subtest_f("psr2_suspend") {
> +		data.op_psr2 = true;
> +		data.op = PLANE_ONOFF;
> +		setup_test_plane(&data, DRM_PLANE_TYPE_CURSOR);
> +		igt_assert(psr_wait_entry_if_enabled(&data));
> +		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> +					      SUSPEND_TEST_NONE);
> +		igt_assert(psr_wait_entry_if_enabled(&data));
> +		run_test(&data);
> +		test_cleanup(&data);
> +	}
> +
>  	igt_fixture {
>  		if (!data.with_psr_disabled)
>  			psr_disable(data.debugfs_fd);

Patch hide | download patch | download mbox

diff --git a/tests/kms_psr.c b/tests/kms_psr.c
index 855679b0..bb90b994 100644
--- a/tests/kms_psr.c
+++ b/tests/kms_psr.c
@@ -199,7 +199,7 @@  static bool psr_wait_entry_if_enabled(data_t *data)
        if (data->with_psr_disabled)
                return true;
 
-       return psr_wait_entry(data->debugfs_fd);
+       return psr_wait_entry(data->debugfs_fd, data->psr_mode);
 }
 
 static inline void manual(const char *expected)
@@ -289,7 +289,7 @@  static void run_test(data_t *data)
                expected = "screen GREEN";
                break;
        }
-       igt_assert(psr_wait_exit(data->debugfs_fd));
+       igt_assert(psr_wait_exit(data->debugfs_fd, data->psr_mode));
        manual(expected);
 }


Comments

On Thu, 2019-01-10 at 15:52 -0800, Dhinakaran Pandiyan wrote:
> On Thu, 2019-01-03 at 06:36 -0800, José Roberto de Souza wrote:
> > The main tests for PSR1 check if hardware tracking is detecting
> > changes in planes when modifing it in different ways and now
> > those tests will also run for PSR2 if supported by source and sink.
> > 
> > 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>
> > ---
> >  tests/kms_psr.c | 112
> > +++++++++++++++++++++++++++++++++++++++++++++-
> > --
> >  1 file changed, 105 insertions(+), 7 deletions(-)
> > 
> > diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> > index 855679b0..d4d38320 100644
> > --- a/tests/kms_psr.c
> > +++ b/tests/kms_psr.c
> > @@ -60,6 +60,7 @@ typedef struct {
> >  	int drm_fd;
> >  	int debugfs_fd;
> >  	enum operations op;
> > +	bool op_psr2;
> >  	uint32_t devid;
> >  	uint32_t crtc_id;
> >  	igt_display_t display;
> > @@ -71,6 +72,7 @@ typedef struct {
> >  	drmModeModeInfo *mode;
> >  	igt_output_t *output;
> >  	bool with_psr_disabled;
> > +	bool supports_psr2;
> >  } data_t;
> >  
> >  static void create_cursor_fb(data_t *data)
> > @@ -194,12 +196,22 @@ static bool sink_support(data_t *data)
> >  	return data->with_psr_disabled || psr_sink_support(data-
> > > debugfs_fd);
> >  }
> >  
> > -static bool psr_wait_entry_if_enabled(data_t *data)
> > +static bool sink_support_psr2(data_t *data)
> >  {
> > -	if (data->with_psr_disabled)
> > -		return true;
> > +	return data->with_psr_disabled || psr2_sink_support(data-
> > > debugfs_fd);
> > +}
> >  
> > -	return psr_wait_entry(data->debugfs_fd);
> > +static bool psr_wait_entry_if_enabled(data_t *data)
> > +{
> > +	if (!data->op_psr2) {
> > +		if (data->with_psr_disabled)
> > +			return true;
> > +		return psr_wait_entry(data->debugfs_fd);
> > +	} else {
> > +		if (data->with_psr_disabled)
> > +			return true;
> > +		return psr2_wait_deep_sleep(data->debugfs_fd);
> > +	}
> >  }
> 
> Unless I am missing something, we can simplify all these changes by
> pushing the PSR mode down through the call chain.
> 
>  
> -static bool psr_active(int debugfs_fd, bool check_active)
> +static bool psr_active(int debugfs_fd, enum psr_mode mode, bool
> check_active)
>  {

I added the PSR2 functions with other names to avoid any
misinterpretation as PSR2 could be active in deep sleep state and doing
selective update and in the states between and in the kms_psr2_su we
need to check both states to properly test.
 
I'm okay in adding the PSR mode to enable(), disable(), sink_support()
and reuse the internal functions too but in the other functions the
code saved is not worth. What do you think DK?

>         bool active;
>         char buf[PSR_STATUS_MAX_LEN];
> +       char *s = mode == PSR_MODE_2 ? "DEEP_SLEEP" ? "SRDENT";
>  
>         igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status",
> buf,
>                                 sizeof(buf));
>         active = strstr(buf, "HW Enabled & Active bit: yes\n") &&
> -               (strstr(buf, "SRDENT") || strstr(buf, "SLEEP"));
> +                strstr(buf, s);
>         return check_active ? active : !active;
>  }
>  
> -bool psr_wait_entry(int debugfs_fd)
> +bool psr_wait_entry(int debugfs_fd, int psr_mode)
>  {
> -       return igt_wait(psr_active(debugfs_fd, true), 500, 20);
> +       return igt_wait(psr_active(debugfs_fd, true, psr_mode), 500,
> 20);
>  }
>  
> -bool psr_wait_exit(int debugfs_fd)
> +bool psr_wait_exit(int debugfs_fd, int psr_mode)
>  {
>         return igt_wait(psr_active(debugfs_fd, false), 40, 10);
>  }
> diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> index 855679b0..bb90b994 100644
> --- a/tests/kms_psr.c
> +++ b/tests/kms_psr.c
> @@ -199,7 +199,7 @@ static bool psr_wait_entry_if_enabled(data_t
> *data)
>         if (data->with_psr_disabled)
>                 return true;
>  
> -       return psr_wait_entry(data->debugfs_fd);
> +       return psr_wait_entry(data->debugfs_fd, data->psr_mode);
>  }
>  
>  static inline void manual(const char *expected)
> @@ -289,7 +289,7 @@ static void run_test(data_t *data)
>                 expected = "screen GREEN";
>                 break;
>         }
> -       igt_assert(psr_wait_exit(data->debugfs_fd));
> +       igt_assert(psr_wait_exit(data->debugfs_fd, data->psr_mode));
>         manual(expected);
>  }
> 
> 
> 
> >  
> >  static inline void manual(const char *expected)
> > @@ -289,11 +301,16 @@ static void run_test(data_t *data)
> >  		expected = "screen GREEN";
> >  		break;
> >  	}
> > -	igt_assert(psr_wait_exit(data->debugfs_fd));
> > +
> > +	if (!data->op_psr2)
> > +		igt_assert(psr_wait_exit(data->debugfs_fd));
> > +	else
> > +		igt_assert(psr2_wait_update(data->debugfs_fd));
> >  	manual(expected);
> >  }
> >  
> > -static void test_cleanup(data_t *data) {
> > +static void test_cleanup(data_t *data)
> > +{
> >  	igt_plane_t *primary;
> >  
> >  	primary = igt_output_get_plane_type(data->output,
> > @@ -304,6 +321,11 @@ static void test_cleanup(data_t *data) {
> >  
> >  	igt_remove_fb(data->drm_fd, &data->fb_green);
> >  	igt_remove_fb(data->drm_fd, &data->fb_white);
> > +
> > +	/* switch to PSR1 again */
> > +	if (data->op_psr2 && !data->with_psr_disabled)
> > +		psr_enable(data->debugfs_fd);
> > +	data->op_psr2 = false;
> >  }
> >  
> >  static void setup_test_plane(data_t *data, int test_plane)
> > @@ -311,6 +333,11 @@ static void setup_test_plane(data_t *data, int
> > test_plane)
> >  	uint32_t white_h, white_v;
> >  	igt_plane_t *primary, *sprite, *cursor;
> >  
> > +	if (data->op_psr2 && !data->with_psr_disabled) {
> > +		igt_require(data->supports_psr2);
> > +		psr2_enable(data->debugfs_fd);
> > +	}
> > +
> >  	igt_create_color_fb(data->drm_fd,
> >  			    data->mode->hdisplay, data->mode->vdisplay,
> >  			    DRM_FORMAT_XRGB8888,
> > @@ -391,7 +418,7 @@ static int opt_handler(int opt, int opt_index,
> > void *_data)
> >  int main(int argc, char *argv[])
> >  {
> >  	const char *help_str =
> > -	       "  --no-psr\tRun test without PSR.";
> > +	       "  --no-psr\tRun test without PSR/PSR2.\n";
> >  	static struct option long_options[] = {
> >  		{"no-psr", 0, 0, 'n'},
> >  		{ 0, 0, 0, 0 }
> > @@ -414,6 +441,7 @@ int main(int argc, char *argv[])
> >  
> >  		igt_require_f(sink_support(&data),
> >  			      "Sink does not support PSR\n");
> > +		data.supports_psr2 = sink_support_psr2(&data);
> >  
> >  		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd,
> > 4096);
> >  		igt_assert(data.bufmgr);
> > @@ -428,6 +456,13 @@ int main(int argc, char *argv[])
> >  		test_cleanup(&data);
> >  	}
> >  
> > +	igt_subtest("psr2_basic") {
> > +		data.op_psr2 = true;
> > +		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
> > +		igt_assert(psr_wait_entry_if_enabled(&data));
> > +		test_cleanup(&data);
> > +	}
> > +
> >  	igt_subtest("no_drrs") {
> >  		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
> >  		igt_assert(psr_wait_entry_if_enabled(&data));
> > @@ -435,6 +470,14 @@ int main(int argc, char *argv[])
> >  		test_cleanup(&data);
> >  	}
> >  
> > +	igt_subtest("psr2_no_drrs") {
> > +		data.op_psr2 = true;
> > +		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
> > +		igt_assert(psr_wait_entry_if_enabled(&data));
> > +		igt_assert(drrs_disabled(&data));
> > +		test_cleanup(&data);
> > +	}
> > +
> >  	for (op = PAGE_FLIP; op <= RENDER; op++) {
> >  		igt_subtest_f("primary_%s", op_str(op)) {
> >  			data.op = op;
> > @@ -445,6 +488,17 @@ int main(int argc, char *argv[])
> >  		}
> >  	}
> >  
> > +	for (op = PAGE_FLIP; op <= RENDER; op++) {
> > +		igt_subtest_f("psr2_primary_%s", op_str(op)) {
> > +			data.op_psr2 = true;
> > +			data.op = op;
> > +			setup_test_plane(&data,
> > DRM_PLANE_TYPE_PRIMARY);
> > +			igt_assert(psr_wait_entry_if_enabled(&data));
> > +			run_test(&data);
> > +			test_cleanup(&data);
> > +		}
> > +	}
> > +
> >  	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
> >  		igt_subtest_f("sprite_%s", op_str(op)) {
> >  			data.op = op;
> > @@ -455,6 +509,17 @@ int main(int argc, char *argv[])
> >  		}
> >  	}
> >  
> > +	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
> > +		igt_subtest_f("psr2_sprite_%s", op_str(op)) {
> > +			data.op_psr2 = true;
> > +			data.op = op;
> > +			setup_test_plane(&data,
> > DRM_PLANE_TYPE_OVERLAY);
> > +			igt_assert(psr_wait_entry_if_enabled(&data));
> > +			run_test(&data);
> > +			test_cleanup(&data);
> > +		}
> > +	}
> > +
> >  	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
> >  		igt_subtest_f("cursor_%s", op_str(op)) {
> >  			data.op = op;
> > @@ -465,6 +530,17 @@ int main(int argc, char *argv[])
> >  		}
> >  	}
> >  
> > +	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
> > +		igt_subtest_f("psr2_cursor_%s", op_str(op)) {
> > +			data.op_psr2 = true;
> > +			data.op = op;
> > +			setup_test_plane(&data, DRM_PLANE_TYPE_CURSOR);
> > +			igt_assert(psr_wait_entry_if_enabled(&data));
> > +			run_test(&data);
> > +			test_cleanup(&data);
> > +		}
> > +	}
> > +
> >  	igt_subtest_f("dpms") {
> >  		data.op = RENDER;
> >  		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
> > @@ -474,6 +550,16 @@ int main(int argc, char *argv[])
> >  		test_cleanup(&data);
> >  	}
> >  
> > +	igt_subtest_f("psr2_dpms") {
> > +		data.op_psr2 = true;
> > +		data.op = RENDER;
> > +		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
> > +		igt_assert(psr_wait_entry_if_enabled(&data));
> > +		dpms_off_on(&data);
> > +		run_test(&data);
> > +		test_cleanup(&data);
> > +	}
> > +
> >  	igt_subtest_f("suspend") {
> >  		data.op = PLANE_ONOFF;
> >  		setup_test_plane(&data, DRM_PLANE_TYPE_CURSOR);
> > @@ -485,6 +571,18 @@ int main(int argc, char *argv[])
> >  		test_cleanup(&data);
> >  	}
> >  
> > +	igt_subtest_f("psr2_suspend") {
> > +		data.op_psr2 = true;
> > +		data.op = PLANE_ONOFF;
> > +		setup_test_plane(&data, DRM_PLANE_TYPE_CURSOR);
> > +		igt_assert(psr_wait_entry_if_enabled(&data));
> > +		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> > +					      SUSPEND_TEST_NONE);
> > +		igt_assert(psr_wait_entry_if_enabled(&data));
> > +		run_test(&data);
> > +		test_cleanup(&data);
> > +	}
> > +
> >  	igt_fixture {
> >  		if (!data.with_psr_disabled)
> >  			psr_disable(data.debugfs_fd);
On Thu, 2019-01-10 at 17:12 -0800, Souza, Jose wrote:
> On Thu, 2019-01-10 at 15:52 -0800, Dhinakaran Pandiyan wrote:
> > On Thu, 2019-01-03 at 06:36 -0800, José Roberto de Souza wrote:
> > > The main tests for PSR1 check if hardware tracking is detecting
> > > changes in planes when modifing it in different ways and now
> > > those tests will also run for PSR2 if supported by source and
> > > sink.
> > > 
> > > 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>
> > > ---
> > >  tests/kms_psr.c | 112
> > > +++++++++++++++++++++++++++++++++++++++++++++-
> > > --
> > >  1 file changed, 105 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> > > index 855679b0..d4d38320 100644
> > > --- a/tests/kms_psr.c
> > > +++ b/tests/kms_psr.c
> > > @@ -60,6 +60,7 @@ typedef struct {
> > >  	int drm_fd;
> > >  	int debugfs_fd;
> > >  	enum operations op;
> > > +	bool op_psr2;
> > >  	uint32_t devid;
> > >  	uint32_t crtc_id;
> > >  	igt_display_t display;
> > > @@ -71,6 +72,7 @@ typedef struct {
> > >  	drmModeModeInfo *mode;
> > >  	igt_output_t *output;
> > >  	bool with_psr_disabled;
> > > +	bool supports_psr2;
> > >  } data_t;
> > >  
> > >  static void create_cursor_fb(data_t *data)
> > > @@ -194,12 +196,22 @@ static bool sink_support(data_t *data)
> > >  	return data->with_psr_disabled || psr_sink_support(data-
> > > > debugfs_fd);
> > > 
> > >  }
> > >  
> > > -static bool psr_wait_entry_if_enabled(data_t *data)
> > > +static bool sink_support_psr2(data_t *data)
> > >  {
> > > -	if (data->with_psr_disabled)
> > > -		return true;
> > > +	return data->with_psr_disabled || psr2_sink_support(data-
> > > > debugfs_fd);
> > > 
> > > +}
> > >  
> > > -	return psr_wait_entry(data->debugfs_fd);
> > > +static bool psr_wait_entry_if_enabled(data_t *data)
> > > +{
> > > +	if (!data->op_psr2) {
> > > +		if (data->with_psr_disabled)
> > > +			return true;
> > > +		return psr_wait_entry(data->debugfs_fd);
> > > +	} else {
> > > +		if (data->with_psr_disabled)
> > > +			return true;
> > > +		return psr2_wait_deep_sleep(data->debugfs_fd);
> > > +	}
> > >  }
> > 
> > Unless I am missing something, we can simplify all these changes by
> > pushing the PSR mode down through the call chain.
> > 
> >  
> > -static bool psr_active(int debugfs_fd, bool check_active)
> > +static bool psr_active(int debugfs_fd, enum psr_mode mode, bool
> > check_active)
> >  {
> 
> I added the PSR2 functions with other names to avoid any
> misinterpretation as PSR2 could be active in deep sleep state and
> doing
> selective update and in the states between and in the kms_psr2_su we
> need to check both states to properly test.

Selective update testing does not need modifying the behavior of any of
these functions, does it? Both PSR1 and PSR2(non-SU) tests wait until
PSR is in an active state, do some operation and then verify we are out
of the state. Since they follow the same test steps, I believe it is
better to share the interface. We also want kms_fbt_fbcon and
kms_frontbuffer_tracking to setup PSR2, that is another reason
psr_wait_entry()/psr_wait_exit() in the library should handle PSR1 v/s
PSR2 instead of the callers in kms_psr.


>  
> I'm okay in adding the PSR mode to enable(), disable(),
> sink_support()
> and reuse the internal functions too but in the other functions the
> code saved is not worth. What do you think DK?
> 
> >         bool active;
> >         char buf[PSR_STATUS_MAX_LEN];
> > +       char *s = mode == PSR_MODE_2 ? "DEEP_SLEEP" ? "SRDENT";
> >  
> >         igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status",
> > buf,
> >                                 sizeof(buf));
> >         active = strstr(buf, "HW Enabled & Active bit: yes\n") &&
> > -               (strstr(buf, "SRDENT") || strstr(buf, "SLEEP"));
> > +                strstr(buf, s);
> >         return check_active ? active : !active;
> >  }
> >  
> > -bool psr_wait_entry(int debugfs_fd)
> > +bool psr_wait_entry(int debugfs_fd, int psr_mode)
> >  {
> > -       return igt_wait(psr_active(debugfs_fd, true), 500, 20);
> > +       return igt_wait(psr_active(debugfs_fd, true, psr_mode),
> > 500,
> > 20);
> >  }
> >  
> > -bool psr_wait_exit(int debugfs_fd)
> > +bool psr_wait_exit(int debugfs_fd, int psr_mode)
> >  {
> >         return igt_wait(psr_active(debugfs_fd, false), 40, 10);
> >  }
> > diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> > index 855679b0..bb90b994 100644
> > --- a/tests/kms_psr.c
> > +++ b/tests/kms_psr.c
> > @@ -199,7 +199,7 @@ static bool psr_wait_entry_if_enabled(data_t
> > *data)
> >         if (data->with_psr_disabled)
> >                 return true;
> >  
> > -       return psr_wait_entry(data->debugfs_fd);
> > +       return psr_wait_entry(data->debugfs_fd, data->psr_mode);
> >  }
> >  
> >  static inline void manual(const char *expected)
> > @@ -289,7 +289,7 @@ static void run_test(data_t *data)
> >                 expected = "screen GREEN";
> >                 break;
> >         }
> > -       igt_assert(psr_wait_exit(data->debugfs_fd));
> > +       igt_assert(psr_wait_exit(data->debugfs_fd, data-
> > >psr_mode));
> >         manual(expected);
> >  }
> > 
> > 
> > 
> > >  
> > >  static inline void manual(const char *expected)
> > > @@ -289,11 +301,16 @@ static void run_test(data_t *data)
> > >  		expected = "screen GREEN";
> > >  		break;
> > >  	}
> > > -	igt_assert(psr_wait_exit(data->debugfs_fd));
> > > +
> > > +	if (!data->op_psr2)
> > > +		igt_assert(psr_wait_exit(data->debugfs_fd));
> > > +	else
> > > +		igt_assert(psr2_wait_update(data->debugfs_fd));
> > >  	manual(expected);
> > >  }
> > >  
> > > -static void test_cleanup(data_t *data) {
> > > +static void test_cleanup(data_t *data)
> > > +{
> > >  	igt_plane_t *primary;
> > >  
> > >  	primary = igt_output_get_plane_type(data->output,
> > > @@ -304,6 +321,11 @@ static void test_cleanup(data_t *data) {
> > >  
> > >  	igt_remove_fb(data->drm_fd, &data->fb_green);
> > >  	igt_remove_fb(data->drm_fd, &data->fb_white);
> > > +
> > > +	/* switch to PSR1 again */
> > > +	if (data->op_psr2 && !data->with_psr_disabled)
> > > +		psr_enable(data->debugfs_fd);
> > > +	data->op_psr2 = false;
> > >  }
> > >  
> > >  static void setup_test_plane(data_t *data, int test_plane)
> > > @@ -311,6 +333,11 @@ static void setup_test_plane(data_t *data,
> > > int
> > > test_plane)
> > >  	uint32_t white_h, white_v;
> > >  	igt_plane_t *primary, *sprite, *cursor;
> > >  
> > > +	if (data->op_psr2 && !data->with_psr_disabled) {
> > > +		igt_require(data->supports_psr2);
> > > +		psr2_enable(data->debugfs_fd);
> > > +	}
> > > +
> > >  	igt_create_color_fb(data->drm_fd,
> > >  			    data->mode->hdisplay, data->mode->vdisplay,
> > >  			    DRM_FORMAT_XRGB8888,
> > > @@ -391,7 +418,7 @@ static int opt_handler(int opt, int
> > > opt_index,
> > > void *_data)
> > >  int main(int argc, char *argv[])
> > >  {
> > >  	const char *help_str =
> > > -	       "  --no-psr\tRun test without PSR.";
> > > +	       "  --no-psr\tRun test without PSR/PSR2.\n";
> > >  	static struct option long_options[] = {
> > >  		{"no-psr", 0, 0, 'n'},
> > >  		{ 0, 0, 0, 0 }
> > > @@ -414,6 +441,7 @@ int main(int argc, char *argv[])
> > >  
> > >  		igt_require_f(sink_support(&data),
> > >  			      "Sink does not support PSR\n");
> > > +		data.supports_psr2 = sink_support_psr2(&data);
> > >  
> > >  		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd,
> > > 4096);
> > >  		igt_assert(data.bufmgr);
> > > @@ -428,6 +456,13 @@ int main(int argc, char *argv[])
> > >  		test_cleanup(&data);
> > >  	}
> > >  
> > > +	igt_subtest("psr2_basic") {
> > > +		data.op_psr2 = true;
> > > +		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
> > > +		igt_assert(psr_wait_entry_if_enabled(&data));
> > > +		test_cleanup(&data);
> > > +	}
> > > +
> > >  	igt_subtest("no_drrs") {
> > >  		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
> > >  		igt_assert(psr_wait_entry_if_enabled(&data));
> > > @@ -435,6 +470,14 @@ int main(int argc, char *argv[])
> > >  		test_cleanup(&data);
> > >  	}
> > >  
> > > +	igt_subtest("psr2_no_drrs") {
> > > +		data.op_psr2 = true;
> > > +		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
> > > +		igt_assert(psr_wait_entry_if_enabled(&data));
> > > +		igt_assert(drrs_disabled(&data));
> > > +		test_cleanup(&data);
> > > +	}
> > > +
> > >  	for (op = PAGE_FLIP; op <= RENDER; op++) {
> > >  		igt_subtest_f("primary_%s", op_str(op)) {
> > >  			data.op = op;
> > > @@ -445,6 +488,17 @@ int main(int argc, char *argv[])
> > >  		}
> > >  	}
> > >  
> > > +	for (op = PAGE_FLIP; op <= RENDER; op++) {
> > > +		igt_subtest_f("psr2_primary_%s", op_str(op)) {
> > > +			data.op_psr2 = true;
> > > +			data.op = op;
> > > +			setup_test_plane(&data,
> > > DRM_PLANE_TYPE_PRIMARY);
> > > +			igt_assert(psr_wait_entry_if_enabled(&data));
> > > +			run_test(&data);
> > > +			test_cleanup(&data);
> > > +		}
> > > +	}
> > > +
> > >  	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
> > >  		igt_subtest_f("sprite_%s", op_str(op)) {
> > >  			data.op = op;
> > > @@ -455,6 +509,17 @@ int main(int argc, char *argv[])
> > >  		}
> > >  	}
> > >  
> > > +	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
> > > +		igt_subtest_f("psr2_sprite_%s", op_str(op)) {
> > > +			data.op_psr2 = true;
> > > +			data.op = op;
> > > +			setup_test_plane(&data,
> > > DRM_PLANE_TYPE_OVERLAY);
> > > +			igt_assert(psr_wait_entry_if_enabled(&data));
> > > +			run_test(&data);
> > > +			test_cleanup(&data);
> > > +		}
> > > +	}
> > > +
> > >  	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
> > >  		igt_subtest_f("cursor_%s", op_str(op)) {
> > >  			data.op = op;
> > > @@ -465,6 +530,17 @@ int main(int argc, char *argv[])
> > >  		}
> > >  	}
> > >  
> > > +	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
> > > +		igt_subtest_f("psr2_cursor_%s", op_str(op)) {
> > > +			data.op_psr2 = true;
> > > +			data.op = op;
> > > +			setup_test_plane(&data, DRM_PLANE_TYPE_CURSOR);
> > > +			igt_assert(psr_wait_entry_if_enabled(&data));
> > > +			run_test(&data);
> > > +			test_cleanup(&data);
> > > +		}
> > > +	}
> > > +
> > >  	igt_subtest_f("dpms") {
> > >  		data.op = RENDER;
> > >  		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
> > > @@ -474,6 +550,16 @@ int main(int argc, char *argv[])
> > >  		test_cleanup(&data);
> > >  	}
> > >  
> > > +	igt_subtest_f("psr2_dpms") {
> > > +		data.op_psr2 = true;
> > > +		data.op = RENDER;
> > > +		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
> > > +		igt_assert(psr_wait_entry_if_enabled(&data));
> > > +		dpms_off_on(&data);
> > > +		run_test(&data);
> > > +		test_cleanup(&data);
> > > +	}
> > > +
> > >  	igt_subtest_f("suspend") {
> > >  		data.op = PLANE_ONOFF;
> > >  		setup_test_plane(&data, DRM_PLANE_TYPE_CURSOR);
> > > @@ -485,6 +571,18 @@ int main(int argc, char *argv[])
> > >  		test_cleanup(&data);
> > >  	}
> > >  
> > > +	igt_subtest_f("psr2_suspend") {
> > > +		data.op_psr2 = true;
> > > +		data.op = PLANE_ONOFF;
> > > +		setup_test_plane(&data, DRM_PLANE_TYPE_CURSOR);
> > > +		igt_assert(psr_wait_entry_if_enabled(&data));
> > > +		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> > > +					      SUSPEND_TEST_NONE);
> > > +		igt_assert(psr_wait_entry_if_enabled(&data));
> > > +		run_test(&data);
> > > +		test_cleanup(&data);
> > > +	}
> > > +
> > >  	igt_fixture {
> > >  		if (!data.with_psr_disabled)
> > >  			psr_disable(data.debugfs_fd);
On Fri, 2019-01-11 at 11:49 -0800, Dhinakaran Pandiyan wrote:
> On Thu, 2019-01-10 at 17:12 -0800, Souza, Jose wrote:
> > On Thu, 2019-01-10 at 15:52 -0800, Dhinakaran Pandiyan wrote:
> > > On Thu, 2019-01-03 at 06:36 -0800, José Roberto de Souza wrote:
> > > > The main tests for PSR1 check if hardware tracking is detecting
> > > > changes in planes when modifing it in different ways and now
> > > > those tests will also run for PSR2 if supported by source and
> > > > sink.
> > > > 
> > > > 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>
> > > > ---
> > > >  tests/kms_psr.c | 112
> > > > +++++++++++++++++++++++++++++++++++++++++++++-
> > > > --
> > > >  1 file changed, 105 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> > > > index 855679b0..d4d38320 100644
> > > > --- a/tests/kms_psr.c
> > > > +++ b/tests/kms_psr.c
> > > > @@ -60,6 +60,7 @@ typedef struct {
> > > >  	int drm_fd;
> > > >  	int debugfs_fd;
> > > >  	enum operations op;
> > > > +	bool op_psr2;
> > > >  	uint32_t devid;
> > > >  	uint32_t crtc_id;
> > > >  	igt_display_t display;
> > > > @@ -71,6 +72,7 @@ typedef struct {
> > > >  	drmModeModeInfo *mode;
> > > >  	igt_output_t *output;
> > > >  	bool with_psr_disabled;
> > > > +	bool supports_psr2;
> > > >  } data_t;
> > > >  
> > > >  static void create_cursor_fb(data_t *data)
> > > > @@ -194,12 +196,22 @@ static bool sink_support(data_t *data)
> > > >  	return data->with_psr_disabled ||
> > > > psr_sink_support(data-
> > > > > debugfs_fd);
> > > > 
> > > >  }
> > > >  
> > > > -static bool psr_wait_entry_if_enabled(data_t *data)
> > > > +static bool sink_support_psr2(data_t *data)
> > > >  {
> > > > -	if (data->with_psr_disabled)
> > > > -		return true;
> > > > +	return data->with_psr_disabled ||
> > > > psr2_sink_support(data-
> > > > > debugfs_fd);
> > > > 
> > > > +}
> > > >  
> > > > -	return psr_wait_entry(data->debugfs_fd);
> > > > +static bool psr_wait_entry_if_enabled(data_t *data)
> > > > +{
> > > > +	if (!data->op_psr2) {
> > > > +		if (data->with_psr_disabled)
> > > > +			return true;
> > > > +		return psr_wait_entry(data->debugfs_fd);
> > > > +	} else {
> > > > +		if (data->with_psr_disabled)
> > > > +			return true;
> > > > +		return psr2_wait_deep_sleep(data->debugfs_fd);
> > > > +	}
> > > >  }
> > > 
> > > Unless I am missing something, we can simplify all these changes
> > > by
> > > pushing the PSR mode down through the call chain.
> > > 
> > >  
> > > -static bool psr_active(int debugfs_fd, bool check_active)
> > > +static bool psr_active(int debugfs_fd, enum psr_mode mode, bool
> > > check_active)
> > >  {
> > 
> > I added the PSR2 functions with other names to avoid any
> > misinterpretation as PSR2 could be active in deep sleep state and
> > doing
> > selective update and in the states between and in the kms_psr2_su
> > we
> > need to check both states to properly test.
> 
> Selective update testing does not need modifying the behavior of any
> of
> these functions, does it? Both PSR1 and PSR2(non-SU) tests wait until
> PSR is in an active state, do some operation and then verify we are
> out
> of the state.

Yes but after perform some operation it can keep into a active state in
PSR2 that is why psr_wait_exit() is not a good name. Same for
psr_wait_entry() it is wait entry in deep sleep? or wait entry in PSR
active?

> Since they follow the same test steps, I believe it is
> better to share the interface. We also want kms_fbt_fbcon and
> kms_frontbuffer_tracking to setup PSR2, that is another reason
> psr_wait_entry()/psr_wait_exit() in the library should handle PSR1
> v/s
> PSR2 instead of the callers in kms_psr.
> 
> 
> >  
> > I'm okay in adding the PSR mode to enable(), disable(),
> > sink_support()
> > and reuse the internal functions too but in the other functions the
> > code saved is not worth. What do you think DK?
> > 
> > >         bool active;
> > >         char buf[PSR_STATUS_MAX_LEN];
> > > +       char *s = mode == PSR_MODE_2 ? "DEEP_SLEEP" ? "SRDENT";
> > >  
> > >         igt_debugfs_simple_read(debugfs_fd,
> > > "i915_edp_psr_status",
> > > buf,
> > >                                 sizeof(buf));
> > >         active = strstr(buf, "HW Enabled & Active bit: yes\n") &&
> > > -               (strstr(buf, "SRDENT") || strstr(buf, "SLEEP"));
> > > +                strstr(buf, s);
> > >         return check_active ? active : !active;
> > >  }
> > >  
> > > -bool psr_wait_entry(int debugfs_fd)
> > > +bool psr_wait_entry(int debugfs_fd, int psr_mode)
> > >  {
> > > -       return igt_wait(psr_active(debugfs_fd, true), 500, 20);
> > > +       return igt_wait(psr_active(debugfs_fd, true, psr_mode),
> > > 500,
> > > 20);
> > >  }
> > >  
> > > -bool psr_wait_exit(int debugfs_fd)
> > > +bool psr_wait_exit(int debugfs_fd, int psr_mode)
> > >  {
> > >         return igt_wait(psr_active(debugfs_fd, false), 40, 10);
> > >  }
> > > diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> > > index 855679b0..bb90b994 100644
> > > --- a/tests/kms_psr.c
> > > +++ b/tests/kms_psr.c
> > > @@ -199,7 +199,7 @@ static bool psr_wait_entry_if_enabled(data_t
> > > *data)
> > >         if (data->with_psr_disabled)
> > >                 return true;
> > >  
> > > -       return psr_wait_entry(data->debugfs_fd);
> > > +       return psr_wait_entry(data->debugfs_fd, data->psr_mode);
> > >  }
> > >  
> > >  static inline void manual(const char *expected)
> > > @@ -289,7 +289,7 @@ static void run_test(data_t *data)
> > >                 expected = "screen GREEN";
> > >                 break;
> > >         }
> > > -       igt_assert(psr_wait_exit(data->debugfs_fd));
> > > +       igt_assert(psr_wait_exit(data->debugfs_fd, data-
> > > > psr_mode));
> > >         manual(expected);
> > >  }
> > > 
> > > 
> > > 
> > > >  
> > > >  static inline void manual(const char *expected)
> > > > @@ -289,11 +301,16 @@ static void run_test(data_t *data)
> > > >  		expected = "screen GREEN";
> > > >  		break;
> > > >  	}
> > > > -	igt_assert(psr_wait_exit(data->debugfs_fd));
> > > > +
> > > > +	if (!data->op_psr2)
> > > > +		igt_assert(psr_wait_exit(data->debugfs_fd));
> > > > +	else
> > > > +		igt_assert(psr2_wait_update(data->debugfs_fd));
> > > >  	manual(expected);
> > > >  }
> > > >  
> > > > -static void test_cleanup(data_t *data) {
> > > > +static void test_cleanup(data_t *data)
> > > > +{
> > > >  	igt_plane_t *primary;
> > > >  
> > > >  	primary = igt_output_get_plane_type(data->output,
> > > > @@ -304,6 +321,11 @@ static void test_cleanup(data_t *data) {
> > > >  
> > > >  	igt_remove_fb(data->drm_fd, &data->fb_green);
> > > >  	igt_remove_fb(data->drm_fd, &data->fb_white);
> > > > +
> > > > +	/* switch to PSR1 again */
> > > > +	if (data->op_psr2 && !data->with_psr_disabled)
> > > > +		psr_enable(data->debugfs_fd);
> > > > +	data->op_psr2 = false;
> > > >  }
> > > >  
> > > >  static void setup_test_plane(data_t *data, int test_plane)
> > > > @@ -311,6 +333,11 @@ static void setup_test_plane(data_t *data,
> > > > int
> > > > test_plane)
> > > >  	uint32_t white_h, white_v;
> > > >  	igt_plane_t *primary, *sprite, *cursor;
> > > >  
> > > > +	if (data->op_psr2 && !data->with_psr_disabled) {
> > > > +		igt_require(data->supports_psr2);
> > > > +		psr2_enable(data->debugfs_fd);
> > > > +	}
> > > > +
> > > >  	igt_create_color_fb(data->drm_fd,
> > > >  			    data->mode->hdisplay, data->mode-
> > > > >vdisplay,
> > > >  			    DRM_FORMAT_XRGB8888,
> > > > @@ -391,7 +418,7 @@ static int opt_handler(int opt, int
> > > > opt_index,
> > > > void *_data)
> > > >  int main(int argc, char *argv[])
> > > >  {
> > > >  	const char *help_str =
> > > > -	       "  --no-psr\tRun test without PSR.";
> > > > +	       "  --no-psr\tRun test without PSR/PSR2.\n";
> > > >  	static struct option long_options[] = {
> > > >  		{"no-psr", 0, 0, 'n'},
> > > >  		{ 0, 0, 0, 0 }
> > > > @@ -414,6 +441,7 @@ int main(int argc, char *argv[])
> > > >  
> > > >  		igt_require_f(sink_support(&data),
> > > >  			      "Sink does not support PSR\n");
> > > > +		data.supports_psr2 = sink_support_psr2(&data);
> > > >  
> > > >  		data.bufmgr =
> > > > drm_intel_bufmgr_gem_init(data.drm_fd,
> > > > 4096);
> > > >  		igt_assert(data.bufmgr);
> > > > @@ -428,6 +456,13 @@ int main(int argc, char *argv[])
> > > >  		test_cleanup(&data);
> > > >  	}
> > > >  
> > > > +	igt_subtest("psr2_basic") {
> > > > +		data.op_psr2 = true;
> > > > +		setup_test_plane(&data,
> > > > DRM_PLANE_TYPE_PRIMARY);
> > > > +		igt_assert(psr_wait_entry_if_enabled(&data));
> > > > +		test_cleanup(&data);
> > > > +	}
> > > > +
> > > >  	igt_subtest("no_drrs") {
> > > >  		setup_test_plane(&data,
> > > > DRM_PLANE_TYPE_PRIMARY);
> > > >  		igt_assert(psr_wait_entry_if_enabled(&data));
> > > > @@ -435,6 +470,14 @@ int main(int argc, char *argv[])
> > > >  		test_cleanup(&data);
> > > >  	}
> > > >  
> > > > +	igt_subtest("psr2_no_drrs") {
> > > > +		data.op_psr2 = true;
> > > > +		setup_test_plane(&data,
> > > > DRM_PLANE_TYPE_PRIMARY);
> > > > +		igt_assert(psr_wait_entry_if_enabled(&data));
> > > > +		igt_assert(drrs_disabled(&data));
> > > > +		test_cleanup(&data);
> > > > +	}
> > > > +
> > > >  	for (op = PAGE_FLIP; op <= RENDER; op++) {
> > > >  		igt_subtest_f("primary_%s", op_str(op)) {
> > > >  			data.op = op;
> > > > @@ -445,6 +488,17 @@ int main(int argc, char *argv[])
> > > >  		}
> > > >  	}
> > > >  
> > > > +	for (op = PAGE_FLIP; op <= RENDER; op++) {
> > > > +		igt_subtest_f("psr2_primary_%s", op_str(op)) {
> > > > +			data.op_psr2 = true;
> > > > +			data.op = op;
> > > > +			setup_test_plane(&data,
> > > > DRM_PLANE_TYPE_PRIMARY);
> > > > +			igt_assert(psr_wait_entry_if_enabled(&d
> > > > ata));
> > > > +			run_test(&data);
> > > > +			test_cleanup(&data);
> > > > +		}
> > > > +	}
> > > > +
> > > >  	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
> > > >  		igt_subtest_f("sprite_%s", op_str(op)) {
> > > >  			data.op = op;
> > > > @@ -455,6 +509,17 @@ int main(int argc, char *argv[])
> > > >  		}
> > > >  	}
> > > >  
> > > > +	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
> > > > +		igt_subtest_f("psr2_sprite_%s", op_str(op)) {
> > > > +			data.op_psr2 = true;
> > > > +			data.op = op;
> > > > +			setup_test_plane(&data,
> > > > DRM_PLANE_TYPE_OVERLAY);
> > > > +			igt_assert(psr_wait_entry_if_enabled(&d
> > > > ata));
> > > > +			run_test(&data);
> > > > +			test_cleanup(&data);
> > > > +		}
> > > > +	}
> > > > +
> > > >  	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
> > > >  		igt_subtest_f("cursor_%s", op_str(op)) {
> > > >  			data.op = op;
> > > > @@ -465,6 +530,17 @@ int main(int argc, char *argv[])
> > > >  		}
> > > >  	}
> > > >  
> > > > +	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
> > > > +		igt_subtest_f("psr2_cursor_%s", op_str(op)) {
> > > > +			data.op_psr2 = true;
> > > > +			data.op = op;
> > > > +			setup_test_plane(&data,
> > > > DRM_PLANE_TYPE_CURSOR);
> > > > +			igt_assert(psr_wait_entry_if_enabled(&d
> > > > ata));
> > > > +			run_test(&data);
> > > > +			test_cleanup(&data);
> > > > +		}
> > > > +	}
> > > > +
> > > >  	igt_subtest_f("dpms") {
> > > >  		data.op = RENDER;
> > > >  		setup_test_plane(&data,
> > > > DRM_PLANE_TYPE_PRIMARY);
> > > > @@ -474,6 +550,16 @@ int main(int argc, char *argv[])
> > > >  		test_cleanup(&data);
> > > >  	}
> > > >  
> > > > +	igt_subtest_f("psr2_dpms") {
> > > > +		data.op_psr2 = true;
> > > > +		data.op = RENDER;
> > > > +		setup_test_plane(&data,
> > > > DRM_PLANE_TYPE_PRIMARY);
> > > > +		igt_assert(psr_wait_entry_if_enabled(&data));
> > > > +		dpms_off_on(&data);
> > > > +		run_test(&data);
> > > > +		test_cleanup(&data);
> > > > +	}
> > > > +
> > > >  	igt_subtest_f("suspend") {
> > > >  		data.op = PLANE_ONOFF;
> > > >  		setup_test_plane(&data, DRM_PLANE_TYPE_CURSOR);
> > > > @@ -485,6 +571,18 @@ int main(int argc, char *argv[])
> > > >  		test_cleanup(&data);
> > > >  	}
> > > >  
> > > > +	igt_subtest_f("psr2_suspend") {
> > > > +		data.op_psr2 = true;
> > > > +		data.op = PLANE_ONOFF;
> > > > +		setup_test_plane(&data, DRM_PLANE_TYPE_CURSOR);
> > > > +		igt_assert(psr_wait_entry_if_enabled(&data));
> > > > +		igt_system_suspend_autoresume(SUSPEND_STATE_MEM
> > > > ,
> > > > +					      SUSPEND_TEST_NONE
> > > > );
> > > > +		igt_assert(psr_wait_entry_if_enabled(&data));
> > > > +		run_test(&data);
> > > > +		test_cleanup(&data);
> > > > +	}
> > > > +
> > > >  	igt_fixture {
> > > >  		if (!data.with_psr_disabled)
> > > >  			psr_disable(data.debugfs_fd);