Re: [PATCH i-g-t, v4] tests/intel/xe_pm: Add tests for suspend without display

["Sundaresan, Sujaritha" <sujaritha.sundaresan@intel.com>]

On 8/26/2024 7:27 PM, Lucas De Marchi wrote:
> On Mon, Aug 26, 2024 at 04:42:06PM GMT, Sujaritha Sundaresan wrote:
>>
>> On 8/23/2024 2:31 AM, Lucas De Marchi wrote:
>>> On Thu, Aug 22, 2024 at 02:45:15PM GMT, Gustavo Sousa wrote:
>>>> Quoting Kamil Konieczny (2024-08-22 13:52:43-03:00)
>>>>> Hi Sujaritha,
>>>>> On 2024-08-22 at 14:50:58 +0530, Sujaritha Sundaresan wrote:
>>>>>> 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
>>>>>>
>>>>>> v4: Change commit name/header (Kamil)
>>>>>>
>>>>>> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
>>>>>> Acked-by: Katarzyna Piecielska 
>>>>>> Katarzyna.piecielska@intel.com<Katarzyna.piecielska@intel.com>
>>>>>> Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com>
>>>>>> ---
>>>>>>  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 eee89428c..1623a56c7 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]
>>>>>> + *
>>>>>>   * 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;
>>>>>
>>>>> This should be skip or save it (as global?) and run test on headless.
>>>>>
>>>>>> +
>>>>>> + 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);
>>>>>
>>>>> This param was renamed to probe_display, see
>>>>> https://patchwork.freedesktop.org/series/137096/
>>>>> tests/intel/xe_module_load: Rename enable_display module param
>>>>> Submitted by Lucas De Marchi on Aug. 9, 2024, 5:30 p.m.
>>>>>
>>>>> +Cc Lucas
>>>>
>>>> And disable_display=1 might be more appropriate, because probe_display
>>>> would simply skip the probe for display?
>>>
>>> Good point. For platforms with display, is it expected that suspend 
>>> works
>>> if DMC is not loaded? Otherwise we will actually have to touch display
>>> to disable it to be allowed to suspend.
>>>
>>> Lucas De Marchi
>>
>> Hi all,
>>
>> Thanks for all of the comments. The need for this test mostly stemmed 
>> from
>>
>> ruling out display as being the reason for a lot of the random 
>> suspend/resume bugs we see.
>>
>> So what do you think should be the approach for this?
>
> what are the suspend/resume bugs we are talking about? Without analyzing
> them, it's hard to answer if they are caused by display or if
> probe_display/disable_display will help at all.
>
> Lucas De Marchi

I will try to find a specific example but it's most the suspend/resume 
incomplete

bugs we see regularly across most platforms. The aim with this was just 
for the debugging process

for those to instantly rule out display as a reason early on.

Thanks,

Suja