[PATCH i-g-t v1] Add support for runtime pm for xe driver

[Lucas De Marchi]

On Thu, Mar 06, 2025 at 10:06:01AM -0600, Gupta, Anshuman wrote:
>
>
>> -----Original Message-----
>> From: De Marchi, Lucas <lucas.demarchi at intel.com>
>> Sent: Thursday, March 6, 2025 9:20 PM
>> To: Kamil Konieczny <kamil.konieczny at linux.intel.com>; Purkait, Soham
>> <soham.purkait at intel.com>; igt-dev at lists.freedesktop.org; Tauro, Riana
>> <riana.tauro at intel.com>; Belgaumkar, Vinay <vinay.belgaumkar at intel.com>;
>> Gupta, Anshuman <anshuman.gupta at intel.com>; Vivi, Rodrigo
>> <rodrigo.vivi at intel.com>
>> Subject: Re: [PATCH i-g-t v1] Add support for runtime pm for xe driver
>>
>> On Thu, Mar 06, 2025 at 02:51:00PM +0100, Kamil Konieczny wrote:
>> >Hi Soham,
>> >On 2025-03-06 at 16:57:54 +0530, Soham Purkait wrote:
>> >
>> >add prefix 'tools/intel_pm_rpm:' in subject, so it will be:
>> >
>> >[PATCH i-g-t v1] tools/intel_pm_rpm: Add support for Xe driver
>> >
>> >> Add support for runtime power management for Xe driver.
>> >>
>> >
>> >Add here your s-o-b:
>> >
>> >Soham Purkait <soham.purkait at intel.com>
>> >
>> >Please use checkpatch.pl for some obvious corrections like this, you
>> >can find this script in Linux kernel sources, also read CONTRIBUTING.md
>> >in i-g-t sources for some options for checkpatch.
>> >
>> >> ---
>> >>  tools/intel_pm_rpm.c | 2 +-
>> >>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/tools/intel_pm_rpm.c b/tools/intel_pm_rpm.c index
>> >> 08c25ca8a..945247d2e 100644
>> >> --- a/tools/intel_pm_rpm.c
>> >> +++ b/tools/intel_pm_rpm.c
>> >> @@ -151,7 +151,7 @@ int main(int argc, char *argv[])
>> >>  			goto exit;
>> >>  		}
>> >>  	} else {
>> >> -		if (!igt_device_find_first_i915_discrete_card(&card)) {
>> >> +		if (!igt_device_find_first_i915_discrete_card(&card) &&
>> >> +!igt_device_find_first_xe_discrete_card(&card)) {
>> >
>> >Split this line:
>> >		if (!igt_device_find_first_i915_discrete_card(&card) &&
>> >		    !igt_device_find_first_xe_discrete_card(&card)) {
>>
>> I'd rather stop spreading this. Particularly in **tools**.
>> If env_device is NULL, why don 't we rather build a match string with the thing
>> we want aka
>>
>> 	"pci:vendor=8086,device=discrete"
>>
>> I'd replace in tools the current uses of
>> igt_device_find_first_i915_discrete_card() with:
>>
>> 	"pci:vendor=8086,device=discrete,driver=i915"
>>
>> ... and implement the driver= match in igt_device_scan if it's not there. Then if
>> it works with either i915 or xe, we remove it.
>>
>> I also think the tool is being less than helpful by trying to work with discrete-
>> only device.
>This tool implemented with intention put the discrete cards in runtime d3cold as it requires to tune each pcie device in the topology.
>As d3cold only supported on  dgpu, we don't need this tool for igpu.

ok, but should this be a check by discrete for each driver or checking
power state and d3cold_allowed in the pci layer?

What do you do if the user passed 00:02.0 as device?

Lucas De Marchi

>Thanks,
>Anshuman.
>>
>> It looks like SET_I915_AUTOSUSPEND_DELAY also needs to be abstracted?
>>
>>
>> Lucas De Marchi
>>
>>
>> >
>> >Btw should we also add support for other cards in multi-GPU case? If
>> >yes this is some more work for another patch.
>> >
>> >Regards,
>> >Kamil
>> >
>> >>  			igt_warn("No discrete gpu found\n");
>> >>  			ret = EXIT_FAILURE;
>> >>  			goto exit;
>> >> --
>> >> 2.34.1
>> >>