[PATCHv3 1/8] drm: Add the lacking DRM_MODE_FLAG_* for matching the DISPLAY_FLAGS_*

[PATCHv3 1/8] drm: Add the lacking DRM_MODE_FLAG_* for matching the DISPLAY_FLAGS_*

[Denis Carikli]

Without that fix, drivers using the drm_display_mode_from_videomode
  function will not be able to get certain information because
  some DISPLAY_FLAGS_* have no corresponding DRM_MODE_FLAG_*.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: driverdev-devel at linuxdriverproject.org
Cc: Matt Sealey <neko@bakuhatsu.net>
Cc: David Airlie <airlied@linux.ie>
Cc: Ville Syrjala <syrjala@sci.fi>
Cc: dri-devel at lists.freedesktop.org
Cc: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: linux-arm-kernel at 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 a duplicated Cc.
- Changed the flags names to match the one in DISPLAY_FLAGS.
- Moved the flags out of the userspace headers.
- Updated the rest of the code accordingly.
---
 drivers/gpu/drm/drm_modes.c |    9 +++++++++
 include/drm/drm_mode.h      |   34 ++++++++++++++++++++++++++++++++++
 include/uapi/drm/drm_mode.h |    4 ++--
 3 files changed, 45 insertions(+), 2 deletions(-)
 create mode 100644 include/drm/drm_mode.h

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index fc2adb6..586c12f 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -537,6 +537,15 @@ int drm_display_mode_from_videomode(const struct videomode *vm,
 		dmode->flags |= DRM_MODE_FLAG_DBLSCAN;
 	if (vm->flags & DISPLAY_FLAGS_DOUBLECLK)
 		dmode->flags |= DRM_MODE_FLAG_DBLCLK;
+	if (vm->flags & DISPLAY_FLAGS_DE_LOW)
+		dmode->flags |= DRM_MODE_FLAG_DE_LOW;
+	if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
+		dmode->flags |= DRM_MODE_FLAG_DE_HIGH;
+	if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
+		dmode->flags |= DRM_MODE_FLAG_PIXDATA_POSEDGE;
+	if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
+		dmode->flags |= DRM_MODE_FLAG_PIXDATA_NEGEDGE;
+
 	drm_mode_set_name(dmode);
 
 	return 0;
diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
new file mode 100644
index 0000000..07e5259
--- /dev/null
+++ b/include/drm/drm_mode.h
@@ -0,0 +1,34 @@
+/*
+ * Copyright 2013 Eukr?a Electromatique <denis@eukrea.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#ifndef _DRM_MODE_H
+#define _DRM_MODE_H
+
+#include <uapi/drm/drm_mode.h>
+
+/* DISPLAY_FLAGS DRM counterpart */
+#define DRM_MODE_FLAG_DE_HIGH			(1<<22)
+#define DRM_MODE_FLAG_DE_LOW			(1<<23)
+#define DRM_MODE_FLAG_PIXDATA_POSEDGE		(1<<24)
+#define DRM_MODE_FLAG_PIXDATA_NEGEDGE		(1<<25)
+
+#endif
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 28acbaf..5681897 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -24,8 +24,8 @@
  * IN THE SOFTWARE.
  */
 
-#ifndef _DRM_MODE_H
-#define _DRM_MODE_H
+#ifndef _UAPI_DRM_MODE_H
+#define _UAPI_DRM_MODE_H
 
 #include <linux/types.h>
 
-- 
1.7.9.5

[PATCHv3 2/8] [media] v4l2: add new V4L2_PIX_FMT_RGB666 pixel format.

[Denis Carikli]

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 at vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: driverdev-devel at linuxdriverproject.org
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel at lists.freedesktop.org
Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: linux-media at vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: linux-arm-kernel at 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>
---
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

[PATCHv3 3/8] staging: imx-drm: ipuv3-crtc: don't harcode some mode flags.

[Denis Carikli]

This change is needed for making the eukrea-cpuimx51
  QVGA display work.

Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: driverdev-devel at 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 at lists.infradead.org
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel at lists.freedesktop.org
Cc: Eric B?nard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
 drivers/staging/imx-drm/ipuv3-crtc.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
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_*"
---
diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c b/drivers/staging/imx-drm/ipuv3-crtc.c
index 670a56a..917818c 100644
--- a/drivers/staging/imx-drm/ipuv3-crtc.c
+++ b/drivers/staging/imx-drm/ipuv3-crtc.c
@@ -155,9 +155,11 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc,
 		sig_cfg.Hsync_pol = 1;
 	if (mode->flags & DRM_MODE_FLAG_PVSYNC)
 		sig_cfg.Vsync_pol = 1;
+	if (mode->flags & DRM_MODE_FLAG_DE_HIGH)
+		sig_cfg.enable_pol = 1;
+	if (mode->flags & DRM_MODE_FLAG_PIXDATA_POSEDGE)
+		sig_cfg.clk_pol = 1;
 
-	sig_cfg.enable_pol = 1;
-	sig_cfg.clk_pol = 1;
 	sig_cfg.width = mode->hdisplay;
 	sig_cfg.height = mode->vdisplay;
 	sig_cfg.pixel_fmt = out_pixel_fmt;
-- 
1.7.9.5

[PATCHv3 4/8] staging: imx-drm: Add RGB666 support for parallel display.

[Denis Carikli]

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 at vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: driverdev-devel at linuxdriverproject.org
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel at lists.freedesktop.org
Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: linux-media at vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: linux-arm-kernel at 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 24aa9be..bb71d6d 100644
--- a/drivers/staging/imx-drm/parallel-display.c
+++ b/drivers/staging/imx-drm/parallel-display.c
@@ -222,6 +222,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

[PATCHv3 1/8] drm: Add the lacking DRM_MODE_FLAG_* for matching the DISPLAY_FLAGS_*

[Russell King - ARM Linux]

On Tue, Nov 12, 2013 at 05:49:18PM +0100, Denis Carikli wrote:
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index fc2adb6..586c12f 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -537,6 +537,15 @@ int drm_display_mode_from_videomode(const struct videomode *vm,
>  		dmode->flags |= DRM_MODE_FLAG_DBLSCAN;
>  	if (vm->flags & DISPLAY_FLAGS_DOUBLECLK)
>  		dmode->flags |= DRM_MODE_FLAG_DBLCLK;
> +	if (vm->flags & DISPLAY_FLAGS_DE_LOW)
> +		dmode->flags |= DRM_MODE_FLAG_DE_LOW;
> +	if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
> +		dmode->flags |= DRM_MODE_FLAG_DE_HIGH;
> +	if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> +		dmode->flags |= DRM_MODE_FLAG_PIXDATA_POSEDGE;
> +	if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> +		dmode->flags |= DRM_MODE_FLAG_PIXDATA_NEGEDGE;
> +

I'm still not convinced that these should be exposed in *any* way to
userspace - I'm with Ville Syrj?l? on this point.

Yes, you've moved their definition out of a uapi header file, but
they're still leaking out of kernel space via calls (and with Xorg,
they'll leak into the DisplayMode structures within the X server.)

Now, here's the thing... The polarity of the display enable signal.
That's a property of the connected device right?  It doesn't change
with respect to the displayed mode unlike the hsync/vsync signals.
If that's true, it should not be here.

Same goes for the pixel clock edge.  If it's a property of the
connected device and doesn't have a dependence on the displayed
mode, then it should not be in the DRM mode structure.

[PATCHv3 2/8] [media] v4l2: add new V4L2_PIX_FMT_RGB666 pixel format.

[Laurent Pinchart]

Hi Denis,

(Dropping the DT reviewers from the CC list to avoid spamming them)

Thank you for the patch.

On Tuesday 12 November 2013 17:49:19 Denis Carikli wrote:
> 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 at vger.kernel.org
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: driverdev-devel at linuxdriverproject.org
> Cc: David Airlie <airlied@linux.ie>
> Cc: dri-devel at lists.freedesktop.org
> Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: linux-media at vger.kernel.org
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: linux-arm-kernel at 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 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   */
-- 
Regards,

Laurent Pinchart

[PATCHv3 3/8] staging: imx-drm: ipuv3-crtc: don't harcode some mode flags.

[Fabio Estevam]

Hi Denis,

On Tue, Nov 12, 2013 at 2:49 PM, Denis Carikli <denis@eukrea.com> wrote:
> This change is needed for making the eukrea-cpuimx51
>   QVGA display work.
>
> Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: driverdev-devel at 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 at lists.infradead.org
> Cc: David Airlie <airlied@linux.ie>
> Cc: dri-devel at lists.freedesktop.org
> Cc: Eric B?nard <eric@eukrea.com>
> Signed-off-by: Denis Carikli <denis@eukrea.com>
> ---
>  drivers/staging/imx-drm/ipuv3-crtc.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 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_*"
> ---
> diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c b/drivers/staging/imx-drm/ipuv3-crtc.c
> index 670a56a..917818c 100644
> --- a/drivers/staging/imx-drm/ipuv3-crtc.c
> +++ b/drivers/staging/imx-drm/ipuv3-crtc.c
> @@ -155,9 +155,11 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc,
>                 sig_cfg.Hsync_pol = 1;
>         if (mode->flags & DRM_MODE_FLAG_PVSYNC)
>                 sig_cfg.Vsync_pol = 1;
> +       if (mode->flags & DRM_MODE_FLAG_DE_HIGH)
> +               sig_cfg.enable_pol = 1;
> +       if (mode->flags & DRM_MODE_FLAG_PIXDATA_POSEDGE)
> +               sig_cfg.clk_pol = 1;
>
> -       sig_cfg.enable_pol = 1;
> -       sig_cfg.clk_pol = 1;


What are the sig_cfg.enable_pol and sig_cfg.clk_pol values you need
for your display to operate correctly?

[PATCHv3 3/8] staging: imx-drm: ipuv3-crtc: don't harcode some mode flags.

[Eric Bénard]

Hi Fabio,

Le Wed, 13 Nov 2013 01:52:25 -0200,
Fabio Estevam <festevam@gmail.com> a ?crit :
> On Tue, Nov 12, 2013 at 2:49 PM, Denis Carikli <denis@eukrea.com> wrote:
> > +       if (mode->flags & DRM_MODE_FLAG_DE_HIGH)
> > +               sig_cfg.enable_pol = 1;
> > +       if (mode->flags & DRM_MODE_FLAG_PIXDATA_POSEDGE)
> > +               sig_cfg.clk_pol = 1;
> >
> > -       sig_cfg.enable_pol = 1;
> > -       sig_cfg.clk_pol = 1;
> 
> 
> What are the sig_cfg.enable_pol and sig_cfg.clk_pol values you need
> for your display to operate correctly?

in ipuv3-crtc, line 159 :
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/imx-drm/ipuv3-crtc.c#n159

polarity of the enable signal and the pixel clock are hard coded to 1.

These settings are then used in ipu-di.c to configurer the IPU display
interface :
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/imx-drm/ipu-v3/ipu-di.c#n631
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/imx-drm/ipu-v3/ipu-di.c#n642

This is a problem as the polarity of these signals can vary from one
TFT panel to an other. In our case, we need and active low enable and a
negative edge clock but currently there is no way to configure these
settings in ipuv3-crtc as they are hardcoded to 1 : that's what Denis
is trying to fix.

Eric

[PATCHv3 1/8] drm: Add the lacking DRM_MODE_FLAG_* for matching the DISPLAY_FLAGS_*

[Eric Bénard]

Hi Russell,

Le Tue, 12 Nov 2013 17:04:55 +0000,
Russell King - ARM Linux <linux@arm.linux.org.uk> a ?crit :
> On Tue, Nov 12, 2013 at 05:49:18PM +0100, Denis Carikli wrote:
> > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> > index fc2adb6..586c12f 100644
> > --- a/drivers/gpu/drm/drm_modes.c
> > +++ b/drivers/gpu/drm/drm_modes.c
> > @@ -537,6 +537,15 @@ int drm_display_mode_from_videomode(const struct videomode *vm,
> >  		dmode->flags |= DRM_MODE_FLAG_DBLSCAN;
> >  	if (vm->flags & DISPLAY_FLAGS_DOUBLECLK)
> >  		dmode->flags |= DRM_MODE_FLAG_DBLCLK;
> > +	if (vm->flags & DISPLAY_FLAGS_DE_LOW)
> > +		dmode->flags |= DRM_MODE_FLAG_DE_LOW;
> > +	if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
> > +		dmode->flags |= DRM_MODE_FLAG_DE_HIGH;
> > +	if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> > +		dmode->flags |= DRM_MODE_FLAG_PIXDATA_POSEDGE;
> > +	if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> > +		dmode->flags |= DRM_MODE_FLAG_PIXDATA_NEGEDGE;
> > +
> 
> I'm still not convinced that these should be exposed in *any* way to
> userspace - I'm with Ville Syrj?l? on this point.
> 
> Yes, you've moved their definition out of a uapi header file, but
> they're still leaking out of kernel space via calls (and with Xorg,
> they'll leak into the DisplayMode structures within the X server.)
> 
> Now, here's the thing... The polarity of the display enable signal.
> That's a property of the connected device right?  It doesn't change
> with respect to the displayed mode unlike the hsync/vsync signals.
> If that's true, it should not be here.
> 
> Same goes for the pixel clock edge.  If it's a property of the
> connected device and doesn't have a dependence on the displayed
> mode, then it should not be in the DRM mode structure.

What would be the right way to configure these settings without
exposing them to userspace ?

As explained in my answer to Fabio, these settings are currently
hardcoded into ipuv3-crtc and we need to configure them to support more
TFT panels using the IPUV3 Parallel Display Interface.

Thanks
Eric

[PATCHv3 1/8] drm: Add the lacking DRM_MODE_FLAG_* for matching the DISPLAY_FLAGS_*

[Philipp Zabel]

Hi Eric,

Am Mittwoch, den 13.11.2013, 08:53 +0100 schrieb Eric B?nard:
> Hi Russell,
> 
> Le Tue, 12 Nov 2013 17:04:55 +0000,
> Russell King - ARM Linux <linux@arm.linux.org.uk> a ?crit :
> > On Tue, Nov 12, 2013 at 05:49:18PM +0100, Denis Carikli wrote:
> > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> > > index fc2adb6..586c12f 100644
> > > --- a/drivers/gpu/drm/drm_modes.c
> > > +++ b/drivers/gpu/drm/drm_modes.c
> > > @@ -537,6 +537,15 @@ int drm_display_mode_from_videomode(const struct videomode *vm,
> > >  		dmode->flags |= DRM_MODE_FLAG_DBLSCAN;
> > >  	if (vm->flags & DISPLAY_FLAGS_DOUBLECLK)
> > >  		dmode->flags |= DRM_MODE_FLAG_DBLCLK;
> > > +	if (vm->flags & DISPLAY_FLAGS_DE_LOW)
> > > +		dmode->flags |= DRM_MODE_FLAG_DE_LOW;
> > > +	if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
> > > +		dmode->flags |= DRM_MODE_FLAG_DE_HIGH;
> > > +	if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> > > +		dmode->flags |= DRM_MODE_FLAG_PIXDATA_POSEDGE;
> > > +	if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> > > +		dmode->flags |= DRM_MODE_FLAG_PIXDATA_NEGEDGE;
> > > +
> > 
> > I'm still not convinced that these should be exposed in *any* way to
> > userspace - I'm with Ville Syrj?l? on this point.
> > 
> > Yes, you've moved their definition out of a uapi header file, but
> > they're still leaking out of kernel space via calls (and with Xorg,
> > they'll leak into the DisplayMode structures within the X server.)
> > 
> > Now, here's the thing... The polarity of the display enable signal.
> > That's a property of the connected device right?  It doesn't change
> > with respect to the displayed mode unlike the hsync/vsync signals.
> > If that's true, it should not be here.
> > 
> > Same goes for the pixel clock edge.  If it's a property of the
> > connected device and doesn't have a dependence on the displayed
> > mode, then it should not be in the DRM mode structure.
> 
> What would be the right way to configure these settings without
> exposing them to userspace ?

I think as a property of the connected device, this should be obtained
from the device tree node of the panel. In the v4l2 style device tree
model this could also be made a property of the link (endpoint).

> As explained in my answer to Fabio, these settings are currently
> hardcoded into ipuv3-crtc and we need to configure them to support more
> TFT panels using the IPUV3 Parallel Display Interface.

regards
Philipp

[PATCHv3 1/8] drm: Add the lacking DRM_MODE_FLAG_* for matching the DISPLAY_FLAGS_*

[Russell King - ARM Linux]

On Wed, Nov 13, 2013 at 08:53:18AM +0100, Eric B?nard wrote:
> Hi Russell,
> 
> Le Tue, 12 Nov 2013 17:04:55 +0000,
> Russell King - ARM Linux <linux@arm.linux.org.uk> a ?crit :
> > On Tue, Nov 12, 2013 at 05:49:18PM +0100, Denis Carikli wrote:
> > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> > > index fc2adb6..586c12f 100644
> > > --- a/drivers/gpu/drm/drm_modes.c
> > > +++ b/drivers/gpu/drm/drm_modes.c
> > > @@ -537,6 +537,15 @@ int drm_display_mode_from_videomode(const struct videomode *vm,
> > >  		dmode->flags |= DRM_MODE_FLAG_DBLSCAN;
> > >  	if (vm->flags & DISPLAY_FLAGS_DOUBLECLK)
> > >  		dmode->flags |= DRM_MODE_FLAG_DBLCLK;
> > > +	if (vm->flags & DISPLAY_FLAGS_DE_LOW)
> > > +		dmode->flags |= DRM_MODE_FLAG_DE_LOW;
> > > +	if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
> > > +		dmode->flags |= DRM_MODE_FLAG_DE_HIGH;
> > > +	if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> > > +		dmode->flags |= DRM_MODE_FLAG_PIXDATA_POSEDGE;
> > > +	if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> > > +		dmode->flags |= DRM_MODE_FLAG_PIXDATA_NEGEDGE;
> > > +
> > 
> > I'm still not convinced that these should be exposed in *any* way to
> > userspace - I'm with Ville Syrj?l? on this point.
> > 
> > Yes, you've moved their definition out of a uapi header file, but
> > they're still leaking out of kernel space via calls (and with Xorg,
> > they'll leak into the DisplayMode structures within the X server.)
> > 
> > Now, here's the thing... The polarity of the display enable signal.
> > That's a property of the connected device right?  It doesn't change
> > with respect to the displayed mode unlike the hsync/vsync signals.
> > If that's true, it should not be here.
> > 
> > Same goes for the pixel clock edge.  If it's a property of the
> > connected device and doesn't have a dependence on the displayed
> > mode, then it should not be in the DRM mode structure.
> 
> What would be the right way to configure these settings without
> exposing them to userspace ?
> 
> As explained in my answer to Fabio, these settings are currently
> hardcoded into ipuv3-crtc and we need to configure them to support more
> TFT panels using the IPUV3 Parallel Display Interface.

First, realise that what you're doing is configuring components within
the IMX driver "suite" so what you need to do is communicate your
requirements _only_ from parallel-display.c to ipuv3-crtc.c.

There's already infrastructure in imx-drm for the display bridges to
communicate various information to the CRTC layer - there's already the
encoder type, and the "pins" for hsync/vsync being communicated via
imx_drm_panel_format*().  This is really no different IMHO.