On 02/27/2017 09:41 AM, Ville Syrjälä wrote:
On Mon, Feb 27, 2017 at 09:21:09AM -0800, clinton.a.taylor@xxxxxxxxx wrote:From: Clint Taylor <clinton.a.taylor@xxxxxxxxx> P010 is a planar 4:2:0 YUV with interleaved UV plane, 10 bits per channel video format. Rockchip's vop support this video format(little endian only) as the input video format. P016 is a planar 4:2:0 YUV 12 bits per channel P016 is a planar 4:2:0 YUV with interleaved UV plane, 16 bits per channel video format. V3: Added P012 and fixed cpp for P010 V4: format definition refined per review Cc: Daniel Stone <daniel@xxxxxxxxxxxxx> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> Signed-off-by: Randy Li <ayaka@xxxxxxxxxxx> Signed-off-by: Clint Taylor <clinton.a.taylor@xxxxxxxxx> --- drivers/gpu/drm/drm_fourcc.c | 4 ++++ include/uapi/drm/drm_fourcc.h | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c index 90d2cc8..5494764 100644 --- a/drivers/gpu/drm/drm_fourcc.c +++ b/drivers/gpu/drm/drm_fourcc.c @@ -165,6 +165,10 @@ const struct drm_format_info *__drm_format_info(u32 format) { .format = DRM_FORMAT_UYVY, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 }, { .format = DRM_FORMAT_VYUY, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 }, { .format = DRM_FORMAT_AYUV, .depth = 0, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, + /* FIXME a pixel in Y for P010 is 10 bits */ + { .format = DRM_FORMAT_P010, .depth = 0, .num_planes = 2, .cpp = { 2, 4, 0 }, .hsub = 2, .vsub = 2 }, + { .format = DRM_FORMAT_P012, .depth = 0, .num_planes = 2, .cpp = { 2, 4, 0 }, .hsub = 2, .vsub = 2 }, + { .format = DRM_FORMAT_P016, .depth = 0, .num_planes = 2, .cpp = { 2, 4, 0 }, .hsub = 2, .vsub = 2 }, };What's this hunk doing here?
This hunk defines the memory layout definition to DRM, so framebuffers can be checked to make sure they are large enough for the format. Can you describe your concern here?
unsigned int i; diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index ef20abb..ad94464 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -128,6 +128,24 @@ #define DRM_FORMAT_NV42 fourcc_code('N', 'V', '4', '2') /* non-subsampled Cb:Cr plane */ /* + * 2 plane YCbCr MSB aligned P0?? formats + * index 0 = Y plane, word array [15:6] P010 + * or + * index 0 = Y plane, word array [15:4] P012 + * or + * index 0 = Y plane, word array [15:0] P016 + * + * index 1 = Cb:Cr plane, [31:22] Cb [15:6] Cr little endian P010 + * or + * index 1 = Cb:Cr plane, [31:20] Cb [15:4] Cr little endian P012 + * or + * index 1 = Cb:Cr plane, [31:16] Cb [15:0] Cr little endian P016 + */Still looks somewhat out of place when compared with the rest of the file.
The other YUV entries in the file all have 8 bit definitions and are easily grouped together. The P0?? formats change their number of bits. The only way I see is to make the comments look like the other YUV formats is to make each format have its own comment block describing the layout. I feel it's better to group them together since the difference between them (bpc) is minor.
perhaps moving the bit definitions to the defines, but then again it won't look like the other YUV format. It will look more like the RGB defines.
-Clint
+#define DRM_FORMAT_P010 fourcc_code('P', '0', '1', '0') /* 2x2 subsampled Cb:Cr plane 10 bits per channel */ +#define DRM_FORMAT_P012 fourcc_code('P', '0', '1', '2') /* 2x2 subsampled Cb:Cr plane 12 bits per channel */ +#define DRM_FORMAT_P016 fourcc_code('P', '0', '1', '6') /* 2x2 subsampled Cb:Cr plane 16 bits per channel */ + +/* * 3 plane YCbCr * index 0: Y plane, [7:0] Y * index 1: Cb plane, [7:0] Cb -- 1.7.9.5
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
