Re: [PATCH i-g-t 05/43] igt_kms: Add support for setting plane rotation

[Damien Lespiau <damien.lespiau@intel.com>]

On Fri, Jul 11, 2014 at 08:37:42AM +0200, Daniel Vetter wrote:
> On Thu, Jul 10, 2014 at 07:00:06PM +0100, Damien Lespiau wrote:
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > ---
> >  lib/igt_kms.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  lib/igt_kms.h | 11 +++++++++++
> >  2 files changed, 65 insertions(+)
> > 
> > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > index 87f5109..69f9977 100644
> > --- a/lib/igt_kms.c
> > +++ b/lib/igt_kms.c
> > @@ -537,6 +537,16 @@ get_plane_property(igt_display_t *display, uint32_t plane_id, const char *name,
> >  			    name, prop_id, value);
> >  }
> >  
> > +static void
> > +igt_plane_set_property(igt_plane_t *plane, uint32_t prop_id, uint64_t value)
> > +{
> > +	igt_pipe_t *pipe = plane->pipe;
> > +	igt_display_t *display = pipe->display;
> > +
> > +	drmModeObjectSetProperty(display->drm_fd, plane->drm_plane->plane_id,
> > +				 DRM_MODE_OBJECT_PLANE, prop_id, value);
> > +}
> 
> I wonder whether we shouldn't have interfaces using the property names and
> maybe for enum properties also the strings and let igt do it's thing. But
> as long as this is just an internal function it should be good enough.

That'd be handy. the only issue is that you'd have to do a lookup of the
property id each time or create a prop_name -> prop_id map in the
different objects. Also, we can provide a bit more typing than a
uint64_t for the value. Trade-offs, trade-offs, ...

-- 
Damien