[igt-dev] [PATCH v3 0/3] amdgpu PSR-SU FFU test case

[igt-dev] [PATCH v3 0/3] amdgpu PSR-SU FFU test case

[David Zhang]

changes in v3:
---------------
- define reusable wrapper for PSR support check
- drop the unnecessary helper panning_pfb_and_psrsu_check() and
  collapse the operations into run_check_psr_su_ffu()
- initial the two overlay FBs w/ global alpha and different color
  not to update the specific alpha region for each panning of the
  primary plane.

David Zhang (3):
  tests/amdgpu/amd_psr: add interactive option of PSR visual confirm
  tests/amdgpu/amd_psr: add PSR-SU full frame update sub test case
  tests/amdgpu/amd_psr: define reusable wrapper for PSR support check

 tests/amdgpu/amd_psr.c | 177 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 167 insertions(+), 10 deletions(-)

-- 
2.25.1

[igt-dev] [PATCH v3 1/3] tests/amdgpu/amd_psr: add interactive option of PSR visual confirm

[David Zhang]

[why & how]
AMDGPU DM debugfs provides a debug option of PSR visual confirm,
we'd make it an interactive option that provides capability to
usermode to choose if or not to enable the PSR visual confirm.

- add interactive option "visual-confirm" and by-default we select
  it as TRUE to enable visual confirm for PSR-SU test cases.
- set PSR visual confirm debugfs if interactive debug option of
  visual confirm set before any test run begins, and reset after
  any test run ends.

changes in v2:
------------------
- drop the "_dm_" from helpers of visual confirm debugfs set/get
- drop the non-necessary visual confirm option check after set
  and reset, before and after the test run, respectively
- align the 2nd line code w/ previous line's opening brace

Cc: Rodrigo Siqueira <rodrigo.siqueira@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Leo Li <sunpeng.li@amd.com>
Cc: Jay Pillai <aurabindo.pillai@amd.com>
Cc: Wayne Lin <wayne.lin@amd.com>

Signed-off-by: David Zhang <dingchen.zhang@amd.com>
---
 tests/amdgpu/amd_psr.c | 42 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/tests/amdgpu/amd_psr.c b/tests/amdgpu/amd_psr.c
index 58e40d2b..0ec50c9f 100644
--- a/tests/amdgpu/amd_psr.c
+++ b/tests/amdgpu/amd_psr.c
@@ -62,6 +62,12 @@ typedef struct data {
 	int h;
 } data_t;
 
+struct {
+	bool visual_confirm;
+} opt = {
+	.visual_confirm = true,	/* visual confirm debug option */
+};
+
 static void draw_color_alpha(igt_fb_t *fb, int x, int y, int w, int h,
 		             double r, double g, double b, double a)
 {
@@ -300,10 +306,31 @@ static void run_check_psr_su_mpo(data_t *data)
 	for (int i = 0; i < N_MPO_TEST_RECT_FB; ++i)
 		igt_remove_fb(data->fd, &rect_fb[i]);
 	test_fini(data);
-	close(data->fd);
 }
 
-igt_main
+const char *help_str =
+"  --visual-confirm           PSR visual confirm debug option enable\n";
+
+struct option long_options[] = {
+	{"visual-confirm",	optional_argument, NULL, 'v'},
+	{ 0, 0, 0, 0 }
+};
+
+static int opt_handler(int option, int option_index, void *data)
+{
+	switch (option) {
+	case 'v':
+		opt.visual_confirm = strtol(optarg, NULL, 0);
+		igt_info(" PSR Visual Confirm %s\n", opt.visual_confirm ? "enabled" : "disabled");
+		break;
+	default:
+		return IGT_OPT_HANDLER_ERROR;
+	}
+
+	return IGT_OPT_HANDLER_SUCCESS;
+}
+
+igt_main_args("", long_options, help_str, opt_handler, NULL)
 {
 	data_t data;
 
@@ -321,6 +348,13 @@ igt_main
 		igt_display_require(&data.display, data.fd);
 		igt_require(&data.display.is_atomic);
 		igt_display_require_output(&data.display);
+
+		/* check if visual confirm option available */
+		if (opt.visual_confirm) {
+			igt_skip_on(!igt_amd_has_visual_confirm(data.fd));
+			igt_skip_on_f(!igt_amd_set_visual_confirm(data.fd, VISUAL_CONFIRM_PSR),
+				      "set PSR visual confirm failed\n");
+		}
 	}
 
 	igt_describe("Test whether PSR can be enabled with static screen");
@@ -335,6 +369,10 @@ igt_main
 
 	igt_fixture
 	{
+		if (opt.visual_confirm) {
+			igt_require_f(igt_amd_set_visual_confirm(data.fd, VISUAL_CONFIRM_DISABLE),
+				      "reset PSR visual confirm option failed\n");
+		}
 		close(data.debugfs_fd);
 		igt_display_fini(&data.display);
 	}
-- 
2.25.1

[igt-dev] [PATCH v3 2/3] tests/amdgpu/amd_psr: add PSR-SU full frame update sub test case

[David Zhang]

[why]
We need a full-frame update (FFU) test case to validate PSR-SU
feature enablement by visual confirm.

[how]
1. create two overlay FBs with full screen size and one primary FB
   w/ 1/4 screen size
2. panning the primary plane to top-left and flip for couple of
   frames
3. wait for couple of seconds to allow visual confirm
4. panning the primary plane from top-left to middle of screen
5. repeat step 3
6. panning the primary plane from middle to bottom-right of screen
7. repeat step 3

If the PSR-SU is enabled as expected, for FFU we'd observe the
visual confirm by seeing the blue border at the bottom of screen
and the bottom of primary plane (megenta rectangle) as well.

Changes in v2:
----------------
- drop the unnecessary helper panning_pfb_and_psrsu_check() and
  collapse the operations into run_check_psr_su_ffu()
- initial the two overlay FBs w/ global alpha and different color
  not to update the specific alpha region for each panning of the
  primary plane.

Cc: Rodrigo Siqueira <rodrigo.siqueira@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Leo Li <sunpeng.li@amd.com>
Cc: Jay Pillai <aurabindo.pillai@amd.com>
Cc: Wayne Lin <wayne.lin@amd.com>

Signed-off-by: David Zhang <dingchen.zhang@amd.com>
---
 tests/amdgpu/amd_psr.c | 111 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 111 insertions(+)

diff --git a/tests/amdgpu/amd_psr.c b/tests/amdgpu/amd_psr.c
index 0ec50c9f..92628fee 100644
--- a/tests/amdgpu/amd_psr.c
+++ b/tests/amdgpu/amd_psr.c
@@ -54,6 +54,7 @@ typedef struct data {
 	igt_output_t *output;
 	igt_pipe_t *pipe;
 	igt_pipe_crc_t *pipe_crc;
+	igt_fb_t ov_fb[2];
 	drmModeModeInfo *mode;
 	enum pipe pipe_id;
 	int fd;
@@ -75,6 +76,7 @@ static void draw_color_alpha(igt_fb_t *fb, int x, int y, int w, int h,
 
 	cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE);
 	igt_paint_color_alpha(cr, x, y, w, h, r, g, b, a);
+	cairo_set_operator(cr, CAIRO_OPERATOR_OVER);
 
 	igt_put_cairo_ctx(cr);
 }
@@ -308,6 +310,111 @@ static void run_check_psr_su_mpo(data_t *data)
 	test_fini(data);
 }
 
+static void panning_rect_fb(data_t *data, igt_fb_t *rect_fb, int rect_w, int rect_h, int curr_x, int curr_y)
+{
+	int ret;
+
+	/* set new position for primary plane */
+	igt_plane_set_position(data->primary, curr_x, curr_y);
+
+	/* flip overlay for couple of frames */
+	igt_info("\n start flipping ...\n");
+	for (int i = 0; i < N_FLIPS; ++i) {
+		igt_info(" About to commit overlay w/ alpha updated in region (x=%d, y=%d, w=%d, h=%d), loop %d \n",
+			 curr_x, curr_y, rect_w, rect_h, i);
+
+		/* do flip overlay */
+		igt_plane_set_fb(data->overlay, &data->ov_fb[i % 2]);
+		igt_plane_set_fb(data->primary, rect_fb);
+		igt_plane_set_size(data->primary, rect_w, rect_h);
+		igt_output_set_pipe(data->output, data->pipe_id);
+
+		ret = igt_display_try_commit_atomic(&data->display, DRM_MODE_PAGE_FLIP_EVENT, NULL);
+		igt_require(ret == 0);
+		kmstest_wait_for_pageflip(data->fd);
+	}
+}
+
+static void run_check_psr_su_ffu(data_t *data)
+{
+	int edp_idx = check_conn_type(data, DRM_MODE_CONNECTOR_eDP);
+	bool sink_support_psrsu = false;
+	bool drv_suport_psrsu = false;
+	igt_fb_t rect_fb; 	// rectangle fbs for primary
+	igt_fb_t ref_fb;	// reference fb
+	int pb_w, pb_h, ob_w, ob_h;
+
+	/* skip the test run if no eDP sink detected */
+	igt_skip_on_f(edp_idx == -1, "no eDP connector found\n");
+
+	/* init */
+	test_init(data);
+	ob_w = data->w;
+	ob_h = data->h;
+	pb_w = data->w / 2;
+	pb_h = data->h / 2;
+
+	/* run the test i.i.f. eDP panel supports and kernel driver both support PSR-SU  */
+	igt_skip_on(!igt_amd_output_has_psr_cap(data->fd, data->output->name));
+	igt_skip_on(!igt_amd_output_has_psr_state(data->fd, data->output->name));
+	sink_support_psrsu = igt_amd_psr_support_sink(data->fd, data->output->name, PSR_MODE_2);
+	igt_skip_on_f(!sink_support_psrsu, "output %s not support PSR-SU\n", data->output->name);
+	drv_suport_psrsu = igt_amd_psr_support_drv(data->fd, data->output->name, PSR_MODE_2);
+	igt_skip_on_f(!drv_suport_psrsu, "kernel driver not support PSR-SU\n");
+
+	/* reference background pattern in grey */
+	igt_create_color_fb(data->fd, data->w, data->h, DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
+			    .5, .5, .5, &ref_fb);
+	igt_plane_set_fb(data->primary, &ref_fb);
+	igt_output_set_pipe(data->output, data->pipe_id);
+	igt_display_commit_atomic(&data->display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
+
+	/*
+	 * overlay and primary fbs creation
+	 * for full frame update (FFU) test case, we don't change primary FB content but to change
+	 * the position of primary FB (panning) and update the overlay plane alpha region.
+	 * Any overlay change is expected to be regarded as FFU from KMD's perspective.
+	 *
+	 * 1. create two overlay FBs with full screen size and one primary FB w/ 1/4 screen size
+	 * 2. panning the primary plane to top-left and flip for couple of frames
+	 * 3. wait for couple of seconds to allow visual confirm
+	 * 4. panning the primary plane from top-left to middle of screen
+	 * 5. repeat step 3
+	 * 6. panning the primary plane from middle to bottom-right of screen
+	 * 7. repeat step 3
+	 */
+
+	/* step 1 */
+	igt_create_color_fb(data->fd, ob_w, ob_h, DRM_FORMAT_ARGB8888, DRM_FORMAT_MOD_LINEAR,
+			    1.0, 1.0, 1.0, &data->ov_fb[0]);
+	draw_color_alpha(&data->ov_fb[0], 0, 0, ob_w, ob_h, 1.0, 1.0, 1.0, .3);
+	igt_create_color_fb(data->fd, ob_w, ob_h, DRM_FORMAT_ARGB8888, DRM_FORMAT_MOD_LINEAR,
+			    1.0, 1.0, 1.0, &data->ov_fb[1]);
+	draw_color_alpha(&data->ov_fb[1], 0, 0, ob_w, ob_h, .0, 1.0, .0, .3);
+
+	igt_create_color_fb(data->fd, pb_w, pb_h, DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
+			    1.0, .0, 1.0, &rect_fb);
+
+	/* step 2 & 3 */
+	panning_rect_fb(data, &rect_fb, pb_w, pb_h, 0, 0);
+	sleep(5);
+
+	/* step 4 & 5 */
+	panning_rect_fb(data, &rect_fb, pb_w, pb_h, pb_w / 2, pb_h / 2);
+	sleep(5);
+
+	/* step 6 & 7 */
+	panning_rect_fb(data, &rect_fb, pb_w, pb_h, pb_w, pb_h);
+	sleep(5);
+
+	/* fini */
+	igt_remove_fb(data->fd, &ref_fb);
+	igt_remove_fb(data->fd, &data->ov_fb[0]);
+	igt_remove_fb(data->fd, &data->ov_fb[1]);
+	igt_remove_fb(data->fd, &rect_fb);
+	test_fini(data);
+}
+
 const char *help_str =
 "  --visual-confirm           PSR visual confirm debug option enable\n";
 
@@ -367,6 +474,10 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
 		     "and to imitate Multiplane Overlay video playback scenario");
 	igt_subtest("psr_su_mpo") run_check_psr_su_mpo(&data);
 
+	igt_describe("Test to validate PSR SU enablement with Visual Confirm "
+		     "and to validate Full Frame Update scenario");
+	igt_subtest("psr_su_ffu") run_check_psr_su_ffu(&data);
+
 	igt_fixture
 	{
 		if (opt.visual_confirm) {
-- 
2.25.1

[igt-dev] [PATCH v3 3/3] tests/amdgpu/amd_psr: define reusable wrapper for PSR support check

[David Zhang]

[why & how]
For amdgpu PSR-SU test cases, we'd first check if the sink device
and amdgpu driver support the PSR-SU feature at the beginning. We
define a local helper to wrap the check for PSR-SU support from
sink and driver as well and called by both MPO and FFU test case,
to avoid the code repetition.

Cc: Rodrigo Siqueira <rodrigo.siqueira@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Leo Li <sunpeng.li@amd.com>
Cc: Jay Pillai <aurabindo.pillai@amd.com>
Cc: Wayne Lin <wayne.lin@amd.com>

Signed-off-by: David Zhang <dingchen.zhang@amd.com>
---
 tests/amdgpu/amd_psr.c | 40 ++++++++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/tests/amdgpu/amd_psr.c b/tests/amdgpu/amd_psr.c
index 92628fee..e7614d71 100644
--- a/tests/amdgpu/amd_psr.c
+++ b/tests/amdgpu/amd_psr.c
@@ -138,6 +138,28 @@ static int check_conn_type(data_t *data, uint32_t type) {
 	return -1;
 }
 
+static bool psr_su_supported(data_t *data)
+{
+	/* run PSR-SU test i.i.f. eDP panel and kernel driver both support PSR-SU */
+	if (!igt_amd_output_has_psr_cap(data->fd, data->output->name))
+		return false;
+
+	if (!igt_amd_output_has_psr_state(data->fd, data->output->name))
+		return false;
+
+	if (!igt_amd_psr_support_sink(data->fd, data->output->name, PSR_MODE_2)) {
+		igt_info(" output %s not support PSR-SU\n", data->output->name);
+		return false;
+	}
+
+	if (!igt_amd_psr_support_drv(data->fd, data->output->name, PSR_MODE_2)) {
+		igt_info(" kernel driver not support PSR-SU\n");
+		return false;
+	}
+
+	return true;
+}
+
 static void run_check_psr(data_t *data, bool test_null_crtc) {
 	int fd, edp_idx, dp_idx, ret, i, psr_state;
 	igt_fb_t ref_fb, ref_fb2;
@@ -217,8 +239,6 @@ static void run_check_psr(data_t *data, bool test_null_crtc) {
 static void run_check_psr_su_mpo(data_t *data)
 {
 	int edp_idx = check_conn_type(data, DRM_MODE_CONNECTOR_eDP);
-	bool sink_support_psrsu = false;
-	bool drv_suport_psrsu = false;
 	igt_fb_t ov_fb;		// fb for overlay
 	igt_fb_t rect_fb[N_MPO_TEST_RECT_FB]; 	// rectangle fbs for primary, emulate as video playback region
 	igt_fb_t ref_fb;	// reference fb
@@ -235,12 +255,7 @@ static void run_check_psr_su_mpo(data_t *data)
 	frame_rate = data->mode->vrefresh;
 
 	/* run the test i.i.f. eDP panel supports and kernel driver both support PSR-SU  */
-	igt_skip_on(!igt_amd_output_has_psr_cap(data->fd, data->output->name));
-	igt_skip_on(!igt_amd_output_has_psr_state(data->fd, data->output->name));
-	sink_support_psrsu = igt_amd_psr_support_sink(data->fd, data->output->name, PSR_MODE_2);
-	igt_skip_on_f(!sink_support_psrsu, "output %s not support PSR-SU\n", data->output->name);
-	drv_suport_psrsu = igt_amd_psr_support_drv(data->fd, data->output->name, PSR_MODE_2);
-	igt_skip_on_f(!drv_suport_psrsu, "kernel driver not support PSR-SU\n");
+	igt_skip_on(!psr_su_supported(data));
 
 	/* reference background pattern in grey */
 	igt_create_color_fb(data->fd, data->w, data->h, DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
@@ -338,8 +353,6 @@ static void panning_rect_fb(data_t *data, igt_fb_t *rect_fb, int rect_w, int rec
 static void run_check_psr_su_ffu(data_t *data)
 {
 	int edp_idx = check_conn_type(data, DRM_MODE_CONNECTOR_eDP);
-	bool sink_support_psrsu = false;
-	bool drv_suport_psrsu = false;
 	igt_fb_t rect_fb; 	// rectangle fbs for primary
 	igt_fb_t ref_fb;	// reference fb
 	int pb_w, pb_h, ob_w, ob_h;
@@ -355,12 +368,7 @@ static void run_check_psr_su_ffu(data_t *data)
 	pb_h = data->h / 2;
 
 	/* run the test i.i.f. eDP panel supports and kernel driver both support PSR-SU  */
-	igt_skip_on(!igt_amd_output_has_psr_cap(data->fd, data->output->name));
-	igt_skip_on(!igt_amd_output_has_psr_state(data->fd, data->output->name));
-	sink_support_psrsu = igt_amd_psr_support_sink(data->fd, data->output->name, PSR_MODE_2);
-	igt_skip_on_f(!sink_support_psrsu, "output %s not support PSR-SU\n", data->output->name);
-	drv_suport_psrsu = igt_amd_psr_support_drv(data->fd, data->output->name, PSR_MODE_2);
-	igt_skip_on_f(!drv_suport_psrsu, "kernel driver not support PSR-SU\n");
+	igt_skip_on(!psr_su_supported(data));
 
 	/* reference background pattern in grey */
 	igt_create_color_fb(data->fd, data->w, data->h, DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
-- 
2.25.1

[igt-dev] ✗ GitLab.Pipeline: warning for AMDGPU PSR-SU FFU test case (rev2)

[Patchwork]

== Series Details ==

Series: AMDGPU PSR-SU FFU test case (rev2)
URL   : https://patchwork.freedesktop.org/series/102345/
State : warning

== Summary ==

Pipeline status: FAILED.

see https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/558245 for the overview.

test:ninja-test-armhf has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/21153959):
  Ok:                   22
  Expected Fail:         3
  Fail:                289
  Unexpected Pass:       0
  Skipped:               0
  Timeout:               0
  
  Full log written to /builds/gfx-ci/igt-ci-tags/build/meson-logs/testlog.txt
  section_end:1649847370:step_script
  section_start:1649847370:upload_artifacts_on_failure
  Uploading artifacts for failed job
  Uploading artifacts...
  build: found 1724 matching files and directories   
  Uploading artifacts as "archive" to coordinator... 201 Created  id=21153959 responseStatus=201 Created token=ZY8gGZXX
  section_end:1649847380:upload_artifacts_on_failure
  section_start:1649847380:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1649847380:cleanup_file_variables
  ERROR: Job failed: exit code 1

== Logs ==

For more details see: https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/558245

[igt-dev] ✗ Fi.CI.BAT: failure for AMDGPU PSR-SU FFU test case (rev2)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 9082 bytes --]

== Series Details ==

Series: AMDGPU PSR-SU FFU test case (rev2)
URL   : https://patchwork.freedesktop.org/series/102345/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_11493 -> IGTPW_6921
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_6921 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_6921, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6921/index.html

Participating hosts (48 -> 48)
------------------------------

  Additional (3): fi-cml-u2 bat-adlm-1 fi-pnv-d510 
  Missing    (3): fi-bsw-cyan fi-icl-u2 fi-bdw-samus 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_6921:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_pm_rpm@basic-rte:
    - fi-cml-u2:          NOTRUN -> [INCOMPLETE][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6921/fi-cml-u2/igt@i915_pm_rpm@basic-rte.html

  * igt@i915_selftest@live@gem_contexts:
    - fi-bdw-5557u:       [PASS][2] -> [INCOMPLETE][3]
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11493/fi-bdw-5557u/igt@i915_selftest@live@gem_contexts.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6921/fi-bdw-5557u/igt@i915_selftest@live@gem_contexts.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@i915_module_load@reload:
    - {bat-adlm-1}:       NOTRUN -> [DMESG-WARN][4]
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6921/bat-adlm-1/igt@i915_module_load@reload.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - {bat-dg2-9}:        [DMESG-WARN][5] ([i915#5193]) -> [INCOMPLETE][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11493/bat-dg2-9/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6921/bat-dg2-9/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  
Known issues
------------

  Here are the changes found in IGTPW_6921 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@semaphore:
    - fi-hsw-4770:        NOTRUN -> [SKIP][7] ([fdo#109271] / [fdo#109315]) +17 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6921/fi-hsw-4770/igt@amdgpu/amd_basic@semaphore.html

  * igt@gem_exec_fence@basic-busy@bcs0:
    - fi-cml-u2:          NOTRUN -> [SKIP][8] ([i915#1208]) +1 similar issue
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6921/fi-cml-u2/igt@gem_exec_fence@basic-busy@bcs0.html

  * igt@gem_huc_copy@huc-copy:
    - fi-pnv-d510:        NOTRUN -> [SKIP][9] ([fdo#109271]) +57 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6921/fi-pnv-d510/igt@gem_huc_copy@huc-copy.html
    - fi-cml-u2:          NOTRUN -> [SKIP][10] ([i915#2190])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6921/fi-cml-u2/igt@gem_huc_copy@huc-copy.html

  * igt@i915_pm_rpm@module-reload:
    - fi-cfl-8109u:       [PASS][11] -> [DMESG-WARN][12] ([i915#62])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11493/fi-cfl-8109u/igt@i915_pm_rpm@module-reload.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6921/fi-cfl-8109u/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live@hangcheck:
    - fi-snb-2600:        [PASS][13] -> [INCOMPLETE][14] ([i915#3921])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11493/fi-snb-2600/igt@i915_selftest@live@hangcheck.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6921/fi-snb-2600/igt@i915_selftest@live@hangcheck.html

  * igt@kms_chamelium@dp-hpd-fast:
    - fi-cml-u2:          NOTRUN -> [SKIP][15] ([fdo#109284] / [fdo#111827]) +8 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6921/fi-cml-u2/igt@kms_chamelium@dp-hpd-fast.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - fi-cml-u2:          NOTRUN -> [SKIP][16] ([fdo#109278]) +1 similar issue
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6921/fi-cml-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-cml-u2:          NOTRUN -> [SKIP][17] ([fdo#109285])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6921/fi-cml-u2/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c:
    - fi-pnv-d510:        NOTRUN -> [SKIP][18] ([fdo#109271] / [i915#5341])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6921/fi-pnv-d510/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
    - fi-cml-u2:          NOTRUN -> [SKIP][19] ([fdo#109278] / [i915#533])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6921/fi-cml-u2/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - fi-cml-u2:          NOTRUN -> [SKIP][20] ([i915#3555])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6921/fi-cml-u2/igt@kms_setmode@basic-clone-single-crtc.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@hangcheck:
    - fi-hsw-4770:        [INCOMPLETE][21] ([i915#4785]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11493/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6921/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html

  * igt@kms_flip@basic-flip-vs-modeset@a-edp1:
    - {bat-adlp-6}:       [DMESG-WARN][23] ([i915#3576]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11493/bat-adlp-6/igt@kms_flip@basic-flip-vs-modeset@a-edp1.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6921/bat-adlp-6/igt@kms_flip@basic-flip-vs-modeset@a-edp1.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109308]: https://bugs.freedesktop.org/show_bug.cgi?id=109308
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1155]: https://gitlab.freedesktop.org/drm/intel/issues/1155
  [i915#1208]: https://gitlab.freedesktop.org/drm/intel/issues/1208
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
  [i915#2867]: https://gitlab.freedesktop.org/drm/intel/issues/2867
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3576]: https://gitlab.freedesktop.org/drm/intel/issues/3576
  [i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#3921]: https://gitlab.freedesktop.org/drm/intel/issues/3921
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4391]: https://gitlab.freedesktop.org/drm/intel/issues/4391
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4785]: https://gitlab.freedesktop.org/drm/intel/issues/4785
  [i915#4897]: https://gitlab.freedesktop.org/drm/intel/issues/4897
  [i915#5193]: https://gitlab.freedesktop.org/drm/intel/issues/5193
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
  [i915#5341]: https://gitlab.freedesktop.org/drm/intel/issues/5341
  [i915#5650]: https://gitlab.freedesktop.org/drm/intel/issues/5650
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_6420 -> IGTPW_6921

  CI-20190529: 20190529
  CI_DRM_11493: 83f019abc2a24a2753dea6eb4416a8210c79adf1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_6921: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6921/index.html
  IGT_6420: a3885810ccc0ce9e6552a20c910a0a322eca466c @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git



== Testlist changes ==

+igt@amdgpu/amd_psr@psr_su_ffu

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6921/index.html

[-- Attachment #2: Type: text/html, Size: 9227 bytes --]

Re: [igt-dev] [PATCH v3 1/3] tests/amdgpu/amd_psr: add interactive option of PSR visual confirm

[Leo Li]

On 2022-04-12 15:21, David Zhang wrote:
> [why & how]
> AMDGPU DM debugfs provides a debug option of PSR visual confirm,
> we'd make it an interactive option that provides capability to
> usermode to choose if or not to enable the PSR visual confirm.
> 
> - add interactive option "visual-confirm" and by-default we select
>    it as TRUE to enable visual confirm for PSR-SU test cases.
> - set PSR visual confirm debugfs if interactive debug option of
>    visual confirm set before any test run begins, and reset after
>    any test run ends.
> 
> changes in v2:
> ------------------
> - drop the "_dm_" from helpers of visual confirm debugfs set/get
> - drop the non-necessary visual confirm option check after set
>    and reset, before and after the test run, respectively
> - align the 2nd line code w/ previous line's opening brace
> 
> Cc: Rodrigo Siqueira <rodrigo.siqueira@amd.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Leo Li <sunpeng.li@amd.com>
> Cc: Jay Pillai <aurabindo.pillai@amd.com>
> Cc: Wayne Lin <wayne.lin@amd.com>
> 
> Signed-off-by: David Zhang <dingchen.zhang@amd.com>
Reviewed-by: Leo Li <sunpeng.li@amd.com>

Thanks!
Leo

> ---
>   tests/amdgpu/amd_psr.c | 42 ++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/amdgpu/amd_psr.c b/tests/amdgpu/amd_psr.c
> index 58e40d2b..0ec50c9f 100644
> --- a/tests/amdgpu/amd_psr.c
> +++ b/tests/amdgpu/amd_psr.c
> @@ -62,6 +62,12 @@ typedef struct data {
>   	int h;
>   } data_t;
>   
> +struct {
> +	bool visual_confirm;
> +} opt = {
> +	.visual_confirm = true,	/* visual confirm debug option */
> +};
> +
>   static void draw_color_alpha(igt_fb_t *fb, int x, int y, int w, int h,
>   		             double r, double g, double b, double a)
>   {
> @@ -300,10 +306,31 @@ static void run_check_psr_su_mpo(data_t *data)
>   	for (int i = 0; i < N_MPO_TEST_RECT_FB; ++i)
>   		igt_remove_fb(data->fd, &rect_fb[i]);
>   	test_fini(data);
> -	close(data->fd);
>   }
>   
> -igt_main
> +const char *help_str =
> +"  --visual-confirm           PSR visual confirm debug option enable\n";
> +
> +struct option long_options[] = {
> +	{"visual-confirm",	optional_argument, NULL, 'v'},
> +	{ 0, 0, 0, 0 }
> +};
> +
> +static int opt_handler(int option, int option_index, void *data)
> +{
> +	switch (option) {
> +	case 'v':
> +		opt.visual_confirm = strtol(optarg, NULL, 0);
> +		igt_info(" PSR Visual Confirm %s\n", opt.visual_confirm ? "enabled" : "disabled");
> +		break;
> +	default:
> +		return IGT_OPT_HANDLER_ERROR;
> +	}
> +
> +	return IGT_OPT_HANDLER_SUCCESS;
> +}
> +
> +igt_main_args("", long_options, help_str, opt_handler, NULL)
>   {
>   	data_t data;
>   
> @@ -321,6 +348,13 @@ igt_main
>   		igt_display_require(&data.display, data.fd);
>   		igt_require(&data.display.is_atomic);
>   		igt_display_require_output(&data.display);
> +
> +		/* check if visual confirm option available */
> +		if (opt.visual_confirm) {
> +			igt_skip_on(!igt_amd_has_visual_confirm(data.fd));
> +			igt_skip_on_f(!igt_amd_set_visual_confirm(data.fd, VISUAL_CONFIRM_PSR),
> +				      "set PSR visual confirm failed\n");
> +		}
>   	}
>   
>   	igt_describe("Test whether PSR can be enabled with static screen");
> @@ -335,6 +369,10 @@ igt_main
>   
>   	igt_fixture
>   	{
> +		if (opt.visual_confirm) {
> +			igt_require_f(igt_amd_set_visual_confirm(data.fd, VISUAL_CONFIRM_DISABLE),
> +				      "reset PSR visual confirm option failed\n");
> +		}
>   		close(data.debugfs_fd);
>   		igt_display_fini(&data.display);
>   	}

Re: [igt-dev] [PATCH v3 2/3] tests/amdgpu/amd_psr: add PSR-SU full frame update sub test case

[Leo Li]

On 2022-04-12 15:21, David Zhang wrote:
> [why]
> We need a full-frame update (FFU) test case to validate PSR-SU
> feature enablement by visual confirm.
> 
> [how]
> 1. create two overlay FBs with full screen size and one primary FB
>     w/ 1/4 screen size
> 2. panning the primary plane to top-left and flip for couple of
>     frames
> 3. wait for couple of seconds to allow visual confirm
> 4. panning the primary plane from top-left to middle of screen
> 5. repeat step 3
> 6. panning the primary plane from middle to bottom-right of screen
> 7. repeat step 3
> 
> If the PSR-SU is enabled as expected, for FFU we'd observe the
> visual confirm by seeing the blue border at the bottom of screen
> and the bottom of primary plane (megenta rectangle) as well.
> 
> Changes in v2:
> ----------------
> - drop the unnecessary helper panning_pfb_and_psrsu_check() and
>    collapse the operations into run_check_psr_su_ffu()
> - initial the two overlay FBs w/ global alpha and different color
>    not to update the specific alpha region for each panning of the
>    primary plane.
> 
> Cc: Rodrigo Siqueira <rodrigo.siqueira@amd.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Leo Li <sunpeng.li@amd.com>
> Cc: Jay Pillai <aurabindo.pillai@amd.com>
> Cc: Wayne Lin <wayne.lin@amd.com>
> 
> Signed-off-by: David Zhang <dingchen.zhang@amd.com>
> ---
>   tests/amdgpu/amd_psr.c | 111 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 111 insertions(+)
> 
> diff --git a/tests/amdgpu/amd_psr.c b/tests/amdgpu/amd_psr.c
> index 0ec50c9f..92628fee 100644
> --- a/tests/amdgpu/amd_psr.c
> +++ b/tests/amdgpu/amd_psr.c
> @@ -54,6 +54,7 @@ typedef struct data {
>   	igt_output_t *output;
>   	igt_pipe_t *pipe;
>   	igt_pipe_crc_t *pipe_crc;
> +	igt_fb_t ov_fb[2];
>   	drmModeModeInfo *mode;
>   	enum pipe pipe_id;
>   	int fd;
> @@ -75,6 +76,7 @@ static void draw_color_alpha(igt_fb_t *fb, int x, int y, int w, int h,
>   
>   	cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE);
>   	igt_paint_color_alpha(cr, x, y, w, h, r, g, b, a);
> +	cairo_set_operator(cr, CAIRO_OPERATOR_OVER);
>   
>   	igt_put_cairo_ctx(cr);
>   }
> @@ -308,6 +310,111 @@ static void run_check_psr_su_mpo(data_t *data)
>   	test_fini(data);
>   }
>   
> +static void panning_rect_fb(data_t *data, igt_fb_t *rect_fb, int rect_w, int rect_h, int curr_x, int curr_y)
> +{
> +	int ret;
> +
> +	/* set new position for primary plane */
> +	igt_plane_set_position(data->primary, curr_x, curr_y);
> +
> +	/* flip overlay for couple of frames */
> +	igt_info("\n start flipping ...\n");
> +	for (int i = 0; i < N_FLIPS; ++i) {
> +		igt_info(" About to commit overlay w/ alpha updated in region (x=%d, y=%d, w=%d, h=%d), loop %d \n",
> +			 curr_x, curr_y, rect_w, rect_h, i);
> +
> +		/* do flip overlay */
> +		igt_plane_set_fb(data->overlay, &data->ov_fb[i % 2]);
> +		igt_plane_set_fb(data->primary, rect_fb);
> +		igt_plane_set_size(data->primary, rect_w, rect_h);
> +		igt_output_set_pipe(data->output, data->pipe_id);
> +
> +		ret = igt_display_try_commit_atomic(&data->display, DRM_MODE_PAGE_FLIP_EVENT, NULL);
> +		igt_require(ret == 0);
> +		kmstest_wait_for_pageflip(data->fd);
> +	}
> +}
> +
> +static void run_check_psr_su_ffu(data_t *data)
> +{
> +	int edp_idx = check_conn_type(data, DRM_MODE_CONNECTOR_eDP);
> +	bool sink_support_psrsu = false;
> +	bool drv_suport_psrsu = false;
> +	igt_fb_t rect_fb; 	// rectangle fbs for primary
> +	igt_fb_t ref_fb;	// reference fb
> +	int pb_w, pb_h, ob_w, ob_h;
> +
> +	/* skip the test run if no eDP sink detected */
> +	igt_skip_on_f(edp_idx == -1, "no eDP connector found\n");
> +
> +	/* init */
> +	test_init(data);
> +	ob_w = data->w;
> +	ob_h = data->h;
> +	pb_w = data->w / 2;
> +	pb_h = data->h / 2;
> +
> +	/* run the test i.i.f. eDP panel supports and kernel driver both support PSR-SU  */
> +	igt_skip_on(!igt_amd_output_has_psr_cap(data->fd, data->output->name));
> +	igt_skip_on(!igt_amd_output_has_psr_state(data->fd, data->output->name));
> +	sink_support_psrsu = igt_amd_psr_support_sink(data->fd, data->output->name, PSR_MODE_2);
> +	igt_skip_on_f(!sink_support_psrsu, "output %s not support PSR-SU\n", data->output->name);
> +	drv_suport_psrsu = igt_amd_psr_support_drv(data->fd, data->output->name, PSR_MODE_2);
> +	igt_skip_on_f(!drv_suport_psrsu, "kernel driver not support PSR-SU\n");
> +
> +	/* reference background pattern in grey */
> +	igt_create_color_fb(data->fd, data->w, data->h, DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
> +			    .5, .5, .5, &ref_fb);
> +	igt_plane_set_fb(data->primary, &ref_fb);
> +	igt_output_set_pipe(data->output, data->pipe_id);
> +	igt_display_commit_atomic(&data->display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> +
> +	/*
> +	 * overlay and primary fbs creation
> +	 * for full frame update (FFU) test case, we don't change primary FB content but to change
> +	 * the position of primary FB (panning) and update the overlay plane alpha region.
> +	 * Any overlay change is expected to be regarded as FFU from KMD's perspective.
> +	 *
> +	 * 1. create two overlay FBs with full screen size and one primary FB w/ 1/4 screen size
> +	 * 2. panning the primary plane to top-left and flip for couple of frames
> +	 * 3. wait for couple of seconds to allow visual confirm
> +	 * 4. panning the primary plane from top-left to middle of screen
> +	 * 5. repeat step 3
> +	 * 6. panning the primary plane from middle to bottom-right of screen
> +	 * 7. repeat step 3
> +	 */
> +
> +	/* step 1 */
> +	igt_create_color_fb(data->fd, ob_w, ob_h, DRM_FORMAT_ARGB8888, DRM_FORMAT_MOD_LINEAR,
> +			    1.0, 1.0, 1.0, &data->ov_fb[0]);
> +	draw_color_alpha(&data->ov_fb[0], 0, 0, ob_w, ob_h, 1.0, 1.0, 1.0, .3);

Looks like we're setting the entire overlay fb as translucent (0.3
alpha), interesting...

Though this should still trigger a full-frame-update when flipping,
visually this might be strange. We'll see the undefined background color
in areas not covered by the primary plane:

+----------------------------+
| +-------------+            |
| |             |            |
| |  Primary    |            |    0.7 of background color shown
| |  (alpha=1.0)|     <---------- outside of primary plane
| +-------------+            |
|    Overlay                 |
|    (alpha=0.3)             |
+----------------------------+

Ideally we only want 0.0 alpha over the primary plane region, with the
rest as solid (1.0) alpha:
+----------------------------+
| +-------------+            |
| |             |            |
| |  Primary  <------------------ Overlay alpha=0.0 over
| |  (alpha=1.0)|            |    primary plane region
| +-------------+            |
|    Overlay                 |
|    (alpha=1.0)             |
+----------------------------+

We can do this as part of panning_rect_fb(), something like so:

```
/* Update primary plane position and size */
igt_plane_set_position(data->primary, curr_x, curr_y);
igt_plane_set_size(data->primary, rect_w, rect_h);

/* Fill in entire overlay plane with different colors and 1.0 alpha */
draw_color_alpha(&data->ov_fb[0], 0, 0, ob_w, ob_h, 0.5, 0.5, 0.5, 1.0);
draw_color_alpha(&data->ov_fb[1], 0, 0, ob_w, ob_h, 1.0, 1.0, 1.0, 1.0);

/* Fill in new alpha region */
draw_color_alpha(&data->ov_fb[0], curr_x, curr_y, rect_w, rect_h,
                  0.5, 0.5, 0.5, 0.0);
draw_color_alpha(&data->ov_fb[1], curr_x, curr_y, rect_w, rect_h,
                  1.0, 1.0, 1.0, 0.0);

/* flip loop */
for (int i = 0; i < N_FLIPS; ++i) {
...
```

Thanks,
Leo

> +	igt_create_color_fb(data->fd, ob_w, ob_h, DRM_FORMAT_ARGB8888, DRM_FORMAT_MOD_LINEAR,
> +			    1.0, 1.0, 1.0, &data->ov_fb[1]);
> +	draw_color_alpha(&data->ov_fb[1], 0, 0, ob_w, ob_h, .0, 1.0, .0, .3);
> +
> +	igt_create_color_fb(data->fd, pb_w, pb_h, DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
> +			    1.0, .0, 1.0, &rect_fb);
> +
> +	/* step 2 & 3 */
> +	panning_rect_fb(data, &rect_fb, pb_w, pb_h, 0, 0);
> +	sleep(5);
> +
> +	/* step 4 & 5 */
> +	panning_rect_fb(data, &rect_fb, pb_w, pb_h, pb_w / 2, pb_h / 2);
> +	sleep(5);
> +
> +	/* step 6 & 7 */
> +	panning_rect_fb(data, &rect_fb, pb_w, pb_h, pb_w, pb_h);
> +	sleep(5);
> +
> +	/* fini */
> +	igt_remove_fb(data->fd, &ref_fb);
> +	igt_remove_fb(data->fd, &data->ov_fb[0]);
> +	igt_remove_fb(data->fd, &data->ov_fb[1]);
> +	igt_remove_fb(data->fd, &rect_fb);
> +	test_fini(data);
> +}
> +
>   const char *help_str =
>   "  --visual-confirm           PSR visual confirm debug option enable\n";
>   
> @@ -367,6 +474,10 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
>   		     "and to imitate Multiplane Overlay video playback scenario");
>   	igt_subtest("psr_su_mpo") run_check_psr_su_mpo(&data);
>   
> +	igt_describe("Test to validate PSR SU enablement with Visual Confirm "
> +		     "and to validate Full Frame Update scenario");
> +	igt_subtest("psr_su_ffu") run_check_psr_su_ffu(&data);
> +
>   	igt_fixture
>   	{
>   		if (opt.visual_confirm) {

Re: [igt-dev] [PATCH v3 3/3] tests/amdgpu/amd_psr: define reusable wrapper for PSR support check

[Leo Li]

On 2022-04-12 15:21, David Zhang wrote:
> [why & how]
> For amdgpu PSR-SU test cases, we'd first check if the sink device
> and amdgpu driver support the PSR-SU feature at the beginning. We
> define a local helper to wrap the check for PSR-SU support from
> sink and driver as well and called by both MPO and FFU test case,
> to avoid the code repetition.
> 
> Cc: Rodrigo Siqueira <rodrigo.siqueira@amd.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Leo Li <sunpeng.li@amd.com>
> Cc: Jay Pillai <aurabindo.pillai@amd.com>
> Cc: Wayne Lin <wayne.lin@amd.com>
> 
> Signed-off-by: David Zhang <dingchen.zhang@amd.com>
> ---
>   tests/amdgpu/amd_psr.c | 40 ++++++++++++++++++++++++----------------
>   1 file changed, 24 insertions(+), 16 deletions(-)
> 
> diff --git a/tests/amdgpu/amd_psr.c b/tests/amdgpu/amd_psr.c
> index 92628fee..e7614d71 100644
> --- a/tests/amdgpu/amd_psr.c
> +++ b/tests/amdgpu/amd_psr.c
> @@ -138,6 +138,28 @@ static int check_conn_type(data_t *data, uint32_t type) {
>   	return -1;
>   }
>   
> +static bool psr_su_supported(data_t *data)
> +{
> +	/* run PSR-SU test i.i.f. eDP panel and kernel driver both support PSR-SU */
> +	if (!igt_amd_output_has_psr_cap(data->fd, data->output->name))
> +		return false;
> +
> +	if (!igt_amd_output_has_psr_state(data->fd, data->output->name))
> +		return false;
> +

Would be good to igt_warn on the above checks too, so the reason for
skipping is clear.

> +	if (!igt_amd_psr_support_sink(data->fd, data->output->name, PSR_MODE_2)) {
> +		igt_info(" output %s not support PSR-SU\n", data->output->name);

Consider using igt_warn()

With those addressed,

Reviewed-by: Leo Li <sunpeng.li@amd.com>

Thanks,
Leo

> +		return false;
> +	}
> +
> +	if (!igt_amd_psr_support_drv(data->fd, data->output->name, PSR_MODE_2)) {
> +		igt_info(" kernel driver not support PSR-SU\n");
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
>   static void run_check_psr(data_t *data, bool test_null_crtc) {
>   	int fd, edp_idx, dp_idx, ret, i, psr_state;
>   	igt_fb_t ref_fb, ref_fb2;
> @@ -217,8 +239,6 @@ static void run_check_psr(data_t *data, bool test_null_crtc) {
>   static void run_check_psr_su_mpo(data_t *data)
>   {
>   	int edp_idx = check_conn_type(data, DRM_MODE_CONNECTOR_eDP);
> -	bool sink_support_psrsu = false;
> -	bool drv_suport_psrsu = false;
>   	igt_fb_t ov_fb;		// fb for overlay
>   	igt_fb_t rect_fb[N_MPO_TEST_RECT_FB]; 	// rectangle fbs for primary, emulate as video playback region
>   	igt_fb_t ref_fb;	// reference fb
> @@ -235,12 +255,7 @@ static void run_check_psr_su_mpo(data_t *data)
>   	frame_rate = data->mode->vrefresh;
>   
>   	/* run the test i.i.f. eDP panel supports and kernel driver both support PSR-SU  */
> -	igt_skip_on(!igt_amd_output_has_psr_cap(data->fd, data->output->name));
> -	igt_skip_on(!igt_amd_output_has_psr_state(data->fd, data->output->name));
> -	sink_support_psrsu = igt_amd_psr_support_sink(data->fd, data->output->name, PSR_MODE_2);
> -	igt_skip_on_f(!sink_support_psrsu, "output %s not support PSR-SU\n", data->output->name);
> -	drv_suport_psrsu = igt_amd_psr_support_drv(data->fd, data->output->name, PSR_MODE_2);
> -	igt_skip_on_f(!drv_suport_psrsu, "kernel driver not support PSR-SU\n");
> +	igt_skip_on(!psr_su_supported(data));
>   
>   	/* reference background pattern in grey */
>   	igt_create_color_fb(data->fd, data->w, data->h, DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,
> @@ -338,8 +353,6 @@ static void panning_rect_fb(data_t *data, igt_fb_t *rect_fb, int rect_w, int rec
>   static void run_check_psr_su_ffu(data_t *data)
>   {
>   	int edp_idx = check_conn_type(data, DRM_MODE_CONNECTOR_eDP);
> -	bool sink_support_psrsu = false;
> -	bool drv_suport_psrsu = false;
>   	igt_fb_t rect_fb; 	// rectangle fbs for primary
>   	igt_fb_t ref_fb;	// reference fb
>   	int pb_w, pb_h, ob_w, ob_h;
> @@ -355,12 +368,7 @@ static void run_check_psr_su_ffu(data_t *data)
>   	pb_h = data->h / 2;
>   
>   	/* run the test i.i.f. eDP panel supports and kernel driver both support PSR-SU  */
> -	igt_skip_on(!igt_amd_output_has_psr_cap(data->fd, data->output->name));
> -	igt_skip_on(!igt_amd_output_has_psr_state(data->fd, data->output->name));
> -	sink_support_psrsu = igt_amd_psr_support_sink(data->fd, data->output->name, PSR_MODE_2);
> -	igt_skip_on_f(!sink_support_psrsu, "output %s not support PSR-SU\n", data->output->name);
> -	drv_suport_psrsu = igt_amd_psr_support_drv(data->fd, data->output->name, PSR_MODE_2);
> -	igt_skip_on_f(!drv_suport_psrsu, "kernel driver not support PSR-SU\n");
> +	igt_skip_on(!psr_su_supported(data));
>   
>   	/* reference background pattern in grey */
>   	igt_create_color_fb(data->fd, data->w, data->h, DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR,