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