Re: [PATCH i-g-t v2 12/23] tests/drm_read: Use igt_crtc_t instead of enum pipe

[Jani Nikula <jani.nikula@intel.com>]

On Sat, 21 Feb 2026, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Convert drm_read from 'enum pipe' to 'igt_crtc_t'.
>
> The pipe usage in igt_main() creates a slight complication
> is this one.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

>
>  #include "scripts/iterators.cocci"
>
> @clean_igt_main@
> identifier PIPE;
> identifier CRTC;
> @@
> - enum pipe PIPE;
> + igt_crtc_t *CRTC;
> ...
> igt_fixture() {
> ...
> - igt_crtc_t *CRTC;
> <...
> (
> - PIPE = CRTC->pipe;
> |
> - PIPE
> + CRTC->pipe
> )
> ...>
> }
> <...
> - PIPE
> + CRTC->pipe
> ...>
>
> @func1@
> typedef igt_output_t;
> typedef igt_crtc_t;
> identifier FUNC, PIPE, CRTC;
> parameter list[N] P;
> @@
> FUNC(P
> -	,enum pipe PIPE
> +	,igt_crtc_t *CRTC
> 	,...)
> {
> ...
> (
> - igt_crtc_t *CRTC = igt_crtc_for_pipe(..., PIPE);
> |
> - igt_crtc_t *CRTC;
> ... when != PIPE = ...
> - CRTC = igt_crtc_for_pipe(..., PIPE);
> )
> <... when != PIPE = ...
> (
> - igt_crtc_for_pipe(..., PIPE)
> + CRTC
> |
> - kmstest_pipe_name(PIPE)
> + igt_crtc_name(CRTC)
> |
> - PIPE
> + CRTC->pipe
> )
> ...>
> }
>
> @depends on func1@
> identifier func1.FUNC;
> expression list[func1.N] EP;
> expression PIPE;
> @@
> FUNC(EP
> -	,PIPE
> +	,igt_crtc_for_pipe(display, PIPE)
> 	,...)
>
> @func2@
> typedef igt_crtc_t;
> identifier FUNC, PIPE;
> parameter list[N] P;
> @@
> FUNC(P
> -	,enum pipe PIPE
> +	,igt_crtc_t *crtc
> 	,...)
> {
> <+... when != PIPE = ...
> (
> - igt_crtc_for_pipe(..., PIPE)
> + crtc
> |
> - kmstest_pipe_name(PIPE)
> + igt_crtc_name(crtc)
> |
> - PIPE
> + crtc->pipe
> )
> ...+>
> }
>
> @depends on func2@
> identifier func2.FUNC;
> expression list[func2.N] EP;
> expression PIPE;
> @@
> FUNC(EP
> -	,PIPE
> +	,igt_crtc_for_pipe(display, PIPE)
> 	,...)
>
> @@
> igt_crtc_t *CRTC;
> @@
> - igt_crtc_for_pipe(..., CRTC->pipe)
> + CRTC
>
> @@
> typedef igt_display_t;
> identifier DISPLAY;
> @@
> - igt_display_t *DISPLAY = ...;
> ... when != DISPLAY
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  tests/drm_read.c | 36 ++++++++++++++++++------------------
>  1 file changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/tests/drm_read.c b/tests/drm_read.c
> index e745e98534bd..23b9ebe0faa3 100644
> --- a/tests/drm_read.c
> +++ b/tests/drm_read.c
> @@ -77,9 +77,9 @@ static void assert_empty(int fd)
>  	do_or_die(poll(&pfd, 1, 0));
>  }
>  
> -static void generate_event(int fd, enum pipe pipe)
> +static void generate_event(int fd, igt_crtc_t *crtc)
>  {
> -	igt_assert(kmstest_get_vblank(fd, pipe, DRM_VBLANK_EVENT));
> +	igt_assert(kmstest_get_vblank(fd, crtc->pipe, DRM_VBLANK_EVENT));
>  }
>  
>  static void wait_for_event(int fd)
> @@ -132,7 +132,7 @@ static void test_invalid_buffer(int in)
>  	teardown(fd);
>  }
>  
> -static void test_fault_buffer(int in, enum pipe pipe)
> +static void test_fault_buffer(int in, igt_crtc_t *crtc)
>  {
>  	int fd = setup(in, 0);
>  	struct drm_mode_map_dumb arg;
> @@ -146,7 +146,7 @@ static void test_fault_buffer(int in, enum pipe pipe)
>  	buf = mmap(0, 4096, PROT_WRITE, MAP_SHARED, fd, arg.offset);
>  	igt_assert(buf != MAP_FAILED);
>  
> -	generate_event(fd, pipe);
> +	generate_event(fd, crtc);
>  
>  	alarm(1);
>  
> @@ -168,13 +168,13 @@ static void test_empty(int in, int nonblock, int expected)
>  	teardown(fd);
>  }
>  
> -static void test_short_buffer(int in, int nonblock, enum pipe pipe)
> +static void test_short_buffer(int in, int nonblock, igt_crtc_t *crtc)
>  {
>  	char buffer[1024]; /* events are typically 32 bytes */
>  	int fd = setup(in, nonblock);
>  
> -	generate_event(fd, pipe);
> -	generate_event(fd, pipe);
> +	generate_event(fd, crtc);
> +	generate_event(fd, crtc);
>  
>  	wait_for_event(fd);
>  
> @@ -214,7 +214,7 @@ static void *thread_short_buffer_wakeup(void *arg)
>  	return NULL;
>  }
>  
> -static void test_short_buffer_wakeup(int in, enum pipe pipe)
> +static void test_short_buffer_wakeup(int in, igt_crtc_t *crtc)
>  {
>  	const int nt = sysconf(_SC_NPROCESSORS_ONLN) + 1;
>  	struct short_buffer_wakeup w = {
> @@ -243,7 +243,7 @@ static void test_short_buffer_wakeup(int in, enum pipe pipe)
>  		sched_yield();
>  
>  		/* One event should wake all threads as none consume */
> -		generate_event(w.fd, pipe);
> +		generate_event(w.fd, crtc);
>  
>  		clock_gettime(CLOCK_REALTIME, &tv);
>  		tv.tv_sec += 5; /* Let's be very generous to the scheduler */
> @@ -274,13 +274,12 @@ int igt_main()
>  {
>  	igt_display_t display;
>  	struct igt_fb fb;
> -	enum pipe pipe;
> +	igt_crtc_t *crtc;
>  	igt_fd_t(fd);
>  
>  	igt_fixture() {
>  		struct sigaction alarm_action = {};
>  		igt_output_t *output;
> -		igt_crtc_t *crtc;
>  
>  		igt_assert_neq(sigaction(SIGALRM, NULL, &alarm_action), -1);
>  		alarm_action.sa_flags &= ~SA_RESTART;
> @@ -302,20 +301,18 @@ int igt_main()
>  
>  			igt_output_set_crtc(output, crtc);
>  			igt_plane_set_fb(igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY), &fb);
> -
> -			pipe = crtc->pipe;
>  			break;
>  		}
>  
>  		igt_display_commit2(&display, display.is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
> -		igt_require(kmstest_get_vblank(fd, pipe, 0));
> +		igt_require(kmstest_get_vblank(fd, crtc->pipe, 0));
>  	}
>  
>  	igt_subtest("invalid-buffer")
>  		test_invalid_buffer(fd);
>  
>  	igt_subtest("fault-buffer")
> -		test_fault_buffer(fd, pipe);
> +		test_fault_buffer(fd, crtc);
>  
>  	igt_subtest("empty-block")
>  		test_empty(fd, 0, EINTR);
> @@ -324,11 +321,14 @@ int igt_main()
>  		test_empty(fd, 1, EAGAIN);
>  
>  	igt_subtest("short-buffer-block")
> -		test_short_buffer(fd, 0, pipe);
> +		test_short_buffer(fd, 0,
> +				  crtc);
>  
>  	igt_subtest("short-buffer-nonblock")
> -		test_short_buffer(fd, 1, pipe);
> +		test_short_buffer(fd, 1,
> +				  crtc);
>  
>  	igt_subtest("short-buffer-wakeup")
> -		test_short_buffer_wakeup(fd, pipe);
> +		test_short_buffer_wakeup(fd,
> +					 crtc);
>  }

-- 
Jani Nikula, Intel