[igt-dev] [PATCH i-g-t 01/20] tests/psr: Print the reason for skipping when sink lacks PSR support

[igt-dev] [PATCH i-g-t 01/20] tests/psr: Print the reason for skipping when sink lacks PSR support

[Dhinakaran Pandiyan]

And rename the function to match what it does.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 tests/kms_psr_sink_crc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 98d29ba9..93807c03 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -193,7 +193,7 @@ static void fill_render(data_t *data, uint32_t handle, unsigned char color)
 	gem_bo_busy(data->drm_fd, handle);
 }
 
-static bool psr_possible(data_t *data)
+static bool sink_support(data_t *data)
 {
 	char buf[512];
 
@@ -513,8 +513,8 @@ int main(int argc, char *argv[])
 
 		igt_set_module_param_int("enable_psr", running_with_psr_disabled ?
 					 0 : 1);
-
-		igt_skip_on(!psr_possible(&data));
+		igt_require_f(sink_support(&data),
+			      "Sink does not support PSR\n");
 
 		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
 		igt_assert(data.bufmgr);
-- 
2.14.1

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

[igt-dev] [PATCH i-g-t 02/20] tests/psr: Remove "psr_" prefix from basic and drrs subtests

[Dhinakaran Pandiyan]

And rename psr_drrs to no_drrs.

Makes the name consistent with other tests.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 tests/intel-ci/fast-feedback.testlist | 2 +-
 tests/kms_psr_sink_crc.c              | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
index 5bad8c79..b08ef770 100644
--- a/tests/intel-ci/fast-feedback.testlist
+++ b/tests/intel-ci/fast-feedback.testlist
@@ -243,7 +243,7 @@ igt@kms_pipe_crc_basic@read-crc-pipe-c-frame-sequence
 igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a
 igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b
 igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c
-igt@kms_psr_sink_crc@psr_basic
+igt@kms_psr_sink_crc@basic
 igt@kms_setmode@basic-clone-single-crtc
 igt@kms_sink_crc_basic
 igt@pm_backlight@basic-brightness
diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 93807c03..e55d2576 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -523,13 +523,13 @@ int main(int argc, char *argv[])
 		display_init(&data);
 	}
 
-	igt_subtest("psr_basic") {
+	igt_subtest("basic") {
 		setup_test_plane(&data);
 		igt_assert(wait_psr_entry(&data));
 		test_cleanup(&data);
 	}
 
-	igt_subtest("psr_drrs") {
+	igt_subtest("no_drrs") {
 		setup_test_plane(&data);
 		igt_assert(drrs_disabled(&data));
 		test_cleanup(&data);
-- 
2.14.1

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

Re: [igt-dev] [PATCH i-g-t 02/20] tests/psr: Remove "psr_" prefix from basic and drrs subtests

[Katarzyna Dec]

On Tue, Apr 10, 2018 at 07:37:14PM -0700, Dhinakaran Pandiyan wrote:
> And rename psr_drrs to no_drrs.
> 
> Makes the name consistent with other tests.
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Katarzyna Dec <katarzyna.dec@intel.com>
> ---
>  tests/intel-ci/fast-feedback.testlist | 2 +-
>  tests/kms_psr_sink_crc.c              | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
> index 5bad8c79..b08ef770 100644
> --- a/tests/intel-ci/fast-feedback.testlist
> +++ b/tests/intel-ci/fast-feedback.testlist
> @@ -243,7 +243,7 @@ igt@kms_pipe_crc_basic@read-crc-pipe-c-frame-sequence
>  igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a
>  igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b
>  igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c
> -igt@kms_psr_sink_crc@psr_basic
> +igt@kms_psr_sink_crc@basic
>  igt@kms_setmode@basic-clone-single-crtc
>  igt@kms_sink_crc_basic
>  igt@pm_backlight@basic-brightness
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index 93807c03..e55d2576 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -523,13 +523,13 @@ int main(int argc, char *argv[])
>  		display_init(&data);
>  	}
>  
> -	igt_subtest("psr_basic") {
> +	igt_subtest("basic") {
>  		setup_test_plane(&data);
>  		igt_assert(wait_psr_entry(&data));
>  		test_cleanup(&data);
>  	}
>  
> -	igt_subtest("psr_drrs") {
> +	igt_subtest("no_drrs") {
>  		setup_test_plane(&data);
>  		igt_assert(drrs_disabled(&data));
>  		test_cleanup(&data);
> -- 
> 2.14.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 03/20] tests/psr: Rename psr_active() to psr_enabled()

[Dhinakaran Pandiyan]

psr_active() checks the debugfs flag "HW Enabled & Active bit", which only
tells us if PSR was enabled by the driver. The state of PSR - active
or inactive is different from this flag for DDI platforms, so rename the
function appropriately.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 tests/kms_psr_sink_crc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index e55d2576..1657ee8a 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -203,7 +203,7 @@ static bool sink_support(data_t *data)
 		strstr(buf, "Sink_Support: yes\n");
 }
 
-static bool psr_active(data_t *data)
+static bool psr_enabled(data_t *data)
 {
 	char buf[512];
 
@@ -217,7 +217,7 @@ static bool wait_psr_entry(data_t *data)
 {
 	int timeout = 5;
 	while (timeout--) {
-		if (psr_active(data))
+		if (psr_enabled(data))
 			return true;
 		sleep(1);
 	}
-- 
2.14.1

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

Re: [igt-dev] [PATCH i-g-t 03/20] tests/psr: Rename psr_active() to psr_enabled()

[Katarzyna Dec]

On Tue, Apr 10, 2018 at 07:37:15PM -0700, Dhinakaran Pandiyan wrote:
> psr_active() checks the debugfs flag "HW Enabled & Active bit", which only
> tells us if PSR was enabled by the driver. The state of PSR - active
> or inactive is different from this flag for DDI platforms, so rename the
> function appropriately.
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Katarzyna Dec <katarzyna.dec@intel.com> 
> ---
>  tests/kms_psr_sink_crc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index e55d2576..1657ee8a 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -203,7 +203,7 @@ static bool sink_support(data_t *data)
>  		strstr(buf, "Sink_Support: yes\n");
>  }
>  
> -static bool psr_active(data_t *data)
> +static bool psr_enabled(data_t *data)
>  {
>  	char buf[512];
>  
> @@ -217,7 +217,7 @@ static bool wait_psr_entry(data_t *data)
>  {
>  	int timeout = 5;
>  	while (timeout--) {
> -		if (psr_active(data))
> +		if (psr_enabled(data))
>  			return true;
>  		sleep(1);
>  	}
> -- 
> 2.14.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 04/20] tests/psr: Store the debugfs file descriptor

[Dhinakaran Pandiyan]

It will be reused to enable PSR debug in the later patches.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 tests/kms_psr_sink_crc.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 1657ee8a..35c7b845 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -65,6 +65,7 @@ static const char *op_str(enum operations op)
 
 typedef struct {
 	int drm_fd;
+	int debugfs_fd;
 	int test_plane;
 	enum operations op;
 	uint32_t devid;
@@ -224,16 +225,14 @@ static bool wait_psr_entry(data_t *data)
 	return false;
 }
 
-static void get_sink_crc(data_t *data, char *crc) {
-	int dir;
-
+static void get_sink_crc(data_t *data, char *crc)
+{
 	if (igt_interactive_debug)
 		return;
 
-	dir = igt_debugfs_dir(data->drm_fd);
-	igt_require_f(igt_sysfs_scanf(dir, "i915_sink_crc_eDP1", "%s\n", crc),
+	igt_require_f(igt_sysfs_scanf(data->debugfs_fd, "i915_sink_crc_eDP1",
+				      "%s\n", crc),
 		      "Sink CRC is unreliable on this machine. Try manual debug with --interactive-debug=no-crc\n");
-	close(dir);
 
 	igt_debug("%s\n", crc);
 
@@ -508,6 +507,7 @@ int main(int argc, char *argv[])
 
 	igt_fixture {
 		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
+		data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
 		kmstest_set_vt_graphics_mode();
 		data.devid = intel_get_drm_devid(data.drm_fd);
 
@@ -619,6 +619,7 @@ int main(int argc, char *argv[])
 	}
 
 	igt_fixture {
+		close(data.debugfs_fd);
 		drm_intel_bufmgr_destroy(data.bufmgr);
 		display_fini(&data);
 	}
-- 
2.14.1

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

Re: [igt-dev] [PATCH i-g-t 04/20] tests/psr: Store the debugfs file descriptor

[Katarzyna Dec]

On Tue, Apr 10, 2018 at 07:37:16PM -0700, Dhinakaran Pandiyan wrote:
> It will be reused to enable PSR debug in the later patches.
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Katarzyna Dec <katarzyna.dec@intel.com> 
> ---
>  tests/kms_psr_sink_crc.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index 1657ee8a..35c7b845 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -65,6 +65,7 @@ static const char *op_str(enum operations op)
>  
>  typedef struct {
>  	int drm_fd;
> +	int debugfs_fd;
>  	int test_plane;
>  	enum operations op;
>  	uint32_t devid;
> @@ -224,16 +225,14 @@ static bool wait_psr_entry(data_t *data)
>  	return false;
>  }
>  
> -static void get_sink_crc(data_t *data, char *crc) {
> -	int dir;
> -
> +static void get_sink_crc(data_t *data, char *crc)
> +{
>  	if (igt_interactive_debug)
>  		return;
>  
> -	dir = igt_debugfs_dir(data->drm_fd);
> -	igt_require_f(igt_sysfs_scanf(dir, "i915_sink_crc_eDP1", "%s\n", crc),
> +	igt_require_f(igt_sysfs_scanf(data->debugfs_fd, "i915_sink_crc_eDP1",
> +				      "%s\n", crc),
>  		      "Sink CRC is unreliable on this machine. Try manual debug with --interactive-debug=no-crc\n");
> -	close(dir);
>  
>  	igt_debug("%s\n", crc);
>  
> @@ -508,6 +507,7 @@ int main(int argc, char *argv[])
>  
>  	igt_fixture {
>  		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
> +		data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
>  		kmstest_set_vt_graphics_mode();
>  		data.devid = intel_get_drm_devid(data.drm_fd);
>  
> @@ -619,6 +619,7 @@ int main(int argc, char *argv[])
>  	}
>  
>  	igt_fixture {
> +		close(data.debugfs_fd);
>  		drm_intel_bufmgr_destroy(data.bufmgr);
>  		display_fini(&data);
>  	}
> -- 
> 2.14.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 05/20] tests/psr: Assert sink CRC length and make use of igt_sysfs_read()

[Dhinakaran Pandiyan]

Include minor fomatting change too.
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 tests/kms_psr_sink_crc.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 35c7b845..af63e689 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -230,14 +230,13 @@ static void get_sink_crc(data_t *data, char *crc)
 	if (igt_interactive_debug)
 		return;
 
-	igt_require_f(igt_sysfs_scanf(data->debugfs_fd, "i915_sink_crc_eDP1",
-				      "%s\n", crc),
+	igt_require_f(igt_sysfs_read(data->debugfs_fd, "i915_sink_crc_eDP1",
+				     crc, CRC_LEN) == CRC_LEN,
 		      "Sink CRC is unreliable on this machine. Try manual debug with --interactive-debug=no-crc\n");
-
-	igt_debug("%s\n", crc);
+	igt_debug("sink CRC: %.*s\n", CRC_LEN, crc);
 
 	/* Black screen is always invalid */
-	igt_assert(strncmp(crc, CRC_BLACK, CRC_LEN) != 0);
+	igt_assert(strncmp(crc, CRC_BLACK, CRC_LEN));
 }
 
 static bool is_green(char *crc)
-- 
2.14.1

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

Re: [igt-dev] [PATCH i-g-t 05/20] tests/psr: Assert sink CRC length and make use of igt_sysfs_read()

[Katarzyna Dec]

On Tue, Apr 10, 2018 at 07:37:17PM -0700, Dhinakaran Pandiyan wrote:
> Include minor fomatting change too.
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Katarzyna Dec <katarzyna.dec@intel.com>
> ---
>  tests/kms_psr_sink_crc.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index 35c7b845..af63e689 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -230,14 +230,13 @@ static void get_sink_crc(data_t *data, char *crc)
>  	if (igt_interactive_debug)
>  		return;
>  
> -	igt_require_f(igt_sysfs_scanf(data->debugfs_fd, "i915_sink_crc_eDP1",
> -				      "%s\n", crc),
> +	igt_require_f(igt_sysfs_read(data->debugfs_fd, "i915_sink_crc_eDP1",
> +				     crc, CRC_LEN) == CRC_LEN,
>  		      "Sink CRC is unreliable on this machine. Try manual debug with --interactive-debug=no-crc\n");
> -
> -	igt_debug("%s\n", crc);
> +	igt_debug("sink CRC: %.*s\n", CRC_LEN, crc);
>  
>  	/* Black screen is always invalid */
> -	igt_assert(strncmp(crc, CRC_BLACK, CRC_LEN) != 0);
> +	igt_assert(strncmp(crc, CRC_BLACK, CRC_LEN));
>  }
>  
>  static bool is_green(char *crc)
> -- 
> 2.14.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 06/20] tests/psr: Optimize check for green frame.

[Dhinakaran Pandiyan]

Eliminate three memcpy's and four sscanf's.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 tests/kms_psr_sink_crc.c | 30 +++++-------------------------
 1 file changed, 5 insertions(+), 25 deletions(-)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index af63e689..94134b7d 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -241,34 +241,14 @@ static void get_sink_crc(data_t *data, char *crc)
 
 static bool is_green(char *crc)
 {
-	char color_mask[5] = "FFFF\0";
-	char rs[5], gs[5], bs[5];
-	unsigned int rh, gh, bh, mask;
-	int ret;
-
+	const char *mask = "0000FFFF0000";
+	uint32_t *p = (uint32_t *)crc, *mask_p = (uint32_t *)mask;
 	if (igt_interactive_debug)
 		return false;
 
-	sscanf(color_mask, "%4x", &mask);
-
-	memcpy(rs, &crc[0], 4);
-	rs[4] = '\0';
-	ret = sscanf(rs, "%4x", &rh);
-	igt_require(ret > 0);
-
-	memcpy(gs, &crc[4], 4);
-	gs[4] = '\0';
-	ret = sscanf(gs, "%4x", &gh);
-	igt_require(ret > 0);
-
-	memcpy(bs, &crc[8], 4);
-	bs[4] = '\0';
-	ret = sscanf(bs, "%4x", &bh);
-	igt_require(ret > 0);
-
-	return ((rh & mask) == 0 &&
-		(gh & mask) != 0 &&
-		(bh & mask) == 0);
+	/* Check R and B components are 0 and G is non-zero */
+	return *p == *mask_p && *(p + 2) == *(mask_p + 2) &&
+	       (*(p + 1) & *(mask_p + 1)) != 0;
 }
 
 static void assert_or_manual(bool condition, const char *expected)
-- 
2.14.1

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

Re: [igt-dev] [PATCH i-g-t 06/20] tests/psr: Optimize check for green frame.

[Katarzyna Dec]

On Tue, Apr 10, 2018 at 07:37:18PM -0700, Dhinakaran Pandiyan wrote:
> Eliminate three memcpy's and four sscanf's.
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Katarzyna Dec <katarzyna.dec@intel.com> 
> ---
>  tests/kms_psr_sink_crc.c | 30 +++++-------------------------
>  1 file changed, 5 insertions(+), 25 deletions(-)
> 
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index af63e689..94134b7d 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -241,34 +241,14 @@ static void get_sink_crc(data_t *data, char *crc)
>  
>  static bool is_green(char *crc)
>  {
> -	char color_mask[5] = "FFFF\0";
> -	char rs[5], gs[5], bs[5];
> -	unsigned int rh, gh, bh, mask;
> -	int ret;
> -
> +	const char *mask = "0000FFFF0000";
> +	uint32_t *p = (uint32_t *)crc, *mask_p = (uint32_t *)mask;
>  	if (igt_interactive_debug)
>  		return false;
>  
> -	sscanf(color_mask, "%4x", &mask);
> -
> -	memcpy(rs, &crc[0], 4);
> -	rs[4] = '\0';
> -	ret = sscanf(rs, "%4x", &rh);
> -	igt_require(ret > 0);
> -
> -	memcpy(gs, &crc[4], 4);
> -	gs[4] = '\0';
> -	ret = sscanf(gs, "%4x", &gh);
> -	igt_require(ret > 0);
> -
> -	memcpy(bs, &crc[8], 4);
> -	bs[4] = '\0';
> -	ret = sscanf(bs, "%4x", &bh);
> -	igt_require(ret > 0);
> -
> -	return ((rh & mask) == 0 &&
> -		(gh & mask) != 0 &&
> -		(bh & mask) == 0);
> +	/* Check R and B components are 0 and G is non-zero */
> +	return *p == *mask_p && *(p + 2) == *(mask_p + 2) &&
> +	       (*(p + 1) & *(mask_p + 1)) != 0;
>  }
>  
>  static void assert_or_manual(bool condition, const char *expected)
> -- 
> 2.14.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 07/20] tests/psr: Kill MMAP_GTT_WAITING

[Dhinakaran Pandiyan]

No PSR event should take 10s, don't see value in this test.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 tests/kms_psr_sink_crc.c | 25 -------------------------
 1 file changed, 25 deletions(-)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 94134b7d..d9cce5ea 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -39,7 +39,6 @@ bool running_with_psr_disabled;
 enum operations {
 	PAGE_FLIP,
 	MMAP_GTT,
-	MMAP_GTT_WAITING,
 	MMAP_CPU,
 	BLT,
 	RENDER,
@@ -52,7 +51,6 @@ static const char *op_str(enum operations op)
 	static const char * const name[] = {
 		[PAGE_FLIP] = "page_flip",
 		[MMAP_GTT] = "mmap_gtt",
-		[MMAP_GTT_WAITING] = "mmap_gtt_waiting",
 		[MMAP_CPU] = "mmap_cpu",
 		[BLT] = "blt",
 		[RENDER] = "render",
@@ -319,29 +317,6 @@ static void run_test(data_t *data)
 		munmap(ptr, data->mod_size);
 		expected = "BLACK or TRANSPARENT mark on top of plane in test";
 		break;
-	case MMAP_GTT_WAITING:
-		ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
-				    PROT_WRITE);
-		gem_set_domain(data->drm_fd, handle,
-			       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
-
-		/* Printing white on white so the screen shouldn't change */
-		memset(ptr, 0xff, data->mod_size);
-		get_sink_crc(data, crc);
-		if (data->test_plane == DRM_PLANE_TYPE_PRIMARY)
-			assert_or_manual(strncmp(ref_crc, crc, CRC_LEN) == 0, "screen WHITE");
-		else
-			assert_or_manual(strncmp(ref_crc, crc, CRC_LEN) == 0,
-			       "GREEN background with WHITE box");
-
-		igt_info("Waiting 10s...\n");
-		sleep(10);
-
-		/* Now lets print black to change the screen */
-		memset(ptr, 0, data->mod_size);
-		munmap(ptr, data->mod_size);
-		expected = "BLACK or TRANSPARENT mark on top of plane in test";
-		break;
 	case MMAP_CPU:
 		ptr = gem_mmap__cpu(data->drm_fd, handle, 0, data->mod_size,
 				    PROT_WRITE);
-- 
2.14.1

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

Re: [igt-dev] [PATCH i-g-t 07/20] tests/psr: Kill MMAP_GTT_WAITING

[Katarzyna Dec]

On Tue, Apr 10, 2018 at 07:37:19PM -0700, Dhinakaran Pandiyan wrote:
> No PSR event should take 10s, don't see value in this test.
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  tests/kms_psr_sink_crc.c | 25 -------------------------
>  1 file changed, 25 deletions(-)
> 
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index 94134b7d..d9cce5ea 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -39,7 +39,6 @@ bool running_with_psr_disabled;
>  enum operations {
>  	PAGE_FLIP,
>  	MMAP_GTT,
> -	MMAP_GTT_WAITING,
>  	MMAP_CPU,
>  	BLT,
>  	RENDER,
> @@ -52,7 +51,6 @@ static const char *op_str(enum operations op)
>  	static const char * const name[] = {
>  		[PAGE_FLIP] = "page_flip",
>  		[MMAP_GTT] = "mmap_gtt",
> -		[MMAP_GTT_WAITING] = "mmap_gtt_waiting",
>  		[MMAP_CPU] = "mmap_cpu",
>  		[BLT] = "blt",
>  		[RENDER] = "render",
> @@ -319,29 +317,6 @@ static void run_test(data_t *data)
>  		munmap(ptr, data->mod_size);
>  		expected = "BLACK or TRANSPARENT mark on top of plane in test";
>  		break;
> -	case MMAP_GTT_WAITING:
> -		ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
> -				    PROT_WRITE);
> -		gem_set_domain(data->drm_fd, handle,
> -			       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
> -
> -		/* Printing white on white so the screen shouldn't change */
> -		memset(ptr, 0xff, data->mod_size);
> -		get_sink_crc(data, crc);
> -		if (data->test_plane == DRM_PLANE_TYPE_PRIMARY)
> -			assert_or_manual(strncmp(ref_crc, crc, CRC_LEN) == 0, "screen WHITE");
> -		else
> -			assert_or_manual(strncmp(ref_crc, crc, CRC_LEN) == 0,
> -			       "GREEN background with WHITE box");
> -
> -		igt_info("Waiting 10s...\n");
> -		sleep(10);
Is this time too long? Couldn't we decrease that? Or maybe that is not a point.
Why this subtest is not relevant any more?
Kasia
> -
> -		/* Now lets print black to change the screen */
> -		memset(ptr, 0, data->mod_size);
> -		munmap(ptr, data->mod_size);
> -		expected = "BLACK or TRANSPARENT mark on top of plane in test";
> -		break;
>  	case MMAP_CPU:
>  		ptr = gem_mmap__cpu(data->drm_fd, handle, 0, data->mod_size,
>  				    PROT_WRITE);
> -- 
> 2.14.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 07/20] tests/psr: Kill MMAP_GTT_WAITING

[Pandiyan, Dhinakaran]

On Thu, 2018-04-12 at 13:11 +0200, Katarzyna Dec wrote:
> On Tue, Apr 10, 2018 at 07:37:19PM -0700, Dhinakaran Pandiyan wrote:
> > No PSR event should take 10s, don't see value in this test.
> > 
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  tests/kms_psr_sink_crc.c | 25 -------------------------
> >  1 file changed, 25 deletions(-)
> > 
> > diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> > index 94134b7d..d9cce5ea 100644
> > --- a/tests/kms_psr_sink_crc.c
> > +++ b/tests/kms_psr_sink_crc.c
> > @@ -39,7 +39,6 @@ bool running_with_psr_disabled;
> >  enum operations {
> >  	PAGE_FLIP,
> >  	MMAP_GTT,
> > -	MMAP_GTT_WAITING,
> >  	MMAP_CPU,
> >  	BLT,
> >  	RENDER,
> > @@ -52,7 +51,6 @@ static const char *op_str(enum operations op)
> >  	static const char * const name[] = {
> >  		[PAGE_FLIP] = "page_flip",
> >  		[MMAP_GTT] = "mmap_gtt",
> > -		[MMAP_GTT_WAITING] = "mmap_gtt_waiting",
> >  		[MMAP_CPU] = "mmap_cpu",
> >  		[BLT] = "blt",
> >  		[RENDER] = "render",
> > @@ -319,29 +317,6 @@ static void run_test(data_t *data)
> >  		munmap(ptr, data->mod_size);
> >  		expected = "BLACK or TRANSPARENT mark on top of plane in test";
> >  		break;
> > -	case MMAP_GTT_WAITING:
> > -		ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
> > -				    PROT_WRITE);
> > -		gem_set_domain(data->drm_fd, handle,
> > -			       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
> > -
> > -		/* Printing white on white so the screen shouldn't change */
> > -		memset(ptr, 0xff, data->mod_size);
> > -		get_sink_crc(data, crc);
> > -		if (data->test_plane == DRM_PLANE_TYPE_PRIMARY)
> > -			assert_or_manual(strncmp(ref_crc, crc, CRC_LEN) == 0, "screen WHITE");
> > -		else
> > -			assert_or_manual(strncmp(ref_crc, crc, CRC_LEN) == 0,
> > -			       "GREEN background with WHITE box");
> > -
> > -		igt_info("Waiting 10s...\n");
> > -		sleep(10);
> Is this time too long? Couldn't we decrease that? Or maybe that is not a point.
> Why this subtest is not relevant any more?


I believe the test was relevant in the past when the feature was being
developed, perhaps there was a driver bug back then.

Like I wrote in the commit message, no PSR event takes 10s to happen, so
there is no point in having this test.

Cc: Rodrigo, who wrote these tests.



> Kasia
> > -
> > -		/* Now lets print black to change the screen */
> > -		memset(ptr, 0, data->mod_size);
> > -		munmap(ptr, data->mod_size);
> > -		expected = "BLACK or TRANSPARENT mark on top of plane in test";
> > -		break;
> >  	case MMAP_CPU:
> >  		ptr = gem_mmap__cpu(data->drm_fd, handle, 0, data->mod_size,
> >  				    PROT_WRITE);
> > -- 
> > 2.14.1
> > 
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 07/20] tests/psr: Kill MMAP_GTT_WAITING

[Rodrigo Vivi]

On Mon, Apr 16, 2018 at 02:44:02PM -0700, Pandiyan, Dhinakaran wrote:
> 
> 
> 
> On Thu, 2018-04-12 at 13:11 +0200, Katarzyna Dec wrote:
> > On Tue, Apr 10, 2018 at 07:37:19PM -0700, Dhinakaran Pandiyan wrote:
> > > No PSR event should take 10s, don't see value in this test.
> > > 
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > ---
> > >  tests/kms_psr_sink_crc.c | 25 -------------------------
> > >  1 file changed, 25 deletions(-)
> > > 
> > > diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> > > index 94134b7d..d9cce5ea 100644
> > > --- a/tests/kms_psr_sink_crc.c
> > > +++ b/tests/kms_psr_sink_crc.c
> > > @@ -39,7 +39,6 @@ bool running_with_psr_disabled;
> > >  enum operations {
> > >  	PAGE_FLIP,
> > >  	MMAP_GTT,
> > > -	MMAP_GTT_WAITING,
> > >  	MMAP_CPU,
> > >  	BLT,
> > >  	RENDER,
> > > @@ -52,7 +51,6 @@ static const char *op_str(enum operations op)
> > >  	static const char * const name[] = {
> > >  		[PAGE_FLIP] = "page_flip",
> > >  		[MMAP_GTT] = "mmap_gtt",
> > > -		[MMAP_GTT_WAITING] = "mmap_gtt_waiting",
> > >  		[MMAP_CPU] = "mmap_cpu",
> > >  		[BLT] = "blt",
> > >  		[RENDER] = "render",
> > > @@ -319,29 +317,6 @@ static void run_test(data_t *data)
> > >  		munmap(ptr, data->mod_size);
> > >  		expected = "BLACK or TRANSPARENT mark on top of plane in test";
> > >  		break;
> > > -	case MMAP_GTT_WAITING:
> > > -		ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
> > > -				    PROT_WRITE);
> > > -		gem_set_domain(data->drm_fd, handle,
> > > -			       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
> > > -
> > > -		/* Printing white on white so the screen shouldn't change */
> > > -		memset(ptr, 0xff, data->mod_size);
> > > -		get_sink_crc(data, crc);
> > > -		if (data->test_plane == DRM_PLANE_TYPE_PRIMARY)
> > > -			assert_or_manual(strncmp(ref_crc, crc, CRC_LEN) == 0, "screen WHITE");
> > > -		else
> > > -			assert_or_manual(strncmp(ref_crc, crc, CRC_LEN) == 0,
> > > -			       "GREEN background with WHITE box");
> > > -
> > > -		igt_info("Waiting 10s...\n");
> > > -		sleep(10);
> > Is this time too long? Couldn't we decrease that? Or maybe that is not a point.
> > Why this subtest is not relevant any more?
> 
> 
> I believe the test was relevant in the past when the feature was being
> developed, perhaps there was a driver bug back then.
> 
> Like I wrote in the commit message, no PSR event takes 10s to happen, so
> there is no point in having this test.
> 
> Cc: Rodrigo, who wrote these tests.

This test got written when we didn't have the frontbuffer tracking
and the psr exit was spread all over the code and there was this case that
Chris raised where after a while if another gtt write was happening my old
hammers wouldn't detect.

With the framebuffer tracking that got solved and this never caught any
error. So I'm in favor of removing it:


Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> 
> 
> > Kasia
> > > -
> > > -		/* Now lets print black to change the screen */
> > > -		memset(ptr, 0, data->mod_size);
> > > -		munmap(ptr, data->mod_size);
> > > -		expected = "BLACK or TRANSPARENT mark on top of plane in test";
> > > -		break;
> > >  	case MMAP_CPU:
> > >  		ptr = gem_mmap__cpu(data->drm_fd, handle, 0, data->mod_size,
> > >  				    PROT_WRITE);
> > > -- 
> > > 2.14.1
> > > 
> > > _______________________________________________
> > > igt-dev mailing list
> > > igt-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 07/20] tests/psr: Kill MMAP_GTT_WAITING

[Dhinakaran Pandiyan]

On Thu, 2018-04-12 at 13:11 +0200, Katarzyna Dec wrote:
> On Tue, Apr 10, 2018 at 07:37:19PM -0700, Dhinakaran Pandiyan wrote:
> > No PSR event should take 10s, don't see value in this test.
> > 
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  tests/kms_psr_sink_crc.c | 25 -------------------------
> >  1 file changed, 25 deletions(-)
> > 
> > diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> > index 94134b7d..d9cce5ea 100644
> > --- a/tests/kms_psr_sink_crc.c
> > +++ b/tests/kms_psr_sink_crc.c
> > @@ -39,7 +39,6 @@ bool running_with_psr_disabled;
> >  enum operations {
> >  	PAGE_FLIP,
> >  	MMAP_GTT,
> > -	MMAP_GTT_WAITING,
> >  	MMAP_CPU,
> >  	BLT,
> >  	RENDER,
> > @@ -52,7 +51,6 @@ static const char *op_str(enum operations op)
> >  	static const char * const name[] = {
> >  		[PAGE_FLIP] = "page_flip",
> >  		[MMAP_GTT] = "mmap_gtt",
> > -		[MMAP_GTT_WAITING] = "mmap_gtt_waiting",
> >  		[MMAP_CPU] = "mmap_cpu",
> >  		[BLT] = "blt",
> >  		[RENDER] = "render",
> > @@ -319,29 +317,6 @@ static void run_test(data_t *data)
> >  		munmap(ptr, data->mod_size);
> >  		expected = "BLACK or TRANSPARENT mark on top of plane in test";
> >  		break;
> > -	case MMAP_GTT_WAITING:
> > -		ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
> > -				    PROT_WRITE);
> > -		gem_set_domain(data->drm_fd, handle,
> > -			       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
> > -
> > -		/* Printing white on white so the screen shouldn't change */
> > -		memset(ptr, 0xff, data->mod_size);
> > -		get_sink_crc(data, crc);
> > -		if (data->test_plane == DRM_PLANE_TYPE_PRIMARY)
> > -			assert_or_manual(strncmp(ref_crc, crc, CRC_LEN) == 0, "screen WHITE");
> > -		else
> > -			assert_or_manual(strncmp(ref_crc, crc, CRC_LEN) == 0,
> > -			       "GREEN background with WHITE box");
> > -
> > -		igt_info("Waiting 10s...\n");
> > -		sleep(10);
> Is this time too long? Couldn't we decrease that? Or maybe that is not a point.
> Why this subtest is not relevant any more?


I believe the test was relevant in the past when the feature was being
developed, perhaps there was a driver bug back then.

Like I wrote in the commit message, no PSR event takes 10s to happen, so
there is no point in having this test.

Cc: Rodrigo, who wrote these tests.



> Kasia
> > -
> > -		/* Now lets print black to change the screen */
> > -		memset(ptr, 0, data->mod_size);
> > -		munmap(ptr, data->mod_size);
> > -		expected = "BLACK or TRANSPARENT mark on top of plane in test";
> > -		break;
> >  	case MMAP_CPU:
> >  		ptr = gem_mmap__cpu(data->drm_fd, handle, 0, data->mod_size,
> >  				    PROT_WRITE);
> > -- 
> > 2.14.1
> > 
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

[igt-dev] [PATCH i-g-t 08/20] tests/psr: Merge PSR dpms and suspend variants.

[Dhinakaran Pandiyan]

I don't see a big difference in what {dpms,suspend}_psr_exit and
{dpms_off, suspend}_psr_active tests uniquely achieve. Combine them so
that we have one dpms and one suspend test.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 tests/kms_psr_sink_crc.c | 34 +++-------------------------------
 1 file changed, 3 insertions(+), 31 deletions(-)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index d9cce5ea..e04e0f6a 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -522,51 +522,23 @@ int main(int argc, char *argv[])
 		}
 	}
 
-	igt_subtest_f("dpms_off_psr_active") {
+	igt_subtest_f("dpms") {
 		data.test_plane = DRM_PLANE_TYPE_PRIMARY;
 		data.op = RENDER;
 		setup_test_plane(&data);
 		igt_assert(wait_psr_entry(&data));
-
 		dpms_off_on(data);
-
-		run_test(&data);
-		test_cleanup(&data);
-	}
-
-	igt_subtest_f("dpms_off_psr_exit") {
-		data.test_plane = DRM_PLANE_TYPE_OVERLAY;
-		data.op = PLANE_ONOFF;
-		setup_test_plane(&data);
-
-		dpms_off_on(data);
-
-		igt_assert(wait_psr_entry(&data));
-		run_test(&data);
-		test_cleanup(&data);
-	}
-
-	igt_subtest_f("suspend_psr_active") {
-		data.test_plane = DRM_PLANE_TYPE_PRIMARY;
-		data.op = PAGE_FLIP;
-		setup_test_plane(&data);
-		igt_assert(wait_psr_entry(&data));
-
-		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
-					      SUSPEND_TEST_NONE);
-
 		run_test(&data);
 		test_cleanup(&data);
 	}
 
-	igt_subtest_f("suspend_psr_exit") {
+	igt_subtest_f("suspend") {
 		data.test_plane = DRM_PLANE_TYPE_CURSOR;
 		data.op = PLANE_ONOFF;
 		setup_test_plane(&data);
-
+		igt_assert(wait_psr_entry(&data));
 		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
 					      SUSPEND_TEST_NONE);
-
 		igt_assert(wait_psr_entry(&data));
 		run_test(&data);
 		test_cleanup(&data);
-- 
2.14.1

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

Re: [igt-dev] [PATCH i-g-t 08/20] tests/psr: Merge PSR dpms and suspend variants.

[Katarzyna Dec]

On Tue, Apr 10, 2018 at 07:37:20PM -0700, Dhinakaran Pandiyan wrote:
> I don't see a big difference in what {dpms,suspend}_psr_exit and
> {dpms_off, suspend}_psr_active tests uniquely achieve. Combine them so
> that we have one dpms and one suspend test.
Could you please add more details why these tests can be merged?
Is it based on documentation or your experience?
Kasia
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  tests/kms_psr_sink_crc.c | 34 +++-------------------------------
>  1 file changed, 3 insertions(+), 31 deletions(-)
> 
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index d9cce5ea..e04e0f6a 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -522,51 +522,23 @@ int main(int argc, char *argv[])
>  		}
>  	}
>  
> -	igt_subtest_f("dpms_off_psr_active") {
> +	igt_subtest_f("dpms") {
>  		data.test_plane = DRM_PLANE_TYPE_PRIMARY;
>  		data.op = RENDER;
>  		setup_test_plane(&data);
>  		igt_assert(wait_psr_entry(&data));
> -
>  		dpms_off_on(data);
> -
> -		run_test(&data);
> -		test_cleanup(&data);
> -	}
> -
> -	igt_subtest_f("dpms_off_psr_exit") {
> -		data.test_plane = DRM_PLANE_TYPE_OVERLAY;
> -		data.op = PLANE_ONOFF;
> -		setup_test_plane(&data);
> -
> -		dpms_off_on(data);
> -
> -		igt_assert(wait_psr_entry(&data));
> -		run_test(&data);
> -		test_cleanup(&data);
> -	}
> -
> -	igt_subtest_f("suspend_psr_active") {
> -		data.test_plane = DRM_PLANE_TYPE_PRIMARY;
> -		data.op = PAGE_FLIP;
> -		setup_test_plane(&data);
> -		igt_assert(wait_psr_entry(&data));
> -
> -		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> -					      SUSPEND_TEST_NONE);
> -
>  		run_test(&data);
>  		test_cleanup(&data);
>  	}
>  
> -	igt_subtest_f("suspend_psr_exit") {
> +	igt_subtest_f("suspend") {
>  		data.test_plane = DRM_PLANE_TYPE_CURSOR;
>  		data.op = PLANE_ONOFF;
>  		setup_test_plane(&data);
> -
> +		igt_assert(wait_psr_entry(&data));
>  		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
>  					      SUSPEND_TEST_NONE);
> -
>  		igt_assert(wait_psr_entry(&data));
>  		run_test(&data);
>  		test_cleanup(&data);
> -- 
> 2.14.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 08/20] tests/psr: Merge PSR dpms and suspend variants.

[Pandiyan, Dhinakaran]

On Thu, 2018-04-12 at 13:20 +0200, Katarzyna Dec wrote:
> On Tue, Apr 10, 2018 at 07:37:20PM -0700, Dhinakaran Pandiyan wrote:
> > I don't see a big difference in what {dpms,suspend}_psr_exit and
> > {dpms_off, suspend}_psr_active tests uniquely achieve. Combine them so
> > that we have one dpms and one suspend test.
> Could you please add more details why these tests can be merged?


The reason is - the tests are pretty much the same. Combining them saves
run time.


> Is it based on documentation or your experience?
> Kasia
> > 
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  tests/kms_psr_sink_crc.c | 34 +++-------------------------------
> >  1 file changed, 3 insertions(+), 31 deletions(-)
> > 
> > diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> > index d9cce5ea..e04e0f6a 100644
> > --- a/tests/kms_psr_sink_crc.c
> > +++ b/tests/kms_psr_sink_crc.c
> > @@ -522,51 +522,23 @@ int main(int argc, char *argv[])
> >  		}
> >  	}
> >  
> > -	igt_subtest_f("dpms_off_psr_active") {
> > +	igt_subtest_f("dpms") {
> >  		data.test_plane = DRM_PLANE_TYPE_PRIMARY;
> >  		data.op = RENDER;
> >  		setup_test_plane(&data);
> >  		igt_assert(wait_psr_entry(&data));
> > -
> >  		dpms_off_on(data);
> > -
> > -		run_test(&data);
> > -		test_cleanup(&data);
> > -	}
> > -
> > -	igt_subtest_f("dpms_off_psr_exit") {
> > -		data.test_plane = DRM_PLANE_TYPE_OVERLAY;
> > -		data.op = PLANE_ONOFF;
> > -		setup_test_plane(&data);
> > -
> > -		dpms_off_on(data);
> > -
> > -		igt_assert(wait_psr_entry(&data));
> > -		run_test(&data);
> > -		test_cleanup(&data);
> > -	}
> > -
> > -	igt_subtest_f("suspend_psr_active") {
> > -		data.test_plane = DRM_PLANE_TYPE_PRIMARY;
> > -		data.op = PAGE_FLIP;
> > -		setup_test_plane(&data);
> > -		igt_assert(wait_psr_entry(&data));
> > -
> > -		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> > -					      SUSPEND_TEST_NONE);
> > -
> >  		run_test(&data);
> >  		test_cleanup(&data);
> >  	}
> >  
> > -	igt_subtest_f("suspend_psr_exit") {
> > +	igt_subtest_f("suspend") {
> >  		data.test_plane = DRM_PLANE_TYPE_CURSOR;
> >  		data.op = PLANE_ONOFF;
> >  		setup_test_plane(&data);
> > -
> > +		igt_assert(wait_psr_entry(&data));
> >  		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> >  					      SUSPEND_TEST_NONE);
> > -
> >  		igt_assert(wait_psr_entry(&data));
> >  		run_test(&data);
> >  		test_cleanup(&data);
> > -- 
> > 2.14.1
> > 
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 08/20] tests/psr: Merge PSR dpms and suspend variants.

[Katarzyna Dec]

On Mon, Apr 16, 2018 at 10:46:14PM +0100, Pandiyan, Dhinakaran wrote:
> On Thu, 2018-04-12 at 13:20 +0200, Katarzyna Dec wrote:
> > On Tue, Apr 10, 2018 at 07:37:20PM -0700, Dhinakaran Pandiyan wrote:
> > > I don't see a big difference in what {dpms,suspend}_psr_exit and
> > > {dpms_off, suspend}_psr_active tests uniquely achieve. Combine them so
> > > that we have one dpms and one suspend test.
> > Could you please add more details why these tests can be merged?
> 
> 
> The reason is - the tests are pretty much the same. Combining them saves
> run time.
> 
> 
> > Is it based on documentation or your experience?
> > Kasia
> > > 
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Katarzyna Dec <katarzyna.dec@intel.com>
> > > ---
> > >  tests/kms_psr_sink_crc.c | 34 +++-------------------------------
> > >  1 file changed, 3 insertions(+), 31 deletions(-)
> > > 
> > > diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> > > index d9cce5ea..e04e0f6a 100644
> > > --- a/tests/kms_psr_sink_crc.c
> > > +++ b/tests/kms_psr_sink_crc.c
> > > @@ -522,51 +522,23 @@ int main(int argc, char *argv[])
> > >  		}
> > >  	}
> > >  
> > > -	igt_subtest_f("dpms_off_psr_active") {
> > > +	igt_subtest_f("dpms") {
> > >  		data.test_plane = DRM_PLANE_TYPE_PRIMARY;
> > >  		data.op = RENDER;
> > >  		setup_test_plane(&data);
> > >  		igt_assert(wait_psr_entry(&data));
> > > -
> > >  		dpms_off_on(data);
> > > -
> > > -		run_test(&data);
> > > -		test_cleanup(&data);
> > > -	}
> > > -
> > > -	igt_subtest_f("dpms_off_psr_exit") {
> > > -		data.test_plane = DRM_PLANE_TYPE_OVERLAY;
> > > -		data.op = PLANE_ONOFF;
> > > -		setup_test_plane(&data);
> > > -
> > > -		dpms_off_on(data);
> > > -
> > > -		igt_assert(wait_psr_entry(&data));
> > > -		run_test(&data);
> > > -		test_cleanup(&data);
> > > -	}
> > > -
> > > -	igt_subtest_f("suspend_psr_active") {
> > > -		data.test_plane = DRM_PLANE_TYPE_PRIMARY;
> > > -		data.op = PAGE_FLIP;
> > > -		setup_test_plane(&data);
> > > -		igt_assert(wait_psr_entry(&data));
> > > -
> > > -		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> > > -					      SUSPEND_TEST_NONE);
> > > -
> > >  		run_test(&data);
> > >  		test_cleanup(&data);
> > >  	}
> > >  
> > > -	igt_subtest_f("suspend_psr_exit") {
> > > +	igt_subtest_f("suspend") {
> > >  		data.test_plane = DRM_PLANE_TYPE_CURSOR;
> > >  		data.op = PLANE_ONOFF;
> > >  		setup_test_plane(&data);
> > > -
> > > +		igt_assert(wait_psr_entry(&data));
> > >  		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> > >  					      SUSPEND_TEST_NONE);
> > > -
> > >  		igt_assert(wait_psr_entry(&data));
> > >  		run_test(&data);
> > >  		test_cleanup(&data);
> > > -- 
> > > 2.14.1
> > > 
> > > _______________________________________________
> > > igt-dev mailing list
> > > igt-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 08/20] tests/psr: Merge PSR dpms and suspend variants.

[Rodrigo Vivi]

On Mon, Apr 16, 2018 at 02:46:14PM -0700, Pandiyan, Dhinakaran wrote:
> On Thu, 2018-04-12 at 13:20 +0200, Katarzyna Dec wrote:
> > On Tue, Apr 10, 2018 at 07:37:20PM -0700, Dhinakaran Pandiyan wrote:
> > > I don't see a big difference in what {dpms,suspend}_psr_exit and
> > > {dpms_off, suspend}_psr_active tests uniquely achieve. Combine them so
> > > that we have one dpms and one suspend test.
> > Could you please add more details why these tests can be merged?
> 
> 
> The reason is - the tests are pretty much the same. Combining them saves
> run time.

Agree.... Also, the other one was never reliable since we could never
assure the state of PSR when doing the operation.

So I'm in favor of this merge:
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> 
> > Is it based on documentation or your experience?
> > Kasia
> > > 
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > ---
> > >  tests/kms_psr_sink_crc.c | 34 +++-------------------------------
> > >  1 file changed, 3 insertions(+), 31 deletions(-)
> > > 
> > > diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> > > index d9cce5ea..e04e0f6a 100644
> > > --- a/tests/kms_psr_sink_crc.c
> > > +++ b/tests/kms_psr_sink_crc.c
> > > @@ -522,51 +522,23 @@ int main(int argc, char *argv[])
> > >  		}
> > >  	}
> > >  
> > > -	igt_subtest_f("dpms_off_psr_active") {
> > > +	igt_subtest_f("dpms") {
> > >  		data.test_plane = DRM_PLANE_TYPE_PRIMARY;
> > >  		data.op = RENDER;
> > >  		setup_test_plane(&data);
> > >  		igt_assert(wait_psr_entry(&data));
> > > -
> > >  		dpms_off_on(data);
> > > -
> > > -		run_test(&data);
> > > -		test_cleanup(&data);
> > > -	}
> > > -
> > > -	igt_subtest_f("dpms_off_psr_exit") {
> > > -		data.test_plane = DRM_PLANE_TYPE_OVERLAY;
> > > -		data.op = PLANE_ONOFF;
> > > -		setup_test_plane(&data);
> > > -
> > > -		dpms_off_on(data);
> > > -
> > > -		igt_assert(wait_psr_entry(&data));
> > > -		run_test(&data);
> > > -		test_cleanup(&data);
> > > -	}
> > > -
> > > -	igt_subtest_f("suspend_psr_active") {
> > > -		data.test_plane = DRM_PLANE_TYPE_PRIMARY;
> > > -		data.op = PAGE_FLIP;
> > > -		setup_test_plane(&data);
> > > -		igt_assert(wait_psr_entry(&data));
> > > -
> > > -		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> > > -					      SUSPEND_TEST_NONE);
> > > -
> > >  		run_test(&data);
> > >  		test_cleanup(&data);
> > >  	}
> > >  
> > > -	igt_subtest_f("suspend_psr_exit") {
> > > +	igt_subtest_f("suspend") {
> > >  		data.test_plane = DRM_PLANE_TYPE_CURSOR;
> > >  		data.op = PLANE_ONOFF;
> > >  		setup_test_plane(&data);
> > > -
> > > +		igt_assert(wait_psr_entry(&data));
> > >  		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> > >  					      SUSPEND_TEST_NONE);
> > > -
> > >  		igt_assert(wait_psr_entry(&data));
> > >  		run_test(&data);
> > >  		test_cleanup(&data);
> > > -- 
> > > 2.14.1
> > > 
> > > _______________________________________________
> > > igt-dev mailing list
> > > igt-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 08/20] tests/psr: Merge PSR dpms and suspend variants.

[Dhinakaran Pandiyan]

On Thu, 2018-04-12 at 13:20 +0200, Katarzyna Dec wrote:
> On Tue, Apr 10, 2018 at 07:37:20PM -0700, Dhinakaran Pandiyan wrote:
> > I don't see a big difference in what {dpms,suspend}_psr_exit and
> > {dpms_off, suspend}_psr_active tests uniquely achieve. Combine them so
> > that we have one dpms and one suspend test.
> Could you please add more details why these tests can be merged?


The reason is - the tests are pretty much the same. Combining them saves
run time.


> Is it based on documentation or your experience?
> Kasia
> > 
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  tests/kms_psr_sink_crc.c | 34 +++-------------------------------
> >  1 file changed, 3 insertions(+), 31 deletions(-)
> > 
> > diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> > index d9cce5ea..e04e0f6a 100644
> > --- a/tests/kms_psr_sink_crc.c
> > +++ b/tests/kms_psr_sink_crc.c
> > @@ -522,51 +522,23 @@ int main(int argc, char *argv[])
> >  		}
> >  	}
> >  
> > -	igt_subtest_f("dpms_off_psr_active") {
> > +	igt_subtest_f("dpms") {
> >  		data.test_plane = DRM_PLANE_TYPE_PRIMARY;
> >  		data.op = RENDER;
> >  		setup_test_plane(&data);
> >  		igt_assert(wait_psr_entry(&data));
> > -
> >  		dpms_off_on(data);
> > -
> > -		run_test(&data);
> > -		test_cleanup(&data);
> > -	}
> > -
> > -	igt_subtest_f("dpms_off_psr_exit") {
> > -		data.test_plane = DRM_PLANE_TYPE_OVERLAY;
> > -		data.op = PLANE_ONOFF;
> > -		setup_test_plane(&data);
> > -
> > -		dpms_off_on(data);
> > -
> > -		igt_assert(wait_psr_entry(&data));
> > -		run_test(&data);
> > -		test_cleanup(&data);
> > -	}
> > -
> > -	igt_subtest_f("suspend_psr_active") {
> > -		data.test_plane = DRM_PLANE_TYPE_PRIMARY;
> > -		data.op = PAGE_FLIP;
> > -		setup_test_plane(&data);
> > -		igt_assert(wait_psr_entry(&data));
> > -
> > -		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> > -					      SUSPEND_TEST_NONE);
> > -
> >  		run_test(&data);
> >  		test_cleanup(&data);
> >  	}
> >  
> > -	igt_subtest_f("suspend_psr_exit") {
> > +	igt_subtest_f("suspend") {
> >  		data.test_plane = DRM_PLANE_TYPE_CURSOR;
> >  		data.op = PLANE_ONOFF;
> >  		setup_test_plane(&data);
> > -
> > +		igt_assert(wait_psr_entry(&data));
> >  		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> >  					      SUSPEND_TEST_NONE);
> > -
> >  		igt_assert(wait_psr_entry(&data));
> >  		run_test(&data);
> >  		test_cleanup(&data);
> > -- 
> > 2.14.1
> > 
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

[igt-dev] [PATCH i-g-t 09/20] tests/psr: Check for PSR entry more often and only for a second.

[Dhinakaran Pandiyan]

1 second interval is too long, check every 100ms.

PSR gets enabled when pipes are enabled on BDW+. It shouldn't take
5 seconds even on VLV/CHV, where we delay a bit. Limit the checks to a
1 second total wait, so that we know if the delay is longer.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 tests/kms_psr_sink_crc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index e04e0f6a..d940ec89 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -214,11 +214,13 @@ static bool psr_enabled(data_t *data)
 
 static bool wait_psr_entry(data_t *data)
 {
-	int timeout = 5;
+	int timeout = 10;
+
 	while (timeout--) {
 		if (psr_enabled(data))
 			return true;
-		sleep(1);
+		usleep(100000);
+		igt_debug("Waiting for PSR entry %d ms\n", (10-timeout)*100);
 	}
 	return false;
 }
-- 
2.14.1

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

Re: [igt-dev] [PATCH i-g-t 09/20] tests/psr: Check for PSR entry more often and only for a second.

[Katarzyna Dec]

On Tue, Apr 10, 2018 at 07:37:21PM -0700, Dhinakaran Pandiyan wrote:
> 1 second interval is too long, check every 100ms.
> 
> PSR gets enabled when pipes are enabled on BDW+. It shouldn't take
> 5 seconds even on VLV/CHV, where we delay a bit. Limit the checks to a
> 1 second total wait, so that we know if the delay is longer.
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  tests/kms_psr_sink_crc.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index e04e0f6a..d940ec89 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -214,11 +214,13 @@ static bool psr_enabled(data_t *data)
>  
>  static bool wait_psr_entry(data_t *data)
>  {
> -	int timeout = 5;
> +	int timeout = 10;
> +
>  	while (timeout--) {
>  		if (psr_enabled(data))
>  			return true;
> -		sleep(1);
> +		usleep(100000);
 Is this change based on documentation or on some experimental values?                                                                                                                                             
  Kasia
> +		igt_debug("Waiting for PSR entry %d ms\n", (10-timeout)*100);
>  	}
>  	return false;
>  }
> -- 
> 2.14.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 09/20] tests/psr: Check for PSR entry more often and only for a second.

[Dhinakaran Pandiyan]

On Thu, 2018-04-12 at 13:25 +0200, Katarzyna Dec wrote:
> On Tue, Apr 10, 2018 at 07:37:21PM -0700, Dhinakaran Pandiyan wrote:
> > 1 second interval is too long, check every 100ms.
> > 
> > PSR gets enabled when pipes are enabled on BDW+. It shouldn't take
> > 5 seconds even on VLV/CHV, where we delay a bit. Limit the checks to a
> > 1 second total wait, so that we know if the delay is longer.
> > 
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  tests/kms_psr_sink_crc.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> > index e04e0f6a..d940ec89 100644
> > --- a/tests/kms_psr_sink_crc.c
> > +++ b/tests/kms_psr_sink_crc.c
> > @@ -214,11 +214,13 @@ static bool psr_enabled(data_t *data)
> >  
> >  static bool wait_psr_entry(data_t *data)
> >  {
> > -	int timeout = 5;
> > +	int timeout = 10;
> > +
> >  	while (timeout--) {
> >  		if (psr_enabled(data))
> >  			return true;
> > -		sleep(1);
> > +		usleep(100000);
>  Is this change based on documentation or on some experimental values?

The order of the delay is in 100 micro seconds. As we don't know the
exact value, checking every 100 us made sense. The total delay of 1
second however is probably not enough on VLV/CHV. 

Anyway this optimization isn't necessary now as I am thinking of killing
PSR support of VLV/CHV in the driver. Please ignore this patch.

>                                                                                                                                           
>   Kasia
> > +		igt_debug("Waiting for PSR entry %d ms\n", (10-timeout)*100);
> >  	}
> >  	return false;
> >  }
> > -- 
> > 2.14.1
> > 
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

[igt-dev] [PATCH i-g-t 10/20] tests/psr: Remove delay between dpms toggle.

[Dhinakaran Pandiyan]

No reason for the delay between dpms off and on.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 tests/kms_psr_sink_crc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index d940ec89..17490726 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -428,7 +428,6 @@ static void dpms_off_on(data_t data)
 {
 	kmstest_set_connector_dpms(data.drm_fd, data.output->config.connector,
 				   DRM_MODE_DPMS_OFF);
-	sleep(1);
 	kmstest_set_connector_dpms(data.drm_fd, data.output->config.connector,
 				   DRM_MODE_DPMS_ON);
 }
-- 
2.14.1

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

Re: [igt-dev] [PATCH i-g-t 10/20] tests/psr: Remove delay between dpms toggle.

[Katarzyna Dec]

On Tue, Apr 10, 2018 at 07:37:22PM -0700, Dhinakaran Pandiyan wrote:
> No reason for the delay between dpms off and on.
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  tests/kms_psr_sink_crc.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index d940ec89..17490726 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -428,7 +428,6 @@ static void dpms_off_on(data_t data)
>  {
>  	kmstest_set_connector_dpms(data.drm_fd, data.output->config.connector,
>  				   DRM_MODE_DPMS_OFF);
> -	sleep(1);
Why there was as sleep at the first place here? Probably there was some reason.
Kasia
>  	kmstest_set_connector_dpms(data.drm_fd, data.output->config.connector,
>  				   DRM_MODE_DPMS_ON);
>  }
> -- 
> 2.14.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 10/20] tests/psr: Remove delay between dpms toggle.

[Dhinakaran Pandiyan]

On Thu, 2018-04-12 at 13:26 +0200, Katarzyna Dec wrote:
> On Tue, Apr 10, 2018 at 07:37:22PM -0700, Dhinakaran Pandiyan wrote:
> > No reason for the delay between dpms off and on.
> > 
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  tests/kms_psr_sink_crc.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> > index d940ec89..17490726 100644
> > --- a/tests/kms_psr_sink_crc.c
> > +++ b/tests/kms_psr_sink_crc.c
> > @@ -428,7 +428,6 @@ static void dpms_off_on(data_t data)
> >  {
> >  	kmstest_set_connector_dpms(data.drm_fd, data.output->config.connector,
> >  				   DRM_MODE_DPMS_OFF);
> > -	sleep(1);
> Why there was as sleep at the first place here? Probably there was some reason.

Probably. It is not obvious though. The test works fine without it and
removing it cuts down the running time, which means we can run other
useful tests.

> Kasia
> >  	kmstest_set_connector_dpms(data.drm_fd, data.output->config.connector,
> >  				   DRM_MODE_DPMS_ON);
> >  }
> > -- 
> > 2.14.1
> > 
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

Re: [igt-dev] [PATCH i-g-t 10/20] tests/psr: Remove delay between dpms toggle.

[Katarzyna Dec]

On Thu, Apr 12, 2018 at 05:24:34PM -0700, Dhinakaran Pandiyan wrote:
> 
> 
> 
> On Thu, 2018-04-12 at 13:26 +0200, Katarzyna Dec wrote:
> > On Tue, Apr 10, 2018 at 07:37:22PM -0700, Dhinakaran Pandiyan wrote:
> > > No reason for the delay between dpms off and on.
> > > 
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > ---
> > >  tests/kms_psr_sink_crc.c | 1 -
> > >  1 file changed, 1 deletion(-)
> > > 
> > > diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> > > index d940ec89..17490726 100644
> > > --- a/tests/kms_psr_sink_crc.c
> > > +++ b/tests/kms_psr_sink_crc.c
> > > @@ -428,7 +428,6 @@ static void dpms_off_on(data_t data)
> > >  {
> > >  	kmstest_set_connector_dpms(data.drm_fd, data.output->config.connector,
> > >  				   DRM_MODE_DPMS_OFF);
> > > -	sleep(1);
> > Why there was as sleep at the first place here? Probably there was some reason.
> 
> Probably. It is not obvious though. The test works fine without it and
> removing it cuts down the running time, which means we can run other
> useful tests.
> 
> > Kasia
Reviewed-by:  Katarzyna Dec <katarzyna.dec@intel.com>
> > >  	kmstest_set_connector_dpms(data.drm_fd, data.output->config.connector,
> > >  				   DRM_MODE_DPMS_ON);
> > >  }
> > > -- 
> > > 2.14.1
> > > 
> > > _______________________________________________
> > > igt-dev mailing list
> > > igt-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 11/20] tests/psr: Check for drrs status only after checking for PSR

[Dhinakaran Pandiyan]

The goal of this test is (or should be) to verify DRRS is disabled if PSR
was enabled. There is no point in checking for DRRS status if PSR was not
enabled in the first place.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 tests/kms_psr_sink_crc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 17490726..a7c6a057 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -486,6 +486,7 @@ int main(int argc, char *argv[])
 
 	igt_subtest("no_drrs") {
 		setup_test_plane(&data);
+		igt_assert(wait_psr_entry(&data));
 		igt_assert(drrs_disabled(&data));
 		test_cleanup(&data);
 	}
-- 
2.14.1

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

Re: [igt-dev] [PATCH i-g-t 11/20] tests/psr: Check for drrs status only after checking for PSR

[Katarzyna Dec]

On Tue, Apr 10, 2018 at 07:37:23PM -0700, Dhinakaran Pandiyan wrote:
> The goal of this test is (or should be) to verify DRRS is disabled if PSR
> was enabled. There is no point in checking for DRRS status if PSR was not
> enabled in the first place.
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Katarzyna Dec <katarzyna.dec@intel.com>
> ---
>  tests/kms_psr_sink_crc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index 17490726..a7c6a057 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -486,6 +486,7 @@ int main(int argc, char *argv[])
>  
>  	igt_subtest("no_drrs") {
>  		setup_test_plane(&data);
> +		igt_assert(wait_psr_entry(&data));
>  		igt_assert(drrs_disabled(&data));
>  		test_cleanup(&data);
>  	}
> -- 
> 2.14.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 12/20] tests/sink_crc_basic: Debug print CRC values

[Dhinakaran Pandiyan]

The CRC values are useful as a reference to compare them with values
generated from other feature (PSR) tests.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 tests/kms_sink_crc_basic.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/kms_sink_crc_basic.c b/tests/kms_sink_crc_basic.c
index 159683e2..1c31e30a 100644
--- a/tests/kms_sink_crc_basic.c
+++ b/tests/kms_sink_crc_basic.c
@@ -63,11 +63,13 @@ static void assert_color(int dir, enum color color)
 		igt_assert_lt(0, r);
 		igt_assert_eq(0, g);
 		igt_assert_eq(0, b);
+		igt_debug("sink CRC for red %.4x%.4x%.4x\n", r, g, b);
 		break;
 	case GREEN:
 		igt_assert_eq(0, r);
 		igt_assert_lt(0, g);
 		igt_assert_eq(0, b);
+		igt_debug("sink CRC for green %.4x%.4x%.4x\n", r, g, b);
 		break;
 	default:
 		igt_fail(IGT_EXIT_FAILURE);
-- 
2.14.1

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

Re: [igt-dev] [PATCH i-g-t 12/20] tests/sink_crc_basic: Debug print CRC values

[Katarzyna Dec]

On Tue, Apr 10, 2018 at 07:37:24PM -0700, Dhinakaran Pandiyan wrote:
> The CRC values are useful as a reference to compare them with values
> generated from other feature (PSR) tests.
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
 Reviewed-by: Katarzyna Dec <katarzyna.dec@intel.com>
> ---
>  tests/kms_sink_crc_basic.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tests/kms_sink_crc_basic.c b/tests/kms_sink_crc_basic.c
> index 159683e2..1c31e30a 100644
> --- a/tests/kms_sink_crc_basic.c
> +++ b/tests/kms_sink_crc_basic.c
> @@ -63,11 +63,13 @@ static void assert_color(int dir, enum color color)
>  		igt_assert_lt(0, r);
>  		igt_assert_eq(0, g);
>  		igt_assert_eq(0, b);
> +		igt_debug("sink CRC for red %.4x%.4x%.4x\n", r, g, b);
>  		break;
>  	case GREEN:
>  		igt_assert_eq(0, r);
>  		igt_assert_lt(0, g);
>  		igt_assert_eq(0, b);
> +		igt_debug("sink CRC for green %.4x%.4x%.4x\n", r, g, b);
>  		break;
>  	default:
>  		igt_fail(IGT_EXIT_FAILURE);
> -- 
> 2.14.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 13/20] tests/psr: Get rid of global variable running_with_psr_disabled

[Dhinakaran Pandiyan]

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 tests/kms_psr_sink_crc.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index a7c6a057..bb92d672 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -31,8 +31,6 @@
 
 #include "intel_bufmgr.h"
 
-bool running_with_psr_disabled;
-
 #define CRC_BLACK "000000000000"
 #define CRC_LEN 12
 
@@ -76,6 +74,7 @@ typedef struct {
 	int mod_stride;
 	drmModeModeInfo *mode;
 	igt_output_t *output;
+	bool with_psr_disabled;
 } data_t;
 
 static void create_cursor_fb(data_t *data)
@@ -93,7 +92,6 @@ static void create_cursor_fb(data_t *data)
 	igt_put_cairo_ctx(data->drm_fd, &data->fb_white, cr);
 }
 
-
 static void setup_output(data_t *data)
 {
 	igt_display_t *display = &data->display;
@@ -198,7 +196,7 @@ static bool sink_support(data_t *data)
 
 	igt_debugfs_read(data->drm_fd, "i915_edp_psr_status", buf);
 
-	return running_with_psr_disabled ||
+	return data->with_psr_disabled ||
 		strstr(buf, "Sink_Support: yes\n");
 }
 
@@ -208,7 +206,7 @@ static bool psr_enabled(data_t *data)
 
 	igt_debugfs_read(data->drm_fd, "i915_edp_psr_status", buf);
 
-	return running_with_psr_disabled ||
+	return data->with_psr_disabled ||
 		strstr(buf, "HW Enabled & Active bit: yes\n");
 }
 
@@ -432,11 +430,13 @@ static void dpms_off_on(data_t data)
 				   DRM_MODE_DPMS_ON);
 }
 
-static int opt_handler(int opt, int opt_index, void *data)
+static int opt_handler(int opt, int opt_index, void *_data)
 {
+	data_t *data = _data;
+
 	switch (opt) {
 	case 'n':
-		running_with_psr_disabled = true;
+		data->with_psr_disabled = true;
 		break;
 	default:
 		igt_assert(0);
@@ -457,7 +457,7 @@ int main(int argc, char *argv[])
 	enum operations op;
 
 	igt_subtest_init_parse_opts(&argc, argv, "", long_options,
-				    help_str, opt_handler, NULL);
+				    help_str, opt_handler, &data);
 	igt_skip_on_simulation();
 
 	igt_fixture {
@@ -466,7 +466,7 @@ int main(int argc, char *argv[])
 		kmstest_set_vt_graphics_mode();
 		data.devid = intel_get_drm_devid(data.drm_fd);
 
-		igt_set_module_param_int("enable_psr", running_with_psr_disabled ?
+		igt_set_module_param_int("enable_psr", data.with_psr_disabled ?
 					 0 : 1);
 		igt_require_f(sink_support(&data),
 			      "Sink does not support PSR\n");
-- 
2.14.1

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

Re: [igt-dev] [PATCH i-g-t 13/20] tests/psr: Get rid of global variable running_with_psr_disabled

[Katarzyna Dec]

On Tue, Apr 10, 2018 at 07:37:25PM -0700, Dhinakaran Pandiyan wrote:
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
 Reviewed-by: Katarzyna Dec <katarzyna.dec@intel.com>
> ---
>  tests/kms_psr_sink_crc.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index a7c6a057..bb92d672 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -31,8 +31,6 @@
>  
>  #include "intel_bufmgr.h"
>  
> -bool running_with_psr_disabled;
> -
>  #define CRC_BLACK "000000000000"
>  #define CRC_LEN 12
>  
> @@ -76,6 +74,7 @@ typedef struct {
>  	int mod_stride;
>  	drmModeModeInfo *mode;
>  	igt_output_t *output;
> +	bool with_psr_disabled;
>  } data_t;
>  
>  static void create_cursor_fb(data_t *data)
> @@ -93,7 +92,6 @@ static void create_cursor_fb(data_t *data)
>  	igt_put_cairo_ctx(data->drm_fd, &data->fb_white, cr);
>  }
>  
> -
>  static void setup_output(data_t *data)
>  {
>  	igt_display_t *display = &data->display;
> @@ -198,7 +196,7 @@ static bool sink_support(data_t *data)
>  
>  	igt_debugfs_read(data->drm_fd, "i915_edp_psr_status", buf);
>  
> -	return running_with_psr_disabled ||
> +	return data->with_psr_disabled ||
>  		strstr(buf, "Sink_Support: yes\n");
>  }
>  
> @@ -208,7 +206,7 @@ static bool psr_enabled(data_t *data)
>  
>  	igt_debugfs_read(data->drm_fd, "i915_edp_psr_status", buf);
>  
> -	return running_with_psr_disabled ||
> +	return data->with_psr_disabled ||
>  		strstr(buf, "HW Enabled & Active bit: yes\n");
>  }
>  
> @@ -432,11 +430,13 @@ static void dpms_off_on(data_t data)
>  				   DRM_MODE_DPMS_ON);
>  }
>  
> -static int opt_handler(int opt, int opt_index, void *data)
> +static int opt_handler(int opt, int opt_index, void *_data)
>  {
> +	data_t *data = _data;
> +
>  	switch (opt) {
>  	case 'n':
> -		running_with_psr_disabled = true;
> +		data->with_psr_disabled = true;
>  		break;
>  	default:
>  		igt_assert(0);
> @@ -457,7 +457,7 @@ int main(int argc, char *argv[])
>  	enum operations op;
>  
>  	igt_subtest_init_parse_opts(&argc, argv, "", long_options,
> -				    help_str, opt_handler, NULL);
> +				    help_str, opt_handler, &data);
>  	igt_skip_on_simulation();
>  
>  	igt_fixture {
> @@ -466,7 +466,7 @@ int main(int argc, char *argv[])
>  		kmstest_set_vt_graphics_mode();
>  		data.devid = intel_get_drm_devid(data.drm_fd);
>  
> -		igt_set_module_param_int("enable_psr", running_with_psr_disabled ?
> +		igt_set_module_param_int("enable_psr", data.with_psr_disabled ?
>  					 0 : 1);
>  		igt_require_f(sink_support(&data),
>  			      "Sink does not support PSR\n");
> -- 
> 2.14.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 14/20] tests/psr: Make testing with sink CRC non-default

[Dhinakaran Pandiyan]

Testing with CRC has always been unreliable. CRC also appears to be
suspectible to signal integrity of the link between source and sink.
But, most importantly, I see that reading sink CRC causes PSR exits
rendering CRC based testing for PSR exits pointless. So hide this under
a non-default --sink-crc command line option.

This makes all tests other than basic and no-drrs require
interactive-debug, but that's okay since using sink CRC is misleading.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 tests/kms_psr_sink_crc.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index bb92d672..c9197c9d 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -75,6 +75,7 @@ typedef struct {
 	drmModeModeInfo *mode;
 	igt_output_t *output;
 	bool with_psr_disabled;
+	bool with_sink_crc;
 } data_t;
 
 static void create_cursor_fb(data_t *data)
@@ -273,6 +274,10 @@ static void run_test(data_t *data)
 	char crc[CRC_LEN];
 	const char *expected = "";
 
+	if (!igt_interactive_debug)
+		igt_require_f(data->with_sink_crc,
+			      "Enable sink CRC with --sink-crc\n");
+
 	/* Confirm that screen became Green */
 	get_sink_crc(data, ref_crc);
 	assert_or_manual(is_green(ref_crc), "screen GREEN");
@@ -438,6 +443,9 @@ static int opt_handler(int opt, int opt_index, void *_data)
 	case 'n':
 		data->with_psr_disabled = true;
 		break;
+	case 'c':
+		data->with_sink_crc = true;
+		break;
 	default:
 		igt_assert(0);
 	}
@@ -448,9 +456,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 to check the CRC test logic.";
+	       "  --no-psr\tRun test without PSR to check the CRC test logic.\n"	\
+	       "  --sink-crc\tRun tests using sink CRC";
 	static struct option long_options[] = {
 		{"no-psr", 0, 0, 'n'},
+		{"sink-crc", 0, 0, 'c'},
 		{ 0, 0, 0, 0 }
 	};
 	data_t data = {};
-- 
2.14.1

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

Re: [igt-dev] [PATCH i-g-t 14/20] tests/psr: Make testing with sink CRC non-default

[Katarzyna Dec]

On Tue, Apr 10, 2018 at 07:37:26PM -0700, Dhinakaran Pandiyan wrote:
> Testing with CRC has always been unreliable. CRC also appears to be
> suspectible to signal integrity of the link between source and sink.
> But, most importantly, I see that reading sink CRC causes PSR exits
> rendering CRC based testing for PSR exits pointless. So hide this under
> a non-default --sink-crc command line option.
> 
> This makes all tests other than basic and no-drrs require
> interactive-debug, but that's okay since using sink CRC is misleading.
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
 Reviewed-by: Katarzyna Dec <katarzyna.dec@intel.com>
> ---
>  tests/kms_psr_sink_crc.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index bb92d672..c9197c9d 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -75,6 +75,7 @@ typedef struct {
>  	drmModeModeInfo *mode;
>  	igt_output_t *output;
>  	bool with_psr_disabled;
> +	bool with_sink_crc;
>  } data_t;
>  
>  static void create_cursor_fb(data_t *data)
> @@ -273,6 +274,10 @@ static void run_test(data_t *data)
>  	char crc[CRC_LEN];
>  	const char *expected = "";
>  
> +	if (!igt_interactive_debug)
> +		igt_require_f(data->with_sink_crc,
> +			      "Enable sink CRC with --sink-crc\n");
> +
>  	/* Confirm that screen became Green */
>  	get_sink_crc(data, ref_crc);
>  	assert_or_manual(is_green(ref_crc), "screen GREEN");
> @@ -438,6 +443,9 @@ static int opt_handler(int opt, int opt_index, void *_data)
>  	case 'n':
>  		data->with_psr_disabled = true;
>  		break;
> +	case 'c':
> +		data->with_sink_crc = true;
> +		break;
>  	default:
>  		igt_assert(0);
>  	}
> @@ -448,9 +456,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 to check the CRC test logic.";
> +	       "  --no-psr\tRun test without PSR to check the CRC test logic.\n"	\
> +	       "  --sink-crc\tRun tests using sink CRC";
>  	static struct option long_options[] = {
>  		{"no-psr", 0, 0, 'n'},
> +		{"sink-crc", 0, 0, 'c'},
>  		{ 0, 0, 0, 0 }
>  	};
>  	data_t data = {};
> -- 
> 2.14.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 15/20] tests/psr: Eliminate storing pointers for igt_plane_type.

[Dhinakaran Pandiyan]

After the initial plane setup, only the test plane is required. One
exception is clean_up where the primary is required, but a call to
igt_output_get_plane_type() can get us that.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 tests/kms_psr_sink_crc.c | 73 +++++++++++++++++++++---------------------------
 1 file changed, 32 insertions(+), 41 deletions(-)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index c9197c9d..f22edb48 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -62,14 +62,13 @@ static const char *op_str(enum operations op)
 typedef struct {
 	int drm_fd;
 	int debugfs_fd;
-	int test_plane;
 	enum operations op;
 	uint32_t devid;
 	uint32_t crtc_id;
 	igt_display_t display;
 	drm_intel_bufmgr *bufmgr;
 	struct igt_fb fb_green, fb_white;
-	igt_plane_t *primary, *sprite, *cursor;
+	igt_plane_t *test_plane;
 	int mod_size;
 	int mod_stride;
 	drmModeModeInfo *mode;
@@ -268,7 +267,7 @@ static bool drrs_disabled(data_t *data)
 static void run_test(data_t *data)
 {
 	uint32_t handle = data->fb_white.gem_handle;
-	igt_plane_t *test_plane;
+	igt_plane_t *test_plane = data->test_plane;
 	void *ptr;
 	char ref_crc[CRC_LEN];
 	char crc[CRC_LEN];
@@ -288,18 +287,13 @@ static void run_test(data_t *data)
 	assert_or_manual(is_green(ref_crc), "screen GREEN");
 
 	/* Setting a secondary fb/plane */
-	switch (data->test_plane) {
-	case DRM_PLANE_TYPE_PRIMARY: default: test_plane = data->primary; break;
-	case DRM_PLANE_TYPE_OVERLAY: test_plane = data->sprite; break;
-	case DRM_PLANE_TYPE_CURSOR: test_plane = data->cursor; break;
-	}
 	igt_plane_set_fb(test_plane, &data->fb_white);
 	igt_display_commit(&data->display);
 
 	/* Confirm it is not Green anymore */
 	igt_assert(wait_psr_entry(data));
 	get_sink_crc(data, ref_crc);
-	if (data->test_plane == DRM_PLANE_TYPE_PRIMARY)
+	if (test_plane->type == DRM_PLANE_TYPE_PRIMARY)
 		assert_or_manual(!is_green(ref_crc), "screen WHITE");
 	else
 		assert_or_manual(!is_green(ref_crc), "GREEN background with WHITE box");
@@ -358,21 +352,22 @@ static void run_test(data_t *data)
 }
 
 static void test_cleanup(data_t *data) {
-	igt_plane_set_fb(data->primary, NULL);
-	if (data->test_plane == DRM_PLANE_TYPE_OVERLAY)
-		igt_plane_set_fb(data->sprite, NULL);
-	if (data->test_plane == DRM_PLANE_TYPE_CURSOR)
-		igt_plane_set_fb(data->cursor, NULL);
+	igt_plane_t *primary;
 
+	primary = igt_output_get_plane_type(data->output,
+					    DRM_PLANE_TYPE_PRIMARY);
+	igt_plane_set_fb(primary, NULL);
+	igt_plane_set_fb(data->test_plane, NULL);
 	igt_display_commit(&data->display);
 
 	igt_remove_fb(data->drm_fd, &data->fb_green);
 	igt_remove_fb(data->drm_fd, &data->fb_white);
 }
 
-static void setup_test_plane(data_t *data)
+static void setup_test_plane(data_t *data, int test_plane)
 {
 	uint32_t white_h, white_v;
+	igt_plane_t *primary, *sprite, *cursor;
 
 	igt_create_color_fb(data->drm_fd,
 			    data->mode->hdisplay, data->mode->vdisplay,
@@ -381,8 +376,10 @@ static void setup_test_plane(data_t *data)
 			    0.0, 1.0, 0.0,
 			    &data->fb_green);
 
-	data->primary = igt_output_get_plane_type(data->output, DRM_PLANE_TYPE_PRIMARY);
-	igt_plane_set_fb(data->primary, NULL);
+	primary = igt_output_get_plane_type(data->output,
+					    DRM_PLANE_TYPE_PRIMARY);
+	igt_plane_set_fb(primary, NULL);
+	data->test_plane = primary;
 
 	white_h = data->mode->hdisplay;
 	white_v = data->mode->vdisplay;
@@ -391,16 +388,14 @@ static void setup_test_plane(data_t *data)
 	data->mod_size = white_h * white_v;
 	data->mod_stride = white_h * 4;
 
-	switch (data->test_plane) {
+	switch (test_plane) {
 	case DRM_PLANE_TYPE_OVERLAY:
-		data->sprite = igt_output_get_plane_type(data->output,
-						    DRM_PLANE_TYPE_OVERLAY);
-		igt_plane_set_fb(data->sprite, NULL);
-		/* To make it different for human eyes let's make
-		 * sprite visible in only one quarter of the primary
-		 */
+		sprite = igt_output_get_plane_type(data->output,
+						   DRM_PLANE_TYPE_OVERLAY);
+		igt_plane_set_fb(sprite, NULL);
 		white_h = white_h/2;
 		white_v = white_v/2;
+		data->test_plane = sprite;
 	case DRM_PLANE_TYPE_PRIMARY:
 		igt_create_color_fb(data->drm_fd,
 				    white_h, white_v,
@@ -410,20 +405,21 @@ static void setup_test_plane(data_t *data)
 				    &data->fb_white);
 		break;
 	case DRM_PLANE_TYPE_CURSOR:
-		data->cursor = igt_output_get_plane_type(data->output,
-						    DRM_PLANE_TYPE_CURSOR);
-		igt_plane_set_fb(data->cursor, NULL);
+		cursor = igt_output_get_plane_type(data->output,
+						   DRM_PLANE_TYPE_CURSOR);
+		igt_plane_set_fb(cursor, NULL);
 		create_cursor_fb(data);
-		igt_plane_set_position(data->cursor, 0, 0);
+		igt_plane_set_position(cursor, 0, 0);
 
 		/* Cursor is 64 x 64, ignoring pitch and bbp again */
 		data->mod_size = 64 * 64;
+		data->test_plane = cursor;
 		break;
 	}
 
 	igt_display_commit(&data->display);
 
-	igt_plane_set_fb(data->primary, &data->fb_green);
+	igt_plane_set_fb(primary, &data->fb_green);
 	igt_display_commit(&data->display);
 }
 
@@ -489,13 +485,13 @@ int main(int argc, char *argv[])
 	}
 
 	igt_subtest("basic") {
-		setup_test_plane(&data);
+		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
 		igt_assert(wait_psr_entry(&data));
 		test_cleanup(&data);
 	}
 
 	igt_subtest("no_drrs") {
-		setup_test_plane(&data);
+		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
 		igt_assert(wait_psr_entry(&data));
 		igt_assert(drrs_disabled(&data));
 		test_cleanup(&data);
@@ -503,9 +499,8 @@ int main(int argc, char *argv[])
 
 	for (op = PAGE_FLIP; op <= RENDER; op++) {
 		igt_subtest_f("primary_%s", op_str(op)) {
-			data.test_plane = DRM_PLANE_TYPE_PRIMARY;
 			data.op = op;
-			setup_test_plane(&data);
+			setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
 			igt_assert(wait_psr_entry(&data));
 			run_test(&data);
 			test_cleanup(&data);
@@ -514,9 +509,8 @@ int main(int argc, char *argv[])
 
 	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
 		igt_subtest_f("sprite_%s", op_str(op)) {
-			data.test_plane = DRM_PLANE_TYPE_OVERLAY;
 			data.op = op;
-			setup_test_plane(&data);
+			setup_test_plane(&data, DRM_PLANE_TYPE_OVERLAY);
 			igt_assert(wait_psr_entry(&data));
 			run_test(&data);
 			test_cleanup(&data);
@@ -525,9 +519,8 @@ int main(int argc, char *argv[])
 
 	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
 		igt_subtest_f("cursor_%s", op_str(op)) {
-			data.test_plane = DRM_PLANE_TYPE_CURSOR;
 			data.op = op;
-			setup_test_plane(&data);
+			setup_test_plane(&data, DRM_PLANE_TYPE_CURSOR);
 			igt_assert(wait_psr_entry(&data));
 			run_test(&data);
 			test_cleanup(&data);
@@ -535,9 +528,8 @@ int main(int argc, char *argv[])
 	}
 
 	igt_subtest_f("dpms") {
-		data.test_plane = DRM_PLANE_TYPE_PRIMARY;
 		data.op = RENDER;
-		setup_test_plane(&data);
+		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
 		igt_assert(wait_psr_entry(&data));
 		dpms_off_on(data);
 		run_test(&data);
@@ -545,9 +537,8 @@ int main(int argc, char *argv[])
 	}
 
 	igt_subtest_f("suspend") {
-		data.test_plane = DRM_PLANE_TYPE_CURSOR;
 		data.op = PLANE_ONOFF;
-		setup_test_plane(&data);
+		setup_test_plane(&data, DRM_PLANE_TYPE_CURSOR);
 		igt_assert(wait_psr_entry(&data));
 		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
 					      SUSPEND_TEST_NONE);
-- 
2.14.1

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

Re: [igt-dev] [PATCH i-g-t 15/20] tests/psr: Eliminate storing pointers for igt_plane_type.

[Katarzyna Dec]

On Tue, Apr 10, 2018 at 07:37:27PM -0700, Dhinakaran Pandiyan wrote:
> After the initial plane setup, only the test plane is required. One
> exception is clean_up where the primary is required, but a call to
> igt_output_get_plane_type() can get us that.
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
 Reviewed-by: Katarzyna Dec <katarzyna.dec@intel.com>
> ---
>  tests/kms_psr_sink_crc.c | 73 +++++++++++++++++++++---------------------------
>  1 file changed, 32 insertions(+), 41 deletions(-)
> 
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index c9197c9d..f22edb48 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -62,14 +62,13 @@ static const char *op_str(enum operations op)
>  typedef struct {
>  	int drm_fd;
>  	int debugfs_fd;
> -	int test_plane;
>  	enum operations op;
>  	uint32_t devid;
>  	uint32_t crtc_id;
>  	igt_display_t display;
>  	drm_intel_bufmgr *bufmgr;
>  	struct igt_fb fb_green, fb_white;
> -	igt_plane_t *primary, *sprite, *cursor;
> +	igt_plane_t *test_plane;
>  	int mod_size;
>  	int mod_stride;
>  	drmModeModeInfo *mode;
> @@ -268,7 +267,7 @@ static bool drrs_disabled(data_t *data)
>  static void run_test(data_t *data)
>  {
>  	uint32_t handle = data->fb_white.gem_handle;
> -	igt_plane_t *test_plane;
> +	igt_plane_t *test_plane = data->test_plane;
>  	void *ptr;
>  	char ref_crc[CRC_LEN];
>  	char crc[CRC_LEN];
> @@ -288,18 +287,13 @@ static void run_test(data_t *data)
>  	assert_or_manual(is_green(ref_crc), "screen GREEN");
>  
>  	/* Setting a secondary fb/plane */
> -	switch (data->test_plane) {
> -	case DRM_PLANE_TYPE_PRIMARY: default: test_plane = data->primary; break;
> -	case DRM_PLANE_TYPE_OVERLAY: test_plane = data->sprite; break;
> -	case DRM_PLANE_TYPE_CURSOR: test_plane = data->cursor; break;
> -	}
>  	igt_plane_set_fb(test_plane, &data->fb_white);
>  	igt_display_commit(&data->display);
>  
>  	/* Confirm it is not Green anymore */
>  	igt_assert(wait_psr_entry(data));
>  	get_sink_crc(data, ref_crc);
> -	if (data->test_plane == DRM_PLANE_TYPE_PRIMARY)
> +	if (test_plane->type == DRM_PLANE_TYPE_PRIMARY)
>  		assert_or_manual(!is_green(ref_crc), "screen WHITE");
>  	else
>  		assert_or_manual(!is_green(ref_crc), "GREEN background with WHITE box");
> @@ -358,21 +352,22 @@ static void run_test(data_t *data)
>  }
>  
>  static void test_cleanup(data_t *data) {
> -	igt_plane_set_fb(data->primary, NULL);
> -	if (data->test_plane == DRM_PLANE_TYPE_OVERLAY)
> -		igt_plane_set_fb(data->sprite, NULL);
> -	if (data->test_plane == DRM_PLANE_TYPE_CURSOR)
> -		igt_plane_set_fb(data->cursor, NULL);
> +	igt_plane_t *primary;
>  
> +	primary = igt_output_get_plane_type(data->output,
> +					    DRM_PLANE_TYPE_PRIMARY);
> +	igt_plane_set_fb(primary, NULL);
> +	igt_plane_set_fb(data->test_plane, NULL);
>  	igt_display_commit(&data->display);
>  
>  	igt_remove_fb(data->drm_fd, &data->fb_green);
>  	igt_remove_fb(data->drm_fd, &data->fb_white);
>  }
>  
> -static void setup_test_plane(data_t *data)
> +static void setup_test_plane(data_t *data, int test_plane)
>  {
>  	uint32_t white_h, white_v;
> +	igt_plane_t *primary, *sprite, *cursor;
>  
>  	igt_create_color_fb(data->drm_fd,
>  			    data->mode->hdisplay, data->mode->vdisplay,
> @@ -381,8 +376,10 @@ static void setup_test_plane(data_t *data)
>  			    0.0, 1.0, 0.0,
>  			    &data->fb_green);
>  
> -	data->primary = igt_output_get_plane_type(data->output, DRM_PLANE_TYPE_PRIMARY);
> -	igt_plane_set_fb(data->primary, NULL);
> +	primary = igt_output_get_plane_type(data->output,
> +					    DRM_PLANE_TYPE_PRIMARY);
> +	igt_plane_set_fb(primary, NULL);
> +	data->test_plane = primary;
>  
>  	white_h = data->mode->hdisplay;
>  	white_v = data->mode->vdisplay;
> @@ -391,16 +388,14 @@ static void setup_test_plane(data_t *data)
>  	data->mod_size = white_h * white_v;
>  	data->mod_stride = white_h * 4;
>  
> -	switch (data->test_plane) {
> +	switch (test_plane) {
>  	case DRM_PLANE_TYPE_OVERLAY:
> -		data->sprite = igt_output_get_plane_type(data->output,
> -						    DRM_PLANE_TYPE_OVERLAY);
> -		igt_plane_set_fb(data->sprite, NULL);
> -		/* To make it different for human eyes let's make
> -		 * sprite visible in only one quarter of the primary
> -		 */
> +		sprite = igt_output_get_plane_type(data->output,
> +						   DRM_PLANE_TYPE_OVERLAY);
> +		igt_plane_set_fb(sprite, NULL);
>  		white_h = white_h/2;
>  		white_v = white_v/2;
> +		data->test_plane = sprite;
>  	case DRM_PLANE_TYPE_PRIMARY:
>  		igt_create_color_fb(data->drm_fd,
>  				    white_h, white_v,
> @@ -410,20 +405,21 @@ static void setup_test_plane(data_t *data)
>  				    &data->fb_white);
>  		break;
>  	case DRM_PLANE_TYPE_CURSOR:
> -		data->cursor = igt_output_get_plane_type(data->output,
> -						    DRM_PLANE_TYPE_CURSOR);
> -		igt_plane_set_fb(data->cursor, NULL);
> +		cursor = igt_output_get_plane_type(data->output,
> +						   DRM_PLANE_TYPE_CURSOR);
> +		igt_plane_set_fb(cursor, NULL);
>  		create_cursor_fb(data);
> -		igt_plane_set_position(data->cursor, 0, 0);
> +		igt_plane_set_position(cursor, 0, 0);
>  
>  		/* Cursor is 64 x 64, ignoring pitch and bbp again */
>  		data->mod_size = 64 * 64;
> +		data->test_plane = cursor;
>  		break;
>  	}
>  
>  	igt_display_commit(&data->display);
>  
> -	igt_plane_set_fb(data->primary, &data->fb_green);
> +	igt_plane_set_fb(primary, &data->fb_green);
>  	igt_display_commit(&data->display);
>  }
>  
> @@ -489,13 +485,13 @@ int main(int argc, char *argv[])
>  	}
>  
>  	igt_subtest("basic") {
> -		setup_test_plane(&data);
> +		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
>  		igt_assert(wait_psr_entry(&data));
>  		test_cleanup(&data);
>  	}
>  
>  	igt_subtest("no_drrs") {
> -		setup_test_plane(&data);
> +		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
>  		igt_assert(wait_psr_entry(&data));
>  		igt_assert(drrs_disabled(&data));
>  		test_cleanup(&data);
> @@ -503,9 +499,8 @@ int main(int argc, char *argv[])
>  
>  	for (op = PAGE_FLIP; op <= RENDER; op++) {
>  		igt_subtest_f("primary_%s", op_str(op)) {
> -			data.test_plane = DRM_PLANE_TYPE_PRIMARY;
>  			data.op = op;
> -			setup_test_plane(&data);
> +			setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
>  			igt_assert(wait_psr_entry(&data));
>  			run_test(&data);
>  			test_cleanup(&data);
> @@ -514,9 +509,8 @@ int main(int argc, char *argv[])
>  
>  	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
>  		igt_subtest_f("sprite_%s", op_str(op)) {
> -			data.test_plane = DRM_PLANE_TYPE_OVERLAY;
>  			data.op = op;
> -			setup_test_plane(&data);
> +			setup_test_plane(&data, DRM_PLANE_TYPE_OVERLAY);
>  			igt_assert(wait_psr_entry(&data));
>  			run_test(&data);
>  			test_cleanup(&data);
> @@ -525,9 +519,8 @@ int main(int argc, char *argv[])
>  
>  	for (op = MMAP_GTT; op <= PLANE_ONOFF; op++) {
>  		igt_subtest_f("cursor_%s", op_str(op)) {
> -			data.test_plane = DRM_PLANE_TYPE_CURSOR;
>  			data.op = op;
> -			setup_test_plane(&data);
> +			setup_test_plane(&data, DRM_PLANE_TYPE_CURSOR);
>  			igt_assert(wait_psr_entry(&data));
>  			run_test(&data);
>  			test_cleanup(&data);
> @@ -535,9 +528,8 @@ int main(int argc, char *argv[])
>  	}
>  
>  	igt_subtest_f("dpms") {
> -		data.test_plane = DRM_PLANE_TYPE_PRIMARY;
>  		data.op = RENDER;
> -		setup_test_plane(&data);
> +		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
>  		igt_assert(wait_psr_entry(&data));
>  		dpms_off_on(data);
>  		run_test(&data);
> @@ -545,9 +537,8 @@ int main(int argc, char *argv[])
>  	}
>  
>  	igt_subtest_f("suspend") {
> -		data.test_plane = DRM_PLANE_TYPE_CURSOR;
>  		data.op = PLANE_ONOFF;
> -		setup_test_plane(&data);
> +		setup_test_plane(&data, DRM_PLANE_TYPE_CURSOR);
>  		igt_assert(wait_psr_entry(&data));
>  		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
>  					      SUSPEND_TEST_NONE);
> -- 
> 2.14.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 16/20] tests/psr: Pass data_t pointer to dpms_off_on

[Dhinakaran Pandiyan]

Consistent with other function signatures in the file.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 tests/kms_psr_sink_crc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index f22edb48..3e06e14e 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -423,11 +423,11 @@ static void setup_test_plane(data_t *data, int test_plane)
 	igt_display_commit(&data->display);
 }
 
-static void dpms_off_on(data_t data)
+static void dpms_off_on(data_t *data)
 {
-	kmstest_set_connector_dpms(data.drm_fd, data.output->config.connector,
+	kmstest_set_connector_dpms(data->drm_fd, data->output->config.connector,
 				   DRM_MODE_DPMS_OFF);
-	kmstest_set_connector_dpms(data.drm_fd, data.output->config.connector,
+	kmstest_set_connector_dpms(data->drm_fd, data->output->config.connector,
 				   DRM_MODE_DPMS_ON);
 }
 
@@ -531,7 +531,7 @@ int main(int argc, char *argv[])
 		data.op = RENDER;
 		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
 		igt_assert(wait_psr_entry(&data));
-		dpms_off_on(data);
+		dpms_off_on(&data);
 		run_test(&data);
 		test_cleanup(&data);
 	}
-- 
2.14.1

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

Re: [igt-dev] [PATCH i-g-t 16/20] tests/psr: Pass data_t pointer to dpms_off_on

[Katarzyna Dec]

On Tue, Apr 10, 2018 at 07:37:28PM -0700, Dhinakaran Pandiyan wrote:
> Consistent with other function signatures in the file.
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
 Reviewed-by: Katarzyna Dec <katarzyna.dec@intel.com>
> ---
>  tests/kms_psr_sink_crc.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index f22edb48..3e06e14e 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -423,11 +423,11 @@ static void setup_test_plane(data_t *data, int test_plane)
>  	igt_display_commit(&data->display);
>  }
>  
> -static void dpms_off_on(data_t data)
> +static void dpms_off_on(data_t *data)
>  {
> -	kmstest_set_connector_dpms(data.drm_fd, data.output->config.connector,
> +	kmstest_set_connector_dpms(data->drm_fd, data->output->config.connector,
>  				   DRM_MODE_DPMS_OFF);
> -	kmstest_set_connector_dpms(data.drm_fd, data.output->config.connector,
> +	kmstest_set_connector_dpms(data->drm_fd, data->output->config.connector,
>  				   DRM_MODE_DPMS_ON);
>  }
>  
> @@ -531,7 +531,7 @@ int main(int argc, char *argv[])
>  		data.op = RENDER;
>  		setup_test_plane(&data, DRM_PLANE_TYPE_PRIMARY);
>  		igt_assert(wait_psr_entry(&data));
> -		dpms_off_on(data);
> +		dpms_off_on(&data);
>  		run_test(&data);
>  		test_cleanup(&data);
>  	}
> -- 
> 2.14.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 17/20] tests/psr: Prepare for using timestamps.

[Dhinakaran Pandiyan]

Wrap some functions, move condition checks etc to make it easy to add
timestamp support.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 tests/kms_psr_sink_crc.c | 86 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 55 insertions(+), 31 deletions(-)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 3e06e14e..043b9e38 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -223,11 +223,8 @@ static bool wait_psr_entry(data_t *data)
 	return false;
 }
 
-static void get_sink_crc(data_t *data, char *crc)
+static void __get_sink_crc(data_t *data, char *crc)
 {
-	if (igt_interactive_debug)
-		return;
-
 	igt_require_f(igt_sysfs_read(data->debugfs_fd, "i915_sink_crc_eDP1",
 				     crc, CRC_LEN) == CRC_LEN,
 		      "Sink CRC is unreliable on this machine. Try manual debug with --interactive-debug=no-crc\n");
@@ -237,22 +234,53 @@ static void get_sink_crc(data_t *data, char *crc)
 	igt_assert(strncmp(crc, CRC_BLACK, CRC_LEN));
 }
 
-static bool is_green(char *crc)
+static bool __is_green(data_t *data, char *out_crc)
 {
 	const char *mask = "0000FFFF0000";
-	uint32_t *p = (uint32_t *)crc, *mask_p = (uint32_t *)mask;
-	if (igt_interactive_debug)
-		return false;
+	uint32_t *p, *mask_p = (uint32_t *)mask;
+	char _crc[CRC_LEN], *crc;
+
+	crc = out_crc ? out_crc : _crc;
+	__get_sink_crc(data, crc);
+	p = (uint32_t *)crc;
 
 	/* Check R and B components are 0 and G is non-zero */
-	return *p == *mask_p && *(p + 2) == *(mask_p + 2) &&
-	       (*(p + 1) & *(mask_p + 1)) != 0;
+	return *p == *mask_p &&
+	       *(p + 2) == *(mask_p + 2) &&
+	       *(p + 1) & *(mask_p + 1);
 }
 
-static void assert_or_manual(bool condition, const char *expected)
+static void is_green_crc(data_t *data, char *out_crc)
 {
-	igt_debug_manual_check("no-crc", expected);
-	igt_assert(igt_interactive_debug || condition);
+	if (!data->with_sink_crc)
+		return;
+
+	igt_assert(__is_green(data, out_crc));
+}
+
+static void is_not_green_crc(data_t *data, char *out_crc)
+{
+	if (!data->with_sink_crc)
+		return;
+
+	igt_fail_on(__is_green(data, out_crc));
+}
+
+static void is_not_equal_crc(data_t *data, const char *ref_crc)
+{
+	char crc[CRC_LEN];
+
+	if (!data->with_sink_crc)
+		return;
+
+	__get_sink_crc(data, crc);
+	igt_assert_f(strncmp(ref_crc, crc, CRC_LEN), "screen update failed\n");
+}
+
+static void manual(const char *expected)
+{
+	if (igt_interactive_debug)
+		igt_debug_manual_check("no-crc", expected);
 }
 
 static bool drrs_disabled(data_t *data)
@@ -270,21 +298,20 @@ static void run_test(data_t *data)
 	igt_plane_t *test_plane = data->test_plane;
 	void *ptr;
 	char ref_crc[CRC_LEN];
-	char crc[CRC_LEN];
 	const char *expected = "";
 
-	if (!igt_interactive_debug)
-		igt_require_f(data->with_sink_crc,
-			      "Enable sink CRC with --sink-crc\n");
+	igt_require_f(igt_interactive_debug || data->with_sink_crc,
+		      "Enable interactive debug with --interactive-debug or "
+		      "enable sink crc with --sink-crc\n");
 
 	/* Confirm that screen became Green */
-	get_sink_crc(data, ref_crc);
-	assert_or_manual(is_green(ref_crc), "screen GREEN");
+	manual("screen GREEN");
+	is_green_crc(data, NULL);
 
 	/* Confirm screen stays Green after PSR got active */
 	igt_assert(wait_psr_entry(data));
-	get_sink_crc(data, ref_crc);
-	assert_or_manual(is_green(ref_crc), "screen GREEN");
+	manual("screen GREEN");
+	is_green_crc(data, NULL);
 
 	/* Setting a secondary fb/plane */
 	igt_plane_set_fb(test_plane, &data->fb_white);
@@ -292,20 +319,17 @@ static void run_test(data_t *data)
 
 	/* Confirm it is not Green anymore */
 	igt_assert(wait_psr_entry(data));
-	get_sink_crc(data, ref_crc);
-	if (test_plane->type == DRM_PLANE_TYPE_PRIMARY)
-		assert_or_manual(!is_green(ref_crc), "screen WHITE");
-	else
-		assert_or_manual(!is_green(ref_crc), "GREEN background with WHITE box");
+	manual(test_plane->type == DRM_PLANE_TYPE_PRIMARY ?
+	       "screen WHITE" : "WHITE box on GREEN");
+	is_not_green_crc(data, ref_crc);
 
 	switch (data->op) {
 	case PAGE_FLIP:
 		/* Only in use when testing primary plane */
 		igt_assert(drmModePageFlip(data->drm_fd, data->crtc_id,
 					   data->fb_green.fb_id, 0, NULL) == 0);
-		get_sink_crc(data, crc);
-		assert_or_manual(is_green(crc), "screen GREEN");
-		expected = "still GREEN";
+		is_green_crc(data, NULL);
+		expected = "screen GREEN";
 		break;
 	case MMAP_GTT:
 		ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
@@ -347,8 +371,8 @@ static void run_test(data_t *data)
 		expected = "screen GREEN";
 		break;
 	}
-	get_sink_crc(data, crc);
-	assert_or_manual(strncmp(ref_crc, crc, CRC_LEN) != 0, expected);
+	manual(expected);
+	is_not_equal_crc(data, ref_crc);
 }
 
 static void test_cleanup(data_t *data) {
-- 
2.14.1

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

Re: [igt-dev] [PATCH i-g-t 17/20] tests/psr: Prepare for using timestamps.

[Katarzyna Dec]

On Tue, Apr 10, 2018 at 07:37:29PM -0700, Dhinakaran Pandiyan wrote:
> Wrap some functions, move condition checks etc to make it easy to add
> timestamp support.

Could you please add more details here? What and why has to be changed?

> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  tests/kms_psr_sink_crc.c | 86 +++++++++++++++++++++++++++++++-----------------
>  1 file changed, 55 insertions(+), 31 deletions(-)
> 
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index 3e06e14e..043b9e38 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -223,11 +223,8 @@ static bool wait_psr_entry(data_t *data)
>  	return false;
>  }
>  
> -static void get_sink_crc(data_t *data, char *crc)
> +static void __get_sink_crc(data_t *data, char *crc)
>  {
> -	if (igt_interactive_debug)
> -		return;
> -
>  	igt_require_f(igt_sysfs_read(data->debugfs_fd, "i915_sink_crc_eDP1",
>  				     crc, CRC_LEN) == CRC_LEN,
>  		      "Sink CRC is unreliable on this machine. Try manual debug with --interactive-debug=no-crc\n");
> @@ -237,22 +234,53 @@ static void get_sink_crc(data_t *data, char *crc)
>  	igt_assert(strncmp(crc, CRC_BLACK, CRC_LEN));
>  }
During reviewing this patch I started to wonder what get_sinc_crc
does indeed. Could you explain it to me, please? Or point some docs
with information?

Kasia
>  
> -static bool is_green(char *crc)
> +static bool __is_green(data_t *data, char *out_crc)
>  {
>  	const char *mask = "0000FFFF0000";
> -	uint32_t *p = (uint32_t *)crc, *mask_p = (uint32_t *)mask;
> -	if (igt_interactive_debug)
> -		return false;
> +	uint32_t *p, *mask_p = (uint32_t *)mask;
> +	char _crc[CRC_LEN], *crc;
> +
> +	crc = out_crc ? out_crc : _crc;
> +	__get_sink_crc(data, crc);
> +	p = (uint32_t *)crc;
>  
>  	/* Check R and B components are 0 and G is non-zero */
> -	return *p == *mask_p && *(p + 2) == *(mask_p + 2) &&
> -	       (*(p + 1) & *(mask_p + 1)) != 0;
> +	return *p == *mask_p &&
> +	       *(p + 2) == *(mask_p + 2) &&
> +	       *(p + 1) & *(mask_p + 1);
>  }
>  
> -static void assert_or_manual(bool condition, const char *expected)
> +static void is_green_crc(data_t *data, char *out_crc)
>  {
> -	igt_debug_manual_check("no-crc", expected);
> -	igt_assert(igt_interactive_debug || condition);
> +	if (!data->with_sink_crc)
> +		return;
> +
> +	igt_assert(__is_green(data, out_crc));
> +}
> +
> +static void is_not_green_crc(data_t *data, char *out_crc)
> +{
> +	if (!data->with_sink_crc)
> +		return;
> +
> +	igt_fail_on(__is_green(data, out_crc));
> +}
> +
> +static void is_not_equal_crc(data_t *data, const char *ref_crc)
> +{
> +	char crc[CRC_LEN];
> +
> +	if (!data->with_sink_crc)
> +		return;
> +
> +	__get_sink_crc(data, crc);
> +	igt_assert_f(strncmp(ref_crc, crc, CRC_LEN), "screen update failed\n");
> +}
> +
> +static void manual(const char *expected)
> +{
> +	if (igt_interactive_debug)
> +		igt_debug_manual_check("no-crc", expected);
>  }
>  
>  static bool drrs_disabled(data_t *data)
> @@ -270,21 +298,20 @@ static void run_test(data_t *data)
>  	igt_plane_t *test_plane = data->test_plane;
>  	void *ptr;
>  	char ref_crc[CRC_LEN];
> -	char crc[CRC_LEN];
>  	const char *expected = "";
>  
> -	if (!igt_interactive_debug)
> -		igt_require_f(data->with_sink_crc,
> -			      "Enable sink CRC with --sink-crc\n");
> +	igt_require_f(igt_interactive_debug || data->with_sink_crc,
> +		      "Enable interactive debug with --interactive-debug or "
> +		      "enable sink crc with --sink-crc\n");
>  
>  	/* Confirm that screen became Green */
> -	get_sink_crc(data, ref_crc);
> -	assert_or_manual(is_green(ref_crc), "screen GREEN");
> +	manual("screen GREEN");
> +	is_green_crc(data, NULL);
>  
>  	/* Confirm screen stays Green after PSR got active */
>  	igt_assert(wait_psr_entry(data));
> -	get_sink_crc(data, ref_crc);
> -	assert_or_manual(is_green(ref_crc), "screen GREEN");
> +	manual("screen GREEN");
> +	is_green_crc(data, NULL);
>  
>  	/* Setting a secondary fb/plane */
>  	igt_plane_set_fb(test_plane, &data->fb_white);
> @@ -292,20 +319,17 @@ static void run_test(data_t *data)
>  
>  	/* Confirm it is not Green anymore */
>  	igt_assert(wait_psr_entry(data));
> -	get_sink_crc(data, ref_crc);
> -	if (test_plane->type == DRM_PLANE_TYPE_PRIMARY)
> -		assert_or_manual(!is_green(ref_crc), "screen WHITE");
> -	else
> -		assert_or_manual(!is_green(ref_crc), "GREEN background with WHITE box");
> +	manual(test_plane->type == DRM_PLANE_TYPE_PRIMARY ?
> +	       "screen WHITE" : "WHITE box on GREEN");
> +	is_not_green_crc(data, ref_crc);
>  
>  	switch (data->op) {
>  	case PAGE_FLIP:
>  		/* Only in use when testing primary plane */
>  		igt_assert(drmModePageFlip(data->drm_fd, data->crtc_id,
>  					   data->fb_green.fb_id, 0, NULL) == 0);
> -		get_sink_crc(data, crc);
> -		assert_or_manual(is_green(crc), "screen GREEN");
> -		expected = "still GREEN";
> +		is_green_crc(data, NULL);
> +		expected = "screen GREEN";
>  		break;
>  	case MMAP_GTT:
>  		ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
> @@ -347,8 +371,8 @@ static void run_test(data_t *data)
>  		expected = "screen GREEN";
>  		break;
>  	}
> -	get_sink_crc(data, crc);
> -	assert_or_manual(strncmp(ref_crc, crc, CRC_LEN) != 0, expected);
> +	manual(expected);
> +	is_not_equal_crc(data, ref_crc);
>  }
>  
>  static void test_cleanup(data_t *data) {
> -- 
> 2.14.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 17/20] tests/psr: Prepare for using timestamps.

[Dhinakaran Pandiyan]

On Thu, 2018-04-12 at 14:50 +0200, Katarzyna Dec wrote:
> On Tue, Apr 10, 2018 at 07:37:29PM -0700, Dhinakaran Pandiyan wrote:
> > Wrap some functions, move condition checks etc to make it easy to add
> > timestamp support.
> 
> Could you please add more details here? What and why has to be changed?
> 
> > 
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  tests/kms_psr_sink_crc.c | 86 +++++++++++++++++++++++++++++++-----------------
> >  1 file changed, 55 insertions(+), 31 deletions(-)
> > 
> > diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> > index 3e06e14e..043b9e38 100644
> > --- a/tests/kms_psr_sink_crc.c
> > +++ b/tests/kms_psr_sink_crc.c
> > @@ -223,11 +223,8 @@ static bool wait_psr_entry(data_t *data)
> >  	return false;
> >  }
> >  
> > -static void get_sink_crc(data_t *data, char *crc)
> > +static void __get_sink_crc(data_t *data, char *crc)
> >  {
> > -	if (igt_interactive_debug)
> > -		return;
> > -
> >  	igt_require_f(igt_sysfs_read(data->debugfs_fd, "i915_sink_crc_eDP1",
> >  				     crc, CRC_LEN) == CRC_LEN,
> >  		      "Sink CRC is unreliable on this machine. Try manual debug with --interactive-debug=no-crc\n");
> > @@ -237,22 +234,53 @@ static void get_sink_crc(data_t *data, char *crc)
> >  	igt_assert(strncmp(crc, CRC_BLACK, CRC_LEN));
> >  }
> During reviewing this patch I started to wonder what get_sinc_crc
> does indeed. Could you explain it to me, please? Or point some docs
> with information?
> 

The DP sink (eDP panel) calculates CRC for the frame it receives;
get_sink_crc() uses a debugfs interface to get this value. The idea is
to check if the frame that the sink is displaying is what we expect. PSR
allows the sink to self-refresh from it's own buffer when screen
contents have not changed. The tests are written to make sure when the
source wants to update the frame, the sink switches to using the frame
from the source and not self-refresh from it's own buffer.


> Kasia
> >  
> > -static bool is_green(char *crc)
> > +static bool __is_green(data_t *data, char *out_crc)
> >  {
> >  	const char *mask = "0000FFFF0000";
> > -	uint32_t *p = (uint32_t *)crc, *mask_p = (uint32_t *)mask;
> > -	if (igt_interactive_debug)
> > -		return false;
> > +	uint32_t *p, *mask_p = (uint32_t *)mask;
> > +	char _crc[CRC_LEN], *crc;
> > +
> > +	crc = out_crc ? out_crc : _crc;
> > +	__get_sink_crc(data, crc);
> > +	p = (uint32_t *)crc;
> >  
> >  	/* Check R and B components are 0 and G is non-zero */
> > -	return *p == *mask_p && *(p + 2) == *(mask_p + 2) &&
> > -	       (*(p + 1) & *(mask_p + 1)) != 0;
> > +	return *p == *mask_p &&
> > +	       *(p + 2) == *(mask_p + 2) &&
> > +	       *(p + 1) & *(mask_p + 1);
> >  }
> >  
> > -static void assert_or_manual(bool condition, const char *expected)
> > +static void is_green_crc(data_t *data, char *out_crc)
> >  {
> > -	igt_debug_manual_check("no-crc", expected);
> > -	igt_assert(igt_interactive_debug || condition);
> > +	if (!data->with_sink_crc)
> > +		return;
> > +
> > +	igt_assert(__is_green(data, out_crc));
> > +}
> > +
> > +static void is_not_green_crc(data_t *data, char *out_crc)
> > +{
> > +	if (!data->with_sink_crc)
> > +		return;
> > +
> > +	igt_fail_on(__is_green(data, out_crc));
> > +}
> > +
> > +static void is_not_equal_crc(data_t *data, const char *ref_crc)
> > +{
> > +	char crc[CRC_LEN];
> > +
> > +	if (!data->with_sink_crc)
> > +		return;
> > +
> > +	__get_sink_crc(data, crc);
> > +	igt_assert_f(strncmp(ref_crc, crc, CRC_LEN), "screen update failed\n");
> > +}
> > +
> > +static void manual(const char *expected)
> > +{
> > +	if (igt_interactive_debug)
> > +		igt_debug_manual_check("no-crc", expected);
> >  }
> >  
> >  static bool drrs_disabled(data_t *data)
> > @@ -270,21 +298,20 @@ static void run_test(data_t *data)
> >  	igt_plane_t *test_plane = data->test_plane;
> >  	void *ptr;
> >  	char ref_crc[CRC_LEN];
> > -	char crc[CRC_LEN];
> >  	const char *expected = "";
> >  
> > -	if (!igt_interactive_debug)
> > -		igt_require_f(data->with_sink_crc,
> > -			      "Enable sink CRC with --sink-crc\n");
> > +	igt_require_f(igt_interactive_debug || data->with_sink_crc,
> > +		      "Enable interactive debug with --interactive-debug or "
> > +		      "enable sink crc with --sink-crc\n");
> >  
> >  	/* Confirm that screen became Green */
> > -	get_sink_crc(data, ref_crc);
> > -	assert_or_manual(is_green(ref_crc), "screen GREEN");
> > +	manual("screen GREEN");
> > +	is_green_crc(data, NULL);
> >  
> >  	/* Confirm screen stays Green after PSR got active */
> >  	igt_assert(wait_psr_entry(data));
> > -	get_sink_crc(data, ref_crc);
> > -	assert_or_manual(is_green(ref_crc), "screen GREEN");
> > +	manual("screen GREEN");
> > +	is_green_crc(data, NULL);
> >  
> >  	/* Setting a secondary fb/plane */
> >  	igt_plane_set_fb(test_plane, &data->fb_white);
> > @@ -292,20 +319,17 @@ static void run_test(data_t *data)
> >  
> >  	/* Confirm it is not Green anymore */
> >  	igt_assert(wait_psr_entry(data));
> > -	get_sink_crc(data, ref_crc);
> > -	if (test_plane->type == DRM_PLANE_TYPE_PRIMARY)
> > -		assert_or_manual(!is_green(ref_crc), "screen WHITE");
> > -	else
> > -		assert_or_manual(!is_green(ref_crc), "GREEN background with WHITE box");
> > +	manual(test_plane->type == DRM_PLANE_TYPE_PRIMARY ?
> > +	       "screen WHITE" : "WHITE box on GREEN");
> > +	is_not_green_crc(data, ref_crc);
> >  
> >  	switch (data->op) {
> >  	case PAGE_FLIP:
> >  		/* Only in use when testing primary plane */
> >  		igt_assert(drmModePageFlip(data->drm_fd, data->crtc_id,
> >  					   data->fb_green.fb_id, 0, NULL) == 0);
> > -		get_sink_crc(data, crc);
> > -		assert_or_manual(is_green(crc), "screen GREEN");
> > -		expected = "still GREEN";
> > +		is_green_crc(data, NULL);
> > +		expected = "screen GREEN";
> >  		break;
> >  	case MMAP_GTT:
> >  		ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
> > @@ -347,8 +371,8 @@ static void run_test(data_t *data)
> >  		expected = "screen GREEN";
> >  		break;
> >  	}
> > -	get_sink_crc(data, crc);
> > -	assert_or_manual(strncmp(ref_crc, crc, CRC_LEN) != 0, expected);
> > +	manual(expected);
> > +	is_not_equal_crc(data, ref_crc);
> >  }
> >  
> >  static void test_cleanup(data_t *data) {
> > -- 
> > 2.14.1
> > 
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

Re: [igt-dev] [PATCH i-g-t 17/20] tests/psr: Prepare for using timestamps.

[Katarzyna Dec]

On Thu, Apr 12, 2018 at 05:21:58PM -0700, Dhinakaran Pandiyan wrote:
> 
> 
> 
> On Thu, 2018-04-12 at 14:50 +0200, Katarzyna Dec wrote:
> > On Tue, Apr 10, 2018 at 07:37:29PM -0700, Dhinakaran Pandiyan wrote:
> > > Wrap some functions, move condition checks etc to make it easy to add
> > > timestamp support.
> > 
> > Could you please add more details here? What and why has to be changed?
> > 
> > > 
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > ---
> > >  tests/kms_psr_sink_crc.c | 86 +++++++++++++++++++++++++++++++-----------------
> > >  1 file changed, 55 insertions(+), 31 deletions(-)
> > > 
> > > diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> > > index 3e06e14e..043b9e38 100644
> > > --- a/tests/kms_psr_sink_crc.c
> > > +++ b/tests/kms_psr_sink_crc.c
> > > @@ -223,11 +223,8 @@ static bool wait_psr_entry(data_t *data)
> > >  	return false;
> > >  }
> > >  
> > > -static void get_sink_crc(data_t *data, char *crc)
> > > +static void __get_sink_crc(data_t *data, char *crc)
> > >  {
> > > -	if (igt_interactive_debug)
> > > -		return;
> > > -
> > >  	igt_require_f(igt_sysfs_read(data->debugfs_fd, "i915_sink_crc_eDP1",
> > >  				     crc, CRC_LEN) == CRC_LEN,
> > >  		      "Sink CRC is unreliable on this machine. Try manual debug with --interactive-debug=no-crc\n");
> > > @@ -237,22 +234,53 @@ static void get_sink_crc(data_t *data, char *crc)
> > >  	igt_assert(strncmp(crc, CRC_BLACK, CRC_LEN));
> > >  }
> > During reviewing this patch I started to wonder what get_sinc_crc
> > does indeed. Could you explain it to me, please? Or point some docs
> > with information?
> > 
> 
> The DP sink (eDP panel) calculates CRC for the frame it receives;
> get_sink_crc() uses a debugfs interface to get this value. The idea is
> to check if the frame that the sink is displaying is what we expect. PSR
> allows the sink to self-refresh from it's own buffer when screen
> contents have not changed. The tests are written to make sure when the
> source wants to update the frame, the sink switches to using the frame
> from the source and not self-refresh from it's own buffer.
> 
That was the answer I was looking for :) Thanks
Reviewed-by: Katarzyna Dec <katarzyna.dec@intel.com>
> 
> > Kasia
> > >  
> > > -static bool is_green(char *crc)
> > > +static bool __is_green(data_t *data, char *out_crc)
> > >  {
> > >  	const char *mask = "0000FFFF0000";
> > > -	uint32_t *p = (uint32_t *)crc, *mask_p = (uint32_t *)mask;
> > > -	if (igt_interactive_debug)
> > > -		return false;
> > > +	uint32_t *p, *mask_p = (uint32_t *)mask;
> > > +	char _crc[CRC_LEN], *crc;
> > > +
> > > +	crc = out_crc ? out_crc : _crc;
> > > +	__get_sink_crc(data, crc);
> > > +	p = (uint32_t *)crc;
> > >  
> > >  	/* Check R and B components are 0 and G is non-zero */
> > > -	return *p == *mask_p && *(p + 2) == *(mask_p + 2) &&
> > > -	       (*(p + 1) & *(mask_p + 1)) != 0;
> > > +	return *p == *mask_p &&
> > > +	       *(p + 2) == *(mask_p + 2) &&
> > > +	       *(p + 1) & *(mask_p + 1);
> > >  }
> > >  
> > > -static void assert_or_manual(bool condition, const char *expected)
> > > +static void is_green_crc(data_t *data, char *out_crc)
> > >  {
> > > -	igt_debug_manual_check("no-crc", expected);
> > > -	igt_assert(igt_interactive_debug || condition);
> > > +	if (!data->with_sink_crc)
> > > +		return;
> > > +
> > > +	igt_assert(__is_green(data, out_crc));
> > > +}
> > > +
> > > +static void is_not_green_crc(data_t *data, char *out_crc)
> > > +{
> > > +	if (!data->with_sink_crc)
> > > +		return;
> > > +
> > > +	igt_fail_on(__is_green(data, out_crc));
> > > +}
> > > +
> > > +static void is_not_equal_crc(data_t *data, const char *ref_crc)
> > > +{
> > > +	char crc[CRC_LEN];
> > > +
> > > +	if (!data->with_sink_crc)
> > > +		return;
> > > +
> > > +	__get_sink_crc(data, crc);
> > > +	igt_assert_f(strncmp(ref_crc, crc, CRC_LEN), "screen update failed\n");
> > > +}
> > > +
> > > +static void manual(const char *expected)
> > > +{
> > > +	if (igt_interactive_debug)
> > > +		igt_debug_manual_check("no-crc", expected);
> > >  }
> > >  
> > >  static bool drrs_disabled(data_t *data)
> > > @@ -270,21 +298,20 @@ static void run_test(data_t *data)
> > >  	igt_plane_t *test_plane = data->test_plane;
> > >  	void *ptr;
> > >  	char ref_crc[CRC_LEN];
> > > -	char crc[CRC_LEN];
> > >  	const char *expected = "";
> > >  
> > > -	if (!igt_interactive_debug)
> > > -		igt_require_f(data->with_sink_crc,
> > > -			      "Enable sink CRC with --sink-crc\n");
> > > +	igt_require_f(igt_interactive_debug || data->with_sink_crc,
> > > +		      "Enable interactive debug with --interactive-debug or "
> > > +		      "enable sink crc with --sink-crc\n");
> > >  
> > >  	/* Confirm that screen became Green */
> > > -	get_sink_crc(data, ref_crc);
> > > -	assert_or_manual(is_green(ref_crc), "screen GREEN");
> > > +	manual("screen GREEN");
> > > +	is_green_crc(data, NULL);
> > >  
> > >  	/* Confirm screen stays Green after PSR got active */
> > >  	igt_assert(wait_psr_entry(data));
> > > -	get_sink_crc(data, ref_crc);
> > > -	assert_or_manual(is_green(ref_crc), "screen GREEN");
> > > +	manual("screen GREEN");
> > > +	is_green_crc(data, NULL);
> > >  
> > >  	/* Setting a secondary fb/plane */
> > >  	igt_plane_set_fb(test_plane, &data->fb_white);
> > > @@ -292,20 +319,17 @@ static void run_test(data_t *data)
> > >  
> > >  	/* Confirm it is not Green anymore */
> > >  	igt_assert(wait_psr_entry(data));
> > > -	get_sink_crc(data, ref_crc);
> > > -	if (test_plane->type == DRM_PLANE_TYPE_PRIMARY)
> > > -		assert_or_manual(!is_green(ref_crc), "screen WHITE");
> > > -	else
> > > -		assert_or_manual(!is_green(ref_crc), "GREEN background with WHITE box");
> > > +	manual(test_plane->type == DRM_PLANE_TYPE_PRIMARY ?
> > > +	       "screen WHITE" : "WHITE box on GREEN");
> > > +	is_not_green_crc(data, ref_crc);
> > >  
> > >  	switch (data->op) {
> > >  	case PAGE_FLIP:
> > >  		/* Only in use when testing primary plane */
> > >  		igt_assert(drmModePageFlip(data->drm_fd, data->crtc_id,
> > >  					   data->fb_green.fb_id, 0, NULL) == 0);
> > > -		get_sink_crc(data, crc);
> > > -		assert_or_manual(is_green(crc), "screen GREEN");
> > > -		expected = "still GREEN";
> > > +		is_green_crc(data, NULL);
> > > +		expected = "screen GREEN";
> > >  		break;
> > >  	case MMAP_GTT:
> > >  		ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
> > > @@ -347,8 +371,8 @@ static void run_test(data_t *data)
> > >  		expected = "screen GREEN";
> > >  		break;
> > >  	}
> > > -	get_sink_crc(data, crc);
> > > -	assert_or_manual(strncmp(ref_crc, crc, CRC_LEN) != 0, expected);
> > > +	manual(expected);
> > > +	is_not_equal_crc(data, ref_crc);
> > >  }
> > >  
> > >  static void test_cleanup(data_t *data) {
> > > -- 
> > > 2.14.1
> > > 
> > > _______________________________________________
> > > igt-dev mailing list
> > > igt-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 18/20] tests/psr: Check for timestamp support

[Dhinakaran Pandiyan]

BDW+ have PSR interrupts from which the kernel can update exit and
pre-entry time stamps. Tests should prefer this over sink crc to
validate PSR.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 tests/kms_psr_sink_crc.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 043b9e38..147b492f 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -75,6 +75,7 @@ typedef struct {
 	igt_output_t *output;
 	bool with_psr_disabled;
 	bool with_sink_crc;
+	bool with_timestamps;
 } data_t;
 
 static void create_cursor_fb(data_t *data)
@@ -200,6 +201,17 @@ static bool sink_support(data_t *data)
 		strstr(buf, "Sink_Support: yes\n");
 }
 
+static void timestamp_support(data_t *data)
+{
+	char buf[512];
+
+	igt_sysfs_set(data->debugfs_fd, "i915_edp_psr_debug", "1");
+	igt_debugfs_read(data->drm_fd, "i915_edp_psr_status", buf);
+	data->with_timestamps = strstr(buf, "Last exit at:");
+	igt_debug("Time stamp support %d\n", data->with_timestamps);
+	igt_sysfs_set(data->debugfs_fd, "i915_edp_psr_debug", "0");
+}
+
 static bool psr_enabled(data_t *data)
 {
 	char buf[512];
@@ -500,6 +512,7 @@ int main(int argc, char *argv[])
 					 0 : 1);
 		igt_require_f(sink_support(&data),
 			      "Sink does not support PSR\n");
+		timestamp_support(&data);
 
 		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
 		igt_assert(data.bufmgr);
-- 
2.14.1

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

Re: [igt-dev] [PATCH i-g-t 18/20] tests/psr: Check for timestamp support

[Katarzyna Dec]

On Tue, Apr 10, 2018 at 07:37:30PM -0700, Dhinakaran Pandiyan wrote:
> BDW+ have PSR interrupts from which the kernel can update exit and
> pre-entry time stamps. Tests should prefer this over sink crc to
> validate PSR.
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  tests/kms_psr_sink_crc.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index 043b9e38..147b492f 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -75,6 +75,7 @@ typedef struct {
>  	igt_output_t *output;
>  	bool with_psr_disabled;
>  	bool with_sink_crc;
> +	bool with_timestamps;
>  } data_t;
>  
>  static void create_cursor_fb(data_t *data)
> @@ -200,6 +201,17 @@ static bool sink_support(data_t *data)
>  		strstr(buf, "Sink_Support: yes\n");
>  }
>  
> +static void timestamp_support(data_t *data)
> +{
> +	char buf[512];
> +
> +	igt_sysfs_set(data->debugfs_fd, "i915_edp_psr_debug", "1");
> +	igt_debugfs_read(data->drm_fd, "i915_edp_psr_status", buf);
> +	data->with_timestamps = strstr(buf, "Last exit at:");
> +	igt_debug("Time stamp support %d\n", data->with_timestamps);
> +	igt_sysfs_set(data->debugfs_fd, "i915_edp_psr_debug", "0");
> +}
> +
Is this information about timestamp in debugs already in the kernel?
Kasia
>  static bool psr_enabled(data_t *data)
>  {
>  	char buf[512];
> @@ -500,6 +512,7 @@ int main(int argc, char *argv[])
>  					 0 : 1);
>  		igt_require_f(sink_support(&data),
>  			      "Sink does not support PSR\n");
> +		timestamp_support(&data);
>  
>  		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
>  		igt_assert(data.bufmgr);
> -- 
> 2.14.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 18/20] tests/psr: Check for timestamp support

[Dhinakaran Pandiyan]

On Thu, 2018-04-12 at 14:52 +0200, Katarzyna Dec wrote:
> On Tue, Apr 10, 2018 at 07:37:30PM -0700, Dhinakaran Pandiyan wrote:
> > BDW+ have PSR interrupts from which the kernel can update exit and
> > pre-entry time stamps. Tests should prefer this over sink crc to
> > validate PSR.
> > 
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  tests/kms_psr_sink_crc.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> > index 043b9e38..147b492f 100644
> > --- a/tests/kms_psr_sink_crc.c
> > +++ b/tests/kms_psr_sink_crc.c
> > @@ -75,6 +75,7 @@ typedef struct {
> >  	igt_output_t *output;
> >  	bool with_psr_disabled;
> >  	bool with_sink_crc;
> > +	bool with_timestamps;
> >  } data_t;
> >  
> >  static void create_cursor_fb(data_t *data)
> > @@ -200,6 +201,17 @@ static bool sink_support(data_t *data)
> >  		strstr(buf, "Sink_Support: yes\n");
> >  }
> >  
> > +static void timestamp_support(data_t *data)
> > +{
> > +	char buf[512];
> > +
> > +	igt_sysfs_set(data->debugfs_fd, "i915_edp_psr_debug", "1");
> > +	igt_debugfs_read(data->drm_fd, "i915_edp_psr_status", buf);
> > +	data->with_timestamps = strstr(buf, "Last exit at:");
> > +	igt_debug("Time stamp support %d\n", data->with_timestamps);
> > +	igt_sysfs_set(data->debugfs_fd, "i915_edp_psr_debug", "0");
> > +}
> > +
> Is this information about timestamp in debugs already in the kernel?

Thanks for the reviews. Not yet, the patches are on the list -
https://patchwork.freedesktop.org/series/41095/

Hoping someone will merge them :)

> Kasia
> >  static bool psr_enabled(data_t *data)
> >  {
> >  	char buf[512];
> > @@ -500,6 +512,7 @@ int main(int argc, char *argv[])
> >  					 0 : 1);
> >  		igt_require_f(sink_support(&data),
> >  			      "Sink does not support PSR\n");
> > +		timestamp_support(&data);
> >  
> >  		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
> >  		igt_assert(data.bufmgr);
> > -- 
> > 2.14.1
> > 
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 18/20] tests/psr: Check for timestamp support

[Dhinakaran Pandiyan]

On Thu, 2018-04-12 at 14:52 +0200, Katarzyna Dec wrote:
> On Tue, Apr 10, 2018 at 07:37:30PM -0700, Dhinakaran Pandiyan wrote:
> > BDW+ have PSR interrupts from which the kernel can update exit and
> > pre-entry time stamps. Tests should prefer this over sink crc to
> > validate PSR.
> > 
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  tests/kms_psr_sink_crc.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> > index 043b9e38..147b492f 100644
> > --- a/tests/kms_psr_sink_crc.c
> > +++ b/tests/kms_psr_sink_crc.c
> > @@ -75,6 +75,7 @@ typedef struct {
> >  	igt_output_t *output;
> >  	bool with_psr_disabled;
> >  	bool with_sink_crc;
> > +	bool with_timestamps;
> >  } data_t;
> >  
> >  static void create_cursor_fb(data_t *data)
> > @@ -200,6 +201,17 @@ static bool sink_support(data_t *data)
> >  		strstr(buf, "Sink_Support: yes\n");
> >  }
> >  
> > +static void timestamp_support(data_t *data)
> > +{
> > +	char buf[512];
> > +
> > +	igt_sysfs_set(data->debugfs_fd, "i915_edp_psr_debug", "1");
> > +	igt_debugfs_read(data->drm_fd, "i915_edp_psr_status", buf);
> > +	data->with_timestamps = strstr(buf, "Last exit at:");
> > +	igt_debug("Time stamp support %d\n", data->with_timestamps);
> > +	igt_sysfs_set(data->debugfs_fd, "i915_edp_psr_debug", "0");
> > +}
> > +
> Is this information about timestamp in debugs already in the kernel?

Thanks for the reviews. Not yet, the patches are on the list -
https://patchwork.freedesktop.org/series/41095/

Hoping someone will merge them :)

> Kasia
> >  static bool psr_enabled(data_t *data)
> >  {
> >  	char buf[512];
> > @@ -500,6 +512,7 @@ int main(int argc, char *argv[])
> >  					 0 : 1);
> >  		igt_require_f(sink_support(&data),
> >  			      "Sink does not support PSR\n");
> > +		timestamp_support(&data);
> >  
> >  		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
> >  		igt_assert(data.bufmgr);
> > -- 
> > 2.14.1
> > 
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

Re: [igt-dev] [PATCH i-g-t 18/20] tests/psr: Check for timestamp support

[Katarzyna Dec]

On Thu, Apr 12, 2018 at 05:13:00PM -0700, Dhinakaran Pandiyan wrote:
> 
> On Thu, 2018-04-12 at 14:52 +0200, Katarzyna Dec wrote:
> > On Tue, Apr 10, 2018 at 07:37:30PM -0700, Dhinakaran Pandiyan wrote:
> > > BDW+ have PSR interrupts from which the kernel can update exit and
> > > pre-entry time stamps. Tests should prefer this over sink crc to
> > > validate PSR.
> > > 
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > ---
> > >  tests/kms_psr_sink_crc.c | 13 +++++++++++++
> > >  1 file changed, 13 insertions(+)
> > > 
> > > diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> > > index 043b9e38..147b492f 100644
> > > --- a/tests/kms_psr_sink_crc.c
> > > +++ b/tests/kms_psr_sink_crc.c
> > > @@ -75,6 +75,7 @@ typedef struct {
> > >  	igt_output_t *output;
> > >  	bool with_psr_disabled;
> > >  	bool with_sink_crc;
> > > +	bool with_timestamps;
> > >  } data_t;
> > >  
> > >  static void create_cursor_fb(data_t *data)
> > > @@ -200,6 +201,17 @@ static bool sink_support(data_t *data)
> > >  		strstr(buf, "Sink_Support: yes\n");
> > >  }
> > >  
> > > +static void timestamp_support(data_t *data)
> > > +{
> > > +	char buf[512];
> > > +
> > > +	igt_sysfs_set(data->debugfs_fd, "i915_edp_psr_debug", "1");
> > > +	igt_debugfs_read(data->drm_fd, "i915_edp_psr_status", buf);
> > > +	data->with_timestamps = strstr(buf, "Last exit at:");
> > > +	igt_debug("Time stamp support %d\n", data->with_timestamps);
> > > +	igt_sysfs_set(data->debugfs_fd, "i915_edp_psr_debug", "0");
> > > +}
> > > +
> > Is this information about timestamp in debugs already in the kernel?
> 
> Thanks for the reviews. Not yet, the patches are on the list -
> https://patchwork.freedesktop.org/series/41095/
> 
> Hoping someone will merge them :)
I will look at them :) (I am not an expert but everyone started somewhere)
Reviewed-by: Katarzyna Dec <katarzyna.dec@intel.com>
> 
> > Kasia
> > >  static bool psr_enabled(data_t *data)
> > >  {
> > >  	char buf[512];
> > > @@ -500,6 +512,7 @@ int main(int argc, char *argv[])
> > >  					 0 : 1);
> > >  		igt_require_f(sink_support(&data),
> > >  			      "Sink does not support PSR\n");
> > > +		timestamp_support(&data);
> > >  
> > >  		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
> > >  		igt_assert(data.bufmgr);
> > > -- 
> > > 2.14.1
> > > 
> > > _______________________________________________
> > > igt-dev mailing list
> > > igt-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 19/20] tests/psr: Test PSR using interrupt timestamps.

[Dhinakaran Pandiyan]

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 tests/kms_psr_sink_crc.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 86 insertions(+), 2 deletions(-)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 147b492f..dd811fb0 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -78,6 +78,11 @@ typedef struct {
 	bool with_timestamps;
 } data_t;
 
+struct timestamps {
+	int64_t last_attempted_entry;
+	int64_t last_exit;
+};
+
 static void create_cursor_fb(data_t *data)
 {
 	cairo_t *cr;
@@ -209,7 +214,9 @@ static void timestamp_support(data_t *data)
 	igt_debugfs_read(data->drm_fd, "i915_edp_psr_status", buf);
 	data->with_timestamps = strstr(buf, "Last exit at:");
 	igt_debug("Time stamp support %d\n", data->with_timestamps);
-	igt_sysfs_set(data->debugfs_fd, "i915_edp_psr_debug", "0");
+
+	if (!data->with_timestamps)
+		igt_sysfs_set(data->debugfs_fd, "i915_edp_psr_debug", "0");
 }
 
 static bool psr_enabled(data_t *data)
@@ -304,6 +311,77 @@ static bool drrs_disabled(data_t *data)
 	return !strstr(buf, "DRRS Supported: Yes\n");
 }
 
+static inline void  __timestamp_read(int fd, int64_t *entry, int64_t *exit)
+{
+	int found;
+	char buf[512];
+	char *begin;
+
+	igt_debugfs_read(fd, "i915_edp_psr_status", buf);
+
+	begin = strstr(buf, "Last attempted entry at: ");
+	igt_assert_f(begin, "Could not read timestamp\n");
+	found = sscanf(begin, "Last attempted entry at: %ld", entry);
+	igt_assert(found == 1);
+
+	begin = strstr(buf, "Last exit at: ");
+	igt_assert_f(begin, "Could not read timestamp\n");
+	found =	sscanf(begin, "Last exit at: %ld\n", exit);
+	igt_assert(found == 1);
+}
+
+static inline void assert_timestamp_entry(data_t *data, struct timestamps *pre)
+{
+	int timeout = 300; /* 3 seconds */
+
+	if (!data->with_timestamps)
+		return;
+
+	do {
+		__timestamp_read(data->drm_fd, &pre->last_attempted_entry,
+				 &pre->last_exit);
+		igt_debug("pre-condition: last attempted exit: %ld last exit: %ld\n",
+			  pre->last_attempted_entry, pre->last_exit);
+
+		if (pre->last_attempted_entry > pre->last_exit)
+			break;
+
+		usleep(10000);
+	} while (--timeout);
+
+	/* FIXME: Wait for a few frames after attempted entry interrupts so that
+	 * PSR becomes active. Check if PSR_STATUS can be polled for a more
+	 * precise check.
+	 */
+	usleep(50000);
+	igt_assert_f(timeout, "timed out waiting for PSR entry\n");
+}
+
+static inline void assert_timestamp_exit(data_t *data, struct timestamps *pre)
+{
+	int64_t last_attempted_entry = 0, last_exit = 0;
+	int timeout = 300; /* 3 seconds */
+
+	if (!data->with_timestamps)
+		return;
+
+	do {
+		__timestamp_read(data->drm_fd, &last_attempted_entry,
+				 &last_exit);
+		igt_debug("post trigger: last attempted entry: %ld last exit: %ld\n",
+			  last_attempted_entry, last_exit);
+
+		igt_assert_f(pre->last_attempted_entry == last_attempted_entry,
+			     "PSR was not active before the exit trigger or the test waited too long to check for exit\n");
+
+		if (last_exit > last_attempted_entry)
+			break;
+
+		usleep(10000);
+	} while (--timeout);
+	igt_assert_f(timeout, "timedout waiting for PSR exit\n");
+}
+
 static void run_test(data_t *data)
 {
 	uint32_t handle = data->fb_white.gem_handle;
@@ -311,8 +389,10 @@ static void run_test(data_t *data)
 	void *ptr;
 	char ref_crc[CRC_LEN];
 	const char *expected = "";
+	struct timestamps pre = {0};
 
-	igt_require_f(igt_interactive_debug || data->with_sink_crc,
+	igt_require_f(data->with_timestamps ||
+		      igt_interactive_debug || data->with_sink_crc,
 		      "Enable interactive debug with --interactive-debug or "
 		      "enable sink crc with --sink-crc\n");
 
@@ -334,6 +414,7 @@ static void run_test(data_t *data)
 	manual(test_plane->type == DRM_PLANE_TYPE_PRIMARY ?
 	       "screen WHITE" : "WHITE box on GREEN");
 	is_not_green_crc(data, ref_crc);
+	assert_timestamp_entry(data, &pre);
 
 	switch (data->op) {
 	case PAGE_FLIP:
@@ -383,6 +464,8 @@ static void run_test(data_t *data)
 		expected = "screen GREEN";
 		break;
 	}
+
+	assert_timestamp_exit(data, &pre);
 	manual(expected);
 	is_not_equal_crc(data, ref_crc);
 }
@@ -585,6 +668,7 @@ int main(int argc, char *argv[])
 	}
 
 	igt_fixture {
+		igt_sysfs_set(data.debugfs_fd, "i915_edp_psr_debug", "0");
 		close(data.debugfs_fd);
 		drm_intel_bufmgr_destroy(data.bufmgr);
 		display_fini(&data);
-- 
2.14.1

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

Re: [igt-dev] [PATCH i-g-t 19/20] tests/psr: Test PSR using interrupt timestamps.

[Katarzyna Dec]

On Tue, Apr 10, 2018 at 07:37:31PM -0700, Dhinakaran Pandiyan wrote:
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Please add more information about this test, why do we need it, what
we want to achieve with it.
This code looks good in general, but I am not experienced enough to
give r-b without others opinion and more info in commit msg.

Kasia
> ---
>  tests/kms_psr_sink_crc.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 86 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index 147b492f..dd811fb0 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -78,6 +78,11 @@ typedef struct {
>  	bool with_timestamps;
>  } data_t;
>  
> +struct timestamps {
> +	int64_t last_attempted_entry;
> +	int64_t last_exit;
> +};
> +
>  static void create_cursor_fb(data_t *data)
>  {
>  	cairo_t *cr;
> @@ -209,7 +214,9 @@ static void timestamp_support(data_t *data)
>  	igt_debugfs_read(data->drm_fd, "i915_edp_psr_status", buf);
>  	data->with_timestamps = strstr(buf, "Last exit at:");
>  	igt_debug("Time stamp support %d\n", data->with_timestamps);
> -	igt_sysfs_set(data->debugfs_fd, "i915_edp_psr_debug", "0");
> +
> +	if (!data->with_timestamps)
> +		igt_sysfs_set(data->debugfs_fd, "i915_edp_psr_debug", "0");
>  }
>  
>  static bool psr_enabled(data_t *data)
> @@ -304,6 +311,77 @@ static bool drrs_disabled(data_t *data)
>  	return !strstr(buf, "DRRS Supported: Yes\n");
>  }
>  
> +static inline void  __timestamp_read(int fd, int64_t *entry, int64_t *exit)
> +{
> +	int found;
> +	char buf[512];
> +	char *begin;
> +
> +	igt_debugfs_read(fd, "i915_edp_psr_status", buf);
> +
> +	begin = strstr(buf, "Last attempted entry at: ");
> +	igt_assert_f(begin, "Could not read timestamp\n");
> +	found = sscanf(begin, "Last attempted entry at: %ld", entry);
> +	igt_assert(found == 1);
> +
> +	begin = strstr(buf, "Last exit at: ");
> +	igt_assert_f(begin, "Could not read timestamp\n");
> +	found =	sscanf(begin, "Last exit at: %ld\n", exit);
> +	igt_assert(found == 1);
> +}
> +
> +static inline void assert_timestamp_entry(data_t *data, struct timestamps *pre)
> +{
> +	int timeout = 300; /* 3 seconds */
> +
> +	if (!data->with_timestamps)
> +		return;
> +
> +	do {
> +		__timestamp_read(data->drm_fd, &pre->last_attempted_entry,
> +				 &pre->last_exit);
> +		igt_debug("pre-condition: last attempted exit: %ld last exit: %ld\n",
> +			  pre->last_attempted_entry, pre->last_exit);
> +
> +		if (pre->last_attempted_entry > pre->last_exit)
> +			break;
> +
> +		usleep(10000);
> +	} while (--timeout);
> +
> +	/* FIXME: Wait for a few frames after attempted entry interrupts so that
> +	 * PSR becomes active. Check if PSR_STATUS can be polled for a more
> +	 * precise check.
> +	 */
> +	usleep(50000);
> +	igt_assert_f(timeout, "timed out waiting for PSR entry\n");
> +}
> +
> +static inline void assert_timestamp_exit(data_t *data, struct timestamps *pre)
> +{
> +	int64_t last_attempted_entry = 0, last_exit = 0;
> +	int timeout = 300; /* 3 seconds */
> +
> +	if (!data->with_timestamps)
> +		return;
> +
> +	do {
> +		__timestamp_read(data->drm_fd, &last_attempted_entry,
> +				 &last_exit);
> +		igt_debug("post trigger: last attempted entry: %ld last exit: %ld\n",
> +			  last_attempted_entry, last_exit);
> +
> +		igt_assert_f(pre->last_attempted_entry == last_attempted_entry,
> +			     "PSR was not active before the exit trigger or the test waited too long to check for exit\n");
> +
> +		if (last_exit > last_attempted_entry)
> +			break;
> +
> +		usleep(10000);
> +	} while (--timeout);
> +	igt_assert_f(timeout, "timedout waiting for PSR exit\n");
> +}
> +
>  static void run_test(data_t *data)
>  {
>  	uint32_t handle = data->fb_white.gem_handle;
> @@ -311,8 +389,10 @@ static void run_test(data_t *data)
>  	void *ptr;
>  	char ref_crc[CRC_LEN];
>  	const char *expected = "";
> +	struct timestamps pre = {0};
>  
> -	igt_require_f(igt_interactive_debug || data->with_sink_crc,
> +	igt_require_f(data->with_timestamps ||
> +		      igt_interactive_debug || data->with_sink_crc,
>  		      "Enable interactive debug with --interactive-debug or "
>  		      "enable sink crc with --sink-crc\n");
>  
> @@ -334,6 +414,7 @@ static void run_test(data_t *data)
>  	manual(test_plane->type == DRM_PLANE_TYPE_PRIMARY ?
>  	       "screen WHITE" : "WHITE box on GREEN");
>  	is_not_green_crc(data, ref_crc);
> +	assert_timestamp_entry(data, &pre);
>  
>  	switch (data->op) {
>  	case PAGE_FLIP:
> @@ -383,6 +464,8 @@ static void run_test(data_t *data)
>  		expected = "screen GREEN";
>  		break;
>  	}
> +
> +	assert_timestamp_exit(data, &pre);
>  	manual(expected);
>  	is_not_equal_crc(data, ref_crc);
>  }
> @@ -585,6 +668,7 @@ int main(int argc, char *argv[])
>  	}
>  
>  	igt_fixture {
> +		igt_sysfs_set(data.debugfs_fd, "i915_edp_psr_debug", "0");
>  		close(data.debugfs_fd);
>  		drm_intel_bufmgr_destroy(data.bufmgr);
>  		display_fini(&data);
> -- 
> 2.14.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t v2] tests/psr: Test PSR using interrupt timestamps.

[Dhinakaran Pandiyan]

sink-crc is problematic and hence not reliable enough for testing PSR.
Since the hardware generates interrupts for PSR pre-entry and PSR exit,
these interrupt timestamps can be made use of to check for PSR exit.

Note: this has been tested only on a SKL, but should work on others as
well.

v2: Commit message (Kasia)
Cc: Katarzyna Dec <katarzyna.dec@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 tests/kms_psr_sink_crc.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 86 insertions(+), 2 deletions(-)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 147b492f..dd811fb0 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -78,6 +78,11 @@ typedef struct {
 	bool with_timestamps;
 } data_t;
 
+struct timestamps {
+	int64_t last_attempted_entry;
+	int64_t last_exit;
+};
+
 static void create_cursor_fb(data_t *data)
 {
 	cairo_t *cr;
@@ -209,7 +214,9 @@ static void timestamp_support(data_t *data)
 	igt_debugfs_read(data->drm_fd, "i915_edp_psr_status", buf);
 	data->with_timestamps = strstr(buf, "Last exit at:");
 	igt_debug("Time stamp support %d\n", data->with_timestamps);
-	igt_sysfs_set(data->debugfs_fd, "i915_edp_psr_debug", "0");
+
+	if (!data->with_timestamps)
+		igt_sysfs_set(data->debugfs_fd, "i915_edp_psr_debug", "0");
 }
 
 static bool psr_enabled(data_t *data)
@@ -304,6 +311,77 @@ static bool drrs_disabled(data_t *data)
 	return !strstr(buf, "DRRS Supported: Yes\n");
 }
 
+static inline void  __timestamp_read(int fd, int64_t *entry, int64_t *exit)
+{
+	int found;
+	char buf[512];
+	char *begin;
+
+	igt_debugfs_read(fd, "i915_edp_psr_status", buf);
+
+	begin = strstr(buf, "Last attempted entry at: ");
+	igt_assert_f(begin, "Could not read timestamp\n");
+	found = sscanf(begin, "Last attempted entry at: %ld", entry);
+	igt_assert(found == 1);
+
+	begin = strstr(buf, "Last exit at: ");
+	igt_assert_f(begin, "Could not read timestamp\n");
+	found =	sscanf(begin, "Last exit at: %ld\n", exit);
+	igt_assert(found == 1);
+}
+
+static inline void assert_timestamp_entry(data_t *data, struct timestamps *pre)
+{
+	int timeout = 300; /* 3 seconds */
+
+	if (!data->with_timestamps)
+		return;
+
+	do {
+		__timestamp_read(data->drm_fd, &pre->last_attempted_entry,
+				 &pre->last_exit);
+		igt_debug("pre-condition: last attempted exit: %ld last exit: %ld\n",
+			  pre->last_attempted_entry, pre->last_exit);
+
+		if (pre->last_attempted_entry > pre->last_exit)
+			break;
+
+		usleep(10000);
+	} while (--timeout);
+
+	/* FIXME: Wait for a few frames after attempted entry interrupts so that
+	 * PSR becomes active. Check if PSR_STATUS can be polled for a more
+	 * precise check.
+	 */
+	usleep(50000);
+	igt_assert_f(timeout, "timed out waiting for PSR entry\n");
+}
+
+static inline void assert_timestamp_exit(data_t *data, struct timestamps *pre)
+{
+	int64_t last_attempted_entry = 0, last_exit = 0;
+	int timeout = 300; /* 3 seconds */
+
+	if (!data->with_timestamps)
+		return;
+
+	do {
+		__timestamp_read(data->drm_fd, &last_attempted_entry,
+				 &last_exit);
+		igt_debug("post trigger: last attempted entry: %ld last exit: %ld\n",
+			  last_attempted_entry, last_exit);
+
+		igt_assert_f(pre->last_attempted_entry == last_attempted_entry,
+			     "PSR was not active before the exit trigger or the test waited too long to check for exit\n");
+
+		if (last_exit > last_attempted_entry)
+			break;
+
+		usleep(10000);
+	} while (--timeout);
+	igt_assert_f(timeout, "timedout waiting for PSR exit\n");
+}
+
 static void run_test(data_t *data)
 {
 	uint32_t handle = data->fb_white.gem_handle;
@@ -311,8 +389,10 @@ static void run_test(data_t *data)
 	void *ptr;
 	char ref_crc[CRC_LEN];
 	const char *expected = "";
+	struct timestamps pre = {0};
 
-	igt_require_f(igt_interactive_debug || data->with_sink_crc,
+	igt_require_f(data->with_timestamps ||
+		      igt_interactive_debug || data->with_sink_crc,
 		      "Enable interactive debug with --interactive-debug or "
 		      "enable sink crc with --sink-crc\n");
 
@@ -334,6 +414,7 @@ static void run_test(data_t *data)
 	manual(test_plane->type == DRM_PLANE_TYPE_PRIMARY ?
 	       "screen WHITE" : "WHITE box on GREEN");
 	is_not_green_crc(data, ref_crc);
+	assert_timestamp_entry(data, &pre);
 
 	switch (data->op) {
 	case PAGE_FLIP:
@@ -383,6 +464,8 @@ static void run_test(data_t *data)
 		expected = "screen GREEN";
 		break;
 	}
+
+	assert_timestamp_exit(data, &pre);
 	manual(expected);
 	is_not_equal_crc(data, ref_crc);
 }
@@ -585,6 +668,7 @@ int main(int argc, char *argv[])
 	}
 
 	igt_fixture {
+		igt_sysfs_set(data.debugfs_fd, "i915_edp_psr_debug", "0");
 		close(data.debugfs_fd);
 		drm_intel_bufmgr_destroy(data.bufmgr);
 		display_fini(&data);
-- 
2.14.1

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

Re: [igt-dev] [PATCH i-g-t v2] tests/psr: Test PSR using interrupt timestamps.

[Katarzyna Dec]

On Mon, Apr 16, 2018 at 03:49:41PM -0700, Dhinakaran Pandiyan wrote:
> sink-crc is problematic and hence not reliable enough for testing PSR.
> Since the hardware generates interrupts for PSR pre-entry and PSR exit,
> these interrupt timestamps can be made use of to check for PSR exit.
> 
> Note: this has been tested only on a SKL, but should work on others as
> well.
> 
> v2: Commit message (Kasia)
> Cc: Katarzyna Dec <katarzyna.dec@intel.com>
LGTM, but I think someone more experiences with display should look at 
this code as well.

Reviewed-by: Katarzyna Dec <katarzyna.dec@intel.com>

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

[igt-dev] [PATCH i-g-t 20/20] tests/psr: Add more PSR subtests to fast feedback list

[Dhinakaran Pandiyan]

None of these tests take more than a couple of seconds and the current
basic subtest is too basic to be useful.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 tests/intel-ci/fast-feedback.testlist | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
index b08ef770..eaac9b45 100644
--- a/tests/intel-ci/fast-feedback.testlist
+++ b/tests/intel-ci/fast-feedback.testlist
@@ -244,6 +244,10 @@ igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a
 igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b
 igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c
 igt@kms_psr_sink_crc@basic
+igt@kms_psr_sink_crc@primary_page_flip
+igt@kms_psr_sink_crc@sprite_plane_onoff
+igt@kms_psr_sink_crc@cursor_plane_move
+igt@kms_psr_sink_crc@dpms
 igt@kms_setmode@basic-clone-single-crtc
 igt@kms_sink_crc_basic
 igt@pm_backlight@basic-brightness
-- 
2.14.1

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

Re: [igt-dev] [PATCH i-g-t 20/20] tests/psr: Add more PSR subtests to fast feedback list

[Petri Latvala]

On Tue, Apr 10, 2018 at 07:37:32PM -0700, Dhinakaran Pandiyan wrote:
> None of these tests take more than a couple of seconds and the current
> basic subtest is too basic to be useful.


These added tests are useless in fast-feedback in this form, or in CI
in general in fact, if all they do is skip without --sink-crc.

https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1243/all.html
https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1243/fi-glk-1/igt@kms_psr_sink_crc@primary_page_flip.html

-- 
Petri Latvala



> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  tests/intel-ci/fast-feedback.testlist | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
> index b08ef770..eaac9b45 100644
> --- a/tests/intel-ci/fast-feedback.testlist
> +++ b/tests/intel-ci/fast-feedback.testlist
> @@ -244,6 +244,10 @@ igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a
>  igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b
>  igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c
>  igt@kms_psr_sink_crc@basic
> +igt@kms_psr_sink_crc@primary_page_flip
> +igt@kms_psr_sink_crc@sprite_plane_onoff
> +igt@kms_psr_sink_crc@cursor_plane_move
> +igt@kms_psr_sink_crc@dpms
>  igt@kms_setmode@basic-clone-single-crtc
>  igt@kms_sink_crc_basic
>  igt@pm_backlight@basic-brightness
> -- 
> 2.14.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 20/20] tests/psr: Add more PSR subtests to fast feedback list

[Pandiyan, Dhinakaran]

Yeah, that is expected. The kernel patches are not in yet.

-DK

> -----Original Message-----
> From: Latvala, Petri
> Sent: Wednesday, April 11, 2018 1:43 AM
> To: Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
> Cc: igt-dev@lists.freedesktop.org; Vivi, Rodrigo <rodrigo.vivi@intel.com>
> Subject: Re: [igt-dev] [PATCH i-g-t 20/20] tests/psr: Add more PSR subtests to
> fast feedback list
> 
> On Tue, Apr 10, 2018 at 07:37:32PM -0700, Dhinakaran Pandiyan wrote:
> > None of these tests take more than a couple of seconds and the current
> > basic subtest is too basic to be useful.
> 
> 
> These added tests are useless in fast-feedback in this form, or in CI in general
> in fact, if all they do is skip without --sink-crc.
> 
> https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1243/all.html
> https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1243/fi-glk-
> 1/igt@kms_psr_sink_crc@primary_page_flip.html
> 
> --
> Petri Latvala
> 
> 
> 
> >
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  tests/intel-ci/fast-feedback.testlist | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-
> feedback.testlist
> > index b08ef770..eaac9b45 100644
> > --- a/tests/intel-ci/fast-feedback.testlist
> > +++ b/tests/intel-ci/fast-feedback.testlist
> > @@ -244,6 +244,10 @@ igt@kms_pipe_crc_basic@suspend-read-crc-
> pipe-a
> >  igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b
> >  igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c
> >  igt@kms_psr_sink_crc@basic
> > +igt@kms_psr_sink_crc@primary_page_flip
> > +igt@kms_psr_sink_crc@sprite_plane_onoff
> > +igt@kms_psr_sink_crc@cursor_plane_move
> > +igt@kms_psr_sink_crc@dpms
> >  igt@kms_setmode@basic-clone-single-crtc
> >  igt@kms_sink_crc_basic
> >  igt@pm_backlight@basic-brightness
> > --
> > 2.14.1
> >
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 20/20] tests/psr: Add more PSR subtests to fast feedback list

[Petri Latvala]

On Wed, Apr 11, 2018 at 07:14:36PM +0300, Pandiyan, Dhinakaran wrote:
> Yeah, that is expected. The kernel patches are not in yet.

The requirement for using --sink-crc is going away when kernel side
patches land? Or how is this planned?


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

Re: [igt-dev] [PATCH i-g-t 20/20] tests/psr: Add more PSR subtests to fast feedback list

[Dhinakaran Pandiyan]

On Fri, 2018-04-13 at 12:16 +0300, Petri Latvala wrote:
> On Wed, Apr 11, 2018 at 07:14:36PM +0300, Pandiyan, Dhinakaran wrote:
> > Yeah, that is expected. The kernel patches are not in yet.
> 
> The requirement for using --sink-crc is going away when kernel side
> patches land? Or how is this planned?
> 
Irrespective of whether or when the kernel patches land, --sink-crc will
not be used for CI. --sink-crc is being retained for developers to
manually use it for debug and possibly fix the code in future.

Having said that, the kernel patches adding time stamp support should
land soon, after which the tests will not skip.





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

Re: [igt-dev] [PATCH i-g-t 20/20] tests/psr: Add more PSR subtests to fast feedback list

[Rodrigo Vivi]

On Sat, Apr 14, 2018 at 06:13:40PM -0700, Dhinakaran Pandiyan wrote:
> 
> On Fri, 2018-04-13 at 12:16 +0300, Petri Latvala wrote:
> > On Wed, Apr 11, 2018 at 07:14:36PM +0300, Pandiyan, Dhinakaran wrote:
> > > Yeah, that is expected. The kernel patches are not in yet.
> > 
> > The requirement for using --sink-crc is going away when kernel side
> > patches land? Or how is this planned?
> > 
> Irrespective of whether or when the kernel patches land, --sink-crc will
> not be used for CI. --sink-crc is being retained for developers to
> manually use it for debug and possibly fix the code in future.
> 
> Having said that, the kernel patches adding time stamp support should
> land soon, after which the tests will not skip.

Ok, I finally looked to all patches on this series and I support all of them.

My only concern is actually with patch 14 that is around this discussion here...

For me it seems that or you run with interactive debug on or with sink_crc
and CI is not running with any so it will all skip.

Unless I missunderstood something on that patch...

If CI still runs without requiring any extra option I'm in favor of the whole series
and I'm tending to merge it soon...

Thanks,
Rodrigo.

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

Re: [igt-dev] [PATCH i-g-t 20/20] tests/psr: Add more PSR subtests to fast feedback list

[Dhinakaran Pandiyan]

On Mon, 2018-04-23 at 16:06 -0700, Rodrigo Vivi wrote:
> On Sat, Apr 14, 2018 at 06:13:40PM -0700, Dhinakaran Pandiyan wrote:
> > 
> > On Fri, 2018-04-13 at 12:16 +0300, Petri Latvala wrote:
> > > On Wed, Apr 11, 2018 at 07:14:36PM +0300, Pandiyan, Dhinakaran wrote:
> > > > Yeah, that is expected. The kernel patches are not in yet.
> > > 
> > > The requirement for using --sink-crc is going away when kernel side
> > > patches land? Or how is this planned?
> > > 
> > Irrespective of whether or when the kernel patches land, --sink-crc will
> > not be used for CI. --sink-crc is being retained for developers to
> > manually use it for debug and possibly fix the code in future.
> > 
> > Having said that, the kernel patches adding time stamp support should
> > land soon, after which the tests will not skip.
> 
> Ok, I finally looked to all patches on this series and I support all of them.
> 
> My only concern is actually with patch 14 that is around this discussion here...
> 
> For me it seems that or you run with interactive debug on or with sink_crc
> and CI is not running with any so it will all skip.
> 
> Unless I missunderstood something on that patch...
> 
> If CI still runs without requiring any extra option I'm in favor of the whole series
> and I'm tending to merge it soon...
> 

CI will not need any options. Timestamps are supported by default on all
platforms except CHV/VLV. We don't have those platforms in CI and I am
planning to nuke PSR support for them.


> Thanks,
> Rodrigo.
> 
> > 
> > 
> > 
> > 
> > 

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

Re: [igt-dev] [PATCH i-g-t 20/20] tests/psr: Add more PSR subtests to fast feedback list

[Petri Latvala]

On 04/24/2018 02:58 AM, Dhinakaran Pandiyan wrote:
>
>
> On Mon, 2018-04-23 at 16:06 -0700, Rodrigo Vivi wrote:
>> On Sat, Apr 14, 2018 at 06:13:40PM -0700, Dhinakaran Pandiyan wrote:
>>> On Fri, 2018-04-13 at 12:16 +0300, Petri Latvala wrote:
>>>> On Wed, Apr 11, 2018 at 07:14:36PM +0300, Pandiyan, Dhinakaran wrote:
>>>>> Yeah, that is expected. The kernel patches are not in yet.
>>>> The requirement for using --sink-crc is going away when kernel side
>>>> patches land? Or how is this planned?
>>>>
>>> Irrespective of whether or when the kernel patches land, --sink-crc will
>>> not be used for CI. --sink-crc is being retained for developers to
>>> manually use it for debug and possibly fix the code in future.
>>>
>>> Having said that, the kernel patches adding time stamp support should
>>> land soon, after which the tests will not skip.
>> Ok, I finally looked to all patches on this series and I support all of them.
>>
>> My only concern is actually with patch 14 that is around this discussion here...
>>
>> For me it seems that or you run with interactive debug on or with sink_crc
>> and CI is not running with any so it will all skip.
>>
>> Unless I missunderstood something on that patch...
>>
>> If CI still runs without requiring any extra option I'm in favor of the whole series
>> and I'm tending to merge it soon...
>>
> CI will not need any options. Timestamps are supported by default on all
> platforms except CHV/VLV. We don't have those platforms in CI and I am
> planning to nuke PSR support for them.
>

These machines are VLV:
fi-byt-j1900
fi-byt-n2820


-- 
Petri Latvala

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

Re: [igt-dev] [PATCH i-g-t 20/20] tests/psr: Add more PSR subtests to fast feedback list

[Saarinen, Jani]

HI, 

> -----Original Message-----
> From: igt-dev [mailto:igt-dev-bounces@lists.freedesktop.org] On Behalf Of Petri
> Latvala
> Sent: tiistai 24. huhtikuuta 2018 12.34
> To: Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>; Vivi, Rodrigo
> <rodrigo.vivi@intel.com>
> Cc: igt-dev@lists.freedesktop.org
> Subject: Re: [igt-dev] [PATCH i-g-t 20/20] tests/psr: Add more PSR subtests to
> fast feedback list
> 
> 
> 
> On 04/24/2018 02:58 AM, Dhinakaran Pandiyan wrote:
> >
> >
> > On Mon, 2018-04-23 at 16:06 -0700, Rodrigo Vivi wrote:
> >> On Sat, Apr 14, 2018 at 06:13:40PM -0700, Dhinakaran Pandiyan wrote:
> >>> On Fri, 2018-04-13 at 12:16 +0300, Petri Latvala wrote:
> >>>> On Wed, Apr 11, 2018 at 07:14:36PM +0300, Pandiyan, Dhinakaran wrote:
> >>>>> Yeah, that is expected. The kernel patches are not in yet.
> >>>> The requirement for using --sink-crc is going away when kernel side
> >>>> patches land? Or how is this planned?
> >>>>
> >>> Irrespective of whether or when the kernel patches land, --sink-crc
> >>> will not be used for CI. --sink-crc is being retained for developers
> >>> to manually use it for debug and possibly fix the code in future.
> >>>
> >>> Having said that, the kernel patches adding time stamp support
> >>> should land soon, after which the tests will not skip.
> >> Ok, I finally looked to all patches on this series and I support all of them.
> >>
> >> My only concern is actually with patch 14 that is around this discussion here...
> >>
> >> For me it seems that or you run with interactive debug on or with
> >> sink_crc and CI is not running with any so it will all skip.
> >>
> >> Unless I missunderstood something on that patch...
> >>
> >> If CI still runs without requiring any extra option I'm in favor of
> >> the whole series and I'm tending to merge it soon...
> >>
> > CI will not need any options. Timestamps are supported by default on
> > all platforms except CHV/VLV. We don't have those platforms in CI and
> > I am planning to nuke PSR support for them.
> >
> 
> These machines are VLV:
> fi-byt-j1900
> fi-byt-n2820
Also CHV=BSW => fi-bsw-n3050

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

Re: [igt-dev] [PATCH i-g-t 20/20] tests/psr: Add more PSR subtests to fast feedback list

[Dhinakaran Pandiyan]

On Tue, 2018-04-24 at 15:32 +0000, Saarinen, Jani wrote:
> HI, 
> 
> > -----Original Message-----
> > From: igt-dev [mailto:igt-dev-bounces@lists.freedesktop.org] On Behalf Of Petri
> > Latvala
> > Sent: tiistai 24. huhtikuuta 2018 12.34
> > To: Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>; Vivi, Rodrigo
> > <rodrigo.vivi@intel.com>
> > Cc: igt-dev@lists.freedesktop.org
> > Subject: Re: [igt-dev] [PATCH i-g-t 20/20] tests/psr: Add more PSR subtests to
> > fast feedback list
> > 
> > 
> > 
> > On 04/24/2018 02:58 AM, Dhinakaran Pandiyan wrote:
> > >
> > >
> > > On Mon, 2018-04-23 at 16:06 -0700, Rodrigo Vivi wrote:
> > >> On Sat, Apr 14, 2018 at 06:13:40PM -0700, Dhinakaran Pandiyan wrote:
> > >>> On Fri, 2018-04-13 at 12:16 +0300, Petri Latvala wrote:
> > >>>> On Wed, Apr 11, 2018 at 07:14:36PM +0300, Pandiyan, Dhinakaran wrote:
> > >>>>> Yeah, that is expected. The kernel patches are not in yet.
> > >>>> The requirement for using --sink-crc is going away when kernel side
> > >>>> patches land? Or how is this planned?
> > >>>>
> > >>> Irrespective of whether or when the kernel patches land, --sink-crc
> > >>> will not be used for CI. --sink-crc is being retained for developers
> > >>> to manually use it for debug and possibly fix the code in future.
> > >>>
> > >>> Having said that, the kernel patches adding time stamp support
> > >>> should land soon, after which the tests will not skip.
> > >> Ok, I finally looked to all patches on this series and I support all of them.
> > >>
> > >> My only concern is actually with patch 14 that is around this discussion here...
> > >>
> > >> For me it seems that or you run with interactive debug on or with
> > >> sink_crc and CI is not running with any so it will all skip.
> > >>
> > >> Unless I missunderstood something on that patch...
> > >>
> > >> If CI still runs without requiring any extra option I'm in favor of
> > >> the whole series and I'm tending to merge it soon...
> > >>
> > > CI will not need any options. Timestamps are supported by default on
> > > all platforms except CHV/VLV. We don't have those platforms in CI and
> > > I am planning to nuke PSR support for them.
> > >
> > 
> > These machines are VLV:
> > fi-byt-j1900
> > fi-byt-n2820
> Also CHV=BSW => fi-bsw-n3050
> 

No psr panels on them.


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

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

Re: [igt-dev] [PATCH i-g-t 20/20] tests/psr: Add more PSR subtests to fast feedback list

[Rodrigo Vivi]

On Tue, Apr 24, 2018 at 10:48:43AM -0700, Dhinakaran Pandiyan wrote:
> 
> 
> 
> On Tue, 2018-04-24 at 15:32 +0000, Saarinen, Jani wrote:
> > HI, 
> > 
> > > -----Original Message-----
> > > From: igt-dev [mailto:igt-dev-bounces@lists.freedesktop.org] On Behalf Of Petri
> > > Latvala
> > > Sent: tiistai 24. huhtikuuta 2018 12.34
> > > To: Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>; Vivi, Rodrigo
> > > <rodrigo.vivi@intel.com>
> > > Cc: igt-dev@lists.freedesktop.org
> > > Subject: Re: [igt-dev] [PATCH i-g-t 20/20] tests/psr: Add more PSR subtests to
> > > fast feedback list
> > > 
> > > 
> > > 
> > > On 04/24/2018 02:58 AM, Dhinakaran Pandiyan wrote:
> > > >
> > > >
> > > > On Mon, 2018-04-23 at 16:06 -0700, Rodrigo Vivi wrote:
> > > >> On Sat, Apr 14, 2018 at 06:13:40PM -0700, Dhinakaran Pandiyan wrote:
> > > >>> On Fri, 2018-04-13 at 12:16 +0300, Petri Latvala wrote:
> > > >>>> On Wed, Apr 11, 2018 at 07:14:36PM +0300, Pandiyan, Dhinakaran wrote:
> > > >>>>> Yeah, that is expected. The kernel patches are not in yet.
> > > >>>> The requirement for using --sink-crc is going away when kernel side
> > > >>>> patches land? Or how is this planned?
> > > >>>>
> > > >>> Irrespective of whether or when the kernel patches land, --sink-crc
> > > >>> will not be used for CI. --sink-crc is being retained for developers
> > > >>> to manually use it for debug and possibly fix the code in future.
> > > >>>
> > > >>> Having said that, the kernel patches adding time stamp support
> > > >>> should land soon, after which the tests will not skip.
> > > >> Ok, I finally looked to all patches on this series and I support all of them.
> > > >>
> > > >> My only concern is actually with patch 14 that is around this discussion here...
> > > >>
> > > >> For me it seems that or you run with interactive debug on or with
> > > >> sink_crc and CI is not running with any so it will all skip.
> > > >>
> > > >> Unless I missunderstood something on that patch...
> > > >>
> > > >> If CI still runs without requiring any extra option I'm in favor of
> > > >> the whole series and I'm tending to merge it soon...
> > > >>
> > > > CI will not need any options. Timestamps are supported by default on
> > > > all platforms except CHV/VLV. We don't have those platforms in CI and
> > > > I am planning to nuke PSR support for them.
> > > >
> > > 
> > > These machines are VLV:
> > > fi-byt-j1900
> > > fi-byt-n2820
> > Also CHV=BSW => fi-bsw-n3050
> > 
> 
> No psr panels on them.

Ok, so it seems no need for waiting...


Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
on the series.

but let's see if Petri has more concerns.

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

Re: [igt-dev] [PATCH i-g-t 20/20] tests/psr: Add more PSR subtests to fast feedback list

[Petri Latvala]

On Tue, Apr 24, 2018 at 02:28:17PM -0700, Rodrigo Vivi wrote:
> On Tue, Apr 24, 2018 at 10:48:43AM -0700, Dhinakaran Pandiyan wrote:
> > 
> > 
> > 
> > On Tue, 2018-04-24 at 15:32 +0000, Saarinen, Jani wrote:
> > > HI, 
> > > 
> > > > -----Original Message-----
> > > > From: igt-dev [mailto:igt-dev-bounces@lists.freedesktop.org] On Behalf Of Petri
> > > > Latvala
> > > > Sent: tiistai 24. huhtikuuta 2018 12.34
> > > > To: Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>; Vivi, Rodrigo
> > > > <rodrigo.vivi@intel.com>
> > > > Cc: igt-dev@lists.freedesktop.org
> > > > Subject: Re: [igt-dev] [PATCH i-g-t 20/20] tests/psr: Add more PSR subtests to
> > > > fast feedback list
> > > > 
> > > > 
> > > > 
> > > > On 04/24/2018 02:58 AM, Dhinakaran Pandiyan wrote:
> > > > >
> > > > >
> > > > > On Mon, 2018-04-23 at 16:06 -0700, Rodrigo Vivi wrote:
> > > > >> On Sat, Apr 14, 2018 at 06:13:40PM -0700, Dhinakaran Pandiyan wrote:
> > > > >>> On Fri, 2018-04-13 at 12:16 +0300, Petri Latvala wrote:
> > > > >>>> On Wed, Apr 11, 2018 at 07:14:36PM +0300, Pandiyan, Dhinakaran wrote:
> > > > >>>>> Yeah, that is expected. The kernel patches are not in yet.
> > > > >>>> The requirement for using --sink-crc is going away when kernel side
> > > > >>>> patches land? Or how is this planned?
> > > > >>>>
> > > > >>> Irrespective of whether or when the kernel patches land, --sink-crc
> > > > >>> will not be used for CI. --sink-crc is being retained for developers
> > > > >>> to manually use it for debug and possibly fix the code in future.
> > > > >>>
> > > > >>> Having said that, the kernel patches adding time stamp support
> > > > >>> should land soon, after which the tests will not skip.
> > > > >> Ok, I finally looked to all patches on this series and I support all of them.
> > > > >>
> > > > >> My only concern is actually with patch 14 that is around this discussion here...
> > > > >>
> > > > >> For me it seems that or you run with interactive debug on or with
> > > > >> sink_crc and CI is not running with any so it will all skip.
> > > > >>
> > > > >> Unless I missunderstood something on that patch...
> > > > >>
> > > > >> If CI still runs without requiring any extra option I'm in favor of
> > > > >> the whole series and I'm tending to merge it soon...
> > > > >>
> > > > > CI will not need any options. Timestamps are supported by default on
> > > > > all platforms except CHV/VLV. We don't have those platforms in CI and
> > > > > I am planning to nuke PSR support for them.
> > > > >
> > > > 
> > > > These machines are VLV:
> > > > fi-byt-j1900
> > > > fi-byt-n2820
> > > Also CHV=BSW => fi-bsw-n3050
> > > 
> > 
> > No psr panels on them.
> 
> Ok, so it seems no need for waiting...
> 
> 
> Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> on the series.
> 
> but let's see if Petri has more concerns.


No objections except for the fast-feedback.testlist changes. PSR
additions is acked in practice, but the tests need to first actually
run.


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

Re: [igt-dev] [PATCH i-g-t 20/20] tests/psr: Add more PSR subtests to fast feedback list

[Pandiyan, Dhinakaran]

On Wed, 2018-04-25 at 11:34 +0300, Petri Latvala wrote:
> On Tue, Apr 24, 2018 at 02:28:17PM -0700, Rodrigo Vivi wrote:
> > On Tue, Apr 24, 2018 at 10:48:43AM -0700, Dhinakaran Pandiyan wrote:
> > > 
> > > 
> > > 
> > > On Tue, 2018-04-24 at 15:32 +0000, Saarinen, Jani wrote:
> > > > HI, 
> > > > 
> > > > > -----Original Message-----
> > > > > From: igt-dev [mailto:igt-dev-bounces@lists.freedesktop.org] On Behalf Of Petri
> > > > > Latvala
> > > > > Sent: tiistai 24. huhtikuuta 2018 12.34
> > > > > To: Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>; Vivi, Rodrigo
> > > > > <rodrigo.vivi@intel.com>
> > > > > Cc: igt-dev@lists.freedesktop.org
> > > > > Subject: Re: [igt-dev] [PATCH i-g-t 20/20] tests/psr: Add more PSR subtests to
> > > > > fast feedback list
> > > > > 
> > > > > 
> > > > > 
> > > > > On 04/24/2018 02:58 AM, Dhinakaran Pandiyan wrote:
> > > > > >
> > > > > >
> > > > > > On Mon, 2018-04-23 at 16:06 -0700, Rodrigo Vivi wrote:
> > > > > >> On Sat, Apr 14, 2018 at 06:13:40PM -0700, Dhinakaran Pandiyan wrote:
> > > > > >>> On Fri, 2018-04-13 at 12:16 +0300, Petri Latvala wrote:
> > > > > >>>> On Wed, Apr 11, 2018 at 07:14:36PM +0300, Pandiyan, Dhinakaran wrote:
> > > > > >>>>> Yeah, that is expected. The kernel patches are not in yet.
> > > > > >>>> The requirement for using --sink-crc is going away when kernel side
> > > > > >>>> patches land? Or how is this planned?
> > > > > >>>>
> > > > > >>> Irrespective of whether or when the kernel patches land, --sink-crc
> > > > > >>> will not be used for CI. --sink-crc is being retained for developers
> > > > > >>> to manually use it for debug and possibly fix the code in future.
> > > > > >>>
> > > > > >>> Having said that, the kernel patches adding time stamp support
> > > > > >>> should land soon, after which the tests will not skip.
> > > > > >> Ok, I finally looked to all patches on this series and I support all of them.
> > > > > >>
> > > > > >> My only concern is actually with patch 14 that is around this discussion here...
> > > > > >>
> > > > > >> For me it seems that or you run with interactive debug on or with
> > > > > >> sink_crc and CI is not running with any so it will all skip.
> > > > > >>
> > > > > >> Unless I missunderstood something on that patch...
> > > > > >>
> > > > > >> If CI still runs without requiring any extra option I'm in favor of
> > > > > >> the whole series and I'm tending to merge it soon...
> > > > > >>
> > > > > > CI will not need any options. Timestamps are supported by default on
> > > > > > all platforms except CHV/VLV. We don't have those platforms in CI and
> > > > > > I am planning to nuke PSR support for them.
> > > > > >
> > > > > 
> > > > > These machines are VLV:
> > > > > fi-byt-j1900
> > > > > fi-byt-n2820
> > > > Also CHV=BSW => fi-bsw-n3050
> > > > 
> > > 
> > > No psr panels on them.
> > 
> > Ok, so it seems no need for waiting...
> > 
> > 
> > Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > on the series.
> > 
> > but let's see if Petri has more concerns.
> 
> 
> No objections except for the fast-feedback.testlist changes. PSR
> additions is acked in practice, but the tests need to first actually
> run.
> 

Petri,

If your concern is about tests getting skipped on PSR capable CI
platforms, that is not a problem anymore. The kernel support was merged
and the tests should not skip on these platforms.

References:
3f983e54fdad drm/i915/psr: Timestamps for PSR entry and exit interrupts.
54fd3149598c drm/i915/psr: Control PSR interrupts via debugfs
e04f7ece1c45 drm/i915: Enable edp psr error interrupts on bdw+
fc34044248b6 drm/i915: Enable edp psr error interrupts on hsw


Anyone with privileges to the patchwork retest button should retest this
series, if there are still concerns.


Rodrigo,

Can you merge the rest of the patches please? Or trigger a retest and
merge the fast-feedback patch too?
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 20/20] tests/psr: Add more PSR subtests to fast feedback list

[Rodrigo Vivi]

On Wed, Apr 25, 2018 at 10:56:51AM -0700, Pandiyan, Dhinakaran wrote:
> 
> 
> 
> On Wed, 2018-04-25 at 11:34 +0300, Petri Latvala wrote:
> > On Tue, Apr 24, 2018 at 02:28:17PM -0700, Rodrigo Vivi wrote:
> > > On Tue, Apr 24, 2018 at 10:48:43AM -0700, Dhinakaran Pandiyan wrote:
> > > > 
> > > > 
> > > > 
> > > > On Tue, 2018-04-24 at 15:32 +0000, Saarinen, Jani wrote:
> > > > > HI, 
> > > > > 
> > > > > > -----Original Message-----
> > > > > > From: igt-dev [mailto:igt-dev-bounces@lists.freedesktop.org] On Behalf Of Petri
> > > > > > Latvala
> > > > > > Sent: tiistai 24. huhtikuuta 2018 12.34
> > > > > > To: Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>; Vivi, Rodrigo
> > > > > > <rodrigo.vivi@intel.com>
> > > > > > Cc: igt-dev@lists.freedesktop.org
> > > > > > Subject: Re: [igt-dev] [PATCH i-g-t 20/20] tests/psr: Add more PSR subtests to
> > > > > > fast feedback list
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > On 04/24/2018 02:58 AM, Dhinakaran Pandiyan wrote:
> > > > > > >
> > > > > > >
> > > > > > > On Mon, 2018-04-23 at 16:06 -0700, Rodrigo Vivi wrote:
> > > > > > >> On Sat, Apr 14, 2018 at 06:13:40PM -0700, Dhinakaran Pandiyan wrote:
> > > > > > >>> On Fri, 2018-04-13 at 12:16 +0300, Petri Latvala wrote:
> > > > > > >>>> On Wed, Apr 11, 2018 at 07:14:36PM +0300, Pandiyan, Dhinakaran wrote:
> > > > > > >>>>> Yeah, that is expected. The kernel patches are not in yet.
> > > > > > >>>> The requirement for using --sink-crc is going away when kernel side
> > > > > > >>>> patches land? Or how is this planned?
> > > > > > >>>>
> > > > > > >>> Irrespective of whether or when the kernel patches land, --sink-crc
> > > > > > >>> will not be used for CI. --sink-crc is being retained for developers
> > > > > > >>> to manually use it for debug and possibly fix the code in future.
> > > > > > >>>
> > > > > > >>> Having said that, the kernel patches adding time stamp support
> > > > > > >>> should land soon, after which the tests will not skip.
> > > > > > >> Ok, I finally looked to all patches on this series and I support all of them.
> > > > > > >>
> > > > > > >> My only concern is actually with patch 14 that is around this discussion here...
> > > > > > >>
> > > > > > >> For me it seems that or you run with interactive debug on or with
> > > > > > >> sink_crc and CI is not running with any so it will all skip.
> > > > > > >>
> > > > > > >> Unless I missunderstood something on that patch...
> > > > > > >>
> > > > > > >> If CI still runs without requiring any extra option I'm in favor of
> > > > > > >> the whole series and I'm tending to merge it soon...
> > > > > > >>
> > > > > > > CI will not need any options. Timestamps are supported by default on
> > > > > > > all platforms except CHV/VLV. We don't have those platforms in CI and
> > > > > > > I am planning to nuke PSR support for them.
> > > > > > >
> > > > > > 
> > > > > > These machines are VLV:
> > > > > > fi-byt-j1900
> > > > > > fi-byt-n2820
> > > > > Also CHV=BSW => fi-bsw-n3050
> > > > > 
> > > > 
> > > > No psr panels on them.
> > > 
> > > Ok, so it seems no need for waiting...
> > > 
> > > 
> > > Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > on the series.
> > > 
> > > but let's see if Petri has more concerns.
> > 
> > 
> > No objections except for the fast-feedback.testlist changes. PSR
> > additions is acked in practice, but the tests need to first actually
> > run.
> > 
> 
> Petri,
> 
> If your concern is about tests getting skipped on PSR capable CI
> platforms, that is not a problem anymore. The kernel support was merged
> and the tests should not skip on these platforms.
> 
> References:
> 3f983e54fdad drm/i915/psr: Timestamps for PSR entry and exit interrupts.
> 54fd3149598c drm/i915/psr: Control PSR interrupts via debugfs
> e04f7ece1c45 drm/i915: Enable edp psr error interrupts on bdw+
> fc34044248b6 drm/i915: Enable edp psr error interrupts on hsw
> 
> 
> Anyone with privileges to the patchwork retest button should retest this
> series, if there are still concerns.

done...

> 
> 
> Rodrigo,
> 
> Can you merge the rest of the patches please? Or trigger a retest and
> merge the fast-feedback patch too?

Since I triggered the re-test I will give a time for it to run to avoid any
race issues between CI and me merging the patches.

(I think I should push only the first patch so you complete 5 patches
and get the rights to push the rest hehe just kidding ;))
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,01/20] tests/psr: Print the reason for skipping when sink lacks PSR support

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,01/20] tests/psr: Print the reason for skipping when sink lacks PSR support
URL   : https://patchwork.freedesktop.org/series/41513/
State : success

== Summary ==

IGT patchset tested on top of latest successful build
e5829165c2ef96140c222429c0f174fb72c4324b lib: Dump meminfo and slabinfo if we complain about insufficient memory

with latest DRM-Tip kernel build CI_DRM_4042
98590826e4a4 drm-tip: 2018y-04m-10d-20h-01m-19s UTC integration manifest

Testlist changes:
+igt@kms_psr_sink_crc@basic
+igt@kms_psr_sink_crc@dpms
+igt@kms_psr_sink_crc@no_drrs
+igt@kms_psr_sink_crc@suspend
-igt@kms_psr_sink_crc@cursor_mmap_gtt_waiting
-igt@kms_psr_sink_crc@dpms_off_psr_active
-igt@kms_psr_sink_crc@dpms_off_psr_exit
-igt@kms_psr_sink_crc@primary_mmap_gtt_waiting
-igt@kms_psr_sink_crc@psr_basic
-igt@kms_psr_sink_crc@psr_drrs
-igt@kms_psr_sink_crc@sprite_mmap_gtt_waiting
-igt@kms_psr_sink_crc@suspend_psr_active
-igt@kms_psr_sink_crc@suspend_psr_exit

---- Known issues:

Test debugfs_test:
        Subgroup read_all_entries:
                incomplete -> PASS       (fi-snb-2520m) fdo#103713
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-c-frame-sequence:
                fail       -> PASS       (fi-cfl-s3) fdo#103481

fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#103481 https://bugs.freedesktop.org/show_bug.cgi?id=103481

fi-bdw-5557u     total:289  pass:264  dwarn:0   dfail:0   fail:0   skip:25  time:430s
fi-bdw-gvtdvm    total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:445s
fi-blb-e6850     total:289  pass:220  dwarn:1   dfail:0   fail:0   skip:68  time:384s
fi-bsw-n3050     total:289  pass:239  dwarn:0   dfail:0   fail:0   skip:50  time:542s
fi-bwr-2160      total:289  pass:180  dwarn:0   dfail:0   fail:0   skip:109 time:300s
fi-bxt-dsi       total:289  pass:255  dwarn:0   dfail:0   fail:0   skip:34  time:519s
fi-bxt-j4205     total:289  pass:256  dwarn:0   dfail:0   fail:0   skip:33  time:517s
fi-byt-j1900     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:526s
fi-byt-n2820     total:289  pass:246  dwarn:0   dfail:0   fail:0   skip:43  time:513s
fi-cfl-8700k     total:289  pass:257  dwarn:0   dfail:0   fail:0   skip:32  time:411s
fi-cfl-s3        total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:576s
fi-cfl-u         total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:518s
fi-elk-e7500     total:289  pass:226  dwarn:0   dfail:0   fail:0   skip:63  time:425s
fi-gdg-551       total:289  pass:176  dwarn:0   dfail:0   fail:1   skip:112 time:315s
fi-glk-1         total:289  pass:257  dwarn:0   dfail:0   fail:0   skip:32  time:549s
fi-glk-j4005     total:289  pass:256  dwarn:0   dfail:0   fail:0   skip:33  time:494s
fi-hsw-4770      total:289  pass:258  dwarn:0   dfail:0   fail:0   skip:31  time:411s
fi-ilk-650       total:289  pass:225  dwarn:0   dfail:0   fail:0   skip:64  time:425s
fi-ivb-3520m     total:289  pass:256  dwarn:0   dfail:0   fail:0   skip:33  time:472s
fi-ivb-3770      total:289  pass:252  dwarn:0   dfail:0   fail:0   skip:37  time:432s
fi-kbl-7500u     total:289  pass:260  dwarn:1   dfail:0   fail:0   skip:28  time:474s
fi-kbl-7567u     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:468s
fi-kbl-r         total:289  pass:258  dwarn:0   dfail:0   fail:0   skip:31  time:514s
fi-pnv-d510      total:289  pass:220  dwarn:1   dfail:0   fail:0   skip:68  time:667s
fi-skl-6260u     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:441s
fi-skl-6600u     total:289  pass:258  dwarn:0   dfail:0   fail:0   skip:31  time:541s
fi-skl-6700k2    total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:508s
fi-skl-6770hq    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:508s
fi-skl-guc       total:289  pass:257  dwarn:0   dfail:0   fail:0   skip:32  time:434s
fi-skl-gvtdvm    total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:449s
fi-snb-2520m     total:289  pass:245  dwarn:0   dfail:0   fail:0   skip:44  time:587s
fi-snb-2600      total:289  pass:245  dwarn:0   dfail:0   fail:0   skip:44  time:404s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1243/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.IGT: warning for series starting with [i-g-t,01/20] tests/psr: Print the reason for skipping when sink lacks PSR support

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,01/20] tests/psr: Print the reason for skipping when sink lacks PSR support
URL   : https://patchwork.freedesktop.org/series/41513/
State : warning

== Summary ==

= CI Bug Log - changes from CI_DRM_4042_full -> IGTPW_1243_full =

== Summary - WARNING ==

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

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1243/

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@gem_mocs_settings@mocs-rc6-blt:
      shard-kbl:          PASS -> SKIP

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-mmap-wc:
      shard-hsw:          PASS -> SKIP +1

    igt@kms_vblank@pipe-b-ts-continuation-idle-hang:
      shard-snb:          SKIP -> PASS +1

    igt@perf_pmu@rc6:
      shard-kbl:          SKIP -> PASS

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_flip@blocking-wf_vblank:
      shard-hsw:          PASS -> FAIL (fdo#103928)

    igt@kms_flip@dpms-vs-vblank-race:
      shard-kbl:          PASS -> FAIL (fdo#103060)
      shard-hsw:          PASS -> FAIL (fdo#103060)

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-snb:          PASS -> INCOMPLETE (fdo#103880)

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-wc:
      shard-snb:          SKIP -> FAIL (fdo#105798) +3

    igt@kms_setmode@basic:
      shard-kbl:          PASS -> FAIL (fdo#99912)

    igt@kms_vblank@pipe-a-wait-busy-hang:
      shard-snb:          PASS -> FAIL (fdo#105798) +3

    igt@perf_pmu@busy-accuracy-50-vecs0:
      shard-kbl:          PASS -> FAIL (fdo#105157)

    
    ==== Possible fixes ====

    igt@kms_busy@extended-pageflip-hang-oldfb-render-b:
      shard-kbl:          DMESG-WARN (fdo#103558, fdo#103313) -> PASS +12

    igt@kms_flip@2x-flip-vs-expired-vblank:
      shard-hsw:          FAIL (fdo#105189) -> PASS

    igt@kms_flip@2x-plain-flip-fb-recreate:
      shard-hsw:          FAIL (fdo#100368) -> PASS

    igt@kms_flip@wf_vblank-ts-check:
      shard-hsw:          FAIL (fdo#103928) -> PASS

    igt@pm_rpm@legacy-planes-dpms:
      shard-kbl:          DMESG-WARN (fdo#103558) -> PASS +18

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103313 https://bugs.freedesktop.org/show_bug.cgi?id=103313
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#103880 https://bugs.freedesktop.org/show_bug.cgi?id=103880
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
  fdo#105157 https://bugs.freedesktop.org/show_bug.cgi?id=105157
  fdo#105189 https://bugs.freedesktop.org/show_bug.cgi?id=105189
  fdo#105798 https://bugs.freedesktop.org/show_bug.cgi?id=105798
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (6 -> 4) ==

  Missing    (2): shard-glk shard-glkb 


== Build changes ==

    * IGT: IGT_4418 -> IGTPW_1243
    * Piglit: piglit_4418 -> piglit_4419

  CI_DRM_4042: 98590826e4a407967faad1e021e3b38647a9e77f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1243: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1243/
  IGT_4418: 7c474e011548d35df6b80ceed81d3e6ca560c71d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4418: 45e115f293fd6acc0c9647cf2d3b76be78819ba5 @ git://anongit.freedesktop.org/piglit
  piglit_4419: 45e115f293fd6acc0c9647cf2d3b76be78819ba5 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1243/shards.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 01/20] tests/psr: Print the reason for skipping when sink lacks PSR support

[Katarzyna Dec]

Hello,
I have started doing a review of your series and I got a general question:
Why in setup_test_plane we at first fill data->fb_green fb and never use
it? What is the main purpose of this setup_test_plane function?
In basic test only this one is used (+checking debugfs).

I will be glad for answers.
Kasia
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 01/20] tests/psr: Print the reason for skipping when sink lacks PSR support

[Katarzyna Dec]

After reviewing whole series, I think that code generally looks ok.
I have left some questions in few patches.
From my point of view general idea of this series is not clear for
people not working with display code on daily basis. More information
in commit msg would make things clearer :)
When such a huge job was done, maybe you could also fix some style
errors (checkpatch reports only few of them), but that is not necessary
for series to get r-b from me.
Patches that are not commented by me look good.

Nice job!
Kasia

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

Re: [igt-dev] [PATCH i-g-t 01/20] tests/psr: Print the reason for skipping when sink lacks PSR support

[Katarzyna Dec]

On Tue, Apr 10, 2018 at 07:37:13PM -0700, Dhinakaran Pandiyan wrote:
> And rename the function to match what it does.
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Katarzyna Dec <katarzyna.dec@intel.com>
> ---
>  tests/kms_psr_sink_crc.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index 98d29ba9..93807c03 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -193,7 +193,7 @@ static void fill_render(data_t *data, uint32_t handle, unsigned char color)
>  	gem_bo_busy(data->drm_fd, handle);
>  }
>  
> -static bool psr_possible(data_t *data)
> +static bool sink_support(data_t *data)
>  {
>  	char buf[512];
>  
> @@ -513,8 +513,8 @@ int main(int argc, char *argv[])
>  
>  		igt_set_module_param_int("enable_psr", running_with_psr_disabled ?
>  					 0 : 1);
> -
> -		igt_skip_on(!psr_possible(&data));
> +		igt_require_f(sink_support(&data),
> +			      "Sink does not support PSR\n");
>  
>  		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
>  		igt_assert(data.bufmgr);
> -- 
> 2.14.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,01/20] tests/psr: Print the reason for skipping when sink lacks PSR support (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,01/20] tests/psr: Print the reason for skipping when sink lacks PSR support (rev2)
URL   : https://patchwork.freedesktop.org/series/41513/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4058 -> IGTPW_1273 =

== Summary - FAILURE ==

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

  External URL: https://patchwork.freedesktop.org/api/1.0/series/41513/revisions/2/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@kms_sink_crc_basic:
      fi-cnl-psr:         PASS -> FAIL

    
    ==== Warnings ====

    igt@prime_vgem@basic-fence-flip:
      fi-cnl-y3:          SKIP -> PASS

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-cnl-psr:         PASS -> DMESG-WARN (fdo#104951)

    igt@prime_busy@basic-after-default:
      fi-glk-1:           NOTRUN -> INCOMPLETE (fdo#103359, k.org#198133)

    
    ==== Possible fixes ====

    igt@debugfs_test@read_all_entries:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

    igt@gem_mmap_gtt@basic-small-bo-tiledx:
      fi-gdg-551:         FAIL (fdo#102575) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-ivb-3520m:       DMESG-WARN (fdo#106084) -> PASS +1
      fi-bxt-dsi:         INCOMPLETE (fdo#103927) -> PASS

    
  fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951
  fdo#106084 https://bugs.freedesktop.org/show_bug.cgi?id=106084
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (35 -> 34) ==

  Additional (1): fi-glk-1 
  Missing    (2): fi-ilk-m540 fi-skl-6700hq 


== Build changes ==

    * IGT: IGT_4432 -> IGTPW_1273

  CI_DRM_4058: 241d827c86078c4709c00251d22ea8f7554e3e36 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1273: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1273/
  IGT_4432: 8b77704db49167f7ebfd1c470d9c129d3b862cb6 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4432: 93b35926a150e318439d2505901288594b3548f5 @ git://anongit.freedesktop.org/piglit



== Testlist changes ==

+igt@kms_psr_sink_crc@basic
+igt@kms_psr_sink_crc@dpms
+igt@kms_psr_sink_crc@no_drrs
+igt@kms_psr_sink_crc@suspend
-igt@kms_psr_sink_crc@cursor_mmap_gtt_waiting
-igt@kms_psr_sink_crc@dpms_off_psr_active
-igt@kms_psr_sink_crc@dpms_off_psr_exit
-igt@kms_psr_sink_crc@primary_mmap_gtt_waiting
-igt@kms_psr_sink_crc@psr_basic
-igt@kms_psr_sink_crc@psr_drrs
-igt@kms_psr_sink_crc@sprite_mmap_gtt_waiting
-igt@kms_psr_sink_crc@suspend_psr_active
-igt@kms_psr_sink_crc@suspend_psr_exit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1273/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,01/20] tests/psr: Print the reason for skipping when sink lacks PSR support (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,01/20] tests/psr: Print the reason for skipping when sink lacks PSR support (rev2)
URL   : https://patchwork.freedesktop.org/series/41513/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4098 -> IGTPW_1302 =

== Summary - FAILURE ==

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

  External URL: https://patchwork.freedesktop.org/api/1.0/series/41513/revisions/2/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@kms_psr_sink_crc@dpms:
      fi-cfl-u:           NOTRUN -> FAIL +3
      fi-cnl-psr:         NOTRUN -> FAIL +3

    igt@kms_psr_sink_crc@sprite_plane_onoff:
      {fi-hsw-4200u}:     NOTRUN -> FAIL +4

    igt@kms_sink_crc_basic:
      fi-cnl-psr:         PASS -> FAIL

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       PASS -> INCOMPLETE (fdo#103713)
      fi-ivb-3520m:       PASS -> DMESG-WARN (fdo#106084) +2
      fi-bxt-dsi:         PASS -> INCOMPLETE (fdo#103927)

    igt@kms_psr_sink_crc@primary_page_flip:
      fi-skl-6600u:       NOTRUN -> DMESG-WARN (fdo#105537) +1

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         PASS -> FAIL (fdo#104008)

    
    ==== Possible fixes ====

    igt@kms_frontbuffer_tracking@basic:
      {fi-hsw-4200u}:     DMESG-FAIL (fdo#106103) -> PASS

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

  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#105537 https://bugs.freedesktop.org/show_bug.cgi?id=105537
  fdo#106084 https://bugs.freedesktop.org/show_bug.cgi?id=106084
  fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103


== Participating hosts (38 -> 35) ==

  Missing    (3): fi-ilk-m540 fi-skl-6700hq fi-pnv-d510 


== Build changes ==

    * IGT: IGT_4449 -> IGTPW_1302

  CI_DRM_4098: b0be536432b06582cd76b4b767a4acf4b20aaf10 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1302: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1302/
  IGT_4449: 0350f0e7f6a0e07281445fc3082aa70419f4aac7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4449: ad8992d3fb27fd604b9ab15e7963c42421ced85c @ git://anongit.freedesktop.org/piglit



== Testlist changes ==

+igt@kms_psr_sink_crc@basic
+igt@kms_psr_sink_crc@dpms
+igt@kms_psr_sink_crc@no_drrs
+igt@kms_psr_sink_crc@suspend
-igt@kms_psr_sink_crc@cursor_mmap_gtt_waiting
-igt@kms_psr_sink_crc@dpms_off_psr_active
-igt@kms_psr_sink_crc@dpms_off_psr_exit
-igt@kms_psr_sink_crc@primary_mmap_gtt_waiting
-igt@kms_psr_sink_crc@psr_basic
-igt@kms_psr_sink_crc@psr_drrs
-igt@kms_psr_sink_crc@sprite_mmap_gtt_waiting
-igt@kms_psr_sink_crc@suspend_psr_active
-igt@kms_psr_sink_crc@suspend_psr_exit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1302/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,01/20] tests/psr: Print the reason for skipping when sink lacks PSR support (rev2)

[Petri Latvala]

On Wed, Apr 25, 2018 at 09:00:58PM +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [i-g-t,01/20] tests/psr: Print the reason for skipping when sink lacks PSR support (rev2)
> URL   : https://patchwork.freedesktop.org/series/41513/
> State : failure
> 
> == Summary ==
> 
> = CI Bug Log - changes from CI_DRM_4098 -> IGTPW_1302 =
> 
> == Summary - FAILURE ==
> 
>   Serious unknown changes coming with IGTPW_1302 absolutely need to be
>   verified manually.
>   
>   If you think the reported changes have nothing to do with the changes
>   introduced in IGTPW_1302, please notify your bug team to allow them
>   to document this new failure mode, which will reduce false positives in CI.
> 
>   External URL: https://patchwork.freedesktop.org/api/1.0/series/41513/revisions/2/mbox/
> 
> == Possible new issues ==
> 
>   Here are the unknown changes that may have been introduced in IGTPW_1302:
> 
>   === IGT changes ===
> 
>     ==== Possible regressions ====
> 
>     igt@kms_psr_sink_crc@dpms:
>       fi-cfl-u:           NOTRUN -> FAIL +3
>       fi-cnl-psr:         NOTRUN -> FAIL +3
> 
>     igt@kms_psr_sink_crc@sprite_plane_onoff:
>       {fi-hsw-4200u}:     NOTRUN -> FAIL +4
> 
>     igt@kms_sink_crc_basic:
>       fi-cnl-psr:         PASS -> FAIL


I pressed the re-test button yet again to doublecheck, but these must
be explained before merging.


-- 
Petri Latvala


> 
>     
> == Known issues ==
> 
>   Here are the changes found in IGTPW_1302 that come from known issues:
> 
>   === IGT changes ===
> 
>     ==== Issues hit ====
> 
>     igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
>       fi-snb-2520m:       PASS -> INCOMPLETE (fdo#103713)
>       fi-ivb-3520m:       PASS -> DMESG-WARN (fdo#106084) +2
>       fi-bxt-dsi:         PASS -> INCOMPLETE (fdo#103927)
> 
>     igt@kms_psr_sink_crc@primary_page_flip:
>       fi-skl-6600u:       NOTRUN -> DMESG-WARN (fdo#105537) +1
> 
>     igt@prime_vgem@basic-fence-flip:
>       fi-ilk-650:         PASS -> FAIL (fdo#104008)
> 
>     
>     ==== Possible fixes ====
> 
>     igt@kms_frontbuffer_tracking@basic:
>       {fi-hsw-4200u}:     DMESG-FAIL (fdo#106103) -> PASS
> 
>     
>   {name}: This element is suppressed. This means it is ignored when computing
>           the status of the difference (SUCCESS, WARNING, or FAILURE).
> 
>   fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
>   fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
>   fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
>   fdo#105537 https://bugs.freedesktop.org/show_bug.cgi?id=105537
>   fdo#106084 https://bugs.freedesktop.org/show_bug.cgi?id=106084
>   fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103
> 
> 
> == Participating hosts (38 -> 35) ==
> 
>   Missing    (3): fi-ilk-m540 fi-skl-6700hq fi-pnv-d510 
> 
> 
> == Build changes ==
> 
>     * IGT: IGT_4449 -> IGTPW_1302
> 
>   CI_DRM_4098: b0be536432b06582cd76b4b767a4acf4b20aaf10 @ git://anongit.freedesktop.org/gfx-ci/linux
>   IGTPW_1302: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1302/
>   IGT_4449: 0350f0e7f6a0e07281445fc3082aa70419f4aac7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
>   piglit_4449: ad8992d3fb27fd604b9ab15e7963c42421ced85c @ git://anongit.freedesktop.org/piglit
> 
> 
> 
> == Testlist changes ==
> 
> +igt@kms_psr_sink_crc@basic
> +igt@kms_psr_sink_crc@dpms
> +igt@kms_psr_sink_crc@no_drrs
> +igt@kms_psr_sink_crc@suspend
> -igt@kms_psr_sink_crc@cursor_mmap_gtt_waiting
> -igt@kms_psr_sink_crc@dpms_off_psr_active
> -igt@kms_psr_sink_crc@dpms_off_psr_exit
> -igt@kms_psr_sink_crc@primary_mmap_gtt_waiting
> -igt@kms_psr_sink_crc@psr_basic
> -igt@kms_psr_sink_crc@psr_drrs
> -igt@kms_psr_sink_crc@sprite_mmap_gtt_waiting
> -igt@kms_psr_sink_crc@suspend_psr_active
> -igt@kms_psr_sink_crc@suspend_psr_exit
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1302/issues.html
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,01/20] tests/psr: Print the reason for skipping when sink lacks PSR support (rev2)

[Dhinakaran Pandiyan]

On Thu, 2018-04-26 at 11:40 +0300, Petri Latvala wrote:
> On Wed, Apr 25, 2018 at 09:00:58PM +0000, Patchwork wrote:
> > 
> > == Series Details ==
> > 
> > Series: series starting with [i-g-t,01/20] tests/psr: Print the
> > reason for skipping when sink lacks PSR support (rev2)
> > URL   : https://patchwork.freedesktop.org/series/41513/
> > State : failure
> > 
> > == Summary ==
> > 
> > = CI Bug Log - changes from CI_DRM_4098 -> IGTPW_1302 =
> > 
> > == Summary - FAILURE ==
> > 
> >   Serious unknown changes coming with IGTPW_1302 absolutely need to
> > be
> >   verified manually.
> >   
> >   If you think the reported changes have nothing to do with the
> > changes
> >   introduced in IGTPW_1302, please notify your bug team to allow
> > them
> >   to document this new failure mode, which will reduce false
> > positives in CI.
> > 
> >   External URL: https://patchwork.freedesktop.org/api/1.0/series/41
> > 513/revisions/2/mbox/
> > 
> > == Possible new issues ==
> > 
> >   Here are the unknown changes that may have been introduced in
> > IGTPW_1302:
> > 
> >   === IGT changes ===
> > 
> >     ==== Possible regressions ====
> > 
> >     igt@kms_psr_sink_crc@dpms:
> >       fi-cfl-u:           NOTRUN -> FAIL +3
> >       fi-cnl-psr:         NOTRUN -> FAIL +3
These are interesting, for some reason the hardware failed to enter PSR
after the driver enabled it. However, the tests have worked on skl, kbl
and the other cfl machine. We should add these tests and then tune the
tests to get them to work reliably on these two machines.

> > 
> >     igt@kms_psr_sink_crc@sprite_plane_onoff:
> >       {fi-hsw-4200u}:     NOTRUN -> FAIL +4
Sent a kernel patch that should address this https://patchwork.freedesk
top.org/patch/221775/

> > 
> >     igt@kms_sink_crc_basic:
> >       fi-cnl-psr:         PASS -> FAIL
Isn't related to the series, sink crc as such is not very reliable.

> 
> I pressed the re-test button yet again to doublecheck, but these must
> be explained before merging.
> 
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,01/20] tests/psr: Print the reason for skipping when sink lacks PSR support (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,01/20] tests/psr: Print the reason for skipping when sink lacks PSR support (rev2)
URL   : https://patchwork.freedesktop.org/series/41513/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4101 -> IGTPW_1304 =

== Summary - FAILURE ==

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

  External URL: https://patchwork.freedesktop.org/api/1.0/series/41513/revisions/2/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@kms_psr_sink_crc@dpms:
      fi-cfl-u:           NOTRUN -> FAIL +3
      fi-cnl-psr:         NOTRUN -> FAIL +3

    igt@kms_psr_sink_crc@sprite_plane_onoff:
      fi-hsw-4200u:       NOTRUN -> FAIL +4

    igt@kms_sink_crc_basic:
      fi-cnl-psr:         PASS -> FAIL

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ringfill@basic-default-hang:
      fi-pnv-d510:        NOTRUN -> DMESG-WARN (fdo#101600)

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
      fi-hsw-4200u:       PASS -> FAIL (fdo#103481)

    igt@kms_psr_sink_crc@primary_page_flip:
      fi-skl-6600u:       NOTRUN -> DMESG-WARN (fdo#105537) +1

    
  fdo#101600 https://bugs.freedesktop.org/show_bug.cgi?id=101600
  fdo#103481 https://bugs.freedesktop.org/show_bug.cgi?id=103481
  fdo#105537 https://bugs.freedesktop.org/show_bug.cgi?id=105537


== Participating hosts (37 -> 35) ==

  Additional (2): fi-bxt-dsi fi-pnv-d510 
  Missing    (4): fi-ctg-p8600 fi-ilk-m540 fi-bdw-5557u fi-skl-6700hq 


== Build changes ==

    * IGT: IGT_4449 -> IGTPW_1304

  CI_DRM_4101: c5d4ac88496d76c1a797724ce5bfebc806d63e2b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1304: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1304/
  IGT_4449: 0350f0e7f6a0e07281445fc3082aa70419f4aac7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4449: ad8992d3fb27fd604b9ab15e7963c42421ced85c @ git://anongit.freedesktop.org/piglit



== Testlist changes ==

+igt@kms_psr_sink_crc@basic
+igt@kms_psr_sink_crc@dpms
+igt@kms_psr_sink_crc@no_drrs
+igt@kms_psr_sink_crc@suspend
-igt@kms_psr_sink_crc@cursor_mmap_gtt_waiting
-igt@kms_psr_sink_crc@dpms_off_psr_active
-igt@kms_psr_sink_crc@dpms_off_psr_exit
-igt@kms_psr_sink_crc@primary_mmap_gtt_waiting
-igt@kms_psr_sink_crc@psr_basic
-igt@kms_psr_sink_crc@psr_drrs
-igt@kms_psr_sink_crc@sprite_mmap_gtt_waiting
-igt@kms_psr_sink_crc@suspend_psr_active
-igt@kms_psr_sink_crc@suspend_psr_exit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1304/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev