Re: [PATCH i-g-t 08/10] lib/kms: Introduce igt_output_set_crtc()

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

On Wed, 10 Dec 2025, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Introduce igt_output_set_crtc() as the intended replacement for
> igt_output_set_pipe().
>
> One step towards using 'crtcs' instead of 'pipes'.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  lib/igt_kms.c | 44 +++++++++++++++++++++++++++++++-------------
>  lib/igt_kms.h |  1 +
>  2 files changed, 32 insertions(+), 13 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 3b71cee61635..40c521b3a331 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -5277,31 +5277,35 @@ int igt_output_preferred_vrefresh(igt_output_t *output)
>  		return 60;
>  }
>  
> +static const char *igt_crtc_name(igt_pipe_t *crtc)
> +{
> +	if (crtc == NULL)
> +		return "None";
> +
> +	return kmstest_pipe_name(crtc->pipe);
> +}
> +
>  /**
> - * igt_output_set_pipe:
> + * igt_output_set_crtc:
>   * @output: Target output for which the pipe is being set to
> - * @pipe: Display pipe to set to
> + * @pipe_obj: CRTC to set to
>   *
> - * This function sets a @pipe to a specific @output connector by
> - * setting the CRTC_ID property of the @pipe. The pipe
> - * is only activated for all pipes except PIPE_NONE.
> + * This function sets a @crtc to a specific @output connector by
> + * setting the CRTC_ID property of the @crtc.
>   */
> -void igt_output_set_pipe(igt_output_t *output, enum pipe pipe)
> +void igt_output_set_crtc(igt_output_t *output, igt_pipe_t *pipe_obj)
>  {
>  	igt_display_t *display = output->display;
> -	igt_pipe_t *old_pipe = NULL, *pipe_obj = NULL;;
> +	igt_pipe_t *old_pipe = NULL;
>  
>  	igt_assert(output->name);
>  
>  	if (output->pending_pipe != PIPE_NONE)
>  		old_pipe = igt_output_get_driving_pipe(output);
>  
> -	if (pipe != PIPE_NONE)
> -		pipe_obj = igt_crtc_for_pipe(display, pipe);
> -
>  	LOG(display, "%s: set_pipe(%s)\n", igt_output_name(output),
> -	    kmstest_pipe_name(pipe));
> -	output->pending_pipe = pipe;
> +	    igt_crtc_name(pipe_obj));
> +	output->pending_pipe = pipe_obj ? pipe_obj->pipe : PIPE_NONE;
>  
>  	if (old_pipe) {
>  		igt_output_t *old_output;
> @@ -5318,7 +5322,7 @@ void igt_output_set_pipe(igt_output_t *output, enum pipe pipe)
>  	}
>  
>  	igt_output_set_prop_value(output, IGT_CONNECTOR_CRTC_ID,
> -				  pipe == PIPE_NONE ? 0 : igt_crtc_for_pipe(display, pipe)->crtc_id);
> +				  pipe_obj ? pipe_obj->crtc_id : 0);
>  
>  	igt_output_refresh(output);
>  
> @@ -5332,6 +5336,20 @@ void igt_output_set_pipe(igt_output_t *output, enum pipe pipe)
>  	}
>  }
>  
> +/**
> + * igt_output_set_pipe:
> + * @output: Target output for which the pipe is being set to
> + * @pipe_obj: Display pipe to set to
> + *
> + * This function sets a @pipe to a specific @output connector by
> + * setting the CRTC_ID property of the @pipe. The pipe
> + * is only activated for all pipes except PIPE_NONE.
> + */
> +void igt_output_set_pipe(igt_output_t *output, enum pipe pipe)
> +{
> +	igt_output_set_crtc(output, igt_crtc_for_pipe(output->display, pipe));

How does that work for the igt_output_set_pipe(output, PIPE_NONE) calls?
Seems like the idea was to pass NULL here? Or should igt_crtc_for_pipe()
handle PIPE_NONE and return NULL?

> +}
> +
>  static
>  bool __override_all_active_output_modes_to_fit_bw(igt_display_t *display,
>  						  igt_output_t *outputs[IGT_MAX_PIPES],
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index a9d2d9027ae8..9ec0f08ada24 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -589,6 +589,7 @@ drmModeModeInfo *igt_output_get_highres_mode(igt_output_t *output);
>  drmModeModeInfo *igt_output_get_lowres_mode(igt_output_t *output);
>  void igt_output_override_mode(igt_output_t *output, const drmModeModeInfo *mode);
>  int igt_output_preferred_vrefresh(igt_output_t *output);
> +void igt_output_set_crtc(igt_output_t *output, igt_pipe_t *pipe_obj);
>  void igt_output_set_pipe(igt_output_t *output, enum pipe pipe);
>  igt_plane_t *igt_output_get_plane(igt_output_t *output, int plane_idx);
>  igt_plane_t *igt_output_get_plane_type(igt_output_t *output, int plane_type);

-- 
Jani Nikula, Intel