[PATCH v14 01/10] [media] v4l2: add new V4L2_PIX_FMT_RGB666 pixel format.

[PATCH v14 01/10] [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.

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>
Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
---
ChangeLog v13->v14:
- None
ChangeLog v10->v13:
- No changes
ChangeLog v9->v10:
- Rebased on top of:
  "211e7f2 [media] DocBook media: drop the old incorrect packed RGB table"
- Added Philipp Zabel's Ack.

ChangeLog v8->v9:
- Removed the Cc. They are now set in git-send-email directly.

ChangeLog v7->v8:
- Added Mauro Carvalho Chehab back to the list of Cc

ChangeLog v6->v7:
- Shrinked even more the Cc list.
ChangeLog v5->v6:
- Remove people not concerned by this patch from the Cc list.

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        |   39 ++++++++++++++++++++
 include/uapi/linux/videodev2.h                     |    1 +
 2 files changed, 40 insertions(+)

diff --git a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
index e1c4f8b..88a7fe1 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>
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 168ff50..08cac01 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -299,6 +299,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

[PATCH v14 02/10] imx-drm: Add RGB666 support for parallel display.

[Denis Carikli]

Signed-off-by: Denis Carikli <denis@eukrea.com>
Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
---
ChangeLog v13->v14:
- Rebased
ChangeLog v9->v13:
- Rebased
ChangeLog v8->v9:
- Rebased.
- Added Philipp Zabel's ack.
- Shortened the patch title.

ChangeLog v8->v9:
- Removed the Cc. They are now set in git-send-email directly.
- Rebased.

ChangeLog v7->v8:
- Shrinked even more the Cc list.

ChangeLog v6->v7:
- Shrinked even more the Cc list.

ChangeLog v5->v6:
- Remove people not concerned by this patch from the Cc list.

ChangeLog v3->v5:
- Use the correct RGB order.

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       |    4 ++--
 drivers/gpu/ipu-v3/ipu-dc.c                        |    9 +++++++++
 drivers/staging/imx-drm/parallel-display.c         |    2 ++
 3 files changed, 13 insertions(+), 2 deletions(-)

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 e75f0e5..c0eb95a 100644
--- a/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
+++ b/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
@@ -60,8 +60,8 @@ Required properties:
 - compatible: Should be "fsl,imx-parallel-display"
 Optional properties:
 - interface_pix_fmt: How this display is connected to the
-  display interface. Currently supported types: "rgb24", "rgb565", "bgr666"
-  and "lvds666".
+  display interface. Currently supported types: "rgb24", "rgb565", "bgr666",
+  "rgb666" and "lvds666".
 - edid: verbatim EDID data block describing attached display.
 - ddc: phandle describing the i2c bus handling the display data
   channel
diff --git a/drivers/gpu/ipu-v3/ipu-dc.c b/drivers/gpu/ipu-v3/ipu-dc.c
index 2326c75..100d410 100644
--- a/drivers/gpu/ipu-v3/ipu-dc.c
+++ b/drivers/gpu/ipu-v3/ipu-dc.c
@@ -93,6 +93,7 @@ enum ipu_dc_map {
 	IPU_DC_MAP_BGR666,
 	IPU_DC_MAP_LVDS666,
 	IPU_DC_MAP_BGR24,
+	IPU_DC_MAP_RGB666,
 };
 
 struct ipu_dc {
@@ -161,6 +162,8 @@ static int ipu_pixfmt_to_map(u32 fmt)
 		return IPU_DC_MAP_LVDS666;
 	case V4L2_PIX_FMT_BGR24:
 		return IPU_DC_MAP_BGR24;
+	case V4L2_PIX_FMT_RGB666:
+		return IPU_DC_MAP_RGB666;
 	default:
 		return -EINVAL;
 	}
@@ -452,6 +455,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, 0, 5, 0xfc); /* blue */
+	ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 1, 11, 0xfc); /* green */
+	ipu_dc_map_config(priv, IPU_DC_MAP_RGB666, 2, 17, 0xfc); /* red */
+
 	return 0;
 }
 
diff --git a/drivers/staging/imx-drm/parallel-display.c b/drivers/staging/imx-drm/parallel-display.c
index b567832..64b34336 100644
--- a/drivers/staging/imx-drm/parallel-display.c
+++ b/drivers/staging/imx-drm/parallel-display.c
@@ -218,6 +218,8 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data)
 			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;
 		else if (!strcmp(fmt, "lvds666"))
 			imxpd->interface_pix_fmt = v4l2_fourcc('L', 'V', 'D', '6');
 	}
-- 
1.7.9.5

[PATCH v14 02/10] imx-drm: Add RGB666 support for parallel display.

[Russell King - ARM Linux]

On Mon, Jun 16, 2014 at 12:11:16PM +0200, Denis Carikli wrote:
> Signed-off-by: Denis Carikli <denis@eukrea.com>
> Acked-by: Philipp Zabel <p.zabel@pengutronix.de>

As I said probably around v10 time, I already have this patch queued,
and I was going to send it to Greg before the previous merge window,
but due to the number of patches I was already carrying, it was lost
amongst the trees.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH v14 03/10] imx-drm: Correct BGR666 and the board's dts that use them.

[Denis Carikli]

The current BGR666 is not consistent with the other color mapings like BGR24.
BGR666 should be in the same byte order than BGR24.

Signed-off-by: Denis Carikli <denis@eukrea.com>
Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
---
ChangeLog v13->v14:
- Rebased
ChangeLog v10->v13:
- Rebased
ChangeLog v9->v10:
- Rebased.
- Added Philipp Zabel's Ack.
- Included Lothar Wa?mann's suggestion about imx-ldb.c.
- Shortened the patch title

ChangeLog v8->v9:
- Removed the Cc. They are now set in git-send-email directly.

ChangeLog v7->v8:
- Shrinked even more the Cc list.

ChangeLog v6->v7:
- Shrinked even more the Cc list.

ChangeLog v5->v6:
- Remove people not concerned by this patch from the Cc list.
- Added a better explanation of the change.

ChangeLog v5:
- New patch.
---
 arch/arm/boot/dts/imx51-apf51dev.dts |    2 +-
 arch/arm/boot/dts/imx53-m53evk.dts   |    2 +-
 drivers/gpu/ipu-v3/ipu-dc.c          |    4 ++--
 drivers/staging/imx-drm/imx-ldb.c    |    4 ++--
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/dts/imx51-apf51dev.dts b/arch/arm/boot/dts/imx51-apf51dev.dts
index c5a9a24..7b3851d 100644
--- a/arch/arm/boot/dts/imx51-apf51dev.dts
+++ b/arch/arm/boot/dts/imx51-apf51dev.dts
@@ -18,7 +18,7 @@
 
 	display at di1 {
 		compatible = "fsl,imx-parallel-display";
-		interface-pix-fmt = "bgr666";
+		interface-pix-fmt = "rgb666";
 		pinctrl-names = "default";
 		pinctrl-0 = <&pinctrl_ipu_disp1>;
 
diff --git a/arch/arm/boot/dts/imx53-m53evk.dts b/arch/arm/boot/dts/imx53-m53evk.dts
index d5d146a..4b036b4 100644
--- a/arch/arm/boot/dts/imx53-m53evk.dts
+++ b/arch/arm/boot/dts/imx53-m53evk.dts
@@ -24,7 +24,7 @@
 	soc {
 		display1: display at di1 {
 			compatible = "fsl,imx-parallel-display";
-			interface-pix-fmt = "bgr666";
+			interface-pix-fmt = "rgb666";
 			pinctrl-names = "default";
 			pinctrl-0 = <&pinctrl_ipu_disp1>;
 
diff --git a/drivers/gpu/ipu-v3/ipu-dc.c b/drivers/gpu/ipu-v3/ipu-dc.c
index 100d410..9974d41 100644
--- a/drivers/gpu/ipu-v3/ipu-dc.c
+++ b/drivers/gpu/ipu-v3/ipu-dc.c
@@ -439,9 +439,9 @@ int ipu_dc_init(struct ipu_soc *ipu, struct device *dev,
 
 	/* 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, 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, 17, 0xfc); /* red */
+	ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 2, 5, 0xfc); /* red */
 
 	/* lvds666 */
 	ipu_dc_map_clear(priv, IPU_DC_MAP_LVDS666);
diff --git a/drivers/staging/imx-drm/imx-ldb.c b/drivers/staging/imx-drm/imx-ldb.c
index 7e3f019..5d22e40 100644
--- a/drivers/staging/imx-drm/imx-ldb.c
+++ b/drivers/staging/imx-drm/imx-ldb.c
@@ -188,11 +188,11 @@ static void imx_ldb_encoder_prepare(struct drm_encoder *encoder)
 	switch (imx_ldb_ch->chno) {
 	case 0:
 		pixel_fmt = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH0_24) ?
-			V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_BGR666;
+			V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_RGB666;
 		break;
 	case 1:
 		pixel_fmt = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH1_24) ?
-			V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_BGR666;
+			V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_RGB666;
 		break;
 	default:
 		dev_err(ldb->dev, "unable to config di%d panel format\n",
-- 
1.7.9.5

[PATCH v14 03/10] imx-drm: Correct BGR666 and the board's dts that use them.

[Russell King - ARM Linux]

On Mon, Jun 16, 2014 at 12:11:17PM +0200, Denis Carikli wrote:
> The current BGR666 is not consistent with the other color mapings like BGR24.
> BGR666 should be in the same byte order than BGR24.
> 
> Signed-off-by: Denis Carikli <denis@eukrea.com>
> Acked-by: Philipp Zabel <p.zabel@pengutronix.de>

As I said probably around v10 time, I already have this patch queued,
and I was going to send it to Greg before the previous merge window,
but due to the number of patches I was already carrying, it was lost
amongst the trees.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH v14 04/10] imx-drm: use defines for clock polarity settings

[Denis Carikli]

Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v13->v14:
- Rebased
ChangeLog 12->v13:
- No changes
ChangeLog 11->v12:
- Improved the define names to match the hardware:
  ENABLE_POL is not a clock signal but instead an enable signal.

ChangeLog v9->v10:
- New patch which was splitted out from:
  "staging: imx-drm: Use de-active and pixelclk-active display-timings.".
- Fixes many issues in "staging: imx-drm: Use de-active and pixelclk-active
  display-timings.":
  - More clear meaning of the polarity settings.
  - The SET_CLK_POL and SET_DE_POL masks are not
    needed anymore.
---
 drivers/gpu/ipu-v3/ipu-di.c          |    4 ++--
 drivers/staging/imx-drm/ipuv3-crtc.c |    4 ++--
 include/video/imx-ipu-v3.h           |    8 +++++++-
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/ipu-v3/ipu-di.c b/drivers/gpu/ipu-v3/ipu-di.c
index c490ba4..d00f357 100644
--- a/drivers/gpu/ipu-v3/ipu-di.c
+++ b/drivers/gpu/ipu-v3/ipu-di.c
@@ -595,7 +595,7 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct ipu_di_signal_cfg *sig)
 		}
 	}
 
-	if (sig->clk_pol)
+	if (sig->clk_pol == CLK_POL_POSEDGE)
 		di_gen |= DI_GEN_POLARITY_DISP_CLK;
 
 	ipu_di_write(di, di_gen, DI_GENERAL);
@@ -606,7 +606,7 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct ipu_di_signal_cfg *sig)
 	reg = ipu_di_read(di, DI_POL);
 	reg &= ~(DI_POL_DRDY_DATA_POLARITY | DI_POL_DRDY_POLARITY_15);
 
-	if (sig->enable_pol)
+	if (sig->enable_pol == ENABLE_POL_HIGH)
 		reg |= DI_POL_DRDY_POLARITY_15;
 	if (sig->data_pol)
 		reg |= DI_POL_DRDY_DATA_POLARITY;
diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c b/drivers/staging/imx-drm/ipuv3-crtc.c
index 720868b..7fec438 100644
--- a/drivers/staging/imx-drm/ipuv3-crtc.c
+++ b/drivers/staging/imx-drm/ipuv3-crtc.c
@@ -165,8 +165,8 @@ 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 = 0;
+	sig_cfg.enable_pol = ENABLE_POL_HIGH;
+	sig_cfg.clk_pol = CLK_POL_NEGEDGE;
 	sig_cfg.width = mode->hdisplay;
 	sig_cfg.height = mode->vdisplay;
 	sig_cfg.pixel_fmt = out_pixel_fmt;
diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
index 3e43e22..8888305 100644
--- a/include/video/imx-ipu-v3.h
+++ b/include/video/imx-ipu-v3.h
@@ -27,6 +27,12 @@ enum ipuv3_type {
 
 #define IPU_PIX_FMT_GBR24	v4l2_fourcc('G', 'B', 'R', '3')
 
+#define CLK_POL_NEGEDGE		0
+#define CLK_POL_POSEDGE		1
+
+#define ENABLE_POL_LOW		0
+#define ENABLE_POL_HIGH		1
+
 /*
  * Bitfield of Display Interface signal polarities.
  */
@@ -37,7 +43,7 @@ struct ipu_di_signal_cfg {
 	unsigned clksel_en:1;
 	unsigned clkidle_en:1;
 	unsigned data_pol:1;	/* true = inverted */
-	unsigned clk_pol:1;	/* true = rising edge */
+	unsigned clk_pol:1;
 	unsigned enable_pol:1;
 	unsigned Hsync_pol:1;	/* true = active high */
 	unsigned Vsync_pol:1;
-- 
1.7.9.5

[PATCH v14 04/10] imx-drm: use defines for clock polarity settings

[Russell King - ARM Linux]

On Mon, Jun 16, 2014 at 12:11:18PM +0200, Denis Carikli wrote:
> Signed-off-by: Denis Carikli <denis@eukrea.com>

It would be nice to have a little more explanation in the commit messages
for these patches.  If you'd like to send me better commit messages for
these patches, I'll add them to what I already have:

	imx-drm: use defines for clock polarity settings

	imx-drm: add RGB666 support for parallel display.

It may also be worth describing the RGB666 format in the commit message
for:

	v4l2: add new V4L2_PIX_FMT_RGB666 pixel format.

And... getting some more acks for these patches would be very useful,
I think I'd like to see Sascha's ack for these... Sascha?

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH v14 04/10] imx-drm: use defines for clock polarity settings

[Denis Carikli]

On 06/24/2014 05:13 PM, Russell King - ARM Linux wrote:
[...]
> If you'd like to send me better commit messages for
> these patches, I'll add them to what I already have:

> 	imx-drm: use defines for clock polarity settings
The comment of the clk_pol field of the ipu_di_signal_cfg struct was 
inverted.
Instead of merely inverting the comment, the values of clk_pol were defined.

> 	imx-drm: add RGB666 support for parallel display.
This permits to drive parallel displays that expect the RGB666 color format.
>
> It may also be worth describing the RGB666 format in the commit message
> for:
>
> 	v4l2: add new V4L2_PIX_FMT_RGB666 pixel format.
The RGB666 color format encodes 6 bits for each color(red, green and 
blue), linearly.
It looks like this in memory:
0                17
RRRRRRGGGGGGBBBBBB

Denis.

[PATCH v14 04/10] imx-drm: use defines for clock polarity settings

[Russell King - ARM Linux]

On Tue, Jun 24, 2014 at 06:25:19PM +0200, Denis Carikli wrote:
> On 06/24/2014 05:13 PM, Russell King - ARM Linux wrote:
> [...]
>> If you'd like to send me better commit messages for
>> these patches, I'll add them to what I already have:
>
>> 	imx-drm: use defines for clock polarity settings
> The comment of the clk_pol field of the ipu_di_signal_cfg struct was  
> inverted.
> Instead of merely inverting the comment, the values of clk_pol were defined.

s/inverting/fixing/

>
>> 	imx-drm: add RGB666 support for parallel display.
> This permits to drive parallel displays that expect the RGB666 color format.

This allows imx-drm to drive ...

>>
>> It may also be worth describing the RGB666 format in the commit message
>> for:
>>
>> 	v4l2: add new V4L2_PIX_FMT_RGB666 pixel format.
> The RGB666 color format encodes 6 bits for each color(red, green and  
> blue), linearly.
> It looks like this in memory:
> 0                17
> RRRRRRGGGGGGBBBBBB

Thanks! I've tweaked them very slightly as detailed above so they read a
bit better.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH v14 04/10] imx-drm: use defines for clock polarity settings

[Sascha Hauer]

On Mon, Jun 16, 2014 at 12:11:18PM +0200, Denis Carikli wrote:
> Signed-off-by: Denis Carikli <denis@eukrea.com>
> ---
> ChangeLog v13->v14:
> - Rebased
> ChangeLog 12->v13:
> - No changes
> ChangeLog 11->v12:
> - Improved the define names to match the hardware:
>   ENABLE_POL is not a clock signal but instead an enable signal.
> 
> ChangeLog v9->v10:
> - New patch which was splitted out from:
>   "staging: imx-drm: Use de-active and pixelclk-active display-timings.".
> - Fixes many issues in "staging: imx-drm: Use de-active and pixelclk-active
>   display-timings.":
>   - More clear meaning of the polarity settings.
>   - The SET_CLK_POL and SET_DE_POL masks are not
>     needed anymore.
> ---
>  drivers/gpu/ipu-v3/ipu-di.c          |    4 ++--
>  drivers/staging/imx-drm/ipuv3-crtc.c |    4 ++--
>  include/video/imx-ipu-v3.h           |    8 +++++++-
>  3 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/ipu-v3/ipu-di.c b/drivers/gpu/ipu-v3/ipu-di.c
> index c490ba4..d00f357 100644
> --- a/drivers/gpu/ipu-v3/ipu-di.c
> +++ b/drivers/gpu/ipu-v3/ipu-di.c
> @@ -595,7 +595,7 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct ipu_di_signal_cfg *sig)
>  		}
>  	}
>  
> -	if (sig->clk_pol)
> +	if (sig->clk_pol == CLK_POL_POSEDGE)
>  		di_gen |= DI_GEN_POLARITY_DISP_CLK;
>  
>  	ipu_di_write(di, di_gen, DI_GENERAL);
> @@ -606,7 +606,7 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct ipu_di_signal_cfg *sig)
>  	reg = ipu_di_read(di, DI_POL);
>  	reg &= ~(DI_POL_DRDY_DATA_POLARITY | DI_POL_DRDY_POLARITY_15);
>  
> -	if (sig->enable_pol)
> +	if (sig->enable_pol == ENABLE_POL_HIGH)
>  		reg |= DI_POL_DRDY_POLARITY_15;
>  	if (sig->data_pol)
>  		reg |= DI_POL_DRDY_DATA_POLARITY;
> diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c b/drivers/staging/imx-drm/ipuv3-crtc.c
> index 720868b..7fec438 100644
> --- a/drivers/staging/imx-drm/ipuv3-crtc.c
> +++ b/drivers/staging/imx-drm/ipuv3-crtc.c
> @@ -165,8 +165,8 @@ 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 = 0;
> +	sig_cfg.enable_pol = ENABLE_POL_HIGH;
> +	sig_cfg.clk_pol = CLK_POL_NEGEDGE;
>  	sig_cfg.width = mode->hdisplay;
>  	sig_cfg.height = mode->vdisplay;
>  	sig_cfg.pixel_fmt = out_pixel_fmt;
> diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
> index 3e43e22..8888305 100644
> --- a/include/video/imx-ipu-v3.h
> +++ b/include/video/imx-ipu-v3.h
> @@ -27,6 +27,12 @@ enum ipuv3_type {
>  
>  #define IPU_PIX_FMT_GBR24	v4l2_fourcc('G', 'B', 'R', '3')
>  
> +#define CLK_POL_NEGEDGE		0
> +#define CLK_POL_POSEDGE		1
> +
> +#define ENABLE_POL_LOW		0
> +#define ENABLE_POL_HIGH		1

Adding defines without a proper namespace (IPU_) outside a driver
private header file is not nice. Anyway, instead of adding the
defines ...

> +
>  /*
>   * Bitfield of Display Interface signal polarities.
>   */
> @@ -37,7 +43,7 @@ struct ipu_di_signal_cfg {
>  	unsigned clksel_en:1;
>  	unsigned clkidle_en:1;
>  	unsigned data_pol:1;	/* true = inverted */
> -	unsigned clk_pol:1;	/* true = rising edge */
> +	unsigned clk_pol:1;
>  	unsigned enable_pol:1;
>  	unsigned Hsync_pol:1;	/* true = active high */
>  	unsigned Vsync_pol:1;

...can we rename the flags to more meaningful names instead?

	unsigned clk_pol_rising_edge:1;
	unsigned enable_pol_high:1;
	unsigned hsync_active_high:1;
	unsigned vsync_active_high:1;

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v14 04/10] imx-drm: use defines for clock polarity settings

[Russell King - ARM Linux]

On Wed, Jun 25, 2014 at 06:48:45AM +0200, Sascha Hauer wrote:
> On Mon, Jun 16, 2014 at 12:11:18PM +0200, Denis Carikli wrote:
> > +
> >  /*
> >   * Bitfield of Display Interface signal polarities.
> >   */
> > @@ -37,7 +43,7 @@ struct ipu_di_signal_cfg {
> >  	unsigned clksel_en:1;
> >  	unsigned clkidle_en:1;
> >  	unsigned data_pol:1;	/* true = inverted */
> > -	unsigned clk_pol:1;	/* true = rising edge */
> > +	unsigned clk_pol:1;
> >  	unsigned enable_pol:1;
> >  	unsigned Hsync_pol:1;	/* true = active high */
> >  	unsigned Vsync_pol:1;
> 
> ...can we rename the flags to more meaningful names instead?
> 
> 	unsigned clk_pol_rising_edge:1;
> 	unsigned enable_pol_high:1;
> 	unsigned hsync_active_high:1;
> 	unsigned vsync_active_high:1;

Now look at patch 7, where these become tri-state:
- don't change
- rising edge/active high
- falling edge/active low

So your suggestion is not a good idea.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH v14 04/10] imx-drm: use defines for clock polarity settings

[Sascha Hauer]

On Wed, Jun 25, 2014 at 09:43:27AM +0100, Russell King - ARM Linux wrote:
> On Wed, Jun 25, 2014 at 06:48:45AM +0200, Sascha Hauer wrote:
> > On Mon, Jun 16, 2014 at 12:11:18PM +0200, Denis Carikli wrote:
> > > +
> > >  /*
> > >   * Bitfield of Display Interface signal polarities.
> > >   */
> > > @@ -37,7 +43,7 @@ struct ipu_di_signal_cfg {
> > >  	unsigned clksel_en:1;
> > >  	unsigned clkidle_en:1;
> > >  	unsigned data_pol:1;	/* true = inverted */
> > > -	unsigned clk_pol:1;	/* true = rising edge */
> > > +	unsigned clk_pol:1;
> > >  	unsigned enable_pol:1;
> > >  	unsigned Hsync_pol:1;	/* true = active high */
> > >  	unsigned Vsync_pol:1;
> > 
> > ...can we rename the flags to more meaningful names instead?
> > 
> > 	unsigned clk_pol_rising_edge:1;
> > 	unsigned enable_pol_high:1;
> > 	unsigned hsync_active_high:1;
> > 	unsigned vsync_active_high:1;
> 
> Now look at patch 7, where these become tri-state:
> - don't change
> - rising edge/active high
> - falling edge/active low
> 
> So your suggestion is not a good idea.

Hm, you're right.

Still I think we should add a prefix to make the context of the flags
clear.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v14 04/10] imx-drm: use defines for clock polarity settings

[Denis Carikli]

On 06/25/2014 06:48 AM, Sascha Hauer wrote:
>> +#define ENABLE_POL_LOW		0
>> +#define ENABLE_POL_HIGH		1
>
> Adding defines without a proper namespace (IPU_) outside a driver
> private header file is not nice. Anyway, instead of adding the
> defines ...
Fixed in "imx-drm: use defines for clock polarity settings" and in 
"imx-drm: Use drm_display_mode timings flags.".

Denis.

[PATCH v14 04/10] imx-drm: use defines for clock polarity settings

[Russell King - ARM Linux]

On Wed, Jun 25, 2014 at 11:44:47AM +0200, Denis Carikli wrote:
> On 06/25/2014 06:48 AM, Sascha Hauer wrote:
>>> +#define ENABLE_POL_LOW		0
>>> +#define ENABLE_POL_HIGH		1
>>
>> Adding defines without a proper namespace (IPU_) outside a driver
>> private header file is not nice. Anyway, instead of adding the
>> defines ...
> Fixed in "imx-drm: use defines for clock polarity settings" and in  
> "imx-drm: Use drm_display_mode timings flags.".

Denis, can you send just this one updated patch, so I can update the
one I have here with this change.  Once you've done that, I'll send
the first four off to Greg.

Thanks.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH v14 updated] imx-drm: use defines for clock polarity settings

[Denis Carikli]

Signed-off-by: Denis Carikli <denis@eukrea.com>
---
 drivers/gpu/ipu-v3/ipu-di.c          |    4 ++--
 drivers/staging/imx-drm/ipuv3-crtc.c |    4 ++--
 include/video/imx-ipu-v3.h           |    8 +++++++-
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/ipu-v3/ipu-di.c b/drivers/gpu/ipu-v3/ipu-di.c
index c490ba4..5f7f799 100644
--- a/drivers/gpu/ipu-v3/ipu-di.c
+++ b/drivers/gpu/ipu-v3/ipu-di.c
@@ -595,7 +595,7 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct ipu_di_signal_cfg *sig)
 		}
 	}
 
-	if (sig->clk_pol)
+	if (sig->clk_pol == IPU_CLK_POL_POSEDGE)
 		di_gen |= DI_GEN_POLARITY_DISP_CLK;
 
 	ipu_di_write(di, di_gen, DI_GENERAL);
@@ -606,7 +606,7 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct ipu_di_signal_cfg *sig)
 	reg = ipu_di_read(di, DI_POL);
 	reg &= ~(DI_POL_DRDY_DATA_POLARITY | DI_POL_DRDY_POLARITY_15);
 
-	if (sig->enable_pol)
+	if (sig->enable_pol == IPU_ENABLE_POL_HIGH)
 		reg |= DI_POL_DRDY_POLARITY_15;
 	if (sig->data_pol)
 		reg |= DI_POL_DRDY_DATA_POLARITY;
diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c b/drivers/staging/imx-drm/ipuv3-crtc.c
index 720868b..4e54f7a 100644
--- a/drivers/staging/imx-drm/ipuv3-crtc.c
+++ b/drivers/staging/imx-drm/ipuv3-crtc.c
@@ -165,8 +165,8 @@ 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 = 0;
+	sig_cfg.enable_pol = IPU_ENABLE_POL_HIGH;
+	sig_cfg.clk_pol = IPU_CLK_POL_NEGEDGE;
 	sig_cfg.width = mode->hdisplay;
 	sig_cfg.height = mode->vdisplay;
 	sig_cfg.pixel_fmt = out_pixel_fmt;
diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
index 3e43e22..d52edb4 100644
--- a/include/video/imx-ipu-v3.h
+++ b/include/video/imx-ipu-v3.h
@@ -27,6 +27,12 @@ enum ipuv3_type {
 
 #define IPU_PIX_FMT_GBR24	v4l2_fourcc('G', 'B', 'R', '3')
 
+#define IPU_CLK_POL_NEGEDGE	0
+#define IPU_CLK_POL_POSEDGE	1
+
+#define IPU_ENABLE_POL_LOW	0
+#define IPU_ENABLE_POL_HIGH	1
+
 /*
  * Bitfield of Display Interface signal polarities.
  */
@@ -37,7 +43,7 @@ struct ipu_di_signal_cfg {
 	unsigned clksel_en:1;
 	unsigned clkidle_en:1;
 	unsigned data_pol:1;	/* true = inverted */
-	unsigned clk_pol:1;	/* true = rising edge */
+	unsigned clk_pol:1;
 	unsigned enable_pol:1;
 	unsigned Hsync_pol:1;	/* true = active high */
 	unsigned Vsync_pol:1;
-- 
1.7.9.5

[PATCH v14 05/10] ARM: dts: imx5*, imx6*: correct display-timings nodes.

[Denis Carikli]

The imx-drm driver can't use the de-active and
pixelclk-active display-timings properties yet.

Instead the data-enable and the pixel data clock
polarity are hardcoded in the imx-drm driver.

So theses properties are now set to keep
the same behaviour when imx-drm will start
using them.

Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v13->v14:
- None
ChangeLog v10->v11:
- imx53-tx53-x03x.dts change was removed because it 
  already had the correct setting.
ChangeLog v9->v10:
- New patch that was splitted out of:
  "staging imx-drm: Use de-active and pixelclk-active
  display-timings."
---
 arch/arm/boot/dts/imx51-babbage.dts       |    2 ++
 arch/arm/boot/dts/imx53-m53evk.dts        |    2 ++
 arch/arm/boot/dts/imx6qdl-gw53xx.dtsi     |    2 ++
 arch/arm/boot/dts/imx6qdl-gw54xx.dtsi     |    2 ++
 arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi |    2 ++
 arch/arm/boot/dts/imx6qdl-sabreauto.dtsi  |    2 ++
 arch/arm/boot/dts/imx6qdl-sabrelite.dtsi  |    2 ++
 arch/arm/boot/dts/imx6qdl-sabresd.dtsi    |    2 ++
 8 files changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/imx51-babbage.dts b/arch/arm/boot/dts/imx51-babbage.dts
index ee51a10..b64a9e3 100644
--- a/arch/arm/boot/dts/imx51-babbage.dts
+++ b/arch/arm/boot/dts/imx51-babbage.dts
@@ -56,6 +56,8 @@
 				vfront-porch = <7>;
 				hsync-len = <60>;
 				vsync-len = <10>;
+				de-active = <1>;
+				pixelclk-active = <0>;
 			};
 		};
 
diff --git a/arch/arm/boot/dts/imx53-m53evk.dts b/arch/arm/boot/dts/imx53-m53evk.dts
index 4b036b4..d03ced7 100644
--- a/arch/arm/boot/dts/imx53-m53evk.dts
+++ b/arch/arm/boot/dts/imx53-m53evk.dts
@@ -41,6 +41,8 @@
 					vfront-porch = <9>;
 					vsync-len = <3>;
 					vsync-active = <1>;
+					de-active = <1>;
+					pixelclk-active = <0>;
 				};
 			};
 		};
diff --git a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
index d3125f0..7f993d6 100644
--- a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
@@ -512,6 +512,8 @@
 				vfront-porch = <7>;
 				hsync-len = <60>;
 				vsync-len = <10>;
+				de-active = <1>;
+				pixelclk-active = <0>;
 			};
 		};
 	};
diff --git a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
index 532347f..e06cf9e 100644
--- a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
@@ -534,6 +534,8 @@
 				vfront-porch = <7>;
 				hsync-len = <60>;
 				vsync-len = <10>;
+				de-active = <1>;
+				pixelclk-active = <0>;
 			};
 		};
 	};
diff --git a/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi b/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
index 4c4b175..bcf5178 100644
--- a/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
@@ -353,6 +353,8 @@
 				vfront-porch = <7>;
 				hsync-len = <60>;
 				vsync-len = <10>;
+				de-active = <1>;
+				pixelclk-active = <0>;
 			};
 		};
 	};
diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
index 009abd6..230bbc6 100644
--- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
@@ -405,6 +405,8 @@
 				vfront-porch = <7>;
 				hsync-len = <60>;
 				vsync-len = <10>;
+				de-active = <1>;
+				pixelclk-active = <0>;
 			};
 		};
 	};
diff --git a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
index 6df6127..9f6b406 100644
--- a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
@@ -353,6 +353,8 @@
 				vfront-porch = <7>;
 				hsync-len = <60>;
 				vsync-len = <10>;
+				de-active = <1>;
+				pixelclk-active = <0>;
 			};
 		};
 	};
diff --git a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
index e446192..3297779 100644
--- a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
@@ -494,6 +494,8 @@
 				vfront-porch = <7>;
 				hsync-len = <60>;
 				vsync-len = <10>;
+				de-active = <1>;
+				pixelclk-active = <0>;
 			};
 		};
 	};
-- 
1.7.9.5

[PATCH v14 05/10] ARM: dts: imx5*, imx6*: correct display-timings nodes.

[Russell King - ARM Linux]

On Mon, Jun 16, 2014 at 12:11:19PM +0200, Denis Carikli wrote:
> The imx-drm driver can't use the de-active and
> pixelclk-active display-timings properties yet.
> 
> Instead the data-enable and the pixel data clock
> polarity are hardcoded in the imx-drm driver.
> 
> So theses properties are now set to keep
> the same behaviour when imx-drm will start
> using them.
> 
> Signed-off-by: Denis Carikli <denis@eukrea.com>

This patch needs either an ack from the arm-soc/iMX maintainers, or
they need to merge it.  As there's little positive agreement on the
series, I can understand why there's reluctance to merge it.

So, can we start having some acks from people please, or at least
commitments to merge this patch when the others are deemed to be
acceptable.  If not, can we have explanations why this should not
be merged.

Thanks.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH v14 05/10] ARM: dts: imx5*, imx6*: correct display-timings nodes.

[Shawn Guo]

On Tue, Jun 24, 2014 at 04:01:58PM +0100, Russell King - ARM Linux wrote:
> On Mon, Jun 16, 2014 at 12:11:19PM +0200, Denis Carikli wrote:
> > The imx-drm driver can't use the de-active and
> > pixelclk-active display-timings properties yet.
> > 
> > Instead the data-enable and the pixel data clock
> > polarity are hardcoded in the imx-drm driver.
> > 
> > So theses properties are now set to keep
> > the same behaviour when imx-drm will start
> > using them.
> > 
> > Signed-off-by: Denis Carikli <denis@eukrea.com>
> 
> This patch needs either an ack from the arm-soc/iMX maintainers, or
> they need to merge it.  As there's little positive agreement on the
> series, I can understand why there's reluctance to merge it.
> 
> So, can we start having some acks from people please, or at least
> commitments to merge this patch when the others are deemed to be
> acceptable.  If not, can we have explanations why this should not
> be merged.

I will be happy to merge dts change through IMX tree once the
binding/diver part gets accepted/applied.

Shawn

[PATCH v14 06/10] drm: drm_display_mode: add signal polarity flags

[Denis Carikli]

We need a way to pass signal polarity informations
  between DRM panels, and the display drivers.

To do that, a pol_flags field was added to drm_display_mode.

Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v13->v14:
- Fixed DRM_MODE_FLAG_POL_DE_HIGH's description.
ChangeLog v12->v13:
- Added Docbook documentation for pol_flags the struct field.
- Removed the _PRESERVE	defines: it was used by patches
  against the imx_drm driver. Now theses patches have been
  adapted not to require that defines.
ChangeLog v11->v12:
- Rebased: This patch now applies against drm_modes.h
- Rebased: It now uses the new DRM_MODE_FLAG_POL_DE flags defines names

ChangeLog v10->v11:
- Since the imx-drm won't be able to retrive its regulators
  from the device tree when using display-timings nodes,
  and that I was told that the drm simple-panel driver 
  already supported that, I then, instead, added what was
  lacking to make the eukrea displays work with the
  drm-simple-panel driver.

  That required a way to get back the display polarity
  informations from the imx-drm driver without affecting
  userspace.
---
 Documentation/DocBook/drm.tmpl |   30 ++++++++++++++++++++++++++++++
 include/drm/drm_modes.h        |    6 ++++++
 2 files changed, 36 insertions(+)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 7df3134..22d435f 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2292,6 +2292,36 @@ void intel_crt_init(struct drm_device *dev)
             and <structfield>height_mm</structfield> fields are only used internally
             during EDID parsing and should not be set when creating modes manually.
           </para>
+          <para>
+            The <structfield>pol_flags</structfield> value represents the display
+            signal polarity flags, it can be a combination of
+            <variablelist>
+              <varlistentry>
+                <term>DRM_MODE_FLAG_POL_PIXDATA_NEGEDGE</term>
+                 <listitem><para>
+                     drive pixel data on falling edge, sample data on rising edge.
+                 </para></listitem>
+              </varlistentry>
+              <varlistentry>
+                <term>DRM_MODE_FLAG_POL_PIXDATA_POSEDGE</term>
+                <listitem><para>
+                  Drive pixel data on rising edge, sample data on falling edge.
+                </para></listitem>
+              </varlistentry>
+              <varlistentry>
+                <term>DRM_MODE_FLAG_POL_DE_LOW</term>
+                <listitem><para>
+                  data-enable pulse is active low
+                </para></listitem>
+              </varlistentry>
+              <varlistentry>
+                <term>DRM_MODE_FLAG_POL_DE_HIGH</term>
+                <listitem><para>
+                  data-enable pulse is active high
+                </para></listitem>
+              </varlistentry>
+            </variablelist>
+          </para>
         </listitem>
         <listitem>
           <synopsis>int (*mode_valid)(struct drm_connector *connector,
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index 91d0582..c5cbe31 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -93,6 +93,11 @@ enum drm_mode_status {
 
 #define DRM_MODE_FLAG_3D_MAX	DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF
 
+#define DRM_MODE_FLAG_POL_PIXDATA_NEGEDGE	BIT(1)
+#define DRM_MODE_FLAG_POL_PIXDATA_POSEDGE	BIT(2)
+#define DRM_MODE_FLAG_POL_DE_LOW		BIT(3)
+#define DRM_MODE_FLAG_POL_DE_HIGH		BIT(4)
+
 struct drm_display_mode {
 	/* Header */
 	struct list_head head;
@@ -144,6 +149,7 @@ struct drm_display_mode {
 	int vrefresh;		/* in Hz */
 	int hsync;		/* in kHz */
 	enum hdmi_picture_aspect picture_aspect_ratio;
+	unsigned int pol_flags;
 };
 
 /* mode specified on the command line */
-- 
1.7.9.5

[PATCH v14 06/10] drm: drm_display_mode: add signal polarity flags

[Russell King - ARM Linux]

On Mon, Jun 16, 2014 at 12:11:20PM +0200, Denis Carikli wrote:
> We need a way to pass signal polarity informations
>   between DRM panels, and the display drivers.
> 
> To do that, a pol_flags field was added to drm_display_mode.
> 
> Signed-off-by: Denis Carikli <denis@eukrea.com>

This patch needs an ack from the DRM people - can someone review it
please?  This series has now been round 14 revisions and it's about
time it was properly reviewed - or a statement made if it's
unacceptable.

> ---
> ChangeLog v13->v14:
> - Fixed DRM_MODE_FLAG_POL_DE_HIGH's description.
> ChangeLog v12->v13:
> - Added Docbook documentation for pol_flags the struct field.
> - Removed the _PRESERVE	defines: it was used by patches
>   against the imx_drm driver. Now theses patches have been
>   adapted not to require that defines.
> ChangeLog v11->v12:
> - Rebased: This patch now applies against drm_modes.h
> - Rebased: It now uses the new DRM_MODE_FLAG_POL_DE flags defines names
> 
> ChangeLog v10->v11:
> - Since the imx-drm won't be able to retrive its regulators
>   from the device tree when using display-timings nodes,
>   and that I was told that the drm simple-panel driver 
>   already supported that, I then, instead, added what was
>   lacking to make the eukrea displays work with the
>   drm-simple-panel driver.
> 
>   That required a way to get back the display polarity
>   informations from the imx-drm driver without affecting
>   userspace.
> ---
>  Documentation/DocBook/drm.tmpl |   30 ++++++++++++++++++++++++++++++
>  include/drm/drm_modes.h        |    6 ++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index 7df3134..22d435f 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -2292,6 +2292,36 @@ void intel_crt_init(struct drm_device *dev)
>              and <structfield>height_mm</structfield> fields are only used internally
>              during EDID parsing and should not be set when creating modes manually.
>            </para>
> +          <para>
> +            The <structfield>pol_flags</structfield> value represents the display
> +            signal polarity flags, it can be a combination of
> +            <variablelist>
> +              <varlistentry>
> +                <term>DRM_MODE_FLAG_POL_PIXDATA_NEGEDGE</term>
> +                 <listitem><para>
> +                     drive pixel data on falling edge, sample data on rising edge.
> +                 </para></listitem>
> +              </varlistentry>
> +              <varlistentry>
> +                <term>DRM_MODE_FLAG_POL_PIXDATA_POSEDGE</term>
> +                <listitem><para>
> +                  Drive pixel data on rising edge, sample data on falling edge.
> +                </para></listitem>
> +              </varlistentry>
> +              <varlistentry>
> +                <term>DRM_MODE_FLAG_POL_DE_LOW</term>
> +                <listitem><para>
> +                  data-enable pulse is active low
> +                </para></listitem>
> +              </varlistentry>
> +              <varlistentry>
> +                <term>DRM_MODE_FLAG_POL_DE_HIGH</term>
> +                <listitem><para>
> +                  data-enable pulse is active high
> +                </para></listitem>
> +              </varlistentry>
> +            </variablelist>
> +          </para>
>          </listitem>
>          <listitem>
>            <synopsis>int (*mode_valid)(struct drm_connector *connector,
> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
> index 91d0582..c5cbe31 100644
> --- a/include/drm/drm_modes.h
> +++ b/include/drm/drm_modes.h
> @@ -93,6 +93,11 @@ enum drm_mode_status {
>  
>  #define DRM_MODE_FLAG_3D_MAX	DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF
>  
> +#define DRM_MODE_FLAG_POL_PIXDATA_NEGEDGE	BIT(1)
> +#define DRM_MODE_FLAG_POL_PIXDATA_POSEDGE	BIT(2)
> +#define DRM_MODE_FLAG_POL_DE_LOW		BIT(3)
> +#define DRM_MODE_FLAG_POL_DE_HIGH		BIT(4)
> +
>  struct drm_display_mode {
>  	/* Header */
>  	struct list_head head;
> @@ -144,6 +149,7 @@ struct drm_display_mode {
>  	int vrefresh;		/* in Hz */
>  	int hsync;		/* in kHz */
>  	enum hdmi_picture_aspect picture_aspect_ratio;
> +	unsigned int pol_flags;
>  };
>  
>  /* mode specified on the command line */
> -- 
> 1.7.9.5
> 

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH v14 06/10] drm: drm_display_mode: add signal polarity flags

[Thierry Reding]

On Tue, Jun 24, 2014 at 03:57:46PM +0100, Russell King - ARM Linux wrote:
> On Mon, Jun 16, 2014 at 12:11:20PM +0200, Denis Carikli wrote:
> > We need a way to pass signal polarity informations
> >   between DRM panels, and the display drivers.
> > 
> > To do that, a pol_flags field was added to drm_display_mode.
> > 
> > Signed-off-by: Denis Carikli <denis@eukrea.com>
> 
> This patch needs an ack from the DRM people - can someone review it
> please?  This series has now been round 14 revisions and it's about
> time it was properly reviewed - or a statement made if it's
> unacceptable.

I didn't follow all of the earlier discussions around this, but it seems
to me like data-enable polarity and the pixel data edge flags are
properties of the interface rather than the video mode.

struct drm_display_mode represents the video timings and I'm not sure if
it's a good idea to extend it with this type of information.

Maybe we need to add a separate type of device to store these parameters
(much like we've done for MIPI DSI devices).

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140625/400d1e9e/attachment.sig>

[PATCH v14 07/10] imx-drm: Use drm_display_mode timings flags.

[Denis Carikli]

The previous hardware behaviour was kept if the
flags are not set.

Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v13->v14:
- Rebased

ChangeLog v12->v13:
- This patch doesn't need the DRM_MODE_FLAG_POL_*_PRESERVE flags anymore.
- code cleanup to improve readability:
  - ENABLE_POL_PRESERVE is now gone
  - Less modifications in ipu_di_init_sync_panel
  - more readable modifications in int ipu_crtc_mode_set
ChangeLog v11->v12:
- Rebased: It now uses the following new flags defines names:
  CLK_POL, ENABLE_POL
- The inversions in ipuv3-crtc.c are now fixed.
- ipuv3-crtc.c was still using mode->private_flags
  from the previous versions of this patchset, that's now fixed.

ChangeLog v10->v11:
- This patch was splitted-out and adapted from:
  "Prepare imx-drm for extra display-timings retrival."
- The display-timings dt specific part was removed.
- The flags names were changed to use the DRM ones from:
  "drm: drm_display_mode: add signal polarity flags"
---
 drivers/gpu/ipu-v3/ipu-di.c          |    2 ++
 drivers/staging/imx-drm/ipuv3-crtc.c |   18 ++++++++++++++++--
 include/video/imx-ipu-v3.h           |    4 ++--
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/ipu-v3/ipu-di.c b/drivers/gpu/ipu-v3/ipu-di.c
index d00f357..1a1e116 100644
--- a/drivers/gpu/ipu-v3/ipu-di.c
+++ b/drivers/gpu/ipu-v3/ipu-di.c
@@ -597,6 +597,8 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct ipu_di_signal_cfg *sig)
 
 	if (sig->clk_pol == CLK_POL_POSEDGE)
 		di_gen |= DI_GEN_POLARITY_DISP_CLK;
+	else if (sig->clk_pol == CLK_POL_NEGEDGE)
+		di_gen &= ~DI_GEN_POLARITY_DISP_CLK;
 
 	ipu_di_write(di, di_gen, DI_GENERAL);
 
diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c b/drivers/staging/imx-drm/ipuv3-crtc.c
index 7fec438..7fdf575 100644
--- a/drivers/staging/imx-drm/ipuv3-crtc.c
+++ b/drivers/staging/imx-drm/ipuv3-crtc.c
@@ -165,8 +165,22 @@ 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 = ENABLE_POL_HIGH;
-	sig_cfg.clk_pol = CLK_POL_NEGEDGE;
+	if (mode->pol_flags & DRM_MODE_FLAG_POL_PIXDATA_POSEDGE)
+		sig_cfg.clk_pol = CLK_POL_POSEDGE;
+	else if (mode->pol_flags & DRM_MODE_FLAG_POL_PIXDATA_NEGEDGE)
+		sig_cfg.clk_pol = CLK_POL_NEGEDGE;
+	else
+		/* If no PIXDATA flags were set, keep the old behaviour */
+		sig_cfg.clk_pol = CLK_POL_NEGEDGE;
+
+	if (mode->pol_flags & DRM_MODE_FLAG_POL_DE_HIGH)
+		sig_cfg.enable_pol = ENABLE_POL_HIGH;
+	else if (mode->pol_flags & DRM_MODE_FLAG_POL_DE_LOW)
+		sig_cfg.enable_pol = ENABLE_POL_LOW;
+	else
+		/* If no DE flags were set, keep the old behaviour */
+		sig_cfg.enable_pol = ENABLE_POL_HIGH;
+
 	sig_cfg.width = mode->hdisplay;
 	sig_cfg.height = mode->vdisplay;
 	sig_cfg.pixel_fmt = out_pixel_fmt;
diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
index 8888305..e660522 100644
--- a/include/video/imx-ipu-v3.h
+++ b/include/video/imx-ipu-v3.h
@@ -43,10 +43,10 @@ struct ipu_di_signal_cfg {
 	unsigned clksel_en:1;
 	unsigned clkidle_en:1;
 	unsigned data_pol:1;	/* true = inverted */
-	unsigned clk_pol:1;
-	unsigned enable_pol:1;
 	unsigned Hsync_pol:1;	/* true = active high */
 	unsigned Vsync_pol:1;
+	u8 clk_pol;
+	u8 enable_pol;
 
 	u16 width;
 	u16 height;
-- 
1.7.9.5

[PATCH v14 07/10] imx-drm: Use drm_display_mode timings flags.

[Russell King - ARM Linux]

On Mon, Jun 16, 2014 at 12:11:21PM +0200, Denis Carikli wrote:
> The previous hardware behaviour was kept if the
> flags are not set.

I'd like to throw in a patch that I've been carrying for a bit now.
It conflicts with your patches, but I'm happy to fix that conflict
locally (and have been doing so for a while now.)

This is related to a slightly different issue - knowing which types
of bridges are bound to a particualar DI.  This matters in part for
selecting the clock routing - as things currently stand, the last
bridge to call imx_drm_panel_format*() gets its way with this.  With
this change, we can see which bridges are bound, and make the
appropriate decision.  At the moment, we are saved from things going
awry as we don't support cloning outputs.

The relevence to your patch set is that some bridges require clk_pol
to be configured appropriately - HDMI requires clk_pol = 0 in order to
work correctly (with the opposite edge, the image is noisy.)

While this approach only allows us to identify the types of bridges
connected to a DI rather than uniquely identifing the bridges themselves,
I think this is not only an improvement, but also a simplification of
the current code, and allows better decisions about things like clk_pol
to be made.

I'm sending it here because it is relevent to Denis' patch set - I will
also send it out separately if people want it separately, though that
will go to a reduced Cc list.

From: Russell King <rmk+kernel@arm.linux.org.uk>
Subject: [PATCH] imx-drm: core: handling of DI clock flags to
 ipu_crtc_mode_set()

We do not need to track the state of the IPU DI's clock flags by having
each display bridge calling back into imx-drm-core, and then back out
into ipuv3-crtc.c.

ipuv3-crtc can instead just scan the list of encoders to retrieve their
type, and build up a picture of which types of encoders are attached.
We can then use this information to configure the IPU DI clocking mode
without any uncertainty - if we have multiple bridges connected to the
same DI, if one of them requires a synchronous DI clock, that's what we
must use.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/staging/imx-drm/imx-drm-core.c |  3 +--
 drivers/staging/imx-drm/imx-drm.h      |  2 +-
 drivers/staging/imx-drm/ipuv3-crtc.c   | 40 +++++++++++++++++++---------------
 3 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/imx-drm/imx-drm-core.c b/drivers/staging/imx-drm/imx-drm-core.c
index def8280d7ee6..6d9376c760ad 100644
--- a/drivers/staging/imx-drm/imx-drm-core.c
+++ b/drivers/staging/imx-drm/imx-drm-core.c
@@ -115,8 +115,7 @@ int imx_drm_panel_format_pins(struct drm_encoder *encoder,
 	helper = &imx_crtc->imx_drm_helper_funcs;
 	if (helper->set_interface_pix_fmt)
 		return helper->set_interface_pix_fmt(encoder->crtc,
-				encoder->encoder_type, interface_pix_fmt,
-				hsync_pin, vsync_pin);
+				interface_pix_fmt, hsync_pin, vsync_pin);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(imx_drm_panel_format_pins);
diff --git a/drivers/staging/imx-drm/imx-drm.h b/drivers/staging/imx-drm/imx-drm.h
index 7453ae00c412..3c559ccd6af0 100644
--- a/drivers/staging/imx-drm/imx-drm.h
+++ b/drivers/staging/imx-drm/imx-drm.h
@@ -17,7 +17,7 @@ int imx_drm_crtc_id(struct imx_drm_crtc *crtc);
 struct imx_drm_crtc_helper_funcs {
 	int (*enable_vblank)(struct drm_crtc *crtc);
 	void (*disable_vblank)(struct drm_crtc *crtc);
-	int (*set_interface_pix_fmt)(struct drm_crtc *crtc, u32 encoder_type,
+	int (*set_interface_pix_fmt)(struct drm_crtc *crtc,
 			u32 pix_fmt, int hsync_pin, int vsync_pin);
 	const struct drm_crtc_helper_funcs *crtc_helper_funcs;
 	const struct drm_crtc_funcs *crtc_funcs;
diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c b/drivers/staging/imx-drm/ipuv3-crtc.c
index 7fec438d8c54..af09032aedb0 100644
--- a/drivers/staging/imx-drm/ipuv3-crtc.c
+++ b/drivers/staging/imx-drm/ipuv3-crtc.c
@@ -51,7 +51,6 @@ struct ipu_crtc {
 	struct drm_framebuffer	*newfb;
 	int			irq;
 	u32			interface_pix_fmt;
-	unsigned long		di_clkflags;
 	int			di_hsync_pin;
 	int			di_vsync_pin;
 };
@@ -146,10 +145,13 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc,
 			       int x, int y,
 			       struct drm_framebuffer *old_fb)
 {
+	struct drm_device *dev = crtc->dev;
+	struct drm_encoder *encoder;
 	struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
-	int ret;
 	struct ipu_di_signal_cfg sig_cfg = {};
+	unsigned long encoder_types = 0;
 	u32 out_pixel_fmt;
+	int ret;
 
 	dev_dbg(ipu_crtc->dev, "%s: mode->hdisplay: %d\n", __func__,
 			mode->hdisplay);
@@ -165,6 +167,24 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc,
 	if (mode->flags & DRM_MODE_FLAG_PVSYNC)
 		sig_cfg.Vsync_pol = 1;
 
+	list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
+		if (encoder->crtc == crtc)
+			encoder_types |= BIT(encoder->encoder_type);
+
+	dev_dbg(ipu_crtc->dev, "%s: attached to encoder types 0x%lx\n",
+		__func__, encoder_types);
+
+	/*
+	 * If we have DAC, TVDAC or LDB, then we need the IPU DI clock
+	 * to be the same as the LDB DI clock.
+	 */
+	if (encoder_types & (BIT(DRM_MODE_ENCODER_DAC) | 
+			     BIT(DRM_MODE_ENCODER_TVDAC) |
+			     BIT(DRM_MODE_ENCODER_LVDS)))
+		sig_cfg.clkflags = IPU_DI_CLKMODE_SYNC | IPU_DI_CLKMODE_EXT;
+	else
+		sig_cfg.clkflags = 0;
+
 	sig_cfg.enable_pol = ENABLE_POL_HIGH;
 	sig_cfg.clk_pol = CLK_POL_NEGEDGE;
 	sig_cfg.width = mode->hdisplay;
@@ -178,7 +198,6 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc,
 	sig_cfg.v_sync_width = mode->vsync_end - mode->vsync_start;
 	sig_cfg.v_end_width = mode->vsync_start - mode->vdisplay;
 	sig_cfg.pixelclock = mode->clock * 1000;
-	sig_cfg.clkflags = ipu_crtc->di_clkflags;
 
 	sig_cfg.v_to_h_sync = 0;
 
@@ -277,7 +296,7 @@ static void ipu_disable_vblank(struct drm_crtc *crtc)
 	ipu_crtc->newfb = NULL;
 }
 
-static int ipu_set_interface_pix_fmt(struct drm_crtc *crtc, u32 encoder_type,
+static int ipu_set_interface_pix_fmt(struct drm_crtc *crtc,
 		u32 pixfmt, int hsync_pin, int vsync_pin)
 {
 	struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
@@ -286,19 +305,6 @@ static int ipu_set_interface_pix_fmt(struct drm_crtc *crtc, u32 encoder_type,
 	ipu_crtc->di_hsync_pin = hsync_pin;
 	ipu_crtc->di_vsync_pin = vsync_pin;
 
-	switch (encoder_type) {
-	case DRM_MODE_ENCODER_DAC:
-	case DRM_MODE_ENCODER_TVDAC:
-	case DRM_MODE_ENCODER_LVDS:
-		ipu_crtc->di_clkflags = IPU_DI_CLKMODE_SYNC |
-			IPU_DI_CLKMODE_EXT;
-		break;
-	case DRM_MODE_ENCODER_TMDS:
-	case DRM_MODE_ENCODER_NONE:
-		ipu_crtc->di_clkflags = 0;
-		break;
-	}
-
 	return 0;
 }
 
-- 
1.8.3.1

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH v14 08/10] drm/panel: Add Eukrea mbimxsd51 displays.

[Denis Carikli]

Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v13->v14:
- None

ChangeLog v12->v13:
- Added a note explaining why the size is zero in
  the eukrea_mbimxsd51_dvi(s)vga structs.
ChangeLog v11->v12:
- Rebased: It now uses the new DRM_MODE_FLAG_POL_DE flags defines names

ChangeLog v10->v11:
- New patch.
---
 .../bindings/panel/eukrea,mbimxsd51-cmo-qvga.txt   |    7 ++
 .../bindings/panel/eukrea,mbimxsd51-dvi-svga.txt   |    7 ++
 .../bindings/panel/eukrea,mbimxsd51-dvi-vga.txt    |    7 ++
 drivers/gpu/drm/panel/panel-simple.c               |   83 ++++++++++++++++++++
 4 files changed, 104 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-cmo-qvga.txt
 create mode 100644 Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-svga.txt
 create mode 100644 Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt

diff --git a/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-cmo-qvga.txt b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-cmo-qvga.txt
new file mode 100644
index 0000000..03679d0
--- /dev/null
+++ b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-cmo-qvga.txt
@@ -0,0 +1,7 @@
+Eukrea CMO-QVGA (320x240 pixels) TFT LCD panel
+
+Required properties:
+- compatible: should be "eukrea,mbimxsd51-cmo-qvga"
+
+This binding is compatible with the simple-panel binding, which is specified
+in simple-panel.txt in this directory.
diff --git a/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-svga.txt b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-svga.txt
new file mode 100644
index 0000000..f408c9a
--- /dev/null
+++ b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-svga.txt
@@ -0,0 +1,7 @@
+Eukrea DVI-SVGA (800x600 pixels) DVI output.
+
+Required properties:
+- compatible: should be "eukrea,mbimxsd51-dvi-svga"
+
+This binding is compatible with the simple-panel binding, which is specified
+in simple-panel.txt in this directory.
diff --git a/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt
new file mode 100644
index 0000000..8ea90da
--- /dev/null
+++ b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt
@@ -0,0 +1,7 @@
+Eukrea DVI-VGA (640x480 pixels) DVI output.
+
+Required properties:
+- compatible: should be "eukrea,mbimxsd51-dvi-vga"
+
+This binding is compatible with the simple-panel binding, which is specified
+in simple-panel.txt in this directory.
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index a251361..adc40a7 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -403,6 +403,80 @@ static const struct panel_desc edt_etm0700g0dh6 = {
 	},
 };
 
+static const struct drm_display_mode eukrea_mbimxsd51_cmoqvga_mode = {
+	.clock = 6500,
+	.hdisplay = 320,
+	.hsync_start = 320 + 38,
+	.hsync_end = 320 + 38 + 20,
+	.htotal = 320 + 38 + 20 + 30,
+	.vdisplay = 240,
+	.vsync_start = 240 + 15,
+	.vsync_end = 240 + 15 + 4,
+	.vtotal = 240 + 15 + 4 + 3,
+	.vrefresh = 60,
+	.pol_flags = DRM_MODE_FLAG_POL_PIXDATA_NEGEDGE |
+		     DRM_MODE_FLAG_POL_DE_LOW,
+};
+
+static const struct panel_desc eukrea_mbimxsd51_cmoqvga = {
+	.modes = &eukrea_mbimxsd51_cmoqvga_mode,
+	.num_modes = 1,
+	.size = {
+		.width = 73,
+		.height = 56,
+	},
+};
+
+static const struct drm_display_mode eukrea_mbimxsd51_dvisvga_mode = {
+	.clock = 44333,
+	.hdisplay = 800,
+	.hsync_start = 800 + 112,
+	.hsync_end = 800 + 112 + 32,
+	.htotal = 800 + 112 + 32 + 80,
+	.vdisplay = 600,
+	.vsync_start = 600 + 3,
+	.vsync_end = 600 + 3 + 17,
+	.vtotal = 600 + 3 + 17 + 4,
+	.vrefresh = 60,
+	.pol_flags = DRM_MODE_FLAG_POL_PIXDATA_POSEDGE |
+		     DRM_MODE_FLAG_POL_DE_HIGH,
+};
+
+static const struct panel_desc eukrea_mbimxsd51_dvisvga = {
+	.modes = &eukrea_mbimxsd51_dvisvga_mode,
+	.num_modes = 1,
+	/* This is a DVI adapter for external displays */
+	.size = {
+		.width = 0,
+		.height = 0,
+	},
+};
+
+static const struct drm_display_mode eukrea_mbimxsd51_dvivga_mode = {
+	.clock = 23750,
+	.hdisplay = 640,
+	.hsync_start = 640 + 80,
+	.hsync_end = 640 + 80 + 16,
+	.htotal = 640 + 80 + 16 + 64,
+	.vdisplay = 480,
+	.vsync_start = 480 + 3,
+	.vsync_end = 480 + 3 + 13,
+	.vtotal  = 480 + 3 + 13 + 4,
+	.vrefresh = 60,
+	.pol_flags = DRM_MODE_FLAG_POL_PIXDATA_POSEDGE |
+		     DRM_MODE_FLAG_POL_DE_HIGH,
+};
+
+static const struct panel_desc eukrea_mbimxsd51_dvivga = {
+	.modes = &eukrea_mbimxsd51_dvivga_mode,
+	.num_modes = 1,
+	/* This is a DVI adapter for external displays */
+	.size = {
+		.width = 0,
+		.height = 0,
+	},
+};
+
 static const struct drm_display_mode lg_lp129qe_mode = {
 	.clock = 285250,
 	.hdisplay = 2560,
@@ -458,6 +532,15 @@ static const struct of_device_id platform_of_match[] = {
 		.compatible = "chunghwa,claa101wa01a",
 		.data = &chunghwa_claa101wa01a
 	}, {
+		.compatible = "eukrea,mbimxsd51-cmo-qvga",
+		.data = &eukrea_mbimxsd51_cmoqvga,
+	}, {
+		.compatible = "eukrea,mbimxsd51-dvi-svga",
+		.data = &eukrea_mbimxsd51_dvisvga,
+	}, {
+		.compatible = "eukrea,mbimxsd51-dvi-vga",
+		.data = &eukrea_mbimxsd51_dvivga,
+	}, {
 		.compatible = "chunghwa,claa101wb01",
 		.data = &chunghwa_claa101wb01
 	}, {
-- 
1.7.9.5

[PATCH v14 08/10] drm/panel: Add Eukrea mbimxsd51 displays.

[Russell King - ARM Linux]

Denis,

This patch creates binding documentation.  Any patch which does so
should be copied to the DT people so they can review the bindings
and give appropriate acks.  It would be better if you separate the
binding documentation updates from the other functional changes too.

I've added them on this reply to see whether they'll feel friendly
enough to comment on the patch as it stands to avoid having to go
through two more rounds on this already-fourteen revision patch set.

On Mon, Jun 16, 2014 at 12:11:22PM +0200, Denis Carikli wrote:
> Signed-off-by: Denis Carikli <denis@eukrea.com>
> ---
> ChangeLog v13->v14:
> - None
> 
> ChangeLog v12->v13:
> - Added a note explaining why the size is zero in
>   the eukrea_mbimxsd51_dvi(s)vga structs.
> ChangeLog v11->v12:
> - Rebased: It now uses the new DRM_MODE_FLAG_POL_DE flags defines names
> 
> ChangeLog v10->v11:
> - New patch.
> ---
>  .../bindings/panel/eukrea,mbimxsd51-cmo-qvga.txt   |    7 ++
>  .../bindings/panel/eukrea,mbimxsd51-dvi-svga.txt   |    7 ++
>  .../bindings/panel/eukrea,mbimxsd51-dvi-vga.txt    |    7 ++
>  drivers/gpu/drm/panel/panel-simple.c               |   83 ++++++++++++++++++++
>  4 files changed, 104 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-cmo-qvga.txt
>  create mode 100644 Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-svga.txt
>  create mode 100644 Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt
> 
> diff --git a/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-cmo-qvga.txt b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-cmo-qvga.txt
> new file mode 100644
> index 0000000..03679d0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-cmo-qvga.txt
> @@ -0,0 +1,7 @@
> +Eukrea CMO-QVGA (320x240 pixels) TFT LCD panel
> +
> +Required properties:
> +- compatible: should be "eukrea,mbimxsd51-cmo-qvga"
> +
> +This binding is compatible with the simple-panel binding, which is specified
> +in simple-panel.txt in this directory.
> diff --git a/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-svga.txt b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-svga.txt
> new file mode 100644
> index 0000000..f408c9a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-svga.txt
> @@ -0,0 +1,7 @@
> +Eukrea DVI-SVGA (800x600 pixels) DVI output.
> +
> +Required properties:
> +- compatible: should be "eukrea,mbimxsd51-dvi-svga"
> +
> +This binding is compatible with the simple-panel binding, which is specified
> +in simple-panel.txt in this directory.
> diff --git a/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt
> new file mode 100644
> index 0000000..8ea90da
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt
> @@ -0,0 +1,7 @@
> +Eukrea DVI-VGA (640x480 pixels) DVI output.
> +
> +Required properties:
> +- compatible: should be "eukrea,mbimxsd51-dvi-vga"
> +
> +This binding is compatible with the simple-panel binding, which is specified
> +in simple-panel.txt in this directory.
> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> index a251361..adc40a7 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -403,6 +403,80 @@ static const struct panel_desc edt_etm0700g0dh6 = {
>  	},
>  };
>  
> +static const struct drm_display_mode eukrea_mbimxsd51_cmoqvga_mode = {
> +	.clock = 6500,
> +	.hdisplay = 320,
> +	.hsync_start = 320 + 38,
> +	.hsync_end = 320 + 38 + 20,
> +	.htotal = 320 + 38 + 20 + 30,
> +	.vdisplay = 240,
> +	.vsync_start = 240 + 15,
> +	.vsync_end = 240 + 15 + 4,
> +	.vtotal = 240 + 15 + 4 + 3,
> +	.vrefresh = 60,
> +	.pol_flags = DRM_MODE_FLAG_POL_PIXDATA_NEGEDGE |
> +		     DRM_MODE_FLAG_POL_DE_LOW,
> +};
> +
> +static const struct panel_desc eukrea_mbimxsd51_cmoqvga = {
> +	.modes = &eukrea_mbimxsd51_cmoqvga_mode,
> +	.num_modes = 1,
> +	.size = {
> +		.width = 73,
> +		.height = 56,
> +	},
> +};
> +
> +static const struct drm_display_mode eukrea_mbimxsd51_dvisvga_mode = {
> +	.clock = 44333,
> +	.hdisplay = 800,
> +	.hsync_start = 800 + 112,
> +	.hsync_end = 800 + 112 + 32,
> +	.htotal = 800 + 112 + 32 + 80,
> +	.vdisplay = 600,
> +	.vsync_start = 600 + 3,
> +	.vsync_end = 600 + 3 + 17,
> +	.vtotal = 600 + 3 + 17 + 4,
> +	.vrefresh = 60,
> +	.pol_flags = DRM_MODE_FLAG_POL_PIXDATA_POSEDGE |
> +		     DRM_MODE_FLAG_POL_DE_HIGH,
> +};
> +
> +static const struct panel_desc eukrea_mbimxsd51_dvisvga = {
> +	.modes = &eukrea_mbimxsd51_dvisvga_mode,
> +	.num_modes = 1,
> +	/* This is a DVI adapter for external displays */
> +	.size = {
> +		.width = 0,
> +		.height = 0,
> +	},
> +};
> +
> +static const struct drm_display_mode eukrea_mbimxsd51_dvivga_mode = {
> +	.clock = 23750,
> +	.hdisplay = 640,
> +	.hsync_start = 640 + 80,
> +	.hsync_end = 640 + 80 + 16,
> +	.htotal = 640 + 80 + 16 + 64,
> +	.vdisplay = 480,
> +	.vsync_start = 480 + 3,
> +	.vsync_end = 480 + 3 + 13,
> +	.vtotal  = 480 + 3 + 13 + 4,
> +	.vrefresh = 60,
> +	.pol_flags = DRM_MODE_FLAG_POL_PIXDATA_POSEDGE |
> +		     DRM_MODE_FLAG_POL_DE_HIGH,
> +};
> +
> +static const struct panel_desc eukrea_mbimxsd51_dvivga = {
> +	.modes = &eukrea_mbimxsd51_dvivga_mode,
> +	.num_modes = 1,
> +	/* This is a DVI adapter for external displays */
> +	.size = {
> +		.width = 0,
> +		.height = 0,
> +	},
> +};
> +
>  static const struct drm_display_mode lg_lp129qe_mode = {
>  	.clock = 285250,
>  	.hdisplay = 2560,
> @@ -458,6 +532,15 @@ static const struct of_device_id platform_of_match[] = {
>  		.compatible = "chunghwa,claa101wa01a",
>  		.data = &chunghwa_claa101wa01a
>  	}, {
> +		.compatible = "eukrea,mbimxsd51-cmo-qvga",
> +		.data = &eukrea_mbimxsd51_cmoqvga,
> +	}, {
> +		.compatible = "eukrea,mbimxsd51-dvi-svga",
> +		.data = &eukrea_mbimxsd51_dvisvga,
> +	}, {
> +		.compatible = "eukrea,mbimxsd51-dvi-vga",
> +		.data = &eukrea_mbimxsd51_dvivga,
> +	}, {
>  		.compatible = "chunghwa,claa101wb01",
>  		.data = &chunghwa_claa101wb01
>  	}, {
> -- 
> 1.7.9.5
> 

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH v14 08/10] drm/panel: Add Eukrea mbimxsd51 displays.

[Rob Herring]

On Tue, Jun 24, 2014 at 10:06 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> Denis,
>
> This patch creates binding documentation.  Any patch which does so
> should be copied to the DT people so they can review the bindings
> and give appropriate acks.  It would be better if you separate the
> binding documentation updates from the other functional changes too.
>
> I've added them on this reply to see whether they'll feel friendly
> enough to comment on the patch as it stands to avoid having to go
> through two more rounds on this already-fourteen revision patch set.
>
> On Mon, Jun 16, 2014 at 12:11:22PM +0200, Denis Carikli wrote:
>> Signed-off-by: Denis Carikli <denis@eukrea.com>
>> ---
>> ChangeLog v13->v14:
>> - None
>>
>> ChangeLog v12->v13:
>> - Added a note explaining why the size is zero in
>>   the eukrea_mbimxsd51_dvi(s)vga structs.
>> ChangeLog v11->v12:
>> - Rebased: It now uses the new DRM_MODE_FLAG_POL_DE flags defines names
>>
>> ChangeLog v10->v11:
>> - New patch.
>> ---
>>  .../bindings/panel/eukrea,mbimxsd51-cmo-qvga.txt   |    7 ++
>>  .../bindings/panel/eukrea,mbimxsd51-dvi-svga.txt   |    7 ++
>>  .../bindings/panel/eukrea,mbimxsd51-dvi-vga.txt    |    7 ++
>>  drivers/gpu/drm/panel/panel-simple.c               |   83 ++++++++++++++++++++
>>  4 files changed, 104 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-cmo-qvga.txt
>>  create mode 100644 Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-svga.txt
>>  create mode 100644 Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt
>>
>> diff --git a/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-cmo-qvga.txt b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-cmo-qvga.txt
>> new file mode 100644
>> index 0000000..03679d0
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-cmo-qvga.txt
>> @@ -0,0 +1,7 @@
>> +Eukrea CMO-QVGA (320x240 pixels) TFT LCD panel
>> +
>> +Required properties:
>> +- compatible: should be "eukrea,mbimxsd51-cmo-qvga"
>> +
>> +This binding is compatible with the simple-panel binding, which is specified
>> +in simple-panel.txt in this directory.
>> diff --git a/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-svga.txt b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-svga.txt
>> new file mode 100644
>> index 0000000..f408c9a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-svga.txt
>> @@ -0,0 +1,7 @@
>> +Eukrea DVI-SVGA (800x600 pixels) DVI output.
>> +
>> +Required properties:
>> +- compatible: should be "eukrea,mbimxsd51-dvi-svga"
>> +
>> +This binding is compatible with the simple-panel binding, which is specified
>> +in simple-panel.txt in this directory.
>> diff --git a/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt
>> new file mode 100644
>> index 0000000..8ea90da
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt
>> @@ -0,0 +1,7 @@
>> +Eukrea DVI-VGA (640x480 pixels) DVI output.
>> +
>> +Required properties:
>> +- compatible: should be "eukrea,mbimxsd51-dvi-vga"
>> +
>> +This binding is compatible with the simple-panel binding, which is specified
>> +in simple-panel.txt in this directory.

Seems like we could just have a list of compatible strings rather than
a mostly duplicated file.

>> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
>> index a251361..adc40a7 100644
>> --- a/drivers/gpu/drm/panel/panel-simple.c
>> +++ b/drivers/gpu/drm/panel/panel-simple.c
>> @@ -403,6 +403,80 @@ static const struct panel_desc edt_etm0700g0dh6 = {
>>       },
>>  };
>>
>> +static const struct drm_display_mode eukrea_mbimxsd51_cmoqvga_mode = {
>> +     .clock = 6500,
>> +     .hdisplay = 320,
>> +     .hsync_start = 320 + 38,
>> +     .hsync_end = 320 + 38 + 20,
>> +     .htotal = 320 + 38 + 20 + 30,
>> +     .vdisplay = 240,
>> +     .vsync_start = 240 + 15,
>> +     .vsync_end = 240 + 15 + 4,
>> +     .vtotal = 240 + 15 + 4 + 3,
>> +     .vrefresh = 60,
>> +     .pol_flags = DRM_MODE_FLAG_POL_PIXDATA_NEGEDGE |
>> +                  DRM_MODE_FLAG_POL_DE_LOW,

Why aren't you using:

Documentation/devicetree/bindings/video/display-timing.txt

Rob

[PATCH v14 08/10] drm/panel: Add Eukrea mbimxsd51 displays.

[Thierry Reding]

On Tue, Jun 24, 2014 at 02:52:11PM -0500, Rob Herring wrote:
> On Tue, Jun 24, 2014 at 10:06 AM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
[...]
> > On Mon, Jun 16, 2014 at 12:11:22PM +0200, Denis Carikli wrote:
[...]
> >> diff --git a/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt
[...]
> >> @@ -0,0 +1,7 @@
> >> +Eukrea DVI-VGA (640x480 pixels) DVI output.
> >> +
> >> +Required properties:
> >> +- compatible: should be "eukrea,mbimxsd51-dvi-vga"
> >> +
> >> +This binding is compatible with the simple-panel binding, which is specified
> >> +in simple-panel.txt in this directory.
> 
> Seems like we could just have a list of compatible strings rather than
> a mostly duplicated file.

We've been doing it this way for all other panels.

> >> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> >> index a251361..adc40a7 100644
> >> --- a/drivers/gpu/drm/panel/panel-simple.c
> >> +++ b/drivers/gpu/drm/panel/panel-simple.c
> >> @@ -403,6 +403,80 @@ static const struct panel_desc edt_etm0700g0dh6 = {
> >>       },
> >>  };
> >>
> >> +static const struct drm_display_mode eukrea_mbimxsd51_cmoqvga_mode = {
> >> +     .clock = 6500,
> >> +     .hdisplay = 320,
> >> +     .hsync_start = 320 + 38,
> >> +     .hsync_end = 320 + 38 + 20,
> >> +     .htotal = 320 + 38 + 20 + 30,
> >> +     .vdisplay = 240,
> >> +     .vsync_start = 240 + 15,
> >> +     .vsync_end = 240 + 15 + 4,
> >> +     .vtotal = 240 + 15 + 4 + 3,
> >> +     .vrefresh = 60,
> >> +     .pol_flags = DRM_MODE_FLAG_POL_PIXDATA_NEGEDGE |
> >> +                  DRM_MODE_FLAG_POL_DE_LOW,
> 
> Why aren't you using:
> 
> Documentation/devicetree/bindings/video/display-timing.txt

Because it's redundant information. We need to have a compatible for the
panel in the device tree anyway and that already implicitly defines the
display mode.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140624/15203d17/attachment.sig>

[PATCH v14 08/10] drm/panel: Add Eukrea mbimxsd51 displays.

[Denis Carikli]

On 06/24/2014 05:06 PM, Russell King - ARM Linux wrote:
 > It would be better if you separate the
> binding documentation updates from the other functional changes too.
Fixed.

Denis.

[PATCH v14 08/10] drm/panel: Add Eukrea mbimxsd51 displays.

[Thierry Reding]

On Mon, Jun 16, 2014 at 12:11:22PM +0200, Denis Carikli wrote:
[...]
> diff --git a/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-svga.txt b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-svga.txt
[...]
> @@ -0,0 +1,7 @@
> +Eukrea DVI-SVGA (800x600 pixels) DVI output.
[...]
> diff --git a/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt
[...]
> @@ -0,0 +1,7 @@
> +Eukrea DVI-VGA (640x480 pixels) DVI output.

DVI outputs shouldn't be using the panel framework and this binding at
all. DVI usually has the means to determine all of this by itself. Why
do you need to represent this as a panel in device tree?

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140624/6168382a/attachment-0001.sig>

[PATCH v14 08/10] drm/panel: Add Eukrea mbimxsd51 displays.

[Eric Bénard]

Hi Thierry,

Le Tue, 24 Jun 2014 23:49:37 +0200,
Thierry Reding <thierry.reding@gmail.com> a ?crit :

> On Mon, Jun 16, 2014 at 12:11:22PM +0200, Denis Carikli wrote:
> [...]
> > diff --git a/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-svga.txt b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-svga.txt
> [...]
> > @@ -0,0 +1,7 @@
> > +Eukrea DVI-SVGA (800x600 pixels) DVI output.
> [...]
> > diff --git a/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt
> [...]
> > @@ -0,0 +1,7 @@
> > +Eukrea DVI-VGA (640x480 pixels) DVI output.
> 
> DVI outputs shouldn't be using the panel framework and this binding at
> all. DVI usually has the means to determine all of this by itself. Why
> do you need to represent this as a panel in device tree?
> 
because on this very simple display board, we only have DVI LVDS signals
without the I2C to detect the display.

Best regards
Eric

[PATCH v14 08/10] drm/panel: Add Eukrea mbimxsd51 displays.

[Thierry Reding]

On Tue, Jun 24, 2014 at 11:56:39PM +0200, Eric B?nard wrote:
> Hi Thierry,
> 
> Le Tue, 24 Jun 2014 23:49:37 +0200,
> Thierry Reding <thierry.reding@gmail.com> a ?crit :
> 
> > On Mon, Jun 16, 2014 at 12:11:22PM +0200, Denis Carikli wrote:
> > [...]
> > > diff --git a/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-svga.txt b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-svga.txt
> > [...]
> > > @@ -0,0 +1,7 @@
> > > +Eukrea DVI-SVGA (800x600 pixels) DVI output.
> > [...]
> > > diff --git a/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt b/Documentation/devicetree/bindings/panel/eukrea,mbimxsd51-dvi-vga.txt
> > [...]
> > > @@ -0,0 +1,7 @@
> > > +Eukrea DVI-VGA (640x480 pixels) DVI output.
> > 
> > DVI outputs shouldn't be using the panel framework and this binding at
> > all. DVI usually has the means to determine all of this by itself. Why
> > do you need to represent this as a panel in device tree?
> > 
> because on this very simple display board, we only have DVI LVDS signals
> without the I2C to detect the display.

That's unfortunate. In that case perhaps a better approach would be to
add a video timings node to the device that provides the DVI output?

The panel bindings are really for internal panels and should define all
of their properties. That's also why they need a specific compatible
string.

What the above two bindings define are really "connectors" with a fixed
resolution rather than panels.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140625/157429d0/attachment.sig>

[PATCH v14 08/10] drm/panel: Add Eukrea mbimxsd51 displays.

[Denis Carikli]

On 06/25/2014 12:04 AM, Thierry Reding wrote:
>> because on this very simple display board, we only have DVI LVDS signals
>> without the I2C to detect the display.
>
> That's unfortunate. In that case perhaps a better approach would be to
> add a video timings node to the device that provides the DVI output?
I've just done that.

Should I resend now? The goal is to avoid as much as possible extra 
versions.

Also, as I said before in a response to "[PATCH v14 09/10] ARM: dts: 
mbimx51sd: Add display support.", the LCD regulator was inverted, it 
worked while inverted because of a bug which is now fixed by:
"imx-drm: parallel-display: Fix DPMS default state."

Right now, I don't have any other changes for this serie beside a simple 
rebase of "dts: imx5*, imx6*: correct display-timings rebased".

Denis.

[PATCH v14 09/10] ARM: dts: mbimx51sd: Add display support.

[Denis Carikli]

The CMO-QVGA, DVI-SVGA and DVI-VGA are added.

Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v13->v14:
- None

ChangeLog v10->v13:
- Rebased
- Removed enable-active-high in reg_lcd_3v3: its GPIO
  already has the GPIO_ACTIVE_HIGH flag.
  Without this removal, the display was off at boot
  and powering it off and on was necessary to get an
  image on it after the boot.
  
ChangeLog v10->v11:
- Now uses the drm-panel instead of the display-timings.
  This is to get regulator support, which is lacking in
  the imx-drm driver when using the display-timings.

ChangeLog v9->v10:
- Rebased
- Now enables the cmo-qvga regulator at boot.

ChangeLog v8->v9:
- Removed the Cc. They are now set in git-send-email directly.
- updated pixelclk-active after the following patch:
  "imx-drm: Match ipu_di_signal_cfg's clk_pol with its description."

ChangeLog v7->v8:
- Rebased the patch: added the now required imx-drm node.
- Adapted the svga clock-frequency value in order to still
  be able to display an image after the following commit:
  "imx-drm: ipu-v3: more inteligent DI clock selection"

ChangeLog v6->v7:
- Shrinked even more the Cc list.
- Since the pingrp headers were removed, the references
  to it where replaced by the actual pins.
- Added the targets to arch/arm/boot/dts/Makefile

ChangeLog v5->v6:
- Reordered the Cc list.

ChangeLog v3->v5:
- Updated to new GPIO defines.
- Updated to new licenses checkpatch requirements.
- one whitespace cleanup.

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.
---
 arch/arm/boot/dts/Makefile                         |    3 ++
 .../imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts  |   40 ++++++++++++++++
 .../imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dts  |   28 +++++++++++
 .../imx51-eukrea-mbimxsd51-baseboard-dvi-vga.dts   |   28 +++++++++++
 .../boot/dts/imx51-eukrea-mbimxsd51-baseboard.dts  |   49 ++++++++++++++++++++
 5 files changed, 148 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/Makefile b/arch/arm/boot/dts/Makefile
index 0f1e8be..f0ec7b7 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -177,6 +177,9 @@ dtb-$(CONFIG_ARCH_MXC) += \
 	imx51-babbage.dtb \
 	imx51-digi-connectcore-jsk.dtb \
 	imx51-eukrea-mbimxsd51-baseboard.dtb \
+	imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dtb \
+	imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dtb \
+	imx51-eukrea-mbimxsd51-baseboard-dvi-vga.dtb \
 	imx53-ard.dtb \
 	imx53-m53evk.dtb \
 	imx53-mba53.dtb \
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..d273d09
--- /dev/null
+++ b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-cmo-qvga.dts
@@ -0,0 +1,40 @@
+/*
+ * 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.
+ */
+
+#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";
+
+	panel: panel {
+		compatible = "eukrea,mbimxsd51-cmo-qvga", "simple-panel";
+		power-supply = <&reg_lcd_3v3>;
+	};
+
+	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 GPIO_ACTIVE_HIGH>;
+		regulator-boot-on;
+	};
+};
+
+&display {
+	status = "okay";
+	fsl,panel = <&panel>;
+};
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..323ebf4
--- /dev/null
+++ b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-svga.dts
@@ -0,0 +1,28 @@
+/*
+ * 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.
+ */
+
+#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";
+
+	panel: panel {
+		compatible = "eukrea,mbimxsd51-dvi-svga", "simple-panel";
+	};
+};
+
+&display {
+	status = "okay";
+	fsl,panel = <&panel>;
+};
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..f065500
--- /dev/null
+++ b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard-dvi-vga.dts
@@ -0,0 +1,28 @@
+/*
+ * 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.
+ */
+
+#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";
+
+	panel: panel {
+		compatible = "eukrea,mbimxsd51-dvi-vga", "simple-panel";
+	};
+};
+
+&display {
+	status = "okay";
+	fsl,panel = <&panel>;
+};
diff --git a/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard.dts b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard.dts
index 75e66c9..02ff24a 100644
--- a/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard.dts
+++ b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard.dts
@@ -32,6 +32,20 @@
 		};
 	};
 
+	display: display at di0 {
+		compatible = "fsl,imx-parallel-display";
+		interface-pix-fmt = "rgb666";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_ipu_disp1>;
+		status = "disabled";
+
+		port {
+			display0_in: endpoint {
+				remote-endpoint = <&ipu_di0_disp0>;
+			};
+		};
+	};
+
 	gpio_keys {
 		compatible = "gpio-keys";
 		pinctrl-names = "default";
@@ -222,6 +236,37 @@
 			>;
 		};
 
+		pinctrl_ipu_disp1: ipudisp1grp {
+			fsl,pins = <
+				MX51_PAD_DISP1_DAT0__DISP1_DAT0	  0x5
+				MX51_PAD_DISP1_DAT1__DISP1_DAT1	  0x5
+				MX51_PAD_DISP1_DAT2__DISP1_DAT2	  0x5
+				MX51_PAD_DISP1_DAT3__DISP1_DAT3	  0x5
+				MX51_PAD_DISP1_DAT4__DISP1_DAT4	  0x5
+				MX51_PAD_DISP1_DAT5__DISP1_DAT5	  0x5
+				MX51_PAD_DISP1_DAT6__DISP1_DAT6	  0x5
+				MX51_PAD_DISP1_DAT7__DISP1_DAT7	  0x5
+				MX51_PAD_DISP1_DAT8__DISP1_DAT8   0x5
+				MX51_PAD_DISP1_DAT9__DISP1_DAT9	  0x5
+				MX51_PAD_DISP1_DAT10__DISP1_DAT10 0x5
+				MX51_PAD_DISP1_DAT11__DISP1_DAT11 0x5
+				MX51_PAD_DISP1_DAT12__DISP1_DAT12 0x5
+				MX51_PAD_DISP1_DAT13__DISP1_DAT13 0x5
+				MX51_PAD_DISP1_DAT14__DISP1_DAT14 0x5
+				MX51_PAD_DISP1_DAT15__DISP1_DAT15 0x5
+				MX51_PAD_DISP1_DAT16__DISP1_DAT16 0x5
+				MX51_PAD_DISP1_DAT17__DISP1_DAT17 0x5
+				MX51_PAD_DISP1_DAT18__DISP1_DAT18 0x5
+				MX51_PAD_DISP1_DAT19__DISP1_DAT19 0x5
+				MX51_PAD_DISP1_DAT20__DISP1_DAT20 0x5
+				MX51_PAD_DISP1_DAT21__DISP1_DAT21 0x5
+				MX51_PAD_DISP1_DAT22__DISP1_DAT22 0x5
+				MX51_PAD_DISP1_DAT23__DISP1_DAT23 0x5
+				MX51_PAD_DI1_PIN2__DI1_PIN2       0x5
+				MX51_PAD_DI1_PIN3__DI1_PIN3       0x5
+			>;
+		};
+
 		pinctrl_reg_lcd_3v3: reg_lcd_3v3 {
 			fsl,pins = <
 				MX51_PAD_CSI1_D9__GPIO3_13 0x1f5
@@ -253,6 +298,10 @@
 	};
 };
 
+&ipu_di0_disp0 {
+	remote-endpoint = <&display0_in>;
+};
+
 &ssi2 {
 	codec-handle = <&tlv320aic23>;
 	fsl,mode = "i2s-slave";
-- 
1.7.9.5

[PATCH v14 09/10] ARM: dts: mbimx51sd: Add display support.

[Denis Carikli]

On 06/16/2014 12:11 PM, Denis Carikli wrote:> +	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 GPIO_ACTIVE_HIGH>;
 > +		regulator-boot-on;
 > +	};
 > +};
This is wrong, I'll fix it in the next serie.

What it really does is to make regulator-fixed think that the gpio is 
active low, the bindings documentation(fixed-regulator.txt) says:
 > - enable-active-high: Polarity of GPIO is Active high
 > If this property is missing, the default assumed is Active low.

Then regulator-boot-on will make it think that the regulator is already 
on and so the regulator will be disabled.
 From the bindings documentation (regulator.txt):
 > regulator-boot-on: bootloader/firmware enabled regulator

Which result at the lcd regulator being physically powered on at boot.
I didn't see that because powering it on at boot is what I want.

How can I do that beside doing it in userspace by issuing the following 
commands:
echo 4 > /sys/devices/display-subsystem/graphics/fb0/blank
echo 0 > /sys/devices/display-subsystem/graphics/fb0/blank

Denis.

[PATCH v14 09/10] ARM: dts: mbimx51sd: Add display support.

[Denis Carikli]

On 06/16/2014 02:29 PM, Denis Carikli wrote:
[...]
> Which result at the lcd regulator being physically powered on at boot.
> I didn't see that because powering it on at boot is what I want.
I fixed that in imx-drm's parallel-display with another patch I just 
sent separately.

Denis.

[PATCH v14 10/10] ARM: dts: mbimx51sd: Add CMO-QVGA backlight support.

[Denis Carikli]

Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v13->v14:
- None

ChangeLog v11->v13:
- No changes
ChangeLog v9->v11:
- Now uses the drm-panel instead of the display-timings.

ChangeLog v8->v9:
- Removed the Cc. They are now set in git-send-email directly.
- The backlight is now on at boot.

ChangeLog v6->v7:
- Shrinked even more the Cc list.

ChangeLog v5->v6:
- Reordered the Cc list.

ChangeLog v3->v5:
- Updated to the new GPIO defines.

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  |   10 ++++++++++
 1 file changed, 10 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 d273d09..6e36dae 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
@@ -17,9 +17,19 @@
 	model = "Eukrea MBIMXSD51 with the CMO-QVGA Display";
 	compatible = "eukrea,mbimxsd51-baseboard-cmo-qvga", "eukrea,mbimxsd51-baseboard", "eukrea,cpuimx51", "fsl,imx51";
 
+	backlight: backlight {
+		compatible = "gpio-backlight";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_backlight_1>;
+		gpios = <&gpio3 4 GPIO_ACTIVE_HIGH>;
+		default-brightness-level = <1>;
+		default-on;
+	};
+
 	panel: panel {
 		compatible = "eukrea,mbimxsd51-cmo-qvga", "simple-panel";
 		power-supply = <&reg_lcd_3v3>;
+		backlight = <&backlight>;
 	};
 
 	reg_lcd_3v3: lcd-en {
-- 
1.7.9.5

[PATCH v14 01/10] [media] v4l2: add new V4L2_PIX_FMT_RGB666 pixel format.

[Russell King - ARM Linux]

On Mon, Jun 16, 2014 at 12:11:15PM +0200, Denis Carikli wrote:
> That new macro is needed by the imx_drm staging driver
>   for supporting the QVGA display of the eukrea-cpuimx51 board.

As I said probably around v10 time, I already have this patch queued,
and I was going to send it to Greg before the previous merge window,
but due to the number of patches I was already carrying, it was lost
amongst the trees.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.