* [PATCHv4][ 1/7] [media] v4l2: add new V4L2_PIX_FMT_RGB666 pixel format.
@ 2013-11-13 9:23 Denis Carikli
2013-11-13 9:23 ` [PATCHv4][ 3/7] staging: imx-drm: Add RGB666 support for parallel display Denis Carikli
0 siblings, 1 reply; 5+ messages in thread
From: Denis Carikli @ 2013-11-13 9:23 UTC (permalink / raw)
To: Shawn Guo
Cc: Sascha Hauer, linux-arm-kernel, Philipp Zabel,
Mauro Carvalho Chehab, Laurent Pinchart, dri-devel, Denis Carikli,
Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, devicetree, Greg Kroah-Hartman, driverdev-devel,
David Airlie, linux-media, Eric Bénard
That new macro is needed by the imx_drm staging driver
for supporting the QVGA display of the eukrea-cpuimx51 board.
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: devicetree@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: driverdev-devel@linuxdriverproject.org
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: linux-media@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
Acked-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
ChangeLog v3->v4:
- Added Laurent Pinchart's Ack.
ChangeLog v2->v3:
- Added some interested people in the Cc list.
- Added Mauro Carvalho Chehab's Ack.
- Added documentation.
---
.../DocBook/media/v4l/pixfmt-packed-rgb.xml | 78 ++++++++++++++++++++
include/uapi/linux/videodev2.h | 1 +
2 files changed, 79 insertions(+)
diff --git a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
index 166c8d6..f6a3e84 100644
--- a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
+++ b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
@@ -279,6 +279,45 @@ colorspace <constant>V4L2_COLORSPACE_SRGB</constant>.</para>
<entry></entry>
<entry></entry>
</row>
+ <row id="V4L2-PIX-FMT-RGB666">
+ <entry><constant>V4L2_PIX_FMT_RGB666</constant></entry>
+ <entry>'RGBH'</entry>
+ <entry></entry>
+ <entry>r<subscript>5</subscript></entry>
+ <entry>r<subscript>4</subscript></entry>
+ <entry>r<subscript>3</subscript></entry>
+ <entry>r<subscript>2</subscript></entry>
+ <entry>r<subscript>1</subscript></entry>
+ <entry>r<subscript>0</subscript></entry>
+ <entry>g<subscript>5</subscript></entry>
+ <entry>g<subscript>4</subscript></entry>
+ <entry></entry>
+ <entry>g<subscript>3</subscript></entry>
+ <entry>g<subscript>2</subscript></entry>
+ <entry>g<subscript>1</subscript></entry>
+ <entry>g<subscript>0</subscript></entry>
+ <entry>b<subscript>5</subscript></entry>
+ <entry>b<subscript>4</subscript></entry>
+ <entry>b<subscript>3</subscript></entry>
+ <entry>b<subscript>2</subscript></entry>
+ <entry></entry>
+ <entry>b<subscript>1</subscript></entry>
+ <entry>b<subscript>0</subscript></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ </row>
<row id="V4L2-PIX-FMT-BGR24">
<entry><constant>V4L2_PIX_FMT_BGR24</constant></entry>
<entry>'BGR3'</entry>
@@ -781,6 +820,45 @@ defined in error. Drivers may interpret them as in <xref
<entry></entry>
<entry></entry>
</row>
+ <row><!-- id="V4L2-PIX-FMT-RGB666" -->
+ <entry><constant>V4L2_PIX_FMT_RGB666</constant></entry>
+ <entry>'RGBH'</entry>
+ <entry></entry>
+ <entry>r<subscript>5</subscript></entry>
+ <entry>r<subscript>4</subscript></entry>
+ <entry>r<subscript>3</subscript></entry>
+ <entry>r<subscript>2</subscript></entry>
+ <entry>r<subscript>1</subscript></entry>
+ <entry>r<subscript>0</subscript></entry>
+ <entry>g<subscript>5</subscript></entry>
+ <entry>g<subscript>4</subscript></entry>
+ <entry></entry>
+ <entry>g<subscript>3</subscript></entry>
+ <entry>g<subscript>2</subscript></entry>
+ <entry>g<subscript>1</subscript></entry>
+ <entry>g<subscript>0</subscript></entry>
+ <entry>b<subscript>5</subscript></entry>
+ <entry>b<subscript>4</subscript></entry>
+ <entry>b<subscript>3</subscript></entry>
+ <entry>b<subscript>2</subscript></entry>
+ <entry></entry>
+ <entry>b<subscript>1</subscript></entry>
+ <entry>b<subscript>0</subscript></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ </row>
<row><!-- id="V4L2-PIX-FMT-BGR24" -->
<entry><constant>V4L2_PIX_FMT_BGR24</constant></entry>
<entry>'BGR3'</entry>
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 437f1b0..e8ff410 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -294,6 +294,7 @@ struct v4l2_pix_format {
#define V4L2_PIX_FMT_RGB555X v4l2_fourcc('R', 'G', 'B', 'Q') /* 16 RGB-5-5-5 BE */
#define V4L2_PIX_FMT_RGB565X v4l2_fourcc('R', 'G', 'B', 'R') /* 16 RGB-5-6-5 BE */
#define V4L2_PIX_FMT_BGR666 v4l2_fourcc('B', 'G', 'R', 'H') /* 18 BGR-6-6-6 */
+#define V4L2_PIX_FMT_RGB666 v4l2_fourcc('R', 'G', 'B', 'H') /* 18 RGB-6-6-6 */
#define V4L2_PIX_FMT_BGR24 v4l2_fourcc('B', 'G', 'R', '3') /* 24 BGR-8-8-8 */
#define V4L2_PIX_FMT_RGB24 v4l2_fourcc('R', 'G', 'B', '3') /* 24 RGB-8-8-8 */
#define V4L2_PIX_FMT_BGR32 v4l2_fourcc('B', 'G', 'R', '4') /* 32 BGR-8-8-8-8 */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCHv4][ 3/7] staging: imx-drm: Add RGB666 support for parallel display.
2013-11-13 9:23 [PATCHv4][ 1/7] [media] v4l2: add new V4L2_PIX_FMT_RGB666 pixel format Denis Carikli
@ 2013-11-13 9:23 ` Denis Carikli
2013-11-13 18:43 ` Troy Kisky
0 siblings, 1 reply; 5+ messages in thread
From: Denis Carikli @ 2013-11-13 9:23 UTC (permalink / raw)
To: Shawn Guo
Cc: Sascha Hauer, linux-arm-kernel, Philipp Zabel,
Mauro Carvalho Chehab, Laurent Pinchart, dri-devel, Denis Carikli,
Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, devicetree, Greg Kroah-Hartman, driverdev-devel,
David Airlie, linux-media, Eric Bénard
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: devicetree@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: driverdev-devel@linuxdriverproject.org
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: linux-media@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v2->v3:
- Added some interested people in the Cc list.
- Removed the commit message long desciption that was just a copy of the short
description.
- Rebased the patch.
- Fixed a copy-paste error in the ipu_dc_map_clear parameter.
---
.../bindings/staging/imx-drm/fsl-imx-drm.txt | 2 +-
drivers/staging/imx-drm/ipu-v3/ipu-dc.c | 9 +++++++++
drivers/staging/imx-drm/parallel-display.c | 2 ++
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt b/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
index b876d49..2d24425 100644
--- a/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
+++ b/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
@@ -29,7 +29,7 @@ Required properties:
- crtc: the crtc this display is connected to, see below
Optional properties:
- interface_pix_fmt: How this display is connected to the
- crtc. Currently supported types: "rgb24", "rgb565", "bgr666"
+ crtc. Currently supported types: "rgb24", "rgb565", "bgr666", "rgb666"
- edid: verbatim EDID data block describing attached display.
- ddc: phandle describing the i2c bus handling the display data
channel
diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
index d0e3bc3..bcc7680 100644
--- a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
+++ b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
@@ -92,6 +92,7 @@ enum ipu_dc_map {
IPU_DC_MAP_GBR24, /* TVEv2 */
IPU_DC_MAP_BGR666,
IPU_DC_MAP_BGR24,
+ IPU_DC_MAP_RGB666,
};
struct ipu_dc {
@@ -155,6 +156,8 @@ static int ipu_pixfmt_to_map(u32 fmt)
return IPU_DC_MAP_BGR666;
case V4L2_PIX_FMT_BGR24:
return IPU_DC_MAP_BGR24;
+ case V4L2_PIX_FMT_RGB666:
+ return IPU_DC_MAP_RGB666;
default:
return -EINVAL;
}
@@ -404,6 +407,12 @@ int ipu_dc_init(struct ipu_soc *ipu, struct device *dev,
ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 1, 15, 0xff); /* green */
ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 0, 23, 0xff); /* blue */
+ /* rgb666 */
+ ipu_dc_map_clear(priv, IPU_DC_MAP_RGB666);
+ ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 2, 17, 0xfc); /* red */
+ ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 1, 11, 0xfc); /* green */
+ ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 0, 5, 0xfc); /* blue */
+
return 0;
}
diff --git a/drivers/staging/imx-drm/parallel-display.c b/drivers/staging/imx-drm/parallel-display.c
index b34f3a3..46b6fce 100644
--- a/drivers/staging/imx-drm/parallel-display.c
+++ b/drivers/staging/imx-drm/parallel-display.c
@@ -248,6 +248,8 @@ static int imx_pd_probe(struct platform_device *pdev)
imxpd->interface_pix_fmt = V4L2_PIX_FMT_RGB565;
else if (!strcmp(fmt, "bgr666"))
imxpd->interface_pix_fmt = V4L2_PIX_FMT_BGR666;
+ else if (!strcmp(fmt, "rgb666"))
+ imxpd->interface_pix_fmt = V4L2_PIX_FMT_RGB666;
}
imxpd->dev = &pdev->dev;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCHv4][ 3/7] staging: imx-drm: Add RGB666 support for parallel display.
2013-11-13 9:23 ` [PATCHv4][ 3/7] staging: imx-drm: Add RGB666 support for parallel display Denis Carikli
@ 2013-11-13 18:43 ` Troy Kisky
2013-11-13 19:12 ` Russell King - ARM Linux
0 siblings, 1 reply; 5+ messages in thread
From: Troy Kisky @ 2013-11-13 18:43 UTC (permalink / raw)
To: Denis Carikli, Shawn Guo
Cc: Sascha Hauer, linux-arm-kernel, Philipp Zabel,
Mauro Carvalho Chehab, Laurent Pinchart, dri-devel, Rob Herring,
Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
devicetree, Greg Kroah-Hartman, driverdev-devel, David Airlie,
linux-media, Eric Bénard
On 11/13/2013 2:23 AM, Denis Carikli wrote:
>
> + /* rgb666 */
> + ipu_dc_map_clear(priv, IPU_DC_MAP_RGB666);
> + ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 2, 17, 0xfc); /* red */
> + ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 1, 11, 0xfc); /* green */
> + ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 0, 5, 0xfc); /* blue */
> +
> return 0;
> }
>
>
Since, rgb24 and bgr24 reverse the byte numbers
/* rgb24 */
ipu_dc_map_clear(priv, IPU_DC_MAP_RGB24);
ipu_dc_map_config(priv, IPU_DC_MAP_RGB24, 0, 7, 0xff); /* blue */
ipu_dc_map_config(priv, IPU_DC_MAP_RGB24, 1, 15, 0xff); /* green */
ipu_dc_map_config(priv, IPU_DC_MAP_RGB24, 2, 23, 0xff); /* red */
/* bgr24 */
ipu_dc_map_clear(priv, IPU_DC_MAP_BGR24);
ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 2, 7, 0xff); /* red */
ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 1, 15, 0xff); /* green */
ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 0, 23, 0xff); /* blue */
Shouldn't rgb666 and bgr666 do the same?
Currently we have,
/* bgr666 */
ipu_dc_map_clear(priv, IPU_DC_MAP_BGR666);
ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 0, 5, 0xfc); /* blue */
ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 1, 11, 0xfc); /*
green */
ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 2, 17, 0xfc); /* red */
Where I'd expect to see
/* bgr666 */
ipu_dc_map_clear(priv, IPU_DC_MAP_BGR666);
ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 0, 17, 0xfc); /* blue */
ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 1, 11, 0xfc); /*
green */
ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 2, 5, 0xfc); /* red */
So, it looks like you are adding a duplicate of bgr666 because bgr666 is
wrong.
Also, I'd prefer to keep the entries in 0,1,2 byte number order(blue,
green, red,
assuming byte 0 is always blue) so that duplicates are easier to spot.
Not related to this patch, but the comments on gbr24 appear wrong as well.
/* gbr24 */
ipu_dc_map_clear(priv, IPU_DC_MAP_GBR24);
ipu_dc_map_config(priv, IPU_DC_MAP_GBR24, 2, 15, 0xff); /* green */
ipu_dc_map_config(priv, IPU_DC_MAP_GBR24, 1, 7, 0xff); /* blue */
ipu_dc_map_config(priv, IPU_DC_MAP_GBR24, 0, 23, 0xff); /* red */
Should be
/* brg24 */
ipu_dc_map_clear(priv, IPU_DC_MAP_BRG24);
ipu_dc_map_config(priv, IPU_DC_MAP_BRG24, 0, 23, 0xff); /* blue*/
ipu_dc_map_config(priv, IPU_DC_MAP_BRG24, 1, 7, 0xff); /* green */
ipu_dc_map_config(priv, IPU_DC_MAP_BRG24, 2, 15, 0xff); /* red */
Of course, my understanding may be totally wrong. If so, please show me
the light!
Troy
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCHv4][ 3/7] staging: imx-drm: Add RGB666 support for parallel display.
2013-11-13 18:43 ` Troy Kisky
@ 2013-11-13 19:12 ` Russell King - ARM Linux
2013-11-13 19:48 ` Laurent Pinchart
0 siblings, 1 reply; 5+ messages in thread
From: Russell King - ARM Linux @ 2013-11-13 19:12 UTC (permalink / raw)
To: Troy Kisky
Cc: Denis Carikli, Shawn Guo, Mark Rutland, devicetree,
driverdev-devel, Sascha Hauer, Pawel Moll, Stephen Warren,
David Airlie, Greg Kroah-Hartman, Ian Campbell, dri-devel,
Laurent Pinchart, Philipp Zabel, Eric Bénard, linux-media,
Rob Herring, linux-arm-kernel, Mauro Carvalho Chehab
On Wed, Nov 13, 2013 at 11:43:44AM -0700, Troy Kisky wrote:
> On 11/13/2013 2:23 AM, Denis Carikli wrote:
>> + /* rgb666 */
>> + ipu_dc_map_clear(priv, IPU_DC_MAP_RGB666);
>> + ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 2, 17, 0xfc); /* red */
>> + ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 1, 11, 0xfc); /* green */
>> + ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 0, 5, 0xfc); /* blue */
>> +
>> return 0;
>> }
>>
>>
>
> Since, rgb24 and bgr24 reverse the byte numbers
> /* rgb24 */
> ipu_dc_map_clear(priv, IPU_DC_MAP_RGB24);
> ipu_dc_map_config(priv, IPU_DC_MAP_RGB24, 0, 7, 0xff); /* blue */
> ipu_dc_map_config(priv, IPU_DC_MAP_RGB24, 1, 15, 0xff); /* green */
> ipu_dc_map_config(priv, IPU_DC_MAP_RGB24, 2, 23, 0xff); /* red */
>
> /* bgr24 */
> ipu_dc_map_clear(priv, IPU_DC_MAP_BGR24);
> ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 2, 7, 0xff); /* red */
> ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 1, 15, 0xff); /* green */
> ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 0, 23, 0xff); /* blue */
>
>
> Shouldn't rgb666 and bgr666 do the same?
> Currently we have,
>
> /* bgr666 */
> ipu_dc_map_clear(priv, IPU_DC_MAP_BGR666);
> ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 0, 5, 0xfc); /* blue */
> ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 1, 11, 0xfc); /*
> green */
> ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 2, 17, 0xfc); /* red */
Yes, I concur - this doesn't make sense to me. BGR666 would mean in
memory:
1 11
7 21 65 0
BBBBBBGGGGGGRRRRRR
which reflects the same order for "RGB24" above.
> Where I'd expect to see
> /* bgr666 */
> ipu_dc_map_clear(priv, IPU_DC_MAP_BGR666);
> ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 0, 17, 0xfc); /* blue */
> ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 1, 11, 0xfc); /*
> green */
> ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 2, 5, 0xfc); /* red */
So this makes sense to me.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCHv4][ 3/7] staging: imx-drm: Add RGB666 support for parallel display.
2013-11-13 19:12 ` Russell King - ARM Linux
@ 2013-11-13 19:48 ` Laurent Pinchart
0 siblings, 0 replies; 5+ messages in thread
From: Laurent Pinchart @ 2013-11-13 19:48 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Troy Kisky, Denis Carikli, Shawn Guo, Mark Rutland, devicetree,
driverdev-devel, Sascha Hauer, Pawel Moll, Stephen Warren,
David Airlie, Greg Kroah-Hartman, Ian Campbell, dri-devel,
Philipp Zabel, Eric Bénard, linux-media, Rob Herring,
linux-arm-kernel, Mauro Carvalho Chehab
Hi Russell,
On Wednesday 13 November 2013 19:12:30 Russell King - ARM Linux wrote:
> On Wed, Nov 13, 2013 at 11:43:44AM -0700, Troy Kisky wrote:
> > On 11/13/2013 2:23 AM, Denis Carikli wrote:
> >> + /* rgb666 */
> >>
> >> + ipu_dc_map_clear(priv, IPU_DC_MAP_RGB666);
> >> + ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 2, 17, 0xfc); /* red */
> >> + ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 1, 11, 0xfc); /* green */
> >> + ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 0, 5, 0xfc); /* blue */
> >> +
> >> return 0;
> >> }
> >
> > Since, rgb24 and bgr24 reverse the byte numbers
> > /* rgb24 */
> > ipu_dc_map_clear(priv, IPU_DC_MAP_RGB24);
> > ipu_dc_map_config(priv, IPU_DC_MAP_RGB24, 0, 7, 0xff); /* blue */
> > ipu_dc_map_config(priv, IPU_DC_MAP_RGB24, 1, 15, 0xff); /* green
> > */
> > ipu_dc_map_config(priv, IPU_DC_MAP_RGB24, 2, 23, 0xff); /* red */
> >
> > /* bgr24 */
> > ipu_dc_map_clear(priv, IPU_DC_MAP_BGR24);
> > ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 2, 7, 0xff); /* red */
> > ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 1, 15, 0xff); /* green
> > */
> > ipu_dc_map_config(priv, IPU_DC_MAP_BGR24, 0, 23, 0xff); /* blue */
> >
> > Shouldn't rgb666 and bgr666 do the same?
> > Currently we have,
> >
> > /* bgr666 */
> > ipu_dc_map_clear(priv, IPU_DC_MAP_BGR666);
> > ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 0, 5, 0xfc); /* blue */
> > ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 1, 11, 0xfc); /*
> > green */
> > ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 2, 17, 0xfc); /* red */
>
> Yes, I concur - this doesn't make sense to me. BGR666 would mean in
> memory:
>
> 1 11
> 7 21 65 0
> BBBBBBGGGGGGRRRRRR
>
> which reflects the same order for "RGB24" above.
Beside component order and number of bits per component, an in-memory RGB
format also defines the memory endianness and, for formats that don't span an
interger number of bytes, the left or right alignment.
BGR666 is currently defined in V4L2 as
Byte 0 1 2
Bit 7 6 5 4 3 2 1 0 7 6 5 4 3 2 1 0 7 6 5 4 3 2 1 0
b5 b4 b3 b2 b1 b0 g5 g4 g3 g2 g1 g0 r5 r4 r3 r2 r1 r0 - - - - - -
(see the *second* table in http://linuxtv.org/downloads/v4l-dvb-apis/packed-rgb.html)
I would thus expect RGB666 to be
Byte 0 1 2
Bit 7 6 5 4 3 2 1 0 7 6 5 4 3 2 1 0 7 6 5 4 3 2 1 0
r5 r4 r3 r2 r1 r0 g5 g4 g3 g2 g1 g0 b5 b4 b3 b2 b1 b0 - - - - - -
We can also define right-aligned formats if needed.
> > Where I'd expect to see
> > /* bgr666 */
> >
> > ipu_dc_map_clear(priv, IPU_DC_MAP_BGR666);
> > ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 0, 17, 0xfc); /* blue
> > */
> > ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 1, 11, 0xfc); /*
> > green */
> > ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 2, 5, 0xfc); /* red */
>
> So this makes sense to me.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-11-13 19:47 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-13 9:23 [PATCHv4][ 1/7] [media] v4l2: add new V4L2_PIX_FMT_RGB666 pixel format Denis Carikli
2013-11-13 9:23 ` [PATCHv4][ 3/7] staging: imx-drm: Add RGB666 support for parallel display Denis Carikli
2013-11-13 18:43 ` Troy Kisky
2013-11-13 19:12 ` Russell King - ARM Linux
2013-11-13 19:48 ` Laurent Pinchart
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox