[PATCH i-g-t 0/3] lib/igt_kms: pipe vs CRTC index cleanups

[PATCH i-g-t 0/3] lib/igt_kms: pipe vs CRTC index cleanups

[Jani Nikula]

Some minor cleanups for pipe vs. CRTC index conversions.

Jani Nikula (3):
  lib/igt_kms: Always return CRTC index from
    kmstest_get_pipe_from_crtc_id()
  lib/igt_kms: Drop the ioctl fallback for CRTC id to pipe mapping
  lib/igt_kms: Remove kmstest_pipe_to_index()

 lib/igt_kms.c               | 75 +++++++++----------------------------
 lib/igt_kms.h               |  3 +-
 tests/intel/kms_fbcon_fbt.c |  2 +-
 tests/kms_async_flips.c     |  6 +--
 tests/kms_setmode.c         |  4 +-
 5 files changed, 24 insertions(+), 66 deletions(-)

-- 
2.47.3

[PATCH i-g-t 1/3] lib/igt_kms: Always return CRTC index from kmstest_get_pipe_from_crtc_id()

[Jani Nikula]

All users of kmstest_get_pipe_from_crtc_id() actually need the CRTC
index, not the hardware pipe. Stop doing the CRTC index to hardware pipe
special case mapping on Intel devices. Rename the function
kmstest_get_crtc_index_from_id().

kms_setmode.c does use the result also for logging, which would now be
incorrect if pipe != CRTC index. But even there, it functionally
requires the CRTC index. The test needs other cleanups.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 lib/igt_kms.c               | 13 +++++--------
 lib/igt_kms.h               |  2 +-
 tests/intel/kms_fbcon_fbt.c |  2 +-
 tests/kms_async_flips.c     |  6 +++---
 tests/kms_setmode.c         |  3 +--
 5 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 77b3ae2afa01..57e6474f2c2d 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1360,16 +1360,14 @@ static int __intel_get_pipe_from_crtc_id(int fd, int crtc_id, int crtc_idx)
 }
 
 /**
- * kmstest_get_pipe_from_crtc_id:
+ * kmstest_get_crtc_index_from_id:
  * @fd: DRM fd
  * @crtc_id: DRM CRTC id
  *
- * Returns: The crtc index for the given DRM CRTC ID @crtc_id. The crtc index
- * is the equivalent of the pipe id.  This value maps directly to an enum pipe
- * value used in other helper functions.  Returns 0 if the index could not be
- * determined.
+ * Returns: The crtc index for the given DRM CRTC ID @crtc_id. Returns 0 if the
+ * index could not be determined.
  */
-int kmstest_get_pipe_from_crtc_id(int fd, int crtc_id)
+int kmstest_get_crtc_index_from_id(int fd, int crtc_id)
 {
 	drmModeRes *res;
 	drmModeCrtc *drm_crtc;
@@ -1391,8 +1389,7 @@ int kmstest_get_pipe_from_crtc_id(int fd, int crtc_id)
 
 	drmModeFreeResources(res);
 
-	return is_intel_device(fd) ?
-		__intel_get_pipe_from_crtc_id(fd, crtc_id, i) : i;
+	return i;
 }
 
 /**
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index e1044101707c..d25c166eba4a 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -148,7 +148,7 @@ void kmstest_dump_mode(drmModeModeInfo *mode);
 #define HDISPLAY_6K_PER_PIPE 6144
 #define HDISPLAY_5K_PER_PIPE 5120
 
-int kmstest_get_pipe_from_crtc_id(int fd, int crtc_id);
+int kmstest_get_crtc_index_from_id(int fd, int crtc_id);
 void kmstest_set_vt_graphics_mode(void);
 void kmstest_restore_vt_mode(void);
 void kmstest_set_vt_text_mode(void);
diff --git a/tests/intel/kms_fbcon_fbt.c b/tests/intel/kms_fbcon_fbt.c
index 1b9e535ebffc..f6de4b0afef6 100644
--- a/tests/intel/kms_fbcon_fbt.c
+++ b/tests/intel/kms_fbcon_fbt.c
@@ -148,7 +148,7 @@ static bool fbc_check_cursor_blinking(struct drm_info *drm)
 	bool ret;
 	int i, pipe;
 
-	pipe = kmstest_get_pipe_from_crtc_id(drm->fd, drm->crtc_id);
+	pipe = kmstest_get_crtc_index_from_id(drm->fd, drm->crtc_id);
 	pipe_crc = igt_pipe_crc_new(drm->fd, pipe, IGT_PIPE_CRC_SOURCE_AUTO);
 
 	igt_pipe_crc_start(pipe_crc);
diff --git a/tests/kms_async_flips.c b/tests/kms_async_flips.c
index 71570385c32e..0ee0cdc47a96 100644
--- a/tests/kms_async_flips.c
+++ b/tests/kms_async_flips.c
@@ -560,7 +560,7 @@ static void wait_for_vblank(data_t *data, unsigned long *vbl_time, unsigned int
 	int pipe;
 
 	memset(&wait_vbl, 0, sizeof(wait_vbl));
-	pipe = kmstest_get_pipe_from_crtc_id(data->drm_fd, data->crtc_id);
+	pipe = kmstest_get_crtc_index_from_id(data->drm_fd, data->crtc_id);
 	pipe_id_flag = kmstest_get_vbl_flag(pipe);
 
 	wait_vbl.request.type = DRM_VBLANK_RELATIVE | pipe_id_flag;
@@ -732,7 +732,7 @@ static void test_invalid(data_t *data)
 
 static void queue_vblank(data_t *data)
 {
-	int pipe = kmstest_get_pipe_from_crtc_id(data->drm_fd, data->crtc_id);
+	int pipe = kmstest_get_crtc_index_from_id(data->drm_fd, data->crtc_id);
 	drmVBlank wait_vbl = {
 		.request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT |
 			kmstest_get_vbl_flag(pipe),
@@ -855,7 +855,7 @@ static void test_crc(data_t *data)
 	igt_assert_eq(ret, 0);
 
 	data->pipe_crc = igt_pipe_crc_new(data->drm_fd,
-					  kmstest_get_pipe_from_crtc_id(data->drm_fd, data->crtc_id),
+					  kmstest_get_crtc_index_from_id(data->drm_fd, data->crtc_id),
 					  IGT_PIPE_CRC_SOURCE_AUTO);
 
 	igt_pipe_crc_start(data->pipe_crc);
diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c
index 108e713610fd..8ed6113205f6 100644
--- a/tests/kms_setmode.c
+++ b/tests/kms_setmode.c
@@ -354,8 +354,7 @@ static void setup_crtcs(const struct test_config *tconf,
 					  resources->crtcs[crtc->crtc_idx]);
 		crtc->crtc_id = drm_crtc->crtc_id;
 		drmModeFreeCrtc(drm_crtc);
-		crtc->pipe_id = kmstest_get_pipe_from_crtc_id(drm_fd,
-							      crtc->crtc_id);
+		crtc->pipe_id = kmstest_get_crtc_index_from_id(drm_fd, crtc->crtc_id);
 
 		crtc->connector_count = 1;
 		for (j = i + 1; j < connector_count; j++)
-- 
2.47.3

[PATCH i-g-t 2/3] lib/igt_kms: Drop the ioctl fallback for CRTC id to pipe mapping

[Jani Nikula]

The kernel support for i915_pipe debugfs has been around for 2½ years in
both i915 and xe. Drop the ioctl fallback for simplicity. Rename the
function __intel_get_pipe_from_crtc_index() as it's now purely about
CRTC index and not id.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 lib/igt_kms.c | 44 +++++++++-----------------------------------
 1 file changed, 9 insertions(+), 35 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 57e6474f2c2d..f6769d31edcb 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1312,51 +1312,27 @@ void kmstest_dump_mode(drmModeModeInfo *mode)
 }
 
 /*
- * With non-contiguous pipes display, crtc mapping is not always same
- * as pipe mapping, In i915 pipe is enum id of i915's crtc object.
- * hence allocating upper bound igt_pipe array to support non-contiguos
- * pipe display and reading pipe enum for a crtc using GET_PIPE_FROM_CRTC_ID
- * ioctl for a pipe to do pipe ordering with respect to crtc list.
+ * The hardware pipe may be different from the CRTC index. Figure out the CRTC
+ * index to pipe mapping from the debugfs.
  */
-static int __intel_get_pipe_from_crtc_id(int fd, int crtc_id, int crtc_idx)
+static int __intel_get_pipe_from_crtc_index(int fd, int crtc_index)
 {
 	char buf[2];
 	int debugfs_fd, res = 0;
+	char pipe_char;
 
-	/*
-	 * No GET_PIPE_FROM_CRTC_ID ioctl support for XE. Instead read
-	 * from the debugfs "i915_pipe".
-	 *
-	 * This debugfs is applicable for both i915 & XE. For i915, still
-	 * we can fallback to ioctl method to support older kernels.
-	 */
-	debugfs_fd = igt_debugfs_pipe_dir(fd, crtc_idx, O_RDONLY);
+	debugfs_fd = igt_debugfs_pipe_dir(fd, crtc_index, O_RDONLY);
 
 	if (debugfs_fd >= 0) {
 		res = igt_debugfs_simple_read(debugfs_fd, "i915_pipe", buf, sizeof(buf));
 		close(debugfs_fd);
 	}
 
-	if (res <= 0) {
-		/* Fallback to older ioctl method. */
-		if (is_i915_device(fd)) {
-			struct drm_i915_get_pipe_from_crtc_id get_pipe;
-
-			get_pipe.pipe = 0;
-			get_pipe.crtc_id =  crtc_id;
+	igt_assert_f(res > 0, "Failed to read the debugfs i915_pipe.\n");
 
-			do_ioctl(fd, DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID,
-				 &get_pipe);
+	igt_assert_eq(sscanf(buf, "%c", &pipe_char), 1);
 
-			return get_pipe.pipe;
-		} else
-			igt_assert_f(false, "XE: Failed to read the debugfs i915_pipe.\n");
-	} else {
-		char pipe;
-
-		igt_assert_eq(sscanf(buf, "%c", &pipe), 1);
-		return kmstest_pipe_to_index(pipe);
-	}
+	return kmstest_pipe_to_index(pipe_char);
 }
 
 /**
@@ -3229,9 +3205,7 @@ void igt_display_require(igt_display_t *display, int drm_fd)
 
 	for (i = 0; i < resources->count_crtcs; i++) {
 		igt_crtc_t *pipe;
-		int pipe_enum = (is_intel_dev)?
-			__intel_get_pipe_from_crtc_id(drm_fd,
-						      resources->crtcs[i], i) : i;
+		int pipe_enum = is_intel_dev ? __intel_get_pipe_from_crtc_index(drm_fd, i) : i;
 
 		pipe = igt_crtc_for_pipe(display, pipe_enum);
 		pipe->pipe = pipe_enum;
-- 
2.47.3

[PATCH i-g-t 3/3] lib/igt_kms: Remove kmstest_pipe_to_index()

[Jani Nikula]

There's only a single user for kmstest_pipe_to_index(), and that should
be the extent of it. There's no reason for other code than the i915_pipe
debugfs parsing to convert from pipe name character to the enum
pipe. And for that, the name of the function is misleading anyway.

Inline the functionality in __intel_get_pipe_from_crtc_index(), using
igt_assert_f() for invalid values instead of propagating -EINVAL which
isn't even checked by the caller.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 lib/igt_kms.c       | 24 ++++++------------------
 lib/igt_kms.h       |  1 -
 tests/kms_setmode.c |  1 +
 3 files changed, 7 insertions(+), 19 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index f6769d31edcb..08b982f0f68f 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1061,22 +1061,6 @@ const char *kmstest_pipe_name(enum pipe pipe)
 	return str + (pipe * 2);
 }
 
-/**
- * kmstest_pipe_to_index:
- * @pipe: display pipe in string format
- *
- * Returns: Index to corresponding pipe
- */
-int kmstest_pipe_to_index(char pipe)
-{
-	int r = pipe - 'A';
-
-	if (r < 0 || r >= IGT_MAX_PIPES)
-		return -EINVAL;
-
-	return r;
-}
-
 /**
  * kmstest_plane_type_name:
  * @plane_type: display plane type
@@ -1318,7 +1302,7 @@ void kmstest_dump_mode(drmModeModeInfo *mode)
 static int __intel_get_pipe_from_crtc_index(int fd, int crtc_index)
 {
 	char buf[2];
-	int debugfs_fd, res = 0;
+	int debugfs_fd, pipe, res = 0;
 	char pipe_char;
 
 	debugfs_fd = igt_debugfs_pipe_dir(fd, crtc_index, O_RDONLY);
@@ -1332,7 +1316,11 @@ static int __intel_get_pipe_from_crtc_index(int fd, int crtc_index)
 
 	igt_assert_eq(sscanf(buf, "%c", &pipe_char), 1);
 
-	return kmstest_pipe_to_index(pipe_char);
+	pipe = pipe_char - 'A';
+
+	igt_assert_f(pipe >= 0 && pipe < IGT_MAX_PIPES, "i915_pipe %c out of range\n", pipe_char);
+
+	return pipe;
 }
 
 /**
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index d25c166eba4a..a3b3d578f16d 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -90,7 +90,6 @@ enum pipe {
         IGT_MAX_PIPES
 };
 const char *kmstest_pipe_name(enum pipe pipe);
-int kmstest_pipe_to_index(char pipe);
 const char *kmstest_plane_type_name(int plane_type);
 
 enum port {
diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c
index 8ed6113205f6..3f61f72f772b 100644
--- a/tests/kms_setmode.c
+++ b/tests/kms_setmode.c
@@ -354,6 +354,7 @@ static void setup_crtcs(const struct test_config *tconf,
 					  resources->crtcs[crtc->crtc_idx]);
 		crtc->crtc_id = drm_crtc->crtc_id;
 		drmModeFreeCrtc(drm_crtc);
+		/* FIXME: The test functionally uses this as CRTC index, not pipe */
 		crtc->pipe_id = kmstest_get_crtc_index_from_id(drm_fd, crtc->crtc_id);
 
 		crtc->connector_count = 1;
-- 
2.47.3

Re: [PATCH i-g-t 3/3] lib/igt_kms: Remove kmstest_pipe_to_index()

[Jani Nikula]

On Tue, 20 Jan 2026, Jani Nikula <jani.nikula@intel.com> wrote:
> There's only a single user for kmstest_pipe_to_index(), and that should
> be the extent of it. There's no reason for other code than the i915_pipe
> debugfs parsing to convert from pipe name character to the enum
> pipe. And for that, the name of the function is misleading anyway.
>
> Inline the functionality in __intel_get_pipe_from_crtc_index(), using
> igt_assert_f() for invalid values instead of propagating -EINVAL which
> isn't even checked by the caller.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  lib/igt_kms.c       | 24 ++++++------------------
>  lib/igt_kms.h       |  1 -
>  tests/kms_setmode.c |  1 +
>  3 files changed, 7 insertions(+), 19 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index f6769d31edcb..08b982f0f68f 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1061,22 +1061,6 @@ const char *kmstest_pipe_name(enum pipe pipe)
>  	return str + (pipe * 2);
>  }
>  
> -/**
> - * kmstest_pipe_to_index:
> - * @pipe: display pipe in string format
> - *
> - * Returns: Index to corresponding pipe
> - */
> -int kmstest_pipe_to_index(char pipe)
> -{
> -	int r = pipe - 'A';
> -
> -	if (r < 0 || r >= IGT_MAX_PIPES)
> -		return -EINVAL;
> -
> -	return r;
> -}
> -
>  /**
>   * kmstest_plane_type_name:
>   * @plane_type: display plane type
> @@ -1318,7 +1302,7 @@ void kmstest_dump_mode(drmModeModeInfo *mode)
>  static int __intel_get_pipe_from_crtc_index(int fd, int crtc_index)
>  {
>  	char buf[2];
> -	int debugfs_fd, res = 0;
> +	int debugfs_fd, pipe, res = 0;
>  	char pipe_char;
>  
>  	debugfs_fd = igt_debugfs_pipe_dir(fd, crtc_index, O_RDONLY);
> @@ -1332,7 +1316,11 @@ static int __intel_get_pipe_from_crtc_index(int fd, int crtc_index)
>  
>  	igt_assert_eq(sscanf(buf, "%c", &pipe_char), 1);
>  
> -	return kmstest_pipe_to_index(pipe_char);
> +	pipe = pipe_char - 'A';
> +
> +	igt_assert_f(pipe >= 0 && pipe < IGT_MAX_PIPES, "i915_pipe %c out of range\n", pipe_char);
> +
> +	return pipe;
>  }
>  
>  /**
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index d25c166eba4a..a3b3d578f16d 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -90,7 +90,6 @@ enum pipe {
>          IGT_MAX_PIPES
>  };
>  const char *kmstest_pipe_name(enum pipe pipe);
> -int kmstest_pipe_to_index(char pipe);
>  const char *kmstest_plane_type_name(int plane_type);
>  
>  enum port {
> diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c
> index 8ed6113205f6..3f61f72f772b 100644
> --- a/tests/kms_setmode.c
> +++ b/tests/kms_setmode.c
> @@ -354,6 +354,7 @@ static void setup_crtcs(const struct test_config *tconf,
>  					  resources->crtcs[crtc->crtc_idx]);
>  		crtc->crtc_id = drm_crtc->crtc_id;
>  		drmModeFreeCrtc(drm_crtc);
> +		/* FIXME: The test functionally uses this as CRTC index, not pipe */

Argh, I git commit -a'd this hunk into the wrong patch. Was supposed to
be patch 1.

>  		crtc->pipe_id = kmstest_get_crtc_index_from_id(drm_fd, crtc->crtc_id);
>  
>  		crtc->connector_count = 1;

-- 
Jani Nikula, Intel

✗ Fi.CI.BUILD: failure for lib/igt_kms: pipe vs CRTC index cleanups

[Patchwork]

== Series Details ==

Series: lib/igt_kms: pipe vs CRTC index cleanups
URL   : https://patchwork.freedesktop.org/series/160345/
State : failure

== Summary ==

Applying: lib/igt_kms: Always return CRTC index from kmstest_get_pipe_from_crtc_id()
Using index info to reconstruct a base tree...
M	lib/igt_kms.c
M	lib/igt_kms.h
M	tests/kms_async_flips.c
Falling back to patching base and 3-way merge...
Auto-merging tests/kms_async_flips.c
CONFLICT (content): Merge conflict in tests/kms_async_flips.c
Auto-merging lib/igt_kms.h
Auto-merging lib/igt_kms.c
Patch failed at 0001 lib/igt_kms: Always return CRTC index from kmstest_get_pipe_from_crtc_id()
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".