Re: [PATCH i-g-t v4 2/2] tests/intel/xe_pm: Add a check for power management capability

["Poosa, Karthik" <karthik.poosa@intel.com>]

On 19-03-2025 13:44, Bernatowicz, Marcin wrote:
>
>
> On 3/19/2025 9:05 AM, Poosa, Karthik wrote:
>>
>> On 18-03-2025 20:31, Jakub Kolakowski wrote:
>>> Add a check for power management capability of device tested in tests
>>> related to D3 states. Currently if test is started on configuration
>>> that does not support the PM capability it doesn't skip, instead
>>> depending on test it may fail, abort or timeout.
>>> With this change test will skip with a clear message why it did.
>>>
>>> Cc: Adam Miszczak<adam.miszczak@linux.intel.com>
>>> Cc: Lukasz Laguna<lukasz.laguna@intel.com>
>>> Cc: Marcin Bernatowicz<marcin.bernatowicz@linux.intel.com>
>>> Signed-off-by: Jakub Kolakowski<jakub1.kolakowski@intel.com>
>>> ---
>>>   tests/intel/xe_pm.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/tests/intel/xe_pm.c b/tests/intel/xe_pm.c
>>> index c2026474d..70f0613fb 100644
>>> --- a/tests/intel/xe_pm.c
>>> +++ b/tests/intel/xe_pm.c
>>> @@ -146,6 +146,9 @@ static void vram_d3cold_threshold_restore(int sig)
>>>   static bool setup_d3(device_t device, enum igt_acpi_d_state state)
>>>   {
>>> +    igt_require_f(igt_has_pci_pm_capability(device.pci_xe),
>>> +              "PCI power management capability not found\n");
>>
>> Hi Jakub,
>>
>> As per PCIe Base spec, all PCI Express Functions are required to 
>> support PCI Power Management Capability.
>>
>> Only legacy PCI devices wouldn't have it.
>>
>> Can you share details of failures without this capability.
>>
>> All Xe supported GPUs are PCIe complaint, this check may not be 
>> needed at all.
>
> This happens for runs with Virtual Function (VF) passed to Virtual 
> Machine (VM).
>
> "9.6 SR-IOV Power Management
> This section defines the PCI Express SR-IOV power management 
> capabilities and protocols.
> The Power Management Capability is required for PFs as described in 
> Chapter 5 .
> For VFs, the Power Management Capability is optional."
>
> -- 
> marcin
Okay, LGTM then.
Reviewed-by: Karthik Poosa <karthik.poosa@intel.com>
>>
>>> +
>>>       dpms_on_off(device, DRM_MODE_DPMS_OFF);
>>>       /*
>