[PATCH 01/11] drm: add plane support

[Rob Clark]

On Tue, Oct 25, 2011 at 2:41 PM, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Tue, Oct 25, 2011 at 11:43:09AM -0500, Rob Clark wrote:
>> On Tue, Oct 25, 2011 at 9:09 AM, Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
>> > diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
>> > index 34a0d22..dafe8df 100644
>> > --- a/include/drm/drm_mode.h
>> > +++ b/include/drm/drm_mode.h
>> > @@ -272,8 +272,9 @@ struct drm_mode_fb_cmd2 {
>> >        __u32 bpp;
>> >        __u32 depth;
>> >        __u32 pixel_format; /* fourcc code from videodev2.h */
>> > -       /* driver specific handle */
>> > -       __u32 handle;
>> > +       __u32 handle_count;
>> > +       /* driver specific buffer object handle array */
>> > +       __u64 handles;
>> >  };
>>
>>
>> Ok, after a bit of discussion with Jakob on IRC, this is what we arrived at:
>>
>> 1) It would be nice to have the option for multi-planar formats to be
>> able to use one single buffer object, or one buffer object per plane.
>> In the case of one buffer per plane, the order is dictated by the
>> fourcc.
>>
>> 2) We do need per-plane stride for some formats, in particular I420
>> which is a bit ambiguous otherwise (ie. is the stride of the U and V
>> planes half the stride as the Y plane, or the same?)
>>
>> 3) Maybe we need per-plane offsets, but I think this case could be
>> handled by just using one bo per plane, so left it out
>>
>> 4) bpp/depth are redundant w/ the pixel_format. Just in case, as a
>> future placeholder, and inspired by 'struct v4l2_pix_format', add a
>> priv field.  Although making the field big enough to hold a pointer if
>> absolutely really needed.
>>
>> 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 */
>>
>>   /* in case of planar formats, either one buffer object,
>>    * or one buffer object per plane, is allowed.  In the
>>    * case per-plane bo's, the order is dictated by the
>>    * fourcc.. ie. NV12 (http://fourcc.org/yuv.php#NV12)
>>    * is described as:
>>    *
>>    *   YUV 4:2:0 image with a plane of 8 bit Y samples
>>    *   followed by an interleaved U/V plane containing
>>    *   8 bit 2x2 subsampled colour difference samples.
>>    *
>>    * So it would consist of Y as first buffer and UV as
>>    * second buffer.  In the case that only a single bo
>>    * is used then buffer[1].handle should be zero.  (But
>>    * a plane specific pitch could still be specified.)
>>    */
>>   struct {
>>     __u32 pitch;
>>     /* driver specific handle */
>>     __u32 handle;
>
> Why not just add a
>        __u32 offset;
>        __u32 rsvd;
> 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
>

I've no objection to an offset field.. I was mostly just trying to
avoid that we keep adding things until we get XvImageFormatValues..

We should either add an offset plus pad field as Daniel suggested, or
at least add a u64 reserved field which could be made into made into
an offset field later if needed.

OTOH, is the format you described *really* still I420?

BR,
-R

>>   } buffer[16];
>>  };
>>
>>
>> BR,
>> -R
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Mail: daniel at ffwll.ch
> Mobile: +41 (0)79 365 57 48
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>