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

Jani Nikula
2026-03-26T12:36:56Z

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