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

[Matt Roper <matthew.d.roper@intel.com>]

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@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?

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?


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