All of lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t v2 1/8] tests/fbcon_fbt: Use psr_wait_entry() to wait PSR be enabled
@ 2019-03-27 20:03 José Roberto de Souza
  2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 2/8] tests/fbcon_fbt: Allow fbcon to bind when running this test José Roberto de Souza
                   ` (8 more replies)
  0 siblings, 9 replies; 20+ messages in thread
From: José Roberto de Souza @ 2019-03-27 20:03 UTC (permalink / raw)
  To: igt-dev; +Cc: Dhinakaran Pandiyan

kms_fbcon_fbt was doing its own handling to wait for PSR to get
enabled while it is already available in lib.

v2: splitted previous patch into this one and the previous one(Dhinakaran)

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 tests/kms_fbcon_fbt.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
index 9d0d5a36..a717e000 100644
--- a/tests/kms_fbcon_fbt.c
+++ b/tests/kms_fbcon_fbt.c
@@ -182,18 +182,9 @@ static void psr_print_status(int debugfs_fd)
 	igt_debug("PSR status: %s\n", buf);
 }
 
-static bool psr_is_enabled(int debugfs_fd)
-{
-	char buf[PSR_STATUS_MAX_LEN];
-
-	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
-				sizeof(buf));
-	return strstr(buf, "\nHW Enabled & Active bit: yes\n");
-}
-
 static bool psr_wait_until_enabled(int debugfs_fd)
 {
-	bool r = igt_wait(psr_is_enabled(debugfs_fd), 5000, 1);
+	bool r = psr_wait_entry(debugfs_fd, PSR_MODE_1);
 
 	psr_print_status(debugfs_fd);
 	return r;
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [igt-dev] [PATCH i-g-t v2 2/8] tests/fbcon_fbt: Allow fbcon to bind when running this test
  2019-03-27 20:03 [igt-dev] [PATCH i-g-t v2 1/8] tests/fbcon_fbt: Use psr_wait_entry() to wait PSR be enabled José Roberto de Souza
@ 2019-03-27 20:03 ` José Roberto de Souza
  2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 3/8] tests/fbcon_fbt: Add wait_until_update() callback to features José Roberto de Souza
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: José Roberto de Souza @ 2019-03-27 20:03 UTC (permalink / raw)
  To: igt-dev; +Cc: Rodrigo Vivi, Dhinakaran Pandiyan

9c4114ec5d87 (lib: Always unbind the fbcon around igt) broke
fbcon_fbt test as fbcon is not allowd to bind when executing any IGT
test, so lets allow it again just for this test.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 tests/kms_fbcon_fbt.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
index a717e000..a9d91839 100644
--- a/tests/kms_fbcon_fbt.c
+++ b/tests/kms_fbcon_fbt.c
@@ -26,6 +26,7 @@
 
 #include "igt.h"
 #include "igt_psr.h"
+#include "igt_sysfs.h"
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
@@ -279,6 +280,12 @@ static void setup_environment(void)
 	drm_fd = drm_open_driver_master(DRIVER_INTEL);
 	igt_require(drm_fd >= 0);
 	igt_assert(close(drm_fd) == 0);
+
+	/*
+	 * igt_subtest_init_parse_opts() disable the fbcon bind, so to test it
+	 * is necessary enable it again
+	 */
+	bind_fbcon(true);
 }
 
 static void teardown_environment(void)
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [igt-dev] [PATCH i-g-t v2 3/8] tests/fbcon_fbt: Add wait_until_update() callback to features
  2019-03-27 20:03 [igt-dev] [PATCH i-g-t v2 1/8] tests/fbcon_fbt: Use psr_wait_entry() to wait PSR be enabled José Roberto de Souza
  2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 2/8] tests/fbcon_fbt: Allow fbcon to bind when running this test José Roberto de Souza
@ 2019-03-27 20:03 ` José Roberto de Souza
  2019-04-09 21:41   ` Dhinakaran Pandiyan
  2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 4/8] tests/fbcon_fbt: Use psr_wait_update() to test updates in PSR José Roberto de Souza
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: José Roberto de Souza @ 2019-03-27 20:03 UTC (permalink / raw)
  To: igt-dev; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi

Lets be more explicit and add and implement a callback to check if
feature had a state update, that is what some points of the test want
to test.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 tests/kms_fbcon_fbt.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
index a9d91839..9ab1d630 100644
--- a/tests/kms_fbcon_fbt.c
+++ b/tests/kms_fbcon_fbt.c
@@ -130,6 +130,11 @@ static bool fbc_wait_until_enabled(int debugfs_fd)
 	return r;
 }
 
+static bool fbc_wait_until_update(int debugfs)
+{
+	return !fbc_wait_until_enabled(debugfs);
+}
+
 typedef bool (*connector_possible_fn)(drmModeConnectorPtr connector);
 
 static void set_mode_for_one_screen(struct drm_info *drm, struct igt_fb *fb,
@@ -196,6 +201,11 @@ static bool psr_supported_on_chipset(int debugfs_fd)
 	return psr_sink_support(debugfs_fd, PSR_MODE_1);
 }
 
+static bool psr_wait_until_update(int debugfs_fd)
+{
+	return !psr_wait_until_enabled(debugfs_fd);
+}
+
 static void disable_features(int debugfs_fd)
 {
 	igt_set_module_param_int("enable_fbc", 0);
@@ -215,16 +225,19 @@ static inline void psr_debugfs_enable(int debugfs_fd)
 struct feature {
 	bool (*supported_on_chipset)(int debugfs_fd);
 	bool (*wait_until_enabled)(int debugfs_fd);
+	bool (*wait_until_update)(int debugfs_fd);
 	bool (*connector_possible_fn)(drmModeConnectorPtr connector);
 	void (*enable)(int debugfs_fd);
 } fbc = {
 	.supported_on_chipset = fbc_supported_on_chipset,
 	.wait_until_enabled = fbc_wait_until_enabled,
+	.wait_until_update = fbc_wait_until_update,
 	.connector_possible_fn = connector_can_fbc,
 	.enable = fbc_modparam_enable,
 }, psr = {
 	.supported_on_chipset = psr_supported_on_chipset,
 	.wait_until_enabled = psr_wait_until_enabled,
+	.wait_until_update = psr_wait_until_update,
 	.connector_possible_fn = connector_can_psr,
 	.enable = psr_debugfs_enable,
 };
@@ -243,7 +256,7 @@ 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.debugfs_fd));
+	igt_assert(feature->wait_until_update(drm.debugfs_fd));
 
 	set_mode_for_one_screen(&drm, &fb, feature->connector_possible_fn);
 	wait_user("Screen set.");
@@ -263,13 +276,13 @@ static void subtest(struct feature *feature, bool suspend)
 	sleep(3);
 
 	wait_user("Back to fbcon.");
-	igt_assert(!feature->wait_until_enabled(drm.debugfs_fd));
+	igt_assert(feature->wait_until_update(drm.debugfs_fd));
 
 	if (suspend) {
 		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
 					      SUSPEND_TEST_NONE);
 		sleep(5);
-		igt_assert(!feature->wait_until_enabled(drm.debugfs_fd));
+		igt_assert(feature->wait_until_update(drm.debugfs_fd));
 	}
 }
 
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [igt-dev] [PATCH i-g-t v2 4/8] tests/fbcon_fbt: Use psr_wait_update() to test updates in PSR
  2019-03-27 20:03 [igt-dev] [PATCH i-g-t v2 1/8] tests/fbcon_fbt: Use psr_wait_entry() to wait PSR be enabled José Roberto de Souza
  2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 2/8] tests/fbcon_fbt: Allow fbcon to bind when running this test José Roberto de Souza
  2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 3/8] tests/fbcon_fbt: Add wait_until_update() callback to features José Roberto de Souza
@ 2019-03-27 20:03 ` José Roberto de Souza
  2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 5/8] tests/fbcon_fbt: Add and use psr_long_wait_update() " José Roberto de Souza
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: José Roberto de Souza @ 2019-03-27 20:03 UTC (permalink / raw)
  To: igt-dev; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi

It will test was is intented and faster as in the first occurence of
PSR exit it will return instead of wait the timeout of
psr_wait_entry().

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 tests/kms_fbcon_fbt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
index 9ab1d630..b57e5f51 100644
--- a/tests/kms_fbcon_fbt.c
+++ b/tests/kms_fbcon_fbt.c
@@ -203,7 +203,7 @@ static bool psr_supported_on_chipset(int debugfs_fd)
 
 static bool psr_wait_until_update(int debugfs_fd)
 {
-	return !psr_wait_until_enabled(debugfs_fd);
+	return psr_wait_update(debugfs_fd, PSR_MODE_1);
 }
 
 static void disable_features(int debugfs_fd)
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [igt-dev] [PATCH i-g-t v2 5/8] tests/fbcon_fbt: Add and use psr_long_wait_update() to test updates in PSR
  2019-03-27 20:03 [igt-dev] [PATCH i-g-t v2 1/8] tests/fbcon_fbt: Use psr_wait_entry() to wait PSR be enabled José Roberto de Souza
                   ` (2 preceding siblings ...)
  2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 4/8] tests/fbcon_fbt: Use psr_wait_update() to test updates in PSR José Roberto de Souza
@ 2019-03-27 20:03 ` José Roberto de Souza
  2019-04-09 17:55   ` Maarten Lankhorst
  2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 6/8] tests/fbcon_fbt: Enable cursor blinking José Roberto de Souza
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: José Roberto de Souza @ 2019-03-27 20:03 UTC (permalink / raw)
  To: igt-dev; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi

After getting back to fbcon it tests if PSR had a update but since
fbcon can take a while it have some big sleep in place, so what it
is actualy testing is if fbcon is updating the screen.
In this test the update of the screen comes from the fbcon cursor
blinking, the problem is that 40ms is a small interval to detect
cursor blinking and the test can fail some times, so a bigger timeout
to wait for a update is need.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 lib/igt_psr.c         | 5 +++++
 lib/igt_psr.h         | 1 +
 tests/kms_fbcon_fbt.c | 2 +-
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index b5847bfd..b92ea73f 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -54,6 +54,11 @@ bool psr_wait_update(int debugfs_fd, enum psr_mode mode)
 	return igt_wait(!psr_active_check(debugfs_fd, mode), 40, 10);
 }
 
+bool psr_long_wait_update(int debugfs_fd, enum psr_mode mode)
+{
+	return igt_wait(!psr_active_check(debugfs_fd, mode), 500, 10);
+}
+
 static ssize_t psr_write(int debugfs_fd, const char *buf)
 {
 	return igt_sysfs_write(debugfs_fd, "i915_edp_psr_debug", buf,
diff --git a/lib/igt_psr.h b/lib/igt_psr.h
index 49599cf8..ca385736 100644
--- a/lib/igt_psr.h
+++ b/lib/igt_psr.h
@@ -37,6 +37,7 @@ enum psr_mode {
 
 bool psr_wait_entry(int debugfs_fd, enum psr_mode mode);
 bool psr_wait_update(int debugfs_fd, enum psr_mode mode);
+bool psr_long_wait_update(int debugfs_fd, enum psr_mode mode);
 bool psr_enable(int debugfs_fd, enum psr_mode);
 bool psr_disable(int debugfs_fd);
 bool psr_sink_support(int debugfs_fd, enum psr_mode);
diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
index b57e5f51..0550034e 100644
--- a/tests/kms_fbcon_fbt.c
+++ b/tests/kms_fbcon_fbt.c
@@ -203,7 +203,7 @@ static bool psr_supported_on_chipset(int debugfs_fd)
 
 static bool psr_wait_until_update(int debugfs_fd)
 {
-	return psr_wait_update(debugfs_fd, PSR_MODE_1);
+	return psr_long_wait_update(debugfs_fd, PSR_MODE_1);
 }
 
 static void disable_features(int debugfs_fd)
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [igt-dev] [PATCH i-g-t v2 6/8] tests/fbcon_fbt: Enable cursor blinking
  2019-03-27 20:03 [igt-dev] [PATCH i-g-t v2 1/8] tests/fbcon_fbt: Use psr_wait_entry() to wait PSR be enabled José Roberto de Souza
                   ` (3 preceding siblings ...)
  2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 5/8] tests/fbcon_fbt: Add and use psr_long_wait_update() " José Roberto de Souza
@ 2019-03-27 20:03 ` José Roberto de Souza
  2019-04-09 17:59   ` Maarten Lankhorst
  2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 7/8] tests/fbcon_fbt: Do not try to open the same driver twice as master José Roberto de Souza
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: José Roberto de Souza @ 2019-03-27 20:03 UTC (permalink / raw)
  To: igt-dev; +Cc: Rodrigo Vivi, Dhinakaran Pandiyan

If cursor blinking is disabled no screen updates will happen and
fbcon_fbt subtests will fail, so lets enable cursor blink while
running this test and restore to the previous value when exiting it.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 lib/igt_sysfs.c       | 46 +++++++++++++++++++++++++++++++++++++++++++
 lib/igt_sysfs.h       |  1 +
 tests/kms_fbcon_fbt.c |  1 +
 3 files changed, 48 insertions(+)

diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
index f806f4fc..904fbd17 100644
--- a/lib/igt_sysfs.c
+++ b/lib/igt_sysfs.c
@@ -610,3 +610,49 @@ void kick_snd_hda_intel(void)
 out:
 	close(fd);
 }
+
+static int fbcon_blink_restore_debugfs_fd = -1;
+static uint8_t fbcon_blink_restore_value;
+
+static void fbcon_blink_restore(int sig)
+{
+	char buffer[4];
+	int r;
+
+	r = snprintf(buffer, sizeof(buffer), "%u", fbcon_blink_restore_value);
+	write(fbcon_blink_restore_debugfs_fd, buffer, r + 1);
+}
+
+/**
+ * fbcon_blink_enable:
+ * @enable: if true enables the fbcon cursor blinking otherwise disables it
+ *
+ * Enables or disables the cursor blinking in fbcon, it also restores the
+ * previous blinking state when exiting test.
+ *
+ */
+void fbcon_blink_enable(bool enable)
+{
+	const char *cur_blink_path = "/sys/class/graphics/fbcon/cursor_blink";
+	char buffer[4];
+	int fd, r;
+
+	fd = open(cur_blink_path, O_RDWR);
+	igt_assert(fd >= 0);
+
+	/* Restore original value on exit */
+	if (fbcon_blink_restore_debugfs_fd == -1) {
+		r = read(fd, buffer, sizeof(buffer));
+		if (r > 0) {
+			fbcon_blink_restore_value = (uint8_t)strtol(buffer,
+								    NULL, 10);
+			fbcon_blink_restore_debugfs_fd = dup(fd);
+			igt_assert(fbcon_blink_restore_debugfs_fd >= 0);
+			igt_install_exit_handler(fbcon_blink_restore);
+		}
+	}
+
+	r = snprintf(buffer, sizeof(buffer), enable ? "1" : "0");
+	write(fd, buffer, r + 1);
+	close(fd);
+}
diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
index c12e36d1..b646df30 100644
--- a/lib/igt_sysfs.h
+++ b/lib/igt_sysfs.h
@@ -57,5 +57,6 @@ bool igt_sysfs_set_boolean(int dir, const char *attr, bool value);
 
 void bind_fbcon(bool enable);
 void kick_snd_hda_intel(void);
+void fbcon_blink_enable(bool enable);
 
 #endif /* __IGT_SYSFS_H__ */
diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
index 0550034e..2f158e2f 100644
--- a/tests/kms_fbcon_fbt.c
+++ b/tests/kms_fbcon_fbt.c
@@ -299,6 +299,7 @@ static void setup_environment(void)
 	 * is necessary enable it again
 	 */
 	bind_fbcon(true);
+	fbcon_blink_enable(true);
 }
 
 static void teardown_environment(void)
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [igt-dev] [PATCH i-g-t v2 7/8] tests/fbcon_fbt: Do not try to open the same driver twice as master
  2019-03-27 20:03 [igt-dev] [PATCH i-g-t v2 1/8] tests/fbcon_fbt: Use psr_wait_entry() to wait PSR be enabled José Roberto de Souza
                   ` (4 preceding siblings ...)
  2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 6/8] tests/fbcon_fbt: Enable cursor blinking José Roberto de Souza
@ 2019-03-27 20:03 ` José Roberto de Souza
  2019-04-09 17:55   ` Maarten Lankhorst
  2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 8/8] tests/fbcon_fbt: Do not keep opening debugfs_fd at every setup_drm() call José Roberto de Souza
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: José Roberto de Souza @ 2019-03-27 20:03 UTC (permalink / raw)
  To: igt-dev

If a subtest fail before the teardown_drm() call it will keep the
driver open as master causing the test to fail when trying to open
the driver again as master, so lets share the drm_info struct and
check if driver is already open.

Starting subtest: psr
(kms_fbcon_fbt:5270) CRITICAL: Test assertion failure function subtest, file ../tests/kms_fbcon_fbt.c:259:
(kms_fbcon_fbt:5270) CRITICAL: Failed assertion: feature->wait_until_update(drm.debugfs_fd)
Stack trace:
  #0 ../lib/igt_core.c:1474 __igt_fail_assert()
  #1 ../tests/kms_fbcon_fbt.c:261 subtest()
  #2 ../tests/kms_fbcon_fbt.c:316 __real_main309()
  #3 ../tests/kms_fbcon_fbt.c:309 main()
  #4 ../csu/libc-start.c:325 __libc_start_main()
  #5 [_start+0x29]
  #6 [<unknown>+0x0]
Subtest psr failed.
**** DEBUG ****
(kms_fbcon_fbt:5270) drmtest-DEBUG: Test requirement passed: !(fd<0)
(kms_fbcon_fbt:5270) igt_debugfs-DEBUG: Opening debugfs directory '/sys/kernel/debug/dri/1'
(kms_fbcon_fbt:5270) DEBUG: Test requirement passed: drm->res
(kms_fbcon_fbt:5270) igt_kms-DEBUG: VT: graphics mode set (mode was 0x0)
(kms_fbcon_fbt:5270) DEBUG: Test requirement passed: feature->supported_on_chipset(drm.debugfs_fd)
(kms_fbcon_fbt:5270) CRITICAL: Test assertion failure function subtest, file ../tests/kms_fbcon_fbt.c:259:
(kms_fbcon_fbt:5270) CRITICAL: Failed assertion: feature->wait_until_update(drm.debugfs_fd)
(kms_fbcon_fbt:5270) igt_core-INFO: Stack trace:
(kms_fbcon_fbt:5270) igt_core-INFO:   #0 ../lib/igt_core.c:1474 __igt_fail_assert()
(kms_fbcon_fbt:5270) igt_core-INFO:   #1 ../tests/kms_fbcon_fbt.c:261 subtest()
(kms_fbcon_fbt:5270) igt_core-INFO:   #2 ../tests/kms_fbcon_fbt.c:316 __real_main309()
(kms_fbcon_fbt:5270) igt_core-INFO:   #3 ../tests/kms_fbcon_fbt.c:309 main()
(kms_fbcon_fbt:5270) igt_core-INFO:   #4 ../csu/libc-start.c:325 __libc_start_main()
(kms_fbcon_fbt:5270) igt_core-INFO:   #5 [_start+0x29]
(kms_fbcon_fbt:5270) igt_core-INFO:   #6 [<unknown>+0x0]
****  END  ****
Subtest psr: FAIL (0.845s)
Starting subtest: fbc-suspend
Test requirement not met in function igt_device_set_master, file ../lib/igt_device.c:55:
Test requirement: __igt_device_set_master(fd) == 0
Can't become DRM master, please check if no other DRM client is running.
Subtest fbc-suspend: SKIP (0.001s)
Starting subtest: psr-suspend
Test requirement not met in function igt_device_set_master, file ../lib/igt_device.c:55:
Test requirement: __igt_device_set_master(fd) == 0
Can't become DRM master, please check if no other DRM client is running.
Subtest psr-suspend: SKIP (0.000s)

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 tests/kms_fbcon_fbt.c | 55 +++++++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 23 deletions(-)

diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
index 2f158e2f..66f214c5 100644
--- a/tests/kms_fbcon_fbt.c
+++ b/tests/kms_fbcon_fbt.c
@@ -60,6 +60,9 @@ static void setup_drm(struct drm_info *drm)
 {
 	int i;
 
+	if (drm->fd >= 0)
+		return;
+
 	drm->fd = drm_open_driver_master(DRIVER_INTEL);
 	drm->debugfs_fd = igt_debugfs_dir(drm->fd);
 
@@ -85,6 +88,7 @@ static void teardown_drm(struct drm_info *drm)
 
 	drmModeFreeResources(drm->res);
 	igt_assert(close(drm->fd) == 0);
+	drm->fd = -1;
 }
 
 static bool fbc_supported_on_chipset(int debugfs_fd)
@@ -242,51 +246,50 @@ struct feature {
 	.enable = psr_debugfs_enable,
 };
 
-static void subtest(struct feature *feature, bool suspend)
+static void subtest(struct drm_info *drm, struct feature *feature, bool suspend)
 {
-	struct drm_info drm;
 	struct igt_fb fb;
 
-	setup_drm(&drm);
+	setup_drm(drm);
 
-	igt_require(feature->supported_on_chipset(drm.debugfs_fd));
+	igt_require(feature->supported_on_chipset(drm->debugfs_fd));
 
-	disable_features(drm.debugfs_fd);
-	feature->enable(drm.debugfs_fd);
+	disable_features(drm->debugfs_fd);
+	feature->enable(drm->debugfs_fd);
 
-	kmstest_unset_all_crtcs(drm.fd, drm.res);
+	kmstest_unset_all_crtcs(drm->fd, drm->res);
 	wait_user("Modes unset.");
-	igt_assert(feature->wait_until_update(drm.debugfs_fd));
+	igt_assert(feature->wait_until_update(drm->debugfs_fd));
 
-	set_mode_for_one_screen(&drm, &fb, feature->connector_possible_fn);
+	set_mode_for_one_screen(drm, &fb, feature->connector_possible_fn);
 	wait_user("Screen set.");
-	igt_assert(feature->wait_until_enabled(drm.debugfs_fd));
+	igt_assert(feature->wait_until_enabled(drm->debugfs_fd));
 
 	if (suspend) {
 		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
 					      SUSPEND_TEST_NONE);
 		sleep(5);
-		igt_assert(feature->wait_until_enabled(drm.debugfs_fd));
+		igt_assert(feature->wait_until_enabled(drm->debugfs_fd));
 	}
 
-	igt_remove_fb(drm.fd, &fb);
-	teardown_drm(&drm);
+	igt_remove_fb(drm->fd, &fb);
+	teardown_drm(drm);
 
 	/* Wait for fbcon to restore itself. */
 	sleep(3);
 
 	wait_user("Back to fbcon.");
-	igt_assert(feature->wait_until_update(drm.debugfs_fd));
+	igt_assert(feature->wait_until_update(drm->debugfs_fd));
 
 	if (suspend) {
 		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
 					      SUSPEND_TEST_NONE);
 		sleep(5);
-		igt_assert(feature->wait_until_update(drm.debugfs_fd));
+		igt_assert(feature->wait_until_update(drm->debugfs_fd));
 	}
 }
 
-static void setup_environment(void)
+static void setup_environment(struct drm_info *drm)
 {
 	int drm_fd;
 
@@ -300,26 +303,32 @@ static void setup_environment(void)
 	 */
 	bind_fbcon(true);
 	fbcon_blink_enable(true);
+
+	drm->fd = -1;
 }
 
-static void teardown_environment(void)
+static void teardown_environment(struct drm_info *drm)
 {
+	if (drm->fd >= 0)
+		teardown_drm(drm);
 }
 
 igt_main
 {
+	struct drm_info drm = {};
+
 	igt_fixture
-		setup_environment();
+		setup_environment(&drm);
 
 	igt_subtest("fbc")
-		subtest(&fbc, false);
+		subtest(&drm, &fbc, false);
 	igt_subtest("psr")
-		subtest(&psr, false);
+		subtest(&drm, &psr, false);
 	igt_subtest("fbc-suspend")
-		subtest(&fbc, true);
+		subtest(&drm, &fbc, true);
 	igt_subtest("psr-suspend")
-		subtest(&psr, true);
+		subtest(&drm, &psr, true);
 
 	igt_fixture
-		teardown_environment();
+		teardown_environment(&drm);
 }
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [igt-dev] [PATCH i-g-t v2 8/8] tests/fbcon_fbt: Do not keep opening debugfs_fd at every setup_drm() call
  2019-03-27 20:03 [igt-dev] [PATCH i-g-t v2 1/8] tests/fbcon_fbt: Use psr_wait_entry() to wait PSR be enabled José Roberto de Souza
                   ` (5 preceding siblings ...)
  2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 7/8] tests/fbcon_fbt: Do not try to open the same driver twice as master José Roberto de Souza
@ 2019-03-27 20:03 ` José Roberto de Souza
  2019-04-09 18:01   ` Maarten Lankhorst
  2019-03-27 21:01 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v2,1/8] tests/fbcon_fbt: Use psr_wait_entry() to wait PSR be enabled Patchwork
  2019-03-28 17:44 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  8 siblings, 1 reply; 20+ messages in thread
From: José Roberto de Souza @ 2019-03-27 20:03 UTC (permalink / raw)
  To: igt-dev

As debugfs_fd is used even after the teardown_drm() call, it do not
closes it causing it to open another debugfs_fd at every call to
setup_drm().
Let move the opening of debugfs_fd to setup_environment() and only
open it once, also lets close it before leave the test.

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 tests/kms_fbcon_fbt.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
index 66f214c5..f7ff671b 100644
--- a/tests/kms_fbcon_fbt.c
+++ b/tests/kms_fbcon_fbt.c
@@ -64,7 +64,6 @@ static void setup_drm(struct drm_info *drm)
 		return;
 
 	drm->fd = drm_open_driver_master(DRIVER_INTEL);
-	drm->debugfs_fd = igt_debugfs_dir(drm->fd);
 
 	drm->res = drmModeGetResources(drm->fd);
 	igt_require(drm->res);
@@ -295,6 +294,7 @@ static void setup_environment(struct drm_info *drm)
 
 	drm_fd = drm_open_driver_master(DRIVER_INTEL);
 	igt_require(drm_fd >= 0);
+	drm->debugfs_fd = igt_debugfs_dir(drm_fd);
 	igt_assert(close(drm_fd) == 0);
 
 	/*
@@ -311,6 +311,8 @@ static void teardown_environment(struct drm_info *drm)
 {
 	if (drm->fd >= 0)
 		teardown_drm(drm);
+
+	close(drm->debugfs_fd);
 }
 
 igt_main
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v2,1/8] tests/fbcon_fbt: Use psr_wait_entry() to wait PSR be enabled
  2019-03-27 20:03 [igt-dev] [PATCH i-g-t v2 1/8] tests/fbcon_fbt: Use psr_wait_entry() to wait PSR be enabled José Roberto de Souza
                   ` (6 preceding siblings ...)
  2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 8/8] tests/fbcon_fbt: Do not keep opening debugfs_fd at every setup_drm() call José Roberto de Souza
@ 2019-03-27 21:01 ` Patchwork
  2019-03-28 17:44 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  8 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2019-03-27 21:01 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,v2,1/8] tests/fbcon_fbt: Use psr_wait_entry() to wait PSR be enabled
URL   : https://patchwork.freedesktop.org/series/58652/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5829 -> IGTPW_2720
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/58652/revisions/1/mbox/

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

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

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_cs_nop@fork-compute0:
    - fi-blb-e6850:       NOTRUN -> SKIP [fdo#109271] +18

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         PASS -> INCOMPLETE [fdo#103927] / [fdo#109720]

  * igt@kms_frontbuffer_tracking@basic:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103167]

  
#### Possible fixes ####

  * igt@i915_module_load@reload:
    - fi-blb-e6850:       INCOMPLETE [fdo#107718] -> PASS

  * igt@i915_selftest@live_contexts:
    - fi-bdw-gvtdvm:      DMESG-FAIL [fdo#110235 ] -> PASS

  * igt@i915_selftest@live_uncore:
    - fi-skl-gvtdvm:      DMESG-FAIL [fdo#110210] -> PASS

  * igt@kms_busy@basic-flip-b:
    - fi-gdg-551:         FAIL [fdo#103182] -> PASS

  * igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
  [fdo#110210]: https://bugs.freedesktop.org/show_bug.cgi?id=110210
  [fdo#110235 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110235 


Participating hosts (41 -> 37)
------------------------------

  Missing    (4): fi-kbl-soraka fi-ilk-m540 fi-bsw-cyan fi-hsw-4200u 


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

    * IGT: IGT_4909 -> IGTPW_2720

  CI_DRM_5829: 32a1e283d330638d2e5de8f89a9ff7c8512b75e8 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2720: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2720/
  IGT_4909: 7df3eeb4f3360cd2b511c31acc1c52bd7ce6587f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,v2,1/8] tests/fbcon_fbt: Use psr_wait_entry() to wait PSR be enabled
  2019-03-27 20:03 [igt-dev] [PATCH i-g-t v2 1/8] tests/fbcon_fbt: Use psr_wait_entry() to wait PSR be enabled José Roberto de Souza
                   ` (7 preceding siblings ...)
  2019-03-27 21:01 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v2,1/8] tests/fbcon_fbt: Use psr_wait_entry() to wait PSR be enabled Patchwork
@ 2019-03-28 17:44 ` Patchwork
  8 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2019-03-28 17:44 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,v2,1/8] tests/fbcon_fbt: Use psr_wait_entry() to wait PSR be enabled
URL   : https://patchwork.freedesktop.org/series/58652/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5829_full -> IGTPW_2720_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/58652/revisions/1/mbox/

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_schedule@fifo-bsd2:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +67

  * igt@gem_pwrite@stolen-normal:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +20

  * igt@gem_workarounds@suspend-resume-context:
    - shard-kbl:          PASS -> INCOMPLETE [fdo#103665]

  * igt@kms_busy@extended-modeset-hang-newfb-render-c:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#110222] +1

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
    - shard-snb:          PASS -> DMESG-WARN [fdo#110222]
    - shard-hsw:          PASS -> DMESG-WARN [fdo#110222]

  * igt@kms_busy@extended-modeset-hang-oldfb-with-reset-render-f:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_busy@extended-pageflip-hang-newfb-render-a:
    - shard-glk:          NOTRUN -> DMESG-WARN [fdo#110222]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
    - shard-glk:          PASS -> DMESG-WARN [fdo#110222]

  * igt@kms_cursor_crc@cursor-64x64-offscreen:
    - shard-apl:          PASS -> INCOMPLETE [fdo#103927]

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
    - shard-hsw:          PASS -> FAIL [fdo#105767]

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
    - shard-glk:          PASS -> FAIL [fdo#104873]

  * igt@kms_flip@plain-flip-fb-recreate:
    - shard-glk:          PASS -> FAIL [fdo#100368]

  * igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-shrfb-draw-pwrite:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] +4

  * igt@kms_frontbuffer_tracking@psr-2p-scndscrn-shrfb-msflip-blt:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] +17

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-d:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_plane_alpha_blend@pipe-b-alpha-transparant-fb:
    - shard-apl:          NOTRUN -> FAIL [fdo#108145]
    - shard-glk:          NOTRUN -> FAIL [fdo#108145] +1

  * igt@kms_plane_scaling@pipe-a-scaler-with-clipping-clamping:
    - shard-glk:          PASS -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_universal_plane@universal-plane-gen9-features-pipe-e:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +6

  * igt@kms_vblank@pipe-c-wait-idle-hang:
    - shard-hsw:          PASS -> INCOMPLETE [fdo#103540]

  
#### Possible fixes ####

  * igt@gem_eio@in-flight-suspend:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS

  * igt@i915_pm_rc6_residency@rc6-accuracy:
    - shard-snb:          SKIP [fdo#109271] -> PASS

  * igt@kms_busy@extended-modeset-hang-newfb-render-b:
    - shard-hsw:          DMESG-WARN [fdo#110222] -> PASS +1
    - shard-kbl:          DMESG-WARN [fdo#110222] -> PASS +1
    - shard-snb:          DMESG-WARN [fdo#110222] -> PASS +1

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
    - shard-glk:          FAIL [fdo#103167] -> PASS
    - shard-kbl:          FAIL [fdo#103167] -> PASS
    - shard-apl:          FAIL [fdo#103167] -> PASS

  * igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb:
    - shard-glk:          FAIL [fdo#108145] -> PASS
    - shard-apl:          FAIL [fdo#108145] -> PASS
    - shard-kbl:          FAIL [fdo#108145] -> PASS

  * igt@kms_plane_scaling@pipe-b-scaler-with-rotation:
    - shard-glk:          SKIP [fdo#109271] / [fdo#109278] -> PASS

  * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
    - shard-kbl:          DMESG-FAIL [fdo#105763] -> PASS

  * igt@kms_vblank@pipe-c-ts-continuation-suspend:
    - shard-apl:          FAIL [fdo#104894] -> PASS
    - shard-kbl:          FAIL [fdo#104894] -> PASS

  
#### Warnings ####

  * igt@runner@aborted:
    - shard-glk:          ( 2 FAIL ) [fdo#109373] / [k.org#202321] -> FAIL [fdo#109373] / [k.org#202321]

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

  [fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
  [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109373]: https://bugs.freedesktop.org/show_bug.cgi?id=109373
  [fdo#110222]: https://bugs.freedesktop.org/show_bug.cgi?id=110222
  [k.org#202321]: https://bugzilla.kernel.org/show_bug.cgi?id=202321


Participating hosts (10 -> 5)
------------------------------

  Missing    (5): shard-skl pig-hsw-4770r pig-glk-j5005 shard-iclb pig-skl-6260u 


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

    * IGT: IGT_4909 -> IGTPW_2720
    * Piglit: piglit_4509 -> None

  CI_DRM_5829: 32a1e283d330638d2e5de8f89a9ff7c8512b75e8 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2720: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2720/
  IGT_4909: 7df3eeb4f3360cd2b511c31acc1c52bd7ce6587f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2 7/8] tests/fbcon_fbt: Do not try to open the same driver twice as master
  2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 7/8] tests/fbcon_fbt: Do not try to open the same driver twice as master José Roberto de Souza
@ 2019-04-09 17:55   ` Maarten Lankhorst
  2019-04-10 21:07     ` Souza, Jose
  0 siblings, 1 reply; 20+ messages in thread
From: Maarten Lankhorst @ 2019-04-09 17:55 UTC (permalink / raw)
  To: José Roberto de Souza, igt-dev

Op 27-03-2019 om 21:03 schreef José Roberto de Souza:
> If a subtest fail before the teardown_drm() call it will keep the
> driver open as master causing the test to fail when trying to open
> the driver again as master, so lets share the drm_info struct and
> check if driver is already open.
>
> Starting subtest: psr
> (kms_fbcon_fbt:5270) CRITICAL: Test assertion failure function subtest, file ../tests/kms_fbcon_fbt.c:259:
> (kms_fbcon_fbt:5270) CRITICAL: Failed assertion: feature->wait_until_update(drm.debugfs_fd)
> Stack trace:
>   #0 ../lib/igt_core.c:1474 __igt_fail_assert()
>   #1 ../tests/kms_fbcon_fbt.c:261 subtest()
>   #2 ../tests/kms_fbcon_fbt.c:316 __real_main309()
>   #3 ../tests/kms_fbcon_fbt.c:309 main()
>   #4 ../csu/libc-start.c:325 __libc_start_main()
>   #5 [_start+0x29]
>   #6 [<unknown>+0x0]
> Subtest psr failed.
> **** DEBUG ****
> (kms_fbcon_fbt:5270) drmtest-DEBUG: Test requirement passed: !(fd<0)
> (kms_fbcon_fbt:5270) igt_debugfs-DEBUG: Opening debugfs directory '/sys/kernel/debug/dri/1'
> (kms_fbcon_fbt:5270) DEBUG: Test requirement passed: drm->res
> (kms_fbcon_fbt:5270) igt_kms-DEBUG: VT: graphics mode set (mode was 0x0)
> (kms_fbcon_fbt:5270) DEBUG: Test requirement passed: feature->supported_on_chipset(drm.debugfs_fd)
> (kms_fbcon_fbt:5270) CRITICAL: Test assertion failure function subtest, file ../tests/kms_fbcon_fbt.c:259:
> (kms_fbcon_fbt:5270) CRITICAL: Failed assertion: feature->wait_until_update(drm.debugfs_fd)
> (kms_fbcon_fbt:5270) igt_core-INFO: Stack trace:
> (kms_fbcon_fbt:5270) igt_core-INFO:   #0 ../lib/igt_core.c:1474 __igt_fail_assert()
> (kms_fbcon_fbt:5270) igt_core-INFO:   #1 ../tests/kms_fbcon_fbt.c:261 subtest()
> (kms_fbcon_fbt:5270) igt_core-INFO:   #2 ../tests/kms_fbcon_fbt.c:316 __real_main309()
> (kms_fbcon_fbt:5270) igt_core-INFO:   #3 ../tests/kms_fbcon_fbt.c:309 main()
> (kms_fbcon_fbt:5270) igt_core-INFO:   #4 ../csu/libc-start.c:325 __libc_start_main()
> (kms_fbcon_fbt:5270) igt_core-INFO:   #5 [_start+0x29]
> (kms_fbcon_fbt:5270) igt_core-INFO:   #6 [<unknown>+0x0]
> ****  END  ****
> Subtest psr: FAIL (0.845s)
> Starting subtest: fbc-suspend
> Test requirement not met in function igt_device_set_master, file ../lib/igt_device.c:55:
> Test requirement: __igt_device_set_master(fd) == 0
> Can't become DRM master, please check if no other DRM client is running.
> Subtest fbc-suspend: SKIP (0.001s)
> Starting subtest: psr-suspend
> Test requirement not met in function igt_device_set_master, file ../lib/igt_device.c:55:
> Test requirement: __igt_device_set_master(fd) == 0
> Can't become DRM master, please check if no other DRM client is running.
> Subtest psr-suspend: SKIP (0.000s)
>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  tests/kms_fbcon_fbt.c | 55 +++++++++++++++++++++++++------------------
>  1 file changed, 32 insertions(+), 23 deletions(-)
>
> diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
> index 2f158e2f..66f214c5 100644
> --- a/tests/kms_fbcon_fbt.c
> +++ b/tests/kms_fbcon_fbt.c
> @@ -60,6 +60,9 @@ static void setup_drm(struct drm_info *drm)
>  {
>  	int i;
>  
> +	if (drm->fd >= 0)
> +		return;
> +
>  	drm->fd = drm_open_driver_master(DRIVER_INTEL);
>  	drm->debugfs_fd = igt_debugfs_dir(drm->fd);
>  
> @@ -85,6 +88,7 @@ static void teardown_drm(struct drm_info *drm)
>  
>  	drmModeFreeResources(drm->res);
>  	igt_assert(close(drm->fd) == 0);
> +	drm->fd = -1;
>  }
>  
>  static bool fbc_supported_on_chipset(int debugfs_fd)
> @@ -242,51 +246,50 @@ struct feature {
>  	.enable = psr_debugfs_enable,
>  };
>  
> -static void subtest(struct feature *feature, bool suspend)
> +static void subtest(struct drm_info *drm, struct feature *feature, bool suspend)
>  {
> -	struct drm_info drm;
>  	struct igt_fb fb;
>  
> -	setup_drm(&drm);
> +	setup_drm(drm);
>  
> -	igt_require(feature->supported_on_chipset(drm.debugfs_fd));
> +	igt_require(feature->supported_on_chipset(drm->debugfs_fd));
>  
> -	disable_features(drm.debugfs_fd);
> -	feature->enable(drm.debugfs_fd);
> +	disable_features(drm->debugfs_fd);
> +	feature->enable(drm->debugfs_fd);
>  
> -	kmstest_unset_all_crtcs(drm.fd, drm.res);
> +	kmstest_unset_all_crtcs(drm->fd, drm->res);
>  	wait_user("Modes unset.");
> -	igt_assert(feature->wait_until_update(drm.debugfs_fd));
> +	igt_assert(feature->wait_until_update(drm->debugfs_fd));
>  
> -	set_mode_for_one_screen(&drm, &fb, feature->connector_possible_fn);
> +	set_mode_for_one_screen(drm, &fb, feature->connector_possible_fn);
>  	wait_user("Screen set.");
> -	igt_assert(feature->wait_until_enabled(drm.debugfs_fd));
> +	igt_assert(feature->wait_until_enabled(drm->debugfs_fd));
>  
>  	if (suspend) {
>  		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
>  					      SUSPEND_TEST_NONE);
>  		sleep(5);
> -		igt_assert(feature->wait_until_enabled(drm.debugfs_fd));
> +		igt_assert(feature->wait_until_enabled(drm->debugfs_fd));
>  	}
>  
> -	igt_remove_fb(drm.fd, &fb);
> -	teardown_drm(&drm);
> +	igt_remove_fb(drm->fd, &fb);
> +	teardown_drm(drm);
>  
>  	/* Wait for fbcon to restore itself. */
>  	sleep(3);
>  
>  	wait_user("Back to fbcon.");
> -	igt_assert(feature->wait_until_update(drm.debugfs_fd));
> +	igt_assert(feature->wait_until_update(drm->debugfs_fd));
>  
>  	if (suspend) {
>  		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
>  					      SUSPEND_TEST_NONE);
>  		sleep(5);
> -		igt_assert(feature->wait_until_update(drm.debugfs_fd));
> +		igt_assert(feature->wait_until_update(drm->debugfs_fd));
>  	}
>  }
>  
> -static void setup_environment(void)
> +static void setup_environment(struct drm_info *drm)
>  {
>  	int drm_fd;
>  
> @@ -300,26 +303,32 @@ static void setup_environment(void)
>  	 */
>  	bind_fbcon(true);
>  	fbcon_blink_enable(true);
> +
> +	drm->fd = -1;
>  }
>  
> -static void teardown_environment(void)
> +static void teardown_environment(struct drm_info *drm)
>  {
> +	if (drm->fd >= 0)
> +		teardown_drm(drm);
>  }
>  
>  igt_main
>  {
> +	struct drm_info drm = {};
drm = { .fd = -1 }; ?
> +
>  	igt_fixture
> -		setup_environment();
> +		setup_environment(&drm);
>  
>  	igt_subtest("fbc")
> -		subtest(&fbc, false);
> +		subtest(&drm, &fbc, false);
>  	igt_subtest("psr")
> -		subtest(&psr, false);
> +		subtest(&drm, &psr, false);
>  	igt_subtest("fbc-suspend")
> -		subtest(&fbc, true);
> +		subtest(&drm, &fbc, true);
>  	igt_subtest("psr-suspend")
> -		subtest(&psr, true);
> +		subtest(&drm, &psr, true);
>  
>  	igt_fixture
> -		teardown_environment();
> +		teardown_environment(&drm);
>  }


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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2 5/8] tests/fbcon_fbt: Add and use psr_long_wait_update() to test updates in PSR
  2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 5/8] tests/fbcon_fbt: Add and use psr_long_wait_update() " José Roberto de Souza
@ 2019-04-09 17:55   ` Maarten Lankhorst
  2019-04-10 21:08     ` Souza, Jose
  0 siblings, 1 reply; 20+ messages in thread
From: Maarten Lankhorst @ 2019-04-09 17:55 UTC (permalink / raw)
  To: José Roberto de Souza, igt-dev; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi

Op 27-03-2019 om 21:03 schreef José Roberto de Souza:
> After getting back to fbcon it tests if PSR had a update but since
> fbcon can take a while it have some big sleep in place, so what it
> is actualy testing is if fbcon is updating the screen.
> In this test the update of the screen comes from the fbcon cursor
> blinking, the problem is that 40ms is a small interval to detect
> cursor blinking and the test can fail some times, so a bigger timeout
> to wait for a update is need.
>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  lib/igt_psr.c         | 5 +++++
>  lib/igt_psr.h         | 1 +
>  tests/kms_fbcon_fbt.c | 2 +-
>  3 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> index b5847bfd..b92ea73f 100644
> --- a/lib/igt_psr.c
> +++ b/lib/igt_psr.c
> @@ -54,6 +54,11 @@ bool psr_wait_update(int debugfs_fd, enum psr_mode mode)
>  	return igt_wait(!psr_active_check(debugfs_fd, mode), 40, 10);
>  }
>  
> +bool psr_long_wait_update(int debugfs_fd, enum psr_mode mode)
> +{
> +	return igt_wait(!psr_active_check(debugfs_fd, mode), 500, 10);
> +}
> +
>  static ssize_t psr_write(int debugfs_fd, const char *buf)
>  {
>  	return igt_sysfs_write(debugfs_fd, "i915_edp_psr_debug", buf,
> diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> index 49599cf8..ca385736 100644
> --- a/lib/igt_psr.h
> +++ b/lib/igt_psr.h
> @@ -37,6 +37,7 @@ enum psr_mode {
>  
>  bool psr_wait_entry(int debugfs_fd, enum psr_mode mode);
>  bool psr_wait_update(int debugfs_fd, enum psr_mode mode);
> +bool psr_long_wait_update(int debugfs_fd, enum psr_mode mode);
>  bool psr_enable(int debugfs_fd, enum psr_mode);
>  bool psr_disable(int debugfs_fd);
>  bool psr_sink_support(int debugfs_fd, enum psr_mode);
> diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
> index b57e5f51..0550034e 100644
> --- a/tests/kms_fbcon_fbt.c
> +++ b/tests/kms_fbcon_fbt.c
> @@ -203,7 +203,7 @@ static bool psr_supported_on_chipset(int debugfs_fd)
>  
>  static bool psr_wait_until_update(int debugfs_fd)
>  {
> -	return psr_wait_update(debugfs_fd, PSR_MODE_1);
> +	return psr_long_wait_update(debugfs_fd, PSR_MODE_1);
>  }
>  
>  static void disable_features(int debugfs_fd)

This patch and 4/8 should be merged? maybe merged with 3/8 as well?

~Maarten

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2 6/8] tests/fbcon_fbt: Enable cursor blinking
  2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 6/8] tests/fbcon_fbt: Enable cursor blinking José Roberto de Souza
@ 2019-04-09 17:59   ` Maarten Lankhorst
  0 siblings, 0 replies; 20+ messages in thread
From: Maarten Lankhorst @ 2019-04-09 17:59 UTC (permalink / raw)
  To: José Roberto de Souza, igt-dev; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi

Op 27-03-2019 om 21:03 schreef José Roberto de Souza:
> If cursor blinking is disabled no screen updates will happen and
> fbcon_fbt subtests will fail, so lets enable cursor blink while
> running this test and restore to the previous value when exiting it.
>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  lib/igt_sysfs.c       | 46 +++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_sysfs.h       |  1 +
>  tests/kms_fbcon_fbt.c |  1 +
>  3 files changed, 48 insertions(+)
>
> diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
> index f806f4fc..904fbd17 100644
> --- a/lib/igt_sysfs.c
> +++ b/lib/igt_sysfs.c
> @@ -610,3 +610,49 @@ void kick_snd_hda_intel(void)
>  out:
>  	close(fd);
>  }
> +
> +static int fbcon_blink_restore_debugfs_fd = -1;
> +static uint8_t fbcon_blink_restore_value;
> +
> +static void fbcon_blink_restore(int sig)
> +{
> +	char buffer[4];
> +	int r;
> +
> +	r = snprintf(buffer, sizeof(buffer), "%u", fbcon_blink_restore_value);
> +	write(fbcon_blink_restore_debugfs_fd, buffer, r + 1);
> +}
> +
> +/**
> + * fbcon_blink_enable:
> + * @enable: if true enables the fbcon cursor blinking otherwise disables it
> + *
> + * Enables or disables the cursor blinking in fbcon, it also restores the
> + * previous blinking state when exiting test.
> + *
> + */
> +void fbcon_blink_enable(bool enable)
> +{
> +	const char *cur_blink_path = "/sys/class/graphics/fbcon/cursor_blink";
> +	char buffer[4];
> +	int fd, r;
> +
> +	fd = open(cur_blink_path, O_RDWR);
> +	igt_assert(fd >= 0);
> +
> +	/* Restore original value on exit */
> +	if (fbcon_blink_restore_debugfs_fd == -1) {
> +		r = read(fd, buffer, sizeof(buffer));
> +		if (r > 0) {
> +			fbcon_blink_restore_value = (uint8_t)strtol(buffer,
> +								    NULL, 10);
> +			fbcon_blink_restore_debugfs_fd = dup(fd);
> +			igt_assert(fbcon_blink_restore_debugfs_fd >= 0);
> +			igt_install_exit_handler(fbcon_blink_restore);
> +		}
> +	}
> +
> +	r = snprintf(buffer, sizeof(buffer), enable ? "1" : "0");
> +	write(fd, buffer, r + 1);
> +	close(fd);
> +}


This looks like it will work, but is it also possible to write to the console to explicitly update it to make it more explicit?

Either way is fine with me.

~Maarten

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2 8/8] tests/fbcon_fbt: Do not keep opening debugfs_fd at every setup_drm() call
  2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 8/8] tests/fbcon_fbt: Do not keep opening debugfs_fd at every setup_drm() call José Roberto de Souza
@ 2019-04-09 18:01   ` Maarten Lankhorst
  0 siblings, 0 replies; 20+ messages in thread
From: Maarten Lankhorst @ 2019-04-09 18:01 UTC (permalink / raw)
  To: José Roberto de Souza, igt-dev

Op 27-03-2019 om 21:03 schreef José Roberto de Souza:
> As debugfs_fd is used even after the teardown_drm() call, it do not
> closes it causing it to open another debugfs_fd at every call to
> setup_drm().
> Let move the opening of debugfs_fd to setup_environment() and only
> open it once, also lets close it before leave the test.
>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  tests/kms_fbcon_fbt.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
> index 66f214c5..f7ff671b 100644
> --- a/tests/kms_fbcon_fbt.c
> +++ b/tests/kms_fbcon_fbt.c
> @@ -64,7 +64,6 @@ static void setup_drm(struct drm_info *drm)
>  		return;
>  
>  	drm->fd = drm_open_driver_master(DRIVER_INTEL);
> -	drm->debugfs_fd = igt_debugfs_dir(drm->fd);
>  
>  	drm->res = drmModeGetResources(drm->fd);
>  	igt_require(drm->res);
> @@ -295,6 +294,7 @@ static void setup_environment(struct drm_info *drm)
>  
>  	drm_fd = drm_open_driver_master(DRIVER_INTEL);
>  	igt_require(drm_fd >= 0);
> +	drm->debugfs_fd = igt_debugfs_dir(drm_fd);
>  	igt_assert(close(drm_fd) == 0);
>  
>  	/*
> @@ -311,6 +311,8 @@ static void teardown_environment(struct drm_info *drm)
>  {
>  	if (drm->fd >= 0)
>  		teardown_drm(drm);
> +
> +	close(drm->debugfs_fd);
>  }
>  
>  igt_main

For whole series, when merging patch 4/8 and 5/8, possibly with 3/8:

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2 3/8] tests/fbcon_fbt: Add wait_until_update() callback to features
  2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 3/8] tests/fbcon_fbt: Add wait_until_update() callback to features José Roberto de Souza
@ 2019-04-09 21:41   ` Dhinakaran Pandiyan
  2019-04-10 20:31     ` Souza, Jose
  0 siblings, 1 reply; 20+ messages in thread
From: Dhinakaran Pandiyan @ 2019-04-09 21:41 UTC (permalink / raw)
  To: José Roberto de Souza, igt-dev; +Cc: Rodrigo Vivi

On Wed, 2019-03-27 at 13:03 -0700, José Roberto de Souza wrote:
> Lets be more explicit and add and implement a callback to check if
> feature had a state update, that is what some points of the test want
> to test.
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  tests/kms_fbcon_fbt.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
> index a9d91839..9ab1d630 100644
> --- a/tests/kms_fbcon_fbt.c
> +++ b/tests/kms_fbcon_fbt.c
> @@ -130,6 +130,11 @@ static bool fbc_wait_until_enabled(int debugfs_fd)
>  	return r;
>  }
>  
> +static bool fbc_wait_until_update(int debugfs)
> +{
> +	return !fbc_wait_until_enabled(debugfs);
> +}
> +
>  typedef bool (*connector_possible_fn)(drmModeConnectorPtr connector);
>  
>  static void set_mode_for_one_screen(struct drm_info *drm, struct igt_fb *fb,
> @@ -196,6 +201,11 @@ static bool psr_supported_on_chipset(int debugfs_fd)
>  	return psr_sink_support(debugfs_fd, PSR_MODE_1);
>  }
>  
> +static bool psr_wait_until_update(int debugfs_fd)
> +{
> +	return !psr_wait_until_enabled(debugfs_fd);
> +}
> +
>  static void disable_features(int debugfs_fd)
>  {
>  	igt_set_module_param_int("enable_fbc", 0);
> @@ -215,16 +225,19 @@ static inline void psr_debugfs_enable(int debugfs_fd)
>  struct feature {
>  	bool (*supported_on_chipset)(int debugfs_fd);
>  	bool (*wait_until_enabled)(int debugfs_fd);
> +	bool (*wait_until_update)(int debugfs_fd);
>  	bool (*connector_possible_fn)(drmModeConnectorPtr connector);
>  	void (*enable)(int debugfs_fd);
>  } fbc = {
>  	.supported_on_chipset = fbc_supported_on_chipset,
>  	.wait_until_enabled = fbc_wait_until_enabled,
> +	.wait_until_update = fbc_wait_until_update,
>  	.connector_possible_fn = connector_can_fbc,
>  	.enable = fbc_modparam_enable,
>  }, psr = {
>  	.supported_on_chipset = psr_supported_on_chipset,
>  	.wait_until_enabled = psr_wait_until_enabled,
> +	.wait_until_update = psr_wait_until_update,
>  	.connector_possible_fn = connector_can_psr,
>  	.enable = psr_debugfs_enable,
>  };
> @@ -243,7 +256,7 @@ 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.debugfs_fd));
> +	igt_assert(feature->wait_until_update(drm.debugfs_fd));
I believe the intention here is to test if PSR and FBC got disabled when pipes
are disabled.

>  
>  	set_mode_for_one_screen(&drm, &fb, feature->connector_possible_fn);
>  	wait_user("Screen set.");
> @@ -263,13 +276,13 @@ static void subtest(struct feature *feature, bool
> suspend)
>  	sleep(3);
>  
>  	wait_user("Back to fbcon.");
> -	igt_assert(!feature->wait_until_enabled(drm.debugfs_fd));
> +	igt_assert(feature->wait_until_update(drm.debugfs_fd));
>  
And here, I believe the expectation is that fbcon leads to frontbuffer
invalidate and thus disabling PSR and FBC.

>  	if (suspend) {
>  		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
>  					      SUSPEND_TEST_NONE);
>  		sleep(5);
> -		igt_assert(!feature->wait_until_enabled(drm.debugfs_fd));
> +		igt_assert(feature->wait_until_update(drm.debugfs_fd));
>  	}
>  }
>  

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2 3/8] tests/fbcon_fbt: Add wait_until_update() callback to features
  2019-04-09 21:41   ` Dhinakaran Pandiyan
@ 2019-04-10 20:31     ` Souza, Jose
  2019-04-10 20:42       ` Pandiyan, Dhinakaran
  0 siblings, 1 reply; 20+ messages in thread
From: Souza, Jose @ 2019-04-10 20:31 UTC (permalink / raw)
  To: igt-dev, Pandiyan, Dhinakaran; +Cc: Vivi, Rodrigo


[-- Attachment #1.1: Type: text/plain, Size: 4082 bytes --]

On Tue, 2019-04-09 at 14:41 -0700, Dhinakaran Pandiyan wrote:
> On Wed, 2019-03-27 at 13:03 -0700, José Roberto de Souza wrote:
> > Lets be more explicit and add and implement a callback to check if
> > feature had a state update, that is what some points of the test
> > want
> > to test.
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  tests/kms_fbcon_fbt.c | 19 ++++++++++++++++---
> >  1 file changed, 16 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
> > index a9d91839..9ab1d630 100644
> > --- a/tests/kms_fbcon_fbt.c
> > +++ b/tests/kms_fbcon_fbt.c
> > @@ -130,6 +130,11 @@ static bool fbc_wait_until_enabled(int
> > debugfs_fd)
> >  	return r;
> >  }
> >  
> > +static bool fbc_wait_until_update(int debugfs)
> > +{
> > +	return !fbc_wait_until_enabled(debugfs);
> > +}
> > +
> >  typedef bool (*connector_possible_fn)(drmModeConnectorPtr
> > connector);
> >  
> >  static void set_mode_for_one_screen(struct drm_info *drm, struct
> > igt_fb *fb,
> > @@ -196,6 +201,11 @@ static bool psr_supported_on_chipset(int
> > debugfs_fd)
> >  	return psr_sink_support(debugfs_fd, PSR_MODE_1);
> >  }
> >  
> > +static bool psr_wait_until_update(int debugfs_fd)
> > +{
> > +	return !psr_wait_until_enabled(debugfs_fd);
> > +}
> > +
> >  static void disable_features(int debugfs_fd)
> >  {
> >  	igt_set_module_param_int("enable_fbc", 0);
> > @@ -215,16 +225,19 @@ static inline void psr_debugfs_enable(int
> > debugfs_fd)
> >  struct feature {
> >  	bool (*supported_on_chipset)(int debugfs_fd);
> >  	bool (*wait_until_enabled)(int debugfs_fd);
> > +	bool (*wait_until_update)(int debugfs_fd);
> >  	bool (*connector_possible_fn)(drmModeConnectorPtr connector);
> >  	void (*enable)(int debugfs_fd);
> >  } fbc = {
> >  	.supported_on_chipset = fbc_supported_on_chipset,
> >  	.wait_until_enabled = fbc_wait_until_enabled,
> > +	.wait_until_update = fbc_wait_until_update,
> >  	.connector_possible_fn = connector_can_fbc,
> >  	.enable = fbc_modparam_enable,
> >  }, psr = {
> >  	.supported_on_chipset = psr_supported_on_chipset,
> >  	.wait_until_enabled = psr_wait_until_enabled,
> > +	.wait_until_update = psr_wait_until_update,
> >  	.connector_possible_fn = connector_can_psr,
> >  	.enable = psr_debugfs_enable,
> >  };
> > @@ -243,7 +256,7 @@ 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.debugfs_fd));
> > +	igt_assert(feature->wait_until_update(drm.debugfs_fd));
> I believe the intention here is to test if PSR and FBC got disabled
> when pipes
> are disabled.

Yeah, here pipes will be disable so all the features, so here should we
keep: igt_assert(!feature->wait_until_enabled(drm.debugfs_fd));

> 
> >  
> >  	set_mode_for_one_screen(&drm, &fb, feature-
> > >connector_possible_fn);
> >  	wait_user("Screen set.");
> > @@ -263,13 +276,13 @@ static void subtest(struct feature *feature,
> > bool
> > suspend)
> >  	sleep(3);
> >  
> >  	wait_user("Back to fbcon.");
> > -	igt_assert(!feature->wait_until_enabled(drm.debugfs_fd));
> > +	igt_assert(feature->wait_until_update(drm.debugfs_fd));
> >  
> And here, I believe the expectation is that fbcon leads to
> frontbuffer
> invalidate and thus disabling PSR and FBC.

It will disable PSR while doing the full modeset but in fbcon PSR will
be enabled between cursor blinks, that is why we need to check for
update here.

> 
> >  	if (suspend) {
> >  		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> >  					      SUSPEND_TEST_NONE);
> >  		sleep(5);
> > -		igt_assert(!feature-
> > >wait_until_enabled(drm.debugfs_fd));
> > +		igt_assert(feature->wait_until_update(drm.debugfs_fd));
> >  	}
> >  }
> >  

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2 3/8] tests/fbcon_fbt: Add wait_until_update() callback to features
  2019-04-10 20:31     ` Souza, Jose
@ 2019-04-10 20:42       ` Pandiyan, Dhinakaran
  2019-04-10 22:04         ` Souza, Jose
  0 siblings, 1 reply; 20+ messages in thread
From: Pandiyan, Dhinakaran @ 2019-04-10 20:42 UTC (permalink / raw)
  To: Souza, Jose, igt-dev; +Cc: Vivi, Rodrigo



> -----Original Message-----
> From: Souza, Jose
> Sent: Wednesday, April 10, 2019 1:32 PM
> To: igt-dev@lists.freedesktop.org; Pandiyan, Dhinakaran
> <dhinakaran.pandiyan@intel.com>
> Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>
> Subject: Re: [igt-dev] [PATCH i-g-t v2 3/8] tests/fbcon_fbt: Add
> wait_until_update() callback to features
> 
> On Tue, 2019-04-09 at 14:41 -0700, Dhinakaran Pandiyan wrote:
> > On Wed, 2019-03-27 at 13:03 -0700, José Roberto de Souza wrote:
> > > Lets be more explicit and add and implement a callback to check if
> > > feature had a state update, that is what some points of the test
> > > want to test.
> > >
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  tests/kms_fbcon_fbt.c | 19 ++++++++++++++++---
> > >  1 file changed, 16 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c index
> > > a9d91839..9ab1d630 100644
> > > --- a/tests/kms_fbcon_fbt.c
> > > +++ b/tests/kms_fbcon_fbt.c
> > > @@ -130,6 +130,11 @@ static bool fbc_wait_until_enabled(int
> > > debugfs_fd)
> > >  	return r;
> > >  }
> > >
> > > +static bool fbc_wait_until_update(int debugfs) {
> > > +	return !fbc_wait_until_enabled(debugfs); }
> > > +
> > >  typedef bool (*connector_possible_fn)(drmModeConnectorPtr
> > > connector);
> > >
> > >  static void set_mode_for_one_screen(struct drm_info *drm, struct
> > > igt_fb *fb, @@ -196,6 +201,11 @@ static bool
> > > psr_supported_on_chipset(int
> > > debugfs_fd)
> > >  	return psr_sink_support(debugfs_fd, PSR_MODE_1);  }
> > >
> > > +static bool psr_wait_until_update(int debugfs_fd) {
> > > +	return !psr_wait_until_enabled(debugfs_fd);
> > > +}
> > > +
> > >  static void disable_features(int debugfs_fd)  {
> > >  	igt_set_module_param_int("enable_fbc", 0); @@ -215,16 +225,19
> @@
> > > static inline void psr_debugfs_enable(int
> > > debugfs_fd)
> > >  struct feature {
> > >  	bool (*supported_on_chipset)(int debugfs_fd);
> > >  	bool (*wait_until_enabled)(int debugfs_fd);
> > > +	bool (*wait_until_update)(int debugfs_fd);
> > >  	bool (*connector_possible_fn)(drmModeConnectorPtr connector);
> > >  	void (*enable)(int debugfs_fd);
> > >  } fbc = {
> > >  	.supported_on_chipset = fbc_supported_on_chipset,
> > >  	.wait_until_enabled = fbc_wait_until_enabled,
> > > +	.wait_until_update = fbc_wait_until_update,
> > >  	.connector_possible_fn = connector_can_fbc,
> > >  	.enable = fbc_modparam_enable,
> > >  }, psr = {
> > >  	.supported_on_chipset = psr_supported_on_chipset,
> > >  	.wait_until_enabled = psr_wait_until_enabled,
> > > +	.wait_until_update = psr_wait_until_update,
> > >  	.connector_possible_fn = connector_can_psr,
> > >  	.enable = psr_debugfs_enable,
> > >  };
> > > @@ -243,7 +256,7 @@ 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.debugfs_fd));
> > > +	igt_assert(feature->wait_until_update(drm.debugfs_fd));
> > I believe the intention here is to test if PSR and FBC got disabled
> > when pipes are disabled.
> 
> Yeah, here pipes will be disable so all the features, so here should we
> keep: igt_assert(!feature->wait_until_enabled(drm.debugfs_fd));
Yup.

> 
> >
> > >
> > >  	set_mode_for_one_screen(&drm, &fb, feature-
> > > >connector_possible_fn);
> > >  	wait_user("Screen set.");
> > > @@ -263,13 +276,13 @@ static void subtest(struct feature *feature,
> > > bool
> > > suspend)
> > >  	sleep(3);
> > >
> > >  	wait_user("Back to fbcon.");
> > > -	igt_assert(!feature->wait_until_enabled(drm.debugfs_fd));
> > > +	igt_assert(feature->wait_until_update(drm.debugfs_fd));
> > >
> > And here, I believe the expectation is that fbcon leads to frontbuffer
> > invalidate and thus disabling PSR and FBC.
> 
> It will disable PSR while doing the full modeset but in fbcon PSR will be
> enabled between cursor blinks, that is why we need to check for update
> here.
> 
I understand, I have seen that myself. Not sure at what point it changed from
fbcon completely disabling PSR and FBC with frontbuffer_invalidate() to the current
behavior. iirc we just get frontbuffer_flush() now. TBH, I don't understand fbcon that
well.

What's the origin value in frontbuffer_invalidate/flush()? Do we get flips for 
cursor blinks? Or is it a fronbuffer_invalidate() followed by a frontbuffer_flush()?

-DK

> >
> > >  	if (suspend) {
> > >  		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> > >  					      SUSPEND_TEST_NONE);
> > >  		sleep(5);
> > > -		igt_assert(!feature-
> > > >wait_until_enabled(drm.debugfs_fd));
> > > +		igt_assert(feature->wait_until_update(drm.debugfs_fd));
> > >  	}
> > >  }
> > >
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2 7/8] tests/fbcon_fbt: Do not try to open the same driver twice as master
  2019-04-09 17:55   ` Maarten Lankhorst
@ 2019-04-10 21:07     ` Souza, Jose
  0 siblings, 0 replies; 20+ messages in thread
From: Souza, Jose @ 2019-04-10 21:07 UTC (permalink / raw)
  To: igt-dev, maarten.lankhorst


[-- Attachment #1.1: Type: text/plain, Size: 7289 bytes --]

On Tue, 2019-04-09 at 19:55 +0200, Maarten Lankhorst wrote:
> Op 27-03-2019 om 21:03 schreef José Roberto de Souza:
> > If a subtest fail before the teardown_drm() call it will keep the
> > driver open as master causing the test to fail when trying to open
> > the driver again as master, so lets share the drm_info struct and
> > check if driver is already open.
> > 
> > Starting subtest: psr
> > (kms_fbcon_fbt:5270) CRITICAL: Test assertion failure function
> > subtest, file ../tests/kms_fbcon_fbt.c:259:
> > (kms_fbcon_fbt:5270) CRITICAL: Failed assertion: feature-
> > >wait_until_update(drm.debugfs_fd)
> > Stack trace:
> >   #0 ../lib/igt_core.c:1474 __igt_fail_assert()
> >   #1 ../tests/kms_fbcon_fbt.c:261 subtest()
> >   #2 ../tests/kms_fbcon_fbt.c:316 __real_main309()
> >   #3 ../tests/kms_fbcon_fbt.c:309 main()
> >   #4 ../csu/libc-start.c:325 __libc_start_main()
> >   #5 [_start+0x29]
> >   #6 [<unknown>+0x0]
> > Subtest psr failed.
> > **** DEBUG ****
> > (kms_fbcon_fbt:5270) drmtest-DEBUG: Test requirement passed:
> > !(fd<0)
> > (kms_fbcon_fbt:5270) igt_debugfs-DEBUG: Opening debugfs directory
> > '/sys/kernel/debug/dri/1'
> > (kms_fbcon_fbt:5270) DEBUG: Test requirement passed: drm->res
> > (kms_fbcon_fbt:5270) igt_kms-DEBUG: VT: graphics mode set (mode was
> > 0x0)
> > (kms_fbcon_fbt:5270) DEBUG: Test requirement passed: feature-
> > >supported_on_chipset(drm.debugfs_fd)
> > (kms_fbcon_fbt:5270) CRITICAL: Test assertion failure function
> > subtest, file ../tests/kms_fbcon_fbt.c:259:
> > (kms_fbcon_fbt:5270) CRITICAL: Failed assertion: feature-
> > >wait_until_update(drm.debugfs_fd)
> > (kms_fbcon_fbt:5270) igt_core-INFO: Stack trace:
> > (kms_fbcon_fbt:5270) igt_core-INFO:   #0 ../lib/igt_core.c:1474
> > __igt_fail_assert()
> > (kms_fbcon_fbt:5270) igt_core-INFO:   #1
> > ../tests/kms_fbcon_fbt.c:261 subtest()
> > (kms_fbcon_fbt:5270) igt_core-INFO:   #2
> > ../tests/kms_fbcon_fbt.c:316 __real_main309()
> > (kms_fbcon_fbt:5270) igt_core-INFO:   #3
> > ../tests/kms_fbcon_fbt.c:309 main()
> > (kms_fbcon_fbt:5270) igt_core-INFO:   #4 ../csu/libc-start.c:325
> > __libc_start_main()
> > (kms_fbcon_fbt:5270) igt_core-INFO:   #5 [_start+0x29]
> > (kms_fbcon_fbt:5270) igt_core-INFO:   #6 [<unknown>+0x0]
> > ****  END  ****
> > Subtest psr: FAIL (0.845s)
> > Starting subtest: fbc-suspend
> > Test requirement not met in function igt_device_set_master, file
> > ../lib/igt_device.c:55:
> > Test requirement: __igt_device_set_master(fd) == 0
> > Can't become DRM master, please check if no other DRM client is
> > running.
> > Subtest fbc-suspend: SKIP (0.001s)
> > Starting subtest: psr-suspend
> > Test requirement not met in function igt_device_set_master, file
> > ../lib/igt_device.c:55:
> > Test requirement: __igt_device_set_master(fd) == 0
> > Can't become DRM master, please check if no other DRM client is
> > running.
> > Subtest psr-suspend: SKIP (0.000s)
> > 
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  tests/kms_fbcon_fbt.c | 55 +++++++++++++++++++++++++------------
> > ------
> >  1 file changed, 32 insertions(+), 23 deletions(-)
> > 
> > diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
> > index 2f158e2f..66f214c5 100644
> > --- a/tests/kms_fbcon_fbt.c
> > +++ b/tests/kms_fbcon_fbt.c
> > @@ -60,6 +60,9 @@ static void setup_drm(struct drm_info *drm)
> >  {
> >  	int i;
> >  
> > +	if (drm->fd >= 0)
> > +		return;
> > +
> >  	drm->fd = drm_open_driver_master(DRIVER_INTEL);
> >  	drm->debugfs_fd = igt_debugfs_dir(drm->fd);
> >  
> > @@ -85,6 +88,7 @@ static void teardown_drm(struct drm_info *drm)
> >  
> >  	drmModeFreeResources(drm->res);
> >  	igt_assert(close(drm->fd) == 0);
> > +	drm->fd = -1;
> >  }
> >  
> >  static bool fbc_supported_on_chipset(int debugfs_fd)
> > @@ -242,51 +246,50 @@ struct feature {
> >  	.enable = psr_debugfs_enable,
> >  };
> >  
> > -static void subtest(struct feature *feature, bool suspend)
> > +static void subtest(struct drm_info *drm, struct feature *feature,
> > bool suspend)
> >  {
> > -	struct drm_info drm;
> >  	struct igt_fb fb;
> >  
> > -	setup_drm(&drm);
> > +	setup_drm(drm);
> >  
> > -	igt_require(feature->supported_on_chipset(drm.debugfs_fd));
> > +	igt_require(feature->supported_on_chipset(drm->debugfs_fd));
> >  
> > -	disable_features(drm.debugfs_fd);
> > -	feature->enable(drm.debugfs_fd);
> > +	disable_features(drm->debugfs_fd);
> > +	feature->enable(drm->debugfs_fd);
> >  
> > -	kmstest_unset_all_crtcs(drm.fd, drm.res);
> > +	kmstest_unset_all_crtcs(drm->fd, drm->res);
> >  	wait_user("Modes unset.");
> > -	igt_assert(feature->wait_until_update(drm.debugfs_fd));
> > +	igt_assert(feature->wait_until_update(drm->debugfs_fd));
> >  
> > -	set_mode_for_one_screen(&drm, &fb, feature-
> > >connector_possible_fn);
> > +	set_mode_for_one_screen(drm, &fb, feature-
> > >connector_possible_fn);
> >  	wait_user("Screen set.");
> > -	igt_assert(feature->wait_until_enabled(drm.debugfs_fd));
> > +	igt_assert(feature->wait_until_enabled(drm->debugfs_fd));
> >  
> >  	if (suspend) {
> >  		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> >  					      SUSPEND_TEST_NONE);
> >  		sleep(5);
> > -		igt_assert(feature-
> > >wait_until_enabled(drm.debugfs_fd));
> > +		igt_assert(feature->wait_until_enabled(drm-
> > >debugfs_fd));
> >  	}
> >  
> > -	igt_remove_fb(drm.fd, &fb);
> > -	teardown_drm(&drm);
> > +	igt_remove_fb(drm->fd, &fb);
> > +	teardown_drm(drm);
> >  
> >  	/* Wait for fbcon to restore itself. */
> >  	sleep(3);
> >  
> >  	wait_user("Back to fbcon.");
> > -	igt_assert(feature->wait_until_update(drm.debugfs_fd));
> > +	igt_assert(feature->wait_until_update(drm->debugfs_fd));
> >  
> >  	if (suspend) {
> >  		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> >  					      SUSPEND_TEST_NONE);
> >  		sleep(5);
> > -		igt_assert(feature->wait_until_update(drm.debugfs_fd));
> > +		igt_assert(feature->wait_until_update(drm-
> > >debugfs_fd));
> >  	}
> >  }
> >  
> > -static void setup_environment(void)
> > +static void setup_environment(struct drm_info *drm)
> >  {
> >  	int drm_fd;
> >  
> > @@ -300,26 +303,32 @@ static void setup_environment(void)
> >  	 */
> >  	bind_fbcon(true);
> >  	fbcon_blink_enable(true);
> > +
> > +	drm->fd = -1;
> >  }
> >  
> > -static void teardown_environment(void)
> > +static void teardown_environment(struct drm_info *drm)
> >  {
> > +	if (drm->fd >= 0)
> > +		teardown_drm(drm);
> >  }
> >  
> >  igt_main
> >  {
> > +	struct drm_info drm = {};
> drm = { .fd = -1 }; ?

Thanks

> > +
> >  	igt_fixture
> > -		setup_environment();
> > +		setup_environment(&drm);
> >  
> >  	igt_subtest("fbc")
> > -		subtest(&fbc, false);
> > +		subtest(&drm, &fbc, false);
> >  	igt_subtest("psr")
> > -		subtest(&psr, false);
> > +		subtest(&drm, &psr, false);
> >  	igt_subtest("fbc-suspend")
> > -		subtest(&fbc, true);
> > +		subtest(&drm, &fbc, true);
> >  	igt_subtest("psr-suspend")
> > -		subtest(&psr, true);
> > +		subtest(&drm, &psr, true);
> >  
> >  	igt_fixture
> > -		teardown_environment();
> > +		teardown_environment(&drm);
> >  }
> 
> 

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2 5/8] tests/fbcon_fbt: Add and use psr_long_wait_update() to test updates in PSR
  2019-04-09 17:55   ` Maarten Lankhorst
@ 2019-04-10 21:08     ` Souza, Jose
  0 siblings, 0 replies; 20+ messages in thread
From: Souza, Jose @ 2019-04-10 21:08 UTC (permalink / raw)
  To: igt-dev, maarten.lankhorst; +Cc: Pandiyan, Dhinakaran, Vivi, Rodrigo


[-- Attachment #1.1: Type: text/plain, Size: 2726 bytes --]

On Tue, 2019-04-09 at 19:55 +0200, Maarten Lankhorst wrote:
> Op 27-03-2019 om 21:03 schreef José Roberto de Souza:
> > After getting back to fbcon it tests if PSR had a update but since
> > fbcon can take a while it have some big sleep in place, so what it
> > is actualy testing is if fbcon is updating the screen.
> > In this test the update of the screen comes from the fbcon cursor
> > blinking, the problem is that 40ms is a small interval to detect
> > cursor blinking and the test can fail some times, so a bigger
> > timeout
> > to wait for a update is need.
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  lib/igt_psr.c         | 5 +++++
> >  lib/igt_psr.h         | 1 +
> >  tests/kms_fbcon_fbt.c | 2 +-
> >  3 files changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> > index b5847bfd..b92ea73f 100644
> > --- a/lib/igt_psr.c
> > +++ b/lib/igt_psr.c
> > @@ -54,6 +54,11 @@ bool psr_wait_update(int debugfs_fd, enum
> > psr_mode mode)
> >  	return igt_wait(!psr_active_check(debugfs_fd, mode), 40, 10);
> >  }
> >  
> > +bool psr_long_wait_update(int debugfs_fd, enum psr_mode mode)
> > +{
> > +	return igt_wait(!psr_active_check(debugfs_fd, mode), 500, 10);
> > +}
> > +
> >  static ssize_t psr_write(int debugfs_fd, const char *buf)
> >  {
> >  	return igt_sysfs_write(debugfs_fd, "i915_edp_psr_debug", buf,
> > diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> > index 49599cf8..ca385736 100644
> > --- a/lib/igt_psr.h
> > +++ b/lib/igt_psr.h
> > @@ -37,6 +37,7 @@ enum psr_mode {
> >  
> >  bool psr_wait_entry(int debugfs_fd, enum psr_mode mode);
> >  bool psr_wait_update(int debugfs_fd, enum psr_mode mode);
> > +bool psr_long_wait_update(int debugfs_fd, enum psr_mode mode);
> >  bool psr_enable(int debugfs_fd, enum psr_mode);
> >  bool psr_disable(int debugfs_fd);
> >  bool psr_sink_support(int debugfs_fd, enum psr_mode);
> > diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
> > index b57e5f51..0550034e 100644
> > --- a/tests/kms_fbcon_fbt.c
> > +++ b/tests/kms_fbcon_fbt.c
> > @@ -203,7 +203,7 @@ static bool psr_supported_on_chipset(int
> > debugfs_fd)
> >  
> >  static bool psr_wait_until_update(int debugfs_fd)
> >  {
> > -	return psr_wait_update(debugfs_fd, PSR_MODE_1);
> > +	return psr_long_wait_update(debugfs_fd, PSR_MODE_1);
> >  }
> >  
> >  static void disable_features(int debugfs_fd)
> 
> This patch and 4/8 should be merged? maybe merged with 3/8 as well?

Done, can you give a review to the squashed patch?

> 
> ~Maarten
> 

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2 3/8] tests/fbcon_fbt: Add wait_until_update() callback to features
  2019-04-10 20:42       ` Pandiyan, Dhinakaran
@ 2019-04-10 22:04         ` Souza, Jose
  0 siblings, 0 replies; 20+ messages in thread
From: Souza, Jose @ 2019-04-10 22:04 UTC (permalink / raw)
  To: igt-dev, Pandiyan, Dhinakaran; +Cc: Vivi, Rodrigo


[-- Attachment #1.1: Type: text/plain, Size: 5634 bytes --]

On Wed, 2019-04-10 at 13:42 -0700, Pandiyan, Dhinakaran wrote:
> > -----Original Message-----
> > From: Souza, Jose
> > Sent: Wednesday, April 10, 2019 1:32 PM
> > To: igt-dev@lists.freedesktop.org; Pandiyan, Dhinakaran
> > <dhinakaran.pandiyan@intel.com>
> > Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>
> > Subject: Re: [igt-dev] [PATCH i-g-t v2 3/8] tests/fbcon_fbt: Add
> > wait_until_update() callback to features
> > 
> > On Tue, 2019-04-09 at 14:41 -0700, Dhinakaran Pandiyan wrote:
> > > On Wed, 2019-03-27 at 13:03 -0700, José Roberto de Souza wrote:
> > > > Lets be more explicit and add and implement a callback to check
> > > > if
> > > > feature had a state update, that is what some points of the
> > > > test
> > > > want to test.
> > > > 
> > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > ---
> > > >  tests/kms_fbcon_fbt.c | 19 ++++++++++++++++---
> > > >  1 file changed, 16 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
> > > > index
> > > > a9d91839..9ab1d630 100644
> > > > --- a/tests/kms_fbcon_fbt.c
> > > > +++ b/tests/kms_fbcon_fbt.c
> > > > @@ -130,6 +130,11 @@ static bool fbc_wait_until_enabled(int
> > > > debugfs_fd)
> > > >  	return r;
> > > >  }
> > > > 
> > > > +static bool fbc_wait_until_update(int debugfs) {
> > > > +	return !fbc_wait_until_enabled(debugfs); }
> > > > +
> > > >  typedef bool (*connector_possible_fn)(drmModeConnectorPtr
> > > > connector);
> > > > 
> > > >  static void set_mode_for_one_screen(struct drm_info *drm,
> > > > struct
> > > > igt_fb *fb, @@ -196,6 +201,11 @@ static bool
> > > > psr_supported_on_chipset(int
> > > > debugfs_fd)
> > > >  	return psr_sink_support(debugfs_fd, PSR_MODE_1);  }
> > > > 
> > > > +static bool psr_wait_until_update(int debugfs_fd) {
> > > > +	return !psr_wait_until_enabled(debugfs_fd);
> > > > +}
> > > > +
> > > >  static void disable_features(int debugfs_fd)  {
> > > >  	igt_set_module_param_int("enable_fbc", 0); @@ -215,16
> > > > +225,19
> > @@
> > > > static inline void psr_debugfs_enable(int
> > > > debugfs_fd)
> > > >  struct feature {
> > > >  	bool (*supported_on_chipset)(int debugfs_fd);
> > > >  	bool (*wait_until_enabled)(int debugfs_fd);
> > > > +	bool (*wait_until_update)(int debugfs_fd);
> > > >  	bool (*connector_possible_fn)(drmModeConnectorPtr
> > > > connector);
> > > >  	void (*enable)(int debugfs_fd);
> > > >  } fbc = {
> > > >  	.supported_on_chipset = fbc_supported_on_chipset,
> > > >  	.wait_until_enabled = fbc_wait_until_enabled,
> > > > +	.wait_until_update = fbc_wait_until_update,
> > > >  	.connector_possible_fn = connector_can_fbc,
> > > >  	.enable = fbc_modparam_enable,
> > > >  }, psr = {
> > > >  	.supported_on_chipset = psr_supported_on_chipset,
> > > >  	.wait_until_enabled = psr_wait_until_enabled,
> > > > +	.wait_until_update = psr_wait_until_update,
> > > >  	.connector_possible_fn = connector_can_psr,
> > > >  	.enable = psr_debugfs_enable,
> > > >  };
> > > > @@ -243,7 +256,7 @@ 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.debugfs_fd));
> > > > +	igt_assert(feature->wait_until_update(drm.debugfs_fd));
> > > I believe the intention here is to test if PSR and FBC got
> > > disabled
> > > when pipes are disabled.
> > 
> > Yeah, here pipes will be disable so all the features, so here
> > should we
> > keep: igt_assert(!feature->wait_until_enabled(drm.debugfs_fd));
> Yup.
> 
> > > >  	set_mode_for_one_screen(&drm, &fb, feature-
> > > > > connector_possible_fn);
> > > >  	wait_user("Screen set.");
> > > > @@ -263,13 +276,13 @@ static void subtest(struct feature
> > > > *feature,
> > > > bool
> > > > suspend)
> > > >  	sleep(3);
> > > > 
> > > >  	wait_user("Back to fbcon.");
> > > > -	igt_assert(!feature-
> > > > >wait_until_enabled(drm.debugfs_fd));
> > > > +	igt_assert(feature->wait_until_update(drm.debugfs_fd));
> > > > 
> > > And here, I believe the expectation is that fbcon leads to
> > > frontbuffer
> > > invalidate and thus disabling PSR and FBC.
> > 
> > It will disable PSR while doing the full modeset but in fbcon PSR
> > will be
> > enabled between cursor blinks, that is why we need to check for
> > update
> > here.
> > 
> I understand, I have seen that myself. Not sure at what point it
> changed from
> fbcon completely disabling PSR and FBC with frontbuffer_invalidate()
> to the current
> behavior. iirc we just get frontbuffer_flush() now. TBH, I don't
> understand fbcon that
> well.
> 
> What's the origin value in frontbuffer_invalidate/flush()? Do we get
> flips for 
> cursor blinks? Or is it a fronbuffer_invalidate() followed by a
> frontbuffer_flush()?

Origin is = 4(ORIGIN_DIRTYFB) and it is only intel_psr_flush() calls so
we just poke cursor registers to force a PSR exit.

> 
> -DK
> 
> > > >  	if (suspend) {
> > > >  		igt_system_suspend_autoresume(SUSPEND_STATE_MEM
> > > > ,
> > > >  					      SUSPEND_TEST_NONE
> > > > );
> > > >  		sleep(5);
> > > > -		igt_assert(!feature-
> > > > > wait_until_enabled(drm.debugfs_fd));
> > > > +		igt_assert(feature-
> > > > >wait_until_update(drm.debugfs_fd));
> > > >  	}
> > > >  }
> > > > 

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2019-04-10 22:04 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-27 20:03 [igt-dev] [PATCH i-g-t v2 1/8] tests/fbcon_fbt: Use psr_wait_entry() to wait PSR be enabled José Roberto de Souza
2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 2/8] tests/fbcon_fbt: Allow fbcon to bind when running this test José Roberto de Souza
2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 3/8] tests/fbcon_fbt: Add wait_until_update() callback to features José Roberto de Souza
2019-04-09 21:41   ` Dhinakaran Pandiyan
2019-04-10 20:31     ` Souza, Jose
2019-04-10 20:42       ` Pandiyan, Dhinakaran
2019-04-10 22:04         ` Souza, Jose
2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 4/8] tests/fbcon_fbt: Use psr_wait_update() to test updates in PSR José Roberto de Souza
2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 5/8] tests/fbcon_fbt: Add and use psr_long_wait_update() " José Roberto de Souza
2019-04-09 17:55   ` Maarten Lankhorst
2019-04-10 21:08     ` Souza, Jose
2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 6/8] tests/fbcon_fbt: Enable cursor blinking José Roberto de Souza
2019-04-09 17:59   ` Maarten Lankhorst
2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 7/8] tests/fbcon_fbt: Do not try to open the same driver twice as master José Roberto de Souza
2019-04-09 17:55   ` Maarten Lankhorst
2019-04-10 21:07     ` Souza, Jose
2019-03-27 20:03 ` [igt-dev] [PATCH i-g-t v2 8/8] tests/fbcon_fbt: Do not keep opening debugfs_fd at every setup_drm() call José Roberto de Souza
2019-04-09 18:01   ` Maarten Lankhorst
2019-03-27 21:01 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v2,1/8] tests/fbcon_fbt: Use psr_wait_entry() to wait PSR be enabled Patchwork
2019-03-28 17:44 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.