Re: [PATCH i-g-t 3/3] lib/kms: use CRTC index for __igt_output_crtc_populate()

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Mar 23, 2026 at 02:06:26PM +0200, Jani Nikula wrote:
> Fill the valid output/crtc combos in __igt_output_crtc_populate() using
> CRTC index instead of pipe. This avoids buffer overruns if we reduce
> igt_display_n_crtcs() to the actual number of CRTCs on the device.
> 
> To keep the for_each_crtc_with_single_output() iteration in pipe order
> on Intel devices, sort the resulting array in a separate
> step. Presumably the performance impact of qsort() on an array of at
> most 16 elements is neglible.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  lib/igt_kms.c | 29 +++++++++++++++++++++++------
>  1 file changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 42329ad0303c..b92bb16aaf9c 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -3588,6 +3588,18 @@ bool output_is_internal_panel(igt_output_t *output)
>  	}
>  }
>  
> +static int output_crtc_pipe_compare(const void *_a, const void *_b)
> +{
> +	const igt_output_crtc_t *a = _a, *b = _b;
> +
> +	/* pipe order for valid output/crtc combos */
> +	if (a->crtc && b->crtc)
> +		return a->crtc->pipe - b->crtc->pipe;
> +
> +	/* valid combos before empty elements */
> +	return !b->crtc - !a->crtc;
> +}
> +
>  igt_output_crtc_t *__igt_output_crtc_populate(igt_display_t *display, igt_output_crtc_t *chosen_outputs)
>  {
>  	unsigned int full_crtc_index_mask = 0, assigned_crtc_index_mask = 0;
> @@ -3634,17 +3646,17 @@ igt_output_crtc_t *__igt_output_crtc_populate(igt_display_t *display, igt_output
>  					/* We found an unassigned CRTC, use it! */
>  					found = true;
>  					assigned_crtc_index_mask |= 1 << crtc->crtc_index;
> -					chosen_outputs[crtc->pipe].output = output;
> -					chosen_outputs[crtc->pipe].crtc = crtc;
> -				} else if (!chosen_outputs[crtc->pipe].output ||
> -					   output_is_internal_panel(chosen_outputs[crtc->pipe].output)) {
> +					chosen_outputs[crtc->crtc_index].output = output;
> +					chosen_outputs[crtc->crtc_index].crtc = crtc;
> +				} else if (!chosen_outputs[crtc->crtc_index].output ||
> +					   output_is_internal_panel(chosen_outputs[crtc->crtc_index].output)) {
>  					/*
>  					 * Overwrite internal panel if not
>  					 * assigned, external outputs are faster
>  					 * to do modesets
>  					 */
> -					chosen_outputs[crtc->pipe].output = output;
> -					chosen_outputs[crtc->pipe].crtc = crtc;
> +					chosen_outputs[crtc->crtc_index].output = output;
> +					chosen_outputs[crtc->crtc_index].crtc = crtc;
>  				}
>  			}
>  
> @@ -3654,6 +3666,11 @@ igt_output_crtc_t *__igt_output_crtc_populate(igt_display_t *display, igt_output
>  		}
>  	}
>  
> +	/* For Intel, sort the output/crtc combos in pipe order */
> +	if (is_intel_device(display->drm_fd))
> +		qsort(chosen_outputs, igt_display_n_crtcs(display),
> +		      sizeof(*chosen_outputs), output_crtc_pipe_compare);

Maybe one day we can drop this sorting entirely. But that needs some
testing, so this seems good for the moment.

Didn't spot anything bad, so for the series
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +
>  	return chosen_outputs;
>  }
>  
> -- 
> 2.47.3

-- 
Ville Syrjälä
Intel