| Message ID | 1371245697-29504-9-git-send-email-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jun 14, 2013 at 11:34:42PM +0200, Laurent Pinchart wrote: > The -w parameter can be used to set a property value from the command > line, using the target object ID and the property name. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > tests/modetest/modetest.c | 108 +++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 106 insertions(+), 2 deletions(-) > > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c > index 778af62..858d480 100644 > --- a/tests/modetest/modetest.c > +++ b/tests/modetest/modetest.c <snip> > @@ -1008,9 +1082,20 @@ static int parse_plane(struct plane_arg *p, const char *arg) > return 0; > } > > +static int parse_property(struct property_arg *p, const char *arg) > +{ > + if (sscanf(arg, "%d:%32[^:]:%" SCNu64, &p->obj_id, p->name, &p->value) != 3) nit: could use stringification to get rid of the magic number 32 here. I didn't spot any real problems in the series. But I must admit that I mainly just glanced at most of the changes in since many of the diffs are a bit hard to read. I also gave it a quick try using sprites and setting a few modes. And I found a bug in i915 while doing that, so clearly it has already proved useful ;) > + return -1; > + > + p->obj_type = 0; > + p->name[DRM_PROP_NAME_LEN] = '\0'; > + > + return 0; > +} > + > static void usage(char *name) > { > - fprintf(stderr, "usage: %s [-cdefMmPpsv]\n", name); > + fprintf(stderr, "usage: %s [-cdefMmPpsvw]\n", name); > > fprintf(stderr, "\n Query options:\n\n"); > fprintf(stderr, "\t-c\tlist connectors\n");
Hi Ville, Thank you for the review. On Monday 24 June 2013 18:08:37 Ville Syrjälä wrote: > On Fri, Jun 14, 2013 at 11:34:42PM +0200, Laurent Pinchart wrote: > > The -w parameter can be used to set a property value from the command > > line, using the target object ID and the property name. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > > > tests/modetest/modetest.c | 108 ++++++++++++++++++++++++++++++++++++++++- > > 1 file changed, 106 insertions(+), 2 deletions(-) > > > > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c > > index 778af62..858d480 100644 > > --- a/tests/modetest/modetest.c > > +++ b/tests/modetest/modetest.c > > <snip> > > > @@ -1008,9 +1082,20 @@ static int parse_plane(struct plane_arg *p, const > > char *arg)> > > return 0; > > > > } > > > > +static int parse_property(struct property_arg *p, const char *arg) > > +{ > > + if (sscanf(arg, "%d:%32[^:]:%" SCNu64, &p->obj_id, p->name, &p- >value) > > != 3) > nit: could use stringification to get rid of the magic number 32 here. What do you mean exactly ? > I didn't spot any real problems in the series. But I must admit that I > mainly just glanced at most of the changes in since many of the diffs > are a bit hard to read. > > I also gave it a quick try using sprites and setting a few modes. And I > found a bug in i915 while doing that, so clearly it has already proved > useful ;) That's nice to know :-) > > + return -1; > > + > > + p->obj_type = 0; > > + p->name[DRM_PROP_NAME_LEN] = '\0'; > > + > > + return 0; > > +} > > + > > > > static void usage(char *name) > > { > > > > - fprintf(stderr, "usage: %s [-cdefMmPpsv]\n", name); > > + fprintf(stderr, "usage: %s [-cdefMmPpsvw]\n", name); > > > > fprintf(stderr, "\n Query options:\n\n"); > > fprintf(stderr, "\t-c\tlist connectors\n");
On Thu, Jun 27, 2013 at 10:10:43AM +0200, Laurent Pinchart wrote: > Hi Ville, > > Thank you for the review. > > On Monday 24 June 2013 18:08:37 Ville Syrjälä wrote: > > On Fri, Jun 14, 2013 at 11:34:42PM +0200, Laurent Pinchart wrote: > > > The -w parameter can be used to set a property value from the command > > > line, using the target object ID and the property name. > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > --- > > > > > > tests/modetest/modetest.c | 108 ++++++++++++++++++++++++++++++++++++++++- > > > 1 file changed, 106 insertions(+), 2 deletions(-) > > > > > > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c > > > index 778af62..858d480 100644 > > > --- a/tests/modetest/modetest.c > > > +++ b/tests/modetest/modetest.c > > > > <snip> > > > > > @@ -1008,9 +1082,20 @@ static int parse_plane(struct plane_arg *p, const > > > char *arg)> > > > return 0; > > > > > > } > > > > > > +static int parse_property(struct property_arg *p, const char *arg) > > > +{ > > > + if (sscanf(arg, "%d:%32[^:]:%" SCNu64, &p->obj_id, p->name, &p- > >value) > > > != 3) > > nit: could use stringification to get rid of the magic number 32 here. > > What do you mean exactly ? Something like this: #define str(x) #x #define xstr(x) str(x) sscanf(arg, "%d:%" xstr(DRM_PROP_NAME_LEN) "[^:]:%" SCNu64, ... Although it does make it a bit hard to parse for a human.
On Thursday 27 June 2013 11:31:48 Ville Syrjälä wrote: > On Thu, Jun 27, 2013 at 10:10:43AM +0200, Laurent Pinchart wrote: > > On Monday 24 June 2013 18:08:37 Ville Syrjälä wrote: > > > On Fri, Jun 14, 2013 at 11:34:42PM +0200, Laurent Pinchart wrote: > > > > The -w parameter can be used to set a property value from the command > > > > line, using the target object ID and the property name. > > > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > --- > > > > > > > > tests/modetest/modetest.c | 108 ++++++++++++++++++++++++++++++++++++- > > > > 1 file changed, 106 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c > > > > index 778af62..858d480 100644 > > > > --- a/tests/modetest/modetest.c > > > > +++ b/tests/modetest/modetest.c > > > > > > <snip> > > > > > > > @@ -1008,9 +1082,20 @@ static int parse_plane(struct plane_arg *p, > > > > const char *arg) > > > > return 0; > > > > } > > > > > > > > +static int parse_property(struct property_arg *p, const char *arg) > > > > +{ > > > > + if (sscanf(arg, "%d:%32[^:]:%" SCNu64, &p->obj_id, p->name, &p- > > > > > >value) > > > > > > > != 3) > > > > > > nit: could use stringification to get rid of the magic number 32 here. > > > > What do you mean exactly ? > > Something like this: > > #define str(x) #x > #define xstr(x) str(x) > sscanf(arg, "%d:%" xstr(DRM_PROP_NAME_LEN) "[^:]:%" SCNu64, ... > > Although it does make it a bit hard to parse for a human. Right. I'm fine with both. "%m[^:]" might be an interesting alternative option.
diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 778af62..858d480 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -709,6 +709,80 @@ connector_find_mode(struct connector_arg *c) } +/* ----------------------------------------------------------------------------- + * Properties + */ + +struct property_arg { + uint32_t obj_id; + uint32_t obj_type; + char name[DRM_PROP_NAME_LEN+1]; + uint32_t prop_id; + uint64_t value; +}; + +static void set_property(struct property_arg *p) +{ + drmModeObjectProperties *props; + drmModePropertyRes **props_info; + const char *obj_type; + int ret; + int i; + + p->obj_type = 0; + p->prop_id = 0; + +#define find_object(_res, __res, type, Type) \ + do { \ + for (i = 0; i < (int)(_res)->__res->count_##type##s; ++i) { \ + struct type *obj = &(_res)->type##s[i]; \ + if (obj->type->type##_id != p->obj_id) \ + continue; \ + p->obj_type = DRM_MODE_OBJECT_##Type; \ + obj_type = #Type; \ + props = obj->props; \ + props_info = obj->props_info; \ + } \ + } while(0) \ + + find_object(resources, res, crtc, CRTC); + if (p->obj_type == 0) + find_object(resources, res, connector, CONNECTOR); + if (p->obj_type == 0) + find_object(resources, plane_res, plane, PLANE); + if (p->obj_type == 0) { + fprintf(stderr, "Object %i not found, can't set property\n", + p->obj_id); + return; + } + + if (!props) { + fprintf(stderr, "%s %i has no properties\n", + obj_type, p->obj_id); + return; + } + + for (i = 0; i < (int)props->count_props; ++i) { + if (!props_info[i]) + continue; + if (strcmp(props_info[i]->name, p->name) == 0) + break; + } + + if (i == (int)props->count_props) { + fprintf(stderr, "%s %i has no %s property\n", + obj_type, p->obj_id, p->name); + return; + } + + p->prop_id = props->props[i]; + + ret = drmModeObjectSetProperty(fd, p->obj_id, p->obj_type, p->prop_id, p->value); + if (ret < 0) + fprintf(stderr, "failed to set %s %i property %s to %" PRIu64 ": %s\n", + obj_type, p->obj_id, p->name, p->value, strerror(errno)); +} + /* -------------------------------------------------------------------------- */ static void @@ -1008,9 +1082,20 @@ static int parse_plane(struct plane_arg *p, const char *arg) return 0; } +static int parse_property(struct property_arg *p, const char *arg) +{ + if (sscanf(arg, "%d:%32[^:]:%" SCNu64, &p->obj_id, p->name, &p->value) != 3) + return -1; + + p->obj_type = 0; + p->name[DRM_PROP_NAME_LEN] = '\0'; + + return 0; +} + static void usage(char *name) { - fprintf(stderr, "usage: %s [-cdefMmPpsv]\n", name); + fprintf(stderr, "usage: %s [-cdefMmPpsvw]\n", name); fprintf(stderr, "\n Query options:\n\n"); fprintf(stderr, "\t-c\tlist connectors\n"); @@ -1023,6 +1108,7 @@ static void usage(char *name) fprintf(stderr, "\t-P <connector_id>:<w>x<h>[@<format>]\tset a plane\n"); fprintf(stderr, "\t-s <connector_id>[@<crtc_id>]:<mode>[@<format>]\tset a mode\n"); fprintf(stderr, "\t-v\ttest vsynced page flipping\n"); + fprintf(stderr, "\t-w <obj_id>:<prop_name>:<value>\tset property\n"); fprintf(stderr, "\n Generic options:\n\n"); fprintf(stderr, "\t-d\tdrop master after mode set\n"); @@ -1055,7 +1141,7 @@ static int page_flipping_supported(void) #endif } -static char optstr[] = "cdefM:mP:ps:v"; +static char optstr[] = "cdefM:mP:ps:vw:"; int main(int argc, char **argv) { @@ -1067,8 +1153,10 @@ int main(int argc, char **argv) char *module = NULL; unsigned int i; int count = 0, plane_count = 0; + unsigned int prop_count = 0; struct connector_arg *con_args = NULL; struct plane_arg *plane_args = NULL; + struct property_arg *prop_args = NULL; unsigned int args = 0; opterr = 0; @@ -1129,6 +1217,19 @@ int main(int argc, char **argv) case 'v': test_vsync = 1; break; + case 'w': + prop_args = realloc(prop_args, + (prop_count + 1) * sizeof *prop_args); + if (prop_args == NULL) { + fprintf(stderr, "memory allocation failed\n"); + return 1; + } + + if (parse_property(&prop_args[prop_count], optarg) < 0) + usage(argv[0]); + + prop_count++; + break; default: usage(argv[0]); break; @@ -1179,6 +1280,9 @@ int main(int argc, char **argv) dump_resource(planes); dump_resource(framebuffers); + for (i = 0; i < prop_count; ++i) + set_property(&prop_args[i]); + if (count > 0) { set_mode(con_args, count, plane_args, plane_count, test_vsync); if (drop_master)
The -w parameter can be used to set a property value from the command line, using the target object ID and the property name. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- tests/modetest/modetest.c | 108 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 106 insertions(+), 2 deletions(-)