[PATCH IGT 00/11] IGT PSR Fix-ups

[PATCH IGT 00/11] IGT PSR Fix-ups

[Jim Bride]

These patches, along with the kernel series at
https://patchwork.freedesktop.org/series/27137/ allow our PSR
IGT tests to run more predictably on HSW, BDW, and SKL.  These
patches depend on the kernel series in order to run properly.  On
the systems I have available the following sets of tests run and pass.
I still see some very sporadic (every few hundred tests executions or so)
failures to read the sink CRC.

HSW:
	* kms_psr_sink_crc (all)
	* kms_frontbuffer_tracking (subtests psr-1p*, my system doesn't
	  support FBC)
	* kms_fbcon_fbt (subtests psr*)

BDW and SKL:
        * kms_psr_sink_crc (all)
    	* kms_frontbuffer_tracking (subtests psr-1p* and fbcpsr-1p*)
	* kms_fbcon_fbt (subtests psr*)

Note: Based on review feedback the changes in this series were
      substantially restructured vs. previous versions.

Jim Bride (11):
  tests/kms_psr_sink_crc: Change assert_or_manual() to a macro
  tests/kms_frontbuffer_tracking: Fix multidraw subtest
  tests/kms_frontbuffer_tracking: Remove unneeded HSW work-around.
  lib: Add utility functions to enable and disable PSR.
  lib: Add library functions for PSR source and sink support
  lib: Add function to return PSR active status
  lib: Add igt_psr_await_status() and modify tests to use it.
  lib: Add igt_psr_print_status() and change tests to use it.
  lib: Add igt_psr_valid_connector() and change tests to use it.
  lib: Add igt_psr_find_good_mode() and modify tests to use it.
  lib: Add igt_psr_get_sink_crc() and change tests to use it.

 lib/Makefile.sources             |   2 +
 lib/igt.h                        |   1 +
 lib/igt_psr.c                    | 233 +++++++++++++++++++++++++++++++++++++++
 lib/igt_psr.h                    |  42 +++++++
 tests/kms_fbcon_fbt.c            |  42 +++----
 tests/kms_frontbuffer_tracking.c |  83 +++++---------
 tests/kms_psr_sink_crc.c         | 148 ++++++++++++-------------
 7 files changed, 398 insertions(+), 153 deletions(-)
 create mode 100644 lib/igt_psr.c
 create mode 100644 lib/igt_psr.h

-- 
2.7.4

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

[PATCH IGT 01/11] tests/kms_psr_sink_crc: Change assert_or_manual() to a macro

[Jim Bride]

Make assert_or_manual() a macro so that we get accurate line number
information when this assertion fails.

v2: Rebase

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
---
 tests/kms_psr_sink_crc.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index bd3fa5e..1a03719 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -278,11 +278,11 @@ static bool is_green(char *crc)
 		(bh & mask) == 0);
 }
 
-static void assert_or_manual(bool condition, const char *expected)
-{
-	igt_debug_manual_check("no-crc", expected);
-	igt_assert(igt_interactive_debug || condition);
-}
+#define assert_or_manual(condition, expected)             \
+do {                                                      \
+	igt_debug_manual_check("no-crc", expected);       \
+	igt_assert(igt_interactive_debug || condition);   \
+} while (0)
 
 static void run_test(data_t *data)
 {
-- 
2.7.4

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

[PATCH IGT 02/11] tests/kms_frontbuffer_tracking: Fix multidraw subtest

[Jim Bride]

The multidraw subtest was not taking whether or not the GEM buffer had
ever been in write-combining mode when checking for PSR state, so fix
that.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
---
 tests/kms_frontbuffer_tracking.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index c24e4a8..6f8fd20 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -2140,7 +2140,8 @@ static void multidraw_subtest(const struct test_mode *t)
 				assertions = used_method != IGT_DRAW_MMAP_GTT ?
 					     ASSERT_LAST_ACTION_CHANGED :
 					     ASSERT_NO_ACTION_CHANGE;
-				if (op_disables_psr(t, used_method))
+				if (op_disables_psr(t, used_method) &&
+				    !wc_used)
 					assertions |= ASSERT_PSR_DISABLED;
 
 				do_assertions(assertions);
-- 
2.7.4

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

[PATCH IGT 03/11] tests/kms_frontbuffer_tracking: Remove unneeded HSW work-around.

[Jim Bride]

This work-around actually causes issues on HSW now.  Without this
code in-place I'm seeing good results on HSW.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
---
 tests/kms_frontbuffer_tracking.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index 6f8fd20..2e9a169 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -436,12 +436,6 @@ static bool connector_get_mode(drmModeConnectorPtr c, drmModeModeInfoPtr *mode)
 	else
 		*mode = &c->modes[0];
 
-	 /* On HSW the CRC WA is so awful that it makes you think everything is
-	  * bugged. */
-	if (IS_HASWELL(intel_get_drm_devid(drm.fd)) &&
-	    c->connector_type == DRM_MODE_CONNECTOR_eDP)
-		*mode = &std_1024_mode;
-
 	return true;
 }
 
-- 
2.7.4

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

[PATCH IGT 04/11] lib: Add utility functions to enable and disable PSR.

[Jim Bride]

Create files to contain PSR-specific IGT functions and add macros
to enable and disable PSR.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
---
 lib/Makefile.sources             |  1 +
 lib/igt.h                        |  1 +
 lib/igt_psr.h                    | 30 ++++++++++++++++++++++++++++++
 tests/kms_fbcon_fbt.c            |  2 +-
 tests/kms_frontbuffer_tracking.c |  4 ++--
 tests/kms_psr_sink_crc.c         |  8 +++++---
 6 files changed, 40 insertions(+), 6 deletions(-)
 create mode 100644 lib/igt_psr.h

diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index 53fdb54..7ec6711 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -83,6 +83,7 @@ lib_source_list =	 	\
 	uwildmat/uwildmat.c	\
 	igt_kmod.c		\
 	igt_kmod.h		\
+	igt_psr.h		\
 	$(NULL)
 
 .PHONY: version.h.tmp
diff --git a/lib/igt.h b/lib/igt.h
index a069deb..807ed99 100644
--- a/lib/igt.h
+++ b/lib/igt.h
@@ -38,6 +38,7 @@
 #include "igt_kms.h"
 #include "igt_pm.h"
 #include "igt_stats.h"
+#include "igt_psr.h"
 #ifdef HAVE_CHAMELIUM
 #include "igt_chamelium.h"
 #endif
diff --git a/lib/igt_psr.h b/lib/igt_psr.h
new file mode 100644
index 0000000..71b1233
--- /dev/null
+++ b/lib/igt_psr.h
@@ -0,0 +1,30 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#ifndef IGT_PSR_H
+#define IGT_PSR_H
+
+#define igt_psr_enable() igt_set_module_param_int("enable_psr", 1)
+#define igt_psr_disable() igt_set_module_param_int("enable_psr", 0)
+
+#endif /* IGT_PSR_H */
diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
index d009091..4b9cb43 100644
--- a/tests/kms_fbcon_fbt.c
+++ b/tests/kms_fbcon_fbt.c
@@ -193,7 +193,7 @@ struct feature {
 static void disable_features(void)
 {
 	igt_set_module_param_int(fbc.param_name, 0);
-	igt_set_module_param_int(psr.param_name, 0);
+	igt_psr_disable();
 }
 
 static void subtest(struct feature *feature, bool suspend)
diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index 2e9a169..3c30745 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -927,8 +927,8 @@ static bool psr_wait_until_enabled(void)
 
 #define fbc_enable() igt_set_module_param_int("enable_fbc", 1)
 #define fbc_disable() igt_set_module_param_int("enable_fbc", 0)
-#define psr_enable() igt_set_module_param_int("enable_psr", 1)
-#define psr_disable() igt_set_module_param_int("enable_psr", 0)
+#define psr_enable() igt_psr_enable()
+#define psr_disable() igt_psr_disable()
 
 static void get_sink_crc(sink_crc_t *crc, bool mandatory)
 {
diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 1a03719..c0404b2 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -507,9 +507,11 @@ 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 ?
-					 0 : 1);
-
+		if (running_with_psr_disabled)
+			igt_psr_disable();
+		else
+			igt_psr_enable();
+		
 		igt_skip_on(!psr_possible(&data));
 
 		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
-- 
2.7.4

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

[PATCH IGT 05/11] lib: Add library functions for PSR source and sink support

[Jim Bride]

Add functions to tell whether the source and sink support PSR as well
as a function to determine whether PSR is possible (both source and
sink support PSR.)  Also modify the PSR tests to use these functions.

Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
---
 lib/Makefile.sources             |  1 +
 lib/igt_psr.c                    | 85 ++++++++++++++++++++++++++++++++++++++++
 lib/igt_psr.h                    |  4 ++
 tests/kms_fbcon_fbt.c            |  5 +--
 tests/kms_frontbuffer_tracking.c |  5 +--
 tests/kms_psr_sink_crc.c         |  6 +--
 6 files changed, 93 insertions(+), 13 deletions(-)
 create mode 100644 lib/igt_psr.c

diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index 7ec6711..6a73c8c 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -83,6 +83,7 @@ lib_source_list =	 	\
 	uwildmat/uwildmat.c	\
 	igt_kmod.c		\
 	igt_kmod.h		\
+	igt_psr.c		\
 	igt_psr.h		\
 	$(NULL)
 
diff --git a/lib/igt_psr.c b/lib/igt_psr.c
new file mode 100644
index 0000000..c5c9b4c
--- /dev/null
+++ b/lib/igt_psr.c
@@ -0,0 +1,85 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include "igt.h"
+#include <stdio.h>
+#include <stdbool.h>
+#include <sys/types.h>
+#include <string.h>
+#include <fcntl.h>
+
+/**
+ * SECTION:igt_psr
+ * @short_description: Panel Self Refresh helpers
+ * @title: Panel Self Refresh
+ * @include: igt.h
+ *
+ * This library provides various helpers to enable Panel Self Refresh,
+ * as well as to check the state of PSR on the system (enabled vs.
+ * disabled, active vs. inactive) or to wait for PSR to be active
+ * or inactive.
+ */
+
+#define BUFSIZE 512 /* Size of char buffer to read debugfs data into */
+
+/**
+ * igt_psr_source_support:
+ *
+ * Returns true if the source supports PSR.
+ */
+bool igt_psr_source_support(int fd)
+{
+	char buf[BUFSIZE];
+
+	igt_debugfs_read(fd, "i915_edp_psr_status", buf);
+
+	return !!strstr(buf, "Source_OK: yes\n");
+}
+
+
+/**
+ * igt_psr_sink_support:
+ *
+ * Returns true if the current eDP panel supports PSR.
+ */
+bool igt_psr_sink_support(int fd)
+{
+	char buf[BUFSIZE];
+
+	igt_debugfs_read(fd, "i915_edp_psr_status", buf);
+	return !!strstr(buf, "Sink_Support: yes\n");
+}
+
+/**
+ * igt_psr_possible:
+ *
+ * Returns true if both the source and sink support PSR.
+ */
+bool igt_psr_possible(int fd)
+{
+	char buf[BUFSIZE];
+
+	igt_debugfs_read(fd, "i915_edp_psr_status", buf);
+
+	return igt_psr_source_support(fd) && igt_psr_sink_support(fd);
+}
diff --git a/lib/igt_psr.h b/lib/igt_psr.h
index 71b1233..98774c8 100644
--- a/lib/igt_psr.h
+++ b/lib/igt_psr.h
@@ -27,4 +27,8 @@
 #define igt_psr_enable() igt_set_module_param_int("enable_psr", 1)
 #define igt_psr_disable() igt_set_module_param_int("enable_psr", 0)
 
+bool igt_psr_source_support(int fd);
+bool igt_psr_sink_support(int fd);
+bool igt_psr_possible(int fd);
+
 #endif /* IGT_PSR_H */
diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
index 4b9cb43..d0ed9f5 100644
--- a/tests/kms_fbcon_fbt.c
+++ b/tests/kms_fbcon_fbt.c
@@ -149,10 +149,7 @@ static void set_mode_for_one_screen(struct drm_info *drm, struct igt_fb *fb,
 
 static bool psr_supported_on_chipset(int fd)
 {
-	char buf[256];
-
-	igt_debugfs_read(fd, "i915_edp_psr_status", buf);
-	return strstr(buf, "Sink_Support: yes\n");
+	return igt_psr_sink_support(fd);
 }
 
 static bool connector_can_psr(drmModeConnectorPtr connector)
diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index 3c30745..347dcc1 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -1543,10 +1543,7 @@ static void teardown_fbc(void)
 
 static bool psr_sink_has_support(void)
 {
-	char buf[256];
-
-	debugfs_read("i915_edp_psr_status", buf);
-	return strstr(buf, "Sink_Support: yes\n");
+	return igt_psr_sink_support(drm.fd);
 }
 
 static void setup_psr(void)
diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index c0404b2..952a109 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -194,12 +194,8 @@ static void fill_render(data_t *data, uint32_t handle, unsigned char color)
 
 static bool psr_possible(data_t *data)
 {
-	char buf[512];
-
-	igt_debugfs_read(data->drm_fd, "i915_edp_psr_status", buf);
-
 	return running_with_psr_disabled ||
-		strstr(buf, "Sink_Support: yes\n");
+		igt_psr_possible(data->drm_fd);
 }
 
 static bool psr_active(data_t *data)
-- 
2.7.4

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

[PATCH IGT 06/11] lib: Add function to return PSR active status

[Jim Bride]

Add igt_psr_active() which returns whether PSR is active or not and modify
tests to use this function.

Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
---
 lib/igt_psr.c                    | 19 +++++++++++++++++++
 lib/igt_psr.h                    |  1 +
 tests/kms_fbcon_fbt.c            |  5 +----
 tests/kms_frontbuffer_tracking.c |  6 +-----
 tests/kms_psr_sink_crc.c         |  5 +----
 5 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index c5c9b4c..d849961 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -83,3 +83,22 @@ bool igt_psr_possible(int fd)
 
 	return igt_psr_source_support(fd) && igt_psr_sink_support(fd);
 }
+
+/**
+ * igt_psr_active:
+ *
+ * Returns true if PSR is active on the panel.
+ */
+bool igt_psr_active(int fd)
+{
+	char buf[BUFSIZE];
+	bool actret = false;
+	bool hwactret = false;
+
+	igt_debugfs_read(fd, "i915_edp_psr_status", buf);
+	hwactret = (strstr(buf, "HW Enabled & Active bit: yes\n") != NULL);
+	actret = (strstr(buf, "Active: yes\n") != NULL);
+	igt_debug("hwactret: %s actret: %s\n", hwactret ? "true" : "false",
+		 actret ? "true" : "false");
+	return hwactret && actret;
+}
diff --git a/lib/igt_psr.h b/lib/igt_psr.h
index 98774c8..b678329 100644
--- a/lib/igt_psr.h
+++ b/lib/igt_psr.h
@@ -30,5 +30,6 @@
 bool igt_psr_source_support(int fd);
 bool igt_psr_sink_support(int fd);
 bool igt_psr_possible(int fd);
+bool igt_psr_active(int fd);
 
 #endif /* IGT_PSR_H */
diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
index d0ed9f5..41ab36d 100644
--- a/tests/kms_fbcon_fbt.c
+++ b/tests/kms_fbcon_fbt.c
@@ -159,10 +159,7 @@ static bool connector_can_psr(drmModeConnectorPtr connector)
 
 static bool psr_is_enabled(int fd)
 {
-	char buf[256];
-
-	igt_debugfs_read(fd, "i915_edp_psr_status", buf);
-	return strstr(buf, "\nActive: yes\n");
+	return igt_psr_active(fd);
 }
 
 static bool psr_wait_until_enabled(int fd)
diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index 347dcc1..b202297 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -800,11 +800,7 @@ static void fbc_print_status(void)
 
 static bool psr_is_enabled(void)
 {
-	char buf[256];
-
-	debugfs_read("i915_edp_psr_status", buf);
-	return strstr(buf, "\nActive: yes\n") &&
-	       strstr(buf, "\nHW Enabled & Active bit: yes\n");
+	return igt_psr_active(drm.fd);
 }
 
 static void psr_print_status(void)
diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 952a109..0c27fc7 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -200,12 +200,9 @@ static bool psr_possible(data_t *data)
 
 static bool psr_active(data_t *data)
 {
-	char buf[512];
-
-	igt_debugfs_read(data->drm_fd, "i915_edp_psr_status", buf);
 
 	return running_with_psr_disabled ||
-		strstr(buf, "HW Enabled & Active bit: yes\n");
+		igt_psr_active(data->drm_fd);
 }
 
 static bool wait_psr_entry(data_t *data)
-- 
2.7.4

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

[PATCH IGT 07/11] lib: Add igt_psr_await_status() and modify tests to use it.

[Jim Bride]

Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
---
 lib/igt_psr.c                    | 12 ++++++++++++
 lib/igt_psr.h                    |  1 +
 tests/kms_fbcon_fbt.c            | 28 ++++++++++++----------------
 tests/kms_frontbuffer_tracking.c | 17 +++++------------
 tests/kms_psr_sink_crc.c         | 16 ++--------------
 5 files changed, 32 insertions(+), 42 deletions(-)

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index d849961..d27c32a 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -102,3 +102,15 @@ bool igt_psr_active(int fd)
 		 actret ? "true" : "false");
 	return hwactret && actret;
 }
+
+/**
+ * igt_psr_await_status:
+ * @active: A boolean that causes the function to wait for PSR to activate
+ *          if set to true, or to wait for PSR to deactivate if false.
+ *
+ * Returns true if the requested condition is met.
+ */
+bool igt_psr_await_status(int fd, bool active)
+{
+	return igt_wait(igt_psr_active(fd) == active, 5000, 1);
+}
diff --git a/lib/igt_psr.h b/lib/igt_psr.h
index b678329..3c355e0 100644
--- a/lib/igt_psr.h
+++ b/lib/igt_psr.h
@@ -31,5 +31,6 @@ bool igt_psr_source_support(int fd);
 bool igt_psr_sink_support(int fd);
 bool igt_psr_possible(int fd);
 bool igt_psr_active(int fd);
+bool igt_psr_await_status(int fd, bool active);
 
 #endif /* IGT_PSR_H */
diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
index 41ab36d..cba632e 100644
--- a/tests/kms_fbcon_fbt.c
+++ b/tests/kms_fbcon_fbt.c
@@ -103,8 +103,9 @@ static bool fbc_is_enabled(int fd)
 	return strstr(buf, "FBC enabled\n");
 }
 
-static bool fbc_wait_until_enabled(int fd)
+static bool fbc_await_status(int fd, bool enabled)
 {
+	igt_assert(enabled);
 	return igt_wait(fbc_is_enabled(fd), 5000, 1);
 }
 
@@ -157,29 +158,24 @@ static bool connector_can_psr(drmModeConnectorPtr connector)
 	return (connector->connector_type == DRM_MODE_CONNECTOR_eDP);
 }
 
-static bool psr_is_enabled(int fd)
+static bool psr_await_status(int fd, bool enabled)
 {
-	return igt_psr_active(fd);
-}
-
-static bool psr_wait_until_enabled(int fd)
-{
-	return igt_wait(psr_is_enabled(fd), 5000, 1);
+	return igt_psr_await_status(fd, enabled);
 }
 
 struct feature {
 	bool (*supported_on_chipset)(int fd);
-	bool (*wait_until_enabled)(int fd);
+	bool (*await_status)(int fd, bool enabled);
 	bool (*connector_possible_fn)(drmModeConnectorPtr connector);
 	const char *param_name;
 } fbc = {
 	.supported_on_chipset = fbc_supported_on_chipset,
-	.wait_until_enabled = fbc_wait_until_enabled,
+	.await_status = fbc_await_status,
 	.connector_possible_fn = connector_can_fbc,
 	.param_name = "enable_fbc",
 }, psr = {
 	.supported_on_chipset = psr_supported_on_chipset,
-	.wait_until_enabled = psr_wait_until_enabled,
+	.await_status = psr_await_status,
 	.connector_possible_fn = connector_can_psr,
 	.param_name = "enable_psr",
 };
@@ -204,17 +200,17 @@ static void subtest(struct feature *feature, bool suspend)
 
 	kmstest_unset_all_crtcs(drm.fd, drm.res);
 	wait_user("Modes unset.");
-	igt_assert(!feature->wait_until_enabled(drm.fd));
+	igt_assert(feature->await_status(drm.fd, false));
 
 	set_mode_for_one_screen(&drm, &fb, feature->connector_possible_fn);
 	wait_user("Screen set.");
-	igt_assert(feature->wait_until_enabled(drm.fd));
+	igt_assert(feature->await_status(drm.fd, true));
 
 	if (suspend) {
 		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
 					      SUSPEND_TEST_NONE);
 		sleep(5);
-		igt_assert(feature->wait_until_enabled(drm.fd));
+		igt_assert(feature->await_status(drm.fd, true));
 	}
 
 	igt_remove_fb(drm.fd, &fb);
@@ -224,13 +220,13 @@ static void subtest(struct feature *feature, bool suspend)
 	sleep(3);
 
 	wait_user("Back to fbcon.");
-	igt_assert(!feature->wait_until_enabled(drm.fd));
+	igt_assert(feature->await_status(drm.fd, false));
 
 	if (suspend) {
 		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
 					      SUSPEND_TEST_NONE);
 		sleep(5);
-		igt_assert(!feature->wait_until_enabled(drm.fd));
+		igt_assert(feature->await_status(drm.fd, false));
 	}
 }
 
diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index b202297..4d20899 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -798,11 +798,6 @@ static void fbc_print_status(void)
 	igt_info("FBC status:\n%s\n", buf);
 }
 
-static bool psr_is_enabled(void)
-{
-	return igt_psr_active(drm.fd);
-}
-
 static void psr_print_status(void)
 {
 	char buf[256];
@@ -916,11 +911,6 @@ static bool fbc_wait_until_enabled(void)
 	return igt_wait(fbc_is_enabled(), 2000, 1);
 }
 
-static bool psr_wait_until_enabled(void)
-{
-	return igt_wait(psr_is_enabled(), 5000, 1);
-}
-
 #define fbc_enable() igt_set_module_param_int("enable_fbc", 1)
 #define fbc_disable() igt_set_module_param_int("enable_fbc", 0)
 #define psr_enable() igt_psr_enable()
@@ -1704,12 +1694,15 @@ static int adjust_assertion_flags(const struct test_mode *t, int flags)
 	}								\
 									\
 	if (flags_ & ASSERT_PSR_ENABLED) {				\
-		if (!psr_wait_until_enabled()) {			\
+		if (!igt_psr_await_status(drm.fd, true)) {		\
 			psr_print_status();				\
 			igt_assert_f(false, "PSR disabled\n");		\
 		}							\
 	} else if (flags_ & ASSERT_PSR_DISABLED) {			\
-		igt_assert(!psr_wait_until_enabled());			\
+		if (!igt_psr_await_status(drm.fd, false)) {             \
+			psr_print_status();                             \
+			igt_assert_f(false, "PSR enabled\n");           \
+		}                                                       \
 	}								\
 } while (0)
 
diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 0c27fc7..c9f7993 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -198,22 +198,10 @@ static bool psr_possible(data_t *data)
 		igt_psr_possible(data->drm_fd);
 }
 
-static bool psr_active(data_t *data)
-{
-
-	return running_with_psr_disabled ||
-		igt_psr_active(data->drm_fd);
-}
-
 static bool wait_psr_entry(data_t *data)
 {
-	int timeout = 5;
-	while (timeout--) {
-		if (psr_active(data))
-			return true;
-		sleep(1);
-	}
-	return false;
+	return running_with_psr_disabled ||
+		igt_psr_await_status(data->drm_fd, true);
 }
 
 static void get_sink_crc(data_t *data, char *crc) {
-- 
2.7.4

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

[PATCH IGT 08/11] lib: Add igt_psr_print_status() and change tests to use it.

[Jim Bride]

Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
---
 lib/igt_psr.c                    | 13 +++++++++++++
 lib/igt_psr.h                    |  1 +
 tests/kms_frontbuffer_tracking.c |  5 +----
 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index d27c32a..8dda659 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -114,3 +114,16 @@ bool igt_psr_await_status(int fd, bool active)
 {
 	return igt_wait(igt_psr_active(fd) == active, 5000, 1);
 }
+
+/**
+ * igt_psr_print_status:
+ *
+ * Dumps the contents of i915_edp_psr_status from debugfs.
+ */
+void igt_psr_print_status(int fd)
+{
+        char buf[BUFSIZE];
+
+        igt_debugfs_read(fd, "i915_edp_psr_status", buf);
+        igt_info("PSR status:\n%s\n", buf);
+}
diff --git a/lib/igt_psr.h b/lib/igt_psr.h
index 3c355e0..d6db297 100644
--- a/lib/igt_psr.h
+++ b/lib/igt_psr.h
@@ -32,5 +32,6 @@ bool igt_psr_sink_support(int fd);
 bool igt_psr_possible(int fd);
 bool igt_psr_active(int fd);
 bool igt_psr_await_status(int fd, bool active);
+void igt_psr_print_status(int fd);
 
 #endif /* IGT_PSR_H */
diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index 4d20899..ee611b9 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -800,10 +800,7 @@ static void fbc_print_status(void)
 
 static void psr_print_status(void)
 {
-	char buf[256];
-
-	debugfs_read("i915_edp_psr_status", buf);
-	igt_info("PSR status:\n%s\n", buf);
+	igt_psr_print_status(drm.fd);
 }
 
 static struct timespec fbc_get_last_action(void)
-- 
2.7.4

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

[PATCH IGT 09/11] lib: Add igt_psr_valid_connector() and change tests to use it.

[Jim Bride]

Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
---
 lib/igt_psr.c                    | 11 +++++++++++
 lib/igt_psr.h                    |  1 +
 tests/kms_frontbuffer_tracking.c | 12 +++++++-----
 tests/kms_psr_sink_crc.c         |  2 +-
 4 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index 8dda659..178842e 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -127,3 +127,14 @@ void igt_psr_print_status(int fd)
         igt_debugfs_read(fd, "i915_edp_psr_status", buf);
         igt_info("PSR status:\n%s\n", buf);
 }
+
+/**
+ * igt_psr_valid_connector:
+ * @connector: a drmModeConnector pointer to check
+ *
+ * Returns true if connector is an eDP connector.
+ */
+bool igt_psr_valid_connector(drmModeConnectorPtr connector)
+{
+	return (connector->connector_type == DRM_MODE_CONNECTOR_eDP);
+}
diff --git a/lib/igt_psr.h b/lib/igt_psr.h
index d6db297..08bccef 100644
--- a/lib/igt_psr.h
+++ b/lib/igt_psr.h
@@ -33,5 +33,6 @@ bool igt_psr_possible(int fd);
 bool igt_psr_active(int fd);
 bool igt_psr_await_status(int fd, bool active);
 void igt_psr_print_status(int fd);
+bool igt_psr_valid_connector(drmModeConnectorPtr connector);
 
 #endif /* IGT_PSR_H */
diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index ee611b9..6f52c49 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -424,11 +424,12 @@ static void init_mode_params(struct modeset_params *params, uint32_t crtc_id,
 static bool connector_get_mode(drmModeConnectorPtr c, drmModeModeInfoPtr *mode)
 {
 	*mode = NULL;
+	bool valid = igt_psr_valid_connector(c);
 
 	if (c->connection != DRM_MODE_CONNECTED || !c->count_modes)
 		return false;
 
-	if (c->connector_type == DRM_MODE_CONNECTOR_eDP && opt.no_edp)
+	if (valid && opt.no_edp)
 		return false;
 
 	if (opt.small_modes)
@@ -461,7 +462,7 @@ static bool find_connector(bool edp_only, bool pipe_a, uint32_t forbidden_id,
 	for (i = 0; i < drm.res->count_connectors; i++) {
 		c = drm.connectors[i];
 
-		if (edp_only && c->connector_type != DRM_MODE_CONNECTOR_eDP)
+		if (edp_only && !igt_psr_valid_connector(c))
 			continue;
 		if (pipe_a && !connector_supports_pipe_a(c))
 			continue;
@@ -1395,7 +1396,7 @@ static void setup_sink_crc(void)
 	drmModeConnectorPtr c;
 
 	c = get_connector(prim_mode_params.connector_id);
-	if (c->connector_type != DRM_MODE_CONNECTOR_eDP) {
+	if (!igt_psr_valid_connector(c)) {
 		igt_info("Sink CRC not supported: primary screen is not eDP\n");
 		sink_crc.supported = false;
 		return;
@@ -1531,8 +1532,9 @@ static bool psr_sink_has_support(void)
 
 static void setup_psr(void)
 {
-	if (get_connector(prim_mode_params.connector_id)->connector_type !=
-	    DRM_MODE_CONNECTOR_eDP) {
+	drmModeConnectorPtr c = get_connector(prim_mode_params.connector_id);
+	
+	if (!igt_psr_valid_connector(c)) {
 		igt_info("Can't test PSR: no usable eDP screen.\n");
 		return;
 	}
diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index c9f7993..da8c173 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -103,7 +103,7 @@ static void setup_output(data_t *data)
 	for_each_pipe_with_valid_output(display, pipe, output) {
 		drmModeConnectorPtr c = output->config.connector;
 
-		if (c->connector_type != DRM_MODE_CONNECTOR_eDP)
+		if (!igt_psr_valid_connector(c))
 			continue;
 
 		igt_output_set_pipe(output, pipe);
-- 
2.7.4

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

[PATCH IGT 10/11] lib: Add igt_psr_find_good_mode() and modify tests to use it.

[Jim Bride]

Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
---
 lib/igt_psr.c                    | 34 +++++++++++++++++++++++++++++++++
 lib/igt_psr.h                    |  2 ++
 tests/kms_fbcon_fbt.c            |  4 ++++
 tests/kms_frontbuffer_tracking.c |  2 +-
 tests/kms_psr_sink_crc.c         | 41 ++++++++++++++++++++++++++++++----------
 5 files changed, 72 insertions(+), 11 deletions(-)

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index 178842e..2fda467 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -138,3 +138,37 @@ bool igt_psr_valid_connector(drmModeConnectorPtr connector)
 {
 	return (connector->connector_type == DRM_MODE_CONNECTOR_eDP);
 }
+
+/**
+ * igt_psr_find_good_mode
+ * @connector: a drmModeConnector pointer to find the mode on
+ * @mode: a drmModeModePtr pointer that is set to the matching mode
+ *
+ * Returns true (and populates *mode with the match) if a valid
+ * PSR mdoe is found, and false otherwise.
+ */
+bool igt_psr_find_good_mode(drmModeConnectorPtr connector,
+			    drmModeModeInfoPtr *mode)
+{
+	int i;
+
+	if (!connector->count_modes) {
+		igt_warn("no modes for connector %d.\n",
+			 connector->connector_id);
+		return false;
+	} else {
+		igt_debug("Connector has %d modes.\n", connector->count_modes);
+	}
+
+	for (i = 0; i < connector->count_modes; i++) {
+		if ((connector->modes[i].vtotal -
+		     connector->modes[i].vdisplay) >= 36) {
+			igt_debug("Mode %d good for PSR.\n", i);
+			*mode = &(connector->modes[i]);
+			return true;
+		} else {
+			igt_debug("Throwing out mode %d\n", i);
+		}
+	}
+	return false;
+}
diff --git a/lib/igt_psr.h b/lib/igt_psr.h
index 08bccef..f4c0a6b 100644
--- a/lib/igt_psr.h
+++ b/lib/igt_psr.h
@@ -34,5 +34,7 @@ bool igt_psr_active(int fd);
 bool igt_psr_await_status(int fd, bool active);
 void igt_psr_print_status(int fd);
 bool igt_psr_valid_connector(drmModeConnectorPtr connector);
+bool igt_psr_find_good_mode(drmModeConnectorPtr connector,
+			    drmModeModeInfoPtr *mode);
 
 #endif /* IGT_PSR_H */
diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
index cba632e..32f363f 100644
--- a/tests/kms_fbcon_fbt.c
+++ b/tests/kms_fbcon_fbt.c
@@ -125,6 +125,10 @@ static void set_mode_for_one_screen(struct drm_info *drm, struct igt_fb *fb,
 
 		if (c->connection == DRM_MODE_CONNECTED && c->count_modes &&
 		    connector_possible(c)) {
+			if (c->connector_type == DRM_MODE_CONNECTOR_eDP) {
+				if (igt_psr_find_good_mode(c, &mode))
+					break;
+			}
 			mode = &c->modes[0];
 			break;
 		}
diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index 6f52c49..fe50cc7 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -435,7 +435,7 @@ static bool connector_get_mode(drmModeConnectorPtr c, drmModeModeInfoPtr *mode)
 	if (opt.small_modes)
 		*mode = get_connector_smallest_mode(c);
 	else
-		*mode = &c->modes[0];
+		igt_assert(igt_psr_find_good_mode(c, mode));
 
 	return true;
 }
diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index da8c173..eb1bbb3 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -70,7 +70,7 @@ typedef struct {
 	uint32_t crtc_id;
 	igt_display_t display;
 	drm_intel_bufmgr *bufmgr;
-	struct igt_fb fb_green, fb_white;
+	struct igt_fb fb_green, fb_white, fb_blue;
 	igt_plane_t *primary, *sprite, *cursor;
 	int mod_size;
 	int mod_stride;
@@ -99,6 +99,7 @@ static void setup_output(data_t *data)
 	igt_display_t *display = &data->display;
 	igt_output_t *output;
 	enum pipe pipe;
+	drmModeModeInfoPtr mode = NULL;
 
 	for_each_pipe_with_valid_output(display, pipe, output) {
 		drmModeConnectorPtr c = output->config.connector;
@@ -106,10 +107,14 @@ static void setup_output(data_t *data)
 		if (!igt_psr_valid_connector(c))
 			continue;
 
+		if (!igt_psr_find_good_mode(c, &mode))
+			continue;
+		igt_assert(mode != NULL);
+		igt_output_override_mode(output, mode);
 		igt_output_set_pipe(output, pipe);
 		data->crtc_id = output->config.crtc->crtc_id;
 		data->output = output;
-		data->mode = igt_output_get_mode(output);
+		data->mode = &output->override_mode;
 
 		return;
 	}
@@ -119,10 +124,33 @@ static void display_init(data_t *data)
 {
 	igt_display_init(&data->display, data->drm_fd);
 	setup_output(data);
+
+		/* We need to be able to do a modeset before we enable PSR to
+	 * ensure that we are running at a mode such that PSR setup can
+	 * complete within a single vblank interval.
+	 */
+	igt_debug("Selected mode:\n");
+	kmstest_dump_mode(data->mode);
+	igt_create_color_fb(data->drm_fd,
+			    data->mode->hdisplay, data->mode->vdisplay,
+			    DRM_FORMAT_XRGB8888,
+			    LOCAL_I915_FORMAT_MOD_X_TILED,
+			    0.0, .0, 1.0,
+			    &data->fb_blue);
+
+	data->primary = igt_output_get_plane_type(data->output,
+						  DRM_PLANE_TYPE_PRIMARY);
+	igt_plane_set_fb(data->primary, &data->fb_blue);
+	igt_display_commit(&data->display);
+	if (running_with_psr_disabled)
+		igt_psr_disable();
+	else
+		igt_psr_enable();
 }
 
 static void display_fini(data_t *data)
 {
+	igt_output_override_mode(data->output, NULL);
 	igt_display_fini(&data->display);
 }
 
@@ -487,19 +515,12 @@ int main(int argc, char *argv[])
 		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
 		kmstest_set_vt_graphics_mode();
 		data.devid = intel_get_drm_devid(data.drm_fd);
-
-		if (running_with_psr_disabled)
-			igt_psr_disable();
-		else
-			igt_psr_enable();
-		
-		igt_skip_on(!psr_possible(&data));
-
 		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
 		igt_assert(data.bufmgr);
 		drm_intel_bufmgr_gem_enable_reuse(data.bufmgr);
 
 		display_init(&data);
+		igt_skip_on(!psr_possible(&data));
 	}
 
 	igt_subtest("psr_basic") {
-- 
2.7.4

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

[PATCH IGT 11/11] lib: Add igt_psr_get_sink_crc() and change tests to use it.

[Jim Bride]

Our tests were sometimes using the string representation of the sink CRC
and sometimes a binary version, so for consistency's sake, as well as to
make the utility function return something closer to what the eDP spec
talks about, all sink CRC operations are now assuming that the sink CRC
is retrieved in binary form.

Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
---
 lib/igt_psr.c                    | 59 ++++++++++++++++++++++++++++++++
 lib/igt_psr.h                    |  2 ++
 tests/kms_frontbuffer_tracking.c | 25 ++++++--------
 tests/kms_psr_sink_crc.c         | 74 ++++++++++++++++++----------------------
 4 files changed, 105 insertions(+), 55 deletions(-)

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index 2fda467..9ea2b93 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -172,3 +172,62 @@ bool igt_psr_find_good_mode(drmModeConnectorPtr connector,
 	}
 	return false;
 }
+
+/**
+ * igt_psr_open_sink_crc:
+ * @drm_fd: a file descriptor to the drm debugfs tree
+ *
+ * Returns a valid file descriptor to the sink CRC debugfs file
+ * if successful or -1 on an error.
+ */
+int igt_psr_open_sink_crc(int drm_fd)
+{
+	int crc_fd;
+
+	crc_fd = openat(drm_fd, "i915_sink_crc_eDP1", O_RDONLY);
+	igt_assert_lte(0, crc_fd);
+	return crc_fd;
+}
+
+
+/**
+ * igt_psr_get_sink_crc:
+ * @fd: a file descriptor to the sink CRC debugfs node
+ * @data: A six byte array of characters that holds the read CRC values
+ *
+ * Returns 0 on success non-zero on error.
+ */
+int igt_psr_get_sink_crc(int fd, char *data)
+{
+	int rc, errno_;
+	char buf[13]; /* two chars per byte x 6 bytes plus a '\0' */
+
+	memset(buf, 0, 13);
+	lseek(fd, 0, SEEK_SET);
+
+	rc = read(fd, buf, 12);
+	errno_ = errno;
+
+	if (rc == -1) {
+		if (errno_ == ENOTTY)
+			igt_info("Sink CRC not supported: panel doesn't support it\n");
+		else if (errno_ != ETIMEDOUT)
+			igt_assert_f(rc != -1, "Unexpected error: %d\n",
+				     errno_);
+		return errno_;
+	} else {
+		int i;
+		unsigned long long val = strtoull(buf, NULL, 16);
+
+		/*
+		 * Stuff the six CRC bytes into their individual locations.
+		 * We start from the least significant byte of the read value
+		 * to simplify masking, shifting each byte off as we set the
+		 * appropriate element in the array.
+		 */
+		for (i = 5; i >= 0; i--, val >>= 8) {
+			data[i] = (unsigned char) (val & 0xff);
+		}
+		return 0;
+	}
+}
diff --git a/lib/igt_psr.h b/lib/igt_psr.h
index f4c0a6b..6f9137b 100644
--- a/lib/igt_psr.h
+++ b/lib/igt_psr.h
@@ -36,5 +36,7 @@ void igt_psr_print_status(int fd);
 bool igt_psr_valid_connector(drmModeConnectorPtr connector);
 bool igt_psr_find_good_mode(drmModeConnectorPtr connector,
 			    drmModeModeInfoPtr *mode);
+int igt_psr_open_sink_crc(int drm_fd);
+int igt_psr_get_sink_crc(int fd, char *data);
 
 #endif /* IGT_PSR_H */
diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index fe50cc7..06a1b99 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -183,7 +183,7 @@ struct {
 };
 
 
-#define SINK_CRC_SIZE 12
+#define SINK_CRC_SIZE 6
 typedef struct {
 	char data[SINK_CRC_SIZE];
 } sink_crc_t;
@@ -916,32 +916,27 @@ static bool fbc_wait_until_enabled(void)
 
 static void get_sink_crc(sink_crc_t *crc, bool mandatory)
 {
-	int rc, errno_;
+	int rc;
+	const char bad_crc[6] = {0, 0, 0, 0, 0, 0};
 
 	if (!sink_crc.supported) {
-		memcpy(crc, "unsupported!", SINK_CRC_SIZE);
+		memcpy(crc, bad_crc, SINK_CRC_SIZE);
 		return;
 	}
 
-	lseek(sink_crc.fd, 0, SEEK_SET);
-
-	rc = read(sink_crc.fd, crc->data, SINK_CRC_SIZE);
-	errno_ = errno;
+	rc = igt_psr_get_sink_crc(sink_crc.fd, crc->data);
 
-	if (rc == -1 && errno_ == ENOTTY) {
+	if (rc == ENOTTY) {
 		igt_info("Sink CRC not supported: panel doesn't support it\n");
 		sink_crc.supported = false;
-	} else if (rc == -1 && errno_ == ETIMEDOUT) {
-		if (sink_crc.reliable) {
-			igt_info("Sink CRC is unreliable on this machine.\n");
+	} else if (rc == ETIMEDOUT) {
+		if (sink_crc.reliable)
 			sink_crc.reliable = false;
-		}
 
 		if (mandatory)
 			igt_skip("Sink CRC is unreliable on this machine.\n");
 	} else {
-		igt_assert_f(rc != -1, "Unexpected error: %d\n", errno_);
-		igt_assert(rc == SINK_CRC_SIZE);
+		igt_assert_f(rc == 0, "Unexpected error: %d\n", rc);
 	}
 }
 
@@ -1410,7 +1405,7 @@ static void setup_sink_crc(void)
 	fill_fb_region(&prim_mode_params.fb, COLOR_PRIM_BG);
 	set_mode_for_params(&prim_mode_params);
 
-	sink_crc.fd = openat(drm.debugfs, "i915_sink_crc_eDP1", O_RDONLY);
+	sink_crc.fd = igt_psr_open_sink_crc(drm.debugfs);
 	igt_assert_lte(0, sink_crc.fd);
 
 	/* Do a first read to try to detect if it's supported. */
diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index eb1bbb3..90a6b34 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -33,7 +33,7 @@
 
 bool running_with_psr_disabled;
 
-#define CRC_BLACK "000000000000"
+#define SINK_CRC_SIZE 6
 
 enum operations {
 	PAGE_FLIP,
@@ -64,6 +64,8 @@ static const char *op_str(enum operations op)
 
 typedef struct {
 	int drm_fd;
+	int debugfs_fd;
+	int crc_fd;
 	int test_plane;
 	enum operations op;
 	uint32_t devid;
@@ -125,7 +127,7 @@ static void display_init(data_t *data)
 	igt_display_init(&data->display, data->drm_fd);
 	setup_output(data);
 
-		/* We need to be able to do a modeset before we enable PSR to
+	/* We need to be able to do a modeset before we enable PSR to
 	 * ensure that we are running at a mode such that PSR setup can
 	 * complete within a single vblank interval.
 	 */
@@ -150,6 +152,8 @@ static void display_init(data_t *data)
 
 static void display_fini(data_t *data)
 {
+	close(data->crc_fd);
+	close(data->debugfs_fd);
 	igt_output_override_mode(data->output, NULL);
 	igt_display_fini(&data->display);
 }
@@ -233,58 +237,43 @@ static bool wait_psr_entry(data_t *data)
 }
 
 static void get_sink_crc(data_t *data, char *crc) {
-	int dir;
+	int rc, tries = 10;
 
 	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),
-		      "Sink CRC is unreliable on this machine. Try manual debug with --interactive-debug=no-crc\n");
-	close(dir);
+	memset(crc, 0, SINK_CRC_SIZE);
+	do {
+		rc = igt_psr_get_sink_crc(data->crc_fd, crc);
+		if (rc)
+			usleep(50000);
+	} while (rc && --tries);
 
-	igt_debug("%s\n", crc);
+	if (rc)
+		igt_skip("Could not get sink CRC after ten tries.\n");
+
+	igt_debug("crc values: 0x%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx\n",
+		  crc[0], crc[1], crc[2], crc[3], crc[4], crc[5]);
 	igt_debug_wait_for_keypress("crc");
 
 	/* The important value was already taken.
 	 * Now give a time for human eyes
 	 */
 	usleep(300000);
-
-	/* Black screen is always invalid */
-	igt_assert(strcmp(crc, CRC_BLACK) != 0);
 }
 
 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;
+	unsigned int rh, gh, bh;
 
 	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);
+	rh = (crc[1] << 8) | crc[0];
+	gh = (crc[3] << 8) | crc[2];
+	bh = (crc[5] << 8) | crc[4];
 
-	return ((rh & mask) == 0 &&
-		(gh & mask) != 0 &&
-		(bh & mask) == 0);
+	return ((rh == 0) && (gh != 0) && (bh == 0));
 }
 
 #define assert_or_manual(condition, expected)             \
@@ -298,8 +287,8 @@ static void run_test(data_t *data)
 	uint32_t handle = data->fb_white.gem_handle;
 	igt_plane_t *test_plane;
 	void *ptr;
-	char ref_crc[12];
-	char crc[12];
+	char ref_crc[SINK_CRC_SIZE];
+	char crc[SINK_CRC_SIZE];
 	const char *expected = "";
 
 	/* Confirm that screen became Green */
@@ -356,10 +345,13 @@ static void run_test(data_t *data)
 		memset(ptr, 0xff, data->mod_size);
 		get_sink_crc(data, crc);
 		if (data->test_plane == DRM_PLANE_TYPE_PRIMARY)
-			assert_or_manual(strcmp(ref_crc, crc) == 0, "screen WHITE");
+			assert_or_manual(memcmp(ref_crc, crc,
+						SINK_CRC_SIZE) == 0,
+					 "screen WHITE");
 		else
-			assert_or_manual(strcmp(ref_crc, crc) == 0,
-			       "GREEN background with WHITE box");
+			assert_or_manual(memcmp(ref_crc, crc,
+						SINK_CRC_SIZE) == 0,
+					 "GREEN background with WHITE box");
 
 		igt_info("Waiting 10s...\n");
 		sleep(10);
@@ -401,7 +393,7 @@ static void run_test(data_t *data)
 		break;
 	}
 	get_sink_crc(data, crc);
-	assert_or_manual(strcmp(ref_crc, crc) != 0, expected);
+	assert_or_manual(memcmp(ref_crc, crc, SINK_CRC_SIZE) != 0, expected);
 }
 
 static void test_cleanup(data_t *data) {
@@ -513,6 +505,8 @@ 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);
+		data.crc_fd = igt_psr_open_sink_crc(data.debugfs_fd);
 		kmstest_set_vt_graphics_mode();
 		data.devid = intel_get_drm_devid(data.drm_fd);
 		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
-- 
2.7.4

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

Re: [PATCH IGT 03/11] tests/kms_frontbuffer_tracking: Remove unneeded HSW work-around.

[Paulo Zanoni]

Em Ter, 2017-07-11 às 15:48 -0700, Jim Bride escreveu:
> This work-around actually causes issues on HSW now.  Without this
> code in-place I'm seeing good results on HSW.

Which issues?

> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Jim Bride <jim.bride@linux.intel.com>
> ---
>  tests/kms_frontbuffer_tracking.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/tests/kms_frontbuffer_tracking.c
> b/tests/kms_frontbuffer_tracking.c
> index 6f8fd20..2e9a169 100644
> --- a/tests/kms_frontbuffer_tracking.c
> +++ b/tests/kms_frontbuffer_tracking.c
> @@ -436,12 +436,6 @@ static bool
> connector_get_mode(drmModeConnectorPtr c, drmModeModeInfoPtr *mode)
>  	else
>  		*mode = &c->modes[0];
>  
> -	 /* On HSW the CRC WA is so awful that it makes you think
> everything is
> -	  * bugged. */
> -	if (IS_HASWELL(intel_get_drm_devid(drm.fd)) &&
> -	    c->connector_type == DRM_MODE_CONNECTOR_eDP)
> -		*mode = &std_1024_mode;
> -
>  	return true;
>  }
>  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx