[PATCH i-g-t 2/3] lib/kms: refactor for_each_crtc_with_single_output() to avoid igt_crtc_for_pipe()

Jani Nikula
2026-03-23T12:06:53Z

Using igt_crtc_for_pipe() in for_each_crtc_with_single_output() is
problematic for further conversion from pipe to igt_crtc_t and CRTC
index usage. In particular, if we return igt_display_n_crtcs() to mean
the actual CRTC count for the device, and have a device with a pipe
fused off in the middle, we'll end up with a buffer overflow.

Add a new structure igt_output_crtc_t for holding the valid output/crtc
pointer combos instead of assuming the index in an igt_output_t pointer
array is the pipe. This doesn't fix everything yet, but is a step in the
right direction. Plus makes the whole loop ever so slightly easier to
understand.

For valid combos, both the output and crtc should always be non-NULL,
and for invalid combos, both of them should always be NULL. However,
check for both in the loop for good measure.

C macros being dumb, we have to rename the macro parameters to be able
to use output and crtc as the structure members. Meh.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
lib/igt_kms.c | 12 +++++++-----
lib/igt_kms.h | 23 ++++++++++++++---------
2 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index b835584bef1e..42329ad0303c 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -3588,7 +3588,7 @@ bool output_is_internal_panel(igt_output_t *output)
}
}

-igt_output_t **__igt_pipe_populate_outputs(igt_display_t *display, igt_output_t **chosen_outputs)
+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;
igt_output_t *output;
@@ -3634,15 +3634,17 @@ igt_output_t **__igt_pipe_populate_outputs(igt_display_t *display, igt_output_t
/* We found an unassigned CRTC, use it! */
found = true;
assigned_crtc_index_mask |= 1 << crtc->crtc_index;
- chosen_outputs[crtc->pipe] = output;
- } else if (!chosen_outputs[crtc->pipe] ||
- output_is_internal_panel(chosen_outputs[crtc->pipe])) {
+ 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)) {
/*
* Overwrite internal panel if not
* assigned, external outputs are faster
* to do modesets
*/
- chosen_outputs[crtc->pipe] = output;
+ chosen_outputs[crtc->pipe].output = output;
+ chosen_outputs[crtc->pipe].crtc = crtc;
}
}

diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index d7cc22d6cd61..aceac18ce240 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -520,6 +520,11 @@ typedef struct {
uint64_t values[IGT_NUM_CONNECTOR_PROPS];
} igt_output_t;

+typedef struct {
+ igt_output_t *output;
+ igt_crtc_t *crtc;
+} igt_output_crtc_t;
+
struct igt_display {
int drm_fd;
int log_shift;
@@ -763,25 +768,25 @@ static inline bool igt_crtc_connector_valid(igt_crtc_t *crtc, igt_output_t *outp
(output) + 1 : ((crtc)++, &(display)->outputs[0])) \
for_each_if ((crtc)->valid && igt_crtc_connector_valid((crtc), (output)))

-igt_output_t **__igt_pipe_populate_outputs(igt_display_t *display,
- igt_output_t **chosen_outputs);
+igt_output_crtc_t *__igt_output_crtc_populate(igt_display_t *display,
+ igt_output_crtc_t *chosen_outputs);

/**
* for_each_crtc_with_single_output:
* @display: a pointer to an #igt_display_t structure
- * @crtc: The CRTC for which this @crtc / @output combination is valid.
- * @output: The output for which this @crtc / @output combination is valid.
+ * @_crtc: The CRTC for which this @_crtc / @_output combination is valid.
+ * @_output: The output for which this @_crtc / @_output combination is valid.
*
* This loop is called over all CRTCs, and will try to find a compatible output
* for each CRTC. Unlike for_each_pipe_with_valid_output(), this function will
* be called at most once for each pipe.
*/
-#define for_each_crtc_with_single_output(display, crtc, output) \
- for (igt_output_t *__outputs[igt_display_n_crtcs(display)], \
- **__output = __igt_pipe_populate_outputs((display), __outputs); \
+#define for_each_crtc_with_single_output(display, _crtc, _output) \
+ for (igt_output_crtc_t __outputs[igt_display_n_crtcs(display)], \
+ *__output = __igt_output_crtc_populate((display), __outputs); \
__output < &__outputs[igt_display_n_crtcs(display)]; __output++) \
- for_each_if (*__output && \
- ((crtc) = igt_crtc_for_pipe((display), (__output - __outputs)), (output) = *__output, 1))
+ for_each_if (__output->output && __output->crtc && \
+ ((_output) = __output->output, (_crtc) = __output->crtc, 1))

/**
* for_each_valid_output_on_crtc:
--
2.47.3