Hi Thierry,
Thank you for the patch.
On Friday 23 January 2015 17:08:21 Thierry Reding wrote:
From: Thierry Reding treding@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@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 ?