Hi,
Following up on v1 [1] and v2 [2], this series is to address non-byte aligned formats in drm. Based on the comments, the set is getting simplified, and any further comment would be appreciated. Brief description:
Patch 1 just changes comment style for change in patch 2 Patch 2 adds macro-pixel information to drm_format_info Patch 3 adds a drm function that uses the macro-pixel information Patch 4 adds new formats along with macro-pixel info Patch 5 shows the example of driver integration Patch 6 includes additional format strings
Thanks, -hyun
[1] https://lists.freedesktop.org/archives/dri-devel/2017-November/158744.html [2] https://www.spinics.net/lists/dri-devel/msg163388.html
Hyun Kwon (6): drm: fourcc.h: Use inline kern-doc style for struct drm_format_info drm: drm_fourcc: Introduce macro-pixel info to drm_format_info drm: fourcc: Add drm_format_plane_width_bytes() drm: drm_fourcc: Add new formats for Xilinx IPs drm: xlnx: zynqmp: Add XV15 and XV20 formats drm: fourcc: Add new formats needed by Xilinx IP
drivers/gpu/drm/drm_fb_cma_helper.c | 3 +- drivers/gpu/drm/drm_fourcc.c | 42 ++++++++++++++++++++++ drivers/gpu/drm/xlnx/zynqmp_disp.c | 22 +++++++++++- include/drm/drm_fourcc.h | 69 ++++++++++++++++++++++++++++++++----- include/uapi/drm/drm_fourcc.h | 15 ++++++++ 5 files changed, 141 insertions(+), 10 deletions(-)
Use the inline kern-doc style for struct drm_format_info for better readability. This is just a preliminary change for further table update.
Signed-off-by: Hyun Kwon hyun.kwon@xilinx.com --- v3 - This is added --- --- include/drm/drm_fourcc.h | 45 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 37 insertions(+), 8 deletions(-)
diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h index 6942e84..b00bae4 100644 --- a/include/drm/drm_fourcc.h +++ b/include/drm/drm_fourcc.h @@ -30,21 +30,50 @@ struct drm_mode_fb_cmd2;
/** * struct drm_format_info - information about a DRM format - * @format: 4CC format identifier (DRM_FORMAT_*) - * @depth: Color depth (number of bits per pixel excluding padding bits), - * valid for a subset of RGB formats only. This is a legacy field, do not - * use in new code and set to 0 for new formats. - * @num_planes: Number of color planes (1 to 3) - * @cpp: Number of bytes per pixel (per plane) - * @hsub: Horizontal chroma subsampling factor - * @vsub: Vertical chroma subsampling factor */ struct drm_format_info { + /** + * @format: + * + * 4CC format identifier (DRM_FORMAT_*) + */ u32 format; + + /** + * @depth: + * + * Color depth (number of bits per pixel excluding padding bits), + * valid for a subset of RGB formats only. This is a legacy field, + * do not use in new code and set to 0 for new formats. + */ u8 depth; + + /** + * @num_planes: + * + * Number of color planes (1 to 3) + */ u8 num_planes; + + /** + * @cpp: + * + * Number of bytes per pixel (per plane) + */ u8 cpp[3]; + + /** + * @hsub: + * + * Horizontal chroma subsampling factor + */ u8 hsub; + + /** + * @vsub: + * + * Vertical chroma subsampling factor + */ u8 vsub; };
On Fri, Feb 09, 2018 at 05:35:51PM -0800, Hyun Kwon wrote:
Use the inline kern-doc style for struct drm_format_info for better readability. This is just a preliminary change for further table update.
Signed-off-by: Hyun Kwon hyun.kwon@xilinx.com
v3
- This is added
include/drm/drm_fourcc.h | 45 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 37 insertions(+), 8 deletions(-)
diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h index 6942e84..b00bae4 100644 --- a/include/drm/drm_fourcc.h +++ b/include/drm/drm_fourcc.h @@ -30,21 +30,50 @@ struct drm_mode_fb_cmd2;
/**
- struct drm_format_info - information about a DRM format
- @format: 4CC format identifier (DRM_FORMAT_*)
- @depth: Color depth (number of bits per pixel excluding padding bits),
- valid for a subset of RGB formats only. This is a legacy field, do not
- use in new code and set to 0 for new formats.
- @num_planes: Number of color planes (1 to 3)
- @cpp: Number of bytes per pixel (per plane)
- @hsub: Horizontal chroma subsampling factor
*/
- @vsub: Vertical chroma subsampling factor
struct drm_format_info {
- /**
* @format:** 4CC format identifier (DRM_FORMAT_*)*/- /**
* @depth:** Color depth (number of bits per pixel excluding padding bits),* valid for a subset of RGB formats only. This is a legacy field,* do not use in new code and set to 0 for new formats.*/- /**
* @num_planes:** Number of color planes (1 to 3)*/- /**
* @cpp:** Number of bytes per pixel (per plane)*/- /**
* @hsub:** Horizontal chroma subsampling factor
Since we now have more space, I think it'd be good to clarify here that this is only valid for YUV formats by adding:
"This is only valid for YUV format."
With that:
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
*/- /**
* @vsub:** Vertical chroma subsampling factor*/};
-- 2.7.4
Multiple pixels can be grouped as a single unit and form a 'macro-pixel'. This is to model formats where multiple non-byte aligned pixels are stored together in a byte-aligned way. For example, if 3 - 10 bit pixels are stored in 32 bit, the 32 bit stroage can be treated as a single macro-pixel with 3 pixels. This aligns non-byte addressable formats with drm core where each pixel / component is expected to be byte aligned.
Add 'pixels_per_macro' to note how many pixels are in a macro-pixel. 'bytes_per_macro' specifies the size of a macro-pixel in bytes.
Signed-off-by: Hyun Kwon hyun.kwon@xilinx.com --- v3 - Rename members and rephrase descriptions - Rephrase the commit message - Use in-line style comments v2 - Introduce macro-pixel over scaling factors --- --- include/drm/drm_fourcc.h | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h index b00bae4..ce59329 100644 --- a/include/drm/drm_fourcc.h +++ b/include/drm/drm_fourcc.h @@ -58,11 +58,33 @@ struct drm_format_info { /** * @cpp: * - * Number of bytes per pixel (per plane) + * Number of bytes per pixel (per plane). @cpp shouldn't be used when + * @pixels_per_macropixel and @bytes_per_macropixel are used. */ u8 cpp[3];
/** + * @pixels_per_macropixel: + * + * Number of pixels per macro-pixel (per plane). A macro-pixel is + * composed of multiple pixels, and there can be extra bits between + * pixels. This must be used along with @bytes_per_macropixel, only + * when single pixel size is not byte-aligned. In this case, @cpp + * is not valid and should be 0. + */ + u8 pixels_per_macropixel[3]; + + /* + * @bytes_per_macropixel: + * + * Number of bytes per macro-pixel (per plane). A macro-pixel is + * composed of multiple pixels. The size of single macro-pixel should + * be byte-aligned. This should be used with @pixels_per_macropixel, + * and @cpp should be 0. + */ + u8 bytes_per_macropixel[3]; + + /** * @hsub: * * Horizontal chroma subsampling factor
On Fri, Feb 09, 2018 at 05:35:52PM -0800, Hyun Kwon wrote:
Multiple pixels can be grouped as a single unit and form a 'macro-pixel'. This is to model formats where multiple non-byte aligned pixels are stored together in a byte-aligned way. For example, if 3 - 10 bit pixels are stored in 32 bit, the 32 bit stroage can be treated as a single macro-pixel with 3 pixels. This aligns non-byte addressable formats with drm core where each pixel / component is expected to be byte aligned.
Add 'pixels_per_macro' to note how many pixels are in a macro-pixel. 'bytes_per_macro' specifies the size of a macro-pixel in bytes.
Signed-off-by: Hyun Kwon hyun.kwon@xilinx.com
v3
- Rename members and rephrase descriptions
- Rephrase the commit message
- Use in-line style comments
v2
- Introduce macro-pixel over scaling factors
include/drm/drm_fourcc.h | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h index b00bae4..ce59329 100644 --- a/include/drm/drm_fourcc.h +++ b/include/drm/drm_fourcc.h @@ -58,11 +58,33 @@ struct drm_format_info { /** * @cpp: *
* Number of bytes per pixel (per plane)
* Number of bytes per pixel (per plane). @cpp shouldn't be used when* @pixels_per_macropixel and @bytes_per_macropixel are used.*/ u8 cpp[3];
/**
* @pixels_per_macropixel:** Number of pixels per macro-pixel (per plane). A macro-pixel is* composed of multiple pixels, and there can be extra bits between* pixels. This must be used along with @bytes_per_macropixel, only* when single pixel size is not byte-aligned. In this case, @cpp* is not valid and should be 0.*/u8 pixels_per_macropixel[3];
/*
* @bytes_per_macropixel:** Number of bytes per macro-pixel (per plane). A macro-pixel is* composed of multiple pixels. The size of single macro-pixel should* be byte-aligned. This should be used with @pixels_per_macropixel,* and @cpp should be 0.*/u8 bytes_per_macropixel[3];
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
- /**
- @hsub:
- Horizontal chroma subsampling factor
-- 2.7.4
drm_format_plane_width_bytes() calculates and returns the number of bytes for given width of specified format. The calculation uses @cpp in drm format info for byte-aligned formats. If the format isn't byte-aligned, @cpp should 0, and the macro pixel information is used. This avoids bit level rounding.
Use this drm_fb_cma_get_gem_addr() for offset calculation.
Signed-off-by: Hyun Kwon hyun.kwon@xilinx.com --- v3 - Update according to member changes - Use @cpp for byte-aligned formats, and macro-pixel for non byte-aligned ones - Squash a change in drm_fb_cma_helper.c into this v2 - This function is added --- --- drivers/gpu/drm/drm_fb_cma_helper.c | 3 ++- drivers/gpu/drm/drm_fourcc.c | 35 +++++++++++++++++++++++++++++++++++ include/drm/drm_fourcc.h | 2 ++ 3 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c index 186d00a..271175e 100644 --- a/drivers/gpu/drm/drm_fb_cma_helper.c +++ b/drivers/gpu/drm/drm_fb_cma_helper.c @@ -124,7 +124,8 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer *fb, return 0;
paddr = obj->paddr + fb->offsets[plane]; - paddr += fb->format->cpp[plane] * (state->src_x >> 16); + paddr += drm_format_plane_width_bytes(fb->format, plane, + state->src_x >> 16); paddr += fb->pitches[plane] * (state->src_y >> 16);
return paddr; diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c index 9c0152d..ed95de2 100644 --- a/drivers/gpu/drm/drm_fourcc.c +++ b/drivers/gpu/drm/drm_fourcc.c @@ -348,3 +348,38 @@ int drm_format_plane_height(int height, uint32_t format, int plane) return height / info->vsub; } EXPORT_SYMBOL(drm_format_plane_height); + +/** + * drm_format_plane_width_bytes - bytes of the given width of the plane + * @info: DRM format information + * @plane: plane index + * @width: width to get the number of bytes + * + * This returns the number of bytes for given @width and @plane. + * The @cpp or macro pixel information should be valid. + * + * Returns: + * The bytes of @width of @plane. 0 for invalid format info. + */ +int drm_format_plane_width_bytes(const struct drm_format_info *info, + int plane, int width) +{ + if (!info || plane >= info->num_planes) + return 0; + + if (info->cpp[plane]) + return info->cpp[plane] * width; + + if (WARN_ON(!info->bytes_per_macropixel[plane] || + !info->pixels_per_macropixel[plane])) { + struct drm_format_name_buf buf; + + DRM_WARN("Either cpp or macro-pixel info should be valid: %s\n", + drm_get_format_name(info->format, &buf)); + return 0; + } + + return DIV_ROUND_UP(width * info->bytes_per_macropixel[plane], + info->pixels_per_macropixel[plane]); +} +EXPORT_SYMBOL(drm_format_plane_width_bytes); diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h index ce59329..8158290 100644 --- a/include/drm/drm_fourcc.h +++ b/include/drm/drm_fourcc.h @@ -119,6 +119,8 @@ int drm_format_horz_chroma_subsampling(uint32_t format); int drm_format_vert_chroma_subsampling(uint32_t format); int drm_format_plane_width(int width, uint32_t format, int plane); int drm_format_plane_height(int height, uint32_t format, int plane); +int drm_format_plane_width_bytes(const struct drm_format_info *info, + int plane, int width); const char *drm_get_format_name(uint32_t format, struct drm_format_name_buf *buf);
#endif /* __DRM_FOURCC_H__ */
On Fri, Feb 09, 2018 at 05:35:53PM -0800, Hyun Kwon wrote:
drm_format_plane_width_bytes() calculates and returns the number of bytes for given width of specified format. The calculation uses @cpp in drm format info for byte-aligned formats. If the format isn't byte-aligned, @cpp should 0, and the macro pixel information is used. This avoids bit level rounding.
Use this drm_fb_cma_get_gem_addr() for offset calculation.
Signed-off-by: Hyun Kwon hyun.kwon@xilinx.com
We also need to adjust the validation in framebuffer_check, that's using raw cpp. I think that's all there is for the core code, but please also grep for cpp anywhere in core code and make sure we have them all. You probably want to use the newly introduced helper function in all these core places (like you do for cma helpers already).
v3
- Update according to member changes
- Use @cpp for byte-aligned formats, and macro-pixel for non byte-aligned ones
- Squash a change in drm_fb_cma_helper.c into this
v2
- This function is added
drivers/gpu/drm/drm_fb_cma_helper.c | 3 ++- drivers/gpu/drm/drm_fourcc.c | 35 +++++++++++++++++++++++++++++++++++ include/drm/drm_fourcc.h | 2 ++ 3 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c index 186d00a..271175e 100644 --- a/drivers/gpu/drm/drm_fb_cma_helper.c +++ b/drivers/gpu/drm/drm_fb_cma_helper.c @@ -124,7 +124,8 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer *fb, return 0;
paddr = obj->paddr + fb->offsets[plane];
- paddr += fb->format->cpp[plane] * (state->src_x >> 16);
paddr += drm_format_plane_width_bytes(fb->format, plane,
state->src_x >> 16);paddr += fb->pitches[plane] * (state->src_y >> 16);
return paddr;
diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c index 9c0152d..ed95de2 100644 --- a/drivers/gpu/drm/drm_fourcc.c +++ b/drivers/gpu/drm/drm_fourcc.c @@ -348,3 +348,38 @@ int drm_format_plane_height(int height, uint32_t format, int plane) return height / info->vsub; } EXPORT_SYMBOL(drm_format_plane_height);
+/**
- drm_format_plane_width_bytes - bytes of the given width of the plane
- @info: DRM format information
- @plane: plane index
- @width: width to get the number of bytes
- This returns the number of bytes for given @width and @plane.
- The @cpp or macro pixel information should be valid.
- Returns:
- The bytes of @width of @plane. 0 for invalid format info.
- */
+int drm_format_plane_width_bytes(const struct drm_format_info *info,
int plane, int width)
We need to extend the kernel code for drm_format_plane_cpp to explain that it can't handle formats with macropixels (for obvious reasons) and that generic code must use drm_format_plane_width_bytes instead of cpp. Drivers can keep using drm_format_plane_cpp as long as they don't support a 4CC code with macro pixels.
+{
- if (!info || plane >= info->num_planes)
return 0;- if (info->cpp[plane])
I think a WARN_ON(bytes_per_macropixel || pixels_per_macropixel); here would be good, to make sure only one or the other is set (to force consistency and avoid silly bugs).
With the above details addressed I think this patch looks good. I'll triple-check the new version for any missing cpp conversions in the drm core once more though. -Daniel
return info->cpp[plane] * width;- if (WARN_ON(!info->bytes_per_macropixel[plane] ||
!info->pixels_per_macropixel[plane])) {struct drm_format_name_buf buf;DRM_WARN("Either cpp or macro-pixel info should be valid: %s\n",drm_get_format_name(info->format, &buf));return 0;- }
- return DIV_ROUND_UP(width * info->bytes_per_macropixel[plane],
info->pixels_per_macropixel[plane]);+} +EXPORT_SYMBOL(drm_format_plane_width_bytes); diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h index ce59329..8158290 100644 --- a/include/drm/drm_fourcc.h +++ b/include/drm/drm_fourcc.h @@ -119,6 +119,8 @@ int drm_format_horz_chroma_subsampling(uint32_t format); int drm_format_vert_chroma_subsampling(uint32_t format); int drm_format_plane_width(int width, uint32_t format, int plane); int drm_format_plane_height(int height, uint32_t format, int plane); +int drm_format_plane_width_bytes(const struct drm_format_info *info,
int plane, int width);const char *drm_get_format_name(uint32_t format, struct drm_format_name_buf *buf);
#endif /* __DRM_FOURCC_H__ */
2.7.4
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, }, { .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
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?
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
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.
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
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
Use drm_format_width_bytes() to support non-byte aligned formats.
Signed-off-by: Hyun Kwon hyun.kwon@xilinx.com --- v3 - 2 patches are squashed --- --- drivers/gpu/drm/xlnx/zynqmp_disp.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c index e47d77d..13053fc 100644 --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c @@ -968,6 +968,24 @@ static const struct zynqmp_disp_fmt av_buf_vid_fmts[] = { .sf[0] = ZYNQMP_DISP_AV_BUF_8BIT_SF, .sf[1] = ZYNQMP_DISP_AV_BUF_8BIT_SF, .sf[2] = ZYNQMP_DISP_AV_BUF_8BIT_SF, + }, { + .drm_fmt = DRM_FORMAT_XV15, + .disp_fmt = ZYNQMP_DISP_AV_BUF_FMT_NL_VID_YV16CI_420_10, + .rgb = false, + .swap = false, + .chroma_sub = true, + .sf[0] = ZYNQMP_DISP_AV_BUF_10BIT_SF, + .sf[1] = ZYNQMP_DISP_AV_BUF_10BIT_SF, + .sf[2] = ZYNQMP_DISP_AV_BUF_10BIT_SF, + }, { + .drm_fmt = DRM_FORMAT_XV20, + .disp_fmt = ZYNQMP_DISP_AV_BUF_FMT_NL_VID_YV16CI_10, + .rgb = false, + .swap = false, + .chroma_sub = true, + .sf[0] = ZYNQMP_DISP_AV_BUF_10BIT_SF, + .sf[1] = ZYNQMP_DISP_AV_BUF_10BIT_SF, + .sf[2] = ZYNQMP_DISP_AV_BUF_10BIT_SF, } };
@@ -2133,6 +2151,7 @@ static int zynqmp_disp_plane_mode_set(struct drm_plane *plane, for (i = 0; i < info->num_planes; i++) { unsigned int width = src_w / (i ? info->hsub : 1); unsigned int height = src_h / (i ? info->vsub : 1); + int width_bytes;
paddr = drm_fb_cma_get_gem_addr(fb, plane->state, i); if (!paddr) { @@ -2141,7 +2160,8 @@ static int zynqmp_disp_plane_mode_set(struct drm_plane *plane, }
layer->dma[i].xt.numf = height; - layer->dma[i].sgl[0].size = width * info->cpp[i]; + width_bytes = drm_format_plane_width_bytes(info, i, width); + layer->dma[i].sgl[0].size = width_bytes; layer->dma[i].sgl[0].icg = fb->pitches[i] - layer->dma[i].sgl[0].size; layer->dma[i].xt.src_start = paddr;
This adds packed YUV and grey scale format fourccs.
Signed-off-by: Hyun Kwon hyun.kwon@xilinx.com
--- v3 - Update entries for changes - Squash fourcc patch into this - Note these don't have any reference in mainline v2 - Split from the previous patch --- --- drivers/gpu/drm/drm_fourcc.c | 5 +++++ include/uapi/drm/drm_fourcc.h | 7 +++++++ 2 files changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c index 36bff7a..cf35251 100644 --- a/drivers/gpu/drm/drm_fourcc.c +++ b/drivers/gpu/drm/drm_fourcc.c @@ -175,6 +175,11 @@ const struct drm_format_info *__drm_format_info(u32 format) { .format = DRM_FORMAT_UYVY, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 }, { .format = DRM_FORMAT_VYUY, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 }, { .format = DRM_FORMAT_AYUV, .depth = 0, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_VUY888, .depth = 0, .num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_XVUY8888, .depth = 0, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_XVUY2101010, .depth = 0, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_Y8, .depth = 0, .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_Y10, .depth = 0, .num_planes = 1, .pixels_per_macropixel = { 3, 0, 0 }, .bytes_per_macropixel = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, };
unsigned int i; diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index 6ac5282..7014a3d 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -112,6 +112,13 @@ extern "C" { #define DRM_FORMAT_VYUY fourcc_code('V', 'Y', 'U', 'Y') /* [31:0] Y1:Cb0:Y0:Cr0 8:8:8:8 little endian */
#define DRM_FORMAT_AYUV fourcc_code('A', 'Y', 'U', 'V') /* [31:0] A:Y:Cb:Cr 8:8:8:8 little endian */ +#define DRM_FORMAT_VUY888 fourcc_code('V', 'U', '2', '4') /* [23:0] Cr:Cb:Y 8:8:8 little endian */ +#define DRM_FORMAT_XVUY8888 fourcc_code('X', 'V', '2', '4') /* [31:0] x:Cr:Cb:Y 8:8:8:8 little endian */ +#define DRM_FORMAT_XVUY2101010 fourcc_code('X', 'Y', '3', '0') /* [31:0] x:Cr:Cb:Y 2:10:10:10 little endian */ + +/* Grey scale */ +#define DRM_FORMAT_Y8 fourcc_code('G', 'R', 'E', 'Y') /* 8 Greyscale */ +#define DRM_FORMAT_Y10 fourcc_code('Y', '1', '0', ' ') /* 10 Greyscale */
/* * 2 plane RGB + A
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Hyun Kwon