diff mbox series

drm: Add R10 and R12 FourCC

Message ID 20211027233140.12268-1-laurent.pinchart@ideasonboard.com (mailing list archive)
State New, archived
Headers show
Series drm: Add R10 and R12 FourCC | expand

Commit Message

Laurent Pinchart Oct. 27, 2021, 11:31 p.m. UTC
Add FourCCs for 10- and 12-bit red formats with padding to 16 bits.
They correspond to the V4L2 10- and 12-bit greyscale (V4L2_PIX_FMT_Y10
and V4L2_PIX_FMT_Y12) formats, as well as the Bayer formats with the
same bit depth (V4L2_PIX_FMT_SBGGR{10,12} and all other Bayer pattern
permutations).

These formats are not used by any kernel driver at this point, but need
to be exposed to applications by libcamera, which uses DRM FourCCs for
pixel formats.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/drm_fourcc.c  | 2 ++
 include/uapi/drm/drm_fourcc.h | 6 ++++++
 2 files changed, 8 insertions(+)


base-commit: 367fe8dc299c968eabdae890536d55d80ea55e01

Comments

Simon Ser Oct. 28, 2021, 9:49 p.m. UTC | #1
> +/* 10 bpp Red */
> +#define DRM_FORMAT_R10		fourcc_code('R', '1', '0', ' ') /* [15:0] x:R 6:10 little endian */
> +
> +/* 12 bpp Red */
> +#define DRM_FORMAT_R12		fourcc_code('R', '1', '2', ' ') /* [15:0] x:R 4:12 little endian */

Are these codes arbitrary, or are they taken from somewhere else?

If they are arbitrary, maybe it'd be better to pick XR10 and XR12 instead of
R10 and R12, to allow a future patch to add other format codes with the padding
at the end if we ever need that.
Simon Ser Oct. 28, 2021, 9:51 p.m. UTC | #2
On Thursday, October 28th, 2021 at 23:49, Simon Ser <contact@emersion.fr> wrote:

> Are these codes arbitrary, or are they taken from somewhere else?
> If they are arbitrary, maybe it'd be better to pick XR10 and XR12 instead of
> R10 and R12, to allow a future patch to add other format codes with the padding
> at the end if we ever need that.

Ah, it seems like XR12 is already taken by XRGB4444…
Laurent Pinchart Oct. 28, 2021, 11:30 p.m. UTC | #3
Hi Simon,

On Thu, Oct 28, 2021 at 09:49:18PM +0000, Simon Ser wrote:
> > +/* 10 bpp Red */
> > +#define DRM_FORMAT_R10		fourcc_code('R', '1', '0', ' ') /* [15:0] x:R 6:10 little endian */
> > +
> > +/* 12 bpp Red */
> > +#define DRM_FORMAT_R12		fourcc_code('R', '1', '2', ' ') /* [15:0] x:R 4:12 little endian */
> 
> Are these codes arbitrary, or are they taken from somewhere else?
> 
> If they are arbitrary, maybe it'd be better to pick XR10 and XR12 instead of
> R10 and R12, to allow a future patch to add other format codes with the padding
> at the end if we ever need that.

They are arbitrary indeed, but modelled after DRM_FORMAT_R8 which used
'R8  '. As you've noticed, XR12 is already taken.

I don't think we'll need a format with padding at that end (with data
aligned to tbe MSB) as that would essentially be DRM_FORMAT_R16.
Simon Ser Oct. 31, 2021, 5:05 p.m. UTC | #4
On Friday, October 29th, 2021 at 01:30, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:

> I don't think we'll need a format with padding at that end (with data
> aligned to tbe MSB) as that would essentially be DRM_FORMAT_R16.

The X channel doesn't mean that the padding is zeroes: the padding has
undefined contents. So using R16 wouldn't quite work. Playing with
DRM_FORMAT_BIG_ENDIAN (!) could do the trick.
Simon Ser Oct. 31, 2021, 5:08 p.m. UTC | #5
On Sunday, October 31st, 2021 at 18:05, Simon Ser <contact@emersion.fr> wrote:

> Playing with
> DRM_FORMAT_BIG_ENDIAN (!) could do the trick.

Hm, nevermind, that wouldn't work, as this would do something like
RRXXXXXXRRRRRRRR.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 783844bfecc1..25837b1d6639 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -134,6 +134,8 @@  const struct drm_format_info *__drm_format_info(u32 format)
 	static const struct drm_format_info formats[] = {
 		{ .format = DRM_FORMAT_C8,		.depth = 8,  .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
 		{ .format = DRM_FORMAT_R8,		.depth = 8,  .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_R10,		.depth = 10, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_R12,		.depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
 		{ .format = DRM_FORMAT_RGB332,		.depth = 8,  .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
 		{ .format = DRM_FORMAT_BGR233,		.depth = 8,  .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
 		{ .format = DRM_FORMAT_XRGB4444,	.depth = 0,  .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 45a914850be0..7f652c96845b 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -104,6 +104,12 @@  extern "C" {
 /* 8 bpp Red */
 #define DRM_FORMAT_R8		fourcc_code('R', '8', ' ', ' ') /* [7:0] R */
 
+/* 10 bpp Red */
+#define DRM_FORMAT_R10		fourcc_code('R', '1', '0', ' ') /* [15:0] x:R 6:10 little endian */
+
+/* 12 bpp Red */
+#define DRM_FORMAT_R12		fourcc_code('R', '1', '2', ' ') /* [15:0] x:R 4:12 little endian */
+
 /* 16 bpp Red */
 #define DRM_FORMAT_R16		fourcc_code('R', '1', '6', ' ') /* [15:0] R little endian */