Re: [PATCH i-g-t 01/17] lib/kms: Replace igt_pipe_has_valid_output() with igt_crtc_has_valid_output()

[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>
>
> Rename igt_pipe_has_valid_output() into igt_crtc_has_valid_output()
> and pass in the full igt_crtc_t*.
>
> And since we already have the crtc the igt_require_pipe() call
> no longer makes any sense. Move that into the one caller
> (igt_display_require_output_on_pipe()) where it might matter.

It's subtle and a bit bonkers, actually. igt_require_pipe() checks for
the crtc being valid. Which it is if it originates from
for_each_crtc(). If pipe is PIPE_NONE, it's a NULL dereference, so we
can assume that doesn't happen. But the crtc may also originate from
igt_crtc_for_pipe(), which does *not* have the crtc validity check.

This patch seems okay, though.

>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  lib/igt_kms.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 6087de365a82..7454d16780f9 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -2821,15 +2821,12 @@ static void igt_require_pipe(igt_display_t *display, enum pipe pipe)
>  			kmstest_pipe_name(pipe));
>  }
>  
> -static bool igt_pipe_has_valid_output(igt_display_t *display, enum pipe pipe)
> +static bool igt_crtc_has_valid_output(igt_crtc_t *crtc)
>  {
> +	igt_display_t *display = crtc->display;
>  	igt_output_t *output;
>  
> -	igt_require_pipe(display, pipe);
> -
> -	for_each_valid_output_on_crtc(display,
> -				      igt_crtc_for_pipe(display, pipe),
> -				      output)
> +	for_each_valid_output_on_crtc(display, crtc, output)
>  		return true;
>  
>  	return false;
> @@ -2949,7 +2946,7 @@ void igt_display_reset_outputs(igt_display_t *display)
>  	for_each_crtc(display, crtc) {
>  		igt_output_t *output;
>  
> -		if (!igt_pipe_has_valid_output(display, crtc->pipe))
> +		if (!igt_crtc_has_valid_output(crtc))
>  			continue;
>  
>  		output = igt_get_single_output_for_pipe(display, crtc->pipe);
> @@ -3266,7 +3263,9 @@ void igt_display_require_output(igt_display_t *display)
>   */
>  void igt_display_require_output_on_pipe(igt_display_t *display, enum pipe pipe)
>  {
> -	if (!igt_pipe_has_valid_output(display, pipe))
> +	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));
>  }

-- 
Jani Nikula, Intel