[PATCH i-g-t v3 4/5] lib/igt_kms: Add helper to obtain a connector by its name or MST path

[Kamil Konieczny]

Hi Louis,
On 2024-11-22 at 16:19:07 +0100, 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 at bootlin.com>
> ---
>  lib/igt_kms.c | 106 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_kms.h |   3 ++
>  2 files changed, 109 insertions(+)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 5d3f609d469d741d9fd68a9a0b41ab0162d322b6..5c242a5bdec619d181c3d0d632f1ea7d94d34743 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -7314,3 +7314,109 @@ 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);
> +	struct timespec start, end;
> +	int i;
> +
> +	if (!res)
> +		return 0;

return NULL;

> +
> +	for (i = 0; i < res->count_connectors; i++) {
> +		char name[50];
------------------------- ^^
Make it a define here or in a header for example:

#define IGT_KMS_CONNECTOR_NAME_SIZE 50

> +
> +		drmModeConnectorPtr connector = drmModeGetConnector(drm_fd, res->connectors[i]);
> +		/*
> +		 * This time is required as sometimes some id in the connector list are not totally
> +		 * ready or can disappear
> +		 */
> +		clock_gettime(CLOCK_MONOTONIC, &start);
> +		end = start;
> +		while (!connector &&
> +		       igt_time_elapsed(&start, &end) <= igt_default_detect_timeout()) {
> +			connector = drmModeGetConnector(drm_fd, res->connectors[i]);
> +			clock_gettime(CLOCK_MONOTONIC, &end);
> +		}
> +

You repeat same code here, why not write down a helper for it?
For example:

		drmModeConnectorPtr connector = igt_wait_for_connector(drm_fd, res->connectors[i]), timeout);


> +		if (igt_time_elapsed(&start, &end) <= igt_default_detect_timeout()) {
> +			igt_assert(connector);

Replace these two lines with:

		if (connector) {

> +
> +			/* We have to generate the connector name on our own */
> +			snprintf(name, ARRAY_SIZE(name), "%s-%u",
> +				 kmstest_connector_type_str(connector->connector_type),
> +				 connector->connector_type_id);

to be safe better:

			len = snprintf(name, ARRAY_SIZE(name) - 1, "%s-%u",
			name[len] = 0;

> +
> +			if (strcmp(port_name, name) == 0) {
> +				drmModeFreeResources(res);

Add empty line here.

> +				return connector;
> +			}

Add empty line here.

> +			drmModeFreeConnector(connector);
> +		}
> +	}

Add empty line here.

> +	drmModeFreeResources(res);

Add empty line here.

> +	return NULL;
> +}
> +
> +/**
> + * igt_get_connector_id_from_name:
> + * @drm_fd: DRM file descriptor
> + * @port_name: Port name to find in the connector
> + *
> + * Returns: The connector id if the port is found, 0 if the port is not found
> + */
> +uint32_t igt_get_connector_id_from_name(int drm_fd, const char *port_name)
> +{
> +	drmModeConnectorPtr connector = igt_get_connector_from_name(drm_fd, port_name);
> +
> +	if (connector) {
> +		int connector_id = connector->connector_id;

s/int/uint32_t/

> +
> +		drmModeFreeConnector(connector);

Add empty line here.

> +		return connector_id;
> +	}

Add empty line here.

> +	return 0;
> +}
> +
> +/**
> + * igt_get_connector_id_from_mst_path:
> + * @drm_fd: DRM file descriptor
> + * @mst_path: MST path to find in the connector
> + *
> + * Returns: 0 when no connector is found with the correct mst path
> + */
> +uint32_t igt_get_connector_id_from_mst_path(int drm_fd, const void *mst_path)
> +{
> +	drmModeResPtr res = drmModeGetResources(drm_fd);
> +	int i;
> +
> +	if (!res)
> +		return 0;
> +
> +	for (i = 0; i < res->count_connectors; i++) {
> +		uint32_t connector_id = res->connectors[i];
> +
> +		drmModePropertyBlobPtr path_blob = kmstest_get_path_blob(drm_fd, connector_id);
> +
> +		if (path_blob) {
> +			if (strcmp(path_blob->data, mst_path) == 0) {

I do not know KMS that good, is it always a string here in
path_blob->data

> +				drmModeFreePropertyBlob(path_blob);
> +				drmModeFreeResources(res);

Add empty line here.

> +				return connector_id;
> +			}

Add empty line here.

> +			drmModeFreePropertyBlob(path_blob);
> +		}
> +	}

Add empty line here.

> +	drmModeFreeResources(res);

Add empty line here.

Regards,
Kamil

> +	return 0;
> +}
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 33079b696c070ee7b62ec1d5705b221ba4ad5a7c..d61ddcaa0a25f1f9bc34eff7314eaa49a3de9cc2 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -1278,5 +1278,8 @@ float igt_default_detect_timeout(void);
>  bool igt_wait_for_connector_status(int drm_fd, unsigned int connector_id, double timeout,
>  				   int drm_mode);
>  int igt_get_connected_connectors(int drm_fd, uint32_t **connector_ids);
> +drmModeConnectorPtr igt_get_connector_from_name(int drm_fd, const char *port_name);
> +uint32_t igt_get_connector_id_from_name(int drm_fd, const char *port_name);
> +uint32_t igt_get_connector_id_from_mst_path(int drm_fd, const void *mst_path);
>  
>  #endif /* __IGT_KMS_H__ */
> 
> -- 
> 2.47.0
>