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

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

On Thu, 26 Mar 2026, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> 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.

Yeah, maybe we'll eventually want to switch to CRTC order iteration in
for_each_crtc() too, but I just don't want to open that can of worms
here yet.

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

Thanks, pushed.

BR,
Jani.


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

-- 
Jani Nikula, Intel