All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC v3 0/6] Support of non-byte aligned formats in drm
@ 2018-02-10  1:35 Hyun Kwon
  2018-02-10  1:35 ` [PATCH RFC v3 1/6] drm: fourcc.h: Use inline kern-doc style for struct drm_format_info Hyun Kwon
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Hyun Kwon @ 2018-02-10  1:35 UTC (permalink / raw)
  To: dri-devel
  Cc: Hyun Kwon, Daniel Vetter, Emil Velikov, Michal Simek, Laurent Pinchart

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

-- 
2.7.4

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH RFC v3 1/6] drm: fourcc.h: Use inline kern-doc style for struct drm_format_info
  2018-02-10  1:35 [PATCH RFC v3 0/6] Support of non-byte aligned formats in drm Hyun Kwon
@ 2018-02-10  1:35 ` Hyun Kwon
  2018-02-19 11:19   ` Daniel Vetter
  2018-02-10  1:35 ` [PATCH RFC v3 2/6] drm: drm_fourcc: Introduce macro-pixel info to drm_format_info Hyun Kwon
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Hyun Kwon @ 2018-02-10  1:35 UTC (permalink / raw)
  To: dri-devel
  Cc: Hyun Kwon, Daniel Vetter, Emil Velikov, Michal Simek, Laurent Pinchart

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;
 };
 
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH RFC v3 2/6] drm: drm_fourcc: Introduce macro-pixel info to drm_format_info
  2018-02-10  1:35 [PATCH RFC v3 0/6] Support of non-byte aligned formats in drm Hyun Kwon
  2018-02-10  1:35 ` [PATCH RFC v3 1/6] drm: fourcc.h: Use inline kern-doc style for struct drm_format_info Hyun Kwon
@ 2018-02-10  1:35 ` Hyun Kwon
  2018-02-19 11:20   ` Daniel Vetter
  2018-02-10  1:35 ` [PATCH RFC v3 3/6] drm: fourcc: Add drm_format_plane_width_bytes() Hyun Kwon
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Hyun Kwon @ 2018-02-10  1:35 UTC (permalink / raw)
  To: dri-devel
  Cc: Hyun Kwon, Daniel Vetter, Emil Velikov, Michal Simek, Laurent Pinchart

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

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH RFC v3 3/6] drm: fourcc: Add drm_format_plane_width_bytes()
  2018-02-10  1:35 [PATCH RFC v3 0/6] Support of non-byte aligned formats in drm Hyun Kwon
  2018-02-10  1:35 ` [PATCH RFC v3 1/6] drm: fourcc.h: Use inline kern-doc style for struct drm_format_info Hyun Kwon
  2018-02-10  1:35 ` [PATCH RFC v3 2/6] drm: drm_fourcc: Introduce macro-pixel info to drm_format_info Hyun Kwon
@ 2018-02-10  1:35 ` Hyun Kwon
  2018-02-19 11:27   ` Daniel Vetter
  2018-02-10  1:35 ` [PATCH RFC v3 4/6] drm: drm_fourcc: Add new formats for Xilinx IPs Hyun Kwon
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Hyun Kwon @ 2018-02-10  1:35 UTC (permalink / raw)
  To: dri-devel
  Cc: Hyun Kwon, Daniel Vetter, Emil Velikov, Michal Simek, Laurent Pinchart

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__ */
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH RFC v3 4/6] drm: drm_fourcc: Add new formats for Xilinx IPs
  2018-02-10  1:35 [PATCH RFC v3 0/6] Support of non-byte aligned formats in drm Hyun Kwon
                   ` (2 preceding siblings ...)
  2018-02-10  1:35 ` [PATCH RFC v3 3/6] drm: fourcc: Add drm_format_plane_width_bytes() Hyun Kwon
@ 2018-02-10  1:35 ` Hyun Kwon
  2018-02-19 14:22   ` Daniel Vetter
  2018-02-10  1:35 ` [PATCH RFC v3 5/6] drm: xlnx: zynqmp: Add XV15 and XV20 formats Hyun Kwon
  2018-02-10  1:35 ` [PATCH RFC v3 6/6] drm: fourcc: Add new formats needed by Xilinx IP Hyun Kwon
  5 siblings, 1 reply; 13+ messages in thread
From: Hyun Kwon @ 2018-02-10  1:35 UTC (permalink / raw)
  To: dri-devel
  Cc: Hyun Kwon, Daniel Vetter, Emil Velikov, Michal Simek,
	Laurent Pinchart, Jeffrey Mouroux

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

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH RFC v3 5/6] drm: xlnx: zynqmp: Add XV15 and XV20 formats
  2018-02-10  1:35 [PATCH RFC v3 0/6] Support of non-byte aligned formats in drm Hyun Kwon
                   ` (3 preceding siblings ...)
  2018-02-10  1:35 ` [PATCH RFC v3 4/6] drm: drm_fourcc: Add new formats for Xilinx IPs Hyun Kwon
@ 2018-02-10  1:35 ` Hyun Kwon
  2018-02-10  1:35 ` [PATCH RFC v3 6/6] drm: fourcc: Add new formats needed by Xilinx IP Hyun Kwon
  5 siblings, 0 replies; 13+ messages in thread
From: Hyun Kwon @ 2018-02-10  1:35 UTC (permalink / raw)
  To: dri-devel
  Cc: Hyun Kwon, Daniel Vetter, Emil Velikov, Michal Simek, Laurent Pinchart

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

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH RFC v3 6/6] drm: fourcc: Add new formats needed by Xilinx IP
  2018-02-10  1:35 [PATCH RFC v3 0/6] Support of non-byte aligned formats in drm Hyun Kwon
                   ` (4 preceding siblings ...)
  2018-02-10  1:35 ` [PATCH RFC v3 5/6] drm: xlnx: zynqmp: Add XV15 and XV20 formats Hyun Kwon
@ 2018-02-10  1:35 ` Hyun Kwon
  5 siblings, 0 replies; 13+ messages in thread
From: Hyun Kwon @ 2018-02-10  1:35 UTC (permalink / raw)
  To: dri-devel
  Cc: Hyun Kwon, Daniel Vetter, Emil Velikov, Michal Simek, Laurent Pinchart

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

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH RFC v3 1/6] drm: fourcc.h: Use inline kern-doc style for struct drm_format_info
  2018-02-10  1:35 ` [PATCH RFC v3 1/6] drm: fourcc.h: Use inline kern-doc style for struct drm_format_info Hyun Kwon
@ 2018-02-19 11:19   ` Daniel Vetter
  0 siblings, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2018-02-19 11:19 UTC (permalink / raw)
  To: Hyun Kwon
  Cc: Daniel Vetter, Emil Velikov, Michal Simek, dri-devel, Laurent Pinchart

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_*)
> +	 */
>  	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

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>

> +	 */
>  	u8 hsub;
> +
> +	/**
> +	 * @vsub:
> +	 *
> +	 * Vertical chroma subsampling factor
> +	 */
>  	u8 vsub;
>  };
>  
> -- 
> 2.7.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH RFC v3 2/6] drm: drm_fourcc: Introduce macro-pixel info to drm_format_info
  2018-02-10  1:35 ` [PATCH RFC v3 2/6] drm: drm_fourcc: Introduce macro-pixel info to drm_format_info Hyun Kwon
@ 2018-02-19 11:20   ` Daniel Vetter
  0 siblings, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2018-02-19 11:20 UTC (permalink / raw)
  To: Hyun Kwon
  Cc: Daniel Vetter, Emil Velikov, Michal Simek, dri-devel, Laurent Pinchart

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
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH RFC v3 3/6] drm: fourcc: Add drm_format_plane_width_bytes()
  2018-02-10  1:35 ` [PATCH RFC v3 3/6] drm: fourcc: Add drm_format_plane_width_bytes() Hyun Kwon
@ 2018-02-19 11:27   ` Daniel Vetter
  0 siblings, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2018-02-19 11:27 UTC (permalink / raw)
  To: Hyun Kwon
  Cc: Daniel Vetter, Emil Velikov, Michal Simek, dri-devel, Laurent Pinchart

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
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH RFC v3 4/6] drm: drm_fourcc: Add new formats for Xilinx IPs
  2018-02-10  1:35 ` [PATCH RFC v3 4/6] drm: drm_fourcc: Add new formats for Xilinx IPs Hyun Kwon
@ 2018-02-19 14:22   ` Daniel Vetter
  2018-02-23  2:41     ` Hyun Kwon
  0 siblings, 1 reply; 13+ messages in thread
From: Daniel Vetter @ 2018-02-19 14:22 UTC (permalink / raw)
  To: Hyun Kwon
  Cc: Daniel Vetter, Emil Velikov, Michal Simek, dri-devel,
	Laurent Pinchart, Jeffrey Mouroux

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
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH RFC v3 4/6] drm: drm_fourcc: Add new formats for Xilinx IPs
  2018-02-19 14:22   ` Daniel Vetter
@ 2018-02-23  2:41     ` Hyun Kwon
  2018-03-05  8:50       ` Daniel Vetter
  0 siblings, 1 reply; 13+ messages in thread
From: Hyun Kwon @ 2018-02-23  2:41 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Daniel Vetter, Emil Velikov, Michal Simek, dri-devel,
	Laurent Pinchart, Jeff Mouroux

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH RFC v3 4/6] drm: drm_fourcc: Add new formats for Xilinx IPs
  2018-02-23  2:41     ` Hyun Kwon
@ 2018-03-05  8:50       ` Daniel Vetter
  0 siblings, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2018-03-05  8:50 UTC (permalink / raw)
  To: Hyun Kwon
  Cc: Daniel Vetter, Emil Velikov, Michal Simek, dri-devel,
	Laurent Pinchart, Jeff Mouroux

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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2018-03-05  8:50 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-10  1:35 [PATCH RFC v3 0/6] Support of non-byte aligned formats in drm Hyun Kwon
2018-02-10  1:35 ` [PATCH RFC v3 1/6] drm: fourcc.h: Use inline kern-doc style for struct drm_format_info Hyun Kwon
2018-02-19 11:19   ` Daniel Vetter
2018-02-10  1:35 ` [PATCH RFC v3 2/6] drm: drm_fourcc: Introduce macro-pixel info to drm_format_info Hyun Kwon
2018-02-19 11:20   ` Daniel Vetter
2018-02-10  1:35 ` [PATCH RFC v3 3/6] drm: fourcc: Add drm_format_plane_width_bytes() Hyun Kwon
2018-02-19 11:27   ` Daniel Vetter
2018-02-10  1:35 ` [PATCH RFC v3 4/6] drm: drm_fourcc: Add new formats for Xilinx IPs Hyun Kwon
2018-02-19 14:22   ` Daniel Vetter
2018-02-23  2:41     ` Hyun Kwon
2018-03-05  8:50       ` Daniel Vetter
2018-02-10  1:35 ` [PATCH RFC v3 5/6] drm: xlnx: zynqmp: Add XV15 and XV20 formats Hyun Kwon
2018-02-10  1:35 ` [PATCH RFC v3 6/6] drm: fourcc: Add new formats needed by Xilinx IP Hyun Kwon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.