* [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
[not found] ` <1384334603-14208-1-git-send-email-denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
0 siblings, 2 replies; 7+ messages in thread
From: Denis Carikli @ 2013-11-13 9:23 UTC (permalink / raw)
To: Shawn Guo
Cc: Mark Rutland, devicetree, driverdev-devel, Sascha Hauer,
Pawel Moll, Stephen Warren, David Airlie, Greg Kroah-Hartman,
Ian Campbell, dri-devel, Denis Carikli, Laurent Pinchart,
Philipp Zabel, Eric Bénard, linux-media, Rob Herring,
linux-arm-kernel, Mauro Carvalho Chehab
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
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 7+ 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 [not found] ` <1384334603-14208-1-git-send-email-denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org> 1 sibling, 1 reply; 7+ messages in thread From: Denis Carikli @ 2013-11-13 9:23 UTC (permalink / raw) To: Shawn Guo Cc: Mark Rutland, devicetree, driverdev-devel, Sascha Hauer, Pawel Moll, Stephen Warren, David Airlie, Greg Kroah-Hartman, Ian Campbell, dri-devel, Denis Carikli, Laurent Pinchart, Philipp Zabel, Eric Bénard, linux-media, Rob Herring, linux-arm-kernel, Mauro Carvalho Chehab 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 _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply related [flat|nested] 7+ 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; 7+ messages in thread From: Troy Kisky @ 2013-11-13 18:43 UTC (permalink / raw) To: Denis Carikli, Shawn Guo Cc: 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 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ messages in thread
[parent not found: <1384334603-14208-1-git-send-email-denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>]
* [PATCHv4][ 4/7] staging: imx-drm: parallel display: add regulator support. [not found] ` <1384334603-14208-1-git-send-email-denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org> @ 2013-11-13 9:23 ` Denis Carikli 2013-11-13 9:32 ` Alexander Shiyan 0 siblings, 1 reply; 7+ messages in thread From: Denis Carikli @ 2013-11-13 9:23 UTC (permalink / raw) To: Shawn Guo Cc: Sascha Hauer, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Philipp Zabel, Mauro Carvalho Chehab, Laurent Pinchart, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Denis Carikli, Dan Carpenter, Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell, devicetree-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman, driverdev-devel-tBiZLqfeLfOHmIFyCCdPziST3g8Odh+X, David Airlie, Eric Bénard Cc: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org> Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> Cc: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> Cc: driverdev-devel-tBiZLqfeLfOHmIFyCCdPziST3g8Odh+X@public.gmane.org Cc: David Airlie <airlied-cv59FeDIM0c@public.gmane.org> Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org Cc: Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> Cc: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org Cc: Eric Bénard <eric-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org> Signed-off-by: Denis Carikli <denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org> --- ChangeLog v2->v3: - Added some interested people in the Cc list. - the lcd-supply is now called display-supply (not all display are LCD). - The code and documentation was updated accordingly. - regulator_is_enabled now guard the regulator enables/disables because regulator_disable does not check the regulator state. --- .../bindings/staging/imx-drm/fsl-imx-drm.txt | 1 + drivers/staging/imx-drm/parallel-display.c | 22 ++++++++++++++++++++ 2 files changed, 23 insertions(+) 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 2d24425..4dd7ce5 100644 --- a/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt +++ b/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt @@ -28,6 +28,7 @@ Required properties: - compatible: Should be "fsl,imx-parallel-display" - crtc: the crtc this display is connected to, see below Optional properties: +- display-supply : phandle to the regulator device tree node if needed. - interface_pix_fmt: How this display is connected to the crtc. Currently supported types: "rgb24", "rgb565", "bgr666", "rgb666" - edid: verbatim EDID data block describing attached display. diff --git a/drivers/staging/imx-drm/parallel-display.c b/drivers/staging/imx-drm/parallel-display.c index 46b6fce..195ec60 100644 --- a/drivers/staging/imx-drm/parallel-display.c +++ b/drivers/staging/imx-drm/parallel-display.c @@ -22,6 +22,7 @@ #include <drm/drmP.h> #include <drm/drm_fb_helper.h> #include <drm/drm_crtc_helper.h> +#include <linux/regulator/consumer.h> #include <linux/videodev2.h> #include "imx-drm.h" @@ -35,6 +36,7 @@ struct imx_parallel_display { struct drm_encoder encoder; struct imx_drm_encoder *imx_drm_encoder; struct device *dev; + struct regulator *disp_reg; void *edid; int edid_len; u32 interface_pix_fmt; @@ -139,6 +141,12 @@ static void imx_pd_encoder_prepare(struct drm_encoder *encoder) { struct imx_parallel_display *imxpd = enc_to_imxpd(encoder); + if (imxpd->disp_reg && !regulator_is_enabled(imxpd->disp_reg)) { + if (regulator_enable(imxpd->disp_reg)) + dev_err(imxpd->dev, + "Failed to enable regulator.\n"); + } + imx_drm_crtc_panel_format(encoder->crtc, DRM_MODE_ENCODER_NONE, imxpd->interface_pix_fmt); } @@ -155,6 +163,12 @@ static void imx_pd_encoder_mode_set(struct drm_encoder *encoder, static void imx_pd_encoder_disable(struct drm_encoder *encoder) { + struct imx_parallel_display *imxpd = enc_to_imxpd(encoder); + + if (imxpd->disp_reg && regulator_is_enabled(imxpd->disp_reg)) { + if (regulator_disable(imxpd->disp_reg)) + dev_err(imxpd->dev, "Failed to disable regulator.\n"); + } } static void imx_pd_encoder_destroy(struct drm_encoder *encoder) @@ -258,6 +272,14 @@ static int imx_pd_probe(struct platform_device *pdev) if (ret) return ret; + imxpd->disp_reg = devm_regulator_get(&pdev->dev, "display"); + if (IS_ERR(imxpd->disp_reg)) { + dev_warn(&pdev->dev, "Operating without display regulator.\n"); + imxpd->disp_reg = NULL; + } else { + dev_info(&pdev->dev, "Using display regulator.\n"); + } + ret = imx_drm_encoder_add_possible_crtcs(imxpd->imx_drm_encoder, np); platform_set_drvdata(pdev, imxpd); -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCHv4][ 4/7] staging: imx-drm: parallel display: add regulator support. 2013-11-13 9:23 ` [PATCHv4][ 4/7] staging: imx-drm: parallel display: add regulator support Denis Carikli @ 2013-11-13 9:32 ` Alexander Shiyan 0 siblings, 0 replies; 7+ messages in thread From: Alexander Shiyan @ 2013-11-13 9:32 UTC (permalink / raw) To: Denis Carikli Cc: Mark Rutland, devicetree, Ian Campbell, Philipp Zabel, Pawel Moll, Stephen Warren, David Airlie, Greg Kroah-Hartman, driverdev-devel, dri-devel, Mauro Carvalho Chehab, Laurent Pinchart, Sascha Hauer, Eric Bénard, Shawn Guo, Rob Herring Hello. Среда, 13 ноября 2013, 10:23 +01:00 от Denis Carikli <denis@eukrea.com>: ... > --- a/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt ... > @@ -139,6 +141,12 @@ static void imx_pd_encoder_prepare(struct drm_encoder *encoder) > { > struct imx_parallel_display *imxpd = enc_to_imxpd(encoder); > > + if (imxpd->disp_reg && !regulator_is_enabled(imxpd->disp_reg)) { if (!IS_ERR(imxpd->disp_reg) && ... > + if (regulator_enable(imxpd->disp_reg)) > + dev_err(imxpd->dev, > + "Failed to enable regulator.\n"); > + } > + > imx_drm_crtc_panel_format(encoder->crtc, DRM_MODE_ENCODER_NONE, > imxpd->interface_pix_fmt); > } > @@ -155,6 +163,12 @@ static void imx_pd_encoder_mode_set(struct drm_encoder *encoder, > > static void imx_pd_encoder_disable(struct drm_encoder *encoder) > { > + struct imx_parallel_display *imxpd = enc_to_imxpd(encoder); > + > + if (imxpd->disp_reg && regulator_is_enabled(imxpd->disp_reg)) { if (!IS_ERR(imxpd->disp_reg) && ... > + if (regulator_disable(imxpd->disp_reg)) > + dev_err(imxpd->dev, "Failed to disable regulator.\n"); > + } > } > > static void imx_pd_encoder_destroy(struct drm_encoder *encoder) > @@ -258,6 +272,14 @@ static int imx_pd_probe(struct platform_device *pdev) > if (ret) > return ret; > > + imxpd->disp_reg = devm_regulator_get(&pdev->dev, "display"); > + if (IS_ERR(imxpd->disp_reg)) { if (PTR_ERR(imxpd->disp_reg) == -EPROBE_DEFER) return -EPROBE_DEFER; > + dev_warn(&pdev->dev, "Operating without display regulator.\n"); dev_dbg > + imxpd->disp_reg = NULL; No need set this to NULL; > + } else { > + dev_info(&pdev->dev, "Using display regulator.\n"); Useless. ... --- _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-11-13 19:48 UTC | newest]
Thread overview: 7+ 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
[not found] ` <1384334603-14208-1-git-send-email-denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
2013-11-13 9:23 ` [PATCHv4][ 4/7] staging: imx-drm: parallel display: add regulator support Denis Carikli
2013-11-13 9:32 ` Alexander Shiyan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).