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

[Gupta, Anshuman]

> -----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.
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
> >>