[PATCH libdrm 08/11] tests: modetest: Accept connector names
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sat Jan 24 15:56:51 PST 2015
Hi Thierry,
Thank you for the patch.
On Friday 23 January 2015 17:08:21 Thierry Reding wrote:
> From: Thierry Reding <treding at nvidia.com>
>
> Allow connector names to be used in the specification of the -s option.
> This requires storing the string passed on the command-line so that it
> can later be resolved to a connector ID (after the DRM device has been
> opened).
>
> Signed-off-by: Thierry Reding <treding at nvidia.com>
> ---
> tests/modetest/modetest.c | 134 +++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 123 insertions(+), 11 deletions(-)
>
> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> index d5fd99ebe1fd..a7cc94f8938c 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c
[snip]
> @@ -327,7 +328,7 @@ static void dump_connectors(struct device *dev)
> int i, j;
>
> printf("Connectors:\n");
> - printf("id\tencoder\tstatus\t\ttype\tsize (mm)\tmodes\tencoders\n");
> + printf("id\tencoder\tstatus\t\tname\t\tsize (mm)\tmodes\tencoders\n");
> for (i = 0; i < dev->resources->res->count_connectors; i++) {
> struct connector *_connector = &dev->resources->connectors[i];
> drmModeConnector *connector = _connector->connector;
> @@ -338,7 +339,7 @@ static void dump_connectors(struct device *dev)
> connector->connector_id,
> connector->encoder_id,
> util_lookup_connector_status_name(connector->connection),
> - util_lookup_connector_type_name(connector->connector_type),
> + _connector->name,
> connector->mmWidth, connector->mmHeight,
> connector->count_modes);
As this is a low-level test tool I believe it would be useful to print both
the name and the ID. Maybe something like "name (id)" ?
[snip]
> @@ -511,6 +516,47 @@ static void free_resources(struct resources *res)
> free(res);
> }
>
> +static unsigned int get_connector_index(struct resources *res, uint32_t
> type)
> +{
> + unsigned int index = 0;
> + int i;
> +
> + for (i = 0; i < res->res->count_connectors; i++)
> + if (res->connectors[i].connector->connector_type == type)
> + index++;
> +
> + return index - 1;
> +}
> +
> +static unsigned int get_order(unsigned int value)
> +{
> + unsigned int order = 0;
> +
> + do {
> + value /= 10;
> + order++;
> + } while (value > 0);
> +
> + return order - 1;
> +}
> +
> +static void connector_set_name(struct connector *connector,
> + struct resources *res)
> +{
> + uint32_t type = connector->connector->connector_type;
> + const char *type_name;
> + unsigned int index;
> + int len;
> +
> + type_name = util_lookup_connector_type_name(type);
> + index = get_connector_index(res, type);
The kernel's connector name is created using connector_type_id, not the
connector index. Shouldn't we do the same here ?
> + len = strlen(type_name) + get_order(index) + 2;
This looks like an over-optimization to me, can't you just add 9 to account
for the largest possible index ? Or, even better, use asprintf ? The function
is a GNU extension but is available on BSD according to its manpage.
> + connector->name = malloc(len + 1);
> + if (connector->name)
> + snprintf(connector->name, len + 1, "%s-%u", type_name, index);
> +}
> +
> static struct resources *get_resources(struct device *dev)
> {
> struct resources *res;
[snip]
> @@ -1405,6 +1483,32 @@ static int cursor_supported(void)
> return 1;
> }
>
> +static int pipe_resolve_connectors(struct pipe_arg *pipe, struct device
> *dev)
> +{
> + drmModeConnector *connector;
> + unsigned int i;
> + uint32_t id;
> + char *endp;
> +
> + for (i = 0; i < pipe->num_cons; i++) {
> + id = strtoul(pipe->cons[i], &endp, 10);
> + if (endp == pipe->cons[i]) {
This won't have the expected behaviour for 9-pin DIN connectors, as the name
starts with a digit. You should instead test for *endp == '\0'.
> + connector = get_connector_by_name(dev, pipe->cons[i]);
> + if (!connector) {
> + fprintf(stderr, "no connector named %s\n",
> + pipe->cons[i]);
> + return -ENODEV;
> + }
> +
> + id = connector->connector_id;
> + }
> +
> + pipe->con_ids[i] = id;
> + }
> +
> + return 0;
> +}
Could update the help text in usage() to reflect the new usage ?
--
Regards,
Laurent Pinchart
More information about the dri-devel
mailing list