Re: [PATCH 01/11] drm: add plane support

[SW Kim <devusr.opensw@gmail.com>]

Daniel Vetter <daniel <at> ffwll.ch> writes:

[snip]

> > struct drm_mode_fb_cmd2 {
> >   __u32 fb_id;
> >   __u32 width, height;
> >   __u32 pixel_format; /* fourcc code from videodev2.h */
> >   __u64 priv;  /* private data, depends on pixelformat */
> > 

[snip]

> >   struct {
> >     __u32 pitch;
> >     /* driver specific handle */
> >     __u32 handle;
> 
> Why not just add a
> 	__u32 offset;
> 	__u32 rsvd;

For normal NV12 format, I agree that just offset is enough.
But considering more specific format like NV12M, that has two separated memory 
spaces, IMHO, it needs handle per each plane.

Each plane of NV12M has it own base address and space between these two memory 
spaces can be used by others.

You can refer following link about NV12M used by V4L2.
http://linuxtv.org/downloads/v4l-dvb-apis/V4L2-PIX-FMT-NV12M.html

> and call it a day. This thing is pretty big already, so that bloat doesn't
> matter that much. Maybe spec that buffer[0].offset must be zero. With that
> you can also do I420 with the following layout
> +-------+
> |YYYYYYY|
> |YYYYYYY|
> |YYYYYYY|
> |YYYYYYY|
> +---+---+
> |UUU|VVV|
> |UUU|VVV|
> +---+---+
> 
> i.e. stride_U == stride_V, with their lines meshed into one line.
> -Daniel
> 
> >   } buffer[16];

and just question, is there any meaning about buffer array size 16?

> >  };

[snip]

Thanks and Regards,
- SW Kim