Hi Thierry,
On Monday 26 January 2015 11:14:32 Thierry Reding wrote:
> On Sun, Jan 25, 2015 at 01:56:51AM +0200, Laurent Pinchart wrote:
> > On Friday 23 January 2015 17:08:21 Thierry Reding wrote:
> >> From: Thierry Reding <treding@xxxxxxxxxx>
> >>
> >> 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@xxxxxxxxxx>
> >> ---
> >>
> >> 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)" ?
>
> The ID is already printed in the very first column.
My bad.
> >> @@ -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 ?
>
> The idea was to mirror what X was doing so that people familiar with the
> xrandr tool would feel right at home. Note that the index here is by
> type, not global. So you'd end up with something like this:
>
> HDMI-A-1
> HDMI-A-2
> LVDS-1
>
> I think that's what most people would find to be the least surprising.
> Using the connector_type_id would again introduce the potential to get
> no-deterministic names (dependent on driver probe ordering in case of
> multiple cards).
Isn't the index dependent on probe ordering as well ?
> That said I now realize that this actually starts numbering connectors
> at 0, so get_connector_index() should probably return index rather than
> index - 1 to be consistent with what X does.
Yes, I think that's a good idea.
> >> + 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.
>
> Always using 9 characters would be wasting 8 bytes per connector for
> something like 99% of the systems.
We're talking about 8 bytes per connector for a test tool. With no more than a
dozen of connectors. Right ? :-)
> I had thought about using asprintf but decided not to use it because it
> isn't always available and it is pretty simple to compute the actual length.
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
