Re: [PATCH i-g-t v4 04/46] lib/igt_kms: Add helper to obtain a connector by its name or MST path

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

On Mon Nov 10, 2025 at 2:39 PM CET, Louis Chauvet wrote:
> Introduce the functions igt_get_connector_from_name() and
> igt_get_connector_id_from_mst_path(). These functions serve to retrieve
> the connector object using a connector name and a connector ID from its
> MST path, respectively.
>
> Given that the connector id may not be consistent, especially with MST
> connectors, these functions are essential to recognize each connector even
> after system reboots and plug/unplug events.
>
> The function igt_get_connector_id_from_name() is a convenient wrapper for
> igt_get_connector_from_name() to streamline its usage when the caller only
> requires the connector id.
>
> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> ---
>  lib/igt_kms.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_kms.h |   3 ++
>  2 files changed, 110 insertions(+)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 6392999bb341..6f2a8abbaf7d 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -93,6 +93,10 @@
>  #define MAX_CONNECTORS 32
>  #define MAX_EDID 2
>  #define DISPLAY_TILE_BLOCK 0x12
> +/**
> + * define IGT_KMS_CONNECTOR_NAME_SIZE - Size used when a connector name is needed
> + */
> +#define IGT_KMS_CONNECTOR_NAME_SIZE 50
>
>  typedef bool (*igt_connector_attr_set)(int dir, const char *attr, const char *value);
>
> @@ -7726,3 +7730,106 @@ int igt_get_connected_connectors(int drm_fd, uint32_t **connector_ids)
>
>  	return connected_count;
>  }
> +
> +/**
> + * igt_get_connector_from_name:
> + * @drm_fd: DRM file descriptor
> + * @port_name: Port name to search
> + *
> + * Returns: The connector if found, NULL otherwise. The pointer must
> + * be freed with drmModeFreeConnector()
> + */
> +drmModeConnectorPtr igt_get_connector_from_name(int drm_fd, const char *port_name)
> +{
> +	drmModeResPtr res = drmModeGetResources(drm_fd);
> +	double timeout = igt_default_display_detect_timeout();
> +	int i, len;
> +
> +	if (!res)
> +		return NULL;
> +
> +	for (i = 0; i < res->count_connectors; i++) {
> +		char name[IGT_KMS_CONNECTOR_NAME_SIZE];
> +
> +		drmModeConnectorPtr connector = igt_wait_for_connector(drm_fd, res->connectors[i],
> +								       timeout);
> +
> +		if (connector) {
> +			/* We have to generate the connector name on our own */
> +			len = snprintf(name, ARRAY_SIZE(name), "%s-%u",

Any reason you prefer ARRAY_SIZE() over sizeof(), which I'd have expected
to see here??

Also, I see you are declaring most variables in the inner scope using them
instead of all at function scope. This is good, but you missed the 'len'
variable, which can be moved inside this scope.

> +				       kmstest_connector_type_str(connector->connector_type),
> +				       connector->connector_type_id);
> +			name[len] = 0;
> +
> +			if (strcmp(port_name, name) == 0) {
> +				drmModeFreeResources(res);
> +
> +				return connector;
> +			}
> +
> +			drmModeFreeConnector(connector);
> +		}
> +	}
> +
> +	drmModeFreeResources(res);
> +
> +	return NULL;
> +}

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


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