Re: [PATCH i-g-t v4 05/46] lib/igt_kms: Add helper to wait for new connectors

["Luca Ceresoli" <luca.ceresoli@bootlin.com>]

On Mon Nov 10, 2025 at 2:39 PM CET, Louis Chauvet wrote:
> Add list comparison utilities and a new function to wait for new
> connectors to appear on the system.
>
> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> ---
>  lib/igt_kms.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_kms.h |  6 ++++
>  2 files changed, 99 insertions(+)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 6f2a8abbaf7d..c77c94d7c437 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -7833,3 +7833,96 @@ uint32_t igt_get_connector_id_from_mst_path(int drm_fd, const void *mst_path)
>
>  	return 0;
>  }
> +
> +/**
> + * list_contains() - Search an element in the list
> + *
> + * @list: Pointer to the list to search into
> + * @list_len: Length of the list
> + * @value: Value to search in the list
> + *
> + * Returns true if @list contains @value
> + */
> +static bool list_contains(const uint32_t *list, int list_len, uint32_t value)
> +{
> +	igt_assert((!list && list_len == 0) || list);
> +	for (int i = 0; i < list_len; i++) {
> +		if (list[i] == value)
> +			return true;
> +	}
> +	return false;
> +}
> +
> +/**
> + * get_list_diff() - Compute and return the difference between two lists

Maybe s/difference/set difference/, because it's really what this function
does: a set difference.

> + *
> + * @list_a: Pointer to the first list to compare
> + * @list_a_len: Length of the first list
> + * @list_b: Pointer to the second list to compare
> + * @list_b_len: Length of the second list
> + * @diff: Out pointer for the difference. Can be null to only count new elements.

"new elements" here makes sense if you use this function in a given way:
list_a is new "new" list and list_b is the "old" list. So I'd rephrase
like:

 * @diff: Out pointer returning a list of items in list_a but not in
          list_b. Can be NULL to only count the elements.

> + *
> + * Returns the number of element which are in @list_a but not in @list_b.
> + */
> +int
> +get_list_diff(const uint32_t *list_a, int list_a_len, const uint32_t *list_b, int list_b_len,
> +	      uint32_t **diff)
> +{
> +	int diff_len = 0;
> +
> +	igt_assert((!list_a && list_a_len == 0) || list_a);
> +	igt_assert((!list_b && list_b_len == 0) || list_b);
> +
> +	if (diff)
> +		*diff = malloc(sizeof(**diff));
> +
> +	for (int i = 0; i < list_a_len; i++) {
> +		if (!list_contains(list_b, list_b_len, list_a[i])) {
> +			if (diff) {
> +				*diff = reallocarray(*diff, diff_len + 1, sizeof(**diff));
> +				igt_assert(*diff);
> +				(*diff)[diff_len] = list_a[i];
> +			}
> +
> +			diff_len++;
> +		}
> +	}
> +
> +	return diff_len;
> +}
> +
> +/**
> + * kms_wait_for_new_connectors() - Wait for new connector to appear
> + *
> + * @list_a: Pointer to the first list to compare
> + * @list_a_len: Length of the first list
> + * @list_b: Pointer to the second list to compare
> + * @list_b_len: Length of the second list
> + * @diff: Out pointer for the difference. Can be null.
> + *
> + * Returns the number of element which differ between the two lists.

These docs are a copy of the get_list_diff() docs, perhaps you forgot to
update them after copy-paste.

Based on the implementation I think this should sound like:

 * @newly_connected: Out pointer returning the list of connectors currently found connected
 * @already_connected: Input pointer to list of connectors previously connected
 * @already_connected_count: Length of @already_connected
 * @drm_fd: DRM file descriptor
 *
 * Returns the number of connectors not in @already_connected and currently
 * found connected.

> + */
> +int kms_wait_for_new_connectors(uint32_t **newly_connected,
> +				const uint32_t *already_connected,
> +				int already_connected_count, int drm_fd)
> +{
> +	int newly_connected_count;
> +	struct timespec start, end;
> +
> +	igt_assert(newly_connected);
> +	igt_assert(drm_fd);
> +
> +	clock_gettime(CLOCK_MONOTONIC, &start);
> +	clock_gettime(CLOCK_MONOTONIC, &end);
> +	do {
> +		if (*newly_connected)
> +			free(*newly_connected);
> +		newly_connected_count = igt_get_connected_connectors(drm_fd, newly_connected);
> +		clock_gettime(CLOCK_MONOTONIC, &end);
> +	} while (get_list_diff(*newly_connected, newly_connected_count,
> +			       already_connected, already_connected_count,
> +			       NULL) == 0 &&
> +		 igt_time_elapsed(&start, &end) <= igt_default_display_detect_timeout());
> +
> +	return newly_connected_count;
> +}

With the above fixed:
 Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com