Re: [PATCH i-g-t 02/17] lib/kms: Replace igt_display_require_output_on_pipe() with igt_display_require_output_on_crtc()

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

On Fri, 27 Feb 2026, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Replace the pipe based igt_display_require_output_on_pipe()
> with igt_display_require_output_on_crtc().
>
>  #include "scripts/iterators.cocci"
>
> @@
> expression DISPLAY, PIPE;
> @@
> - igt_display_require_output_on_pipe(DISPLAY, PIPE)
> + igt_display_require_output_on_crtc(igt_crtc_for_pipe(DISPLAY, PIPE))
>
> @@
> igt_crtc_t *CRTC;
> @@
> - igt_crtc_for_pipe(..., CRTC->pipe)
> + CRTC
>
> @@
> type T;
> @@
> T igt_display_require_output_on_pipe(...);
> +void igt_display_require_output_on_crtc(igt_crtc_t *crtc);
>
> @@
> @@
> igt_display_require_output_on_pipe(...) { ... }
> +/**
> + * igt_display_require_output_on_crtc:
> + * @crtc: CRTC
> + *
> + * Checks whether there's a valid @crtc/@output combination for the given @crtc
> + */
> +void igt_display_require_output_on_crtc(igt_crtc_t *crtc)
> +{
> +       if (!igt_crtc_has_valid_output(crtc))
> +               igt_skip("No valid connector found on CRTC %s\n", igt_crtc_name(crtc));
> +}
>
> @@
> @@
> - igt_display_require_output_on_pipe(...) { ... }
>
> @@
> @@
> - igt_pipe_has_valid_output(...) { ... }
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  lib/igt_kms.c                 | 17 +++++++----------
>  lib/igt_kms.h                 |  2 +-
>  tests/intel/kms_ccs.c         |  3 +--
>  tests/kms_plane_alpha_blend.c |  2 +-
>  tests/nouveau_crc.c           |  3 +--
>  5 files changed, 11 insertions(+), 16 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 7454d16780f9..11ed109d74fb 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -3254,19 +3254,16 @@ void igt_display_require_output(igt_display_t *display)
>  }
>  
>  /**
> - * igt_display_require_output_on_pipe:
> - * @display: A pointer to an #igt_display_t structure
> - * @pipe: Display pipe
> + * igt_display_require_output_on_crtc:
> + * @crtc: CRTC
>   *
> - * Checks whether there's a valid @pipe/@output combination for the given @display and @pipe
> - * Skips test if a valid @pipe is not found
> + * Checks whether there's a valid @crtc/@output combination for the given @crtc
>   */
> -void igt_display_require_output_on_pipe(igt_display_t *display, enum pipe pipe)
> +void igt_display_require_output_on_crtc(igt_crtc_t *crtc)
>  {
> -	igt_require_pipe(display, pipe);
> -
> -	if (!igt_crtc_has_valid_output(igt_crtc_for_pipe(display, pipe)))
> -		igt_skip("No valid connector found on pipe %s\n", kmstest_pipe_name(pipe));
> +	if (!igt_crtc_has_valid_output(crtc))
> +		igt_skip("No valid connector found on CRTC %s\n",
> +			 igt_crtc_name(crtc));
>  }
>  
>  /**
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 1e1f3ead5c99..c79c8664e9a9 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -570,7 +570,7 @@ void igt_display_commit_atomic(igt_display_t *display, uint32_t flags, void *use
>  int  igt_display_try_commit2(igt_display_t *display, enum igt_commit_style s);
>  int  igt_display_drop_events(igt_display_t *display);
>  void igt_display_require_output(igt_display_t *display);
> -void igt_display_require_output_on_pipe(igt_display_t *display, enum pipe pipe);
> +void igt_display_require_output_on_crtc(igt_crtc_t *crtc);
>  int igt_display_n_crtcs(igt_display_t *display);
>  
>  const char *igt_crtc_name(igt_crtc_t *crtc);
> diff --git a/tests/intel/kms_ccs.c b/tests/intel/kms_ccs.c
> index 2b570af355fc..4d0bda6a78f7 100644
> --- a/tests/intel/kms_ccs.c
> +++ b/tests/intel/kms_ccs.c
> @@ -1171,8 +1171,7 @@ static void test_output(data_t *data, const int testnum)
>  						igt_info("Testing with seed %d\n", data->seed);
>  
>  					if (data->flags & TEST_ALL_PLANES) {
> -						igt_display_require_output_on_pipe(&data->display,
> -										   crtc->pipe);
> +						igt_display_require_output_on_crtc(crtc);
>  
>  						for_each_plane_on_crtc(crtc,
>  								       data->plane) {
> diff --git a/tests/kms_plane_alpha_blend.c b/tests/kms_plane_alpha_blend.c
> index 41a3b71c3861..8b16a35ea70c 100644
> --- a/tests/kms_plane_alpha_blend.c
> +++ b/tests/kms_plane_alpha_blend.c
> @@ -659,7 +659,7 @@ static bool pipe_check(data_t *data, igt_crtc_t *crtc,
>  	igt_plane_t *plane;
>  	bool plane_alpha = false, plane_blend = false, multiply = false;
>  
> -	igt_display_require_output_on_pipe(display, crtc->pipe);
> +	igt_display_require_output_on_crtc(crtc);
>  	for_each_plane_on_crtc(crtc, plane) {
>  		if (!igt_plane_has_prop(plane, IGT_PLANE_ALPHA))
>  			continue;
> diff --git a/tests/nouveau_crc.c b/tests/nouveau_crc.c
> index 3137884dcfc6..98394674e463 100644
> --- a/tests/nouveau_crc.c
> +++ b/tests/nouveau_crc.c
> @@ -340,8 +340,7 @@ int igt_main()
>  			int dir;
>  
>  			data.crtc = igt_crtc_for_pipe(&data.display, pipe);
> -			igt_display_require_output_on_pipe(&data.display,
> -							   data.crtc->pipe);
> +			igt_display_require_output_on_crtc(data.crtc);

Because of what I explained in the previous patch, I believe this is now
broken. This one still uses for_each_pipe_static() iterating up to
IGT_MAX_PIPES, but the CRTCs are only valid up to the actual CRTC
count. crtc->display isn't initialized for invalid CRTCs.

I think the simple temporary fix is to add a manual data.crtc->valid
check here, similar to gem_eio.c which uses random pipes.

Long term, we just need to get rid of having invalid igt_crtc_t around.

BR,
Jani.


>  
>  			/* Disable the output from the previous iteration of pipe tests, if there is
>  			 * one

-- 
Jani Nikula, Intel