<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="EN-US" link="blue" vlink="purple" style="word-wrap:break-word"> <div class="WordSection1"> <o:p> </o:p> <o:p> </o:p> <div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt"> <div> <div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"> From: Sundaresan, Sujaritha <sujaritha.sundaresan@intel.com> Sent: Tuesday, August 27, 2024 1:42 PM To: Sousa, Gustavo <gustavo.sousa@intel.com>; De Marchi, Lucas <lucas.demarchi@intel.com> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>; igt-dev@lists.freedesktop.org; Gupta, Anshuman <anshuman.gupta@intel.com>; Piecielska, Katarzyna <katarzyna.piecielska@intel.com> Subject: Re: [PATCH i-g-t, v4] tests/intel/xe_pm: Add tests for suspend without display<o:p></o:p> </div> </div> <o:p> </o:p> <o:p> </o:p> <div> On 8/26/2024 8:06 PM, Gustavo Sousa wrote:<o:p></o:p> </div> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> <pre>Quoting Lucas De Marchi (2024-08-22 18:01:19-03:00)<o:p></o:p></pre> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> <pre>On Thu, Aug 22, 2024 at 02:45:15PM GMT, Gustavo Sousa wrote:<o:p></o:p></pre> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> <pre>Quoting Kamil Konieczny (2024-08-22 13:52:43-03:00)<o:p></o:p></pre> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> <pre>Hi Sujaritha,<o:p></o:p></pre> <pre>On 2024-08-22 at 14:50:58 +0530, Sujaritha Sundaresan wrote:<o:p></o:p></pre> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> <pre>Add tests to validate basic execution suspend/resume cycle<o:p></o:p></pre> <pre>without display module to rule out display related issues<o:p></o:p></pre> <pre>from the suspend/resume stack.<o:p></o:p></pre> <pre><o:p> </o:p></pre> <pre>v2: Add normal reload cycle after running test (Anshuman)<o:p></o:p></pre> <pre><o:p> </o:p></pre> <pre>v3: Rebase<o:p></o:p></pre> <pre><o:p> </o:p></pre> <pre>v4: Change commit name/header (Kamil)<o:p></o:p></pre> <pre><o:p> </o:p></pre> <pre>Signed-off-by: Sujaritha Sundaresan <a href="mailto:sujaritha.sundaresan@intel.com"><sujaritha.sundaresan@intel.com></a><o:p></o:p></pre> <pre>Acked-by: Katarzyna Piecielska <a href="mailto:Katarzyna.piecielska@intel.com">Katarzyna.piecielska@intel.com</a><a href="mailto:Katarzyna.piecielska@intel.com"><Katarzyna.piecielska@intel.com></a><o:p></o:p></pre> <pre>Reviewed-by: Anshuman Gupta <a href="mailto:anshuman.gupta@intel.com"><anshuman.gupta@intel.com></a><o:p></o:p></pre> <pre>---<o:p></o:p></pre> <pre> tests/intel/xe_pm.c | 34 ++++++++++++++++++++++++++++++++++<o:p></o:p></pre> <pre> 1 file changed, 34 insertions(+)<o:p></o:p></pre> <pre><o:p> </o:p></pre> <pre>diff --git a/tests/intel/xe_pm.c b/tests/intel/xe_pm.c<o:p></o:p></pre> <pre>index eee89428c..1623a56c7 100644<o:p></o:p></pre> <pre>--- a/tests/intel/xe_pm.c<o:p></o:p></pre> <pre>+++ b/tests/intel/xe_pm.c<o:p></o:p></pre> <pre>@@ -17,6 +17,7 @@<o:p></o:p></pre> <pre><o:p> </o:p></pre> <pre> #include "igt.h"<o:p></o:p></pre> <pre> #include "lib/igt_device.h"<o:p></o:p></pre> <pre>+#include "lib/igt_kmod.h"<o:p></o:p></pre> <pre> #include "lib/igt_pm.h"<o:p></o:p></pre> <pre> #include "lib/igt_sysfs.h"<o:p></o:p></pre> <pre> #include "lib/igt_syncobj.h"<o:p></o:p></pre> <pre>@@ -229,6 +230,10 @@ static void close_fw_handle(int sig)<o:p></o:p></pre> <pre> * Description: suspend/autoresume on %arg[1] state and exec after RPM<o:p></o:p></pre> <pre> * Functionality: pm - %arg[1]<o:p></o:p></pre> <pre> *<o:p></o:p></pre> <pre>+ * SUBTEST: %s-without-display<o:p></o:p></pre> <pre>+ * Description: suspend/autoresume on %arg[1] state without display<o:p></o:p></pre> <pre>+ * Functionality: pm - %arg[1]<o:p></o:p></pre> <pre>+ *<o:p></o:p></pre> <pre> * arg[1]:<o:p></o:p></pre> <pre> *<o:p></o:p></pre> <pre> * @s2idle: s2idle<o:p></o:p></pre> <pre>@@ -681,6 +686,7 @@ igt_main<o:p></o:p></pre> <pre> struct drm_xe_engine_class_instance *hwe;<o:p></o:p></pre> <pre> device_t device;<o:p></o:p></pre> <pre> uint32_t d3cold_allowed;<o:p></o:p></pre> <pre>+ const char *opts;<o:p></o:p></pre> <pre> int sysfs_fd;<o:p></o:p></pre> <pre><o:p> </o:p></pre> <pre> const struct s_state {<o:p></o:p></pre> <pre>@@ -757,6 +763,34 @@ igt_main<o:p></o:p></pre> <pre> NO_RPM, 0);<o:p></o:p></pre> <pre> }<o:p></o:p></pre> <pre><o:p> </o:p></pre> <pre>+ igt_subtest_f("%s-without-display", s->name) {<o:p></o:p></pre> <pre>+<o:p></o:p></pre> <pre>+ if (!drmModeGetResources(device.fd_xe))<o:p></o:p></pre> <pre>+ return;<o:p></o:p></pre> </blockquote> <pre><o:p> </o:p></pre> <pre>This should be skip or save it (as global?) and run test on headless.<o:p></o:p></pre> <pre><o:p> </o:p></pre> <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt"> <pre>+<o:p></o:p></pre> <pre>+ xe_for_each_engine(device.fd_xe, hwe) {<o:p></o:p></pre> <pre>+<o:p></o:p></pre> <pre>+ igt_debug("Reload w/o display\n");<o:p></o:p></pre> <pre>+<o:p></o:p></pre> <pre>+ igt_kmsg(KMSG_INFO "Unloading Xe\n");<o:p></o:p></pre> <pre>+ igt_assert_eq(igt_xe_driver_unload(), 0);<o:p></o:p></pre> <pre>+<o:p></o:p></pre> <pre>+ igt_kmsg(KMSG_INFO "Re-loading Xe without display\n");<o:p></o:p></pre> <pre>+ igt_assert_eq(igt_xe_driver_load("enable_display=0"), 0);<o:p></o:p></pre> </blockquote> <pre><o:p> </o:p></pre> <pre>This param was renamed to probe_display, see<o:p></o:p></pre> <pre><a href="https://patchwork.freedesktop.org/series/137096/">https://patchwork.freedesktop.org/series/137096/</a><o:p></o:p></pre> <pre>tests/intel/xe_module_load: Rename enable_display module param<o:p></o:p></pre> <pre>Submitted by Lucas De Marchi on Aug. 9, 2024, 5:30 p.m.<o:p></o:p></pre> <pre><o:p> </o:p></pre> <pre>+Cc Lucas<o:p></o:p></pre> </blockquote> <pre><o:p> </o:p></pre> <pre>And disable_display=1 might be more appropriate, because probe_display<o:p></o:p></pre> <pre>would simply skip the probe for display?<o:p></o:p></pre> </blockquote> <pre><o:p> </o:p></pre> <pre>Good point. For platforms with display, is it expected that suspend works<o:p></o:p></pre> <pre>if DMC is not loaded? Otherwise we will actually have to touch display<o:p></o:p></pre> <pre>to disable it to be allowed to suspend.<o:p></o:p></pre> </blockquote> <pre><o:p> </o:p></pre> <pre>Well, for s2idle (s0ix for intel), DC6 is required, which in turn<o:p></o:p></pre> <pre>requires the DMC.<o:p></o:p></pre> <pre>For s0ix DC9 is the pre-requisite rather then DC6, DC9 does not have any dependency on DMC.<o:p></o:p></pre> <pre>Our test is not intended to validate s0ix , neither our CI DUTS are equipped with recipe of s0ix.<o:p></o:p></pre> <pre>We need to validate xe driver functionality with suspend/resume. <o:p></o:p></pre> <pre><o:p> </o:p></pre> <pre>For s3, I'm not really sure what DC state would be required, but I would<o:p></o:p></pre> <pre>assume that we would need the driver to put the display in a low-power<o:p></o:p></pre> <pre>state, so disable_display=1 seems to be needed here. Someone with more<o:p></o:p></pre> <pre>experience could chime in to confirm this.<o:p></o:p></pre> <pre>Agree we would need to keep the display in to low power state, with that I agree that disable_display=1 suits here.<o:p></o:p></pre> <pre><o:p> </o:p></pre> <pre>Thanks,<o:p></o:p></pre> <pre>Anshuman<o:p></o:p></pre> <pre><o:p> </o:p></pre> <pre>--<o:p></o:p></pre> <pre>Gustavo Sousa<o:p></o:p></pre> </blockquote> Is there someone else specific that can be brought into this conversation to help answer these questions ?<o:p></o:p> Thanks,<o:p></o:p> Suja <o:p></o:p> </div> </div> </body> </html>