diff mbox

[RFC/PATCH] drm: Add XRGB8626262 (RGB 6:6:6) pixel format

Message ID 1364375969-5266-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com (mailing list archive)
State New, archived
Headers show

Commit Message

Laurent Pinchart March 27, 2013, 9:19 a.m. UTC
This format is an odd beast, implemented by Renesas R-Car hardware. It
stores RGB 6:6:6 pixels in 32 bits as

[31:0] x:R:x:G:x:B:x 8:6:2:6:2:6:2 little endian

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---

Hello,

I came across this weird format on a Renesas SoC display controller. This is
essentially XRGB8888 with the two low order bits of each component ignored by
the hardware.

The proposed name is intentionally over-descriptive to trigger comments
(hopefully resulting in a proposal for a better name :-)).

To be honest I'm not too sure what kind of use case such a format could have.

Comments

Ville Syrjälä March 27, 2013, 11:06 a.m. UTC | #1
On Wed, Mar 27, 2013 at 10:19:29AM +0100, Laurent Pinchart wrote:
> This format is an odd beast, implemented by Renesas R-Car hardware. It
> stores RGB 6:6:6 pixels in 32 bits as
> 
> [31:0] x:R:x:G:x:B:x 8:6:2:6:2:6:2 little endian
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
> 
> Hello,
> 
> I came across this weird format on a Renesas SoC display controller. This is
> essentially XRGB8888 with the two low order bits of each component ignored by
> the hardware.

It sounds like it's no different than shoveling XRGB8888 down a 6bpc
pipe w/o dithering.

Could we just pretend it's XRGB8888, or can the low order bits have
some special meaning which would require treating them as special?
Laurent Pinchart March 27, 2013, 2:09 p.m. UTC | #2
Hi Ville,

On Wednesday 27 March 2013 13:06:20 Ville Syrjälä wrote:
> On Wed, Mar 27, 2013 at 10:19:29AM +0100, Laurent Pinchart wrote:
> > This format is an odd beast, implemented by Renesas R-Car hardware. It
> > stores RGB 6:6:6 pixels in 32 bits as
> > 
> > [31:0] x:R:x:G:x:B:x 8:6:2:6:2:6:2 little endian
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> > Hello,
> > 
> > I came across this weird format on a Renesas SoC display controller. This
> > is essentially XRGB8888 with the two low order bits of each component
> > ignored by the hardware.
> 
> It sounds like it's no different than shoveling XRGB8888 down a 6bpc
> pipe w/o dithering.
> 
> Could we just pretend it's XRGB8888, or can the low order bits have
> some special meaning which would require treating them as special?

The hardware supports both XRGB8888 and the weird RGB 666 format, so I need a 
new 4CC if I want to expose both to userspace.

If the display uses a 6bpc pipe XRGB888 and RGB 666 should be identical. If 
the display uses a 8bpc pipe then the two formats will be different. What 
remains to be found is a use case where an application would fill the two low 
order bits with a non-zero value and expect the hardware to ignore them.
Ville Syrjälä March 27, 2013, 2:16 p.m. UTC | #3
On Wed, Mar 27, 2013 at 03:09:48PM +0100, Laurent Pinchart wrote:
> Hi Ville,
> 
> On Wednesday 27 March 2013 13:06:20 Ville Syrjälä wrote:
> > On Wed, Mar 27, 2013 at 10:19:29AM +0100, Laurent Pinchart wrote:
> > > This format is an odd beast, implemented by Renesas R-Car hardware. It
> > > stores RGB 6:6:6 pixels in 32 bits as
> > > 
> > > [31:0] x:R:x:G:x:B:x 8:6:2:6:2:6:2 little endian
> > > 
> > > Signed-off-by: Laurent Pinchart
> > > <laurent.pinchart+renesas@ideasonboard.com>
> > > ---
> > > 
> > > Hello,
> > > 
> > > I came across this weird format on a Renesas SoC display controller. This
> > > is essentially XRGB8888 with the two low order bits of each component
> > > ignored by the hardware.
> > 
> > It sounds like it's no different than shoveling XRGB8888 down a 6bpc
> > pipe w/o dithering.
> > 
> > Could we just pretend it's XRGB8888, or can the low order bits have
> > some special meaning which would require treating them as special?
> 
> The hardware supports both XRGB8888 and the weird RGB 666 format, so I need a 
> new 4CC if I want to expose both to userspace.

I see.

> If the display uses a 6bpc pipe XRGB888 and RGB 666 should be identical. If 
> the display uses a 8bpc pipe then the two formats will be different. What 
> remains to be found is a use case where an application would fill the two low 
> order bits with a non-zero value and expect the hardware to ignore them.

Right. Unless the 6:6:6 format has some tangible benefit, like being
faster to render, or using the extra bits for some other information,
personally I'd lean towards not exposing it.
Laurent Pinchart March 27, 2013, 2:21 p.m. UTC | #4
Hi Ville,

On Wednesday 27 March 2013 16:16:26 Ville Syrjälä wrote:
> On Wed, Mar 27, 2013 at 03:09:48PM +0100, Laurent Pinchart wrote:
> > On Wednesday 27 March 2013 13:06:20 Ville Syrjälä wrote:
> > > On Wed, Mar 27, 2013 at 10:19:29AM +0100, Laurent Pinchart wrote:
> > > > This format is an odd beast, implemented by Renesas R-Car hardware. It
> > > > stores RGB 6:6:6 pixels in 32 bits as
> > > > 
> > > > [31:0] x:R:x:G:x:B:x 8:6:2:6:2:6:2 little endian
> > > > 
> > > > Signed-off-by: Laurent Pinchart
> > > > <laurent.pinchart+renesas@ideasonboard.com>
> > > > ---
> > > > 
> > > > Hello,
> > > > 
> > > > I came across this weird format on a Renesas SoC display controller.
> > > > This is essentially XRGB8888 with the two low order bits of each
> > > > component ignored by the hardware.
> > > 
> > > It sounds like it's no different than shoveling XRGB8888 down a 6bpc
> > > pipe w/o dithering.
> > > 
> > > Could we just pretend it's XRGB8888, or can the low order bits have
> > > some special meaning which would require treating them as special?
> > 
> > The hardware supports both XRGB8888 and the weird RGB 666 format, so I
> > need a new 4CC if I want to expose both to userspace.
> 
> I see.
> 
> > If the display uses a 6bpc pipe XRGB888 and RGB 666 should be identical.
> > If the display uses a 8bpc pipe then the two formats will be different.
> > What remains to be found is a use case where an application would fill the
> > two low order bits with a non-zero value and expect the hardware to ignore
> > them.
>
> Right. Unless the 6:6:6 format has some tangible benefit, like being
> faster to render, or using the extra bits for some other information,
> personally I'd lean towards not exposing it.

I agree with you. I was mostly interested to see if someone had any use case 
for this format. Unless one crops up I will then drop this patch.

Thank you for sharing your opinion.
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index f2d667b..7e1a19d 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2210,6 +2210,7 @@  static int format_check(const struct drm_mode_fb_cmd2 *r)
 	case DRM_FORMAT_BGR565:
 	case DRM_FORMAT_RGB888:
 	case DRM_FORMAT_BGR888:
+	case DRM_FORMAT_XRGB8626262:
 	case DRM_FORMAT_XRGB8888:
 	case DRM_FORMAT_XBGR8888:
 	case DRM_FORMAT_RGBX8888:
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 646ae5f..1f01161 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -67,6 +67,8 @@ 
 #define DRM_FORMAT_BGR888	fourcc_code('B', 'G', '2', '4') /* [23:0] B:G:R little endian */
 
 /* 32 bpp RGB */
+#define DRM_FORMAT_XRGB8626262	fourcc_code('X', 'R', '1', '8') /* [31:0] x:R:x:G:x:B:x 8:6:2:6:2:6:2 little endian */
+
 #define DRM_FORMAT_XRGB8888	fourcc_code('X', 'R', '2', '4') /* [31:0] x:R:G:B 8:8:8:8 little endian */
 #define DRM_FORMAT_XBGR8888	fourcc_code('X', 'B', '2', '4') /* [31:0] x:B:G:R 8:8:8:8 little endian */
 #define DRM_FORMAT_RGBX8888	fourcc_code('R', 'X', '2', '4') /* [31:0] R:G:B:x 8:8:8:8 little endian */