linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/3] drm: Add some new format DRM_FORMAT_NVXX_10
       [not found] <1569486289-152061-1-git-send-email-hjc@rock-chips.com>
@ 2019-09-26  8:24 ` Sandy Huang
  2019-09-29  3:26   ` [v2,1/3] " james qian wang (Arm Technology China)
                     ` (2 more replies)
  2019-09-26  8:24 ` [PATCH v2 2/3] drm/rockchip: Add rockchip_vop_get_offset to get offset Sandy Huang
  2019-09-26  8:24 ` [PATCH v2 3/3] drm/rockchip: Add support 10bit yuv format Sandy Huang
  2 siblings, 3 replies; 16+ messages in thread
From: Sandy Huang @ 2019-09-26  8:24 UTC (permalink / raw)
  To: dri-devel, Maarten Lankhorst, Maxime Ripard, Sean Paul,
	David Airlie, Daniel Vetter
  Cc: hjc, heiko, linux-kernel

These new format is supported by some rockchip socs:

DRM_FORMAT_NV12_10/DRM_FORMAT_NV21_10
DRM_FORMAT_NV16_10/DRM_FORMAT_NV61_10
DRM_FORMAT_NV24_10/DRM_FORMAT_NV42_10

Signed-off-by: Sandy Huang <hjc@rock-chips.com>
---
 drivers/gpu/drm/drm_fourcc.c  | 18 ++++++++++++++++++
 include/uapi/drm/drm_fourcc.h | 14 ++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index c630064..ccd78a3 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -261,6 +261,24 @@ const struct drm_format_info *__drm_format_info(u32 format)
 		{ .format = DRM_FORMAT_P016,		.depth = 0,  .num_planes = 2,
 		  .char_per_block = { 2, 4, 0 }, .block_w = { 1, 0, 0 }, .block_h = { 1, 0, 0 },
 		  .hsub = 2, .vsub = 2, .is_yuv = true},
+		{ .format = DRM_FORMAT_NV12_10,		.depth = 0,  .num_planes = 2,
+		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
+		  .hsub = 2, .vsub = 2, .is_yuv = true},
+		{ .format = DRM_FORMAT_NV21_10,		.depth = 0,  .num_planes = 2,
+		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
+		  .hsub = 2, .vsub = 2, .is_yuv = true},
+		{ .format = DRM_FORMAT_NV16_10,		.depth = 0,  .num_planes = 2,
+		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
+		  .hsub = 2, .vsub = 1, .is_yuv = true},
+		{ .format = DRM_FORMAT_NV61_10,		.depth = 0,  .num_planes = 2,
+		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
+		  .hsub = 2, .vsub = 1, .is_yuv = true},
+		{ .format = DRM_FORMAT_NV24_10,		.depth = 0,  .num_planes = 2,
+		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
+		  .hsub = 1, .vsub = 1, .is_yuv = true},
+		{ .format = DRM_FORMAT_NV42_10,		.depth = 0,  .num_planes = 2,
+		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
+		  .hsub = 1, .vsub = 1, .is_yuv = true},
 		{ .format = DRM_FORMAT_P210,		.depth = 0,
 		  .num_planes = 2, .char_per_block = { 2, 4, 0 },
 		  .block_w = { 1, 0, 0 }, .block_h = { 1, 0, 0 }, .hsub = 2,
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 3feeaa3..08e2221 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -238,6 +238,20 @@ extern "C" {
 #define DRM_FORMAT_NV42		fourcc_code('N', 'V', '4', '2') /* non-subsampled Cb:Cr plane */
 
 /*
+ * 2 plane YCbCr
+ * index 0 = Y plane, Y3:Y2:Y1:Y0 10:10:10:10
+ * index 1 = Cb:Cr plane, Cb3:Cr3:Cb2:Cr2:Cb1:Cr1:Cb0:Cr0 10:10:10:10:10:10:10:10
+ * or
+ * index 1 = Cr:Cb plane, Cr3:Cb3:Cr2:Cb2:Cr1:Cb1:Cr0:Cb0 10:10:10:10:10:10:10:10
+ */
+#define DRM_FORMAT_NV12_10	fourcc_code('N', 'A', '1', '2') /* 2x2 subsampled Cr:Cb plane */
+#define DRM_FORMAT_NV21_10	fourcc_code('N', 'A', '2', '1') /* 2x2 subsampled Cb:Cr plane */
+#define DRM_FORMAT_NV16_10	fourcc_code('N', 'A', '1', '6') /* 2x1 subsampled Cr:Cb plane */
+#define DRM_FORMAT_NV61_10	fourcc_code('N', 'A', '6', '1') /* 2x1 subsampled Cb:Cr plane */
+#define DRM_FORMAT_NV24_10	fourcc_code('N', 'A', '2', '4') /* non-subsampled Cr:Cb plane */
+#define DRM_FORMAT_NV42_10	fourcc_code('N', 'A', '4', '2') /* non-subsampled Cb:Cr plane */
+
+/*
  * 2 plane YCbCr MSB aligned
  * index 0 = Y plane, [15:0] Y:x [10:6] little endian
  * index 1 = Cr:Cb plane, [31:0] Cr:x:Cb:x [10:6:10:6] little endian
-- 
2.7.4




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

* [PATCH v2 2/3] drm/rockchip: Add rockchip_vop_get_offset to get offset
       [not found] <1569486289-152061-1-git-send-email-hjc@rock-chips.com>
  2019-09-26  8:24 ` [PATCH v2 1/3] drm: Add some new format DRM_FORMAT_NVXX_10 Sandy Huang
@ 2019-09-26  8:24 ` Sandy Huang
  2019-09-26  8:24 ` [PATCH v2 3/3] drm/rockchip: Add support 10bit yuv format Sandy Huang
  2 siblings, 0 replies; 16+ messages in thread
From: Sandy Huang @ 2019-09-26  8:24 UTC (permalink / raw)
  To: dri-devel, Sandy Huang, Heiko Stübner, David Airlie, Daniel Vetter
  Cc: maarten.lankhorst, linux-arm-kernel, linux-rockchip, linux-kernel

Add rockchip_vop_get_offset to get offset, this can compatible legacy
and block_h/w format describe.

Signed-off-by: Sandy Huang <hjc@rock-chips.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 61 ++++++++++++++++++++++++-----
 1 file changed, 52 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 2f821c5..ce74218 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -249,6 +249,56 @@ static bool has_rb_swapped(uint32_t format)
 	}
 }
 
+static u32 rockchip_vop_get_offset(struct drm_plane_state *state, u8 plane)
+{
+	const struct drm_format_info *info;
+	struct drm_rect *src = &state->src;
+	struct drm_framebuffer *fb = state->fb;
+	u32 format = fb->format->format;
+	u8 h_div = 1, v_div = 1;
+	u32 block_w, block_h, block_size, block_start_y, num_hblocks;
+	u32 sample_x, sample_y;
+	u32 offset;
+
+	info = drm_format_info(format);
+	if (!info || plane >= info->num_planes)
+		return 0;
+
+	if (plane > 0) {
+		h_div = fb->format->hsub;
+		v_div = fb->format->vsub;
+	}
+
+	switch (format) {
+	case DRM_FORMAT_NV12_10:
+	case DRM_FORMAT_NV21_10:
+	case DRM_FORMAT_NV16_10:
+	case DRM_FORMAT_NV61_10:
+	case DRM_FORMAT_NV24_10:
+	case DRM_FORMAT_NV42_10:
+		block_w = drm_format_info_block_width(fb->format, plane);
+		block_h = drm_format_info_block_height(fb->format, plane);
+		block_size = fb->format->char_per_block[plane];
+
+		sample_x = (src->x1 >> 16) / h_div;
+		sample_y = (src->y1 >> 16) / v_div;
+		block_start_y = (sample_y / block_h) * block_h;
+		num_hblocks = sample_x / block_w;
+
+		offset = fb->pitches[plane] * block_start_y;
+		offset += block_size * num_hblocks;
+
+		break;
+	default:
+		offset = (src->x1 >> 16) * fb->format->cpp[plane] / h_div;
+		offset += (src->y1 >> 16) * fb->pitches[plane] / v_div;
+
+		break;
+	}
+
+	return offset;
+}
+
 static enum vop_data_format vop_convert_format(uint32_t format)
 {
 	switch (format) {
@@ -832,8 +882,7 @@ static void vop_plane_atomic_update(struct drm_plane *plane,
 	dsp_sty = dest->y1 + crtc->mode.vtotal - crtc->mode.vsync_start;
 	dsp_st = dsp_sty << 16 | (dsp_stx & 0xffff);
 
-	offset = (src->x1 >> 16) * fb->format->cpp[0];
-	offset += (src->y1 >> 16) * fb->pitches[0];
+	offset = rockchip_vop_get_offset(state, 0);
 	dma_addr = rk_obj->dma_addr + offset + fb->offsets[0];
 
 	/*
@@ -857,16 +906,10 @@ static void vop_plane_atomic_update(struct drm_plane *plane,
 		    (state->rotation & DRM_MODE_REFLECT_X) ? 1 : 0);
 
 	if (is_yuv) {
-		int hsub = fb->format->hsub;
-		int vsub = fb->format->vsub;
-		int bpp = fb->format->cpp[1];
-
 		uv_obj = fb->obj[1];
 		rk_uv_obj = to_rockchip_obj(uv_obj);
 
-		offset = (src->x1 >> 16) * bpp / hsub;
-		offset += (src->y1 >> 16) * fb->pitches[1] / vsub;
-
+		offset = rockchip_vop_get_offset(state, 1);
 		dma_addr = rk_uv_obj->dma_addr + offset + fb->offsets[1];
 		VOP_WIN_SET(vop, win, uv_vir, DIV_ROUND_UP(fb->pitches[1], 4));
 		VOP_WIN_SET(vop, win, uv_mst, dma_addr);
-- 
2.7.4




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

* [PATCH v2 3/3] drm/rockchip: Add support 10bit yuv format
       [not found] <1569486289-152061-1-git-send-email-hjc@rock-chips.com>
  2019-09-26  8:24 ` [PATCH v2 1/3] drm: Add some new format DRM_FORMAT_NVXX_10 Sandy Huang
  2019-09-26  8:24 ` [PATCH v2 2/3] drm/rockchip: Add rockchip_vop_get_offset to get offset Sandy Huang
@ 2019-09-26  8:24 ` Sandy Huang
  2 siblings, 0 replies; 16+ messages in thread
From: Sandy Huang @ 2019-09-26  8:24 UTC (permalink / raw)
  To: dri-devel, Sandy Huang, Heiko Stübner, David Airlie, Daniel Vetter
  Cc: maarten.lankhorst, linux-arm-kernel, linux-rockchip, linux-kernel

Add support 10bit yuv format display for rockchip some socs,
include:
    RK3288/RK3228/RK3328/RK3368/RK3399

Signed-off-by: Sandy Huang <hjc@rock-chips.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 16 ++++++++++++++++
 drivers/gpu/drm/rockchip/rockchip_drm_vop.h |  1 +
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c |  2 ++
 3 files changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index ce74218..4b87d88 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -249,6 +249,21 @@ static bool has_rb_swapped(uint32_t format)
 	}
 }
 
+static bool is_10bit_yuv(uint32_t format)
+{
+	switch (format) {
+	case DRM_FORMAT_NV12_10:
+	case DRM_FORMAT_NV21_10:
+	case DRM_FORMAT_NV16_10:
+	case DRM_FORMAT_NV61_10:
+	case DRM_FORMAT_NV24_10:
+	case DRM_FORMAT_NV42_10:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static u32 rockchip_vop_get_offset(struct drm_plane_state *state, u8 plane)
 {
 	const struct drm_format_info *info;
@@ -913,6 +928,7 @@ static void vop_plane_atomic_update(struct drm_plane *plane,
 		dma_addr = rk_uv_obj->dma_addr + offset + fb->offsets[1];
 		VOP_WIN_SET(vop, win, uv_vir, DIV_ROUND_UP(fb->pitches[1], 4));
 		VOP_WIN_SET(vop, win, uv_mst, dma_addr);
+		VOP_WIN_SET(vop, win, fmt_10, is_10bit_yuv(fb->format->format));
 
 		for (i = 0; i < NUM_YUV2YUV_COEFFICIENTS; i++) {
 			VOP_WIN_YUV2YUV_COEFFICIENT_SET(vop,
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
index 2149a889..adc2b0b5 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
@@ -133,6 +133,7 @@ struct vop_win_phy {
 	struct vop_reg gate;
 	struct vop_reg format;
 	struct vop_reg rb_swap;
+	struct vop_reg fmt_10;
 	struct vop_reg act_info;
 	struct vop_reg dsp_info;
 	struct vop_reg dsp_st;
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
index d1494be..732e535 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
@@ -544,6 +544,7 @@ static const struct vop_win_phy rk3288_win01_data = {
 	.nformats = ARRAY_SIZE(formats_win_full),
 	.enable = VOP_REG(RK3288_WIN0_CTRL0, 0x1, 0),
 	.format = VOP_REG(RK3288_WIN0_CTRL0, 0x7, 1),
+	.fmt_10 = VOP_REG(RK3288_WIN0_CTRL0, 0x1, 4),
 	.rb_swap = VOP_REG(RK3288_WIN0_CTRL0, 0x1, 12),
 	.act_info = VOP_REG(RK3288_WIN0_ACT_INFO, 0x1fff1fff, 0),
 	.dsp_info = VOP_REG(RK3288_WIN0_DSP_INFO, 0x0fff0fff, 0),
@@ -674,6 +675,7 @@ static const struct vop_win_phy rk3368_win01_data = {
 	.nformats = ARRAY_SIZE(formats_win_full),
 	.enable = VOP_REG(RK3368_WIN0_CTRL0, 0x1, 0),
 	.format = VOP_REG(RK3368_WIN0_CTRL0, 0x7, 1),
+	.fmt_10 = VOP_REG(RK3368_WIN0_CTRL0, 0x1, 4),
 	.rb_swap = VOP_REG(RK3368_WIN0_CTRL0, 0x1, 12),
 	.x_mir_en = VOP_REG(RK3368_WIN0_CTRL0, 0x1, 21),
 	.y_mir_en = VOP_REG(RK3368_WIN0_CTRL0, 0x1, 22),
-- 
2.7.4




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

* Re: [v2,1/3] drm: Add some new format DRM_FORMAT_NVXX_10
  2019-09-26  8:24 ` [PATCH v2 1/3] drm: Add some new format DRM_FORMAT_NVXX_10 Sandy Huang
@ 2019-09-29  3:26   ` james qian wang (Arm Technology China)
  2019-09-30 10:48   ` [PATCH v2 1/3] " Ville Syrjälä
  2019-10-09 14:50   ` Daniel Vetter
  2 siblings, 0 replies; 16+ messages in thread
From: james qian wang (Arm Technology China) @ 2019-09-29  3:26 UTC (permalink / raw)
  To: Sandy Huang
  Cc: dri-devel, Maarten Lankhorst, Maxime Ripard, Sean Paul,
	David Airlie, Daniel Vetter, linux-kernel, nd

On Thu, Sep 26, 2019 at 04:24:47PM +0800, Sandy Huang wrote:
> These new format is supported by some rockchip socs:
> 
> DRM_FORMAT_NV12_10/DRM_FORMAT_NV21_10
> DRM_FORMAT_NV16_10/DRM_FORMAT_NV61_10
> DRM_FORMAT_NV24_10/DRM_FORMAT_NV42_10
> 
> Signed-off-by: Sandy Huang <hjc@rock-chips.com>
> ---
>  drivers/gpu/drm/drm_fourcc.c  | 18 ++++++++++++++++++
>  include/uapi/drm/drm_fourcc.h | 14 ++++++++++++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index c630064..ccd78a3 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -261,6 +261,24 @@ const struct drm_format_info *__drm_format_info(u32 format)
>  		{ .format = DRM_FORMAT_P016,		.depth = 0,  .num_planes = 2,
>  		  .char_per_block = { 2, 4, 0 }, .block_w = { 1, 0, 0 }, .block_h = { 1, 0, 0 },
>  		  .hsub = 2, .vsub = 2, .is_yuv = true},
> +		{ .format = DRM_FORMAT_NV12_10,		.depth = 0,  .num_planes = 2,
> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },

Hi Sandy:

Their is a problem here for char_per_block size of plane[0]:

Since: 5 * 8 != 4 * 4 * 10;

Seems you mis-set the block_w/h, per your block size the block is 2x2, and it should be:

 .char_per_block = { 5, 10, 0 }, .block_w = { 2, 2, 0 }, .block_h = { 2, 2, 0 },


Best Regards:
James

> +		  .hsub = 2, .vsub = 2, .is_yuv = true},
> +		{ .format = DRM_FORMAT_NV21_10,		.depth = 0,  .num_planes = 2,
> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> +		  .hsub = 2, .vsub = 2, .is_yuv = true},
> +		{ .format = DRM_FORMAT_NV16_10,		.depth = 0,  .num_planes = 2,
> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> +		  .hsub = 2, .vsub = 1, .is_yuv = true},
> +		{ .format = DRM_FORMAT_NV61_10,		.depth = 0,  .num_planes = 2,
> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> +		  .hsub = 2, .vsub = 1, .is_yuv = true},
> +		{ .format = DRM_FORMAT_NV24_10,		.depth = 0,  .num_planes = 2,
> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> +		  .hsub = 1, .vsub = 1, .is_yuv = true},
> +		{ .format = DRM_FORMAT_NV42_10,		.depth = 0,  .num_planes = 2,
> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> +		  .hsub = 1, .vsub = 1, .is_yuv = true},
>  		{ .format = DRM_FORMAT_P210,		.depth = 0,
>  		  .num_planes = 2, .char_per_block = { 2, 4, 0 },
>  		  .block_w = { 1, 0, 0 }, .block_h = { 1, 0, 0 }, .hsub = 2,
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 3feeaa3..08e2221 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -238,6 +238,20 @@ extern "C" {
>  #define DRM_FORMAT_NV42		fourcc_code('N', 'V', '4', '2') /* non-subsampled Cb:Cr plane */
>  
>  /*
> + * 2 plane YCbCr
> + * index 0 = Y plane, Y3:Y2:Y1:Y0 10:10:10:10
> + * index 1 = Cb:Cr plane, Cb3:Cr3:Cb2:Cr2:Cb1:Cr1:Cb0:Cr0 10:10:10:10:10:10:10:10
> + * or
> + * index 1 = Cr:Cb plane, Cr3:Cb3:Cr2:Cb2:Cr1:Cb1:Cr0:Cb0 10:10:10:10:10:10:10:10
> + */
> +#define DRM_FORMAT_NV12_10	fourcc_code('N', 'A', '1', '2') /* 2x2 subsampled Cr:Cb plane */
> +#define DRM_FORMAT_NV21_10	fourcc_code('N', 'A', '2', '1') /* 2x2 subsampled Cb:Cr plane */
> +#define DRM_FORMAT_NV16_10	fourcc_code('N', 'A', '1', '6') /* 2x1 subsampled Cr:Cb plane */
> +#define DRM_FORMAT_NV61_10	fourcc_code('N', 'A', '6', '1') /* 2x1 subsampled Cb:Cr plane */
> +#define DRM_FORMAT_NV24_10	fourcc_code('N', 'A', '2', '4') /* non-subsampled Cr:Cb plane */
> +#define DRM_FORMAT_NV42_10	fourcc_code('N', 'A', '4', '2') /* non-subsampled Cb:Cr plane */
> +
> +/*
>   * 2 plane YCbCr MSB aligned
>   * index 0 = Y plane, [15:0] Y:x [10:6] little endian
>   * index 1 = Cr:Cb plane, [31:0] Cr:x:Cb:x [10:6:10:6] little endian

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

* Re: [PATCH v2 1/3] drm: Add some new format DRM_FORMAT_NVXX_10
  2019-09-26  8:24 ` [PATCH v2 1/3] drm: Add some new format DRM_FORMAT_NVXX_10 Sandy Huang
  2019-09-29  3:26   ` [v2,1/3] " james qian wang (Arm Technology China)
@ 2019-09-30 10:48   ` Ville Syrjälä
  2019-10-08  2:40     ` sandy.huang
  2019-10-09 14:50   ` Daniel Vetter
  2 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2019-09-30 10:48 UTC (permalink / raw)
  To: Sandy Huang
  Cc: dri-devel, Maarten Lankhorst, Maxime Ripard, Sean Paul,
	David Airlie, Daniel Vetter, linux-kernel

On Thu, Sep 26, 2019 at 04:24:47PM +0800, Sandy Huang wrote:
> These new format is supported by some rockchip socs:
> 
> DRM_FORMAT_NV12_10/DRM_FORMAT_NV21_10
> DRM_FORMAT_NV16_10/DRM_FORMAT_NV61_10
> DRM_FORMAT_NV24_10/DRM_FORMAT_NV42_10
> 
> Signed-off-by: Sandy Huang <hjc@rock-chips.com>
> ---
>  drivers/gpu/drm/drm_fourcc.c  | 18 ++++++++++++++++++
>  include/uapi/drm/drm_fourcc.h | 14 ++++++++++++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index c630064..ccd78a3 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -261,6 +261,24 @@ const struct drm_format_info *__drm_format_info(u32 format)
>  		{ .format = DRM_FORMAT_P016,		.depth = 0,  .num_planes = 2,
>  		  .char_per_block = { 2, 4, 0 }, .block_w = { 1, 0, 0 }, .block_h = { 1, 0, 0 },
>  		  .hsub = 2, .vsub = 2, .is_yuv = true},
> +		{ .format = DRM_FORMAT_NV12_10,		.depth = 0,  .num_planes = 2,
> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> +		  .hsub = 2, .vsub = 2, .is_yuv = true},
> +		{ .format = DRM_FORMAT_NV21_10,		.depth = 0,  .num_planes = 2,
> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> +		  .hsub = 2, .vsub = 2, .is_yuv = true},
> +		{ .format = DRM_FORMAT_NV16_10,		.depth = 0,  .num_planes = 2,
> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> +		  .hsub = 2, .vsub = 1, .is_yuv = true},
> +		{ .format = DRM_FORMAT_NV61_10,		.depth = 0,  .num_planes = 2,
> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> +		  .hsub = 2, .vsub = 1, .is_yuv = true},
> +		{ .format = DRM_FORMAT_NV24_10,		.depth = 0,  .num_planes = 2,
> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> +		  .hsub = 1, .vsub = 1, .is_yuv = true},
> +		{ .format = DRM_FORMAT_NV42_10,		.depth = 0,  .num_planes = 2,
> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> +		  .hsub = 1, .vsub = 1, .is_yuv = true},
>  		{ .format = DRM_FORMAT_P210,		.depth = 0,
>  		  .num_planes = 2, .char_per_block = { 2, 4, 0 },
>  		  .block_w = { 1, 0, 0 }, .block_h = { 1, 0, 0 }, .hsub = 2,
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 3feeaa3..08e2221 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -238,6 +238,20 @@ extern "C" {
>  #define DRM_FORMAT_NV42		fourcc_code('N', 'V', '4', '2') /* non-subsampled Cb:Cr plane */
>  
>  /*
> + * 2 plane YCbCr
> + * index 0 = Y plane, Y3:Y2:Y1:Y0 10:10:10:10
> + * index 1 = Cb:Cr plane, Cb3:Cr3:Cb2:Cr2:Cb1:Cr1:Cb0:Cr0 10:10:10:10:10:10:10:10
> + * or
> + * index 1 = Cr:Cb plane, Cr3:Cb3:Cr2:Cb2:Cr1:Cb1:Cr0:Cb0 10:10:10:10:10:10:10:10

So now you're defining it as some kind of byte aligned block.
With that specifying endianness would now make sense since
otherwise this tells us absolutely nothing about the memory
layout.

So I'd either do that, or go back to not specifying anything and
use some weasel words like "mamory layout is implementation defined"
which of course means no one can use it for anything that involves
any kind of cross vendor stuff.

> + */
> +#define DRM_FORMAT_NV12_10	fourcc_code('N', 'A', '1', '2') /* 2x2 subsampled Cr:Cb plane */
> +#define DRM_FORMAT_NV21_10	fourcc_code('N', 'A', '2', '1') /* 2x2 subsampled Cb:Cr plane */
> +#define DRM_FORMAT_NV16_10	fourcc_code('N', 'A', '1', '6') /* 2x1 subsampled Cr:Cb plane */
> +#define DRM_FORMAT_NV61_10	fourcc_code('N', 'A', '6', '1') /* 2x1 subsampled Cb:Cr plane */
> +#define DRM_FORMAT_NV24_10	fourcc_code('N', 'A', '2', '4') /* non-subsampled Cr:Cb plane */
> +#define DRM_FORMAT_NV42_10	fourcc_code('N', 'A', '4', '2') /* non-subsampled Cb:Cr plane */
> +
> +/*
>   * 2 plane YCbCr MSB aligned
>   * index 0 = Y plane, [15:0] Y:x [10:6] little endian
>   * index 1 = Cr:Cb plane, [31:0] Cr:x:Cb:x [10:6:10:6] little endian
> -- 
> 2.7.4
> 
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel

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

* Re: [PATCH v2 1/3] drm: Add some new format DRM_FORMAT_NVXX_10
  2019-09-30 10:48   ` [PATCH v2 1/3] " Ville Syrjälä
@ 2019-10-08  2:40     ` sandy.huang
  2019-10-08 11:33       ` Ville Syrjälä
  0 siblings, 1 reply; 16+ messages in thread
From: sandy.huang @ 2019-10-08  2:40 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: dri-devel, Maarten Lankhorst, Maxime Ripard, Sean Paul,
	David Airlie, Daniel Vetter, linux-kernel

Hi ville syrjala,

在 2019/9/30 下午6:48, Ville Syrjälä 写道:
> On Thu, Sep 26, 2019 at 04:24:47PM +0800, Sandy Huang wrote:
>> These new format is supported by some rockchip socs:
>>
>> DRM_FORMAT_NV12_10/DRM_FORMAT_NV21_10
>> DRM_FORMAT_NV16_10/DRM_FORMAT_NV61_10
>> DRM_FORMAT_NV24_10/DRM_FORMAT_NV42_10
>>
>> Signed-off-by: Sandy Huang <hjc@rock-chips.com>
>> ---
>>   drivers/gpu/drm/drm_fourcc.c  | 18 ++++++++++++++++++
>>   include/uapi/drm/drm_fourcc.h | 14 ++++++++++++++
>>   2 files changed, 32 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
>> index c630064..ccd78a3 100644
>> --- a/drivers/gpu/drm/drm_fourcc.c
>> +++ b/drivers/gpu/drm/drm_fourcc.c
>> @@ -261,6 +261,24 @@ const struct drm_format_info *__drm_format_info(u32 format)
>>   		{ .format = DRM_FORMAT_P016,		.depth = 0,  .num_planes = 2,
>>   		  .char_per_block = { 2, 4, 0 }, .block_w = { 1, 0, 0 }, .block_h = { 1, 0, 0 },
>>   		  .hsub = 2, .vsub = 2, .is_yuv = true},
>> +		{ .format = DRM_FORMAT_NV12_10,		.depth = 0,  .num_planes = 2,
>> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
>> +		  .hsub = 2, .vsub = 2, .is_yuv = true},
>> +		{ .format = DRM_FORMAT_NV21_10,		.depth = 0,  .num_planes = 2,
>> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
>> +		  .hsub = 2, .vsub = 2, .is_yuv = true},
>> +		{ .format = DRM_FORMAT_NV16_10,		.depth = 0,  .num_planes = 2,
>> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
>> +		  .hsub = 2, .vsub = 1, .is_yuv = true},
>> +		{ .format = DRM_FORMAT_NV61_10,		.depth = 0,  .num_planes = 2,
>> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
>> +		  .hsub = 2, .vsub = 1, .is_yuv = true},
>> +		{ .format = DRM_FORMAT_NV24_10,		.depth = 0,  .num_planes = 2,
>> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
>> +		  .hsub = 1, .vsub = 1, .is_yuv = true},
>> +		{ .format = DRM_FORMAT_NV42_10,		.depth = 0,  .num_planes = 2,
>> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
>> +		  .hsub = 1, .vsub = 1, .is_yuv = true},
>>   		{ .format = DRM_FORMAT_P210,		.depth = 0,
>>   		  .num_planes = 2, .char_per_block = { 2, 4, 0 },
>>   		  .block_w = { 1, 0, 0 }, .block_h = { 1, 0, 0 }, .hsub = 2,
>> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
>> index 3feeaa3..08e2221 100644
>> --- a/include/uapi/drm/drm_fourcc.h
>> +++ b/include/uapi/drm/drm_fourcc.h
>> @@ -238,6 +238,20 @@ extern "C" {
>>   #define DRM_FORMAT_NV42		fourcc_code('N', 'V', '4', '2') /* non-subsampled Cb:Cr plane */
>>   
>>   /*
>> + * 2 plane YCbCr
>> + * index 0 = Y plane, Y3:Y2:Y1:Y0 10:10:10:10
>> + * index 1 = Cb:Cr plane, Cb3:Cr3:Cb2:Cr2:Cb1:Cr1:Cb0:Cr0 10:10:10:10:10:10:10:10
>> + * or
>> + * index 1 = Cr:Cb plane, Cr3:Cb3:Cr2:Cb2:Cr1:Cb1:Cr0:Cb0 10:10:10:10:10:10:10:10
> So now you're defining it as some kind of byte aligned block.
> With that specifying endianness would now make sense since
> otherwise this tells us absolutely nothing about the memory
> layout.
>
> So I'd either do that, or go back to not specifying anything and
> use some weasel words like "mamory layout is implementation defined"
> which of course means no one can use it for anything that involves
> any kind of cross vendor stuff.
/*
  * 2 plane YCbCr
  * index 0 = Y plane, [39: 0] Y3:Y2:Y1:Y0 10:10:10:10  little endian
  * index 1 = Cb:Cr plane, [79: 0] Cb3:Cr3:Cb2:Cr2:Cb1:Cr1:Cb0:Cr0 
10:10:10:10:10:10:10:10  little endian
  * or
  * index 1 = Cr:Cb plane, [79: 0] Cr3:Cb3:Cr2:Cb2:Cr1:Cb1:Cr0:Cb0 
10:10:10:10:10:10:10:10  little endian
  */

Is this description ok?

>> + */
>> +#define DRM_FORMAT_NV12_10	fourcc_code('N', 'A', '1', '2') /* 2x2 subsampled Cr:Cb plane */
>> +#define DRM_FORMAT_NV21_10	fourcc_code('N', 'A', '2', '1') /* 2x2 subsampled Cb:Cr plane */
>> +#define DRM_FORMAT_NV16_10	fourcc_code('N', 'A', '1', '6') /* 2x1 subsampled Cr:Cb plane */
>> +#define DRM_FORMAT_NV61_10	fourcc_code('N', 'A', '6', '1') /* 2x1 subsampled Cb:Cr plane */
>> +#define DRM_FORMAT_NV24_10	fourcc_code('N', 'A', '2', '4') /* non-subsampled Cr:Cb plane */
>> +#define DRM_FORMAT_NV42_10	fourcc_code('N', 'A', '4', '2') /* non-subsampled Cb:Cr plane */
>> +
>> +/*
>>    * 2 plane YCbCr MSB aligned
>>    * index 0 = Y plane, [15:0] Y:x [10:6] little endian
>>    * index 1 = Cr:Cb plane, [31:0] Cr:x:Cb:x [10:6:10:6] little endian
>> -- 
>> 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] 16+ messages in thread

* Re: [PATCH v2 1/3] drm: Add some new format DRM_FORMAT_NVXX_10
  2019-10-08  2:40     ` sandy.huang
@ 2019-10-08 11:33       ` Ville Syrjälä
  2019-10-08 11:49         ` sandy.huang
  0 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2019-10-08 11:33 UTC (permalink / raw)
  To: sandy.huang
  Cc: dri-devel, Maarten Lankhorst, Maxime Ripard, Sean Paul,
	David Airlie, Daniel Vetter, linux-kernel

On Tue, Oct 08, 2019 at 10:40:20AM +0800, sandy.huang wrote:
> Hi ville syrjala,
> 
> 在 2019/9/30 下午6:48, Ville Syrjälä 写道:
> > On Thu, Sep 26, 2019 at 04:24:47PM +0800, Sandy Huang wrote:
> >> These new format is supported by some rockchip socs:
> >>
> >> DRM_FORMAT_NV12_10/DRM_FORMAT_NV21_10
> >> DRM_FORMAT_NV16_10/DRM_FORMAT_NV61_10
> >> DRM_FORMAT_NV24_10/DRM_FORMAT_NV42_10
> >>
> >> Signed-off-by: Sandy Huang <hjc@rock-chips.com>
> >> ---
> >>   drivers/gpu/drm/drm_fourcc.c  | 18 ++++++++++++++++++
> >>   include/uapi/drm/drm_fourcc.h | 14 ++++++++++++++
> >>   2 files changed, 32 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> >> index c630064..ccd78a3 100644
> >> --- a/drivers/gpu/drm/drm_fourcc.c
> >> +++ b/drivers/gpu/drm/drm_fourcc.c
> >> @@ -261,6 +261,24 @@ const struct drm_format_info *__drm_format_info(u32 format)
> >>   		{ .format = DRM_FORMAT_P016,		.depth = 0,  .num_planes = 2,
> >>   		  .char_per_block = { 2, 4, 0 }, .block_w = { 1, 0, 0 }, .block_h = { 1, 0, 0 },
> >>   		  .hsub = 2, .vsub = 2, .is_yuv = true},
> >> +		{ .format = DRM_FORMAT_NV12_10,		.depth = 0,  .num_planes = 2,
> >> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> >> +		  .hsub = 2, .vsub = 2, .is_yuv = true},
> >> +		{ .format = DRM_FORMAT_NV21_10,		.depth = 0,  .num_planes = 2,
> >> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> >> +		  .hsub = 2, .vsub = 2, .is_yuv = true},
> >> +		{ .format = DRM_FORMAT_NV16_10,		.depth = 0,  .num_planes = 2,
> >> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> >> +		  .hsub = 2, .vsub = 1, .is_yuv = true},
> >> +		{ .format = DRM_FORMAT_NV61_10,		.depth = 0,  .num_planes = 2,
> >> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> >> +		  .hsub = 2, .vsub = 1, .is_yuv = true},
> >> +		{ .format = DRM_FORMAT_NV24_10,		.depth = 0,  .num_planes = 2,
> >> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> >> +		  .hsub = 1, .vsub = 1, .is_yuv = true},
> >> +		{ .format = DRM_FORMAT_NV42_10,		.depth = 0,  .num_planes = 2,
> >> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> >> +		  .hsub = 1, .vsub = 1, .is_yuv = true},
> >>   		{ .format = DRM_FORMAT_P210,		.depth = 0,
> >>   		  .num_planes = 2, .char_per_block = { 2, 4, 0 },
> >>   		  .block_w = { 1, 0, 0 }, .block_h = { 1, 0, 0 }, .hsub = 2,
> >> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> >> index 3feeaa3..08e2221 100644
> >> --- a/include/uapi/drm/drm_fourcc.h
> >> +++ b/include/uapi/drm/drm_fourcc.h
> >> @@ -238,6 +238,20 @@ extern "C" {
> >>   #define DRM_FORMAT_NV42		fourcc_code('N', 'V', '4', '2') /* non-subsampled Cb:Cr plane */
> >>   
> >>   /*
> >> + * 2 plane YCbCr
> >> + * index 0 = Y plane, Y3:Y2:Y1:Y0 10:10:10:10
> >> + * index 1 = Cb:Cr plane, Cb3:Cr3:Cb2:Cr2:Cb1:Cr1:Cb0:Cr0 10:10:10:10:10:10:10:10
> >> + * or
> >> + * index 1 = Cr:Cb plane, Cr3:Cb3:Cr2:Cb2:Cr1:Cb1:Cr0:Cb0 10:10:10:10:10:10:10:10
> > So now you're defining it as some kind of byte aligned block.
> > With that specifying endianness would now make sense since
> > otherwise this tells us absolutely nothing about the memory
> > layout.
> >
> > So I'd either do that, or go back to not specifying anything and
> > use some weasel words like "mamory layout is implementation defined"
> > which of course means no one can use it for anything that involves
> > any kind of cross vendor stuff.
> /*
>   * 2 plane YCbCr
>   * index 0 = Y plane, [39: 0] Y3:Y2:Y1:Y0 10:10:10:10  little endian
>   * index 1 = Cb:Cr plane, [79: 0] Cb3:Cr3:Cb2:Cr2:Cb1:Cr1:Cb0:Cr0 
> 10:10:10:10:10:10:10:10  little endian
>   * or
>   * index 1 = Cr:Cb plane, [79: 0] Cr3:Cb3:Cr2:Cb2:Cr1:Cb1:Cr0:Cb0 
> 10:10:10:10:10:10:10:10  little endian
>   */
> 
> Is this description ok?

Seems OK to me, if it actually describes the format correctly.

Though I'm not sure why the CbCr is defines as an 80bit block
and Y has a 40bit block. 40bits should be enough for CbCr as well.

> 
> >> + */
> >> +#define DRM_FORMAT_NV12_10	fourcc_code('N', 'A', '1', '2') /* 2x2 subsampled Cr:Cb plane */
> >> +#define DRM_FORMAT_NV21_10	fourcc_code('N', 'A', '2', '1') /* 2x2 subsampled Cb:Cr plane */
> >> +#define DRM_FORMAT_NV16_10	fourcc_code('N', 'A', '1', '6') /* 2x1 subsampled Cr:Cb plane */
> >> +#define DRM_FORMAT_NV61_10	fourcc_code('N', 'A', '6', '1') /* 2x1 subsampled Cb:Cr plane */
> >> +#define DRM_FORMAT_NV24_10	fourcc_code('N', 'A', '2', '4') /* non-subsampled Cr:Cb plane */
> >> +#define DRM_FORMAT_NV42_10	fourcc_code('N', 'A', '4', '2') /* non-subsampled Cb:Cr plane */
> >> +
> >> +/*
> >>    * 2 plane YCbCr MSB aligned
> >>    * index 0 = Y plane, [15:0] Y:x [10:6] little endian
> >>    * index 1 = Cr:Cb plane, [31:0] Cr:x:Cb:x [10:6:10:6] little endian
> >> -- 
> >> 2.7.4
> >>
> >>
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Ville Syrjälä
Intel

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

* Re: [PATCH v2 1/3] drm: Add some new format DRM_FORMAT_NVXX_10
  2019-10-08 11:33       ` Ville Syrjälä
@ 2019-10-08 11:49         ` sandy.huang
  2019-10-11  3:35           ` sandy.huang
  0 siblings, 1 reply; 16+ messages in thread
From: sandy.huang @ 2019-10-08 11:49 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: dri-devel, Maarten Lankhorst, Maxime Ripard, Sean Paul,
	David Airlie, Daniel Vetter, linux-kernel


在 2019/10/8 下午7:33, Ville Syrjälä 写道:
> On Tue, Oct 08, 2019 at 10:40:20AM +0800, sandy.huang wrote:
>> Hi ville syrjala,
>>
>> 在 2019/9/30 下午6:48, Ville Syrjälä 写道:
>>> On Thu, Sep 26, 2019 at 04:24:47PM +0800, Sandy Huang wrote:
>>>> These new format is supported by some rockchip socs:
>>>>
>>>> DRM_FORMAT_NV12_10/DRM_FORMAT_NV21_10
>>>> DRM_FORMAT_NV16_10/DRM_FORMAT_NV61_10
>>>> DRM_FORMAT_NV24_10/DRM_FORMAT_NV42_10
>>>>
>>>> Signed-off-by: Sandy Huang <hjc@rock-chips.com>
>>>> ---
>>>>    drivers/gpu/drm/drm_fourcc.c  | 18 ++++++++++++++++++
>>>>    include/uapi/drm/drm_fourcc.h | 14 ++++++++++++++
>>>>    2 files changed, 32 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
>>>> index c630064..ccd78a3 100644
>>>> --- a/drivers/gpu/drm/drm_fourcc.c
>>>> +++ b/drivers/gpu/drm/drm_fourcc.c
>>>> @@ -261,6 +261,24 @@ const struct drm_format_info *__drm_format_info(u32 format)
>>>>    		{ .format = DRM_FORMAT_P016,		.depth = 0,  .num_planes = 2,
>>>>    		  .char_per_block = { 2, 4, 0 }, .block_w = { 1, 0, 0 }, .block_h = { 1, 0, 0 },
>>>>    		  .hsub = 2, .vsub = 2, .is_yuv = true},
>>>> +		{ .format = DRM_FORMAT_NV12_10,		.depth = 0,  .num_planes = 2,
>>>> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
>>>> +		  .hsub = 2, .vsub = 2, .is_yuv = true},
>>>> +		{ .format = DRM_FORMAT_NV21_10,		.depth = 0,  .num_planes = 2,
>>>> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
>>>> +		  .hsub = 2, .vsub = 2, .is_yuv = true},
>>>> +		{ .format = DRM_FORMAT_NV16_10,		.depth = 0,  .num_planes = 2,
>>>> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
>>>> +		  .hsub = 2, .vsub = 1, .is_yuv = true},
>>>> +		{ .format = DRM_FORMAT_NV61_10,		.depth = 0,  .num_planes = 2,
>>>> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
>>>> +		  .hsub = 2, .vsub = 1, .is_yuv = true},
>>>> +		{ .format = DRM_FORMAT_NV24_10,		.depth = 0,  .num_planes = 2,
>>>> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
>>>> +		  .hsub = 1, .vsub = 1, .is_yuv = true},
>>>> +		{ .format = DRM_FORMAT_NV42_10,		.depth = 0,  .num_planes = 2,
>>>> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
>>>> +		  .hsub = 1, .vsub = 1, .is_yuv = true},
>>>>    		{ .format = DRM_FORMAT_P210,		.depth = 0,
>>>>    		  .num_planes = 2, .char_per_block = { 2, 4, 0 },
>>>>    		  .block_w = { 1, 0, 0 }, .block_h = { 1, 0, 0 }, .hsub = 2,
>>>> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
>>>> index 3feeaa3..08e2221 100644
>>>> --- a/include/uapi/drm/drm_fourcc.h
>>>> +++ b/include/uapi/drm/drm_fourcc.h
>>>> @@ -238,6 +238,20 @@ extern "C" {
>>>>    #define DRM_FORMAT_NV42		fourcc_code('N', 'V', '4', '2') /* non-subsampled Cb:Cr plane */
>>>>    
>>>>    /*
>>>> + * 2 plane YCbCr
>>>> + * index 0 = Y plane, Y3:Y2:Y1:Y0 10:10:10:10
>>>> + * index 1 = Cb:Cr plane, Cb3:Cr3:Cb2:Cr2:Cb1:Cr1:Cb0:Cr0 10:10:10:10:10:10:10:10
>>>> + * or
>>>> + * index 1 = Cr:Cb plane, Cr3:Cb3:Cr2:Cb2:Cr1:Cb1:Cr0:Cb0 10:10:10:10:10:10:10:10
>>> So now you're defining it as some kind of byte aligned block.
>>> With that specifying endianness would now make sense since
>>> otherwise this tells us absolutely nothing about the memory
>>> layout.
>>>
>>> So I'd either do that, or go back to not specifying anything and
>>> use some weasel words like "mamory layout is implementation defined"
>>> which of course means no one can use it for anything that involves
>>> any kind of cross vendor stuff.
>> /*
>>    * 2 plane YCbCr
>>    * index 0 = Y plane, [39: 0] Y3:Y2:Y1:Y0 10:10:10:10  little endian
>>    * index 1 = Cb:Cr plane, [79: 0] Cb3:Cr3:Cb2:Cr2:Cb1:Cr1:Cb0:Cr0
>> 10:10:10:10:10:10:10:10  little endian
>>    * or
>>    * index 1 = Cr:Cb plane, [79: 0] Cr3:Cb3:Cr2:Cb2:Cr1:Cb1:Cr0:Cb0
>> 10:10:10:10:10:10:10:10  little endian
>>    */
>>
>> Is this description ok?
> Seems OK to me, if it actually describes the format correctly.
>
> Though I'm not sure why the CbCr is defines as an 80bit block
> and Y has a 40bit block. 40bits should be enough for CbCr as well.
>
well, this is taken into account yuv444,  4 y point corresponding with 4 
uv point.

if only describes the layout memory, here can change to 40bit block.

thanks.

>>>> + */
>>>> +#define DRM_FORMAT_NV12_10	fourcc_code('N', 'A', '1', '2') /* 2x2 subsampled Cr:Cb plane */
>>>> +#define DRM_FORMAT_NV21_10	fourcc_code('N', 'A', '2', '1') /* 2x2 subsampled Cb:Cr plane */
>>>> +#define DRM_FORMAT_NV16_10	fourcc_code('N', 'A', '1', '6') /* 2x1 subsampled Cr:Cb plane */
>>>> +#define DRM_FORMAT_NV61_10	fourcc_code('N', 'A', '6', '1') /* 2x1 subsampled Cb:Cr plane */
>>>> +#define DRM_FORMAT_NV24_10	fourcc_code('N', 'A', '2', '4') /* non-subsampled Cr:Cb plane */
>>>> +#define DRM_FORMAT_NV42_10	fourcc_code('N', 'A', '4', '2') /* non-subsampled Cb:Cr plane */
>>>> +
>>>> +/*
>>>>     * 2 plane YCbCr MSB aligned
>>>>     * index 0 = Y plane, [15:0] Y:x [10:6] little endian
>>>>     * index 1 = Cr:Cb plane, [31:0] Cr:x:Cb:x [10:6:10:6] little endian
>>>> -- 
>>>> 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] 16+ messages in thread

* Re: [PATCH v2 1/3] drm: Add some new format DRM_FORMAT_NVXX_10
  2019-09-26  8:24 ` [PATCH v2 1/3] drm: Add some new format DRM_FORMAT_NVXX_10 Sandy Huang
  2019-09-29  3:26   ` [v2,1/3] " james qian wang (Arm Technology China)
  2019-09-30 10:48   ` [PATCH v2 1/3] " Ville Syrjälä
@ 2019-10-09 14:50   ` Daniel Vetter
  2019-11-08 15:08     ` Heiko Stübner
  2 siblings, 1 reply; 16+ messages in thread
From: Daniel Vetter @ 2019-10-09 14:50 UTC (permalink / raw)
  To: Sandy Huang
  Cc: dri-devel, Maarten Lankhorst, Maxime Ripard, Sean Paul,
	David Airlie, Daniel Vetter, heiko, linux-kernel

On Thu, Sep 26, 2019 at 04:24:47PM +0800, Sandy Huang wrote:
> These new format is supported by some rockchip socs:
> 
> DRM_FORMAT_NV12_10/DRM_FORMAT_NV21_10
> DRM_FORMAT_NV16_10/DRM_FORMAT_NV61_10
> DRM_FORMAT_NV24_10/DRM_FORMAT_NV42_10
> 
> Signed-off-by: Sandy Huang <hjc@rock-chips.com>
> ---
>  drivers/gpu/drm/drm_fourcc.c  | 18 ++++++++++++++++++
>  include/uapi/drm/drm_fourcc.h | 14 ++++++++++++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index c630064..ccd78a3 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -261,6 +261,24 @@ const struct drm_format_info *__drm_format_info(u32 format)
>  		{ .format = DRM_FORMAT_P016,		.depth = 0,  .num_planes = 2,
>  		  .char_per_block = { 2, 4, 0 }, .block_w = { 1, 0, 0 }, .block_h = { 1, 0, 0 },
>  		  .hsub = 2, .vsub = 2, .is_yuv = true},
> +		{ .format = DRM_FORMAT_NV12_10,		.depth = 0,  .num_planes = 2,
> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> +		  .hsub = 2, .vsub = 2, .is_yuv = true},
> +		{ .format = DRM_FORMAT_NV21_10,		.depth = 0,  .num_planes = 2,
> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> +		  .hsub = 2, .vsub = 2, .is_yuv = true},
> +		{ .format = DRM_FORMAT_NV16_10,		.depth = 0,  .num_planes = 2,
> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> +		  .hsub = 2, .vsub = 1, .is_yuv = true},
> +		{ .format = DRM_FORMAT_NV61_10,		.depth = 0,  .num_planes = 2,
> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> +		  .hsub = 2, .vsub = 1, .is_yuv = true},
> +		{ .format = DRM_FORMAT_NV24_10,		.depth = 0,  .num_planes = 2,
> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> +		  .hsub = 1, .vsub = 1, .is_yuv = true},
> +		{ .format = DRM_FORMAT_NV42_10,		.depth = 0,  .num_planes = 2,
> +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> +		  .hsub = 1, .vsub = 1, .is_yuv = true},
>  		{ .format = DRM_FORMAT_P210,		.depth = 0,
>  		  .num_planes = 2, .char_per_block = { 2, 4, 0 },
>  		  .block_w = { 1, 0, 0 }, .block_h = { 1, 0, 0 }, .hsub = 2,

Yup this is what I had in mind with using the block stuff to describe your
new 10bit yuv formats. Thanks for respining.

Once we've nailed the exact bit description of the format precisely this
can be merged imo.
-Daniel

> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 3feeaa3..08e2221 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -238,6 +238,20 @@ extern "C" {
>  #define DRM_FORMAT_NV42		fourcc_code('N', 'V', '4', '2') /* non-subsampled Cb:Cr plane */
>  
>  /*
> + * 2 plane YCbCr
> + * index 0 = Y plane, Y3:Y2:Y1:Y0 10:10:10:10
> + * index 1 = Cb:Cr plane, Cb3:Cr3:Cb2:Cr2:Cb1:Cr1:Cb0:Cr0 10:10:10:10:10:10:10:10
> + * or
> + * index 1 = Cr:Cb plane, Cr3:Cb3:Cr2:Cb2:Cr1:Cb1:Cr0:Cb0 10:10:10:10:10:10:10:10
> + */
> +#define DRM_FORMAT_NV12_10	fourcc_code('N', 'A', '1', '2') /* 2x2 subsampled Cr:Cb plane */
> +#define DRM_FORMAT_NV21_10	fourcc_code('N', 'A', '2', '1') /* 2x2 subsampled Cb:Cr plane */
> +#define DRM_FORMAT_NV16_10	fourcc_code('N', 'A', '1', '6') /* 2x1 subsampled Cr:Cb plane */
> +#define DRM_FORMAT_NV61_10	fourcc_code('N', 'A', '6', '1') /* 2x1 subsampled Cb:Cr plane */
> +#define DRM_FORMAT_NV24_10	fourcc_code('N', 'A', '2', '4') /* non-subsampled Cr:Cb plane */
> +#define DRM_FORMAT_NV42_10	fourcc_code('N', 'A', '4', '2') /* non-subsampled Cb:Cr plane */
> +
> +/*
>   * 2 plane YCbCr MSB aligned
>   * index 0 = Y plane, [15:0] Y:x [10:6] little endian
>   * index 1 = Cr:Cb plane, [31:0] Cr:x:Cb:x [10:6:10:6] little endian
> -- 
> 2.7.4
> 
> 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

* Re: [PATCH v2 1/3] drm: Add some new format DRM_FORMAT_NVXX_10
  2019-10-08 11:49         ` sandy.huang
@ 2019-10-11  3:35           ` sandy.huang
       [not found]             ` <20191011064433.GA18503@jamwan02-TSP300>
  0 siblings, 1 reply; 16+ messages in thread
From: sandy.huang @ 2019-10-11  3:35 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: Maxime Ripard, linux-kernel, dri-devel, David Airlie, Sean Paul,
	james.qian.wang

Hi james.qian.wang,

     Thank for you remind, fou some unknow reason, i miss the the mail 
from you:(, i get this message from 
https://patchwork.kernel.org/patch/11161937/

sorry about that.

     About the format block describe, I also found some unreasonable,  
this format need 2 line aligned, so the block_h need to sed as 2, and 
the char_per_block need set as w * h * 10 for y plane, and w * h * 2 * 
10 for uv plane, so the following describe maybe more correct, thanks.

         { .format = DRM_FORMAT_NV12_10,        .depth = 0, .num_planes = 2,
           .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, 
.block_h = { 2, 2, 0 },
           .hsub = 2, .vsub = 2, .is_yuv = true},
         { .format = DRM_FORMAT_NV21_10,        .depth = 0, .num_planes = 2,
           .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, 
.block_h = { 2, 2, 0 },
           .hsub = 2, .vsub = 2, .is_yuv = true},
         { .format = DRM_FORMAT_NV16_10,        .depth = 0, .num_planes = 2,
           .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, 
.block_h = { 2, 2, 0 },
           .hsub = 2, .vsub = 1, .is_yuv = true},
         { .format = DRM_FORMAT_NV61_10,        .depth = 0, .num_planes = 2,
           .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, 
.block_h = { 2, 2, 0 },
           .hsub = 2, .vsub = 1, .is_yuv = true},
         { .format = DRM_FORMAT_NV24_10,        .depth = 0, .num_planes = 2,
           .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, 
.block_h = { 2, 2, 0 },
           .hsub = 1, .vsub = 1, .is_yuv = true},
         { .format = DRM_FORMAT_NV42_10,        .depth = 0, .num_planes = 2,
           .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, 
.block_h = { 2, 2, 0 },
           .hsub = 1, .vsub = 1, .is_yuv = true},


 > >          { .format = DRM_FORMAT_P016,        .depth = 0,  
.num_planes = 2,
 > >            .char_per_block = { 2, 4, 0 }, .block_w = { 1, 0, 0 }, 
.block_h = { 1, 0, 0 },
 > >            .hsub = 2, .vsub = 2, .is_yuv = true},
 > > +        { .format = DRM_FORMAT_NV12_10,        .depth = 0,  
.num_planes = 2,
 > > +          .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, 
.block_h = { 4, 4, 0 },

 > Hi Sandy:
 > Their is a problem here for char_per_block size of plane[0]:
 > Since: 5 * 8 != 4 * 4 * 10;
 > Seems you mis-set the block_w/h, per your block size the block is 
2x2, and it should be:
 >  .char_per_block = { 5, 10, 0 }, .block_w = { 2, 2, 0 }, .block_h = { 
2, 2, 0 },

 > Best Regards:
 > James






在 2019/10/8 下午7:49, sandy.huang 写道:
>
> 在 2019/10/8 下午7:33, Ville Syrjälä 写道:
>> On Tue, Oct 08, 2019 at 10:40:20AM +0800, sandy.huang wrote:
>>> Hi ville syrjala,
>>>
>>> 在 2019/9/30 下午6:48, Ville Syrjälä 写道:
>>>> On Thu, Sep 26, 2019 at 04:24:47PM +0800, Sandy Huang wrote:
>>>>> These new format is supported by some rockchip socs:
>>>>>
>>>>> DRM_FORMAT_NV12_10/DRM_FORMAT_NV21_10
>>>>> DRM_FORMAT_NV16_10/DRM_FORMAT_NV61_10
>>>>> DRM_FORMAT_NV24_10/DRM_FORMAT_NV42_10
>>>>>
>>>>> Signed-off-by: Sandy Huang <hjc@rock-chips.com>
>>>>> ---
>>>>>    drivers/gpu/drm/drm_fourcc.c  | 18 ++++++++++++++++++
>>>>>    include/uapi/drm/drm_fourcc.h | 14 ++++++++++++++
>>>>>    2 files changed, 32 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/drm_fourcc.c 
>>>>> b/drivers/gpu/drm/drm_fourcc.c
>>>>> index c630064..ccd78a3 100644
>>>>> --- a/drivers/gpu/drm/drm_fourcc.c
>>>>> +++ b/drivers/gpu/drm/drm_fourcc.c
>>>>> @@ -261,6 +261,24 @@ const struct drm_format_info 
>>>>> *__drm_format_info(u32 format)
>>>>>            { .format = DRM_FORMAT_P016,        .depth = 0,  
>>>>> .num_planes = 2,
>>>>>              .char_per_block = { 2, 4, 0 }, .block_w = { 1, 0, 0 
>>>>> }, .block_h = { 1, 0, 0 },
>>>>>              .hsub = 2, .vsub = 2, .is_yuv = true},
>>>>> +        { .format = DRM_FORMAT_NV12_10,        .depth = 0,  
>>>>> .num_planes = 2,
>>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, 
>>>>> .block_h = { 4, 4, 0 },
>>>>> +          .hsub = 2, .vsub = 2, .is_yuv = true},
>>>>> +        { .format = DRM_FORMAT_NV21_10,        .depth = 0,  
>>>>> .num_planes = 2,
>>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, 
>>>>> .block_h = { 4, 4, 0 },
>>>>> +          .hsub = 2, .vsub = 2, .is_yuv = true},
>>>>> +        { .format = DRM_FORMAT_NV16_10,        .depth = 0,  
>>>>> .num_planes = 2,
>>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, 
>>>>> .block_h = { 4, 4, 0 },
>>>>> +          .hsub = 2, .vsub = 1, .is_yuv = true},
>>>>> +        { .format = DRM_FORMAT_NV61_10,        .depth = 0,  
>>>>> .num_planes = 2,
>>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, 
>>>>> .block_h = { 4, 4, 0 },
>>>>> +          .hsub = 2, .vsub = 1, .is_yuv = true},
>>>>> +        { .format = DRM_FORMAT_NV24_10,        .depth = 0,  
>>>>> .num_planes = 2,
>>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, 
>>>>> .block_h = { 4, 4, 0 },
>>>>> +          .hsub = 1, .vsub = 1, .is_yuv = true},
>>>>> +        { .format = DRM_FORMAT_NV42_10,        .depth = 0,  
>>>>> .num_planes = 2,
>>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, 
>>>>> .block_h = { 4, 4, 0 },
>>>>> +          .hsub = 1, .vsub = 1, .is_yuv = true},
>>>>>            { .format = DRM_FORMAT_P210,        .depth = 0,
>>>>>              .num_planes = 2, .char_per_block = { 2, 4, 0 },
>>>>>              .block_w = { 1, 0, 0 }, .block_h = { 1, 0, 0 }, .hsub 
>>>>> = 2,
>>>>> diff --git a/include/uapi/drm/drm_fourcc.h 
>>>>> b/include/uapi/drm/drm_fourcc.h
>>>>> index 3feeaa3..08e2221 100644
>>>>> --- a/include/uapi/drm/drm_fourcc.h
>>>>> +++ b/include/uapi/drm/drm_fourcc.h
>>>>> @@ -238,6 +238,20 @@ extern "C" {
>>>>>    #define DRM_FORMAT_NV42        fourcc_code('N', 'V', '4', '2') 
>>>>> /* non-subsampled Cb:Cr plane */
>>>>>       /*
>>>>> + * 2 plane YCbCr
>>>>> + * index 0 = Y plane, Y3:Y2:Y1:Y0 10:10:10:10
>>>>> + * index 1 = Cb:Cr plane, Cb3:Cr3:Cb2:Cr2:Cb1:Cr1:Cb0:Cr0 
>>>>> 10:10:10:10:10:10:10:10
>>>>> + * or
>>>>> + * index 1 = Cr:Cb plane, Cr3:Cb3:Cr2:Cb2:Cr1:Cb1:Cr0:Cb0 
>>>>> 10:10:10:10:10:10:10:10
>>>> So now you're defining it as some kind of byte aligned block.
>>>> With that specifying endianness would now make sense since
>>>> otherwise this tells us absolutely nothing about the memory
>>>> layout.
>>>>
>>>> So I'd either do that, or go back to not specifying anything and
>>>> use some weasel words like "mamory layout is implementation defined"
>>>> which of course means no one can use it for anything that involves
>>>> any kind of cross vendor stuff.
>>> /*
>>>    * 2 plane YCbCr
>>>    * index 0 = Y plane, [39: 0] Y3:Y2:Y1:Y0 10:10:10:10 little endian
>>>    * index 1 = Cb:Cr plane, [79: 0] Cb3:Cr3:Cb2:Cr2:Cb1:Cr1:Cb0:Cr0
>>> 10:10:10:10:10:10:10:10  little endian
>>>    * or
>>>    * index 1 = Cr:Cb plane, [79: 0] Cr3:Cb3:Cr2:Cb2:Cr1:Cb1:Cr0:Cb0
>>> 10:10:10:10:10:10:10:10  little endian
>>>    */
>>>
>>> Is this description ok?
>> Seems OK to me, if it actually describes the format correctly.
>>
>> Though I'm not sure why the CbCr is defines as an 80bit block
>> and Y has a 40bit block. 40bits should be enough for CbCr as well.
>>
> well, this is taken into account yuv444,  4 y point corresponding with 
> 4 uv point.
>
> if only describes the layout memory, here can change to 40bit block.
>
> thanks.
>
>>>>> + */
>>>>> +#define DRM_FORMAT_NV12_10    fourcc_code('N', 'A', '1', '2') /* 
>>>>> 2x2 subsampled Cr:Cb plane */
>>>>> +#define DRM_FORMAT_NV21_10    fourcc_code('N', 'A', '2', '1') /* 
>>>>> 2x2 subsampled Cb:Cr plane */
>>>>> +#define DRM_FORMAT_NV16_10    fourcc_code('N', 'A', '1', '6') /* 
>>>>> 2x1 subsampled Cr:Cb plane */
>>>>> +#define DRM_FORMAT_NV61_10    fourcc_code('N', 'A', '6', '1') /* 
>>>>> 2x1 subsampled Cb:Cr plane */
>>>>> +#define DRM_FORMAT_NV24_10    fourcc_code('N', 'A', '2', '4') /* 
>>>>> non-subsampled Cr:Cb plane */
>>>>> +#define DRM_FORMAT_NV42_10    fourcc_code('N', 'A', '4', '2') /* 
>>>>> non-subsampled Cb:Cr plane */
>>>>> +
>>>>> +/*
>>>>>     * 2 plane YCbCr MSB aligned
>>>>>     * index 0 = Y plane, [15:0] Y:x [10:6] little endian
>>>>>     * index 1 = Cr:Cb plane, [31:0] Cr:x:Cb:x [10:6:10:6] little 
>>>>> endian
>>>>> -- 
>>>>> 2.7.4
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> dri-devel mailing list
>>>>> dri-devel@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



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

* Re: [PATCH v2 1/3] drm: Add some new format DRM_FORMAT_NVXX_10
       [not found]             ` <20191011064433.GA18503@jamwan02-TSP300>
@ 2019-10-11  7:07               ` sandy.huang
       [not found]                 ` <20191011072250.GA20592@jamwan02-TSP300>
  0 siblings, 1 reply; 16+ messages in thread
From: sandy.huang @ 2019-10-11  7:07 UTC (permalink / raw)
  To: james qian wang (Arm Technology China)
  Cc: Ville Syrjälä,
	Maxime Ripard, linux-kernel, dri-devel, David Airlie, Sean Paul,
	nd


在 2019/10/11 下午2:44, james qian wang (Arm Technology China) 写道:
> On Fri, Oct 11, 2019 at 11:35:53AM +0800, sandy.huang wrote:
>> Hi james.qian.wang,
>>
>>      Thank for you remind, fou some unknow reason, i miss the the mail from
>> you:(, i get this message from https://patchwork.kernel.org/patch/11161937/
>>
>> sorry about that.
>>
>>      About the format block describe, I also found some unreasonable,  this
>> format need 2 line aligned, so the block_h need to sed as 2, and the
>> char_per_block need set as w * h * 10 for y plane, and w * h * 2 * 10 for uv
>> plane, so the following describe maybe more correct, thanks.
>>
>>          { .format = DRM_FORMAT_NV12_10,        .depth = 0, .num_planes = 2,
>>            .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, .block_h
>> = { 2, 2, 0 },
>>            .hsub = 2, .vsub = 2, .is_yuv = true},
> Hi Sandy:
> I think for such NV12 YUV-422 (hsub = 2, vsub = 2) 2x2 subsampled format
> the block size can be:
>
> the Y plane:  2x2;
> The UV plane: 1x2; (H direction sample 1 Cb and 1Cr, V direction 2 lines got 2)
>
> Then:
>
> .char_per_block = {5, 5, 0} block_w = {2, 1, 0}. block_h = {2, 2, 0};
>
> Thanks
> James

Hi James,

If the block_w is 2 pixel, one line size at block is 2*10 bit %8 != 0,  
although we use block to describe this format, but actually the data is 
still stored one line by one line, still need 4 pixel aligned. so i 
think here need use 4pixel*2line for per block,

Thanks,

sandy.huang.

>
>>            .hsub = 2, .vsub = 2, .is_yuv = true},
>>          { .format = DRM_FORMAT_NV21_10,        .depth = 0, .num_planes = 2,
>>            .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, .block_h
>> = { 2, 2, 0 },
>>            .hsub = 2, .vsub = 2, .is_yuv = true},
>>          { .format = DRM_FORMAT_NV16_10,        .depth = 0, .num_planes = 2,
>>            .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, .block_h
>> = { 2, 2, 0 },
>>            .hsub = 2, .vsub = 1, .is_yuv = true},
>>          { .format = DRM_FORMAT_NV61_10,        .depth = 0, .num_planes = 2,
>>            .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, .block_h
>> = { 2, 2, 0 },
>>            .hsub = 2, .vsub = 1, .is_yuv = true},
>>          { .format = DRM_FORMAT_NV24_10,        .depth = 0, .num_planes = 2,
>>            .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, .block_h
>> = { 2, 2, 0 },
>>            .hsub = 1, .vsub = 1, .is_yuv = true},
>>          { .format = DRM_FORMAT_NV42_10,        .depth = 0, .num_planes = 2,
>>            .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, .block_h
>> = { 2, 2, 0 },
>>            .hsub = 1, .vsub = 1, .is_yuv = true},
>>
>>
>>>>            { .format = DRM_FORMAT_P016,        .depth = 0,  .num_planes =
>> 2,
>>>>              .char_per_block = { 2, 4, 0 }, .block_w = { 1, 0, 0 },
>> .block_h = { 1, 0, 0 },
>>>>              .hsub = 2, .vsub = 2, .is_yuv = true},
>>>> +        { .format = DRM_FORMAT_NV12_10,        .depth = 0,  .num_planes
>> = 2,
>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 },
>> .block_h = { 4, 4, 0 },
>>
>>> Hi Sandy:
>>> Their is a problem here for char_per_block size of plane[0]:
>>> Since: 5 * 8 != 4 * 4 * 10;
>>> Seems you mis-set the block_w/h, per your block size the block is 2x2, and
>> it should be:
>>>    .char_per_block = { 5, 10, 0 }, .block_w = { 2, 2, 0 }, .block_h = { 2,
>> 2, 0 },
>>
>>> Best Regards:
>>> James
>>
>>
>>
>>
>>
>> 在 2019/10/8 下午7:49, sandy.huang 写道:
>>> 在 2019/10/8 下午7:33, Ville Syrjälä 写道:
>>>> On Tue, Oct 08, 2019 at 10:40:20AM +0800, sandy.huang wrote:
>>>>> Hi ville syrjala,
>>>>>
>>>>> 在 2019/9/30 下午6:48, Ville Syrjälä 写道:
>>>>>> On Thu, Sep 26, 2019 at 04:24:47PM +0800, Sandy Huang wrote:
>>>>>>> These new format is supported by some rockchip socs:
>>>>>>>
>>>>>>> DRM_FORMAT_NV12_10/DRM_FORMAT_NV21_10
>>>>>>> DRM_FORMAT_NV16_10/DRM_FORMAT_NV61_10
>>>>>>> DRM_FORMAT_NV24_10/DRM_FORMAT_NV42_10
>>>>>>>
>>>>>>> Signed-off-by: Sandy Huang <hjc@rock-chips.com>
>>>>>>> ---
>>>>>>>     drivers/gpu/drm/drm_fourcc.c  | 18 ++++++++++++++++++
>>>>>>>     include/uapi/drm/drm_fourcc.h | 14 ++++++++++++++
>>>>>>>     2 files changed, 32 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/drm_fourcc.c
>>>>>>> b/drivers/gpu/drm/drm_fourcc.c
>>>>>>> index c630064..ccd78a3 100644
>>>>>>> --- a/drivers/gpu/drm/drm_fourcc.c
>>>>>>> +++ b/drivers/gpu/drm/drm_fourcc.c
>>>>>>> @@ -261,6 +261,24 @@ const struct drm_format_info
>>>>>>> *__drm_format_info(u32 format)
>>>>>>>             { .format = DRM_FORMAT_P016,        .depth =
>>>>>>> 0,  .num_planes = 2,
>>>>>>>               .char_per_block = { 2, 4, 0 }, .block_w = {
>>>>>>> 1, 0, 0 }, .block_h = { 1, 0, 0 },
>>>>>>>               .hsub = 2, .vsub = 2, .is_yuv = true},
>>>>>>> +        { .format = DRM_FORMAT_NV12_10,        .depth =
>>>>>>> 0,  .num_planes = 2,
>>>>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = {
>>>>>>> 4, 4, 0 }, .block_h = { 4, 4, 0 },
>>>>>>> +          .hsub = 2, .vsub = 2, .is_yuv = true},
>>>>>>> +        { .format = DRM_FORMAT_NV21_10,        .depth =
>>>>>>> 0,  .num_planes = 2,
>>>>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = {
>>>>>>> 4, 4, 0 }, .block_h = { 4, 4, 0 },
>>>>>>> +          .hsub = 2, .vsub = 2, .is_yuv = true},
>>>>>>> +        { .format = DRM_FORMAT_NV16_10,        .depth =
>>>>>>> 0,  .num_planes = 2,
>>>>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = {
>>>>>>> 4, 4, 0 }, .block_h = { 4, 4, 0 },
>>>>>>> +          .hsub = 2, .vsub = 1, .is_yuv = true},
>>>>>>> +        { .format = DRM_FORMAT_NV61_10,        .depth =
>>>>>>> 0,  .num_planes = 2,
>>>>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = {
>>>>>>> 4, 4, 0 }, .block_h = { 4, 4, 0 },
>>>>>>> +          .hsub = 2, .vsub = 1, .is_yuv = true},
>>>>>>> +        { .format = DRM_FORMAT_NV24_10,        .depth =
>>>>>>> 0,  .num_planes = 2,
>>>>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = {
>>>>>>> 4, 4, 0 }, .block_h = { 4, 4, 0 },
>>>>>>> +          .hsub = 1, .vsub = 1, .is_yuv = true},
>>>>>>> +        { .format = DRM_FORMAT_NV42_10,        .depth =
>>>>>>> 0,  .num_planes = 2,
>>>>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = {
>>>>>>> 4, 4, 0 }, .block_h = { 4, 4, 0 },
>>>>>>> +          .hsub = 1, .vsub = 1, .is_yuv = true},
>>>>>>>             { .format = DRM_FORMAT_P210,        .depth = 0,
>>>>>>>               .num_planes = 2, .char_per_block = { 2, 4, 0 },
>>>>>>>               .block_w = { 1, 0, 0 }, .block_h = { 1, 0,
>>>>>>> 0 }, .hsub = 2,
>>>>>>> diff --git a/include/uapi/drm/drm_fourcc.h
>>>>>>> b/include/uapi/drm/drm_fourcc.h
>>>>>>> index 3feeaa3..08e2221 100644
>>>>>>> --- a/include/uapi/drm/drm_fourcc.h
>>>>>>> +++ b/include/uapi/drm/drm_fourcc.h
>>>>>>> @@ -238,6 +238,20 @@ extern "C" {
>>>>>>>     #define DRM_FORMAT_NV42        fourcc_code('N', 'V',
>>>>>>> '4', '2') /* non-subsampled Cb:Cr plane */
>>>>>>>        /*
>>>>>>> + * 2 plane YCbCr
>>>>>>> + * index 0 = Y plane, Y3:Y2:Y1:Y0 10:10:10:10
>>>>>>> + * index 1 = Cb:Cr plane,
>>>>>>> Cb3:Cr3:Cb2:Cr2:Cb1:Cr1:Cb0:Cr0 10:10:10:10:10:10:10:10
>>>>>>> + * or
>>>>>>> + * index 1 = Cr:Cb plane,
>>>>>>> Cr3:Cb3:Cr2:Cb2:Cr1:Cb1:Cr0:Cb0 10:10:10:10:10:10:10:10
>>>>>> So now you're defining it as some kind of byte aligned block.
>>>>>> With that specifying endianness would now make sense since
>>>>>> otherwise this tells us absolutely nothing about the memory
>>>>>> layout.
>>>>>>
>>>>>> So I'd either do that, or go back to not specifying anything and
>>>>>> use some weasel words like "mamory layout is implementation defined"
>>>>>> which of course means no one can use it for anything that involves
>>>>>> any kind of cross vendor stuff.
>>>>> /*
>>>>>     * 2 plane YCbCr
>>>>>     * index 0 = Y plane, [39: 0] Y3:Y2:Y1:Y0 10:10:10:10 little endian
>>>>>     * index 1 = Cb:Cr plane, [79: 0] Cb3:Cr3:Cb2:Cr2:Cb1:Cr1:Cb0:Cr0
>>>>> 10:10:10:10:10:10:10:10  little endian
>>>>>     * or
>>>>>     * index 1 = Cr:Cb plane, [79: 0] Cr3:Cb3:Cr2:Cb2:Cr1:Cb1:Cr0:Cb0
>>>>> 10:10:10:10:10:10:10:10  little endian
>>>>>     */
>>>>>
>>>>> Is this description ok?
>>>> Seems OK to me, if it actually describes the format correctly.
>>>>
>>>> Though I'm not sure why the CbCr is defines as an 80bit block
>>>> and Y has a 40bit block. 40bits should be enough for CbCr as well.
>>>>
>>> well, this is taken into account yuv444,  4 y point corresponding with 4
>>> uv point.
>>>
>>> if only describes the layout memory, here can change to 40bit block.
>>>
>>> thanks.
>>>
>>>>>>> + */
>>>>>>> +#define DRM_FORMAT_NV12_10    fourcc_code('N', 'A',
>>>>>>> '1', '2') /* 2x2 subsampled Cr:Cb plane */
>>>>>>> +#define DRM_FORMAT_NV21_10    fourcc_code('N', 'A',
>>>>>>> '2', '1') /* 2x2 subsampled Cb:Cr plane */
>>>>>>> +#define DRM_FORMAT_NV16_10    fourcc_code('N', 'A',
>>>>>>> '1', '6') /* 2x1 subsampled Cr:Cb plane */
>>>>>>> +#define DRM_FORMAT_NV61_10    fourcc_code('N', 'A',
>>>>>>> '6', '1') /* 2x1 subsampled Cb:Cr plane */
>>>>>>> +#define DRM_FORMAT_NV24_10    fourcc_code('N', 'A',
>>>>>>> '2', '4') /* non-subsampled Cr:Cb plane */
>>>>>>> +#define DRM_FORMAT_NV42_10    fourcc_code('N', 'A',
>>>>>>> '4', '2') /* non-subsampled Cb:Cr plane */
>>>>>>> +
>>>>>>> +/*
>>>>>>>      * 2 plane YCbCr MSB aligned
>>>>>>>      * index 0 = Y plane, [15:0] Y:x [10:6] little endian
>>>>>>>      * index 1 = Cr:Cb plane, [31:0] Cr:x:Cb:x
>>>>>>> [10:6:10:6] little endian
>>>>>>> -- 
>>>>>>> 2.7.4
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> dri-devel mailing list
>>>>>>> dri-devel@lists.freedesktop.org
>>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel



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

* Re: [PATCH v2 1/3] drm: Add some new format DRM_FORMAT_NVXX_10
       [not found]                 ` <20191011072250.GA20592@jamwan02-TSP300>
@ 2019-10-11  7:32                   ` sandy.huang
       [not found]                     ` <20191011083247.GA22224@jamwan02-TSP300>
  0 siblings, 1 reply; 16+ messages in thread
From: sandy.huang @ 2019-10-11  7:32 UTC (permalink / raw)
  To: james qian wang (Arm Technology China)
  Cc: Ville Syrjälä,
	Maxime Ripard, linux-kernel, dri-devel, David Airlie, Sean Paul,
	nd


在 2019/10/11 下午3:22, james qian wang (Arm Technology China) 写道:
> On Fri, Oct 11, 2019 at 03:07:22PM +0800, sandy.huang wrote:
>> 在 2019/10/11 下午2:44, james qian wang (Arm Technology China) 写道:
>>> On Fri, Oct 11, 2019 at 11:35:53AM +0800, sandy.huang wrote:
>>>> Hi james.qian.wang,
>>>>
>>>>       Thank for you remind, fou some unknow reason, i miss the the mail from
>>>> you:(, i get this message from https://patchwork.kernel.org/patch/11161937/
>>>>
>>>> sorry about that.
>>>>
>>>>       About the format block describe, I also found some unreasonable,  this
>>>> format need 2 line aligned, so the block_h need to sed as 2, and the
>>>> char_per_block need set as w * h * 10 for y plane, and w * h * 2 * 10 for uv
>>>> plane, so the following describe maybe more correct, thanks.
>>>>
>>>>           { .format = DRM_FORMAT_NV12_10,        .depth = 0, .num_planes = 2,
>>>>             .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, .block_h
>>>> = { 2, 2, 0 },
>>>>             .hsub = 2, .vsub = 2, .is_yuv = true},
>>> Hi Sandy:
>>> I think for such NV12 YUV-422 (hsub = 2, vsub = 2) 2x2 subsampled format
>>> the block size can be:
>>>
>>> the Y plane:  2x2;
>>> The UV plane: 1x2; (H direction sample 1 Cb and 1Cr, V direction 2 lines got 2)
>>>
>>> Then:
>>>
>>> .char_per_block = {5, 5, 0} block_w = {2, 1, 0}. block_h = {2, 2, 0};
>>>
>>> Thanks
>>> James
>> Hi James,
>>
>> If the block_w is 2 pixel, one line size at block is 2*10 bit %8 != 0,
> Hi Sandy:
> you got a mistake here, the bpp of UV plane is 20, 10bit Cb + 10 bit Cr.
here is for y plane.
>> although we use block to describe this format, but actually the data is
>> still stored one line by one line, still need 4 pixel aligned. so i think
>> here need use 4pixel*2line for per block
> I think this is your hardware specific requirement.
>
> Thanks
> James

yes, this is a new format first used at rockchip platform.


Thanks,

sandy.huang

>> Thanks,
>>
>> sandy.huang.
>>
>>>>             .hsub = 2, .vsub = 2, .is_yuv = true},
>>>>           { .format = DRM_FORMAT_NV21_10,        .depth = 0, .num_planes = 2,
>>>>             .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, .block_h
>>>> = { 2, 2, 0 },
>>>>             .hsub = 2, .vsub = 2, .is_yuv = true},
>>>>           { .format = DRM_FORMAT_NV16_10,        .depth = 0, .num_planes = 2,
>>>>             .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, .block_h
>>>> = { 2, 2, 0 },
>>>>             .hsub = 2, .vsub = 1, .is_yuv = true},
>>>>           { .format = DRM_FORMAT_NV61_10,        .depth = 0, .num_planes = 2,
>>>>             .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, .block_h
>>>> = { 2, 2, 0 },
>>>>             .hsub = 2, .vsub = 1, .is_yuv = true},
>>>>           { .format = DRM_FORMAT_NV24_10,        .depth = 0, .num_planes = 2,
>>>>             .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, .block_h
>>>> = { 2, 2, 0 },
>>>>             .hsub = 1, .vsub = 1, .is_yuv = true},
>>>>           { .format = DRM_FORMAT_NV42_10,        .depth = 0, .num_planes = 2,
>>>>             .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, .block_h
>>>> = { 2, 2, 0 },
>>>>             .hsub = 1, .vsub = 1, .is_yuv = true},
>>>>
>>>>
>>>>>>             { .format = DRM_FORMAT_P016,        .depth = 0,  .num_planes =
>>>> 2,
>>>>>>               .char_per_block = { 2, 4, 0 }, .block_w = { 1, 0, 0 },
>>>> .block_h = { 1, 0, 0 },
>>>>>>               .hsub = 2, .vsub = 2, .is_yuv = true},
>>>>>> +        { .format = DRM_FORMAT_NV12_10,        .depth = 0,  .num_planes
>>>> = 2,
>>>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 },
>>>> .block_h = { 4, 4, 0 },
>>>>
>>>>> Hi Sandy:
>>>>> Their is a problem here for char_per_block size of plane[0]:
>>>>> Since: 5 * 8 != 4 * 4 * 10;
>>>>> Seems you mis-set the block_w/h, per your block size the block is 2x2, and
>>>> it should be:
>>>>>     .char_per_block = { 5, 10, 0 }, .block_w = { 2, 2, 0 }, .block_h = { 2,
>>>> 2, 0 },
>>>>
>>>>> Best Regards:
>>>>> James
>>>>
>>>>
>>>>
>>>>
>>>> 在 2019/10/8 下午7:49, sandy.huang 写道:
>>>>> 在 2019/10/8 下午7:33, Ville Syrjälä 写道:
>>>>>> On Tue, Oct 08, 2019 at 10:40:20AM +0800, sandy.huang wrote:
>>>>>>> Hi ville syrjala,
>>>>>>>
>>>>>>> 在 2019/9/30 下午6:48, Ville Syrjälä 写道:
>>>>>>>> On Thu, Sep 26, 2019 at 04:24:47PM +0800, Sandy Huang wrote:
>>>>>>>>> These new format is supported by some rockchip socs:
>>>>>>>>>
>>>>>>>>> DRM_FORMAT_NV12_10/DRM_FORMAT_NV21_10
>>>>>>>>> DRM_FORMAT_NV16_10/DRM_FORMAT_NV61_10
>>>>>>>>> DRM_FORMAT_NV24_10/DRM_FORMAT_NV42_10
>>>>>>>>>
>>>>>>>>> Signed-off-by: Sandy Huang <hjc@rock-chips.com>
>>>>>>>>> ---
>>>>>>>>>      drivers/gpu/drm/drm_fourcc.c  | 18 ++++++++++++++++++
>>>>>>>>>      include/uapi/drm/drm_fourcc.h | 14 ++++++++++++++
>>>>>>>>>      2 files changed, 32 insertions(+)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/gpu/drm/drm_fourcc.c
>>>>>>>>> b/drivers/gpu/drm/drm_fourcc.c
>>>>>>>>> index c630064..ccd78a3 100644
>>>>>>>>> --- a/drivers/gpu/drm/drm_fourcc.c
>>>>>>>>> +++ b/drivers/gpu/drm/drm_fourcc.c
>>>>>>>>> @@ -261,6 +261,24 @@ const struct drm_format_info
>>>>>>>>> *__drm_format_info(u32 format)
>>>>>>>>>              { .format = DRM_FORMAT_P016,        .depth =
>>>>>>>>> 0,  .num_planes = 2,
>>>>>>>>>                .char_per_block = { 2, 4, 0 }, .block_w = {
>>>>>>>>> 1, 0, 0 }, .block_h = { 1, 0, 0 },
>>>>>>>>>                .hsub = 2, .vsub = 2, .is_yuv = true},
>>>>>>>>> +        { .format = DRM_FORMAT_NV12_10,        .depth =
>>>>>>>>> 0,  .num_planes = 2,
>>>>>>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = {
>>>>>>>>> 4, 4, 0 }, .block_h = { 4, 4, 0 },
>>>>>>>>> +          .hsub = 2, .vsub = 2, .is_yuv = true},
>>>>>>>>> +        { .format = DRM_FORMAT_NV21_10,        .depth =
>>>>>>>>> 0,  .num_planes = 2,
>>>>>>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = {
>>>>>>>>> 4, 4, 0 }, .block_h = { 4, 4, 0 },
>>>>>>>>> +          .hsub = 2, .vsub = 2, .is_yuv = true},
>>>>>>>>> +        { .format = DRM_FORMAT_NV16_10,        .depth =
>>>>>>>>> 0,  .num_planes = 2,
>>>>>>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = {
>>>>>>>>> 4, 4, 0 }, .block_h = { 4, 4, 0 },
>>>>>>>>> +          .hsub = 2, .vsub = 1, .is_yuv = true},
>>>>>>>>> +        { .format = DRM_FORMAT_NV61_10,        .depth =
>>>>>>>>> 0,  .num_planes = 2,
>>>>>>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = {
>>>>>>>>> 4, 4, 0 }, .block_h = { 4, 4, 0 },
>>>>>>>>> +          .hsub = 2, .vsub = 1, .is_yuv = true},
>>>>>>>>> +        { .format = DRM_FORMAT_NV24_10,        .depth =
>>>>>>>>> 0,  .num_planes = 2,
>>>>>>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = {
>>>>>>>>> 4, 4, 0 }, .block_h = { 4, 4, 0 },
>>>>>>>>> +          .hsub = 1, .vsub = 1, .is_yuv = true},
>>>>>>>>> +        { .format = DRM_FORMAT_NV42_10,        .depth =
>>>>>>>>> 0,  .num_planes = 2,
>>>>>>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = {
>>>>>>>>> 4, 4, 0 }, .block_h = { 4, 4, 0 },
>>>>>>>>> +          .hsub = 1, .vsub = 1, .is_yuv = true},
>>>>>>>>>              { .format = DRM_FORMAT_P210,        .depth = 0,
>>>>>>>>>                .num_planes = 2, .char_per_block = { 2, 4, 0 },
>>>>>>>>>                .block_w = { 1, 0, 0 }, .block_h = { 1, 0,
>>>>>>>>> 0 }, .hsub = 2,
>>>>>>>>> diff --git a/include/uapi/drm/drm_fourcc.h
>>>>>>>>> b/include/uapi/drm/drm_fourcc.h
>>>>>>>>> index 3feeaa3..08e2221 100644
>>>>>>>>> --- a/include/uapi/drm/drm_fourcc.h
>>>>>>>>> +++ b/include/uapi/drm/drm_fourcc.h
>>>>>>>>> @@ -238,6 +238,20 @@ extern "C" {
>>>>>>>>>      #define DRM_FORMAT_NV42        fourcc_code('N', 'V',
>>>>>>>>> '4', '2') /* non-subsampled Cb:Cr plane */
>>>>>>>>>         /*
>>>>>>>>> + * 2 plane YCbCr
>>>>>>>>> + * index 0 = Y plane, Y3:Y2:Y1:Y0 10:10:10:10
>>>>>>>>> + * index 1 = Cb:Cr plane,
>>>>>>>>> Cb3:Cr3:Cb2:Cr2:Cb1:Cr1:Cb0:Cr0 10:10:10:10:10:10:10:10
>>>>>>>>> + * or
>>>>>>>>> + * index 1 = Cr:Cb plane,
>>>>>>>>> Cr3:Cb3:Cr2:Cb2:Cr1:Cb1:Cr0:Cb0 10:10:10:10:10:10:10:10
>>>>>>>> So now you're defining it as some kind of byte aligned block.
>>>>>>>> With that specifying endianness would now make sense since
>>>>>>>> otherwise this tells us absolutely nothing about the memory
>>>>>>>> layout.
>>>>>>>>
>>>>>>>> So I'd either do that, or go back to not specifying anything and
>>>>>>>> use some weasel words like "mamory layout is implementation defined"
>>>>>>>> which of course means no one can use it for anything that involves
>>>>>>>> any kind of cross vendor stuff.
>>>>>>> /*
>>>>>>>      * 2 plane YCbCr
>>>>>>>      * index 0 = Y plane, [39: 0] Y3:Y2:Y1:Y0 10:10:10:10 little endian
>>>>>>>      * index 1 = Cb:Cr plane, [79: 0] Cb3:Cr3:Cb2:Cr2:Cb1:Cr1:Cb0:Cr0
>>>>>>> 10:10:10:10:10:10:10:10  little endian
>>>>>>>      * or
>>>>>>>      * index 1 = Cr:Cb plane, [79: 0] Cr3:Cb3:Cr2:Cb2:Cr1:Cb1:Cr0:Cb0
>>>>>>> 10:10:10:10:10:10:10:10  little endian
>>>>>>>      */
>>>>>>>
>>>>>>> Is this description ok?
>>>>>> Seems OK to me, if it actually describes the format correctly.
>>>>>>
>>>>>> Though I'm not sure why the CbCr is defines as an 80bit block
>>>>>> and Y has a 40bit block. 40bits should be enough for CbCr as well.
>>>>>>
>>>>> well, this is taken into account yuv444,  4 y point corresponding with 4
>>>>> uv point.
>>>>>
>>>>> if only describes the layout memory, here can change to 40bit block.
>>>>>
>>>>> thanks.
>>>>>
>>>>>>>>> + */
>>>>>>>>> +#define DRM_FORMAT_NV12_10    fourcc_code('N', 'A',
>>>>>>>>> '1', '2') /* 2x2 subsampled Cr:Cb plane */
>>>>>>>>> +#define DRM_FORMAT_NV21_10    fourcc_code('N', 'A',
>>>>>>>>> '2', '1') /* 2x2 subsampled Cb:Cr plane */
>>>>>>>>> +#define DRM_FORMAT_NV16_10    fourcc_code('N', 'A',
>>>>>>>>> '1', '6') /* 2x1 subsampled Cr:Cb plane */
>>>>>>>>> +#define DRM_FORMAT_NV61_10    fourcc_code('N', 'A',
>>>>>>>>> '6', '1') /* 2x1 subsampled Cb:Cr plane */
>>>>>>>>> +#define DRM_FORMAT_NV24_10    fourcc_code('N', 'A',
>>>>>>>>> '2', '4') /* non-subsampled Cr:Cb plane */
>>>>>>>>> +#define DRM_FORMAT_NV42_10    fourcc_code('N', 'A',
>>>>>>>>> '4', '2') /* non-subsampled Cb:Cr plane */
>>>>>>>>> +
>>>>>>>>> +/*
>>>>>>>>>       * 2 plane YCbCr MSB aligned
>>>>>>>>>       * index 0 = Y plane, [15:0] Y:x [10:6] little endian
>>>>>>>>>       * index 1 = Cr:Cb plane, [31:0] Cr:x:Cb:x
>>>>>>>>> [10:6:10:6] little endian
>>>>>>>>> -- 
>>>>>>>>> 2.7.4
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> _______________________________________________
>>>>>>>>> dri-devel mailing list
>>>>>>>>> dri-devel@lists.freedesktop.org
>>>>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>> _______________________________________________
>>>>> dri-devel mailing list
>>>>> dri-devel@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel



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

* Re: [PATCH v2 1/3] drm: Add some new format DRM_FORMAT_NVXX_10
       [not found]                     ` <20191011083247.GA22224@jamwan02-TSP300>
@ 2019-10-11  9:45                       ` sandy.huang
  2019-10-14  8:34                         ` Daniel Vetter
  0 siblings, 1 reply; 16+ messages in thread
From: sandy.huang @ 2019-10-11  9:45 UTC (permalink / raw)
  To: james qian wang (Arm Technology China)
  Cc: Ville Syrjälä,
	Maxime Ripard, linux-kernel, dri-devel, David Airlie, Sean Paul,
	nd

Hi, james, ville syrjala, david,

在 2019/10/11 下午4:32, james qian wang (Arm Technology China) 写道:
> On Fri, Oct 11, 2019 at 03:32:17PM +0800, sandy.huang wrote:
>> 在 2019/10/11 下午3:22, james qian wang (Arm Technology China) 写道:
>>> On Fri, Oct 11, 2019 at 03:07:22PM +0800, sandy.huang wrote:
>>>> 在 2019/10/11 下午2:44, james qian wang (Arm Technology China) 写道:
>>>>> On Fri, Oct 11, 2019 at 11:35:53AM +0800, sandy.huang wrote:
>>>>>> Hi james.qian.wang,
>>>>>>
>>>>>>        Thank for you remind, fou some unknow reason, i miss the the mail from
>>>>>> you:(, i get this message from https://patchwork.kernel.org/patch/11161937/
>>>>>>
>>>>>> sorry about that.
>>>>>>
>>>>>>        About the format block describe, I also found some unreasonable,  this
>>>>>> format need 2 line aligned, so the block_h need to sed as 2, and the
>>>>>> char_per_block need set as w * h * 10 for y plane, and w * h * 2 * 10 for uv
>>>>>> plane, so the following describe maybe more correct, thanks.
>>>>>>
>>>>>>            { .format = DRM_FORMAT_NV12_10,        .depth = 0, .num_planes = 2,
>>>>>>              .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, .block_h
>>>>>> = { 2, 2, 0 },
>>>>>>              .hsub = 2, .vsub = 2, .is_yuv = true},
>>>>> Hi Sandy:
>>>>> I think for such NV12 YUV-422 (hsub = 2, vsub = 2) 2x2 subsampled format
>>>>> the block size can be:
>>>>>
>>>>> the Y plane:  2x2;
>>>>> The UV plane: 1x2; (H direction sample 1 Cb and 1Cr, V direction 2 lines got 2)
>>>>>
>>>>> Then:
>>>>>
>>>>> .char_per_block = {5, 5, 0} block_w = {2, 1, 0}. block_h = {2, 2, 0};
>>>>>
>>>>> Thanks
>>>>> James
>>>> Hi James,
>>>>
>>>> If the block_w is 2 pixel, one line size at block is 2*10 bit %8 != 0,
>>> Hi Sandy:
>>> you got a mistake here, the bpp of UV plane is 20, 10bit Cb + 10 bit Cr.
>> here is for y plane.
> Sorry, Are we talking about the block size calcaltion here ?
>
> block_size = block_w * block_h * plane_bpp
>
> for you Y plane a 2x2 block is: 2 x 2 * 10 bpp = 40bits
>
> And the block info is for computing the minimum pitch, and don't
> consider the specific hardware alignment here.
>
> see: drm_format_info_min_pitch()
>
> If you hardware need alignment, you need to put that consideration into your
> specific driver.
>
> James.

Hi david and ville syrjala,

     Do you have any Suggestions?

     James think Y plane 2x2 block size is enough to describe this 
format, but i prefer to use 4x2 block size, this can include the 
alignment message.

just like the malidp_de_plane_check()@malidp_plane.c have the following  
code, here use the block size to check alignment.

     block_w = drm_format_info_block_width(fb->format, 0);
     block_h = drm_format_info_block_height(fb->format, 0);
     if (fb->width % block_w || fb->height % block_h) {
         DRM_DEBUG_KMS("Buffer width/height needs to be a multiple of 
tile sizes");
         return -EINVAL;
     }
     if ((state->src_x >> 16) % block_w || (state->src_y >> 16) % block_h) {
         DRM_DEBUG_KMS("Plane src_x/src_y needs to be a multiple of tile 
sizes");
         return -EINVAL;
     }

can you give me some suggestions?

thanks,

sandy.huang

>
>>>> although we use block to describe this format, but actually the data is
>>>> still stored one line by one line, still need 4 pixel aligned. so i think
>>>> here need use 4pixel*2line for per block
>>> I think this is your hardware specific requirement.
>>>
>>> Thanks
>>> James
>> yes, this is a new format first used at rockchip platform.
>>
>>
>> Thanks,
>>
>> sandy.huang
>>
>>>> Thanks,
>>>>
>>>> sandy.huang.
>>>>
>>>>>>              .hsub = 2, .vsub = 2, .is_yuv = true},
>>>>>>            { .format = DRM_FORMAT_NV21_10,        .depth = 0, .num_planes = 2,
>>>>>>              .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, .block_h
>>>>>> = { 2, 2, 0 },
>>>>>>              .hsub = 2, .vsub = 2, .is_yuv = true},
>>>>>>            { .format = DRM_FORMAT_NV16_10,        .depth = 0, .num_planes = 2,
>>>>>>              .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, .block_h
>>>>>> = { 2, 2, 0 },
>>>>>>              .hsub = 2, .vsub = 1, .is_yuv = true},
>>>>>>            { .format = DRM_FORMAT_NV61_10,        .depth = 0, .num_planes = 2,
>>>>>>              .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, .block_h
>>>>>> = { 2, 2, 0 },
>>>>>>              .hsub = 2, .vsub = 1, .is_yuv = true},
>>>>>>            { .format = DRM_FORMAT_NV24_10,        .depth = 0, .num_planes = 2,
>>>>>>              .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, .block_h
>>>>>> = { 2, 2, 0 },
>>>>>>              .hsub = 1, .vsub = 1, .is_yuv = true},
>>>>>>            { .format = DRM_FORMAT_NV42_10,        .depth = 0, .num_planes = 2,
>>>>>>              .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, .block_h
>>>>>> = { 2, 2, 0 },
>>>>>>              .hsub = 1, .vsub = 1, .is_yuv = true},
>>>>>>
>>>>>>
>>>>>>>>              { .format = DRM_FORMAT_P016,        .depth = 0,  .num_planes =
>>>>>> 2,
>>>>>>>>                .char_per_block = { 2, 4, 0 }, .block_w = { 1, 0, 0 },
>>>>>> .block_h = { 1, 0, 0 },
>>>>>>>>                .hsub = 2, .vsub = 2, .is_yuv = true},
>>>>>>>> +        { .format = DRM_FORMAT_NV12_10,        .depth = 0,  .num_planes
>>>>>> = 2,
>>>>>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 },
>>>>>> .block_h = { 4, 4, 0 },
>>>>>>
>>>>>>> Hi Sandy:
>>>>>>> Their is a problem here for char_per_block size of plane[0]:
>>>>>>> Since: 5 * 8 != 4 * 4 * 10;
>>>>>>> Seems you mis-set the block_w/h, per your block size the block is 2x2, and
>>>>>> it should be:
>>>>>>>      .char_per_block = { 5, 10, 0 }, .block_w = { 2, 2, 0 }, .block_h = { 2,
>>>>>> 2, 0 },
>>>>>>
>>>>>>> Best Regards:
>>>>>>> James
>>>>>>
>>>>>>
>>>>>>
>>>>>> 在 2019/10/8 下午7:49, sandy.huang 写道:
>>>>>>> 在 2019/10/8 下午7:33, Ville Syrjälä 写道:
>>>>>>>> On Tue, Oct 08, 2019 at 10:40:20AM +0800, sandy.huang wrote:
>>>>>>>>> Hi ville syrjala,
>>>>>>>>>
>>>>>>>>> 在 2019/9/30 下午6:48, Ville Syrjälä 写道:
>>>>>>>>>> On Thu, Sep 26, 2019 at 04:24:47PM +0800, Sandy Huang wrote:
>>>>>>>>>>> These new format is supported by some rockchip socs:
>>>>>>>>>>>
>>>>>>>>>>> DRM_FORMAT_NV12_10/DRM_FORMAT_NV21_10
>>>>>>>>>>> DRM_FORMAT_NV16_10/DRM_FORMAT_NV61_10
>>>>>>>>>>> DRM_FORMAT_NV24_10/DRM_FORMAT_NV42_10
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Sandy Huang <hjc@rock-chips.com>
>>>>>>>>>>> ---
>>>>>>>>>>>       drivers/gpu/drm/drm_fourcc.c  | 18 ++++++++++++++++++
>>>>>>>>>>>       include/uapi/drm/drm_fourcc.h | 14 ++++++++++++++
>>>>>>>>>>>       2 files changed, 32 insertions(+)
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/drivers/gpu/drm/drm_fourcc.c
>>>>>>>>>>> b/drivers/gpu/drm/drm_fourcc.c
>>>>>>>>>>> index c630064..ccd78a3 100644
>>>>>>>>>>> --- a/drivers/gpu/drm/drm_fourcc.c
>>>>>>>>>>> +++ b/drivers/gpu/drm/drm_fourcc.c
>>>>>>>>>>> @@ -261,6 +261,24 @@ const struct drm_format_info
>>>>>>>>>>> *__drm_format_info(u32 format)
>>>>>>>>>>>               { .format = DRM_FORMAT_P016,        .depth =
>>>>>>>>>>> 0,  .num_planes = 2,
>>>>>>>>>>>                 .char_per_block = { 2, 4, 0 }, .block_w = {
>>>>>>>>>>> 1, 0, 0 }, .block_h = { 1, 0, 0 },
>>>>>>>>>>>                 .hsub = 2, .vsub = 2, .is_yuv = true},
>>>>>>>>>>> +        { .format = DRM_FORMAT_NV12_10,        .depth =
>>>>>>>>>>> 0,  .num_planes = 2,
>>>>>>>>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = {
>>>>>>>>>>> 4, 4, 0 }, .block_h = { 4, 4, 0 },
>>>>>>>>>>> +          .hsub = 2, .vsub = 2, .is_yuv = true},
>>>>>>>>>>> +        { .format = DRM_FORMAT_NV21_10,        .depth =
>>>>>>>>>>> 0,  .num_planes = 2,
>>>>>>>>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = {
>>>>>>>>>>> 4, 4, 0 }, .block_h = { 4, 4, 0 },
>>>>>>>>>>> +          .hsub = 2, .vsub = 2, .is_yuv = true},
>>>>>>>>>>> +        { .format = DRM_FORMAT_NV16_10,        .depth =
>>>>>>>>>>> 0,  .num_planes = 2,
>>>>>>>>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = {
>>>>>>>>>>> 4, 4, 0 }, .block_h = { 4, 4, 0 },
>>>>>>>>>>> +          .hsub = 2, .vsub = 1, .is_yuv = true},
>>>>>>>>>>> +        { .format = DRM_FORMAT_NV61_10,        .depth =
>>>>>>>>>>> 0,  .num_planes = 2,
>>>>>>>>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = {
>>>>>>>>>>> 4, 4, 0 }, .block_h = { 4, 4, 0 },
>>>>>>>>>>> +          .hsub = 2, .vsub = 1, .is_yuv = true},
>>>>>>>>>>> +        { .format = DRM_FORMAT_NV24_10,        .depth =
>>>>>>>>>>> 0,  .num_planes = 2,
>>>>>>>>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = {
>>>>>>>>>>> 4, 4, 0 }, .block_h = { 4, 4, 0 },
>>>>>>>>>>> +          .hsub = 1, .vsub = 1, .is_yuv = true},
>>>>>>>>>>> +        { .format = DRM_FORMAT_NV42_10,        .depth =
>>>>>>>>>>> 0,  .num_planes = 2,
>>>>>>>>>>> +          .char_per_block = { 5, 10, 0 }, .block_w = {
>>>>>>>>>>> 4, 4, 0 }, .block_h = { 4, 4, 0 },
>>>>>>>>>>> +          .hsub = 1, .vsub = 1, .is_yuv = true},
>>>>>>>>>>>               { .format = DRM_FORMAT_P210,        .depth = 0,
>>>>>>>>>>>                 .num_planes = 2, .char_per_block = { 2, 4, 0 },
>>>>>>>>>>>                 .block_w = { 1, 0, 0 }, .block_h = { 1, 0,
>>>>>>>>>>> 0 }, .hsub = 2,
>>>>>>>>>>> diff --git a/include/uapi/drm/drm_fourcc.h
>>>>>>>>>>> b/include/uapi/drm/drm_fourcc.h
>>>>>>>>>>> index 3feeaa3..08e2221 100644
>>>>>>>>>>> --- a/include/uapi/drm/drm_fourcc.h
>>>>>>>>>>> +++ b/include/uapi/drm/drm_fourcc.h
>>>>>>>>>>> @@ -238,6 +238,20 @@ extern "C" {
>>>>>>>>>>>       #define DRM_FORMAT_NV42        fourcc_code('N', 'V',
>>>>>>>>>>> '4', '2') /* non-subsampled Cb:Cr plane */
>>>>>>>>>>>          /*
>>>>>>>>>>> + * 2 plane YCbCr
>>>>>>>>>>> + * index 0 = Y plane, Y3:Y2:Y1:Y0 10:10:10:10
>>>>>>>>>>> + * index 1 = Cb:Cr plane,
>>>>>>>>>>> Cb3:Cr3:Cb2:Cr2:Cb1:Cr1:Cb0:Cr0 10:10:10:10:10:10:10:10
>>>>>>>>>>> + * or
>>>>>>>>>>> + * index 1 = Cr:Cb plane,
>>>>>>>>>>> Cr3:Cb3:Cr2:Cb2:Cr1:Cb1:Cr0:Cb0 10:10:10:10:10:10:10:10
>>>>>>>>>> So now you're defining it as some kind of byte aligned block.
>>>>>>>>>> With that specifying endianness would now make sense since
>>>>>>>>>> otherwise this tells us absolutely nothing about the memory
>>>>>>>>>> layout.
>>>>>>>>>>
>>>>>>>>>> So I'd either do that, or go back to not specifying anything and
>>>>>>>>>> use some weasel words like "mamory layout is implementation defined"
>>>>>>>>>> which of course means no one can use it for anything that involves
>>>>>>>>>> any kind of cross vendor stuff.
>>>>>>>>> /*
>>>>>>>>>       * 2 plane YCbCr
>>>>>>>>>       * index 0 = Y plane, [39: 0] Y3:Y2:Y1:Y0 10:10:10:10 little endian
>>>>>>>>>       * index 1 = Cb:Cr plane, [79: 0] Cb3:Cr3:Cb2:Cr2:Cb1:Cr1:Cb0:Cr0
>>>>>>>>> 10:10:10:10:10:10:10:10  little endian
>>>>>>>>>       * or
>>>>>>>>>       * index 1 = Cr:Cb plane, [79: 0] Cr3:Cb3:Cr2:Cb2:Cr1:Cb1:Cr0:Cb0
>>>>>>>>> 10:10:10:10:10:10:10:10  little endian
>>>>>>>>>       */
>>>>>>>>>
>>>>>>>>> Is this description ok?
>>>>>>>> Seems OK to me, if it actually describes the format correctly.
>>>>>>>>
>>>>>>>> Though I'm not sure why the CbCr is defines as an 80bit block
>>>>>>>> and Y has a 40bit block. 40bits should be enough for CbCr as well.
>>>>>>>>
>>>>>>> well, this is taken into account yuv444,  4 y point corresponding with 4
>>>>>>> uv point.
>>>>>>>
>>>>>>> if only describes the layout memory, here can change to 40bit block.
>>>>>>>
>>>>>>> thanks.
>>>>>>>
>>>>>>>>>>> + */
>>>>>>>>>>> +#define DRM_FORMAT_NV12_10    fourcc_code('N', 'A',
>>>>>>>>>>> '1', '2') /* 2x2 subsampled Cr:Cb plane */
>>>>>>>>>>> +#define DRM_FORMAT_NV21_10    fourcc_code('N', 'A',
>>>>>>>>>>> '2', '1') /* 2x2 subsampled Cb:Cr plane */
>>>>>>>>>>> +#define DRM_FORMAT_NV16_10    fourcc_code('N', 'A',
>>>>>>>>>>> '1', '6') /* 2x1 subsampled Cr:Cb plane */
>>>>>>>>>>> +#define DRM_FORMAT_NV61_10    fourcc_code('N', 'A',
>>>>>>>>>>> '6', '1') /* 2x1 subsampled Cb:Cr plane */
>>>>>>>>>>> +#define DRM_FORMAT_NV24_10    fourcc_code('N', 'A',
>>>>>>>>>>> '2', '4') /* non-subsampled Cr:Cb plane */
>>>>>>>>>>> +#define DRM_FORMAT_NV42_10    fourcc_code('N', 'A',
>>>>>>>>>>> '4', '2') /* non-subsampled Cb:Cr plane */
>>>>>>>>>>> +
>>>>>>>>>>> +/*
>>>>>>>>>>>        * 2 plane YCbCr MSB aligned
>>>>>>>>>>>        * index 0 = Y plane, [15:0] Y:x [10:6] little endian
>>>>>>>>>>>        * index 1 = Cr:Cb plane, [31:0] Cr:x:Cb:x
>>>>>>>>>>> [10:6:10:6] little endian
>>>>>>>>>>> -- 
>>>>>>>>>>> 2.7.4
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> _______________________________________________
>>>>>>>>>>> dri-devel mailing list
>>>>>>>>>>> dri-devel@lists.freedesktop.org
>>>>>>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>>> _______________________________________________
>>>>>>> dri-devel mailing list
>>>>>>> dri-devel@lists.freedesktop.org
>>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel



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

* Re: [PATCH v2 1/3] drm: Add some new format DRM_FORMAT_NVXX_10
  2019-10-11  9:45                       ` sandy.huang
@ 2019-10-14  8:34                         ` Daniel Vetter
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2019-10-14  8:34 UTC (permalink / raw)
  To: sandy.huang
  Cc: james qian wang (Arm Technology China),
	Maxime Ripard, linux-kernel, dri-devel, David Airlie, nd,
	Sean Paul

On Fri, Oct 11, 2019 at 05:45:45PM +0800, sandy.huang wrote:
> Hi, james, ville syrjala, david,
> 
> 在 2019/10/11 下午4:32, james qian wang (Arm Technology China) 写道:
> > On Fri, Oct 11, 2019 at 03:32:17PM +0800, sandy.huang wrote:
> > > 在 2019/10/11 下午3:22, james qian wang (Arm Technology China) 写道:
> > > > On Fri, Oct 11, 2019 at 03:07:22PM +0800, sandy.huang wrote:
> > > > > 在 2019/10/11 下午2:44, james qian wang (Arm Technology China) 写道:
> > > > > > On Fri, Oct 11, 2019 at 11:35:53AM +0800, sandy.huang wrote:
> > > > > > > Hi james.qian.wang,
> > > > > > > 
> > > > > > >        Thank for you remind, fou some unknow reason, i miss the the mail from
> > > > > > > you:(, i get this message from https://patchwork.kernel.org/patch/11161937/
> > > > > > > 
> > > > > > > sorry about that.
> > > > > > > 
> > > > > > >        About the format block describe, I also found some unreasonable,  this
> > > > > > > format need 2 line aligned, so the block_h need to sed as 2, and the
> > > > > > > char_per_block need set as w * h * 10 for y plane, and w * h * 2 * 10 for uv
> > > > > > > plane, so the following describe maybe more correct, thanks.
> > > > > > > 
> > > > > > >            { .format = DRM_FORMAT_NV12_10,        .depth = 0, .num_planes = 2,
> > > > > > >              .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, .block_h
> > > > > > > = { 2, 2, 0 },
> > > > > > >              .hsub = 2, .vsub = 2, .is_yuv = true},
> > > > > > Hi Sandy:
> > > > > > I think for such NV12 YUV-422 (hsub = 2, vsub = 2) 2x2 subsampled format
> > > > > > the block size can be:
> > > > > > 
> > > > > > the Y plane:  2x2;
> > > > > > The UV plane: 1x2; (H direction sample 1 Cb and 1Cr, V direction 2 lines got 2)
> > > > > > 
> > > > > > Then:
> > > > > > 
> > > > > > .char_per_block = {5, 5, 0} block_w = {2, 1, 0}. block_h = {2, 2, 0};
> > > > > > 
> > > > > > Thanks
> > > > > > James
> > > > > Hi James,
> > > > > 
> > > > > If the block_w is 2 pixel, one line size at block is 2*10 bit %8 != 0,
> > > > Hi Sandy:
> > > > you got a mistake here, the bpp of UV plane is 20, 10bit Cb + 10 bit Cr.
> > > here is for y plane.
> > Sorry, Are we talking about the block size calcaltion here ?
> > 
> > block_size = block_w * block_h * plane_bpp
> > 
> > for you Y plane a 2x2 block is: 2 x 2 * 10 bpp = 40bits
> > 
> > And the block info is for computing the minimum pitch, and don't
> > consider the specific hardware alignment here.
> > 
> > see: drm_format_info_min_pitch()
> > 
> > If you hardware need alignment, you need to put that consideration into your
> > specific driver.
> > 
> > James.
> 
> Hi david and ville syrjala,
> 
>     Do you have any Suggestions?
> 
>     James think Y plane 2x2 block size is enough to describe this format,
> but i prefer to use 4x2 block size, this can include the alignment message.
> 
> just like the malidp_de_plane_check()@malidp_plane.c have the following 
> code, here use the block size to check alignment.
> 
>     block_w = drm_format_info_block_width(fb->format, 0);
>     block_h = drm_format_info_block_height(fb->format, 0);
>     if (fb->width % block_w || fb->height % block_h) {
>         DRM_DEBUG_KMS("Buffer width/height needs to be a multiple of tile
> sizes");
>         return -EINVAL;
>     }
>     if ((state->src_x >> 16) % block_w || (state->src_y >> 16) % block_h) {
>         DRM_DEBUG_KMS("Plane src_x/src_y needs to be a multiple of tile
> sizes");
>         return -EINVAL;
>     }
> 
> can you give me some suggestions?

For the linear layout (i.e. modifier == 0) the blocks are meant to be laid
out one after the other, linearly, in memory. If you have some additional
hw alignment constraint then that should be checked in the driver.

If you have some tiling on top, then that should be encoded in a different
modifier (which can then set its own block sizes, and have its own rules
for how they're laid out in memory).

Taking this all together I think what we want here is a 1x4 Y block and a
1x2 UV block size in pixels. The sub-sampling is expressed in hsub/vsub
for the UV plane, we don't align the block-sizes two 2 rows to encode that
(see all the other yuv planar formats for examples).
-Daniel

> 
> thanks,
> 
> sandy.huang
> 
> > 
> > > > > although we use block to describe this format, but actually the data is
> > > > > still stored one line by one line, still need 4 pixel aligned. so i think
> > > > > here need use 4pixel*2line for per block
> > > > I think this is your hardware specific requirement.
> > > > 
> > > > Thanks
> > > > James
> > > yes, this is a new format first used at rockchip platform.
> > > 
> > > 
> > > Thanks,
> > > 
> > > sandy.huang
> > > 
> > > > > Thanks,
> > > > > 
> > > > > sandy.huang.
> > > > > 
> > > > > > >              .hsub = 2, .vsub = 2, .is_yuv = true},
> > > > > > >            { .format = DRM_FORMAT_NV21_10,        .depth = 0, .num_planes = 2,
> > > > > > >              .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, .block_h
> > > > > > > = { 2, 2, 0 },
> > > > > > >              .hsub = 2, .vsub = 2, .is_yuv = true},
> > > > > > >            { .format = DRM_FORMAT_NV16_10,        .depth = 0, .num_planes = 2,
> > > > > > >              .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, .block_h
> > > > > > > = { 2, 2, 0 },
> > > > > > >              .hsub = 2, .vsub = 1, .is_yuv = true},
> > > > > > >            { .format = DRM_FORMAT_NV61_10,        .depth = 0, .num_planes = 2,
> > > > > > >              .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, .block_h
> > > > > > > = { 2, 2, 0 },
> > > > > > >              .hsub = 2, .vsub = 1, .is_yuv = true},
> > > > > > >            { .format = DRM_FORMAT_NV24_10,        .depth = 0, .num_planes = 2,
> > > > > > >              .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, .block_h
> > > > > > > = { 2, 2, 0 },
> > > > > > >              .hsub = 1, .vsub = 1, .is_yuv = true},
> > > > > > >            { .format = DRM_FORMAT_NV42_10,        .depth = 0, .num_planes = 2,
> > > > > > >              .char_per_block = { 10, 10, 0 }, .block_w = { 4, 2, 0 }, .block_h
> > > > > > > = { 2, 2, 0 },
> > > > > > >              .hsub = 1, .vsub = 1, .is_yuv = true},
> > > > > > > 
> > > > > > > 
> > > > > > > > >              { .format = DRM_FORMAT_P016,        .depth = 0,  .num_planes =
> > > > > > > 2,
> > > > > > > > >                .char_per_block = { 2, 4, 0 }, .block_w = { 1, 0, 0 },
> > > > > > > .block_h = { 1, 0, 0 },
> > > > > > > > >                .hsub = 2, .vsub = 2, .is_yuv = true},
> > > > > > > > > +        { .format = DRM_FORMAT_NV12_10,        .depth = 0,  .num_planes
> > > > > > > = 2,
> > > > > > > > > +          .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 },
> > > > > > > .block_h = { 4, 4, 0 },
> > > > > > > 
> > > > > > > > Hi Sandy:
> > > > > > > > Their is a problem here for char_per_block size of plane[0]:
> > > > > > > > Since: 5 * 8 != 4 * 4 * 10;
> > > > > > > > Seems you mis-set the block_w/h, per your block size the block is 2x2, and
> > > > > > > it should be:
> > > > > > > >      .char_per_block = { 5, 10, 0 }, .block_w = { 2, 2, 0 }, .block_h = { 2,
> > > > > > > 2, 0 },
> > > > > > > 
> > > > > > > > Best Regards:
> > > > > > > > James
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 在 2019/10/8 下午7:49, sandy.huang 写道:
> > > > > > > > 在 2019/10/8 下午7:33, Ville Syrjälä 写道:
> > > > > > > > > On Tue, Oct 08, 2019 at 10:40:20AM +0800, sandy.huang wrote:
> > > > > > > > > > Hi ville syrjala,
> > > > > > > > > > 
> > > > > > > > > > 在 2019/9/30 下午6:48, Ville Syrjälä 写道:
> > > > > > > > > > > On Thu, Sep 26, 2019 at 04:24:47PM +0800, Sandy Huang wrote:
> > > > > > > > > > > > These new format is supported by some rockchip socs:
> > > > > > > > > > > > 
> > > > > > > > > > > > DRM_FORMAT_NV12_10/DRM_FORMAT_NV21_10
> > > > > > > > > > > > DRM_FORMAT_NV16_10/DRM_FORMAT_NV61_10
> > > > > > > > > > > > DRM_FORMAT_NV24_10/DRM_FORMAT_NV42_10
> > > > > > > > > > > > 
> > > > > > > > > > > > Signed-off-by: Sandy Huang <hjc@rock-chips.com>
> > > > > > > > > > > > ---
> > > > > > > > > > > >       drivers/gpu/drm/drm_fourcc.c  | 18 ++++++++++++++++++
> > > > > > > > > > > >       include/uapi/drm/drm_fourcc.h | 14 ++++++++++++++
> > > > > > > > > > > >       2 files changed, 32 insertions(+)
> > > > > > > > > > > > 
> > > > > > > > > > > > diff --git a/drivers/gpu/drm/drm_fourcc.c
> > > > > > > > > > > > b/drivers/gpu/drm/drm_fourcc.c
> > > > > > > > > > > > index c630064..ccd78a3 100644
> > > > > > > > > > > > --- a/drivers/gpu/drm/drm_fourcc.c
> > > > > > > > > > > > +++ b/drivers/gpu/drm/drm_fourcc.c
> > > > > > > > > > > > @@ -261,6 +261,24 @@ const struct drm_format_info
> > > > > > > > > > > > *__drm_format_info(u32 format)
> > > > > > > > > > > >               { .format = DRM_FORMAT_P016,        .depth =
> > > > > > > > > > > > 0,  .num_planes = 2,
> > > > > > > > > > > >                 .char_per_block = { 2, 4, 0 }, .block_w = {
> > > > > > > > > > > > 1, 0, 0 }, .block_h = { 1, 0, 0 },
> > > > > > > > > > > >                 .hsub = 2, .vsub = 2, .is_yuv = true},
> > > > > > > > > > > > +        { .format = DRM_FORMAT_NV12_10,        .depth =
> > > > > > > > > > > > 0,  .num_planes = 2,
> > > > > > > > > > > > +          .char_per_block = { 5, 10, 0 }, .block_w = {
> > > > > > > > > > > > 4, 4, 0 }, .block_h = { 4, 4, 0 },
> > > > > > > > > > > > +          .hsub = 2, .vsub = 2, .is_yuv = true},
> > > > > > > > > > > > +        { .format = DRM_FORMAT_NV21_10,        .depth =
> > > > > > > > > > > > 0,  .num_planes = 2,
> > > > > > > > > > > > +          .char_per_block = { 5, 10, 0 }, .block_w = {
> > > > > > > > > > > > 4, 4, 0 }, .block_h = { 4, 4, 0 },
> > > > > > > > > > > > +          .hsub = 2, .vsub = 2, .is_yuv = true},
> > > > > > > > > > > > +        { .format = DRM_FORMAT_NV16_10,        .depth =
> > > > > > > > > > > > 0,  .num_planes = 2,
> > > > > > > > > > > > +          .char_per_block = { 5, 10, 0 }, .block_w = {
> > > > > > > > > > > > 4, 4, 0 }, .block_h = { 4, 4, 0 },
> > > > > > > > > > > > +          .hsub = 2, .vsub = 1, .is_yuv = true},
> > > > > > > > > > > > +        { .format = DRM_FORMAT_NV61_10,        .depth =
> > > > > > > > > > > > 0,  .num_planes = 2,
> > > > > > > > > > > > +          .char_per_block = { 5, 10, 0 }, .block_w = {
> > > > > > > > > > > > 4, 4, 0 }, .block_h = { 4, 4, 0 },
> > > > > > > > > > > > +          .hsub = 2, .vsub = 1, .is_yuv = true},
> > > > > > > > > > > > +        { .format = DRM_FORMAT_NV24_10,        .depth =
> > > > > > > > > > > > 0,  .num_planes = 2,
> > > > > > > > > > > > +          .char_per_block = { 5, 10, 0 }, .block_w = {
> > > > > > > > > > > > 4, 4, 0 }, .block_h = { 4, 4, 0 },
> > > > > > > > > > > > +          .hsub = 1, .vsub = 1, .is_yuv = true},
> > > > > > > > > > > > +        { .format = DRM_FORMAT_NV42_10,        .depth =
> > > > > > > > > > > > 0,  .num_planes = 2,
> > > > > > > > > > > > +          .char_per_block = { 5, 10, 0 }, .block_w = {
> > > > > > > > > > > > 4, 4, 0 }, .block_h = { 4, 4, 0 },
> > > > > > > > > > > > +          .hsub = 1, .vsub = 1, .is_yuv = true},
> > > > > > > > > > > >               { .format = DRM_FORMAT_P210,        .depth = 0,
> > > > > > > > > > > >                 .num_planes = 2, .char_per_block = { 2, 4, 0 },
> > > > > > > > > > > >                 .block_w = { 1, 0, 0 }, .block_h = { 1, 0,
> > > > > > > > > > > > 0 }, .hsub = 2,
> > > > > > > > > > > > diff --git a/include/uapi/drm/drm_fourcc.h
> > > > > > > > > > > > b/include/uapi/drm/drm_fourcc.h
> > > > > > > > > > > > index 3feeaa3..08e2221 100644
> > > > > > > > > > > > --- a/include/uapi/drm/drm_fourcc.h
> > > > > > > > > > > > +++ b/include/uapi/drm/drm_fourcc.h
> > > > > > > > > > > > @@ -238,6 +238,20 @@ extern "C" {
> > > > > > > > > > > >       #define DRM_FORMAT_NV42        fourcc_code('N', 'V',
> > > > > > > > > > > > '4', '2') /* non-subsampled Cb:Cr plane */
> > > > > > > > > > > >          /*
> > > > > > > > > > > > + * 2 plane YCbCr
> > > > > > > > > > > > + * index 0 = Y plane, Y3:Y2:Y1:Y0 10:10:10:10
> > > > > > > > > > > > + * index 1 = Cb:Cr plane,
> > > > > > > > > > > > Cb3:Cr3:Cb2:Cr2:Cb1:Cr1:Cb0:Cr0 10:10:10:10:10:10:10:10
> > > > > > > > > > > > + * or
> > > > > > > > > > > > + * index 1 = Cr:Cb plane,
> > > > > > > > > > > > Cr3:Cb3:Cr2:Cb2:Cr1:Cb1:Cr0:Cb0 10:10:10:10:10:10:10:10
> > > > > > > > > > > So now you're defining it as some kind of byte aligned block.
> > > > > > > > > > > With that specifying endianness would now make sense since
> > > > > > > > > > > otherwise this tells us absolutely nothing about the memory
> > > > > > > > > > > layout.
> > > > > > > > > > > 
> > > > > > > > > > > So I'd either do that, or go back to not specifying anything and
> > > > > > > > > > > use some weasel words like "mamory layout is implementation defined"
> > > > > > > > > > > which of course means no one can use it for anything that involves
> > > > > > > > > > > any kind of cross vendor stuff.
> > > > > > > > > > /*
> > > > > > > > > >       * 2 plane YCbCr
> > > > > > > > > >       * index 0 = Y plane, [39: 0] Y3:Y2:Y1:Y0 10:10:10:10 little endian
> > > > > > > > > >       * index 1 = Cb:Cr plane, [79: 0] Cb3:Cr3:Cb2:Cr2:Cb1:Cr1:Cb0:Cr0
> > > > > > > > > > 10:10:10:10:10:10:10:10  little endian
> > > > > > > > > >       * or
> > > > > > > > > >       * index 1 = Cr:Cb plane, [79: 0] Cr3:Cb3:Cr2:Cb2:Cr1:Cb1:Cr0:Cb0
> > > > > > > > > > 10:10:10:10:10:10:10:10  little endian
> > > > > > > > > >       */
> > > > > > > > > > 
> > > > > > > > > > Is this description ok?
> > > > > > > > > Seems OK to me, if it actually describes the format correctly.
> > > > > > > > > 
> > > > > > > > > Though I'm not sure why the CbCr is defines as an 80bit block
> > > > > > > > > and Y has a 40bit block. 40bits should be enough for CbCr as well.
> > > > > > > > > 
> > > > > > > > well, this is taken into account yuv444,  4 y point corresponding with 4
> > > > > > > > uv point.
> > > > > > > > 
> > > > > > > > if only describes the layout memory, here can change to 40bit block.
> > > > > > > > 
> > > > > > > > thanks.
> > > > > > > > 
> > > > > > > > > > > > + */
> > > > > > > > > > > > +#define DRM_FORMAT_NV12_10    fourcc_code('N', 'A',
> > > > > > > > > > > > '1', '2') /* 2x2 subsampled Cr:Cb plane */
> > > > > > > > > > > > +#define DRM_FORMAT_NV21_10    fourcc_code('N', 'A',
> > > > > > > > > > > > '2', '1') /* 2x2 subsampled Cb:Cr plane */
> > > > > > > > > > > > +#define DRM_FORMAT_NV16_10    fourcc_code('N', 'A',
> > > > > > > > > > > > '1', '6') /* 2x1 subsampled Cr:Cb plane */
> > > > > > > > > > > > +#define DRM_FORMAT_NV61_10    fourcc_code('N', 'A',
> > > > > > > > > > > > '6', '1') /* 2x1 subsampled Cb:Cr plane */
> > > > > > > > > > > > +#define DRM_FORMAT_NV24_10    fourcc_code('N', 'A',
> > > > > > > > > > > > '2', '4') /* non-subsampled Cr:Cb plane */
> > > > > > > > > > > > +#define DRM_FORMAT_NV42_10    fourcc_code('N', 'A',
> > > > > > > > > > > > '4', '2') /* non-subsampled Cb:Cr plane */
> > > > > > > > > > > > +
> > > > > > > > > > > > +/*
> > > > > > > > > > > >        * 2 plane YCbCr MSB aligned
> > > > > > > > > > > >        * index 0 = Y plane, [15:0] Y:x [10:6] little endian
> > > > > > > > > > > >        * index 1 = Cr:Cb plane, [31:0] Cr:x:Cb:x
> > > > > > > > > > > > [10:6:10:6] little endian
> > > > > > > > > > > > -- 
> > > > > > > > > > > > 2.7.4
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > _______________________________________________
> > > > > > > > > > > > dri-devel mailing list
> > > > > > > > > > > > dri-devel@lists.freedesktop.org
> > > > > > > > > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > > > > > > > _______________________________________________
> > > > > > > > dri-devel mailing list
> > > > > > > > dri-devel@lists.freedesktop.org
> > > > > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

* Re: [PATCH v2 1/3] drm: Add some new format DRM_FORMAT_NVXX_10
  2019-10-09 14:50   ` Daniel Vetter
@ 2019-11-08 15:08     ` Heiko Stübner
  2019-11-08 16:37       ` Daniel Vetter
  0 siblings, 1 reply; 16+ messages in thread
From: Heiko Stübner @ 2019-11-08 15:08 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Sandy Huang, dri-devel, Maarten Lankhorst, Maxime Ripard,
	Sean Paul, David Airlie, linux-kernel

Hi Daniel, Sandy,

Am Mittwoch, 9. Oktober 2019, 16:50:08 CET schrieb Daniel Vetter:
> On Thu, Sep 26, 2019 at 04:24:47PM +0800, Sandy Huang wrote:
> > These new format is supported by some rockchip socs:
> > 
> > DRM_FORMAT_NV12_10/DRM_FORMAT_NV21_10
> > DRM_FORMAT_NV16_10/DRM_FORMAT_NV61_10
> > DRM_FORMAT_NV24_10/DRM_FORMAT_NV42_10
> > 
> > Signed-off-by: Sandy Huang <hjc@rock-chips.com>
> > ---
> >  drivers/gpu/drm/drm_fourcc.c  | 18 ++++++++++++++++++
> >  include/uapi/drm/drm_fourcc.h | 14 ++++++++++++++
> >  2 files changed, 32 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> > index c630064..ccd78a3 100644
> > --- a/drivers/gpu/drm/drm_fourcc.c
> > +++ b/drivers/gpu/drm/drm_fourcc.c
> > @@ -261,6 +261,24 @@ const struct drm_format_info *__drm_format_info(u32 format)
> >  		{ .format = DRM_FORMAT_P016,		.depth = 0,  .num_planes = 2,
> >  		  .char_per_block = { 2, 4, 0 }, .block_w = { 1, 0, 0 }, .block_h = { 1, 0, 0 },
> >  		  .hsub = 2, .vsub = 2, .is_yuv = true},
> > +		{ .format = DRM_FORMAT_NV12_10,		.depth = 0,  .num_planes = 2,
> > +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> > +		  .hsub = 2, .vsub = 2, .is_yuv = true},
> > +		{ .format = DRM_FORMAT_NV21_10,		.depth = 0,  .num_planes = 2,
> > +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> > +		  .hsub = 2, .vsub = 2, .is_yuv = true},
> > +		{ .format = DRM_FORMAT_NV16_10,		.depth = 0,  .num_planes = 2,
> > +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> > +		  .hsub = 2, .vsub = 1, .is_yuv = true},
> > +		{ .format = DRM_FORMAT_NV61_10,		.depth = 0,  .num_planes = 2,
> > +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> > +		  .hsub = 2, .vsub = 1, .is_yuv = true},
> > +		{ .format = DRM_FORMAT_NV24_10,		.depth = 0,  .num_planes = 2,
> > +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> > +		  .hsub = 1, .vsub = 1, .is_yuv = true},
> > +		{ .format = DRM_FORMAT_NV42_10,		.depth = 0,  .num_planes = 2,
> > +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> > +		  .hsub = 1, .vsub = 1, .is_yuv = true},
> >  		{ .format = DRM_FORMAT_P210,		.depth = 0,
> >  		  .num_planes = 2, .char_per_block = { 2, 4, 0 },
> >  		  .block_w = { 1, 0, 0 }, .block_h = { 1, 0, 0 }, .hsub = 2,
> 
> Yup this is what I had in mind with using the block stuff to describe your
> new 10bit yuv formats. Thanks for respining.
> 
> Once we've nailed the exact bit description of the format precisely this
> can be merged imo.

I just saw this series still sitting in my inbox, so was wondering about the
"once we've nailed the exact bit description..." and what is missing for that.

Thanks
Heiko


> > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> > index 3feeaa3..08e2221 100644
> > --- a/include/uapi/drm/drm_fourcc.h
> > +++ b/include/uapi/drm/drm_fourcc.h
> > @@ -238,6 +238,20 @@ extern "C" {
> >  #define DRM_FORMAT_NV42		fourcc_code('N', 'V', '4', '2') /* non-subsampled Cb:Cr plane */
> >  
> >  /*
> > + * 2 plane YCbCr
> > + * index 0 = Y plane, Y3:Y2:Y1:Y0 10:10:10:10
> > + * index 1 = Cb:Cr plane, Cb3:Cr3:Cb2:Cr2:Cb1:Cr1:Cb0:Cr0 10:10:10:10:10:10:10:10
> > + * or
> > + * index 1 = Cr:Cb plane, Cr3:Cb3:Cr2:Cb2:Cr1:Cb1:Cr0:Cb0 10:10:10:10:10:10:10:10
> > + */
> > +#define DRM_FORMAT_NV12_10	fourcc_code('N', 'A', '1', '2') /* 2x2 subsampled Cr:Cb plane */
> > +#define DRM_FORMAT_NV21_10	fourcc_code('N', 'A', '2', '1') /* 2x2 subsampled Cb:Cr plane */
> > +#define DRM_FORMAT_NV16_10	fourcc_code('N', 'A', '1', '6') /* 2x1 subsampled Cr:Cb plane */
> > +#define DRM_FORMAT_NV61_10	fourcc_code('N', 'A', '6', '1') /* 2x1 subsampled Cb:Cr plane */
> > +#define DRM_FORMAT_NV24_10	fourcc_code('N', 'A', '2', '4') /* non-subsampled Cr:Cb plane */
> > +#define DRM_FORMAT_NV42_10	fourcc_code('N', 'A', '4', '2') /* non-subsampled Cb:Cr plane */
> > +
> > +/*
> >   * 2 plane YCbCr MSB aligned
> >   * index 0 = Y plane, [15:0] Y:x [10:6] little endian
> >   * index 1 = Cr:Cb plane, [31:0] Cr:x:Cb:x [10:6:10:6] little endian
> 
> 





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

* Re: [PATCH v2 1/3] drm: Add some new format DRM_FORMAT_NVXX_10
  2019-11-08 15:08     ` Heiko Stübner
@ 2019-11-08 16:37       ` Daniel Vetter
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2019-11-08 16:37 UTC (permalink / raw)
  To: Heiko Stübner
  Cc: Daniel Vetter, Sandy Huang, dri-devel, Maarten Lankhorst,
	Maxime Ripard, Sean Paul, David Airlie, linux-kernel

On Fri, Nov 08, 2019 at 04:08:50PM +0100, Heiko Stübner wrote:
> Hi Daniel, Sandy,
> 
> Am Mittwoch, 9. Oktober 2019, 16:50:08 CET schrieb Daniel Vetter:
> > On Thu, Sep 26, 2019 at 04:24:47PM +0800, Sandy Huang wrote:
> > > These new format is supported by some rockchip socs:
> > > 
> > > DRM_FORMAT_NV12_10/DRM_FORMAT_NV21_10
> > > DRM_FORMAT_NV16_10/DRM_FORMAT_NV61_10
> > > DRM_FORMAT_NV24_10/DRM_FORMAT_NV42_10
> > > 
> > > Signed-off-by: Sandy Huang <hjc@rock-chips.com>
> > > ---
> > >  drivers/gpu/drm/drm_fourcc.c  | 18 ++++++++++++++++++
> > >  include/uapi/drm/drm_fourcc.h | 14 ++++++++++++++
> > >  2 files changed, 32 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> > > index c630064..ccd78a3 100644
> > > --- a/drivers/gpu/drm/drm_fourcc.c
> > > +++ b/drivers/gpu/drm/drm_fourcc.c
> > > @@ -261,6 +261,24 @@ const struct drm_format_info *__drm_format_info(u32 format)
> > >  		{ .format = DRM_FORMAT_P016,		.depth = 0,  .num_planes = 2,
> > >  		  .char_per_block = { 2, 4, 0 }, .block_w = { 1, 0, 0 }, .block_h = { 1, 0, 0 },
> > >  		  .hsub = 2, .vsub = 2, .is_yuv = true},
> > > +		{ .format = DRM_FORMAT_NV12_10,		.depth = 0,  .num_planes = 2,
> > > +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> > > +		  .hsub = 2, .vsub = 2, .is_yuv = true},
> > > +		{ .format = DRM_FORMAT_NV21_10,		.depth = 0,  .num_planes = 2,
> > > +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> > > +		  .hsub = 2, .vsub = 2, .is_yuv = true},
> > > +		{ .format = DRM_FORMAT_NV16_10,		.depth = 0,  .num_planes = 2,
> > > +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> > > +		  .hsub = 2, .vsub = 1, .is_yuv = true},
> > > +		{ .format = DRM_FORMAT_NV61_10,		.depth = 0,  .num_planes = 2,
> > > +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> > > +		  .hsub = 2, .vsub = 1, .is_yuv = true},
> > > +		{ .format = DRM_FORMAT_NV24_10,		.depth = 0,  .num_planes = 2,
> > > +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> > > +		  .hsub = 1, .vsub = 1, .is_yuv = true},
> > > +		{ .format = DRM_FORMAT_NV42_10,		.depth = 0,  .num_planes = 2,
> > > +		  .char_per_block = { 5, 10, 0 }, .block_w = { 4, 4, 0 }, .block_h = { 4, 4, 0 },
> > > +		  .hsub = 1, .vsub = 1, .is_yuv = true},
> > >  		{ .format = DRM_FORMAT_P210,		.depth = 0,
> > >  		  .num_planes = 2, .char_per_block = { 2, 4, 0 },
> > >  		  .block_w = { 1, 0, 0 }, .block_h = { 1, 0, 0 }, .hsub = 2,
> > 
> > Yup this is what I had in mind with using the block stuff to describe your
> > new 10bit yuv formats. Thanks for respining.
> > 
> > Once we've nailed the exact bit description of the format precisely this
> > can be merged imo.
> 
> I just saw this series still sitting in my inbox, so was wondering about the
> "once we've nailed the exact bit description..." and what is missing for that.

Since my name is on this mail ... what I meant here is that in principle
I'm ok, but someone else needs to check that we've documented all the
details properly. And once that review is done they or you can also merge
it. From my side not going to dig more into this, we have tons of
reviewers/committers.
-Daniel


> 
> Thanks
> Heiko
> 
> 
> > > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> > > index 3feeaa3..08e2221 100644
> > > --- a/include/uapi/drm/drm_fourcc.h
> > > +++ b/include/uapi/drm/drm_fourcc.h
> > > @@ -238,6 +238,20 @@ extern "C" {
> > >  #define DRM_FORMAT_NV42		fourcc_code('N', 'V', '4', '2') /* non-subsampled Cb:Cr plane */
> > >  
> > >  /*
> > > + * 2 plane YCbCr
> > > + * index 0 = Y plane, Y3:Y2:Y1:Y0 10:10:10:10
> > > + * index 1 = Cb:Cr plane, Cb3:Cr3:Cb2:Cr2:Cb1:Cr1:Cb0:Cr0 10:10:10:10:10:10:10:10
> > > + * or
> > > + * index 1 = Cr:Cb plane, Cr3:Cb3:Cr2:Cb2:Cr1:Cb1:Cr0:Cb0 10:10:10:10:10:10:10:10
> > > + */
> > > +#define DRM_FORMAT_NV12_10	fourcc_code('N', 'A', '1', '2') /* 2x2 subsampled Cr:Cb plane */
> > > +#define DRM_FORMAT_NV21_10	fourcc_code('N', 'A', '2', '1') /* 2x2 subsampled Cb:Cr plane */
> > > +#define DRM_FORMAT_NV16_10	fourcc_code('N', 'A', '1', '6') /* 2x1 subsampled Cr:Cb plane */
> > > +#define DRM_FORMAT_NV61_10	fourcc_code('N', 'A', '6', '1') /* 2x1 subsampled Cb:Cr plane */
> > > +#define DRM_FORMAT_NV24_10	fourcc_code('N', 'A', '2', '4') /* non-subsampled Cr:Cb plane */
> > > +#define DRM_FORMAT_NV42_10	fourcc_code('N', 'A', '4', '2') /* non-subsampled Cb:Cr plane */
> > > +
> > > +/*
> > >   * 2 plane YCbCr MSB aligned
> > >   * index 0 = Y plane, [15:0] Y:x [10:6] little endian
> > >   * index 1 = Cr:Cb plane, [31:0] Cr:x:Cb:x [10:6:10:6] little endian
> > 
> > 
> 
> 
> 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

end of thread, other threads:[~2019-11-08 16:37 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1569486289-152061-1-git-send-email-hjc@rock-chips.com>
2019-09-26  8:24 ` [PATCH v2 1/3] drm: Add some new format DRM_FORMAT_NVXX_10 Sandy Huang
2019-09-29  3:26   ` [v2,1/3] " james qian wang (Arm Technology China)
2019-09-30 10:48   ` [PATCH v2 1/3] " Ville Syrjälä
2019-10-08  2:40     ` sandy.huang
2019-10-08 11:33       ` Ville Syrjälä
2019-10-08 11:49         ` sandy.huang
2019-10-11  3:35           ` sandy.huang
     [not found]             ` <20191011064433.GA18503@jamwan02-TSP300>
2019-10-11  7:07               ` sandy.huang
     [not found]                 ` <20191011072250.GA20592@jamwan02-TSP300>
2019-10-11  7:32                   ` sandy.huang
     [not found]                     ` <20191011083247.GA22224@jamwan02-TSP300>
2019-10-11  9:45                       ` sandy.huang
2019-10-14  8:34                         ` Daniel Vetter
2019-10-09 14:50   ` Daniel Vetter
2019-11-08 15:08     ` Heiko Stübner
2019-11-08 16:37       ` Daniel Vetter
2019-09-26  8:24 ` [PATCH v2 2/3] drm/rockchip: Add rockchip_vop_get_offset to get offset Sandy Huang
2019-09-26  8:24 ` [PATCH v2 3/3] drm/rockchip: Add support 10bit yuv format Sandy Huang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).