Re: [igt-dev] [PATCH i-g-t 6/8] tests/psr: Add the same test coverage that we have for PSR1 to PSR2

[Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>]

On Tue, 2018-12-04 at 15:09 -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 | 118 +++++++++++++++++++++++++++++++++++++++++++++-
> --
>  1 file changed, 111 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> index 116fe409..d87b8ea1 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,8 @@ typedef struct {
>  	drmModeModeInfo *mode;
>  	igt_output_t *output;
>  	bool with_psr_disabled;
> +	bool with_psr2_disabled;
Do we need this? Is there a use case running PSR2 tests with PSR1
enabled? 


> +	bool supports_psr2;
>  } data_t;
>  
>  static void create_cursor_fb(data_t *data)
> @@ -194,12 +197,22 @@ static bool sink_support(data_t *data)
>  	return data->with_psr_disabled || psr_supported(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_psr2_disabled || psr2_supported(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_psr2_disabled)
> +			return true;
> +		return psr2_wait_deep_sleep(data->debugfs_fd);
> +	}
>  }
>  
>  static inline void manual(const char *expected)
> @@ -289,11 +302,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));
Move the PSR1/PSR2 logic inside psr_wait_exit() ?

>  	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 +322,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 +334,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);//op_psr2 is set in
> case this fails?
> +		psr2_enable(data->debugfs_fd);
> +	}
> +
>  	igt_create_color_fb(data->drm_fd,
>  			    data->mode->hdisplay, data->mode->vdisplay,
>  			    DRM_FORMAT_XRGB8888,
> @@ -381,6 +409,9 @@ static int opt_handler(int opt, int opt_index,
> void *_data)
>  	case 'n':
>  		data->with_psr_disabled = true;
>  		break;
> +	case 'm':
> +		data->with_psr2_disabled = true;
> +		break;
>  	default:
>  		igt_assert(0);
>  	}
> @@ -391,9 +422,11 @@ 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.\n"
> +	       "  --no-psr2\tRun test without PSR2.";
>  	static struct option long_options[] = {
>  		{"no-psr", 0, 0, 'n'},
> +		{"no-psr2", 0, 0, 'm'},
>  		{ 0, 0, 0, 0 }
>  	};
>  	data_t data = {};
> @@ -414,6 +447,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 +462,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 +476,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 +494,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 +515,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 +536,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 +556,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 +577,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);
> +	}
> +
How about using a loop to iterate PSR1 and PSR2 modes?
>  	igt_fixture {
>  		if (!data.with_psr_disabled)
>  			psr_disable(data.debugfs_fd);

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev