On Thu, Apr 5, 2012 at 1:13 PM, Ville Syrjälä
<ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> On Fri, Mar 30, 2012 at 01:12:58PM +0300, Ville Syrjälä wrote:
>> On Fri, Mar 30, 2012 at 11:54:50AM +0900, Seung-Woo Kim wrote:
>> > Multi buffer plane pixel formats are added as like kernel header.
>> >
>> > Signed-off-by: Seung-Woo Kim <sw0312.kim@xxxxxxxxxxx>
>> > ---
>> > include/drm/drm_fourcc.h | 7 +++++++
>> > 1 files changed, 7 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
>> > index 85facb0..7cfd95a 100644
>> > --- a/include/drm/drm_fourcc.h
>> > +++ b/include/drm/drm_fourcc.h
>> > @@ -107,6 +107,10 @@
>> > #define DRM_FORMAT_NV16 fourcc_code('N', 'V', '1', '6') /* 2x1 subsampled Cr:Cb plane */
>> > #define DRM_FORMAT_NV61 fourcc_code('N', 'V', '6', '1') /* 2x1 subsampled Cb:Cr plane */
>> >
>> > +/* 2 non contiguous plane YCbCr */
>> > +#define DRM_FORMAT_NV12M fourcc_code('N', 'M', '1', '2') /* 2x2 subsampled Cr:Cb plane */
>>
>> NAK. DRM_FORMAT_NV12 handles this just fine.
>
> And I just realized that I was already too late with my NAK since this a
> libdrm patch. Apparently the kernel drm_fourcc.h changes were snuck in
> via some backdoor without review. Sigh.
>
> So they're now in Linus's tree. But looks like format_check() was never
> updated to accept them, so there's no way anyone could actually be using
> them. So Dave, can we still remove them from the kernel header?
>
I would tend to agree.. the existing addfb2 API allows for
multi-planar, so I don't really see the need for special fourcc values
for this.
If it was really the case that the hw *only* supported multiplanar
(which seems a bit odd), then I'd recommend using the non-valid fourcc
space (ie. one or more of the bytes should be non-7bit-ascii, like
0x80000000 | NV12), and keep that in a driver specific header. The
same approach could be used to handle non-standard formats like fb
compression.
BR,
-R
>
> Just to clarify once mode. The original planar formats I defined in
> drm_fourcc.h handle non-contiguous planes. The drivers are supposed to
> use handles[],offsets[],pitches[] to handle this. The 'index' comments
> in the drm_fourcc.h tells you which index of those arrays matches which
> plane. This means that DRM_FORMAT_NV12M is functionally _identical_ to
> DRM_FORMAT_NV12, and the same holds for the three plane format that got
> submarined in.
>
> The driver is not even supposed to accept DRM_FORMAT_NV12 unless both
> index 0 and 1 are filled in properly by userspace. If the planes are
> contiguous then userspace _must_ pass the same handle for index 0 and
> 1, and use offsets[] to tell the driver where each plane is. If the
> hardware can't handle non-contiguous planes (never seen such hardware
> myself) then the driver must refuse the operation if userspace passes
> such a buffer to it.
>
> I already posted a patch with a drm_framebuffer_check() function that
> does basic sanity checking on this stuff. I'll pull some checksa from
> that function and add them directly to drm_mode_addfb2(). Some of the
> checks require the driver to pass the BO sizes, so those I can't add.
> Also the plane overlap checks I had shouldn't be in generic code
> because some hardware can handle line-by-line interleaved planes, and
> my code would refuse those. Ideally the code should be improved to
> allow such interleaved planes, and then the check could be added to the
> generic code.
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
