<!DOCTYPE html><html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<p><br>
</p>
<div class="moz-cite-prefix">On 8/6/2024 6:20 PM, Kamil Konieczny
wrote:<br>
</div>
<blockquote type="cite" cite="mid:20240806125020.jygbaulfsnpv6rqd@kamilkon-DESK.igk.intel.com">
<pre class="moz-quote-pre" wrap="">Hi Sundaresan,,
On 2024-08-06 at 11:50:46 +0530, Sundaresan, Sujaritha wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">
On 8/1/2024 5:15 PM, Kamil Konieczny wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">Hi Sujaritha,
On 2024-07-30 at 17:05:08 +0530, Sujaritha Sundaresan wrote:
small nit about subject, you wrote:
[PATCH i-g-t, v3] tests/intel: Add tests to run suspend without display
imho this should be:
[PATCH i-g-t, v3] tests/intel/xe_pm: Add tests for suspend without display
More nits below.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Hey Kamil,
Sure this change I can make.
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">Add tests to validate basic execution suspend/resume cycle
without display module to rule out display related issues
from the suspend/resume stack.
v2: Add normal reload cycle after running test (Anshuman)
v3: Rebase
Signed-off-by: Sujaritha Sundaresan <a class="moz-txt-link-rfc2396E" href="mailto:sujaritha.sundaresan@intel.com"><sujaritha.sundaresan@intel.com></a>
Reviewed-by: Anshuman Gupta <a class="moz-txt-link-rfc2396E" href="mailto:anshuman.gupta@intel.com"><anshuman.gupta@intel.com></a>
---
tests/intel/xe_pm.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/tests/intel/xe_pm.c b/tests/intel/xe_pm.c
index 8b115e2f6..03f742265 100644
--- a/tests/intel/xe_pm.c
+++ b/tests/intel/xe_pm.c
@@ -17,6 +17,7 @@
#include "igt.h"
#include "lib/igt_device.h"
+#include "lib/igt_kmod.h"
#include "lib/igt_pm.h"
#include "lib/igt_sysfs.h"
#include "lib/igt_syncobj.h"
@@ -229,6 +230,10 @@ static void close_fw_handle(int sig)
* Description: suspend/autoresume on %arg[1] state and exec after RPM
* Functionality: pm - %arg[1]
*
+ * SUBTEST: %s-without-display
+ * Description: suspend/autoresume on %arg[1] state without display
+ * Functionality: pm - %arg[1]
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">I see you copy-pasted it but imho both Description and
Functionality documentation fields should be static, here and
in other places.
+cc Katarzyna Piecielska <a class="moz-txt-link-rfc2396E" href="mailto:katarzyna.piecielska@intel.com"><katarzyna.piecielska@intel.com></a></pre>
</blockquote>
</blockquote>
</blockquote>
<p>Hi Kamil,</p>
<p>Sorry I didn't get this change. This is inline with the rest of
the file right ?</p>
<blockquote type="cite" cite="mid:20240806125020.jygbaulfsnpv6rqd@kamilkon-DESK.igk.intel.com">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ *
* arg[1]:
*
* @s2idle: s2idle
@@ -681,6 +686,7 @@ igt_main
struct drm_xe_engine_class_instance *hwe;
device_t device;
uint32_t d3cold_allowed;
+ const char *opts;
int sysfs_fd;
const struct s_state {
@@ -757,6 +763,34 @@ igt_main
NO_RPM, 0);
}
+ igt_subtest_f("%s-without-display", s->name) {
+
+ if (!drmModeGetResources(device.fd_xe))
+ return;
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">Why 'return' here?! Imho this should be checked in fixture
or be a skip. Or other way around - what about a headless board
or one without any connected display?
Regards,
Kamil
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
I think this patch idea sort stemmed from the cases where we have a display
connected and
want to make sure that the suspend/resume issues are not being caused by the
display.
But would you suggest expanding the test to have the headless/no display
situations? If so what changes are you suggesting for that ?
Thanks,
Suja
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
I would suggest turn this into a igt_skip_on_f(), not a return.
Regards,
Kamil
</pre>
</blockquote>
<p>Sure I will switch this to <br>
</p>
<p>igt_skip_on(<span style="white-space: pre-wrap">!drmModeGetResources(device.fd_xe))</span></p>
<p><span style="white-space: pre-wrap">
</span></p>
<p><span style="white-space: pre-wrap">Thanks,</span></p>
<p><span style="white-space: pre-wrap">Suja
</span></p>
<blockquote type="cite" cite="mid:20240806125020.jygbaulfsnpv6rqd@kamilkon-DESK.igk.intel.com">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+
+ xe_for_each_engine(device.fd_xe, hwe) {
+
+ igt_debug("Reload w/o display\n");
+
+ igt_kmsg(KMSG_INFO "Unloading Xe\n");
+ igt_assert_eq(igt_xe_driver_unload(), 0);
+
+ igt_kmsg(KMSG_INFO "Re-loading Xe without display\n");
+ igt_assert_eq(igt_xe_driver_load("enable_display=0"), 0);
+
+ test_exec(device, hwe, 1, 2, s->state,
+ NO_RPM, 0);
+
+ igt_debug("Reload as normal\n");
+
+ igt_kmsg(KMSG_INFO "Unloading Xe\n");
+ igt_assert_eq(igt_xe_driver_unload(), 0);
+
+ igt_kmsg(KMSG_INFO "Re-loading Xe\n");
+ igt_assert_eq(igt_xe_driver_load(opts), 0);
+ }
+ }
+
for (const struct vm_op *op = vm_op; op->name; op++) {
igt_subtest_f("%s-vm-bind-%s", s->name, op->name) {
xe_for_each_engine(device.fd_xe, hwe)
--
2.34.1
</pre>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</body>
</html>