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 */
If de-active and/or pixelclk-active properties were set in the display-timings DT node, they were not used.
Instead the data-enable and the pixel data clock polarity were hardcoded.
This change is needed for making the eukrea-cpuimx51 QVGA display work.
Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: driverdev-devel@linuxdriverproject.org Cc: Philipp Zabel p.zabel@pengutronix.de Cc: Sascha Hauer kernel@pengutronix.de Cc: Shawn Guo shawn.guo@linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: David Airlie airlied@linux.ie Cc: dri-devel@lists.freedesktop.org Cc: Eric Bénard eric@eukrea.com Signed-off-by: Denis Carikli denis@eukrea.com --- ChangeLog v3->v4: - The old patch was named "staging: imx-drm: ipuv3-crtc: don't harcode some mode". - Reworked the patch entierly: we now takes the mode flags from the device tree.
ChangeLog v2->v3: - Added some interested people in the Cc list. - Ajusted the flags to match the changes in "drm: Add the lacking DRM_MODE_FLAG_* for matching the DISPLAY_FLAGS_*" --- drivers/staging/imx-drm/imx-drm.h | 3 +++ drivers/staging/imx-drm/ipuv3-crtc.c | 11 +++++++++-- drivers/staging/imx-drm/parallel-display.c | 28 ++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/imx-drm/imx-drm.h b/drivers/staging/imx-drm/imx-drm.h index ae90c9c..dfdc180 100644 --- a/drivers/staging/imx-drm/imx-drm.h +++ b/drivers/staging/imx-drm/imx-drm.h @@ -5,6 +5,9 @@
#define IPU_PIX_FMT_GBR24 v4l2_fourcc('G', 'B', 'R', '3')
+#define IMXDRM_MODE_FLAG_DE_HIGH (1<<0) +#define IMXDRM_MODE_FLAG_PIXDATA_POSEDGE (1<<1) + struct drm_crtc; struct drm_connector; struct drm_device; diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c b/drivers/staging/imx-drm/ipuv3-crtc.c index 670a56a..82dce06 100644 --- a/drivers/staging/imx-drm/ipuv3-crtc.c +++ b/drivers/staging/imx-drm/ipuv3-crtc.c @@ -156,8 +156,15 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc, if (mode->flags & DRM_MODE_FLAG_PVSYNC) sig_cfg.Vsync_pol = 1;
- sig_cfg.enable_pol = 1; - sig_cfg.clk_pol = 1; + /* Such flags are not availables in the DRM modes header, + * and we don't want to export them to userspace. + */ + if (mode->private_flags & IMXDRM_MODE_FLAG_DE_HIGH) + sig_cfg.enable_pol = 1; + + if (mode->private_flags & IMXDRM_MODE_FLAG_PIXDATA_POSEDGE) + sig_cfg.clk_pol = 1; + sig_cfg.width = mode->hdisplay; sig_cfg.height = mode->vdisplay; sig_cfg.pixel_fmt = out_pixel_fmt; diff --git a/drivers/staging/imx-drm/parallel-display.c b/drivers/staging/imx-drm/parallel-display.c index 24aa9be..09658ac 100644 --- a/drivers/staging/imx-drm/parallel-display.c +++ b/drivers/staging/imx-drm/parallel-display.c @@ -74,7 +74,35 @@ static int imx_pd_connector_get_modes(struct drm_connector *connector)
if (np) { struct drm_display_mode *mode = drm_mode_create(connector->dev); + struct device_node *timings_np; + struct device_node *mode_np; + u32 val; + of_get_drm_display_mode(np, &imxpd->mode, 0); + + /* Such flags are not availables in the DRM modes header, + * and we don't want to export them to userspace. + */ + timings_np = of_get_child_by_name(np, "display-timings"); + if (timings_np) { + /* get the display mode node */ + mode_np = of_parse_phandle(timings_np, + "native-mode", 0); + if (!mode_np) + mode_np = of_get_next_child(timings_np, NULL); + + /* set de-active to 1 if not set */ + of_property_read_u32(mode_np, "de-active", &val); + if (!!val) + imxpd->mode.private_flags |= + IMXDRM_MODE_FLAG_DE_HIGH; + + /* set pixelclk-active to 1 if not set */ + of_property_read_u32(mode_np, "pixelclk-active", &val); + if (!!val) + imxpd->mode.private_flags |= + IMXDRM_MODE_FLAG_PIXDATA_POSEDGE; + } drm_mode_copy(mode, &imxpd->mode); mode->type |= DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED, drm_mode_probed_add(connector, mode);
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;
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
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.
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.
Cc: Dan Carpenter dan.carpenter@oracle.com 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: 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. - 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);
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.
...
---
The CMO-QVGA, DVI-SVGA and DVI-VGA are added.
Cc: Shawn Guo shawn.guo@linaro.org Cc: Sascha Hauer kernel@pengutronix.de 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: - Splitted out from the patch that added support for the cpuimx51/mbimxsd51 boards. - This patch now only adds display support. - Added some interested people in the Cc list, and removed some people that might be annoyed by the receiving of that patch which is unrelated to their subsystem. - rebased and reworked the dts displays addition. - Also rebased and reworked the fsl,pins usage. --- .../imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts | 61 ++++++++++++++++++++ .../imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dts | 47 +++++++++++++++ .../imx51-eukrea-mbimxsd51-baseboard-dvi-vga.dts | 47 +++++++++++++++ .../boot/dts/imx51-eukrea-mbimxsd51-baseboard.dts | 13 +++++ 4 files changed, 168 insertions(+) create mode 100644 arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts create mode 100644 arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dts create mode 100644 arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-vga.dts
diff --git a/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts new file mode 100644 index 0000000..4c781e7 --- /dev/null +++ b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts @@ -0,0 +1,61 @@ +/* + * Copyright 2013 Eukréa Electromatique denis@eukrea.com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, + * MA 02110-1301, USA. + */ + +#include "imx51-eukrea-mbimxsd51-baseboard.dts" + +/ { + model = "Eukrea MBIMXSD51 with the CMO-QVGA Display"; + compatible = "eukrea,mbimxsd51-baseboard-cmo-qvga", "eukrea,mbimxsd51-baseboard", "eukrea,cpuimx51", "fsl,imx51"; + + reg_lcd_3v3: lcd-en { + compatible = "regulator-fixed"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_reg_lcd_3v3>; + regulator-name = "lcd-3v3"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + gpio = <&gpio3 13 0>; + enable-active-high; + }; + +}; + +&display { + display-supply = <®_lcd_3v3>; + status = "okay"; + display-timings { + model = "CMO-QVGA"; + bits-per-pixel = <16>; + cmoqvga { + native-mode; + clock-frequency = <6500000>; + hactive = <320>; + vactive = <240>; + hfront-porch = <20>; + hback-porch = <38>; + vfront-porch = <4>; + vback-porch = <15>; + hsync-len = <30>; + vsync-len = <3>; + hsync-active = <0>; + vsync-active = <0>; + de-active = <0>; + pixelclk-active = <1>; + }; + }; +}; diff --git a/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dts b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dts new file mode 100644 index 0000000..e9be19c --- /dev/null +++ b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dts @@ -0,0 +1,47 @@ +/* + * Copyright 2013 Eukréa Electromatique denis@eukrea.com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, + * MA 02110-1301, USA. + */ + +#include "imx51-eukrea-mbimxsd51-baseboard.dts" + +/ { + model = "Eukrea MBIMXSD51 with the DVI-SVGA Display"; + compatible = "eukrea,mbimxsd51-baseboard-dvi-svga", "eukrea,mbimxsd51-baseboard", "eukrea,cpuimx51", "fsl,imx51"; +}; + +&display { + status = "okay"; + display-timings { + model = "DVI-SVGA"; + bits-per-pixel = <16>; + svga { + clock-frequency = <38251000>; + hactive = <800>; + vactive = <600>; + hback-porch = <112>; + hfront-porch = <32>; + vback-porch = <3>; + vfront-porch = <17>; + hsync-len = <80>; + vsync-len = <4>; + hsync-active = <1>; + vsync-active = <1>; + de-active = <1>; + pixelclk-active = <0>; + }; + }; +}; diff --git a/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-vga.dts b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-vga.dts new file mode 100644 index 0000000..aa99551 --- /dev/null +++ b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-vga.dts @@ -0,0 +1,47 @@ +/* + * Copyright 2013 Eukréa Electromatique denis@eukrea.com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, + * MA 02110-1301, USA. + */ + +#include "imx51-eukrea-mbimxsd51-baseboard.dts" + +/ { + model = "Eukrea MBIMXSD51 with the DVI-VGA Display"; + compatible = "eukrea,mbimxsd51-baseboard-dvi-vga", "eukrea,mbimxsd51-baseboard", "eukrea,cpuimx51", "fsl,imx51"; +}; + +&display { + status = "okay"; + display-timings { + model = "DVI-VGA"; + bits-per-pixel = <16>; + vga { + clock-frequency = <23750000>; + hactive = <640>; + vactive = <480>; + hback-porch = <80>; + hfront-porch = <16>; + vback-porch = <3>; + vfront-porch = <13>; + hsync-len = <64>; + vsync-len = <4>; + hsync-active = <1>; + vsync-active = <1>; + de-active = <1>; + pixelclk-active = <0>; + }; + }; +}; diff --git a/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard.dts b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard.dts index d44a85d..71c2829 100644 --- a/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard.dts +++ b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard.dts @@ -23,6 +23,15 @@ model = "Eukrea CPUIMX51"; compatible = "eukrea,mbimxsd51","eukrea,cpuimx51", "fsl,imx51";
+ display: display@di0 { + compatible = "fsl,imx-parallel-display"; + crtcs = <&ipu 0>; + interface-pix-fmt = "rgb666"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_ipu_disp1>; + status = "disabled"; + }; + gpio_keys { compatible = "gpio-keys"; pinctrl-names = "default"; @@ -116,6 +125,10 @@ >; };
+ pinctrl_ipu_disp1: ipudisp1grp { + fsl,pins = <MX51_IPU_DISP1_PINGRP1>; + }; + pinctrl_reg_lcd_3v3: reg_lcd_3v3 { fsl,pins = < MX51_PAD_CSI1_D9__GPIO3_13 0x1f5
Cc: Shawn Guo shawn.guo@linaro.org Cc: Sascha Hauer kernel@pengutronix.de 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: - Splitted out from the patch that added support for the cpuimx51/mbimxsd51 boards. - This patch now only adds backlight support. - Added some interested people in the Cc list, and removed some people that might be annoyed by the receiving of that patch which is unrelated to their subsystem. --- .../imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts index 4c781e7..73b00b7 100644 --- a/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts +++ b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts @@ -22,6 +22,14 @@ model = "Eukrea MBIMXSD51 with the CMO-QVGA Display"; compatible = "eukrea,mbimxsd51-baseboard-cmo-qvga", "eukrea,mbimxsd51-baseboard", "eukrea,cpuimx51", "fsl,imx51";
+ backlight { + compatible = "gpio-backlight"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_backlight_1>; + gpios = <&gpio3 4 0>; + default-brightness-level = <1>; + }; + reg_lcd_3v3: lcd-en { compatible = "regulator-fixed"; pinctrl-names = "default";
The eukrea mbimxsd51 has a gpio backlight for its LCD display, so we turn that driver on.
Cc: Sascha Hauer kernel@pengutronix.de Cc: linux-arm-kernel@lists.infradead.org Cc: Fabio Estevam fabio.estevam@freescale.com Cc: Shawn Guo shawn.guo@linaro.org Cc: Eric Bénard eric@eukrea.com Signed-off-by: Denis Carikli denis@eukrea.com --- arch/arm/configs/imx_v6_v7_defconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig index c814e0e..910122c 100644 --- a/arch/arm/configs/imx_v6_v7_defconfig +++ b/arch/arm/configs/imx_v6_v7_defconfig @@ -178,6 +178,7 @@ CONFIG_LCD_L4F00242T03=y CONFIG_LCD_PLATFORM=y CONFIG_BACKLIGHT_CLASS_DEVICE=y CONFIG_BACKLIGHT_PWM=y +CONFIG_BACKLIGHT_GPIO=y CONFIG_FRAMEBUFFER_CONSOLE=y CONFIG_LOGO=y CONFIG_SOUND=y
dri-devel@lists.freedesktop.org