[PATCH libdrm] libdrm: update drm/drm_fourcc.h from kernel to add multi plane formats

[PATCH libdrm] libdrm: update drm/drm_fourcc.h from kernel to add multi plane formats

[Seung-Woo Kim]

Multi buffer plane pixel formats are added as like kernel header.

Signed-off-by: Seung-Woo Kim <sw0312.kim@samsung.com>
---
 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 */
+#define DRM_FORMAT_NV12MT	fourcc_code('T', 'M', '1', '2') /* 2x2 subsampled Cr:Cb plane 64x32 macroblocks */
+
 /*
  * 3 plane YCbCr
  * index 0: Y plane, [7:0] Y
@@ -127,4 +131,7 @@
 #define DRM_FORMAT_YUV444	fourcc_code('Y', 'U', '2', '4') /* non-subsampled Cb (1) and Cr (2) planes */
 #define DRM_FORMAT_YVU444	fourcc_code('Y', 'V', '2', '4') /* non-subsampled Cr (1) and Cb (2) planes */
 
+/* 3 non contiguous plane YCbCr */
+#define DRM_FORMAT_YUV420M	fourcc_code('Y', 'M', '1', '2') /* 2x2 subsampled Cb (1) and Cr (2) planes */
+
 #endif /* DRM_FOURCC_H */
-- 
1.7.4.1

Re: [PATCH libdrm] libdrm: update drm/drm_fourcc.h from kernel to add multi plane formats

[Ville Syrjälä]

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@samsung.com>
> ---
>  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.

> +#define DRM_FORMAT_NV12MT	fourcc_code('T', 'M', '1', '2') /* 2x2 subsampled Cr:Cb plane 64x32 macroblocks */

This one is more difficult. Until now tiling was always handled in
driver specific manner. OTOH if this format is really supported by
different devices from multiple vendors, then it would probably
make sense to add it as a standard format.

> +
>  /*
>   * 3 plane YCbCr
>   * index 0: Y plane, [7:0] Y
> @@ -127,4 +131,7 @@
>  #define DRM_FORMAT_YUV444	fourcc_code('Y', 'U', '2', '4') /* non-subsampled Cb (1) and Cr (2) planes */
>  #define DRM_FORMAT_YVU444	fourcc_code('Y', 'V', '2', '4') /* non-subsampled Cr (1) and Cb (2) planes */
>  
> +/* 3 non contiguous plane YCbCr */
> +#define DRM_FORMAT_YUV420M	fourcc_code('Y', 'M', '1', '2') /* 2x2 subsampled Cb (1) and Cr (2) planes */a

NAK. DRM_FORMAT_YUV420 handles this.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH libdrm] libdrm: update drm/drm_fourcc.h from kernel to add multi plane formats

[김승우]

Hi Ville,

I skipped explanation about NV12M and other two formats because these
formats are already in kernel drm_fourcc.h.

I think it is better to add a difference between NV12 and NV12M here.

NV12M has Y plane and CbCr plane and these are in non contiguous memory
region. Compared with NV12, NV12M's memory shape is like following.
NV12  : ______(Y)(CbCr)_______
NV12M : __(Y)_ ..... _(CbCr)__

Y and CbCr plane of NV12 can be expressed with one memory address and
offset of each plane. but NV12M needs memory address of each plane.

On 2012년 03월 30일 19:12, 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@samsung.com>
>> ---
>>   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.
>

Exynos soc supports two kinds of memory shape explained above, so two
different types are need for exynos soc.

>> +#define DRM_FORMAT_NV12MT	fourcc_code('T', 'M', '1', '2') /* 2x2 subsampled Cr:Cb plane 64x32 macroblocks */
> This one is more difficult. Until now tiling was always handled in
> driver specific manner. OTOH if this format is really supported by
> different devices from multiple vendors, then it would probably
> make sense to add it as a standard format.
>
Exynos soc also supports normal and tiled pixel data and pixel data is 
shared
between hw blocks for example from scaler to hdmi.
So driver can not handle it internally.

IMHO, support for various shape can be helpful even though only exynos soc
family suppors these formats currently.

>> +
>>   /*
>>    * 3 plane YCbCr
>>    * index 0: Y plane, [7:0] Y
>> @@ -127,4 +131,7 @@
>>   #define DRM_FORMAT_YUV444	fourcc_code('Y', 'U', '2', '4') /* non-subsampled Cb (1) and Cr (2) planes */
>>   #define DRM_FORMAT_YVU444	fourcc_code('Y', 'V', '2', '4') /* non-subsampled Cr (1) and Cb (2) planes */
>>
>> +/* 3 non contiguous plane YCbCr */
>> +#define DRM_FORMAT_YUV420M	fourcc_code('Y', 'M', '1', '2') /* 2x2 subsampled Cb (1) and Cr (2) planes */a
> NAK. DRM_FORMAT_YUV420 handles this.

This is same with case of NV12M.

Regards.

-- 
Seung-Woo Kim
Samsung Software R&D Center
--


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

Re: [PATCH libdrm] libdrm: update drm/drm_fourcc.h from kernel to add multi plane formats

[Ville Syrjälä]

On Fri, Mar 30, 2012 at 08:07:16PM +0900, 김승우 wrote:
> Hi Ville,
> 
> I skipped explanation about NV12M and other two formats because these
> formats are already in kernel drm_fourcc.h.
> 
> I think it is better to add a difference between NV12 and NV12M here.
> 
> NV12M has Y plane and CbCr plane and these are in non contiguous memory
> region. Compared with NV12, NV12M's memory shape is like following.
> NV12  : ______(Y)(CbCr)_______
> NV12M : __(Y)_ ..... _(CbCr)__
> 
> Y and CbCr plane of NV12 can be expressed with one memory address and
> offset of each plane. but NV12M needs memory address of each plane.

Yes, I know the difference between NV12 and NV12M. But that doesn't
change the fact that you can already represent NV12M with
DRM_FORMAT_NV12 (hint handles[], offsets[]). Maybe I should have named
it DRM_FORMAT_YUV420_2P or something like that. Anyone have a time
machine I could borrow?

> On 2012년 03월 30일 19:12, 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@samsung.com>
> >> ---
> >>   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.
> >
> 
> Exynos soc supports two kinds of memory shape explained above, so two
> different types are need for exynos soc.
> 
> >> +#define DRM_FORMAT_NV12MT	fourcc_code('T', 'M', '1', '2') /* 2x2 subsampled Cr:Cb plane 64x32 macroblocks */
> > This one is more difficult. Until now tiling was always handled in
> > driver specific manner. OTOH if this format is really supported by
> > different devices from multiple vendors, then it would probably
> > make sense to add it as a standard format.
> >
> Exynos soc also supports normal and tiled pixel data and pixel data is 
> shared
> between hw blocks for example from scaler to hdmi.
> So driver can not handle it internally.

If it's all in one driver, it surely can. And even with multiple drivers
you could still pass that information via some kernel internal mechanism.

I'm not opposed to supporting such layouts, but I'm just worried that
tomorrow someone comes up a new device that supports 100 new tiling
layouts for some format, and then we need to add all of them as separate
DRM_FORMATs. And there is already plenty of hardware out there that can
do different tiled layouts to yours, in fact I have some on my desk even
now.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm] libdrm: update drm/drm_fourcc.h from kernel to add multi plane formats

[Marcus Lorentzon]

On 03/30/2012 12:12 PM, Ville Syrjälä wrote:
>> +#define DRM_FORMAT_NV12MT	fourcc_code('T', 'M', '1', '2') /* 2x2 subsampled Cr:Cb plane 64x32 macroblocks */
> This one is more difficult. Until now tiling was always handled in
> driver specific manner. OTOH if this format is really supported by
> different devices from multiple vendors, then it would probably
> make sense to add it as a standard format.
>
What about adding a DRM_FORMAT_PRIV_NV12MT and force vendor to add 
detailed documentation about this new format. Because if the format is 
not defined and documented, how will we ever discover if vendors have 
similar formats? And if we document all these MB formats "everyone" 
uses, then maybe we can get HW vendors to unify on a de facto standard. 
It would be nice to have an efficient format supported by everyone, 
instead of having to fallback to old linear formats every time you 
interact with another device.

/BR
/Marcus

Re: [PATCH libdrm] libdrm: update drm/drm_fourcc.h from kernel to add multi plane formats

[Sylwester Nawrocki]

On 03/30/2012 01:09 PM, Marcus Lorentzon wrote:
> On 03/30/2012 12:12 PM, Ville Syrjälä wrote:
>>> +#define DRM_FORMAT_NV12MT fourcc_code('T', 'M', '1', '2') /* 2x2
>>> subsampled Cr:Cb plane 64x32 macroblocks */
>> This one is more difficult. Until now tiling was always handled in
>> driver specific manner. OTOH if this format is really supported by
>> different devices from multiple vendors, then it would probably
>> make sense to add it as a standard format.
>>
> What about adding a DRM_FORMAT_PRIV_NV12MT and force vendor to add
> detailed documentation about this new format. Because if the format is
> not defined and documented, how will we ever discover if vendors have
> similar formats? And if we document all these MB formats "everyone"
> uses, then maybe we can get HW vendors to unify on a de facto standard.
> It would be nice to have an efficient format supported by everyone,
> instead of having to fallback to old linear formats every time you
> interact with another device.

Just for the record, documentation of this tiled macro-block format
can be found here:

http://linuxtv.org/downloads/v4l-dvb-apis/re27.html

--
Regards,
Sylwester

Re: [PATCH libdrm] libdrm: update drm/drm_fourcc.h from kernel to add multi plane formats

[Ville Syrjälä]

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@samsung.com>
> > ---
> >  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?


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

Re: [PATCH libdrm] libdrm: update drm/drm_fourcc.h from kernel to add multi plane formats

[Rob Clark]

On Thu, Apr 5, 2012 at 1:13 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> 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@samsung.com>
>> > ---
>> >  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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [PATCH libdrm] libdrm: update drm/drm_fourcc.h from kernel to add multi plane formats

[Inki Dae]

Hi Ville,

> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> Sent: Friday, April 06, 2012 3:14 AM
> To: airlied@redhat.com
> Cc: inki.dae@samsung.com; kyungmin.park@samsung.com; dri-
> devel@lists.freedesktop.org; Seung-Woo Kim
> Subject: Re: [PATCH libdrm] libdrm: update drm/drm_fourcc.h from kernel to
> add multi plane formats
> 
> 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@samsung.com>
> > > ---
> > >  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.
> 

We had already requested review for it. for this you can refer to link
below:
	
http://lists.freedesktop.org/archives/dri-devel/2011-December/017654.html

> 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?
> 

Yes, right. these formats aren't used for any SoCs except Exynos series yet
but just we are first. I think they should be added because anyone may use
them someday at least possible. but they couldn’t be standard format so we
can send the patch that these formats are removed from drm_fourcc.h anytime
if you are still opposite to my opinion. but please know that at least in
case of v4l2, they have already been used as standard format. For this you
can refer to include/linux/videodev2.h file in mainline and Dave, please
give me your opinion.


Thanks,
Inki Dae

>
> 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

Re: [PATCH libdrm] libdrm: update drm/drm_fourcc.h from kernel to add multi plane formats

[Ville Syrjälä]

On Fri, Apr 06, 2012 at 03:05:36PM +0900, Inki Dae wrote:
> Hi Ville,
> 
> > -----Original Message-----
> > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> > Sent: Friday, April 06, 2012 3:14 AM
> > To: airlied@redhat.com
> > Cc: inki.dae@samsung.com; kyungmin.park@samsung.com; dri-
> > devel@lists.freedesktop.org; Seung-Woo Kim
> > Subject: Re: [PATCH libdrm] libdrm: update drm/drm_fourcc.h from kernel to
> > add multi plane formats
> > 
> > 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@samsung.com>
> > > > ---
> > > >  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.
> > 
> 
> We had already requested review for it. for this you can refer to link
> below:
> 	
> http://lists.freedesktop.org/archives/dri-devel/2011-December/017654.html

I see. I couldn't find it in my work mailbox for some reason, and I
don't remember having seen the patch before. I suppose I just missed it
due to Christmas vacations, and was too blind to see it in my mailbox.
Also google decicded to filter my search results too much, so I didn't
spot it via the web archives either. I'm sorry for the false accusation.

> > 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?
> > 
> 
> Yes, right. these formats aren't used for any SoCs except Exynos series yet
> but just we are first. I think they should be added because anyone may use
> them someday at least possible.

Since DRM_FORMAT_NV12M is _identical_ to DRM_FORMAT_NV12, I see no point
in adding it (similarly for YUV420M vs. YUV420).

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/

Re: [PATCH libdrm] libdrm: update drm/drm_fourcc.h from kernel to add multi plane formats

[Kyungmin Park]

[-- Attachment #1.1: Type: text/plain, Size: 3442 bytes --]

+ V4L2 develper
Sylwester, please give your opinions? How to handle it at v4l2 side.

Thank you,
Kyungmin Park
On Apr 6, 2012 12:44 AM, "Ville Syrjälä" <syrjala@sci.fi> wrote:

> On Fri, Apr 06, 2012 at 03:05:36PM +0900, Inki Dae wrote:
> > Hi Ville,
> >
> > > -----Original Message-----
> > > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> > > Sent: Friday, April 06, 2012 3:14 AM
> > > To: airlied@redhat.com
> > > Cc: inki.dae@samsung.com; kyungmin.park@samsung.com; dri-
> > > devel@lists.freedesktop.org; Seung-Woo Kim
> > > Subject: Re: [PATCH libdrm] libdrm: update drm/drm_fourcc.h from
> kernel to
> > > add multi plane formats
> > >
> > > 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@samsung.com>
> > > > > ---
> > > > >  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.
> > >
> >
> > We had already requested review for it. for this you can refer to link
> > below:
> >
> >
> http://lists.freedesktop.org/archives/dri-devel/2011-December/017654.html
>
> I see. I couldn't find it in my work mailbox for some reason, and I
> don't remember having seen the patch before. I suppose I just missed it
> due to Christmas vacations, and was too blind to see it in my mailbox.
> Also google decicded to filter my search results too much, so I didn't
> spot it via the web archives either. I'm sorry for the false accusation.
>
> > > 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?
> > >
> >
> > Yes, right. these formats aren't used for any SoCs except Exynos series
> yet
> > but just we are first. I think they should be added because anyone may
> use
> > them someday at least possible.
>
> Since DRM_FORMAT_NV12M is _identical_ to DRM_FORMAT_NV12, I see no point
> in adding it (similarly for YUV420M vs. YUV420).
>
> --
> Ville Syrjälä
> syrjala@sci.fi
> http://www.sci.fi/~syrjala/
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

[-- Attachment #1.2: Type: text/html, Size: 4943 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH libdrm] libdrm: update drm/drm_fourcc.h from kernel to add multi plane formats

[Sylwester Nawrocki]

Hello,

On 04/06/2012 11:22 AM, Kyungmin Park wrote:
> + V4L2 develper
> Sylwester, please give your opinions? How to handle it at v4l2 side.
> 
> Thank you,
> Kyungmin Park
> 
> On Apr 6, 2012 12:44 AM, "Ville Syrjälä" <syrjala@sci.fi
> <mailto:syrjala@sci.fi>> wrote:
> 
>     On Fri, Apr 06, 2012 at 03:05:36PM +0900, Inki Dae wrote:
>     > Hi Ville,
>     >
>     > > -----Original Message-----
>     > > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com
>     <mailto:ville.syrjala@linux.intel.com>]
>     > > Sent: Friday, April 06, 2012 3:14 AM
>     > > To: airlied@redhat.com <mailto:airlied@redhat.com>
>     > > Cc: inki.dae@samsung.com <mailto:inki.dae@samsung.com>;
>     kyungmin.park@samsung.com <mailto:kyungmin.park@samsung.com>; dri-
>     > > devel@lists.freedesktop.org <mailto:devel@lists.freedesktop.org>;
>     Seung-Woo Kim
>     > > Subject: Re: [PATCH libdrm] libdrm: update drm/drm_fourcc.h from kernel to
>     > > add multi plane formats
>     > >
>     > > 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@samsung.com
>     <mailto:sw0312.kim@samsung.com>>
>     > > > > ---
>     > > > >  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.
>     > >
>     >
>     > We had already requested review for it. for this you can refer to link
>     > below:
>     >
>     > http://lists.freedesktop.org/archives/dri-devel/2011-December/017654.html
> 
>     I see. I couldn't find it in my work mailbox for some reason, and I
>     don't remember having seen the patch before. I suppose I just missed it
>     due to Christmas vacations, and was too blind to see it in my mailbox.
>     Also google decicded to filter my search results too much, so I didn't
>     spot it via the web archives either. I'm sorry for the false accusation.
> 
>     > > 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?
>     > >
>     >
>     > Yes, right. these formats aren't used for any SoCs except Exynos series yet
>     > but just we are first. I think they should be added because anyone may use
>     > them someday at least possible.
> 
>     Since DRM_FORMAT_NV12M is _identical_ to DRM_FORMAT_NV12, I see no point
>     in adding it (similarly for YUV420M vs. YUV420).

In V4L2 the fourcc also determines the number of memory planes in the frame
buffer. The multi-planar API has been added to support devices with odd
alignment requirements (image components, like Y/CbCr in separate physical
memory buffers) for which it was difficult to map whole frame into contiguous
user memory region.

Here is some excample:
http://linuxtv.org/downloads/v4l-dvb-apis/V4L2-PIX-FMT-NV12M.html

I'm not terribly familiar with the buffer structure in DRM, maybe there are
better ways to handle something like this in DRI.

>     --
>     Ville Syrjälä
>     syrjala@sci.fi <mailto:syrjala@sci.fi>
>     http://www.sci.fi/~syrjala/

Regards,
-- 
Sylwester Nawrocki
Samsung Poland R&D Center

Re: [PATCH libdrm] libdrm: update drm/drm_fourcc.h from kernel to add multi plane formats

[daeinki]

나의 iPad에서 보냄

2012. 4. 6. 오후 4:43 Ville Syrjälä <syrjala@sci.fi> 작성:

> On Fri, Apr 06, 2012 at 03:05:36PM +0900, Inki Dae wrote:
>> Hi Ville,
>> 
>>> -----Original Message-----
>>> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>>> Sent: Friday, April 06, 2012 3:14 AM
>>> To: airlied@redhat.com
>>> Cc: inki.dae@samsung.com; kyungmin.park@samsung.com; dri-
>>> devel@lists.freedesktop.org; Seung-Woo Kim
>>> Subject: Re: [PATCH libdrm] libdrm: update drm/drm_fourcc.h from kernel to
>>> add multi plane formats
>>> 
>>> 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@samsung.com>
>>>>> ---
>>>>> 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.
>>> 
>> 
>> We had already requested review for it. for this you can refer to link
>> below:
>>    
>> http://lists.freedesktop.org/archives/dri-devel/2011-December/017654.html
> 
> I see. I couldn't find it in my work mailbox for some reason, and I
> don't remember having seen the patch before. I suppose I just missed it
> due to Christmas vacations, and was too blind to see it in my mailbox.
> Also google decicded to filter my search results too much, so I didn't
> spot it via the web archives either. I'm sorry for the false accusation.
> 
>>> 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?
>>> 
>> 
>> Yes, right. these formats aren't used for any SoCs except Exynos series yet
>> but just we are first. I think they should be added because anyone may use
>> them someday at least possible.
> 
> Since DRM_FORMAT_NV12M is _identical_ to DRM_FORMAT_NV12, I see no point
> in adding it (similarly for YUV420M vs. YUV420).
> 

but with this way,  user and device driver should understand NV12M format and how gem handles should send to driver and also how the driver should decide whether these handles mean NV12M or NV12. I think these things are unnecessary codes. with such theory, maybe current some fourcc formats couldn't be removed because we can identify desired format if we add some codes for it. as you mentioned, it's important to avoid such duplicated formats but also to consider user and device driver. so I think it's not good for user and driver should understand and decide them. Please give me your opinion.

thanks,
Inki Dae

> -- 
> Ville Syrjälä
> syrjala@sci.fi
> http://www.sci.fi/~syrjala/
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel