Re: [PATCH v2 1/3] drm: Add some new format DRM_FORMAT_NVXX_10

From: sandy.huang
Date: Fri Oct 11 2019 - 03:07:39 EST

Next message: Michal Simek: "Re: [PATCH] rtc: xilinx: Fix calibval variable type"
Previous message: Peter Zijlstra: "Re: [PATCH RFC] perf_event: Add support for LSM and SELinux checks"
In reply to: sandy.huang: "Re: [PATCH v2 1/3] drm: Add some new format DRM_FORMAT_NVXX_10"
Next in thread: sandy.huang: "Re: [PATCH v2 1/3] drm: Add some new format DRM_FORMAT_NVXX_10"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

å 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@xxxxxxxxxxxxxx>
---
ÂÂ 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Next message: Michal Simek: "Re: [PATCH] rtc: xilinx: Fix calibval variable type"
Previous message: Peter Zijlstra: "Re: [PATCH RFC] perf_event: Add support for LSM and SELinux checks"
In reply to: sandy.huang: "Re: [PATCH v2 1/3] drm: Add some new format DRM_FORMAT_NVXX_10"
Next in thread: sandy.huang: "Re: [PATCH v2 1/3] drm: Add some new format DRM_FORMAT_NVXX_10"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]