On Thu, Feb 22, 2018 at 06:41:24PM -0800, Hyun Kwon wrote:
Hi Danidel,
Thanks for the comment.
On Mon, 2018-02-19 at 06:22:56 -0800, Daniel Vetter wrote:
On Fri, Feb 09, 2018 at 05:35:54PM -0800, Hyun Kwon wrote:
This patch adds new formats needed by Xilinx IP. Pixels are not byte-aligned in these formats, and the drm_format_info for these formats has macro-pixel information.
Signed-off-by: Jeffrey Mouroux jmouroux@xilinx.com Signed-off-by: Hyun Kwon hyun.kwon@xilinx.com
v3
- Update entries for changes
- Squash fourcc patch into this
v2
- Add detailed descriptions
- Remove formats with no user
drivers/gpu/drm/drm_fourcc.c | 2 ++ include/uapi/drm/drm_fourcc.h | 8 ++++++++ 2 files changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c index ed95de2..36bff7a 100644 --- a/drivers/gpu/drm/drm_fourcc.c +++ b/drivers/gpu/drm/drm_fourcc.c @@ -168,6 +168,8 @@ const struct drm_format_info *__drm_format_info(u32 format) { .format = DRM_FORMAT_NV61, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1 }, { .format = DRM_FORMAT_NV24, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 }, { .format = DRM_FORMAT_NV42, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 },
{ .format = DRM_FORMAT_XV15, .depth = 0, .num_planes = 2, .pixels_per_macropixel = { 3, 3, 0 }, .bytes_per_macropixel = { 4, 8, 0 }, .hsub = 2, .vsub = 2, },{ .format = DRM_FORMAT_XV20, .depth = 0, .num_planes = 2, .pixels_per_macropixel = { 3, 3, 0 }, .bytes_per_macropixel = { 4, 8, 0 }, .hsub = 2, .vsub = 1, },There's no need to set fields explicitly to 0. I think we could even do a separate patch to nuke all the .depth = 0, assignments.
One thing that I've realized now that your new pixel formats stick out: How is macropixel supposed to interact with hsub/vsub? From you example it looks like macropixels are applied after subsampling (i.e. a macropixel block of 3 pixels, but hsub = 2 means the macroblock will actually span 6 pixels). I think the kerneldoc in the earlier patch should explain this is allowed, and how it's supposed to work exactly.
Also, do we have open-source userspace somewhere for this new pixel format?
We have modified modetest to test these formats. The change, especially the pattern generation part, isn't clean enough to be shared at the moment. But I can do some clean-up and share if that helps.
Then this change (may not be the latest set) was used to prototype the support in the gstreamer kmssink plug-in, but it's implemented by Collabora. Not sure if the change is accessible, but I can check.
Yeah the gstremare kmssink changes would be the userspace enabling we're looking for for merging to upstream. Please include a link to the pull requests/mailing list thread where those patches get reviewed in the commit message for this. -Daniel
I'll address rest of your comments.
Thanks, -hyun
Thanks, Daniel
{ .format = DRM_FORMAT_YUYV, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 }, { .format = DRM_FORMAT_YVYU, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 }, { .format = DRM_FORMAT_UYVY, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index e04613d..6ac5282 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -142,6 +142,14 @@ extern "C" { #define DRM_FORMAT_NV42 fourcc_code('N', 'V', '4', '2') /* non-subsampled Cb:Cr plane */
/*
- 2 plane 10 bit per component YCbCr
- index 0 = Y plane, [31:0] x:Y2:Y1:Y0 2:10:10:10 little endian
- index 1 = Cb:Cr plane, [63:0] x:Cb2:Cr2:Cb1:x:Cr1:Cb0:Cr0 2:10:10:10:2:10:10:10 little endian
- */
+#define DRM_FORMAT_XV15 fourcc_code('X', 'V', '1', '5') /* 2x2 subsampled Cb:Cr plane 2:10:10:10 */ +#define DRM_FORMAT_XV20 fourcc_code('X', 'V', '2', '0') /* 2x1 subsampled Cb:Cr plane 2:10:10:10 */
+/*
- 3 plane YCbCr
- index 0: Y plane, [7:0] Y
- index 1: Cb plane, [7:0] Cb
-- 2.7.4
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch