From mboxrd@z Thu Jan  1 00:00:00 1970
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Subject: Re: [PATCH v6 08/23] modetest: Add a command line parameter to set
	properties
Date: Thu, 27 Jun 2013 10:10:43 +0200
Message-ID: <2998730.VBVDGyOTOW@avalon>
References: <1371245697-29504-1-git-send-email-laurent.pinchart@ideasonboard.com>
	<1371245697-29504-9-git-send-email-laurent.pinchart@ideasonboard.com>
	<20130624150837.GM5004@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Return-path: <dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org>
Received: from perceval.ideasonboard.com (perceval.ideasonboard.com
	[95.142.166.194])
	by gabe.freedesktop.org (Postfix) with ESMTP id 6CD04E5CC6
	for <dri-devel@lists.freedesktop.org>;
	Thu, 27 Jun 2013 01:10:26 -0700 (PDT)
In-Reply-To: <20130624150837.GM5004@intel.com>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/dri-devel>,
	<mailto:dri-devel-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/dri-devel>
List-Post: <mailto:dri-devel@lists.freedesktop.org>
List-Help: <mailto:dri-devel-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/dri-devel>,
	<mailto:dri-devel-request@lists.freedesktop.org?subject=subscribe>
Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org
Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org
To: Ville =?ISO-8859-1?Q?Syrj=E4l=E4?= <ville.syrjala@linux.intel.com>
Cc: dri-devel@lists.freedesktop.org
List-Id: dri-devel@lists.freedesktop.org

Hi Ville,

Thank you for the review.

On Monday 24 June 2013 18:08:37 Ville Syrj=E4l=E4 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)
> > !=3D 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 =3D 0;
> > +	p->name[DRM_PROP_NAME_LEN] =3D '\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");
-- =

Regards,

Laurent Pinchart