Re: [PATCH i-g-t v7 21/48] lib/unigraf: Introduce device configuration

Kory Maincent
2026-02-23T14:22:35Z

On Mon, 23 Feb 2026 13:09:28 +0100
Louis Chauvet <louis.chauvet@bootlin.com> wrote:

> As there could be multiple devices detected by libTSI, add a configuration
> field to ensure that igt will use the proper unigraf device.
>
> The unigraf integration will search for a [Unigraf] entry containing a
> Device=
>
> For example, it can look like:
>
> [Unigraf]
> Device=UCD-500 [2434C620]
>
> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> ---
> lib/unigraf/unigraf.c | 57
> +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 55
> insertions(+), 2 deletions(-)
>
> diff --git a/lib/unigraf/unigraf.c b/lib/unigraf/unigraf.c
> index df18ab178e16..ec709a55c106 100644
> --- a/lib/unigraf/unigraf.c
> +++ b/lib/unigraf/unigraf.c
> @@ -10,8 +10,9 @@
> #include <stdint.h>
>
> #include "unigraf.h"
> -#include "TSI.h"
> #include "TSI_types.h"
> +#include "TSI.h"

Nit: You move this include for nothing.

> +#include "igt_rc.h"
>
> #define unigraf_debug(fmt, ...) igt_debug("TSI:%p: " fmt,
> unigraf_device, ##__VA_ARGS__)
> @@ -19,6 +20,21 @@ static TSI_HANDLE unigraf_device;
> static char *unigraf_default_edid;
> static char *unigraf_connector_name;
>
> +/**
> + * UNIGRAF_NAME_MAX - Maximum name length to be used for TSI functions
> + */
> +#define UNIGRAF_NAME_MAX 1024
> +
> +/**
> + * UNIGRAF_CONFIG_GROUP - Name of the unigraf group in the configuration file
> + */
> +#define UNIGRAF_CONFIG_GROUP "Unigraf"
> +
> +/**
> + * UNIGRAF_CONFIG_DEVICE_NAME - Key of the device name in the configuration
> file
> + */
> +#define UNIGRAF_CONFIG_DEVICE_NAME "Device"
> +
> static void unigraf_close_device(void)
> {
> if (!unigraf_device)
> @@ -65,6 +81,22 @@ static unsigned int unigraf_device_count(void)
> return unigraf_assert(TSIX_DEV_GetDeviceCount());
> }
>
> +static int unigraf_find_device(char *request)
> +{
> + int device_count = unigraf_device_count();
> +
> + for (int i = 0; i < device_count; i++) {
> + char dev_name[UNIGRAF_NAME_MAX] = "";
> +
> + memset(dev_name, 0, UNIGRAF_NAME_MAX);

This memset is useless here.

> + unigraf_assert(TSIX_DEV_GetDeviceName(i, dev_name,
> UNIGRAF_NAME_MAX));
> + unigraf_debug("Detected unigraf device %d: %s\n", i,
> dev_name);
> + if (!strncmp(dev_name, request, UNIGRAF_NAME_MAX))
> + return i;
> + }
> + return -ENODEV;
> +}
> +
> /**
> * unigraf_open_device() - Search and open a device.
> * @drm_fd: File descriptor of the currently used drm device
> @@ -76,8 +108,10 @@ static unsigned int unigraf_device_count(void)
> bool unigraf_open_device(int drm_fd)
> {
> TSI_RESULT r;
> + GError *cfg_error = NULL;
> + char *cfg_device = NULL;
> int device_count;
> - int chosen_device = 0;
> + int chosen_device;
> int chosen_role = 0;
> int chosen_input = 0;
>
> @@ -88,6 +122,15 @@ bool unigraf_open_device(int drm_fd)
>
> unigraf_init();
>
> + if (igt_key_file) {
> + cfg_device = g_key_file_get_string(igt_key_file,
> UNIGRAF_CONFIG_GROUP,
> +
> UNIGRAF_CONFIG_DEVICE_NAME, &cfg_error);
> + if (cfg_error) {
> + unigraf_debug("No device name configured, uses first
> device available.\n");
> + cfg_device = NULL;
> + }
> + }
> +
> unigraf_assert(TSIX_DEV_RescanDevices(0, TSI_DEVCAP_VIDEO_CAPTURE,
> 0));
> device_count = unigraf_device_count();
> @@ -96,6 +139,16 @@ bool unigraf_open_device(int drm_fd)
> return false;
> }
>
> + if (!cfg_device) {
> + chosen_device = 0;
> + } else {

Why removing the chosen_device initialization at the top of the function and add
this if condition?
if (cfg_device) {...} is sufficient here.

> + chosen_device = unigraf_find_device(cfg_device);
> + if (chosen_device < 0) {
> + igt_warn("The requested unigraf device %s is not
> found, err: %d.\n", cfg_device, chosen_device);
> + return false;
> + }
> + }
> +
> unigraf_device = TSIX_DEV_OpenDevice(chosen_device, &r);
> unigraf_assert(r);
> igt_assert(unigraf_device);
>

Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering