From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Jani Nikula <jani.nikula@intel.com>
Cc: igt-dev@lists.freedesktop.org
Subject: Re: [PATCH i-g-t] lib/igt_debugfs: remove mode parameter from igt_debugfs_crtc_dir()
Date: Mon, 16 Mar 2026 16:49:04 +0200 [thread overview]
Message-ID: <abgYYNS15fQimwNe@intel.com> (raw)
In-Reply-To: <20260316112919.1162830-1-jani.nikula@intel.com>
On Mon, Mar 16, 2026 at 01:29:19PM +0200, Jani Nikula wrote:
> The mode for opening the CRTC debugfs directory should always be
> O_DIRECTORY | O_RDONLY. There's no point in letting the callers pass in
> the mode. Indeed, most of them omit O_RDONLY, and some omit O_DIRECTORY.
>
> Remove the mode from igt_debugfs_crtc_dir() and subsequently
> igt_crtc_debugfs_dir(), always using O_DIRECTORY | O_RDONLY.
>
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> lib/i915/intel_drrs.c | 6 +++---
> lib/i915/intel_fbc.c | 4 ++--
> lib/igt_debugfs.c | 5 ++---
> lib/igt_debugfs.h | 2 +-
> lib/igt_kms.c | 8 ++++----
> lib/igt_kms.h | 2 +-
> tests/intel/kms_frontbuffer_tracking.c | 2 +-
> tests/nouveau_crc.c | 2 +-
> 8 files changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/lib/i915/intel_drrs.c b/lib/i915/intel_drrs.c
> index 1ad993c1e90b..bfe58d7a0694 100644
> --- a/lib/i915/intel_drrs.c
> +++ b/lib/i915/intel_drrs.c
> @@ -24,7 +24,7 @@ bool intel_is_drrs_supported(igt_crtc_t *crtc)
> char buf[256];
> int dir;
>
> - dir = igt_crtc_debugfs_dir(crtc, O_DIRECTORY);
> + dir = igt_crtc_debugfs_dir(crtc);
> igt_require_fd(dir);
> igt_debugfs_simple_read(dir, "i915_drrs_status", buf, sizeof(buf));
> close(dir);
> @@ -63,7 +63,7 @@ static void drrs_set(int device, int crtc_index, unsigned int val)
> igt_debug("Manually %sabling DRRS. %u\n", val ? "en" : "dis", val);
> snprintf(buf, sizeof(buf), "%d", val);
>
> - dir = igt_debugfs_crtc_dir(device, crtc_index, O_DIRECTORY);
> + dir = igt_debugfs_crtc_dir(device, crtc_index);
> igt_require_fd(dir);
> ret = igt_sysfs_write(dir, "i915_drrs_ctl", buf, sizeof(buf) - 1);
> close(dir);
> @@ -125,7 +125,7 @@ bool intel_is_drrs_inactive(igt_crtc_t *crtc)
> char buf[256];
> int dir;
>
> - dir = igt_crtc_debugfs_dir(crtc, O_DIRECTORY);
> + dir = igt_crtc_debugfs_dir(crtc);
> igt_require_fd(dir);
> igt_debugfs_simple_read(dir, "i915_drrs_status", buf, sizeof(buf));
> close(dir);
> diff --git a/lib/i915/intel_fbc.c b/lib/i915/intel_fbc.c
> index 9c1767846c25..b8c714d430e9 100644
> --- a/lib/i915/intel_fbc.c
> +++ b/lib/i915/intel_fbc.c
> @@ -36,7 +36,7 @@ bool intel_fbc_supported(igt_crtc_t *crtc)
> char buf[FBC_STATUS_BUF_LEN];
> int dir;
>
> - dir = igt_crtc_debugfs_dir(crtc, O_DIRECTORY);
> + dir = igt_crtc_debugfs_dir(crtc);
> igt_require_fd(dir);
> igt_debugfs_simple_read(dir, "i915_fbc_status", buf, sizeof(buf));
> close(dir);
> @@ -53,7 +53,7 @@ static bool _intel_fbc_is_enabled(igt_crtc_t *crtc, int log_level, char *last_fb
> bool print = true;
> int dir;
>
> - dir = igt_crtc_debugfs_dir(crtc, O_DIRECTORY);
> + dir = igt_crtc_debugfs_dir(crtc);
> igt_require_fd(dir);
> igt_debugfs_simple_read(dir, "i915_fbc_status", buf, sizeof(buf));
> close(dir);
> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> index 11f5ee9984ee..a184843c9acc 100644
> --- a/lib/igt_debugfs.c
> +++ b/lib/igt_debugfs.c
> @@ -288,7 +288,6 @@ int igt_debugfs_connector_dir(int device, char *conn_name, int mode)
> * igt_debugfs_crtc_dir:
> * @device: fd of the device
> * @crtc_index: CRTC index
> - * @mode: mode bits as used by open()
> *
> * This opens the debugfs directory corresponding to the CRTC index on the
> * device for use with igt_sysfs_get() and related functions. This is just
> @@ -297,12 +296,12 @@ int igt_debugfs_connector_dir(int device, char *conn_name, int mode)
> * Returns:
> * The directory fd, or -1 on failure.
> */
> -int igt_debugfs_crtc_dir(int device, int crtc_index, int mode)
> +int igt_debugfs_crtc_dir(int device, int crtc_index)
> {
> char buf[128];
>
> snprintf(buf, sizeof(buf), "crtc-%d", crtc_index);
> - return igt_debugfs_open(device, buf, mode);
> + return igt_debugfs_open(device, buf, O_DIRECTORY | O_RDONLY);
> }
>
> /**
> diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
> index 2c546f299a43..30d4b0ff92ba 100644
> --- a/lib/igt_debugfs.h
> +++ b/lib/igt_debugfs.h
> @@ -34,7 +34,7 @@ char *igt_debugfs_path(int device, char *path, int pathlen);
>
> int igt_debugfs_dir(int device);
> int igt_debugfs_connector_dir(int device, char *conn_name, int mode);
> -int igt_debugfs_crtc_dir(int device, int crtc_index, int mode);
> +int igt_debugfs_crtc_dir(int device, int crtc_index);
>
> int igt_debugfs_open(int fd, const char *filename, int mode);
> bool igt_debugfs_is_dir(int drm_fd, const char *name, int gt_id);
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 57f5e099a313..9068e3d7bedd 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1309,7 +1309,7 @@ int __intel_get_pipe_from_crtc_index(int fd, int crtc_index)
> int debugfs_fd, pipe, res = 0;
> char pipe_char;
>
> - debugfs_fd = igt_debugfs_crtc_dir(fd, crtc_index, O_RDONLY);
> + debugfs_fd = igt_debugfs_crtc_dir(fd, crtc_index);
>
> if (debugfs_fd >= 0) {
> res = igt_debugfs_simple_read(debugfs_fd, "i915_pipe", buf, sizeof(buf));
> @@ -5254,9 +5254,9 @@ igt_pipe_crc_t *igt_crtc_crc_new_nonblock(igt_crtc_t *crtc, const char *source)
> return igt_pipe_crc_new_nonblock(crtc->display->drm_fd, crtc->crtc_index, source);
> }
>
> -int igt_crtc_debugfs_dir(igt_crtc_t *crtc, int mode)
> +int igt_crtc_debugfs_dir(igt_crtc_t *crtc)
> {
> - return igt_debugfs_crtc_dir(crtc->display->drm_fd, crtc->crtc_index, mode);
> + return igt_debugfs_crtc_dir(crtc->display->drm_fd, crtc->crtc_index);
> }
>
> const char *igt_crtc_name(igt_crtc_t *crtc)
> @@ -6643,7 +6643,7 @@ unsigned int igt_get_crtc_current_bpc(igt_crtc_t *crtc)
> int fd, res;
> unsigned int current;
>
> - fd = igt_crtc_debugfs_dir(crtc, O_RDONLY);
> + fd = igt_crtc_debugfs_dir(crtc);
> igt_assert(fd >= 0);
>
> if (is_intel_device(drmfd))
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 0bb2734ea3d7..83d3d5159a9c 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -596,7 +596,7 @@ typedef struct _igt_pipe_crc igt_pipe_crc_t;
> igt_pipe_crc_t *igt_crtc_crc_new(igt_crtc_t *crtc, const char *source);
> igt_pipe_crc_t *igt_crtc_crc_new_nonblock(igt_crtc_t *crtc, const char *source);
>
> -int igt_crtc_debugfs_dir(igt_crtc_t *crtc, int mode);
> +int igt_crtc_debugfs_dir(igt_crtc_t *crtc);
>
> igt_crtc_t *igt_output_get_driving_crtc(igt_output_t *output);
> const char *igt_output_name(igt_output_t *output);
> diff --git a/tests/intel/kms_frontbuffer_tracking.c b/tests/intel/kms_frontbuffer_tracking.c
> index 5471b8c51c99..c16f63199bdd 100644
> --- a/tests/intel/kms_frontbuffer_tracking.c
> +++ b/tests/intel/kms_frontbuffer_tracking.c
> @@ -1535,7 +1535,7 @@ static void __debugfs_read_crtc(const char *param, char *buf, int len)
> {
> int dir;
>
> - dir = igt_crtc_debugfs_dir(prim_mode_params.crtc, O_DIRECTORY);
> + dir = igt_crtc_debugfs_dir(prim_mode_params.crtc);
> igt_require_fd(dir);
> igt_debugfs_simple_read(dir, param, buf, len);
> close(dir);
> diff --git a/tests/nouveau_crc.c b/tests/nouveau_crc.c
> index 29e2d84e76ac..ee63e931616c 100644
> --- a/tests/nouveau_crc.c
> +++ b/tests/nouveau_crc.c
> @@ -370,7 +370,7 @@ int igt_main()
> igt_plane_set_fb(data.primary, &data.default_fb);
> igt_display_commit(&data.display);
>
> - dir = igt_crtc_debugfs_dir(data.crtc, O_DIRECTORY);
> + dir = igt_crtc_debugfs_dir(data.crtc);
> igt_require_fd(dir);
> data.nv_crc_dir = openat(dir, "nv_crc", O_DIRECTORY);
> close(dir);
> --
> 2.47.3
--
Ville Syrjälä
Intel
next prev parent reply other threads:[~2026-03-16 14:49 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-16 11:29 [PATCH i-g-t] lib/igt_debugfs: remove mode parameter from igt_debugfs_crtc_dir() Jani Nikula
2026-03-16 14:49 ` Ville Syrjälä [this message]
2026-03-17 9:02 ` ✗ Xe.CI.BAT: failure for " Patchwork
2026-03-17 12:19 ` ✓ i915.CI.BAT: success " Patchwork
2026-03-18 14:12 ` ✗ Xe.CI.FULL: failure " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=abgYYNS15fQimwNe@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=igt-dev@lists.freedesktop.org \
--cc=jani.nikula@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.