Re: [igt-dev] [i-g-t, v3, 3/5] lib/igt_debugfs: Add igt_debugfs_pipe_dir()

Jeremy Cline
2020-08-18T21:01:23Z

Hi,

On Fri, Apr 17, 2020 at 05:10:23PM -0400, Lyude wrote:
> From: Lyude Paul <lyude@redhat.com>
>
> Like igt_debugfs_connector_dir(), but for pipes instead.
>
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> ---
> lib/igt_debugfs.c | 29 +++++++++++++++++++++++++++++
> lib/igt_debugfs.h | 1 +
> 2 files changed, 30 insertions(+)
>
> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> index bf6be552..3c3b11e1 100644
> --- a/lib/igt_debugfs.c
> +++ b/lib/igt_debugfs.c
> @@ -260,6 +260,35 @@ int igt_debugfs_connector_dir(int device, char *conn_name, int mode)
> return ret;
> }
>
> +/**
> + * igt_debugfs_pipe_dir:
> + * @device: fd of the device
> + * @pipe: index of pipe
> + * @mode: mode bits as used by open()
> + *
> + * This opens the debugfs directory corresponding to the pipe index on the
> + * device for use with igt_sysfs_get() and related functions.
> + *
> + * Returns:
> + * The directory fd, or -1 on failure.
> + */
> +int igt_debugfs_pipe_dir(int device, int pipe, int mode)
> +{
> + char buf[128];
> + int dir, ret;
> +
> + dir = igt_debugfs_dir(device);
> + if (dir < 0)
> + return dir;
> +
> + snprintf(buf, sizeof(buf), "crtc-%d", pipe);
> + ret = openat(dir, buf, mode);
> +
> + close(dir);
> +
> + return ret;
> +}
> +

There seems to be a lot of overlap between this,
igt_debugfs_connector_dir(), and igt_debugfs_open(). As far as I can
tell, the latter two functions are completely identical so it's unclear
to me what the advantage of using one over the other is.

If I'm not mistaken igt_debugfs_pipe_dir() could be reduced to:

{
char buf[128];
snprintf(buf, sizeof(buf), "crtc-%d", pipe);
return igt_debugfs_open(device, buf, mode);
}

and the docblock could just note it's sugar for igt_debugfs_open().

- Jeremy