Re: [Intel-gfx] [PATCH 05/17] drm: Add atomic driver interface definitions for objects

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

On Wed, Nov 05, 2014 at 06:04:59PM +0100, Daniel Vetter wrote:
> On Wed, Nov 5, 2014 at 5:26 PM, Thierry Reding <thierry.reding@gmail.com> wrote:
> >> +struct drm_plane_state {
> >> +     struct drm_crtc *crtc;
> >> +     struct drm_framebuffer *fb;
> >> +
> >> +     /* Signed dest location allows it to be partially off screen */
> >> +     int32_t crtc_x, crtc_y;
> >> +     uint32_t crtc_w, crtc_h;
> >
> > Should these perhaps be unsized types? For the same reasons you argued
> > the other day.
> >
> >> +
> >> +     /* Source values are 16.16 fixed point */
> >> +     uint32_t src_x, src_y;
> >> +     uint32_t src_h, src_w;
> >
> > Do we really use the 16.16 fixed point format for these? Maybe now would
> > be a good time to get rid of that if we don't need it. If they're not a
> > 16.16 fixed point format they could also be unsized.
> 
> The samantics and types intentionally and precisely match what we
> currently pass in through the set_plane ioctl. And yeah most drivers
> can't do subpixel precise upscaling, but some hardware can do that. So
> I don't see a point in changing the interface here.
> 
> I've implemented all the other stuff you've spotted.

Just a pass by comment, it'd be awesome to have a fixed point 16.16 type
at some point so we don't have to always specify that an uint32_t
variable happens to be 16.16.

-- 
Damien
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel