[PATCH i-g-t 2/6] lib/igt_gt: Document fd argument in igt_open_forcewake_handle()

[Lucas De Marchi]

On Fri, Sep 20, 2024 at 10:15:14AM GMT, Matt Roper wrote:
>On Wed, Sep 18, 2024 at 09:36:25AM -0700, Lucas De Marchi wrote:
>> Document the fd argument to make it clear what -1 means, which is
>> usually not the desired behavior.
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> ---
>>  lib/igt_gt.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/lib/igt_gt.c b/lib/igt_gt.c
>> index b043e2d41..d831d2448 100644
>> --- a/lib/igt_gt.c
>> +++ b/lib/igt_gt.c
>> @@ -492,6 +492,7 @@ void igt_stop_hang_helper(void)
>>
>>  /**
>>   * igt_open_forcewake_handle:
>> + * @fd: open i915 or xe drm file descriptor or -1 to use the first device found
>
>Is it actually "first found" or is it just literally device 0 (which may
>not even exist anymore)?  It looks like if -1 gets passed down to
>igt_debugfs_path as the 'device' parameter, we just do a memset of the
>stat structure to zero it, wind up setting idx = minor(0), and then use
>the resulting idx (which should be 0) in the debugfs pathname.  Am I
>missing something?

no, that is correct. If -1 is passed, it goes to <debugfs>/dri/0

>
>It looks like there are a few places in IGT passing -1 to debugfs
>functions like this.  Maybe we should add a #define for it with a more
>descriptive name to igt_debugfs.h?

I was so annoyed by it and my thought was "this needs to eventually be
dropped". We may have been silently changing the setting in one device
while working with another one.

Now I think forcewake for i915 has always been broken for DG2. And
sometimes it may have worked if it happens to be dri/0 :-/

So I'm thinking.... rather than this patch, maybe just fix each place
passing -1 and rework it? Changing the fallback to at least respect 
igt_device_scan.c wouldn't be a terrible idea.

Lucas De Marchi

>
>
>Matt
>
>>   *
>>   * This functions opens the debugfs forcewake file and so prevents the GT from
>>   * suspending. The reference is automatically dropped when the is closed.
>> --
>> 2.46.1
>>
>
>-- 
>Matt Roper
>Graphics Software Engineer
>Linux GPU Platform Enablement
>Intel Corporation