All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv3][ 1/5] fbdev: Add the lacking FB_SYNC_* for matching the DISPLAY_FLAGS_*
@ 2013-10-23 12:43 ` Denis Carikli
  0 siblings, 0 replies; 29+ messages in thread
From: Denis Carikli @ 2013-10-23 12:43 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Shawn Guo, Eric Bénard,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Denis Carikli,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Pawel Moll,
	Mark Rutland, Stephen Warren, Ian Campbell,
	devicetree-u79uwXL29TY76Z2rM5mHXA

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

Cc: Jean-Christophe Plagniol-Villard <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
Cc: Tomi Valkeinen <tomi.valkeinen-l0cyMroinI0@public.gmane.org>
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: Eric Bénard <eric-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Denis Carikli <denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
---
ChangeLog v2->v3:
- Added Jean-Christophe PLAGNIOL-VILLARD's ACK.
---
 drivers/video/fbmon.c   |    4 ++++
 include/uapi/linux/fb.h |    2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index 6103fa6..29a9ed0 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -1402,6 +1402,10 @@ int fb_videomode_from_videomode(const struct videomode *vm,
 		fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
 	if (vm->flags & DISPLAY_FLAGS_VSYNC_HIGH)
 		fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
+	if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
+		fbmode->sync |= FB_SYNC_DE_HIGH_ACT;
+	if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
+		fbmode->sync |= FB_SYNC_PIXDAT_HIGH_ACT;
 	if (vm->flags & DISPLAY_FLAGS_INTERLACED)
 		fbmode->vmode |= FB_VMODE_INTERLACED;
 	if (vm->flags & DISPLAY_FLAGS_DOUBLESCAN)
diff --git a/include/uapi/linux/fb.h b/include/uapi/linux/fb.h
index fb795c3..30487df 100644
--- a/include/uapi/linux/fb.h
+++ b/include/uapi/linux/fb.h
@@ -215,6 +215,8 @@ struct fb_bitfield {
 					/* vtotal = 144d/288n/576i => PAL  */
 					/* vtotal = 121d/242n/484i => NTSC */
 #define FB_SYNC_ON_GREEN	32	/* sync on green */
+#define FB_SYNC_DE_HIGH_ACT	64	/* data enable high active */
+#define FB_SYNC_PIXDAT_HIGH_ACT	64	/* data enable high active */
 
 #define FB_VMODE_NONINTERLACED  0	/* non interlaced */
 #define FB_VMODE_INTERLACED	1	/* interlaced	*/
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCHv3][ 1/5] fbdev: Add the lacking FB_SYNC_* for matching the DISPLAY_FLAGS_*
@ 2013-10-23 12:43 ` Denis Carikli
  0 siblings, 0 replies; 29+ messages in thread
From: Denis Carikli @ 2013-10-23 12:43 UTC (permalink / raw)
  To: linux-arm-kernel

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

Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: devicetree@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
ChangeLog v2->v3:
- Added Jean-Christophe PLAGNIOL-VILLARD's ACK.
---
 drivers/video/fbmon.c   |    4 ++++
 include/uapi/linux/fb.h |    2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index 6103fa6..29a9ed0 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -1402,6 +1402,10 @@ int fb_videomode_from_videomode(const struct videomode *vm,
 		fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
 	if (vm->flags & DISPLAY_FLAGS_VSYNC_HIGH)
 		fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
+	if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
+		fbmode->sync |= FB_SYNC_DE_HIGH_ACT;
+	if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
+		fbmode->sync |= FB_SYNC_PIXDAT_HIGH_ACT;
 	if (vm->flags & DISPLAY_FLAGS_INTERLACED)
 		fbmode->vmode |= FB_VMODE_INTERLACED;
 	if (vm->flags & DISPLAY_FLAGS_DOUBLESCAN)
diff --git a/include/uapi/linux/fb.h b/include/uapi/linux/fb.h
index fb795c3..30487df 100644
--- a/include/uapi/linux/fb.h
+++ b/include/uapi/linux/fb.h
@@ -215,6 +215,8 @@ struct fb_bitfield {
 					/* vtotal = 144d/288n/576i => PAL  */
 					/* vtotal = 121d/242n/484i => NTSC */
 #define FB_SYNC_ON_GREEN	32	/* sync on green */
+#define FB_SYNC_DE_HIGH_ACT	64	/* data enable high active */
+#define FB_SYNC_PIXDAT_HIGH_ACT	64	/* data enable high active */
 
 #define FB_VMODE_NONINTERLACED  0	/* non interlaced */
 #define FB_VMODE_INTERLACED	1	/* interlaced	*/
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCHv3][ 1/5] fbdev: Add the lacking FB_SYNC_* for matching the DISPLAY_FLAGS_*
@ 2013-10-23 12:43 ` Denis Carikli
  0 siblings, 0 replies; 29+ messages in thread
From: Denis Carikli @ 2013-10-23 12:43 UTC (permalink / raw)
  To: linux-arm-kernel

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

Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev at vger.kernel.org
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: devicetree at vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel at lists.infradead.org
Cc: Eric B?nard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
ChangeLog v2->v3:
- Added Jean-Christophe PLAGNIOL-VILLARD's ACK.
---
 drivers/video/fbmon.c   |    4 ++++
 include/uapi/linux/fb.h |    2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
index 6103fa6..29a9ed0 100644
--- a/drivers/video/fbmon.c
+++ b/drivers/video/fbmon.c
@@ -1402,6 +1402,10 @@ int fb_videomode_from_videomode(const struct videomode *vm,
 		fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
 	if (vm->flags & DISPLAY_FLAGS_VSYNC_HIGH)
 		fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
+	if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
+		fbmode->sync |= FB_SYNC_DE_HIGH_ACT;
+	if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
+		fbmode->sync |= FB_SYNC_PIXDAT_HIGH_ACT;
 	if (vm->flags & DISPLAY_FLAGS_INTERLACED)
 		fbmode->vmode |= FB_VMODE_INTERLACED;
 	if (vm->flags & DISPLAY_FLAGS_DOUBLESCAN)
diff --git a/include/uapi/linux/fb.h b/include/uapi/linux/fb.h
index fb795c3..30487df 100644
--- a/include/uapi/linux/fb.h
+++ b/include/uapi/linux/fb.h
@@ -215,6 +215,8 @@ struct fb_bitfield {
 					/* vtotal = 144d/288n/576i => PAL  */
 					/* vtotal = 121d/242n/484i => NTSC */
 #define FB_SYNC_ON_GREEN	32	/* sync on green */
+#define FB_SYNC_DE_HIGH_ACT	64	/* data enable high active */
+#define FB_SYNC_PIXDAT_HIGH_ACT	64	/* data enable high active */
 
 #define FB_VMODE_NONINTERLACED  0	/* non interlaced */
 #define FB_VMODE_INTERLACED	1	/* interlaced	*/
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCHv3][ 2/5] dma: ipu: Add devicetree support.
  2013-10-23 12:43 ` Denis Carikli
@ 2013-10-23 12:43     ` Denis Carikli
  -1 siblings, 0 replies; 29+ messages in thread
From: Denis Carikli @ 2013-10-23 12:43 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Shawn Guo, Eric Bénard,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Denis Carikli,
	Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
	Ian Campbell, devicetree-u79uwXL29TY76Z2rM5mHXA, Vinod Koul,
	Dan Williams

Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Vinod Koul <vinod.koul-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: Eric Bénard <eric-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Denis Carikli <denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
---
ChangeLog v2->v3:
- The DMA channels are not exposed anymore in order to look more like the IPUv3
  bindings.
---
 .../devicetree/bindings/dma/fsl-imx-ipu.txt        |   33 ++++++++++++++++++++
 drivers/dma/ipu/ipu_idmac.c                        |    8 +++++
 2 files changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/fsl-imx-ipu.txt

diff --git a/Documentation/devicetree/bindings/dma/fsl-imx-ipu.txt b/Documentation/devicetree/bindings/dma/fsl-imx-ipu.txt
new file mode 100644
index 0000000..ee66e1a
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/fsl-imx-ipu.txt
@@ -0,0 +1,33 @@
+* Freescale Image Processing Unit DMA support for i.MX3x.
+
+This dma driver supports the imx31 and imx35 devices.
+
+Required properties:
+- compatible : Should be "fsl,imx31-ipu".
+- reg : Should contain DMA registers location and length
+- interrupts : First item should be DMA interrupt, second one is optional and
+    should contain DMA Error interrupt.
+
+Example:
+
+	ipu: ipu@53fc0000 {
+		compatible = "fsl,imx31-ipu";
+		reg = <	0x53fc0000 0x5f
+			0x53fc0088 0x2b >;
+		interrupts = <42 41>;
+		clocks = <&clks 55>;
+		clock-names = "";
+		status = "disabled";
+	};
+
+* DMA client
+
+Clients have to specify the DMA requests with phandles in a list.
+
+Required properties:
+Example:
+
+	lcdc: mx3fb@53fc00b4 {
+		...
+		...
+	};
diff --git a/drivers/dma/ipu/ipu_idmac.c b/drivers/dma/ipu/ipu_idmac.c
index cb9c0bc..d853ee1 100644
--- a/drivers/dma/ipu/ipu_idmac.c
+++ b/drivers/dma/ipu/ipu_idmac.c
@@ -22,6 +22,7 @@
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/dma/ipu-dma.h>
 
 #include "../dmaengine.h"
@@ -1768,6 +1769,12 @@ static int ipu_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id ipu_dma_of_dev_id[] = {
+	{ .compatible = "fsl,imx31-ipu", },
+	{ /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ipu_dma_of_dev_id);
+
 /*
  * We need two MEM resources - with IPU-common and Image Converter registers,
  * including PF_CONF and IDMAC_* registers, and two IRQs - function and error
@@ -1775,6 +1782,7 @@ static int ipu_remove(struct platform_device *pdev)
 static struct platform_driver ipu_platform_driver = {
 	.driver = {
 		.name	= "ipu-core",
+		.of_match_table = of_match_ptr(ipu_dma_of_dev_id),
 		.owner	= THIS_MODULE,
 	},
 	.remove		= ipu_remove,
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCHv3][ 2/5] dma: ipu: Add devicetree support.
@ 2013-10-23 12:43     ` Denis Carikli
  0 siblings, 0 replies; 29+ messages in thread
From: Denis Carikli @ 2013-10-23 12:43 UTC (permalink / raw)
  To: linux-arm-kernel

Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: devicetree at vger.kernel.org
Cc: Vinod Koul <vinod.koul@intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel at lists.infradead.org
Cc: Eric B?nard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v2->v3:
- The DMA channels are not exposed anymore in order to look more like the IPUv3
  bindings.
---
 .../devicetree/bindings/dma/fsl-imx-ipu.txt        |   33 ++++++++++++++++++++
 drivers/dma/ipu/ipu_idmac.c                        |    8 +++++
 2 files changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/fsl-imx-ipu.txt

diff --git a/Documentation/devicetree/bindings/dma/fsl-imx-ipu.txt b/Documentation/devicetree/bindings/dma/fsl-imx-ipu.txt
new file mode 100644
index 0000000..ee66e1a
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/fsl-imx-ipu.txt
@@ -0,0 +1,33 @@
+* Freescale Image Processing Unit DMA support for i.MX3x.
+
+This dma driver supports the imx31 and imx35 devices.
+
+Required properties:
+- compatible : Should be "fsl,imx31-ipu".
+- reg : Should contain DMA registers location and length
+- interrupts : First item should be DMA interrupt, second one is optional and
+    should contain DMA Error interrupt.
+
+Example:
+
+	ipu: ipu at 53fc0000 {
+		compatible = "fsl,imx31-ipu";
+		reg = <	0x53fc0000 0x5f
+			0x53fc0088 0x2b >;
+		interrupts = <42 41>;
+		clocks = <&clks 55>;
+		clock-names = "";
+		status = "disabled";
+	};
+
+* DMA client
+
+Clients have to specify the DMA requests with phandles in a list.
+
+Required properties:
+Example:
+
+	lcdc: mx3fb at 53fc00b4 {
+		...
+		...
+	};
diff --git a/drivers/dma/ipu/ipu_idmac.c b/drivers/dma/ipu/ipu_idmac.c
index cb9c0bc..d853ee1 100644
--- a/drivers/dma/ipu/ipu_idmac.c
+++ b/drivers/dma/ipu/ipu_idmac.c
@@ -22,6 +22,7 @@
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/dma/ipu-dma.h>
 
 #include "../dmaengine.h"
@@ -1768,6 +1769,12 @@ static int ipu_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id ipu_dma_of_dev_id[] = {
+	{ .compatible = "fsl,imx31-ipu", },
+	{ /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ipu_dma_of_dev_id);
+
 /*
  * We need two MEM resources - with IPU-common and Image Converter registers,
  * including PF_CONF and IDMAC_* registers, and two IRQs - function and error
@@ -1775,6 +1782,7 @@ static int ipu_remove(struct platform_device *pdev)
 static struct platform_driver ipu_platform_driver = {
 	.driver = {
 		.name	= "ipu-core",
+		.of_match_table = of_match_ptr(ipu_dma_of_dev_id),
 		.owner	= THIS_MODULE,
 	},
 	.remove		= ipu_remove,
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCHv3][ 3/5] video: mx3fb: Add device tree suport.
  2013-10-23 12:43 ` Denis Carikli
  (?)
@ 2013-10-23 12:43     ` Denis Carikli
  -1 siblings, 0 replies; 29+ messages in thread
From: Denis Carikli @ 2013-10-23 12:43 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Shawn Guo, Eric Bénard,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Denis Carikli,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Pawel Moll,
	Mark Rutland, Stephen Warren, Ian Campbell,
	devicetree-u79uwXL29TY76Z2rM5mHXA

Cc: Jean-Christophe Plagniol-Villard <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
Cc: Tomi Valkeinen <tomi.valkeinen-l0cyMroinI0@public.gmane.org>
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: Eric Bénard <eric-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Denis Carikli <denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
---
ChangeLog v2->v3:
- The device tree bindings were reworked in order to make it look more like the
  IPUv3 bindings.
- The interface_pix_fmt property now looks like the IPUv3 one.
---
 .../devicetree/bindings/video/fsl,mx3-fb.txt       |   35 ++++++
 drivers/video/Kconfig                              |    2 +
 drivers/video/mx3fb.c                              |  125 +++++++++++++++++---
 3 files changed, 147 insertions(+), 15 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/video/fsl,mx3-fb.txt

diff --git a/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
new file mode 100644
index 0000000..0b31374
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
@@ -0,0 +1,35 @@
+Freescale MX3 fb
+================
+
+Required properties:
+- compatible: Should be "fsl,mx3fb". compatible chips include the imx31 and the
+  imx35.
+- reg: should be register base and length as documented in the datasheet.
+- clocks: Handle to the ipu_gate clock.
+
+Example:
+
+lcdc: mx3fb@53fc00b4 {
+	compatible = "fsl,mx3-fb";
+	reg = <0x53fc00b4 0x0b>;
+	clocks = <&clks 55>;
+};
+
+Display support
+===============
+Required properties:
+- model : The user-visible name of the display.
+
+Optional properties:
+- interface_pix_fmt: How this display is connected to the
+  crtc. Currently supported types: "rgb24", "rgb565", "rgb666"
+
+It can also have an optional timing subnode as described in
+  Documentation/devicetree/bindings/video/display-timing.txt.
+
+Example:
+
+display@di0 {
+	    interface-pix-fmt = "rgb666";
+	    model = "CMO-QVGA";
+};
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 14317b7..2a638df 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2359,6 +2359,8 @@ config FB_MX3
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
+	select VIDEOMODE_HELPERS
+	select FB_MODE_HELPERS
 	default y
 	help
 	  This is a framebuffer device for the i.MX31 LCD Controller. So
diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
index 804f874..de5a6c8 100644
--- a/drivers/video/mx3fb.c
+++ b/drivers/video/mx3fb.c
@@ -31,6 +31,8 @@
 #include <linux/platform_data/dma-imx.h>
 #include <linux/platform_data/video-mx3fb.h>
 
+#include <video/of_display_timing.h>
+
 #include <asm/io.h>
 #include <asm/uaccess.h>
 
@@ -757,11 +759,13 @@ static int __set_par(struct fb_info *fbi, bool lock)
 			sig_cfg.Hsync_pol = true;
 		if (fbi->var.sync & FB_SYNC_VERT_HIGH_ACT)
 			sig_cfg.Vsync_pol = true;
-		if (fbi->var.sync & FB_SYNC_CLK_INVERT)
+		if ((fbi->var.sync & FB_SYNC_CLK_INVERT) ||
+		    (fbi->var.sync & FB_SYNC_PIXDAT_HIGH_ACT))
 			sig_cfg.clk_pol = true;
 		if (fbi->var.sync & FB_SYNC_DATA_INVERT)
 			sig_cfg.data_pol = true;
-		if (fbi->var.sync & FB_SYNC_OE_ACT_HIGH)
+		if ((fbi->var.sync & FB_SYNC_OE_ACT_HIGH) ||
+		    (fbi->var.sync & FB_SYNC_DE_HIGH_ACT))
 			sig_cfg.enable_pol = true;
 		if (fbi->var.sync & FB_SYNC_CLK_IDLE_EN)
 			sig_cfg.clkidle_en = true;
@@ -1351,21 +1355,75 @@ static struct fb_info *mx3fb_init_fbinfo(struct device *dev, struct fb_ops *ops)
 	return fbi;
 }
 
+static int match_dt_disp_data(const char *property)
+{
+	if (!strcmp("rgb666", property))
+		return IPU_DISP_DATA_MAPPING_RGB666;
+	else if (!strcmp("rgb565", property))
+		return IPU_DISP_DATA_MAPPING_RGB565;
+	else if (!strcmp("rgb24", property))
+		return IPU_DISP_DATA_MAPPING_RGB888;
+	else
+		return -EINVAL;
+}
+
 static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 {
 	struct device *dev = mx3fb->dev;
 	struct mx3fb_platform_data *mx3fb_pdata = dev_get_platdata(dev);
-	const char *name = mx3fb_pdata->name;
+	struct device_node *np = dev->of_node;
+	const char *name;
+	const char *ipu_disp_format;
 	unsigned int irq;
 	struct fb_info *fbi;
 	struct mx3fb_info *mx3fbi;
 	const struct fb_videomode *mode;
 	int ret, num_modes;
+	struct fb_videomode of_mode;
+	struct device_node *display_np, *root_np;
+
+	if (np) {
+		root_np = of_find_node_by_path("/");
+		if (!root_np) {
+			dev_err(dev, "Can't get the \"/\" node.\n");
+			return -EINVAL;
+		}
+
+		display_np = of_find_node_by_name(root_np, "display");
+		if (!display_np) {
+			dev_err(dev, "Can't get the display device node.\n");
+			return -EINVAL;
+		}
+
+		of_property_read_string(display_np, "interface-pix-fmt",
+					&ipu_disp_format);
+		if (!ipu_disp_format) {
+			mx3fb->disp_data_fmt = IPU_DISP_DATA_MAPPING_RGB666;
+			dev_warn(dev,
+				"ipu display data mapping was not defined, using the default rgb666.\n");
+		} else {
+			mx3fb->disp_data_fmt =
+				match_dt_disp_data(ipu_disp_format);
+		}
+
+		if (mx3fb->disp_data_fmt == -EINVAL) {
+			dev_err(dev, "Illegal display data format \"%s\"\n",
+				ipu_disp_format);
+			return -EINVAL;
+		}
 
-	if (mx3fb_pdata->disp_data_fmt >= ARRAY_SIZE(di_mappings)) {
-		dev_err(dev, "Illegal display data format %d\n",
+		of_property_read_string(display_np, "model", &name);
+		if (ret) {
+			dev_err(dev, "Missing display model name\n");
+			return -EINVAL;
+		}
+	} else {
+		name = mx3fb_pdata->name;
+		if (mx3fb_pdata->disp_data_fmt >= ARRAY_SIZE(di_mappings)) {
+			dev_err(dev, "Illegal display data format %d\n",
 				mx3fb_pdata->disp_data_fmt);
-		return -EINVAL;
+			return -EINVAL;
+		}
 	}
 
 	ichan->client = mx3fb;
@@ -1386,12 +1444,24 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 		goto emode;
 	}
 
-	if (mx3fb_pdata->mode && mx3fb_pdata->num_modes) {
-		mode = mx3fb_pdata->mode;
-		num_modes = mx3fb_pdata->num_modes;
+	if (np) {
+		ret = of_get_fb_videomode(display_np, &of_mode,
+					  OF_USE_NATIVE_MODE);
+		if (!ret) {
+			mode = &of_mode;
+			num_modes = 1;
+		} else {
+			mode = mx3fb_modedb;
+			num_modes = ARRAY_SIZE(mx3fb_modedb);
+		}
 	} else {
-		mode = mx3fb_modedb;
-		num_modes = ARRAY_SIZE(mx3fb_modedb);
+		if (mx3fb_pdata->mode || !mx3fb_pdata->num_modes) {
+			mode = mx3fb_pdata->mode;
+			num_modes = mx3fb_pdata->num_modes;
+		} else {
+			mode = mx3fb_modedb;
+			num_modes = ARRAY_SIZE(mx3fb_modedb);
+		}
 	}
 
 	if (!fb_find_mode(&fbi->var, fbi, fb_mode, mode,
@@ -1421,7 +1491,8 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 	mx3fbi->mx3fb		= mx3fb;
 	mx3fbi->blank		= FB_BLANK_NORMAL;
 
-	mx3fb->disp_data_fmt	= mx3fb_pdata->disp_data_fmt;
+	if (!np)
+		mx3fb->disp_data_fmt = mx3fb_pdata->disp_data_fmt;
 
 	init_completion(&mx3fbi->flip_cmpl);
 	disable_irq(ichan->eof_irq);
@@ -1449,13 +1520,26 @@ emode:
 	return ret;
 }
 
+static int imx_dma_is_dt_ipu(struct dma_chan *chan)
+{
+	/* Example: 53fc0000.ipu */
+	if (chan && chan->device && chan->device->dev) {
+		char *name = dev_name(chan->device->dev);
+		name = strchr(name, '.');
+		if (name)
+			return !strcmp(name, ".ipu");
+	}
+
+	return 0;
+}
+
 static bool chan_filter(struct dma_chan *chan, void *arg)
 {
 	struct dma_chan_request *rq = arg;
 	struct device *dev;
 	struct mx3fb_platform_data *mx3fb_pdata;
 
-	if (!imx_dma_is_ipu(chan))
+	if (!imx_dma_is_ipu(chan) && !imx_dma_is_dt_ipu(chan))
 		return false;
 
 	if (!rq)
@@ -1464,8 +1548,12 @@ static bool chan_filter(struct dma_chan *chan, void *arg)
 	dev = rq->mx3fb->dev;
 	mx3fb_pdata = dev_get_platdata(dev);
 
-	return rq->id == chan->chan_id &&
-		mx3fb_pdata->dma_dev == chan->device->dev;
+	/* When using the devicetree, mx3fb_pdata is NULL */
+	if (imx_dma_is_dt_ipu(chan))
+		return rq->id == chan->chan_id;
+	else
+		return rq->id == chan->chan_id &&
+			mx3fb_pdata->dma_dev == chan->device->dev;
 }
 
 static void release_fbi(struct fb_info *fbi)
@@ -1565,9 +1653,16 @@ static int mx3fb_remove(struct platform_device *dev)
 	return 0;
 }
 
+static struct of_device_id mx3fb_of_dev_id[] = {
+	{ .compatible = "fsl,mx3-fb", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, mx3fb_of_dev_id);
+
 static struct platform_driver mx3fb_driver = {
 	.driver = {
 		.name = MX3FB_NAME,
+		.of_match_table = mx3fb_of_dev_id,
 		.owner = THIS_MODULE,
 	},
 	.probe = mx3fb_probe,
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCHv3][ 3/5] video: mx3fb: Add device tree suport.
@ 2013-10-23 12:43     ` Denis Carikli
  0 siblings, 0 replies; 29+ messages in thread
From: Denis Carikli @ 2013-10-23 12:43 UTC (permalink / raw)
  To: linux-arm-kernel

Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: devicetree@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v2->v3:
- The device tree bindings were reworked in order to make it look more like the
  IPUv3 bindings.
- The interface_pix_fmt property now looks like the IPUv3 one.
---
 .../devicetree/bindings/video/fsl,mx3-fb.txt       |   35 ++++++
 drivers/video/Kconfig                              |    2 +
 drivers/video/mx3fb.c                              |  125 +++++++++++++++++---
 3 files changed, 147 insertions(+), 15 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/video/fsl,mx3-fb.txt

diff --git a/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
new file mode 100644
index 0000000..0b31374
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
@@ -0,0 +1,35 @@
+Freescale MX3 fb
+========
+
+Required properties:
+- compatible: Should be "fsl,mx3fb". compatible chips include the imx31 and the
+  imx35.
+- reg: should be register base and length as documented in the datasheet.
+- clocks: Handle to the ipu_gate clock.
+
+Example:
+
+lcdc: mx3fb@53fc00b4 {
+	compatible = "fsl,mx3-fb";
+	reg = <0x53fc00b4 0x0b>;
+	clocks = <&clks 55>;
+};
+
+Display support
+=======+Required properties:
+- model : The user-visible name of the display.
+
+Optional properties:
+- interface_pix_fmt: How this display is connected to the
+  crtc. Currently supported types: "rgb24", "rgb565", "rgb666"
+
+It can also have an optional timing subnode as described in
+  Documentation/devicetree/bindings/video/display-timing.txt.
+
+Example:
+
+display@di0 {
+	    interface-pix-fmt = "rgb666";
+	    model = "CMO-QVGA";
+};
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 14317b7..2a638df 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2359,6 +2359,8 @@ config FB_MX3
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
+	select VIDEOMODE_HELPERS
+	select FB_MODE_HELPERS
 	default y
 	help
 	  This is a framebuffer device for the i.MX31 LCD Controller. So
diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
index 804f874..de5a6c8 100644
--- a/drivers/video/mx3fb.c
+++ b/drivers/video/mx3fb.c
@@ -31,6 +31,8 @@
 #include <linux/platform_data/dma-imx.h>
 #include <linux/platform_data/video-mx3fb.h>
 
+#include <video/of_display_timing.h>
+
 #include <asm/io.h>
 #include <asm/uaccess.h>
 
@@ -757,11 +759,13 @@ static int __set_par(struct fb_info *fbi, bool lock)
 			sig_cfg.Hsync_pol = true;
 		if (fbi->var.sync & FB_SYNC_VERT_HIGH_ACT)
 			sig_cfg.Vsync_pol = true;
-		if (fbi->var.sync & FB_SYNC_CLK_INVERT)
+		if ((fbi->var.sync & FB_SYNC_CLK_INVERT) ||
+		    (fbi->var.sync & FB_SYNC_PIXDAT_HIGH_ACT))
 			sig_cfg.clk_pol = true;
 		if (fbi->var.sync & FB_SYNC_DATA_INVERT)
 			sig_cfg.data_pol = true;
-		if (fbi->var.sync & FB_SYNC_OE_ACT_HIGH)
+		if ((fbi->var.sync & FB_SYNC_OE_ACT_HIGH) ||
+		    (fbi->var.sync & FB_SYNC_DE_HIGH_ACT))
 			sig_cfg.enable_pol = true;
 		if (fbi->var.sync & FB_SYNC_CLK_IDLE_EN)
 			sig_cfg.clkidle_en = true;
@@ -1351,21 +1355,75 @@ static struct fb_info *mx3fb_init_fbinfo(struct device *dev, struct fb_ops *ops)
 	return fbi;
 }
 
+static int match_dt_disp_data(const char *property)
+{
+	if (!strcmp("rgb666", property))
+		return IPU_DISP_DATA_MAPPING_RGB666;
+	else if (!strcmp("rgb565", property))
+		return IPU_DISP_DATA_MAPPING_RGB565;
+	else if (!strcmp("rgb24", property))
+		return IPU_DISP_DATA_MAPPING_RGB888;
+	else
+		return -EINVAL;
+}
+
 static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 {
 	struct device *dev = mx3fb->dev;
 	struct mx3fb_platform_data *mx3fb_pdata = dev_get_platdata(dev);
-	const char *name = mx3fb_pdata->name;
+	struct device_node *np = dev->of_node;
+	const char *name;
+	const char *ipu_disp_format;
 	unsigned int irq;
 	struct fb_info *fbi;
 	struct mx3fb_info *mx3fbi;
 	const struct fb_videomode *mode;
 	int ret, num_modes;
+	struct fb_videomode of_mode;
+	struct device_node *display_np, *root_np;
+
+	if (np) {
+		root_np = of_find_node_by_path("/");
+		if (!root_np) {
+			dev_err(dev, "Can't get the \"/\" node.\n");
+			return -EINVAL;
+		}
+
+		display_np = of_find_node_by_name(root_np, "display");
+		if (!display_np) {
+			dev_err(dev, "Can't get the display device node.\n");
+			return -EINVAL;
+		}
+
+		of_property_read_string(display_np, "interface-pix-fmt",
+					&ipu_disp_format);
+		if (!ipu_disp_format) {
+			mx3fb->disp_data_fmt = IPU_DISP_DATA_MAPPING_RGB666;
+			dev_warn(dev,
+				"ipu display data mapping was not defined, using the default rgb666.\n");
+		} else {
+			mx3fb->disp_data_fmt +				match_dt_disp_data(ipu_disp_format);
+		}
+
+		if (mx3fb->disp_data_fmt = -EINVAL) {
+			dev_err(dev, "Illegal display data format \"%s\"\n",
+				ipu_disp_format);
+			return -EINVAL;
+		}
 
-	if (mx3fb_pdata->disp_data_fmt >= ARRAY_SIZE(di_mappings)) {
-		dev_err(dev, "Illegal display data format %d\n",
+		of_property_read_string(display_np, "model", &name);
+		if (ret) {
+			dev_err(dev, "Missing display model name\n");
+			return -EINVAL;
+		}
+	} else {
+		name = mx3fb_pdata->name;
+		if (mx3fb_pdata->disp_data_fmt >= ARRAY_SIZE(di_mappings)) {
+			dev_err(dev, "Illegal display data format %d\n",
 				mx3fb_pdata->disp_data_fmt);
-		return -EINVAL;
+			return -EINVAL;
+		}
 	}
 
 	ichan->client = mx3fb;
@@ -1386,12 +1444,24 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 		goto emode;
 	}
 
-	if (mx3fb_pdata->mode && mx3fb_pdata->num_modes) {
-		mode = mx3fb_pdata->mode;
-		num_modes = mx3fb_pdata->num_modes;
+	if (np) {
+		ret = of_get_fb_videomode(display_np, &of_mode,
+					  OF_USE_NATIVE_MODE);
+		if (!ret) {
+			mode = &of_mode;
+			num_modes = 1;
+		} else {
+			mode = mx3fb_modedb;
+			num_modes = ARRAY_SIZE(mx3fb_modedb);
+		}
 	} else {
-		mode = mx3fb_modedb;
-		num_modes = ARRAY_SIZE(mx3fb_modedb);
+		if (mx3fb_pdata->mode || !mx3fb_pdata->num_modes) {
+			mode = mx3fb_pdata->mode;
+			num_modes = mx3fb_pdata->num_modes;
+		} else {
+			mode = mx3fb_modedb;
+			num_modes = ARRAY_SIZE(mx3fb_modedb);
+		}
 	}
 
 	if (!fb_find_mode(&fbi->var, fbi, fb_mode, mode,
@@ -1421,7 +1491,8 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 	mx3fbi->mx3fb		= mx3fb;
 	mx3fbi->blank		= FB_BLANK_NORMAL;
 
-	mx3fb->disp_data_fmt	= mx3fb_pdata->disp_data_fmt;
+	if (!np)
+		mx3fb->disp_data_fmt = mx3fb_pdata->disp_data_fmt;
 
 	init_completion(&mx3fbi->flip_cmpl);
 	disable_irq(ichan->eof_irq);
@@ -1449,13 +1520,26 @@ emode:
 	return ret;
 }
 
+static int imx_dma_is_dt_ipu(struct dma_chan *chan)
+{
+	/* Example: 53fc0000.ipu */
+	if (chan && chan->device && chan->device->dev) {
+		char *name = dev_name(chan->device->dev);
+		name = strchr(name, '.');
+		if (name)
+			return !strcmp(name, ".ipu");
+	}
+
+	return 0;
+}
+
 static bool chan_filter(struct dma_chan *chan, void *arg)
 {
 	struct dma_chan_request *rq = arg;
 	struct device *dev;
 	struct mx3fb_platform_data *mx3fb_pdata;
 
-	if (!imx_dma_is_ipu(chan))
+	if (!imx_dma_is_ipu(chan) && !imx_dma_is_dt_ipu(chan))
 		return false;
 
 	if (!rq)
@@ -1464,8 +1548,12 @@ static bool chan_filter(struct dma_chan *chan, void *arg)
 	dev = rq->mx3fb->dev;
 	mx3fb_pdata = dev_get_platdata(dev);
 
-	return rq->id = chan->chan_id &&
-		mx3fb_pdata->dma_dev = chan->device->dev;
+	/* When using the devicetree, mx3fb_pdata is NULL */
+	if (imx_dma_is_dt_ipu(chan))
+		return rq->id = chan->chan_id;
+	else
+		return rq->id = chan->chan_id &&
+			mx3fb_pdata->dma_dev = chan->device->dev;
 }
 
 static void release_fbi(struct fb_info *fbi)
@@ -1565,9 +1653,16 @@ static int mx3fb_remove(struct platform_device *dev)
 	return 0;
 }
 
+static struct of_device_id mx3fb_of_dev_id[] = {
+	{ .compatible = "fsl,mx3-fb", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, mx3fb_of_dev_id);
+
 static struct platform_driver mx3fb_driver = {
 	.driver = {
 		.name = MX3FB_NAME,
+		.of_match_table = mx3fb_of_dev_id,
 		.owner = THIS_MODULE,
 	},
 	.probe = mx3fb_probe,
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCHv3][ 3/5] video: mx3fb: Add device tree suport.
@ 2013-10-23 12:43     ` Denis Carikli
  0 siblings, 0 replies; 29+ messages in thread
From: Denis Carikli @ 2013-10-23 12:43 UTC (permalink / raw)
  To: linux-arm-kernel

Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev at vger.kernel.org
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: devicetree at vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel at lists.infradead.org
Cc: Eric B?nard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v2->v3:
- The device tree bindings were reworked in order to make it look more like the
  IPUv3 bindings.
- The interface_pix_fmt property now looks like the IPUv3 one.
---
 .../devicetree/bindings/video/fsl,mx3-fb.txt       |   35 ++++++
 drivers/video/Kconfig                              |    2 +
 drivers/video/mx3fb.c                              |  125 +++++++++++++++++---
 3 files changed, 147 insertions(+), 15 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/video/fsl,mx3-fb.txt

diff --git a/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
new file mode 100644
index 0000000..0b31374
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
@@ -0,0 +1,35 @@
+Freescale MX3 fb
+================
+
+Required properties:
+- compatible: Should be "fsl,mx3fb". compatible chips include the imx31 and the
+  imx35.
+- reg: should be register base and length as documented in the datasheet.
+- clocks: Handle to the ipu_gate clock.
+
+Example:
+
+lcdc: mx3fb at 53fc00b4 {
+	compatible = "fsl,mx3-fb";
+	reg = <0x53fc00b4 0x0b>;
+	clocks = <&clks 55>;
+};
+
+Display support
+===============
+Required properties:
+- model : The user-visible name of the display.
+
+Optional properties:
+- interface_pix_fmt: How this display is connected to the
+  crtc. Currently supported types: "rgb24", "rgb565", "rgb666"
+
+It can also have an optional timing subnode as described in
+  Documentation/devicetree/bindings/video/display-timing.txt.
+
+Example:
+
+display at di0 {
+	    interface-pix-fmt = "rgb666";
+	    model = "CMO-QVGA";
+};
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 14317b7..2a638df 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2359,6 +2359,8 @@ config FB_MX3
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
+	select VIDEOMODE_HELPERS
+	select FB_MODE_HELPERS
 	default y
 	help
 	  This is a framebuffer device for the i.MX31 LCD Controller. So
diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
index 804f874..de5a6c8 100644
--- a/drivers/video/mx3fb.c
+++ b/drivers/video/mx3fb.c
@@ -31,6 +31,8 @@
 #include <linux/platform_data/dma-imx.h>
 #include <linux/platform_data/video-mx3fb.h>
 
+#include <video/of_display_timing.h>
+
 #include <asm/io.h>
 #include <asm/uaccess.h>
 
@@ -757,11 +759,13 @@ static int __set_par(struct fb_info *fbi, bool lock)
 			sig_cfg.Hsync_pol = true;
 		if (fbi->var.sync & FB_SYNC_VERT_HIGH_ACT)
 			sig_cfg.Vsync_pol = true;
-		if (fbi->var.sync & FB_SYNC_CLK_INVERT)
+		if ((fbi->var.sync & FB_SYNC_CLK_INVERT) ||
+		    (fbi->var.sync & FB_SYNC_PIXDAT_HIGH_ACT))
 			sig_cfg.clk_pol = true;
 		if (fbi->var.sync & FB_SYNC_DATA_INVERT)
 			sig_cfg.data_pol = true;
-		if (fbi->var.sync & FB_SYNC_OE_ACT_HIGH)
+		if ((fbi->var.sync & FB_SYNC_OE_ACT_HIGH) ||
+		    (fbi->var.sync & FB_SYNC_DE_HIGH_ACT))
 			sig_cfg.enable_pol = true;
 		if (fbi->var.sync & FB_SYNC_CLK_IDLE_EN)
 			sig_cfg.clkidle_en = true;
@@ -1351,21 +1355,75 @@ static struct fb_info *mx3fb_init_fbinfo(struct device *dev, struct fb_ops *ops)
 	return fbi;
 }
 
+static int match_dt_disp_data(const char *property)
+{
+	if (!strcmp("rgb666", property))
+		return IPU_DISP_DATA_MAPPING_RGB666;
+	else if (!strcmp("rgb565", property))
+		return IPU_DISP_DATA_MAPPING_RGB565;
+	else if (!strcmp("rgb24", property))
+		return IPU_DISP_DATA_MAPPING_RGB888;
+	else
+		return -EINVAL;
+}
+
 static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 {
 	struct device *dev = mx3fb->dev;
 	struct mx3fb_platform_data *mx3fb_pdata = dev_get_platdata(dev);
-	const char *name = mx3fb_pdata->name;
+	struct device_node *np = dev->of_node;
+	const char *name;
+	const char *ipu_disp_format;
 	unsigned int irq;
 	struct fb_info *fbi;
 	struct mx3fb_info *mx3fbi;
 	const struct fb_videomode *mode;
 	int ret, num_modes;
+	struct fb_videomode of_mode;
+	struct device_node *display_np, *root_np;
+
+	if (np) {
+		root_np = of_find_node_by_path("/");
+		if (!root_np) {
+			dev_err(dev, "Can't get the \"/\" node.\n");
+			return -EINVAL;
+		}
+
+		display_np = of_find_node_by_name(root_np, "display");
+		if (!display_np) {
+			dev_err(dev, "Can't get the display device node.\n");
+			return -EINVAL;
+		}
+
+		of_property_read_string(display_np, "interface-pix-fmt",
+					&ipu_disp_format);
+		if (!ipu_disp_format) {
+			mx3fb->disp_data_fmt = IPU_DISP_DATA_MAPPING_RGB666;
+			dev_warn(dev,
+				"ipu display data mapping was not defined, using the default rgb666.\n");
+		} else {
+			mx3fb->disp_data_fmt =
+				match_dt_disp_data(ipu_disp_format);
+		}
+
+		if (mx3fb->disp_data_fmt == -EINVAL) {
+			dev_err(dev, "Illegal display data format \"%s\"\n",
+				ipu_disp_format);
+			return -EINVAL;
+		}
 
-	if (mx3fb_pdata->disp_data_fmt >= ARRAY_SIZE(di_mappings)) {
-		dev_err(dev, "Illegal display data format %d\n",
+		of_property_read_string(display_np, "model", &name);
+		if (ret) {
+			dev_err(dev, "Missing display model name\n");
+			return -EINVAL;
+		}
+	} else {
+		name = mx3fb_pdata->name;
+		if (mx3fb_pdata->disp_data_fmt >= ARRAY_SIZE(di_mappings)) {
+			dev_err(dev, "Illegal display data format %d\n",
 				mx3fb_pdata->disp_data_fmt);
-		return -EINVAL;
+			return -EINVAL;
+		}
 	}
 
 	ichan->client = mx3fb;
@@ -1386,12 +1444,24 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 		goto emode;
 	}
 
-	if (mx3fb_pdata->mode && mx3fb_pdata->num_modes) {
-		mode = mx3fb_pdata->mode;
-		num_modes = mx3fb_pdata->num_modes;
+	if (np) {
+		ret = of_get_fb_videomode(display_np, &of_mode,
+					  OF_USE_NATIVE_MODE);
+		if (!ret) {
+			mode = &of_mode;
+			num_modes = 1;
+		} else {
+			mode = mx3fb_modedb;
+			num_modes = ARRAY_SIZE(mx3fb_modedb);
+		}
 	} else {
-		mode = mx3fb_modedb;
-		num_modes = ARRAY_SIZE(mx3fb_modedb);
+		if (mx3fb_pdata->mode || !mx3fb_pdata->num_modes) {
+			mode = mx3fb_pdata->mode;
+			num_modes = mx3fb_pdata->num_modes;
+		} else {
+			mode = mx3fb_modedb;
+			num_modes = ARRAY_SIZE(mx3fb_modedb);
+		}
 	}
 
 	if (!fb_find_mode(&fbi->var, fbi, fb_mode, mode,
@@ -1421,7 +1491,8 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 	mx3fbi->mx3fb		= mx3fb;
 	mx3fbi->blank		= FB_BLANK_NORMAL;
 
-	mx3fb->disp_data_fmt	= mx3fb_pdata->disp_data_fmt;
+	if (!np)
+		mx3fb->disp_data_fmt = mx3fb_pdata->disp_data_fmt;
 
 	init_completion(&mx3fbi->flip_cmpl);
 	disable_irq(ichan->eof_irq);
@@ -1449,13 +1520,26 @@ emode:
 	return ret;
 }
 
+static int imx_dma_is_dt_ipu(struct dma_chan *chan)
+{
+	/* Example: 53fc0000.ipu */
+	if (chan && chan->device && chan->device->dev) {
+		char *name = dev_name(chan->device->dev);
+		name = strchr(name, '.');
+		if (name)
+			return !strcmp(name, ".ipu");
+	}
+
+	return 0;
+}
+
 static bool chan_filter(struct dma_chan *chan, void *arg)
 {
 	struct dma_chan_request *rq = arg;
 	struct device *dev;
 	struct mx3fb_platform_data *mx3fb_pdata;
 
-	if (!imx_dma_is_ipu(chan))
+	if (!imx_dma_is_ipu(chan) && !imx_dma_is_dt_ipu(chan))
 		return false;
 
 	if (!rq)
@@ -1464,8 +1548,12 @@ static bool chan_filter(struct dma_chan *chan, void *arg)
 	dev = rq->mx3fb->dev;
 	mx3fb_pdata = dev_get_platdata(dev);
 
-	return rq->id == chan->chan_id &&
-		mx3fb_pdata->dma_dev == chan->device->dev;
+	/* When using the devicetree, mx3fb_pdata is NULL */
+	if (imx_dma_is_dt_ipu(chan))
+		return rq->id == chan->chan_id;
+	else
+		return rq->id == chan->chan_id &&
+			mx3fb_pdata->dma_dev == chan->device->dev;
 }
 
 static void release_fbi(struct fb_info *fbi)
@@ -1565,9 +1653,16 @@ static int mx3fb_remove(struct platform_device *dev)
 	return 0;
 }
 
+static struct of_device_id mx3fb_of_dev_id[] = {
+	{ .compatible = "fsl,mx3-fb", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, mx3fb_of_dev_id);
+
 static struct platform_driver mx3fb_driver = {
 	.driver = {
 		.name = MX3FB_NAME,
+		.of_match_table = mx3fb_of_dev_id,
 		.owner = THIS_MODULE,
 	},
 	.probe = mx3fb_probe,
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCHv3][ 4/5] video: mx3fb: Introduce regulator support.
  2013-10-23 12:43 ` Denis Carikli
@ 2013-10-23 12:43   ` Denis Carikli
  -1 siblings, 0 replies; 29+ messages in thread
From: Denis Carikli @ 2013-10-23 12:43 UTC (permalink / raw)
  To: linux-arm-kernel

This commit is based on the following commit by Fabio Estevam:
  4344429 video: mxsfb: Introduce regulator support

Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v2->v3:
- The prints are now replaced with non line wrapped prints.
- The regulator retrival has been adapted to the new DT bindings which looks
  more like the IPUv3 ones.
- The regulator_is_enabled checks were kept, because regulator_disable do not
  do such check.
---
 drivers/video/mx3fb.c |   67 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 66 insertions(+), 1 deletion(-)

diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
index de5a6c8..1f2ce6d 100644
--- a/drivers/video/mx3fb.c
+++ b/drivers/video/mx3fb.c
@@ -27,6 +27,7 @@
 #include <linux/clk.h>
 #include <linux/mutex.h>
 #include <linux/dma/ipu-dma.h>
+#include <linux/regulator/consumer.h>
 
 #include <linux/platform_data/dma-imx.h>
 #include <linux/platform_data/video-mx3fb.h>
@@ -269,6 +270,7 @@ struct mx3fb_info {
 	struct dma_async_tx_descriptor	*txd;
 	dma_cookie_t			cookie;
 	struct scatterlist		sg[2];
+	struct regulator		*reg_lcd;
 
 	struct fb_var_screeninfo	cur_var; /* current var info */
 };
@@ -1005,6 +1007,7 @@ static void __blank(int blank, struct fb_info *fbi)
 	struct mx3fb_info *mx3_fbi = fbi->par;
 	struct mx3fb_data *mx3fb = mx3_fbi->mx3fb;
 	int was_blank = mx3_fbi->blank;
+	int ret;
 
 	mx3_fbi->blank = blank;
 
@@ -1023,6 +1026,15 @@ static void __blank(int blank, struct fb_info *fbi)
 	case FB_BLANK_HSYNC_SUSPEND:
 	case FB_BLANK_NORMAL:
 		sdc_set_brightness(mx3fb, 0);
+		if (mx3_fbi->reg_lcd) {
+			if (regulator_is_enabled(mx3_fbi->reg_lcd)) {
+				ret = regulator_disable(mx3_fbi->reg_lcd);
+				if (ret)
+					dev_warn(fbi->device,
+						 "lcd regulator disable failed with error: %d\n",
+						 ret);
+			}
+		}
 		memset((char *)fbi->screen_base, 0, fbi->fix.smem_len);
 		/* Give LCD time to update - enough for 50 and 60 Hz */
 		msleep(25);
@@ -1030,6 +1042,15 @@ static void __blank(int blank, struct fb_info *fbi)
 		break;
 	case FB_BLANK_UNBLANK:
 		sdc_enable_channel(mx3_fbi);
+		if (mx3_fbi->reg_lcd) {
+			if (!regulator_is_enabled(mx3_fbi->reg_lcd)) {
+				ret = regulator_enable(mx3_fbi->reg_lcd);
+				if (ret)
+					dev_warn(fbi->device,
+						 "lcd regulator enable failed with error: %d\n",
+						 ret);
+			}
+		}
 		sdc_set_brightness(mx3fb, mx3fb->backlight_level);
 		break;
 	}
@@ -1206,6 +1227,7 @@ static int mx3fb_suspend(struct platform_device *pdev, pm_message_t state)
 {
 	struct mx3fb_data *mx3fb = platform_get_drvdata(pdev);
 	struct mx3fb_info *mx3_fbi = mx3fb->fbi->par;
+	int ret;
 
 	console_lock();
 	fb_set_suspend(mx3fb->fbi, 1);
@@ -1214,7 +1236,15 @@ static int mx3fb_suspend(struct platform_device *pdev, pm_message_t state)
 	if (mx3_fbi->blank = FB_BLANK_UNBLANK) {
 		sdc_disable_channel(mx3_fbi);
 		sdc_set_brightness(mx3fb, 0);
-
+		if (mx3_fbi->reg_lcd) {
+			if (regulator_is_enabled(mx3_fbi->reg_lcd)) {
+				ret = regulator_disable(mx3_fbi->reg_lcd);
+				if (ret)
+					dev_warn(&pdev->dev,
+						 "lcd regulator disable failed with error: %d\n",
+						 ret);
+			}
+		}
 	}
 	return 0;
 }
@@ -1226,10 +1256,20 @@ static int mx3fb_resume(struct platform_device *pdev)
 {
 	struct mx3fb_data *mx3fb = platform_get_drvdata(pdev);
 	struct mx3fb_info *mx3_fbi = mx3fb->fbi->par;
+	int ret;
 
 	if (mx3_fbi->blank = FB_BLANK_UNBLANK) {
 		sdc_enable_channel(mx3_fbi);
 		sdc_set_brightness(mx3fb, mx3fb->backlight_level);
+		if (mx3_fbi->reg_lcd) {
+			if (!regulator_is_enabled(mx3_fbi->reg_lcd)) {
+				ret = regulator_enable(mx3_fbi->reg_lcd);
+				if (ret)
+					dev_warn(&pdev->dev,
+						 "lcd regulator enable failed with error: %d\n",
+						 ret);
+			}
+		}
 	}
 
 	console_lock();
@@ -1373,6 +1413,7 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 	struct mx3fb_platform_data *mx3fb_pdata = dev_get_platdata(dev);
 	struct device_node *np = dev->of_node;
 	const char *name;
+	const char *regulator_name;
 	const char *ipu_disp_format;
 	unsigned int irq;
 	struct fb_info *fbi;
@@ -1395,6 +1436,9 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 			return -EINVAL;
 		}
 
+		of_property_read_string(display_np, "regulator-name",
+					&regulator_name);
+
 		of_property_read_string(display_np, "interface-pix-fmt",
 					&ipu_disp_format);
 		if (!ipu_disp_format) {
@@ -1509,6 +1553,24 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 	if (ret < 0)
 		goto erfb;
 
+	/* In dt mode,
+	 * using devm_regulator_get would require that the proprety referencing
+	 * the regulator phandle has to be inside the mx3fb node.
+	 */
+	if (np) {
+		if (regulator_name)
+			mx3fbi->reg_lcd = regulator_get(NULL, regulator_name);
+	} else {
+		mx3fbi->reg_lcd = devm_regulator_get(dev, "lcd");
+	}
+
+	if (IS_ERR(mx3fbi->reg_lcd)) {
+		dev_warn(mx3fb->dev, "Operating without regulator \"lcd\"\n");
+		mx3fbi->reg_lcd = NULL;
+	} else {
+		dev_info(mx3fb->dev, "Using \"lcd\" Regulator\n");
+	}
+
 	return 0;
 
 erfb:
@@ -1575,6 +1637,7 @@ static int mx3fb_probe(struct platform_device *pdev)
 	dma_cap_mask_t mask;
 	struct dma_chan *chan;
 	struct dma_chan_request rq;
+	struct mx3fb_info *mx3_fbi;
 
 	/*
 	 * Display Interface (DI) and Synchronous Display Controller (SDC)
@@ -1630,6 +1693,8 @@ ersdc0:
 	dmaengine_put();
 	iounmap(mx3fb->reg_base);
 eremap:
+	mx3_fbi = mx3fb->fbi->par;
+	regulator_put(mx3_fbi->reg_lcd);
 	kfree(mx3fb);
 	dev_err(dev, "mx3fb: failed to register fb\n");
 	return ret;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCHv3][ 4/5] video: mx3fb: Introduce regulator support.
@ 2013-10-23 12:43   ` Denis Carikli
  0 siblings, 0 replies; 29+ messages in thread
From: Denis Carikli @ 2013-10-23 12:43 UTC (permalink / raw)
  To: linux-arm-kernel

This commit is based on the following commit by Fabio Estevam:
  4344429 video: mxsfb: Introduce regulator support

Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev at vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel at lists.infradead.org
Cc: Eric B?nard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v2->v3:
- The prints are now replaced with non line wrapped prints.
- The regulator retrival has been adapted to the new DT bindings which looks
  more like the IPUv3 ones.
- The regulator_is_enabled checks were kept, because regulator_disable do not
  do such check.
---
 drivers/video/mx3fb.c |   67 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 66 insertions(+), 1 deletion(-)

diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
index de5a6c8..1f2ce6d 100644
--- a/drivers/video/mx3fb.c
+++ b/drivers/video/mx3fb.c
@@ -27,6 +27,7 @@
 #include <linux/clk.h>
 #include <linux/mutex.h>
 #include <linux/dma/ipu-dma.h>
+#include <linux/regulator/consumer.h>
 
 #include <linux/platform_data/dma-imx.h>
 #include <linux/platform_data/video-mx3fb.h>
@@ -269,6 +270,7 @@ struct mx3fb_info {
 	struct dma_async_tx_descriptor	*txd;
 	dma_cookie_t			cookie;
 	struct scatterlist		sg[2];
+	struct regulator		*reg_lcd;
 
 	struct fb_var_screeninfo	cur_var; /* current var info */
 };
@@ -1005,6 +1007,7 @@ static void __blank(int blank, struct fb_info *fbi)
 	struct mx3fb_info *mx3_fbi = fbi->par;
 	struct mx3fb_data *mx3fb = mx3_fbi->mx3fb;
 	int was_blank = mx3_fbi->blank;
+	int ret;
 
 	mx3_fbi->blank = blank;
 
@@ -1023,6 +1026,15 @@ static void __blank(int blank, struct fb_info *fbi)
 	case FB_BLANK_HSYNC_SUSPEND:
 	case FB_BLANK_NORMAL:
 		sdc_set_brightness(mx3fb, 0);
+		if (mx3_fbi->reg_lcd) {
+			if (regulator_is_enabled(mx3_fbi->reg_lcd)) {
+				ret = regulator_disable(mx3_fbi->reg_lcd);
+				if (ret)
+					dev_warn(fbi->device,
+						 "lcd regulator disable failed with error: %d\n",
+						 ret);
+			}
+		}
 		memset((char *)fbi->screen_base, 0, fbi->fix.smem_len);
 		/* Give LCD time to update - enough for 50 and 60 Hz */
 		msleep(25);
@@ -1030,6 +1042,15 @@ static void __blank(int blank, struct fb_info *fbi)
 		break;
 	case FB_BLANK_UNBLANK:
 		sdc_enable_channel(mx3_fbi);
+		if (mx3_fbi->reg_lcd) {
+			if (!regulator_is_enabled(mx3_fbi->reg_lcd)) {
+				ret = regulator_enable(mx3_fbi->reg_lcd);
+				if (ret)
+					dev_warn(fbi->device,
+						 "lcd regulator enable failed with error: %d\n",
+						 ret);
+			}
+		}
 		sdc_set_brightness(mx3fb, mx3fb->backlight_level);
 		break;
 	}
@@ -1206,6 +1227,7 @@ static int mx3fb_suspend(struct platform_device *pdev, pm_message_t state)
 {
 	struct mx3fb_data *mx3fb = platform_get_drvdata(pdev);
 	struct mx3fb_info *mx3_fbi = mx3fb->fbi->par;
+	int ret;
 
 	console_lock();
 	fb_set_suspend(mx3fb->fbi, 1);
@@ -1214,7 +1236,15 @@ static int mx3fb_suspend(struct platform_device *pdev, pm_message_t state)
 	if (mx3_fbi->blank == FB_BLANK_UNBLANK) {
 		sdc_disable_channel(mx3_fbi);
 		sdc_set_brightness(mx3fb, 0);
-
+		if (mx3_fbi->reg_lcd) {
+			if (regulator_is_enabled(mx3_fbi->reg_lcd)) {
+				ret = regulator_disable(mx3_fbi->reg_lcd);
+				if (ret)
+					dev_warn(&pdev->dev,
+						 "lcd regulator disable failed with error: %d\n",
+						 ret);
+			}
+		}
 	}
 	return 0;
 }
@@ -1226,10 +1256,20 @@ static int mx3fb_resume(struct platform_device *pdev)
 {
 	struct mx3fb_data *mx3fb = platform_get_drvdata(pdev);
 	struct mx3fb_info *mx3_fbi = mx3fb->fbi->par;
+	int ret;
 
 	if (mx3_fbi->blank == FB_BLANK_UNBLANK) {
 		sdc_enable_channel(mx3_fbi);
 		sdc_set_brightness(mx3fb, mx3fb->backlight_level);
+		if (mx3_fbi->reg_lcd) {
+			if (!regulator_is_enabled(mx3_fbi->reg_lcd)) {
+				ret = regulator_enable(mx3_fbi->reg_lcd);
+				if (ret)
+					dev_warn(&pdev->dev,
+						 "lcd regulator enable failed with error: %d\n",
+						 ret);
+			}
+		}
 	}
 
 	console_lock();
@@ -1373,6 +1413,7 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 	struct mx3fb_platform_data *mx3fb_pdata = dev_get_platdata(dev);
 	struct device_node *np = dev->of_node;
 	const char *name;
+	const char *regulator_name;
 	const char *ipu_disp_format;
 	unsigned int irq;
 	struct fb_info *fbi;
@@ -1395,6 +1436,9 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 			return -EINVAL;
 		}
 
+		of_property_read_string(display_np, "regulator-name",
+					&regulator_name);
+
 		of_property_read_string(display_np, "interface-pix-fmt",
 					&ipu_disp_format);
 		if (!ipu_disp_format) {
@@ -1509,6 +1553,24 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 	if (ret < 0)
 		goto erfb;
 
+	/* In dt mode,
+	 * using devm_regulator_get would require that the proprety referencing
+	 * the regulator phandle has to be inside the mx3fb node.
+	 */
+	if (np) {
+		if (regulator_name)
+			mx3fbi->reg_lcd = regulator_get(NULL, regulator_name);
+	} else {
+		mx3fbi->reg_lcd = devm_regulator_get(dev, "lcd");
+	}
+
+	if (IS_ERR(mx3fbi->reg_lcd)) {
+		dev_warn(mx3fb->dev, "Operating without regulator \"lcd\"\n");
+		mx3fbi->reg_lcd = NULL;
+	} else {
+		dev_info(mx3fb->dev, "Using \"lcd\" Regulator\n");
+	}
+
 	return 0;
 
 erfb:
@@ -1575,6 +1637,7 @@ static int mx3fb_probe(struct platform_device *pdev)
 	dma_cap_mask_t mask;
 	struct dma_chan *chan;
 	struct dma_chan_request rq;
+	struct mx3fb_info *mx3_fbi;
 
 	/*
 	 * Display Interface (DI) and Synchronous Display Controller (SDC)
@@ -1630,6 +1693,8 @@ ersdc0:
 	dmaengine_put();
 	iounmap(mx3fb->reg_base);
 eremap:
+	mx3_fbi = mx3fb->fbi->par;
+	regulator_put(mx3_fbi->reg_lcd);
 	kfree(mx3fb);
 	dev_err(dev, "mx3fb: failed to register fb\n");
 	return ret;
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCHv3][ 5/5] ARM: dts: mbimxsd35 Add video and displays support.
  2013-10-23 12:43 ` Denis Carikli
  (?)
@ 2013-10-23 12:43     ` Denis Carikli
  -1 siblings, 0 replies; 29+ messages in thread
From: Denis Carikli @ 2013-10-23 12:43 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Shawn Guo, Eric Bénard,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Denis Carikli,
	Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
	Ian Campbell, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA

Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: Jean-Christophe Plagniol-Villard <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
Cc: Tomi Valkeinen <tomi.valkeinen-l0cyMroinI0@public.gmane.org>
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Eric Bénard <eric-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>

Signed-off-by: Denis Carikli <denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
---
ChangeLog v2->v3:
- The dts were adapted to the new DT bindings which looks more like the IPUv3
  ones.
---
 .../imx35-eukrea-mbimxsd35-baseboard-cmo-qvga.dts  |   61 ++++++++++++++++++++
 .../imx35-eukrea-mbimxsd35-baseboard-dvi-svga.dts  |   51 ++++++++++++++++
 .../imx35-eukrea-mbimxsd35-baseboard-dvi-vga.dts   |   51 ++++++++++++++++
 3 files changed, 163 insertions(+)
 create mode 100644 arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-cmo-qvga.dts
 create mode 100644 arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-svga.dts
 create mode 100644 arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-vga.dts

diff --git a/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-cmo-qvga.dts b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-cmo-qvga.dts
new file mode 100644
index 0000000..6d186ca
--- /dev/null
+++ b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-cmo-qvga.dts
@@ -0,0 +1,61 @@
+/*
+ * Copyright 2013 Eukréa Electromatique <denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301, USA.
+ */
+
+#include "imx35-eukrea-mbimxsd35-baseboard.dts"
+
+/ {
+	model = "Eukrea MBIMXSD35 with the CMO-QVGA Display";
+	compatible = "eukrea,mbimxsd35-baseboard-cmo-qvga", "eukrea,mbimxsd35-baseboard", "eukrea,cpuimx35", "fsl,imx35";
+
+	display@di0 {
+		regulator-name = "lcd";
+		interface-pix-fmt = "rgb666";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_lcdc_1>;
+		model = "CMO-QVGA";
+		display-timings {
+			qvga_timings: 320x240 {
+				clock-frequency = <6500000>;
+				hactive = <320>;
+				vactive = <240>;
+				hback-porch = <68>;
+				hfront-porch = <20>;
+				vback-porch = <15>;
+				vfront-porch = <4>;
+				hsync-len = <30>;
+				vsync-len = <3>;
+			};
+		};
+	};
+
+	reg_lcd_3v3: lcd-en {
+		compatible = "regulator-fixed";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_reg_lcd_3v3>;
+		regulator-name = "lcd";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpio = <&gpio1 4 0>;
+		enable-active-high;
+	};
+};
+
+&lcdc {
+	lcd-supply = <&reg_lcd_3v3>;
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-svga.dts b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-svga.dts
new file mode 100644
index 0000000..ccf5f48
--- /dev/null
+++ b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-svga.dts
@@ -0,0 +1,51 @@
+/*
+ * Copyright 2013 Eukréa Electromatique <denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301, USA.
+ */
+
+#include "imx35-eukrea-mbimxsd35-baseboard.dts"
+
+/ {
+	model = "Eukrea MBIMXSD35 with the DVI-SVGA Display";
+	compatible = "eukrea,mbimxsd35-baseboard-dvi-svga", "eukrea,mbimxsd35-baseboard", "eukrea,cpuimx35", "fsl,imx35";
+	display@di0 {
+		interface-pix-fmt = "rgb666";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_lcdc_1>;
+		model = "DVI-SVGA";
+		display-timings {
+			svga_timings: 800x600 {
+				clock-frequency = <40000000>;
+				hactive = <800>;
+				vactive = <600>;
+				hback-porch = <75>;
+				hfront-porch = <75>;
+				vback-porch = <7>;
+				vfront-porch = <75>;
+				hsync-len = <7>;
+				vsync-len = <7>;
+				hsync-active = <1>;
+				vsync-active = <1>;
+				de-active = <1>;
+				pixelclk-active = <0>;
+			};
+		};
+	};
+};
+
+&lcdc {
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-vga.dts b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-vga.dts
new file mode 100644
index 0000000..6e31684
--- /dev/null
+++ b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-vga.dts
@@ -0,0 +1,51 @@
+/*
+ * Copyright 2013 Eukréa Electromatique <denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301, USA.
+ */
+
+#include "imx35-eukrea-mbimxsd35-baseboard.dts"
+
+/ {
+	model = "Eukrea MBIMXSD35 with the DVI-VGA Display";
+	compatible = "eukrea,mbimxsd35-baseboard-dvi-vga", "eukrea,mbimxsd35-baseboard", "eukrea,cpuimx35", "fsl,imx35";
+	display@di0 {
+		interface-pix-fmt = "rgb666";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_lcdc_1>;
+		model = "DVI-VGA";
+		display-timings {
+			vga_timings: 640x480 {
+				clock-frequency = <31250000>;
+				hactive = <640>;
+				vactive = <480>;
+				hback-porch = <100>;
+				hfront-porch = <100>;
+				vback-porch = <7>;
+				vfront-porch = <100>;
+				hsync-len = <7>;
+				vsync-len = <7>;
+				hsync-active = <1>;
+				vsync-active = <1>;
+				de-active = <1>;
+				pixelclk-active = <0>;
+			};
+		};
+	};
+};
+
+&lcdc {
+	status = "okay";
+};
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCHv3][ 5/5] ARM: dts: mbimxsd35 Add video and displays support.
@ 2013-10-23 12:43     ` Denis Carikli
  0 siblings, 0 replies; 29+ messages in thread
From: Denis Carikli @ 2013-10-23 12:43 UTC (permalink / raw)
  To: linux-arm-kernel

Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: devicetree@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: Eric Bénard <eric@eukrea.com>

Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v2->v3:
- The dts were adapted to the new DT bindings which looks more like the IPUv3
  ones.
---
 .../imx35-eukrea-mbimxsd35-baseboard-cmo-qvga.dts  |   61 ++++++++++++++++++++
 .../imx35-eukrea-mbimxsd35-baseboard-dvi-svga.dts  |   51 ++++++++++++++++
 .../imx35-eukrea-mbimxsd35-baseboard-dvi-vga.dts   |   51 ++++++++++++++++
 3 files changed, 163 insertions(+)
 create mode 100644 arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-cmo-qvga.dts
 create mode 100644 arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-svga.dts
 create mode 100644 arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-vga.dts

diff --git a/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-cmo-qvga.dts b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-cmo-qvga.dts
new file mode 100644
index 0000000..6d186ca
--- /dev/null
+++ b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-cmo-qvga.dts
@@ -0,0 +1,61 @@
+/*
+ * Copyright 2013 Eukréa Electromatique <denis@eukrea.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301, USA.
+ */
+
+#include "imx35-eukrea-mbimxsd35-baseboard.dts"
+
+/ {
+	model = "Eukrea MBIMXSD35 with the CMO-QVGA Display";
+	compatible = "eukrea,mbimxsd35-baseboard-cmo-qvga", "eukrea,mbimxsd35-baseboard", "eukrea,cpuimx35", "fsl,imx35";
+
+	display@di0 {
+		regulator-name = "lcd";
+		interface-pix-fmt = "rgb666";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_lcdc_1>;
+		model = "CMO-QVGA";
+		display-timings {
+			qvga_timings: 320x240 {
+				clock-frequency = <6500000>;
+				hactive = <320>;
+				vactive = <240>;
+				hback-porch = <68>;
+				hfront-porch = <20>;
+				vback-porch = <15>;
+				vfront-porch = <4>;
+				hsync-len = <30>;
+				vsync-len = <3>;
+			};
+		};
+	};
+
+	reg_lcd_3v3: lcd-en {
+		compatible = "regulator-fixed";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_reg_lcd_3v3>;
+		regulator-name = "lcd";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpio = <&gpio1 4 0>;
+		enable-active-high;
+	};
+};
+
+&lcdc {
+	lcd-supply = <&reg_lcd_3v3>;
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-svga.dts b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-svga.dts
new file mode 100644
index 0000000..ccf5f48
--- /dev/null
+++ b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-svga.dts
@@ -0,0 +1,51 @@
+/*
+ * Copyright 2013 Eukréa Electromatique <denis@eukrea.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301, USA.
+ */
+
+#include "imx35-eukrea-mbimxsd35-baseboard.dts"
+
+/ {
+	model = "Eukrea MBIMXSD35 with the DVI-SVGA Display";
+	compatible = "eukrea,mbimxsd35-baseboard-dvi-svga", "eukrea,mbimxsd35-baseboard", "eukrea,cpuimx35", "fsl,imx35";
+	display@di0 {
+		interface-pix-fmt = "rgb666";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_lcdc_1>;
+		model = "DVI-SVGA";
+		display-timings {
+			svga_timings: 800x600 {
+				clock-frequency = <40000000>;
+				hactive = <800>;
+				vactive = <600>;
+				hback-porch = <75>;
+				hfront-porch = <75>;
+				vback-porch = <7>;
+				vfront-porch = <75>;
+				hsync-len = <7>;
+				vsync-len = <7>;
+				hsync-active = <1>;
+				vsync-active = <1>;
+				de-active = <1>;
+				pixelclk-active = <0>;
+			};
+		};
+	};
+};
+
+&lcdc {
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-vga.dts b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-vga.dts
new file mode 100644
index 0000000..6e31684
--- /dev/null
+++ b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-vga.dts
@@ -0,0 +1,51 @@
+/*
+ * Copyright 2013 Eukréa Electromatique <denis@eukrea.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301, USA.
+ */
+
+#include "imx35-eukrea-mbimxsd35-baseboard.dts"
+
+/ {
+	model = "Eukrea MBIMXSD35 with the DVI-VGA Display";
+	compatible = "eukrea,mbimxsd35-baseboard-dvi-vga", "eukrea,mbimxsd35-baseboard", "eukrea,cpuimx35", "fsl,imx35";
+	display@di0 {
+		interface-pix-fmt = "rgb666";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_lcdc_1>;
+		model = "DVI-VGA";
+		display-timings {
+			vga_timings: 640x480 {
+				clock-frequency = <31250000>;
+				hactive = <640>;
+				vactive = <480>;
+				hback-porch = <100>;
+				hfront-porch = <100>;
+				vback-porch = <7>;
+				vfront-porch = <100>;
+				hsync-len = <7>;
+				vsync-len = <7>;
+				hsync-active = <1>;
+				vsync-active = <1>;
+				de-active = <1>;
+				pixelclk-active = <0>;
+			};
+		};
+	};
+};
+
+&lcdc {
+	status = "okay";
+};
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCHv3][ 5/5] ARM: dts: mbimxsd35 Add video and displays support.
@ 2013-10-23 12:43     ` Denis Carikli
  0 siblings, 0 replies; 29+ messages in thread
From: Denis Carikli @ 2013-10-23 12:43 UTC (permalink / raw)
  To: linux-arm-kernel

Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: devicetree at vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev at vger.kernel.org
Cc: Eric B?nard <eric@eukrea.com>

Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v2->v3:
- The dts were adapted to the new DT bindings which looks more like the IPUv3
  ones.
---
 .../imx35-eukrea-mbimxsd35-baseboard-cmo-qvga.dts  |   61 ++++++++++++++++++++
 .../imx35-eukrea-mbimxsd35-baseboard-dvi-svga.dts  |   51 ++++++++++++++++
 .../imx35-eukrea-mbimxsd35-baseboard-dvi-vga.dts   |   51 ++++++++++++++++
 3 files changed, 163 insertions(+)
 create mode 100644 arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-cmo-qvga.dts
 create mode 100644 arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-svga.dts
 create mode 100644 arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-vga.dts

diff --git a/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-cmo-qvga.dts b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-cmo-qvga.dts
new file mode 100644
index 0000000..6d186ca
--- /dev/null
+++ b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-cmo-qvga.dts
@@ -0,0 +1,61 @@
+/*
+ * Copyright 2013 Eukr?a Electromatique <denis@eukrea.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301, USA.
+ */
+
+#include "imx35-eukrea-mbimxsd35-baseboard.dts"
+
+/ {
+	model = "Eukrea MBIMXSD35 with the CMO-QVGA Display";
+	compatible = "eukrea,mbimxsd35-baseboard-cmo-qvga", "eukrea,mbimxsd35-baseboard", "eukrea,cpuimx35", "fsl,imx35";
+
+	display at di0 {
+		regulator-name = "lcd";
+		interface-pix-fmt = "rgb666";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_lcdc_1>;
+		model = "CMO-QVGA";
+		display-timings {
+			qvga_timings: 320x240 {
+				clock-frequency = <6500000>;
+				hactive = <320>;
+				vactive = <240>;
+				hback-porch = <68>;
+				hfront-porch = <20>;
+				vback-porch = <15>;
+				vfront-porch = <4>;
+				hsync-len = <30>;
+				vsync-len = <3>;
+			};
+		};
+	};
+
+	reg_lcd_3v3: lcd-en {
+		compatible = "regulator-fixed";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_reg_lcd_3v3>;
+		regulator-name = "lcd";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		gpio = <&gpio1 4 0>;
+		enable-active-high;
+	};
+};
+
+&lcdc {
+	lcd-supply = <&reg_lcd_3v3>;
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-svga.dts b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-svga.dts
new file mode 100644
index 0000000..ccf5f48
--- /dev/null
+++ b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-svga.dts
@@ -0,0 +1,51 @@
+/*
+ * Copyright 2013 Eukr?a Electromatique <denis@eukrea.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301, USA.
+ */
+
+#include "imx35-eukrea-mbimxsd35-baseboard.dts"
+
+/ {
+	model = "Eukrea MBIMXSD35 with the DVI-SVGA Display";
+	compatible = "eukrea,mbimxsd35-baseboard-dvi-svga", "eukrea,mbimxsd35-baseboard", "eukrea,cpuimx35", "fsl,imx35";
+	display at di0 {
+		interface-pix-fmt = "rgb666";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_lcdc_1>;
+		model = "DVI-SVGA";
+		display-timings {
+			svga_timings: 800x600 {
+				clock-frequency = <40000000>;
+				hactive = <800>;
+				vactive = <600>;
+				hback-porch = <75>;
+				hfront-porch = <75>;
+				vback-porch = <7>;
+				vfront-porch = <75>;
+				hsync-len = <7>;
+				vsync-len = <7>;
+				hsync-active = <1>;
+				vsync-active = <1>;
+				de-active = <1>;
+				pixelclk-active = <0>;
+			};
+		};
+	};
+};
+
+&lcdc {
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-vga.dts b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-vga.dts
new file mode 100644
index 0000000..6e31684
--- /dev/null
+++ b/arch/arm/boot/dts/imx35-eukrea-mbimxsd35-baseboard-dvi-vga.dts
@@ -0,0 +1,51 @@
+/*
+ * Copyright 2013 Eukr?a Electromatique <denis@eukrea.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301, USA.
+ */
+
+#include "imx35-eukrea-mbimxsd35-baseboard.dts"
+
+/ {
+	model = "Eukrea MBIMXSD35 with the DVI-VGA Display";
+	compatible = "eukrea,mbimxsd35-baseboard-dvi-vga", "eukrea,mbimxsd35-baseboard", "eukrea,cpuimx35", "fsl,imx35";
+	display at di0 {
+		interface-pix-fmt = "rgb666";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_lcdc_1>;
+		model = "DVI-VGA";
+		display-timings {
+			vga_timings: 640x480 {
+				clock-frequency = <31250000>;
+				hactive = <640>;
+				vactive = <480>;
+				hback-porch = <100>;
+				hfront-porch = <100>;
+				vback-porch = <7>;
+				vfront-porch = <100>;
+				hsync-len = <7>;
+				vsync-len = <7>;
+				hsync-active = <1>;
+				vsync-active = <1>;
+				de-active = <1>;
+				pixelclk-active = <0>;
+			};
+		};
+	};
+};
+
+&lcdc {
+	status = "okay";
+};
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCHv3][ 3/5] video: mx3fb: Add device tree suport.
  2013-10-23 12:43     ` Denis Carikli
@ 2013-10-25 19:50       ` Grant Likely
  -1 siblings, 0 replies; 29+ messages in thread
From: Grant Likely @ 2013-10-25 19:50 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Mark Rutland, devicetree, linux-fbdev, Pawel Moll, Ian Campbell,
	Stephen Warren, Rob Herring, Denis Carikli, Tomi Valkeinen,
	Shawn Guo, Jean-Christophe Plagniol-Villard, linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 10354 bytes --]

On Wed, 23 Oct 2013 14:43:47 +0200, Denis Carikli <denis@eukrea.com> wrote:
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: devicetree@vger.kernel.org
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: Eric Bénard <eric@eukrea.com>
> Signed-off-by: Denis Carikli <denis@eukrea.com>
> ---
> ChangeLog v2->v3:
> - The device tree bindings were reworked in order to make it look more like the
>   IPUv3 bindings.
> - The interface_pix_fmt property now looks like the IPUv3 one.
> ---
>  .../devicetree/bindings/video/fsl,mx3-fb.txt       |   35 ++++++
>  drivers/video/Kconfig                              |    2 +
>  drivers/video/mx3fb.c                              |  125 +++++++++++++++++---
>  3 files changed, 147 insertions(+), 15 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> 
> diff --git a/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> new file mode 100644
> index 0000000..0b31374
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> @@ -0,0 +1,35 @@
> +Freescale MX3 fb
> +================
> +
> +Required properties:
> +- compatible: Should be "fsl,mx3fb". compatible chips include the imx31 and the
> +  imx35.
> +- reg: should be register base and length as documented in the datasheet.
> +- clocks: Handle to the ipu_gate clock.
> +
> +Example:
> +
> +lcdc: mx3fb@53fc00b4 {
> +	compatible = "fsl,mx3-fb";
> +	reg = <0x53fc00b4 0x0b>;
> +	clocks = <&clks 55>;
> +};

This (and some of the other bindings) are trivial, and they are all
associated with a single SoC. I think it would be better to collect all
the mx3 bindings into a single file rather than distributing them all
over the bindings tree.

I started thinking about this after some of the DT conversations in
Edinburgh this week. Unless there is a high likelyhood of components
being used separately, I think it is far more useful to collect all the
bindings for an SoC into a single file. It will certainly reduce a lot
of the boilerplate that we've been collecting in bindings documentation
files.

A long time ago I took that approach for the mpc5200 documentation[1].
Take a look at that organization and let me know what you think.

[1] Documentation/devicetree/bindings/powerpc/fsl/mpc5200.txt

g.

> +
> +Display support
> +===============
> +Required properties:
> +- model : The user-visible name of the display.
> +
> +Optional properties:
> +- interface_pix_fmt: How this display is connected to the
> +  crtc. Currently supported types: "rgb24", "rgb565", "rgb666"
> +
> +It can also have an optional timing subnode as described in
> +  Documentation/devicetree/bindings/video/display-timing.txt.
> +
> +Example:
> +
> +display@di0 {
> +	    interface-pix-fmt = "rgb666";
> +	    model = "CMO-QVGA";
> +};
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 14317b7..2a638df 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -2359,6 +2359,8 @@ config FB_MX3
>  	select FB_CFB_FILLRECT
>  	select FB_CFB_COPYAREA
>  	select FB_CFB_IMAGEBLIT
> +	select VIDEOMODE_HELPERS
> +	select FB_MODE_HELPERS
>  	default y
>  	help
>  	  This is a framebuffer device for the i.MX31 LCD Controller. So
> diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
> index 804f874..de5a6c8 100644
> --- a/drivers/video/mx3fb.c
> +++ b/drivers/video/mx3fb.c
> @@ -31,6 +31,8 @@
>  #include <linux/platform_data/dma-imx.h>
>  #include <linux/platform_data/video-mx3fb.h>
>  
> +#include <video/of_display_timing.h>
> +
>  #include <asm/io.h>
>  #include <asm/uaccess.h>
>  
> @@ -757,11 +759,13 @@ static int __set_par(struct fb_info *fbi, bool lock)
>  			sig_cfg.Hsync_pol = true;
>  		if (fbi->var.sync & FB_SYNC_VERT_HIGH_ACT)
>  			sig_cfg.Vsync_pol = true;
> -		if (fbi->var.sync & FB_SYNC_CLK_INVERT)
> +		if ((fbi->var.sync & FB_SYNC_CLK_INVERT) ||
> +		    (fbi->var.sync & FB_SYNC_PIXDAT_HIGH_ACT))
>  			sig_cfg.clk_pol = true;
>  		if (fbi->var.sync & FB_SYNC_DATA_INVERT)
>  			sig_cfg.data_pol = true;
> -		if (fbi->var.sync & FB_SYNC_OE_ACT_HIGH)
> +		if ((fbi->var.sync & FB_SYNC_OE_ACT_HIGH) ||
> +		    (fbi->var.sync & FB_SYNC_DE_HIGH_ACT))
>  			sig_cfg.enable_pol = true;
>  		if (fbi->var.sync & FB_SYNC_CLK_IDLE_EN)
>  			sig_cfg.clkidle_en = true;
> @@ -1351,21 +1355,75 @@ static struct fb_info *mx3fb_init_fbinfo(struct device *dev, struct fb_ops *ops)
>  	return fbi;
>  }
>  
> +static int match_dt_disp_data(const char *property)
> +{
> +	if (!strcmp("rgb666", property))
> +		return IPU_DISP_DATA_MAPPING_RGB666;
> +	else if (!strcmp("rgb565", property))
> +		return IPU_DISP_DATA_MAPPING_RGB565;
> +	else if (!strcmp("rgb24", property))
> +		return IPU_DISP_DATA_MAPPING_RGB888;
> +	else
> +		return -EINVAL;
> +}
> +
>  static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
>  {
>  	struct device *dev = mx3fb->dev;
>  	struct mx3fb_platform_data *mx3fb_pdata = dev_get_platdata(dev);
> -	const char *name = mx3fb_pdata->name;
> +	struct device_node *np = dev->of_node;
> +	const char *name;
> +	const char *ipu_disp_format;
>  	unsigned int irq;
>  	struct fb_info *fbi;
>  	struct mx3fb_info *mx3fbi;
>  	const struct fb_videomode *mode;
>  	int ret, num_modes;
> +	struct fb_videomode of_mode;
> +	struct device_node *display_np, *root_np;
> +
> +	if (np) {
> +		root_np = of_find_node_by_path("/");
> +		if (!root_np) {
> +			dev_err(dev, "Can't get the \"/\" node.\n");
> +			return -EINVAL;
> +		}
> +
> +		display_np = of_find_node_by_name(root_np, "display");
> +		if (!display_np) {
> +			dev_err(dev, "Can't get the display device node.\n");
> +			return -EINVAL;
> +		}

Finding a node by name is bad practice. Is is possible to find the node
via compatible?

> +
> +		of_property_read_string(display_np, "interface-pix-fmt",
> +					&ipu_disp_format);
> +		if (!ipu_disp_format) {
> +			mx3fb->disp_data_fmt = IPU_DISP_DATA_MAPPING_RGB666;
> +			dev_warn(dev,
> +				"ipu display data mapping was not defined, using the default rgb666.\n");
> +		} else {
> +			mx3fb->disp_data_fmt =
> +				match_dt_disp_data(ipu_disp_format);
> +		}
> +
> +		if (mx3fb->disp_data_fmt == -EINVAL) {
> +			dev_err(dev, "Illegal display data format \"%s\"\n",
> +				ipu_disp_format);
> +			return -EINVAL;
> +		}
>  
> -	if (mx3fb_pdata->disp_data_fmt >= ARRAY_SIZE(di_mappings)) {
> -		dev_err(dev, "Illegal display data format %d\n",
> +		of_property_read_string(display_np, "model", &name);
> +		if (ret) {
> +			dev_err(dev, "Missing display model name\n");
> +			return -EINVAL;
> +		}
> +	} else {
> +		name = mx3fb_pdata->name;
> +		if (mx3fb_pdata->disp_data_fmt >= ARRAY_SIZE(di_mappings)) {
> +			dev_err(dev, "Illegal display data format %d\n",
>  				mx3fb_pdata->disp_data_fmt);
> -		return -EINVAL;
> +			return -EINVAL;
> +		}
>  	}
>  
>  	ichan->client = mx3fb;
> @@ -1386,12 +1444,24 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
>  		goto emode;
>  	}
>  
> -	if (mx3fb_pdata->mode && mx3fb_pdata->num_modes) {
> -		mode = mx3fb_pdata->mode;
> -		num_modes = mx3fb_pdata->num_modes;
> +	if (np) {
> +		ret = of_get_fb_videomode(display_np, &of_mode,
> +					  OF_USE_NATIVE_MODE);
> +		if (!ret) {
> +			mode = &of_mode;
> +			num_modes = 1;
> +		} else {
> +			mode = mx3fb_modedb;
> +			num_modes = ARRAY_SIZE(mx3fb_modedb);
> +		}
>  	} else {
> -		mode = mx3fb_modedb;
> -		num_modes = ARRAY_SIZE(mx3fb_modedb);
> +		if (mx3fb_pdata->mode || !mx3fb_pdata->num_modes) {
> +			mode = mx3fb_pdata->mode;
> +			num_modes = mx3fb_pdata->num_modes;
> +		} else {
> +			mode = mx3fb_modedb;
> +			num_modes = ARRAY_SIZE(mx3fb_modedb);
> +		}
>  	}
>  
>  	if (!fb_find_mode(&fbi->var, fbi, fb_mode, mode,
> @@ -1421,7 +1491,8 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
>  	mx3fbi->mx3fb		= mx3fb;
>  	mx3fbi->blank		= FB_BLANK_NORMAL;
>  
> -	mx3fb->disp_data_fmt	= mx3fb_pdata->disp_data_fmt;
> +	if (!np)
> +		mx3fb->disp_data_fmt = mx3fb_pdata->disp_data_fmt;
>  
>  	init_completion(&mx3fbi->flip_cmpl);
>  	disable_irq(ichan->eof_irq);
> @@ -1449,13 +1520,26 @@ emode:
>  	return ret;
>  }
>  
> +static int imx_dma_is_dt_ipu(struct dma_chan *chan)
> +{
> +	/* Example: 53fc0000.ipu */
> +	if (chan && chan->device && chan->device->dev) {
> +		char *name = dev_name(chan->device->dev);
> +		name = strchr(name, '.');
> +		if (name)
> +			return !strcmp(name, ".ipu");
> +	}
> +
> +	return 0;
> +}
> +
>  static bool chan_filter(struct dma_chan *chan, void *arg)
>  {
>  	struct dma_chan_request *rq = arg;
>  	struct device *dev;
>  	struct mx3fb_platform_data *mx3fb_pdata;
>  
> -	if (!imx_dma_is_ipu(chan))
> +	if (!imx_dma_is_ipu(chan) && !imx_dma_is_dt_ipu(chan))
>  		return false;
>  
>  	if (!rq)
> @@ -1464,8 +1548,12 @@ static bool chan_filter(struct dma_chan *chan, void *arg)
>  	dev = rq->mx3fb->dev;
>  	mx3fb_pdata = dev_get_platdata(dev);
>  
> -	return rq->id == chan->chan_id &&
> -		mx3fb_pdata->dma_dev == chan->device->dev;
> +	/* When using the devicetree, mx3fb_pdata is NULL */
> +	if (imx_dma_is_dt_ipu(chan))
> +		return rq->id == chan->chan_id;
> +	else
> +		return rq->id == chan->chan_id &&
> +			mx3fb_pdata->dma_dev == chan->device->dev;
>  }
>  
>  static void release_fbi(struct fb_info *fbi)
> @@ -1565,9 +1653,16 @@ static int mx3fb_remove(struct platform_device *dev)
>  	return 0;
>  }
>  
> +static struct of_device_id mx3fb_of_dev_id[] = {
> +	{ .compatible = "fsl,mx3-fb", },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, mx3fb_of_dev_id);
> +
>  static struct platform_driver mx3fb_driver = {
>  	.driver = {
>  		.name = MX3FB_NAME,
> +		.of_match_table = mx3fb_of_dev_id,
>  		.owner = THIS_MODULE,
>  	},
>  	.probe = mx3fb_probe,
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCHv3][ 3/5] video: mx3fb: Add device tree suport.
@ 2013-10-25 19:50       ` Grant Likely
  0 siblings, 0 replies; 29+ messages in thread
From: Grant Likely @ 2013-10-25 19:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, 23 Oct 2013 14:43:47 +0200, Denis Carikli <denis@eukrea.com> wrote:
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev at vger.kernel.org
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: devicetree at vger.kernel.org
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: Eric B??nard <eric@eukrea.com>
> Signed-off-by: Denis Carikli <denis@eukrea.com>
> ---
> ChangeLog v2->v3:
> - The device tree bindings were reworked in order to make it look more like the
>   IPUv3 bindings.
> - The interface_pix_fmt property now looks like the IPUv3 one.
> ---
>  .../devicetree/bindings/video/fsl,mx3-fb.txt       |   35 ++++++
>  drivers/video/Kconfig                              |    2 +
>  drivers/video/mx3fb.c                              |  125 +++++++++++++++++---
>  3 files changed, 147 insertions(+), 15 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> 
> diff --git a/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> new file mode 100644
> index 0000000..0b31374
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> @@ -0,0 +1,35 @@
> +Freescale MX3 fb
> +================
> +
> +Required properties:
> +- compatible: Should be "fsl,mx3fb". compatible chips include the imx31 and the
> +  imx35.
> +- reg: should be register base and length as documented in the datasheet.
> +- clocks: Handle to the ipu_gate clock.
> +
> +Example:
> +
> +lcdc: mx3fb at 53fc00b4 {
> +	compatible = "fsl,mx3-fb";
> +	reg = <0x53fc00b4 0x0b>;
> +	clocks = <&clks 55>;
> +};

This (and some of the other bindings) are trivial, and they are all
associated with a single SoC. I think it would be better to collect all
the mx3 bindings into a single file rather than distributing them all
over the bindings tree.

I started thinking about this after some of the DT conversations in
Edinburgh this week. Unless there is a high likelyhood of components
being used separately, I think it is far more useful to collect all the
bindings for an SoC into a single file. It will certainly reduce a lot
of the boilerplate that we've been collecting in bindings documentation
files.

A long time ago I took that approach for the mpc5200 documentation[1].
Take a look at that organization and let me know what you think.

[1] Documentation/devicetree/bindings/powerpc/fsl/mpc5200.txt

g.

> +
> +Display support
> +===============
> +Required properties:
> +- model : The user-visible name of the display.
> +
> +Optional properties:
> +- interface_pix_fmt: How this display is connected to the
> +  crtc. Currently supported types: "rgb24", "rgb565", "rgb666"
> +
> +It can also have an optional timing subnode as described in
> +  Documentation/devicetree/bindings/video/display-timing.txt.
> +
> +Example:
> +
> +display at di0 {
> +	    interface-pix-fmt = "rgb666";
> +	    model = "CMO-QVGA";
> +};
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 14317b7..2a638df 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -2359,6 +2359,8 @@ config FB_MX3
>  	select FB_CFB_FILLRECT
>  	select FB_CFB_COPYAREA
>  	select FB_CFB_IMAGEBLIT
> +	select VIDEOMODE_HELPERS
> +	select FB_MODE_HELPERS
>  	default y
>  	help
>  	  This is a framebuffer device for the i.MX31 LCD Controller. So
> diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
> index 804f874..de5a6c8 100644
> --- a/drivers/video/mx3fb.c
> +++ b/drivers/video/mx3fb.c
> @@ -31,6 +31,8 @@
>  #include <linux/platform_data/dma-imx.h>
>  #include <linux/platform_data/video-mx3fb.h>
>  
> +#include <video/of_display_timing.h>
> +
>  #include <asm/io.h>
>  #include <asm/uaccess.h>
>  
> @@ -757,11 +759,13 @@ static int __set_par(struct fb_info *fbi, bool lock)
>  			sig_cfg.Hsync_pol = true;
>  		if (fbi->var.sync & FB_SYNC_VERT_HIGH_ACT)
>  			sig_cfg.Vsync_pol = true;
> -		if (fbi->var.sync & FB_SYNC_CLK_INVERT)
> +		if ((fbi->var.sync & FB_SYNC_CLK_INVERT) ||
> +		    (fbi->var.sync & FB_SYNC_PIXDAT_HIGH_ACT))
>  			sig_cfg.clk_pol = true;
>  		if (fbi->var.sync & FB_SYNC_DATA_INVERT)
>  			sig_cfg.data_pol = true;
> -		if (fbi->var.sync & FB_SYNC_OE_ACT_HIGH)
> +		if ((fbi->var.sync & FB_SYNC_OE_ACT_HIGH) ||
> +		    (fbi->var.sync & FB_SYNC_DE_HIGH_ACT))
>  			sig_cfg.enable_pol = true;
>  		if (fbi->var.sync & FB_SYNC_CLK_IDLE_EN)
>  			sig_cfg.clkidle_en = true;
> @@ -1351,21 +1355,75 @@ static struct fb_info *mx3fb_init_fbinfo(struct device *dev, struct fb_ops *ops)
>  	return fbi;
>  }
>  
> +static int match_dt_disp_data(const char *property)
> +{
> +	if (!strcmp("rgb666", property))
> +		return IPU_DISP_DATA_MAPPING_RGB666;
> +	else if (!strcmp("rgb565", property))
> +		return IPU_DISP_DATA_MAPPING_RGB565;
> +	else if (!strcmp("rgb24", property))
> +		return IPU_DISP_DATA_MAPPING_RGB888;
> +	else
> +		return -EINVAL;
> +}
> +
>  static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
>  {
>  	struct device *dev = mx3fb->dev;
>  	struct mx3fb_platform_data *mx3fb_pdata = dev_get_platdata(dev);
> -	const char *name = mx3fb_pdata->name;
> +	struct device_node *np = dev->of_node;
> +	const char *name;
> +	const char *ipu_disp_format;
>  	unsigned int irq;
>  	struct fb_info *fbi;
>  	struct mx3fb_info *mx3fbi;
>  	const struct fb_videomode *mode;
>  	int ret, num_modes;
> +	struct fb_videomode of_mode;
> +	struct device_node *display_np, *root_np;
> +
> +	if (np) {
> +		root_np = of_find_node_by_path("/");
> +		if (!root_np) {
> +			dev_err(dev, "Can't get the \"/\" node.\n");
> +			return -EINVAL;
> +		}
> +
> +		display_np = of_find_node_by_name(root_np, "display");
> +		if (!display_np) {
> +			dev_err(dev, "Can't get the display device node.\n");
> +			return -EINVAL;
> +		}

Finding a node by name is bad practice. Is is possible to find the node
via compatible?

> +
> +		of_property_read_string(display_np, "interface-pix-fmt",
> +					&ipu_disp_format);
> +		if (!ipu_disp_format) {
> +			mx3fb->disp_data_fmt = IPU_DISP_DATA_MAPPING_RGB666;
> +			dev_warn(dev,
> +				"ipu display data mapping was not defined, using the default rgb666.\n");
> +		} else {
> +			mx3fb->disp_data_fmt =
> +				match_dt_disp_data(ipu_disp_format);
> +		}
> +
> +		if (mx3fb->disp_data_fmt == -EINVAL) {
> +			dev_err(dev, "Illegal display data format \"%s\"\n",
> +				ipu_disp_format);
> +			return -EINVAL;
> +		}
>  
> -	if (mx3fb_pdata->disp_data_fmt >= ARRAY_SIZE(di_mappings)) {
> -		dev_err(dev, "Illegal display data format %d\n",
> +		of_property_read_string(display_np, "model", &name);
> +		if (ret) {
> +			dev_err(dev, "Missing display model name\n");
> +			return -EINVAL;
> +		}
> +	} else {
> +		name = mx3fb_pdata->name;
> +		if (mx3fb_pdata->disp_data_fmt >= ARRAY_SIZE(di_mappings)) {
> +			dev_err(dev, "Illegal display data format %d\n",
>  				mx3fb_pdata->disp_data_fmt);
> -		return -EINVAL;
> +			return -EINVAL;
> +		}
>  	}
>  
>  	ichan->client = mx3fb;
> @@ -1386,12 +1444,24 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
>  		goto emode;
>  	}
>  
> -	if (mx3fb_pdata->mode && mx3fb_pdata->num_modes) {
> -		mode = mx3fb_pdata->mode;
> -		num_modes = mx3fb_pdata->num_modes;
> +	if (np) {
> +		ret = of_get_fb_videomode(display_np, &of_mode,
> +					  OF_USE_NATIVE_MODE);
> +		if (!ret) {
> +			mode = &of_mode;
> +			num_modes = 1;
> +		} else {
> +			mode = mx3fb_modedb;
> +			num_modes = ARRAY_SIZE(mx3fb_modedb);
> +		}
>  	} else {
> -		mode = mx3fb_modedb;
> -		num_modes = ARRAY_SIZE(mx3fb_modedb);
> +		if (mx3fb_pdata->mode || !mx3fb_pdata->num_modes) {
> +			mode = mx3fb_pdata->mode;
> +			num_modes = mx3fb_pdata->num_modes;
> +		} else {
> +			mode = mx3fb_modedb;
> +			num_modes = ARRAY_SIZE(mx3fb_modedb);
> +		}
>  	}
>  
>  	if (!fb_find_mode(&fbi->var, fbi, fb_mode, mode,
> @@ -1421,7 +1491,8 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
>  	mx3fbi->mx3fb		= mx3fb;
>  	mx3fbi->blank		= FB_BLANK_NORMAL;
>  
> -	mx3fb->disp_data_fmt	= mx3fb_pdata->disp_data_fmt;
> +	if (!np)
> +		mx3fb->disp_data_fmt = mx3fb_pdata->disp_data_fmt;
>  
>  	init_completion(&mx3fbi->flip_cmpl);
>  	disable_irq(ichan->eof_irq);
> @@ -1449,13 +1520,26 @@ emode:
>  	return ret;
>  }
>  
> +static int imx_dma_is_dt_ipu(struct dma_chan *chan)
> +{
> +	/* Example: 53fc0000.ipu */
> +	if (chan && chan->device && chan->device->dev) {
> +		char *name = dev_name(chan->device->dev);
> +		name = strchr(name, '.');
> +		if (name)
> +			return !strcmp(name, ".ipu");
> +	}
> +
> +	return 0;
> +}
> +
>  static bool chan_filter(struct dma_chan *chan, void *arg)
>  {
>  	struct dma_chan_request *rq = arg;
>  	struct device *dev;
>  	struct mx3fb_platform_data *mx3fb_pdata;
>  
> -	if (!imx_dma_is_ipu(chan))
> +	if (!imx_dma_is_ipu(chan) && !imx_dma_is_dt_ipu(chan))
>  		return false;
>  
>  	if (!rq)
> @@ -1464,8 +1548,12 @@ static bool chan_filter(struct dma_chan *chan, void *arg)
>  	dev = rq->mx3fb->dev;
>  	mx3fb_pdata = dev_get_platdata(dev);
>  
> -	return rq->id == chan->chan_id &&
> -		mx3fb_pdata->dma_dev == chan->device->dev;
> +	/* When using the devicetree, mx3fb_pdata is NULL */
> +	if (imx_dma_is_dt_ipu(chan))
> +		return rq->id == chan->chan_id;
> +	else
> +		return rq->id == chan->chan_id &&
> +			mx3fb_pdata->dma_dev == chan->device->dev;
>  }
>  
>  static void release_fbi(struct fb_info *fbi)
> @@ -1565,9 +1653,16 @@ static int mx3fb_remove(struct platform_device *dev)
>  	return 0;
>  }
>  
> +static struct of_device_id mx3fb_of_dev_id[] = {
> +	{ .compatible = "fsl,mx3-fb", },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, mx3fb_of_dev_id);
> +
>  static struct platform_driver mx3fb_driver = {
>  	.driver = {
>  		.name = MX3FB_NAME,
> +		.of_match_table = mx3fb_of_dev_id,
>  		.owner = THIS_MODULE,
>  	},
>  	.probe = mx3fb_probe,
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv3][ 3/5] video: mx3fb: Add device tree suport.
  2013-10-25 19:50       ` Grant Likely
  (?)
@ 2013-10-26  0:18           ` Sascha Hauer
  -1 siblings, 0 replies; 29+ messages in thread
From: Sascha Hauer @ 2013-10-26  0:18 UTC (permalink / raw)
  To: Grant Likely
  Cc: Denis Carikli, Sascha Hauer, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Pawel Moll, Stephen Warren,
	Ian Campbell, Rob Herring, Tomi Valkeinen, Eric Bénard,
	Shawn Guo, Jean-Christophe Plagniol-Villard,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Fri, Oct 25, 2013 at 08:50:40PM +0100, Grant Likely wrote:
> On Wed, 23 Oct 2013 14:43:47 +0200, Denis Carikli <denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org> wrote:
> > Cc: Jean-Christophe Plagniol-Villard <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
> > Cc: Tomi Valkeinen <tomi.valkeinen-l0cyMroinI0@public.gmane.org>
> > Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> > Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
> > Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> > Cc: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
> > Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
> > Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Cc: Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> > Cc: Eric Bénard <eric-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
> > Signed-off-by: Denis Carikli <denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
> > ---
> > ChangeLog v2->v3:
> > - The device tree bindings were reworked in order to make it look more like the
> >   IPUv3 bindings.
> > - The interface_pix_fmt property now looks like the IPUv3 one.
> > ---
> >  .../devicetree/bindings/video/fsl,mx3-fb.txt       |   35 ++++++
> >  drivers/video/Kconfig                              |    2 +
> >  drivers/video/mx3fb.c                              |  125 +++++++++++++++++---
> >  3 files changed, 147 insertions(+), 15 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> > new file mode 100644
> > index 0000000..0b31374
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> > @@ -0,0 +1,35 @@
> > +Freescale MX3 fb
> > +================
> > +
> > +Required properties:
> > +- compatible: Should be "fsl,mx3fb". compatible chips include the imx31 and the
> > +  imx35.
> > +- reg: should be register base and length as documented in the datasheet.
> > +- clocks: Handle to the ipu_gate clock.
> > +
> > +Example:
> > +
> > +lcdc: mx3fb@53fc00b4 {
> > +	compatible = "fsl,mx3-fb";
> > +	reg = <0x53fc00b4 0x0b>;
> > +	clocks = <&clks 55>;
> > +};
> 
> This (and some of the other bindings) are trivial, and they are all
> associated with a single SoC. I think it would be better to collect all
> the mx3 bindings into a single file rather than distributing them all
> over the bindings tree.
> 
> I started thinking about this after some of the DT conversations in
> Edinburgh this week. Unless there is a high likelyhood of components
> being used separately, I think it is far more useful to collect all the
> bindings for an SoC into a single file. It will certainly reduce a lot
> of the boilerplate that we've been collecting in bindings documentation
> files.
> 
> A long time ago I took that approach for the mpc5200 documentation[1].
> Take a look at that organization and let me know what you think.

I don't think this is a good idea. When a new SoC comes out we don't
know which components will be reused on the next SoC. This will cause a
lot of bikeshedding when it actually is reused and then has to be moved.

Also I would find it quite inconsistent if I had to lookup some devices
in a SoC file and most bindings in subsystem specific files. So when
searching for a binding I would first have to know if the hardware is
unique to the SoC or not.

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 |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv3][ 3/5] video: mx3fb: Add device tree suport.
@ 2013-10-26  0:18           ` Sascha Hauer
  0 siblings, 0 replies; 29+ messages in thread
From: Sascha Hauer @ 2013-10-26  0:18 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Oct 25, 2013 at 08:50:40PM +0100, Grant Likely wrote:
> On Wed, 23 Oct 2013 14:43:47 +0200, Denis Carikli <denis@eukrea.com> wrote:
> > Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > Cc: linux-fbdev@vger.kernel.org
> > Cc: Rob Herring <rob.herring@calxeda.com>
> > Cc: Pawel Moll <pawel.moll@arm.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Stephen Warren <swarren@wwwdotorg.org>
> > Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> > Cc: devicetree@vger.kernel.org
> > Cc: Sascha Hauer <kernel@pengutronix.de>
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: Eric Bénard <eric@eukrea.com>
> > Signed-off-by: Denis Carikli <denis@eukrea.com>
> > ---
> > ChangeLog v2->v3:
> > - The device tree bindings were reworked in order to make it look more like the
> >   IPUv3 bindings.
> > - The interface_pix_fmt property now looks like the IPUv3 one.
> > ---
> >  .../devicetree/bindings/video/fsl,mx3-fb.txt       |   35 ++++++
> >  drivers/video/Kconfig                              |    2 +
> >  drivers/video/mx3fb.c                              |  125 +++++++++++++++++---
> >  3 files changed, 147 insertions(+), 15 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> > new file mode 100644
> > index 0000000..0b31374
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> > @@ -0,0 +1,35 @@
> > +Freescale MX3 fb
> > +========
> > +
> > +Required properties:
> > +- compatible: Should be "fsl,mx3fb". compatible chips include the imx31 and the
> > +  imx35.
> > +- reg: should be register base and length as documented in the datasheet.
> > +- clocks: Handle to the ipu_gate clock.
> > +
> > +Example:
> > +
> > +lcdc: mx3fb@53fc00b4 {
> > +	compatible = "fsl,mx3-fb";
> > +	reg = <0x53fc00b4 0x0b>;
> > +	clocks = <&clks 55>;
> > +};
> 
> This (and some of the other bindings) are trivial, and they are all
> associated with a single SoC. I think it would be better to collect all
> the mx3 bindings into a single file rather than distributing them all
> over the bindings tree.
> 
> I started thinking about this after some of the DT conversations in
> Edinburgh this week. Unless there is a high likelyhood of components
> being used separately, I think it is far more useful to collect all the
> bindings for an SoC into a single file. It will certainly reduce a lot
> of the boilerplate that we've been collecting in bindings documentation
> files.
> 
> A long time ago I took that approach for the mpc5200 documentation[1].
> Take a look at that organization and let me know what you think.

I don't think this is a good idea. When a new SoC comes out we don't
know which components will be reused on the next SoC. This will cause a
lot of bikeshedding when it actually is reused and then has to be moved.

Also I would find it quite inconsistent if I had to lookup some devices
in a SoC file and most bindings in subsystem specific files. So when
searching for a binding I would first have to know if the hardware is
unique to the SoC or not.

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 |

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCHv3][ 3/5] video: mx3fb: Add device tree suport.
@ 2013-10-26  0:18           ` Sascha Hauer
  0 siblings, 0 replies; 29+ messages in thread
From: Sascha Hauer @ 2013-10-26  0:18 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Oct 25, 2013 at 08:50:40PM +0100, Grant Likely wrote:
> On Wed, 23 Oct 2013 14:43:47 +0200, Denis Carikli <denis@eukrea.com> wrote:
> > Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > Cc: linux-fbdev at vger.kernel.org
> > Cc: Rob Herring <rob.herring@calxeda.com>
> > Cc: Pawel Moll <pawel.moll@arm.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Stephen Warren <swarren@wwwdotorg.org>
> > Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> > Cc: devicetree at vger.kernel.org
> > Cc: Sascha Hauer <kernel@pengutronix.de>
> > Cc: linux-arm-kernel at lists.infradead.org
> > Cc: Eric B?nard <eric@eukrea.com>
> > Signed-off-by: Denis Carikli <denis@eukrea.com>
> > ---
> > ChangeLog v2->v3:
> > - The device tree bindings were reworked in order to make it look more like the
> >   IPUv3 bindings.
> > - The interface_pix_fmt property now looks like the IPUv3 one.
> > ---
> >  .../devicetree/bindings/video/fsl,mx3-fb.txt       |   35 ++++++
> >  drivers/video/Kconfig                              |    2 +
> >  drivers/video/mx3fb.c                              |  125 +++++++++++++++++---
> >  3 files changed, 147 insertions(+), 15 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> > new file mode 100644
> > index 0000000..0b31374
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> > @@ -0,0 +1,35 @@
> > +Freescale MX3 fb
> > +================
> > +
> > +Required properties:
> > +- compatible: Should be "fsl,mx3fb". compatible chips include the imx31 and the
> > +  imx35.
> > +- reg: should be register base and length as documented in the datasheet.
> > +- clocks: Handle to the ipu_gate clock.
> > +
> > +Example:
> > +
> > +lcdc: mx3fb at 53fc00b4 {
> > +	compatible = "fsl,mx3-fb";
> > +	reg = <0x53fc00b4 0x0b>;
> > +	clocks = <&clks 55>;
> > +};
> 
> This (and some of the other bindings) are trivial, and they are all
> associated with a single SoC. I think it would be better to collect all
> the mx3 bindings into a single file rather than distributing them all
> over the bindings tree.
> 
> I started thinking about this after some of the DT conversations in
> Edinburgh this week. Unless there is a high likelyhood of components
> being used separately, I think it is far more useful to collect all the
> bindings for an SoC into a single file. It will certainly reduce a lot
> of the boilerplate that we've been collecting in bindings documentation
> files.
> 
> A long time ago I took that approach for the mpc5200 documentation[1].
> Take a look at that organization and let me know what you think.

I don't think this is a good idea. When a new SoC comes out we don't
know which components will be reused on the next SoC. This will cause a
lot of bikeshedding when it actually is reused and then has to be moved.

Also I would find it quite inconsistent if I had to lookup some devices
in a SoC file and most bindings in subsystem specific files. So when
searching for a binding I would first have to know if the hardware is
unique to the SoC or not.

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 |

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv3][ 3/5] video: mx3fb: Add device tree suport.
  2013-10-23 12:43     ` Denis Carikli
  (?)
@ 2013-10-26  6:40         ` Kumar Gala
  -1 siblings, 0 replies; 29+ messages in thread
From: Kumar Gala @ 2013-10-26  6:40 UTC (permalink / raw)
  To: Denis Carikli
  Cc: Sascha Hauer, Shawn Guo, Eric Bénard,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Pawel Moll,
	Mark Rutland, Stephen Warren, Ian Campbell,
	devicetree-u79uwXL29TY76Z2rM5mHXA


On Oct 23, 2013, at 7:43 AM, Denis Carikli wrote:

> Cc: Jean-Christophe Plagniol-Villard <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
> Cc: Tomi Valkeinen <tomi.valkeinen-l0cyMroinI0@public.gmane.org>
> Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> Cc: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
> Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> Cc: Eric Bénard <eric-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Denis Carikli <denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
> ---
> ChangeLog v2->v3:
> - The device tree bindings were reworked in order to make it look more like the
>  IPUv3 bindings.
> - The interface_pix_fmt property now looks like the IPUv3 one.
> ---
> .../devicetree/bindings/video/fsl,mx3-fb.txt       |   35 ++++++
> drivers/video/Kconfig                              |    2 +
> drivers/video/mx3fb.c                              |  125 +++++++++++++++++---
> 3 files changed, 147 insertions(+), 15 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> 
> diff --git a/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> new file mode 100644
> index 0000000..0b31374
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> @@ -0,0 +1,35 @@
> +Freescale MX3 fb

Can you spell out framebuffer here instead of fb

> +================
> +
> +Required properties:
> +- compatible: Should be "fsl,mx3fb". compatible chips include the imx31 and the
> +  imx35.
> +- reg: should be register base and length as documented in the datasheet.
> +- clocks: Handle to the ipu_gate clock.
> +
> +Example:
> +
> +lcdc: mx3fb@53fc00b4 {
> +	compatible = "fsl,mx3-fb";
> +	reg = <0x53fc00b4 0x0b>;
> +	clocks = <&clks 55>;
> +};
> +
> +Display support
> +===============
> +Required properties:
> +- model : The user-visible name of the display.
> +
> +Optional properties:
> +- interface_pix_fmt: How this display is connected to the
> +  crtc. Currently supported types: "rgb24", "rgb565", "rgb666"


Why is there no compatible for the display?

> +
> +It can also have an optional timing subnode as described in
> +  Documentation/devicetree/bindings/video/display-timing.txt.
> +
> +Example:
> +
> +display@di0 {
> +	    interface-pix-fmt = "rgb666";
> +	    model = "CMO-QVGA";
> +};

how do you relate the display to the framebuffer?

- k

> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 14317b7..2a638df 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -2359,6 +2359,8 @@ config FB_MX3
> 	select FB_CFB_FILLRECT
> 	select FB_CFB_COPYAREA
> 	select FB_CFB_IMAGEBLIT
> +	select VIDEOMODE_HELPERS
> +	select FB_MODE_HELPERS
> 	default y
> 	help
> 	  This is a framebuffer device for the i.MX31 LCD Controller. So
> diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
> index 804f874..de5a6c8 100644
> --- a/drivers/video/mx3fb.c
> +++ b/drivers/video/mx3fb.c
> @@ -31,6 +31,8 @@
> #include <linux/platform_data/dma-imx.h>
> #include <linux/platform_data/video-mx3fb.h>
> 
> +#include <video/of_display_timing.h>
> +
> #include <asm/io.h>
> #include <asm/uaccess.h>
> 
> @@ -757,11 +759,13 @@ static int __set_par(struct fb_info *fbi, bool lock)
> 			sig_cfg.Hsync_pol = true;
> 		if (fbi->var.sync & FB_SYNC_VERT_HIGH_ACT)
> 			sig_cfg.Vsync_pol = true;
> -		if (fbi->var.sync & FB_SYNC_CLK_INVERT)
> +		if ((fbi->var.sync & FB_SYNC_CLK_INVERT) ||
> +		    (fbi->var.sync & FB_SYNC_PIXDAT_HIGH_ACT))
> 			sig_cfg.clk_pol = true;
> 		if (fbi->var.sync & FB_SYNC_DATA_INVERT)
> 			sig_cfg.data_pol = true;
> -		if (fbi->var.sync & FB_SYNC_OE_ACT_HIGH)
> +		if ((fbi->var.sync & FB_SYNC_OE_ACT_HIGH) ||
> +		    (fbi->var.sync & FB_SYNC_DE_HIGH_ACT))
> 			sig_cfg.enable_pol = true;
> 		if (fbi->var.sync & FB_SYNC_CLK_IDLE_EN)
> 			sig_cfg.clkidle_en = true;
> @@ -1351,21 +1355,75 @@ static struct fb_info *mx3fb_init_fbinfo(struct device *dev, struct fb_ops *ops)
> 	return fbi;
> }
> 
> +static int match_dt_disp_data(const char *property)
> +{
> +	if (!strcmp("rgb666", property))
> +		return IPU_DISP_DATA_MAPPING_RGB666;
> +	else if (!strcmp("rgb565", property))
> +		return IPU_DISP_DATA_MAPPING_RGB565;
> +	else if (!strcmp("rgb24", property))
> +		return IPU_DISP_DATA_MAPPING_RGB888;
> +	else
> +		return -EINVAL;
> +}
> +
> static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
> {
> 	struct device *dev = mx3fb->dev;
> 	struct mx3fb_platform_data *mx3fb_pdata = dev_get_platdata(dev);
> -	const char *name = mx3fb_pdata->name;
> +	struct device_node *np = dev->of_node;
> +	const char *name;
> +	const char *ipu_disp_format;
> 	unsigned int irq;
> 	struct fb_info *fbi;
> 	struct mx3fb_info *mx3fbi;
> 	const struct fb_videomode *mode;
> 	int ret, num_modes;
> +	struct fb_videomode of_mode;
> +	struct device_node *display_np, *root_np;
> +
> +	if (np) {
> +		root_np = of_find_node_by_path("/");
> +		if (!root_np) {
> +			dev_err(dev, "Can't get the \"/\" node.\n");
> +			return -EINVAL;
> +		}
> +
> +		display_np = of_find_node_by_name(root_np, "display");
> +		if (!display_np) {
> +			dev_err(dev, "Can't get the display device node.\n");
> +			return -EINVAL;
> +		}
> +
> +		of_property_read_string(display_np, "interface-pix-fmt",
> +					&ipu_disp_format);
> +		if (!ipu_disp_format) {
> +			mx3fb->disp_data_fmt = IPU_DISP_DATA_MAPPING_RGB666;
> +			dev_warn(dev,
> +				"ipu display data mapping was not defined, using the default rgb666.\n");
> +		} else {
> +			mx3fb->disp_data_fmt =
> +				match_dt_disp_data(ipu_disp_format);
> +		}
> +
> +		if (mx3fb->disp_data_fmt == -EINVAL) {
> +			dev_err(dev, "Illegal display data format \"%s\"\n",
> +				ipu_disp_format);
> +			return -EINVAL;
> +		}
> 
> -	if (mx3fb_pdata->disp_data_fmt >= ARRAY_SIZE(di_mappings)) {
> -		dev_err(dev, "Illegal display data format %d\n",
> +		of_property_read_string(display_np, "model", &name);
> +		if (ret) {
> +			dev_err(dev, "Missing display model name\n");
> +			return -EINVAL;
> +		}
> +	} else {
> +		name = mx3fb_pdata->name;
> +		if (mx3fb_pdata->disp_data_fmt >= ARRAY_SIZE(di_mappings)) {
> +			dev_err(dev, "Illegal display data format %d\n",
> 				mx3fb_pdata->disp_data_fmt);
> -		return -EINVAL;
> +			return -EINVAL;
> +		}
> 	}
> 
> 	ichan->client = mx3fb;
> @@ -1386,12 +1444,24 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
> 		goto emode;
> 	}
> 
> -	if (mx3fb_pdata->mode && mx3fb_pdata->num_modes) {
> -		mode = mx3fb_pdata->mode;
> -		num_modes = mx3fb_pdata->num_modes;
> +	if (np) {
> +		ret = of_get_fb_videomode(display_np, &of_mode,
> +					  OF_USE_NATIVE_MODE);
> +		if (!ret) {
> +			mode = &of_mode;
> +			num_modes = 1;
> +		} else {
> +			mode = mx3fb_modedb;
> +			num_modes = ARRAY_SIZE(mx3fb_modedb);
> +		}
> 	} else {
> -		mode = mx3fb_modedb;
> -		num_modes = ARRAY_SIZE(mx3fb_modedb);
> +		if (mx3fb_pdata->mode || !mx3fb_pdata->num_modes) {
> +			mode = mx3fb_pdata->mode;
> +			num_modes = mx3fb_pdata->num_modes;
> +		} else {
> +			mode = mx3fb_modedb;
> +			num_modes = ARRAY_SIZE(mx3fb_modedb);
> +		}
> 	}
> 
> 	if (!fb_find_mode(&fbi->var, fbi, fb_mode, mode,
> @@ -1421,7 +1491,8 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
> 	mx3fbi->mx3fb		= mx3fb;
> 	mx3fbi->blank		= FB_BLANK_NORMAL;
> 
> -	mx3fb->disp_data_fmt	= mx3fb_pdata->disp_data_fmt;
> +	if (!np)
> +		mx3fb->disp_data_fmt = mx3fb_pdata->disp_data_fmt;
> 
> 	init_completion(&mx3fbi->flip_cmpl);
> 	disable_irq(ichan->eof_irq);
> @@ -1449,13 +1520,26 @@ emode:
> 	return ret;
> }
> 
> +static int imx_dma_is_dt_ipu(struct dma_chan *chan)
> +{
> +	/* Example: 53fc0000.ipu */
> +	if (chan && chan->device && chan->device->dev) {
> +		char *name = dev_name(chan->device->dev);
> +		name = strchr(name, '.');
> +		if (name)
> +			return !strcmp(name, ".ipu");
> +	}
> +
> +	return 0;
> +}
> +
> static bool chan_filter(struct dma_chan *chan, void *arg)
> {
> 	struct dma_chan_request *rq = arg;
> 	struct device *dev;
> 	struct mx3fb_platform_data *mx3fb_pdata;
> 
> -	if (!imx_dma_is_ipu(chan))
> +	if (!imx_dma_is_ipu(chan) && !imx_dma_is_dt_ipu(chan))
> 		return false;
> 
> 	if (!rq)
> @@ -1464,8 +1548,12 @@ static bool chan_filter(struct dma_chan *chan, void *arg)
> 	dev = rq->mx3fb->dev;
> 	mx3fb_pdata = dev_get_platdata(dev);
> 
> -	return rq->id == chan->chan_id &&
> -		mx3fb_pdata->dma_dev == chan->device->dev;
> +	/* When using the devicetree, mx3fb_pdata is NULL */
> +	if (imx_dma_is_dt_ipu(chan))
> +		return rq->id == chan->chan_id;
> +	else
> +		return rq->id == chan->chan_id &&
> +			mx3fb_pdata->dma_dev == chan->device->dev;
> }
> 
> static void release_fbi(struct fb_info *fbi)
> @@ -1565,9 +1653,16 @@ static int mx3fb_remove(struct platform_device *dev)
> 	return 0;
> }
> 
> +static struct of_device_id mx3fb_of_dev_id[] = {
> +	{ .compatible = "fsl,mx3-fb", },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, mx3fb_of_dev_id);
> +
> static struct platform_driver mx3fb_driver = {
> 	.driver = {
> 		.name = MX3FB_NAME,
> +		.of_match_table = mx3fb_of_dev_id,
> 		.owner = THIS_MODULE,
> 	},
> 	.probe = mx3fb_probe,
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv3][ 3/5] video: mx3fb: Add device tree suport.
@ 2013-10-26  6:40         ` Kumar Gala
  0 siblings, 0 replies; 29+ messages in thread
From: Kumar Gala @ 2013-10-26  6:40 UTC (permalink / raw)
  To: linux-arm-kernel


On Oct 23, 2013, at 7:43 AM, Denis Carikli wrote:

> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: devicetree@vger.kernel.org
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: Eric Bénard <eric@eukrea.com>
> Signed-off-by: Denis Carikli <denis@eukrea.com>
> ---
> ChangeLog v2->v3:
> - The device tree bindings were reworked in order to make it look more like the
>  IPUv3 bindings.
> - The interface_pix_fmt property now looks like the IPUv3 one.
> ---
> .../devicetree/bindings/video/fsl,mx3-fb.txt       |   35 ++++++
> drivers/video/Kconfig                              |    2 +
> drivers/video/mx3fb.c                              |  125 +++++++++++++++++---
> 3 files changed, 147 insertions(+), 15 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> 
> diff --git a/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> new file mode 100644
> index 0000000..0b31374
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> @@ -0,0 +1,35 @@
> +Freescale MX3 fb

Can you spell out framebuffer here instead of fb

> +========
> +
> +Required properties:
> +- compatible: Should be "fsl,mx3fb". compatible chips include the imx31 and the
> +  imx35.
> +- reg: should be register base and length as documented in the datasheet.
> +- clocks: Handle to the ipu_gate clock.
> +
> +Example:
> +
> +lcdc: mx3fb@53fc00b4 {
> +	compatible = "fsl,mx3-fb";
> +	reg = <0x53fc00b4 0x0b>;
> +	clocks = <&clks 55>;
> +};
> +
> +Display support
> +=======> +Required properties:
> +- model : The user-visible name of the display.
> +
> +Optional properties:
> +- interface_pix_fmt: How this display is connected to the
> +  crtc. Currently supported types: "rgb24", "rgb565", "rgb666"


Why is there no compatible for the display?

> +
> +It can also have an optional timing subnode as described in
> +  Documentation/devicetree/bindings/video/display-timing.txt.
> +
> +Example:
> +
> +display@di0 {
> +	    interface-pix-fmt = "rgb666";
> +	    model = "CMO-QVGA";
> +};

how do you relate the display to the framebuffer?

- k

> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 14317b7..2a638df 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -2359,6 +2359,8 @@ config FB_MX3
> 	select FB_CFB_FILLRECT
> 	select FB_CFB_COPYAREA
> 	select FB_CFB_IMAGEBLIT
> +	select VIDEOMODE_HELPERS
> +	select FB_MODE_HELPERS
> 	default y
> 	help
> 	  This is a framebuffer device for the i.MX31 LCD Controller. So
> diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
> index 804f874..de5a6c8 100644
> --- a/drivers/video/mx3fb.c
> +++ b/drivers/video/mx3fb.c
> @@ -31,6 +31,8 @@
> #include <linux/platform_data/dma-imx.h>
> #include <linux/platform_data/video-mx3fb.h>
> 
> +#include <video/of_display_timing.h>
> +
> #include <asm/io.h>
> #include <asm/uaccess.h>
> 
> @@ -757,11 +759,13 @@ static int __set_par(struct fb_info *fbi, bool lock)
> 			sig_cfg.Hsync_pol = true;
> 		if (fbi->var.sync & FB_SYNC_VERT_HIGH_ACT)
> 			sig_cfg.Vsync_pol = true;
> -		if (fbi->var.sync & FB_SYNC_CLK_INVERT)
> +		if ((fbi->var.sync & FB_SYNC_CLK_INVERT) ||
> +		    (fbi->var.sync & FB_SYNC_PIXDAT_HIGH_ACT))
> 			sig_cfg.clk_pol = true;
> 		if (fbi->var.sync & FB_SYNC_DATA_INVERT)
> 			sig_cfg.data_pol = true;
> -		if (fbi->var.sync & FB_SYNC_OE_ACT_HIGH)
> +		if ((fbi->var.sync & FB_SYNC_OE_ACT_HIGH) ||
> +		    (fbi->var.sync & FB_SYNC_DE_HIGH_ACT))
> 			sig_cfg.enable_pol = true;
> 		if (fbi->var.sync & FB_SYNC_CLK_IDLE_EN)
> 			sig_cfg.clkidle_en = true;
> @@ -1351,21 +1355,75 @@ static struct fb_info *mx3fb_init_fbinfo(struct device *dev, struct fb_ops *ops)
> 	return fbi;
> }
> 
> +static int match_dt_disp_data(const char *property)
> +{
> +	if (!strcmp("rgb666", property))
> +		return IPU_DISP_DATA_MAPPING_RGB666;
> +	else if (!strcmp("rgb565", property))
> +		return IPU_DISP_DATA_MAPPING_RGB565;
> +	else if (!strcmp("rgb24", property))
> +		return IPU_DISP_DATA_MAPPING_RGB888;
> +	else
> +		return -EINVAL;
> +}
> +
> static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
> {
> 	struct device *dev = mx3fb->dev;
> 	struct mx3fb_platform_data *mx3fb_pdata = dev_get_platdata(dev);
> -	const char *name = mx3fb_pdata->name;
> +	struct device_node *np = dev->of_node;
> +	const char *name;
> +	const char *ipu_disp_format;
> 	unsigned int irq;
> 	struct fb_info *fbi;
> 	struct mx3fb_info *mx3fbi;
> 	const struct fb_videomode *mode;
> 	int ret, num_modes;
> +	struct fb_videomode of_mode;
> +	struct device_node *display_np, *root_np;
> +
> +	if (np) {
> +		root_np = of_find_node_by_path("/");
> +		if (!root_np) {
> +			dev_err(dev, "Can't get the \"/\" node.\n");
> +			return -EINVAL;
> +		}
> +
> +		display_np = of_find_node_by_name(root_np, "display");
> +		if (!display_np) {
> +			dev_err(dev, "Can't get the display device node.\n");
> +			return -EINVAL;
> +		}
> +
> +		of_property_read_string(display_np, "interface-pix-fmt",
> +					&ipu_disp_format);
> +		if (!ipu_disp_format) {
> +			mx3fb->disp_data_fmt = IPU_DISP_DATA_MAPPING_RGB666;
> +			dev_warn(dev,
> +				"ipu display data mapping was not defined, using the default rgb666.\n");
> +		} else {
> +			mx3fb->disp_data_fmt > +				match_dt_disp_data(ipu_disp_format);
> +		}
> +
> +		if (mx3fb->disp_data_fmt = -EINVAL) {
> +			dev_err(dev, "Illegal display data format \"%s\"\n",
> +				ipu_disp_format);
> +			return -EINVAL;
> +		}
> 
> -	if (mx3fb_pdata->disp_data_fmt >= ARRAY_SIZE(di_mappings)) {
> -		dev_err(dev, "Illegal display data format %d\n",
> +		of_property_read_string(display_np, "model", &name);
> +		if (ret) {
> +			dev_err(dev, "Missing display model name\n");
> +			return -EINVAL;
> +		}
> +	} else {
> +		name = mx3fb_pdata->name;
> +		if (mx3fb_pdata->disp_data_fmt >= ARRAY_SIZE(di_mappings)) {
> +			dev_err(dev, "Illegal display data format %d\n",
> 				mx3fb_pdata->disp_data_fmt);
> -		return -EINVAL;
> +			return -EINVAL;
> +		}
> 	}
> 
> 	ichan->client = mx3fb;
> @@ -1386,12 +1444,24 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
> 		goto emode;
> 	}
> 
> -	if (mx3fb_pdata->mode && mx3fb_pdata->num_modes) {
> -		mode = mx3fb_pdata->mode;
> -		num_modes = mx3fb_pdata->num_modes;
> +	if (np) {
> +		ret = of_get_fb_videomode(display_np, &of_mode,
> +					  OF_USE_NATIVE_MODE);
> +		if (!ret) {
> +			mode = &of_mode;
> +			num_modes = 1;
> +		} else {
> +			mode = mx3fb_modedb;
> +			num_modes = ARRAY_SIZE(mx3fb_modedb);
> +		}
> 	} else {
> -		mode = mx3fb_modedb;
> -		num_modes = ARRAY_SIZE(mx3fb_modedb);
> +		if (mx3fb_pdata->mode || !mx3fb_pdata->num_modes) {
> +			mode = mx3fb_pdata->mode;
> +			num_modes = mx3fb_pdata->num_modes;
> +		} else {
> +			mode = mx3fb_modedb;
> +			num_modes = ARRAY_SIZE(mx3fb_modedb);
> +		}
> 	}
> 
> 	if (!fb_find_mode(&fbi->var, fbi, fb_mode, mode,
> @@ -1421,7 +1491,8 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
> 	mx3fbi->mx3fb		= mx3fb;
> 	mx3fbi->blank		= FB_BLANK_NORMAL;
> 
> -	mx3fb->disp_data_fmt	= mx3fb_pdata->disp_data_fmt;
> +	if (!np)
> +		mx3fb->disp_data_fmt = mx3fb_pdata->disp_data_fmt;
> 
> 	init_completion(&mx3fbi->flip_cmpl);
> 	disable_irq(ichan->eof_irq);
> @@ -1449,13 +1520,26 @@ emode:
> 	return ret;
> }
> 
> +static int imx_dma_is_dt_ipu(struct dma_chan *chan)
> +{
> +	/* Example: 53fc0000.ipu */
> +	if (chan && chan->device && chan->device->dev) {
> +		char *name = dev_name(chan->device->dev);
> +		name = strchr(name, '.');
> +		if (name)
> +			return !strcmp(name, ".ipu");
> +	}
> +
> +	return 0;
> +}
> +
> static bool chan_filter(struct dma_chan *chan, void *arg)
> {
> 	struct dma_chan_request *rq = arg;
> 	struct device *dev;
> 	struct mx3fb_platform_data *mx3fb_pdata;
> 
> -	if (!imx_dma_is_ipu(chan))
> +	if (!imx_dma_is_ipu(chan) && !imx_dma_is_dt_ipu(chan))
> 		return false;
> 
> 	if (!rq)
> @@ -1464,8 +1548,12 @@ static bool chan_filter(struct dma_chan *chan, void *arg)
> 	dev = rq->mx3fb->dev;
> 	mx3fb_pdata = dev_get_platdata(dev);
> 
> -	return rq->id = chan->chan_id &&
> -		mx3fb_pdata->dma_dev = chan->device->dev;
> +	/* When using the devicetree, mx3fb_pdata is NULL */
> +	if (imx_dma_is_dt_ipu(chan))
> +		return rq->id = chan->chan_id;
> +	else
> +		return rq->id = chan->chan_id &&
> +			mx3fb_pdata->dma_dev = chan->device->dev;
> }
> 
> static void release_fbi(struct fb_info *fbi)
> @@ -1565,9 +1653,16 @@ static int mx3fb_remove(struct platform_device *dev)
> 	return 0;
> }
> 
> +static struct of_device_id mx3fb_of_dev_id[] = {
> +	{ .compatible = "fsl,mx3-fb", },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, mx3fb_of_dev_id);
> +
> static struct platform_driver mx3fb_driver = {
> 	.driver = {
> 		.name = MX3FB_NAME,
> +		.of_match_table = mx3fb_of_dev_id,
> 		.owner = THIS_MODULE,
> 	},
> 	.probe = mx3fb_probe,
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCHv3][ 3/5] video: mx3fb: Add device tree suport.
@ 2013-10-26  6:40         ` Kumar Gala
  0 siblings, 0 replies; 29+ messages in thread
From: Kumar Gala @ 2013-10-26  6:40 UTC (permalink / raw)
  To: linux-arm-kernel


On Oct 23, 2013, at 7:43 AM, Denis Carikli wrote:

> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev at vger.kernel.org
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: devicetree at vger.kernel.org
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: Eric B?nard <eric@eukrea.com>
> Signed-off-by: Denis Carikli <denis@eukrea.com>
> ---
> ChangeLog v2->v3:
> - The device tree bindings were reworked in order to make it look more like the
>  IPUv3 bindings.
> - The interface_pix_fmt property now looks like the IPUv3 one.
> ---
> .../devicetree/bindings/video/fsl,mx3-fb.txt       |   35 ++++++
> drivers/video/Kconfig                              |    2 +
> drivers/video/mx3fb.c                              |  125 +++++++++++++++++---
> 3 files changed, 147 insertions(+), 15 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> 
> diff --git a/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> new file mode 100644
> index 0000000..0b31374
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> @@ -0,0 +1,35 @@
> +Freescale MX3 fb

Can you spell out framebuffer here instead of fb

> +================
> +
> +Required properties:
> +- compatible: Should be "fsl,mx3fb". compatible chips include the imx31 and the
> +  imx35.
> +- reg: should be register base and length as documented in the datasheet.
> +- clocks: Handle to the ipu_gate clock.
> +
> +Example:
> +
> +lcdc: mx3fb at 53fc00b4 {
> +	compatible = "fsl,mx3-fb";
> +	reg = <0x53fc00b4 0x0b>;
> +	clocks = <&clks 55>;
> +};
> +
> +Display support
> +===============
> +Required properties:
> +- model : The user-visible name of the display.
> +
> +Optional properties:
> +- interface_pix_fmt: How this display is connected to the
> +  crtc. Currently supported types: "rgb24", "rgb565", "rgb666"


Why is there no compatible for the display?

> +
> +It can also have an optional timing subnode as described in
> +  Documentation/devicetree/bindings/video/display-timing.txt.
> +
> +Example:
> +
> +display at di0 {
> +	    interface-pix-fmt = "rgb666";
> +	    model = "CMO-QVGA";
> +};

how do you relate the display to the framebuffer?

- k

> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 14317b7..2a638df 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -2359,6 +2359,8 @@ config FB_MX3
> 	select FB_CFB_FILLRECT
> 	select FB_CFB_COPYAREA
> 	select FB_CFB_IMAGEBLIT
> +	select VIDEOMODE_HELPERS
> +	select FB_MODE_HELPERS
> 	default y
> 	help
> 	  This is a framebuffer device for the i.MX31 LCD Controller. So
> diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
> index 804f874..de5a6c8 100644
> --- a/drivers/video/mx3fb.c
> +++ b/drivers/video/mx3fb.c
> @@ -31,6 +31,8 @@
> #include <linux/platform_data/dma-imx.h>
> #include <linux/platform_data/video-mx3fb.h>
> 
> +#include <video/of_display_timing.h>
> +
> #include <asm/io.h>
> #include <asm/uaccess.h>
> 
> @@ -757,11 +759,13 @@ static int __set_par(struct fb_info *fbi, bool lock)
> 			sig_cfg.Hsync_pol = true;
> 		if (fbi->var.sync & FB_SYNC_VERT_HIGH_ACT)
> 			sig_cfg.Vsync_pol = true;
> -		if (fbi->var.sync & FB_SYNC_CLK_INVERT)
> +		if ((fbi->var.sync & FB_SYNC_CLK_INVERT) ||
> +		    (fbi->var.sync & FB_SYNC_PIXDAT_HIGH_ACT))
> 			sig_cfg.clk_pol = true;
> 		if (fbi->var.sync & FB_SYNC_DATA_INVERT)
> 			sig_cfg.data_pol = true;
> -		if (fbi->var.sync & FB_SYNC_OE_ACT_HIGH)
> +		if ((fbi->var.sync & FB_SYNC_OE_ACT_HIGH) ||
> +		    (fbi->var.sync & FB_SYNC_DE_HIGH_ACT))
> 			sig_cfg.enable_pol = true;
> 		if (fbi->var.sync & FB_SYNC_CLK_IDLE_EN)
> 			sig_cfg.clkidle_en = true;
> @@ -1351,21 +1355,75 @@ static struct fb_info *mx3fb_init_fbinfo(struct device *dev, struct fb_ops *ops)
> 	return fbi;
> }
> 
> +static int match_dt_disp_data(const char *property)
> +{
> +	if (!strcmp("rgb666", property))
> +		return IPU_DISP_DATA_MAPPING_RGB666;
> +	else if (!strcmp("rgb565", property))
> +		return IPU_DISP_DATA_MAPPING_RGB565;
> +	else if (!strcmp("rgb24", property))
> +		return IPU_DISP_DATA_MAPPING_RGB888;
> +	else
> +		return -EINVAL;
> +}
> +
> static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
> {
> 	struct device *dev = mx3fb->dev;
> 	struct mx3fb_platform_data *mx3fb_pdata = dev_get_platdata(dev);
> -	const char *name = mx3fb_pdata->name;
> +	struct device_node *np = dev->of_node;
> +	const char *name;
> +	const char *ipu_disp_format;
> 	unsigned int irq;
> 	struct fb_info *fbi;
> 	struct mx3fb_info *mx3fbi;
> 	const struct fb_videomode *mode;
> 	int ret, num_modes;
> +	struct fb_videomode of_mode;
> +	struct device_node *display_np, *root_np;
> +
> +	if (np) {
> +		root_np = of_find_node_by_path("/");
> +		if (!root_np) {
> +			dev_err(dev, "Can't get the \"/\" node.\n");
> +			return -EINVAL;
> +		}
> +
> +		display_np = of_find_node_by_name(root_np, "display");
> +		if (!display_np) {
> +			dev_err(dev, "Can't get the display device node.\n");
> +			return -EINVAL;
> +		}
> +
> +		of_property_read_string(display_np, "interface-pix-fmt",
> +					&ipu_disp_format);
> +		if (!ipu_disp_format) {
> +			mx3fb->disp_data_fmt = IPU_DISP_DATA_MAPPING_RGB666;
> +			dev_warn(dev,
> +				"ipu display data mapping was not defined, using the default rgb666.\n");
> +		} else {
> +			mx3fb->disp_data_fmt =
> +				match_dt_disp_data(ipu_disp_format);
> +		}
> +
> +		if (mx3fb->disp_data_fmt == -EINVAL) {
> +			dev_err(dev, "Illegal display data format \"%s\"\n",
> +				ipu_disp_format);
> +			return -EINVAL;
> +		}
> 
> -	if (mx3fb_pdata->disp_data_fmt >= ARRAY_SIZE(di_mappings)) {
> -		dev_err(dev, "Illegal display data format %d\n",
> +		of_property_read_string(display_np, "model", &name);
> +		if (ret) {
> +			dev_err(dev, "Missing display model name\n");
> +			return -EINVAL;
> +		}
> +	} else {
> +		name = mx3fb_pdata->name;
> +		if (mx3fb_pdata->disp_data_fmt >= ARRAY_SIZE(di_mappings)) {
> +			dev_err(dev, "Illegal display data format %d\n",
> 				mx3fb_pdata->disp_data_fmt);
> -		return -EINVAL;
> +			return -EINVAL;
> +		}
> 	}
> 
> 	ichan->client = mx3fb;
> @@ -1386,12 +1444,24 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
> 		goto emode;
> 	}
> 
> -	if (mx3fb_pdata->mode && mx3fb_pdata->num_modes) {
> -		mode = mx3fb_pdata->mode;
> -		num_modes = mx3fb_pdata->num_modes;
> +	if (np) {
> +		ret = of_get_fb_videomode(display_np, &of_mode,
> +					  OF_USE_NATIVE_MODE);
> +		if (!ret) {
> +			mode = &of_mode;
> +			num_modes = 1;
> +		} else {
> +			mode = mx3fb_modedb;
> +			num_modes = ARRAY_SIZE(mx3fb_modedb);
> +		}
> 	} else {
> -		mode = mx3fb_modedb;
> -		num_modes = ARRAY_SIZE(mx3fb_modedb);
> +		if (mx3fb_pdata->mode || !mx3fb_pdata->num_modes) {
> +			mode = mx3fb_pdata->mode;
> +			num_modes = mx3fb_pdata->num_modes;
> +		} else {
> +			mode = mx3fb_modedb;
> +			num_modes = ARRAY_SIZE(mx3fb_modedb);
> +		}
> 	}
> 
> 	if (!fb_find_mode(&fbi->var, fbi, fb_mode, mode,
> @@ -1421,7 +1491,8 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
> 	mx3fbi->mx3fb		= mx3fb;
> 	mx3fbi->blank		= FB_BLANK_NORMAL;
> 
> -	mx3fb->disp_data_fmt	= mx3fb_pdata->disp_data_fmt;
> +	if (!np)
> +		mx3fb->disp_data_fmt = mx3fb_pdata->disp_data_fmt;
> 
> 	init_completion(&mx3fbi->flip_cmpl);
> 	disable_irq(ichan->eof_irq);
> @@ -1449,13 +1520,26 @@ emode:
> 	return ret;
> }
> 
> +static int imx_dma_is_dt_ipu(struct dma_chan *chan)
> +{
> +	/* Example: 53fc0000.ipu */
> +	if (chan && chan->device && chan->device->dev) {
> +		char *name = dev_name(chan->device->dev);
> +		name = strchr(name, '.');
> +		if (name)
> +			return !strcmp(name, ".ipu");
> +	}
> +
> +	return 0;
> +}
> +
> static bool chan_filter(struct dma_chan *chan, void *arg)
> {
> 	struct dma_chan_request *rq = arg;
> 	struct device *dev;
> 	struct mx3fb_platform_data *mx3fb_pdata;
> 
> -	if (!imx_dma_is_ipu(chan))
> +	if (!imx_dma_is_ipu(chan) && !imx_dma_is_dt_ipu(chan))
> 		return false;
> 
> 	if (!rq)
> @@ -1464,8 +1548,12 @@ static bool chan_filter(struct dma_chan *chan, void *arg)
> 	dev = rq->mx3fb->dev;
> 	mx3fb_pdata = dev_get_platdata(dev);
> 
> -	return rq->id == chan->chan_id &&
> -		mx3fb_pdata->dma_dev == chan->device->dev;
> +	/* When using the devicetree, mx3fb_pdata is NULL */
> +	if (imx_dma_is_dt_ipu(chan))
> +		return rq->id == chan->chan_id;
> +	else
> +		return rq->id == chan->chan_id &&
> +			mx3fb_pdata->dma_dev == chan->device->dev;
> }
> 
> static void release_fbi(struct fb_info *fbi)
> @@ -1565,9 +1653,16 @@ static int mx3fb_remove(struct platform_device *dev)
> 	return 0;
> }
> 
> +static struct of_device_id mx3fb_of_dev_id[] = {
> +	{ .compatible = "fsl,mx3-fb", },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, mx3fb_of_dev_id);
> +
> static struct platform_driver mx3fb_driver = {
> 	.driver = {
> 		.name = MX3FB_NAME,
> +		.of_match_table = mx3fb_of_dev_id,
> 		.owner = THIS_MODULE,
> 	},
> 	.probe = mx3fb_probe,
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv3][ 3/5] video: mx3fb: Add device tree suport.
  2013-10-26  0:18           ` Sascha Hauer
  (?)
@ 2013-10-26  6:43               ` Kumar Gala
  -1 siblings, 0 replies; 29+ messages in thread
From: Kumar Gala @ 2013-10-26  6:43 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Grant Likely, Denis Carikli, Sascha Hauer, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Pawel Moll, Stephen Warren,
	Ian Campbell, Rob Herring, Tomi Valkeinen, Eric Bénard,
	Shawn Guo, Jean-Christophe Plagniol-Villard,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r


On Oct 25, 2013, at 7:18 PM, Sascha Hauer wrote:

> On Fri, Oct 25, 2013 at 08:50:40PM +0100, Grant Likely wrote:
>> On Wed, 23 Oct 2013 14:43:47 +0200, Denis Carikli <denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org> wrote:
>>> Cc: Jean-Christophe Plagniol-Villard <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
>>> Cc: Tomi Valkeinen <tomi.valkeinen-l0cyMroinI0@public.gmane.org>
>>> Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
>>> Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
>>> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
>>> Cc: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
>>> Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
>>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> Cc: Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>>> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>>> Cc: Eric Bénard <eric-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
>>> Signed-off-by: Denis Carikli <denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
>>> ---
>>> ChangeLog v2->v3:
>>> - The device tree bindings were reworked in order to make it look more like the
>>>  IPUv3 bindings.
>>> - The interface_pix_fmt property now looks like the IPUv3 one.
>>> ---
>>> .../devicetree/bindings/video/fsl,mx3-fb.txt       |   35 ++++++
>>> drivers/video/Kconfig                              |    2 +
>>> drivers/video/mx3fb.c                              |  125 +++++++++++++++++---
>>> 3 files changed, 147 insertions(+), 15 deletions(-)
>>> create mode 100644 Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
>>> 
>>> diff --git a/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
>>> new file mode 100644
>>> index 0000000..0b31374
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
>>> @@ -0,0 +1,35 @@
>>> +Freescale MX3 fb
>>> +================
>>> +
>>> +Required properties:
>>> +- compatible: Should be "fsl,mx3fb". compatible chips include the imx31 and the
>>> +  imx35.
>>> +- reg: should be register base and length as documented in the datasheet.
>>> +- clocks: Handle to the ipu_gate clock.
>>> +
>>> +Example:
>>> +
>>> +lcdc: mx3fb@53fc00b4 {
>>> +	compatible = "fsl,mx3-fb";
>>> +	reg = <0x53fc00b4 0x0b>;
>>> +	clocks = <&clks 55>;
>>> +};
>> 
>> This (and some of the other bindings) are trivial, and they are all
>> associated with a single SoC. I think it would be better to collect all
>> the mx3 bindings into a single file rather than distributing them all
>> over the bindings tree.
>> 
>> I started thinking about this after some of the DT conversations in
>> Edinburgh this week. Unless there is a high likelyhood of components
>> being used separately, I think it is far more useful to collect all the
>> bindings for an SoC into a single file. It will certainly reduce a lot
>> of the boilerplate that we've been collecting in bindings documentation
>> files.
>> 
>> A long time ago I took that approach for the mpc5200 documentation[1].
>> Take a look at that organization and let me know what you think.
> 
> I don't think this is a good idea. When a new SoC comes out we don't
> know which components will be reused on the next SoC. This will cause a
> lot of bikeshedding when it actually is reused and then has to be moved.
> 
> Also I would find it quite inconsistent if I had to lookup some devices
> in a SoC file and most bindings in subsystem specific files. So when
> searching for a binding I would first have to know if the hardware is
> unique to the SoC or not.

I agree that as new SoC come out its better to have these things split out.  Also for IP that is sold by vendors like Synopsys/Designware that get used on a lot of SoCs its makes it easier to ensure consistency by having the binding split out for the IP and not the SoC.

Also, by having bindings for similar devices for different SoCs kept together it becomes easier for us to see patterns across SoCs as we might come up with a more generic binding in the future.  Far more difficult if things are SoC oriented.

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv3][ 3/5] video: mx3fb: Add device tree suport.
@ 2013-10-26  6:43               ` Kumar Gala
  0 siblings, 0 replies; 29+ messages in thread
From: Kumar Gala @ 2013-10-26  6:43 UTC (permalink / raw)
  To: linux-arm-kernel


On Oct 25, 2013, at 7:18 PM, Sascha Hauer wrote:

> On Fri, Oct 25, 2013 at 08:50:40PM +0100, Grant Likely wrote:
>> On Wed, 23 Oct 2013 14:43:47 +0200, Denis Carikli <denis@eukrea.com> wrote:
>>> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
>>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>> Cc: linux-fbdev@vger.kernel.org
>>> Cc: Rob Herring <rob.herring@calxeda.com>
>>> Cc: Pawel Moll <pawel.moll@arm.com>
>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>> Cc: Stephen Warren <swarren@wwwdotorg.org>
>>> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
>>> Cc: devicetree@vger.kernel.org
>>> Cc: Sascha Hauer <kernel@pengutronix.de>
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Cc: Eric Bénard <eric@eukrea.com>
>>> Signed-off-by: Denis Carikli <denis@eukrea.com>
>>> ---
>>> ChangeLog v2->v3:
>>> - The device tree bindings were reworked in order to make it look more like the
>>>  IPUv3 bindings.
>>> - The interface_pix_fmt property now looks like the IPUv3 one.
>>> ---
>>> .../devicetree/bindings/video/fsl,mx3-fb.txt       |   35 ++++++
>>> drivers/video/Kconfig                              |    2 +
>>> drivers/video/mx3fb.c                              |  125 +++++++++++++++++---
>>> 3 files changed, 147 insertions(+), 15 deletions(-)
>>> create mode 100644 Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
>>> 
>>> diff --git a/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
>>> new file mode 100644
>>> index 0000000..0b31374
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
>>> @@ -0,0 +1,35 @@
>>> +Freescale MX3 fb
>>> +========
>>> +
>>> +Required properties:
>>> +- compatible: Should be "fsl,mx3fb". compatible chips include the imx31 and the
>>> +  imx35.
>>> +- reg: should be register base and length as documented in the datasheet.
>>> +- clocks: Handle to the ipu_gate clock.
>>> +
>>> +Example:
>>> +
>>> +lcdc: mx3fb@53fc00b4 {
>>> +	compatible = "fsl,mx3-fb";
>>> +	reg = <0x53fc00b4 0x0b>;
>>> +	clocks = <&clks 55>;
>>> +};
>> 
>> This (and some of the other bindings) are trivial, and they are all
>> associated with a single SoC. I think it would be better to collect all
>> the mx3 bindings into a single file rather than distributing them all
>> over the bindings tree.
>> 
>> I started thinking about this after some of the DT conversations in
>> Edinburgh this week. Unless there is a high likelyhood of components
>> being used separately, I think it is far more useful to collect all the
>> bindings for an SoC into a single file. It will certainly reduce a lot
>> of the boilerplate that we've been collecting in bindings documentation
>> files.
>> 
>> A long time ago I took that approach for the mpc5200 documentation[1].
>> Take a look at that organization and let me know what you think.
> 
> I don't think this is a good idea. When a new SoC comes out we don't
> know which components will be reused on the next SoC. This will cause a
> lot of bikeshedding when it actually is reused and then has to be moved.
> 
> Also I would find it quite inconsistent if I had to lookup some devices
> in a SoC file and most bindings in subsystem specific files. So when
> searching for a binding I would first have to know if the hardware is
> unique to the SoC or not.

I agree that as new SoC come out its better to have these things split out.  Also for IP that is sold by vendors like Synopsys/Designware that get used on a lot of SoCs its makes it easier to ensure consistency by having the binding split out for the IP and not the SoC.

Also, by having bindings for similar devices for different SoCs kept together it becomes easier for us to see patterns across SoCs as we might come up with a more generic binding in the future.  Far more difficult if things are SoC oriented.

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCHv3][ 3/5] video: mx3fb: Add device tree suport.
@ 2013-10-26  6:43               ` Kumar Gala
  0 siblings, 0 replies; 29+ messages in thread
From: Kumar Gala @ 2013-10-26  6:43 UTC (permalink / raw)
  To: linux-arm-kernel


On Oct 25, 2013, at 7:18 PM, Sascha Hauer wrote:

> On Fri, Oct 25, 2013 at 08:50:40PM +0100, Grant Likely wrote:
>> On Wed, 23 Oct 2013 14:43:47 +0200, Denis Carikli <denis@eukrea.com> wrote:
>>> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
>>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>> Cc: linux-fbdev at vger.kernel.org
>>> Cc: Rob Herring <rob.herring@calxeda.com>
>>> Cc: Pawel Moll <pawel.moll@arm.com>
>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>> Cc: Stephen Warren <swarren@wwwdotorg.org>
>>> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
>>> Cc: devicetree at vger.kernel.org
>>> Cc: Sascha Hauer <kernel@pengutronix.de>
>>> Cc: linux-arm-kernel at lists.infradead.org
>>> Cc: Eric B?nard <eric@eukrea.com>
>>> Signed-off-by: Denis Carikli <denis@eukrea.com>
>>> ---
>>> ChangeLog v2->v3:
>>> - The device tree bindings were reworked in order to make it look more like the
>>>  IPUv3 bindings.
>>> - The interface_pix_fmt property now looks like the IPUv3 one.
>>> ---
>>> .../devicetree/bindings/video/fsl,mx3-fb.txt       |   35 ++++++
>>> drivers/video/Kconfig                              |    2 +
>>> drivers/video/mx3fb.c                              |  125 +++++++++++++++++---
>>> 3 files changed, 147 insertions(+), 15 deletions(-)
>>> create mode 100644 Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
>>> 
>>> diff --git a/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
>>> new file mode 100644
>>> index 0000000..0b31374
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
>>> @@ -0,0 +1,35 @@
>>> +Freescale MX3 fb
>>> +================
>>> +
>>> +Required properties:
>>> +- compatible: Should be "fsl,mx3fb". compatible chips include the imx31 and the
>>> +  imx35.
>>> +- reg: should be register base and length as documented in the datasheet.
>>> +- clocks: Handle to the ipu_gate clock.
>>> +
>>> +Example:
>>> +
>>> +lcdc: mx3fb at 53fc00b4 {
>>> +	compatible = "fsl,mx3-fb";
>>> +	reg = <0x53fc00b4 0x0b>;
>>> +	clocks = <&clks 55>;
>>> +};
>> 
>> This (and some of the other bindings) are trivial, and they are all
>> associated with a single SoC. I think it would be better to collect all
>> the mx3 bindings into a single file rather than distributing them all
>> over the bindings tree.
>> 
>> I started thinking about this after some of the DT conversations in
>> Edinburgh this week. Unless there is a high likelyhood of components
>> being used separately, I think it is far more useful to collect all the
>> bindings for an SoC into a single file. It will certainly reduce a lot
>> of the boilerplate that we've been collecting in bindings documentation
>> files.
>> 
>> A long time ago I took that approach for the mpc5200 documentation[1].
>> Take a look at that organization and let me know what you think.
> 
> I don't think this is a good idea. When a new SoC comes out we don't
> know which components will be reused on the next SoC. This will cause a
> lot of bikeshedding when it actually is reused and then has to be moved.
> 
> Also I would find it quite inconsistent if I had to lookup some devices
> in a SoC file and most bindings in subsystem specific files. So when
> searching for a binding I would first have to know if the hardware is
> unique to the SoC or not.

I agree that as new SoC come out its better to have these things split out.  Also for IP that is sold by vendors like Synopsys/Designware that get used on a lot of SoCs its makes it easier to ensure consistency by having the binding split out for the IP and not the SoC.

Also, by having bindings for similar devices for different SoCs kept together it becomes easier for us to see patterns across SoCs as we might come up with a more generic binding in the future.  Far more difficult if things are SoC oriented.

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv3][ 3/5] video: mx3fb: Add device tree suport.
  2013-10-26  6:43               ` Kumar Gala
@ 2013-10-27 13:56                 ` Grant Likely
  -1 siblings, 0 replies; 29+ messages in thread
From: Grant Likely @ 2013-10-27 13:56 UTC (permalink / raw)
  To: Kumar Gala, Sascha Hauer
  Cc: Mark Rutland, devicetree, linux-fbdev, Pawel Moll,
	Stephen Warren, Ian Campbell, Rob Herring, Denis Carikli,
	Tomi Valkeinen, Sascha Hauer, Shawn Guo,
	Jean-Christophe Plagniol-Villard, linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 4762 bytes --]

On Sat, 26 Oct 2013 01:43:23 -0500, Kumar Gala <galak@codeaurora.org> wrote:
> 
> On Oct 25, 2013, at 7:18 PM, Sascha Hauer wrote:
> 
> > On Fri, Oct 25, 2013 at 08:50:40PM +0100, Grant Likely wrote:
> >> On Wed, 23 Oct 2013 14:43:47 +0200, Denis Carikli <denis@eukrea.com> wrote:
> >>> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> >>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> >>> Cc: linux-fbdev@vger.kernel.org
> >>> Cc: Rob Herring <rob.herring@calxeda.com>
> >>> Cc: Pawel Moll <pawel.moll@arm.com>
> >>> Cc: Mark Rutland <mark.rutland@arm.com>
> >>> Cc: Stephen Warren <swarren@wwwdotorg.org>
> >>> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> >>> Cc: devicetree@vger.kernel.org
> >>> Cc: Sascha Hauer <kernel@pengutronix.de>
> >>> Cc: linux-arm-kernel@lists.infradead.org
> >>> Cc: Eric Bénard <eric@eukrea.com>
> >>> Signed-off-by: Denis Carikli <denis@eukrea.com>
> >>> ---
> >>> ChangeLog v2->v3:
> >>> - The device tree bindings were reworked in order to make it look more like the
> >>>  IPUv3 bindings.
> >>> - The interface_pix_fmt property now looks like the IPUv3 one.
> >>> ---
> >>> .../devicetree/bindings/video/fsl,mx3-fb.txt       |   35 ++++++
> >>> drivers/video/Kconfig                              |    2 +
> >>> drivers/video/mx3fb.c                              |  125 +++++++++++++++++---
> >>> 3 files changed, 147 insertions(+), 15 deletions(-)
> >>> create mode 100644 Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> >>> 
> >>> diff --git a/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> >>> new file mode 100644
> >>> index 0000000..0b31374
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> >>> @@ -0,0 +1,35 @@
> >>> +Freescale MX3 fb
> >>> +================
> >>> +
> >>> +Required properties:
> >>> +- compatible: Should be "fsl,mx3fb". compatible chips include the imx31 and the
> >>> +  imx35.
> >>> +- reg: should be register base and length as documented in the datasheet.
> >>> +- clocks: Handle to the ipu_gate clock.
> >>> +
> >>> +Example:
> >>> +
> >>> +lcdc: mx3fb@53fc00b4 {
> >>> +	compatible = "fsl,mx3-fb";
> >>> +	reg = <0x53fc00b4 0x0b>;
> >>> +	clocks = <&clks 55>;
> >>> +};
> >> 
> >> This (and some of the other bindings) are trivial, and they are all
> >> associated with a single SoC. I think it would be better to collect all
> >> the mx3 bindings into a single file rather than distributing them all
> >> over the bindings tree.
> >> 
> >> I started thinking about this after some of the DT conversations in
> >> Edinburgh this week. Unless there is a high likelyhood of components
> >> being used separately, I think it is far more useful to collect all the
> >> bindings for an SoC into a single file. It will certainly reduce a lot
> >> of the boilerplate that we've been collecting in bindings documentation
> >> files.
> >> 
> >> A long time ago I took that approach for the mpc5200 documentation[1].
> >> Take a look at that organization and let me know what you think.
> > 
> > I don't think this is a good idea. When a new SoC comes out we don't
> > know which components will be reused on the next SoC. This will cause a
> > lot of bikeshedding when it actually is reused and then has to be moved.
> > 
> > Also I would find it quite inconsistent if I had to lookup some devices
> > in a SoC file and most bindings in subsystem specific files. So when
> > searching for a binding I would first have to know if the hardware is
> > unique to the SoC or not.
> 
> I agree that as new SoC come out its better to have these things split
> out.  Also for IP that is sold by vendors like Synopsys/Designware
> that get used on a lot of SoCs its makes it easier to ensure
> consistency by having the binding split out for the IP and not the
> SoC.

After looking at how many files we're now creating under
Documentation/devicetree/bindings, I'm starting to think we need to go
entirely the other way and put whole the whole family of an SoC into a
single binding file, particularly for the SoC integration level stuff
that isn't likely to be shared with SoCs from other vendors.

A decision on this can be deferred as we're working on schema tools.
Some of that may fall out when we figure out what the structure of the
schema files should be.

> Also, by having bindings for similar devices for different SoCs kept together it becomes easier for us to see patterns across SoCs as we might come up with a more generic binding in the future.  Far more difficult if things are SoC oriented.

The downside is bindings for a single SoC get distributed among a lot of
files which makes it more difficult to follow how the SoC is put
together and configured.

g.



[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCHv3][ 3/5] video: mx3fb: Add device tree suport.
@ 2013-10-27 13:56                 ` Grant Likely
  0 siblings, 0 replies; 29+ messages in thread
From: Grant Likely @ 2013-10-27 13:56 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, 26 Oct 2013 01:43:23 -0500, Kumar Gala <galak@codeaurora.org> wrote:
> 
> On Oct 25, 2013, at 7:18 PM, Sascha Hauer wrote:
> 
> > On Fri, Oct 25, 2013 at 08:50:40PM +0100, Grant Likely wrote:
> >> On Wed, 23 Oct 2013 14:43:47 +0200, Denis Carikli <denis@eukrea.com> wrote:
> >>> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> >>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> >>> Cc: linux-fbdev at vger.kernel.org
> >>> Cc: Rob Herring <rob.herring@calxeda.com>
> >>> Cc: Pawel Moll <pawel.moll@arm.com>
> >>> Cc: Mark Rutland <mark.rutland@arm.com>
> >>> Cc: Stephen Warren <swarren@wwwdotorg.org>
> >>> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> >>> Cc: devicetree at vger.kernel.org
> >>> Cc: Sascha Hauer <kernel@pengutronix.de>
> >>> Cc: linux-arm-kernel at lists.infradead.org
> >>> Cc: Eric B??nard <eric@eukrea.com>
> >>> Signed-off-by: Denis Carikli <denis@eukrea.com>
> >>> ---
> >>> ChangeLog v2->v3:
> >>> - The device tree bindings were reworked in order to make it look more like the
> >>>  IPUv3 bindings.
> >>> - The interface_pix_fmt property now looks like the IPUv3 one.
> >>> ---
> >>> .../devicetree/bindings/video/fsl,mx3-fb.txt       |   35 ++++++
> >>> drivers/video/Kconfig                              |    2 +
> >>> drivers/video/mx3fb.c                              |  125 +++++++++++++++++---
> >>> 3 files changed, 147 insertions(+), 15 deletions(-)
> >>> create mode 100644 Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> >>> 
> >>> diff --git a/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> >>> new file mode 100644
> >>> index 0000000..0b31374
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/video/fsl,mx3-fb.txt
> >>> @@ -0,0 +1,35 @@
> >>> +Freescale MX3 fb
> >>> +================
> >>> +
> >>> +Required properties:
> >>> +- compatible: Should be "fsl,mx3fb". compatible chips include the imx31 and the
> >>> +  imx35.
> >>> +- reg: should be register base and length as documented in the datasheet.
> >>> +- clocks: Handle to the ipu_gate clock.
> >>> +
> >>> +Example:
> >>> +
> >>> +lcdc: mx3fb at 53fc00b4 {
> >>> +	compatible = "fsl,mx3-fb";
> >>> +	reg = <0x53fc00b4 0x0b>;
> >>> +	clocks = <&clks 55>;
> >>> +};
> >> 
> >> This (and some of the other bindings) are trivial, and they are all
> >> associated with a single SoC. I think it would be better to collect all
> >> the mx3 bindings into a single file rather than distributing them all
> >> over the bindings tree.
> >> 
> >> I started thinking about this after some of the DT conversations in
> >> Edinburgh this week. Unless there is a high likelyhood of components
> >> being used separately, I think it is far more useful to collect all the
> >> bindings for an SoC into a single file. It will certainly reduce a lot
> >> of the boilerplate that we've been collecting in bindings documentation
> >> files.
> >> 
> >> A long time ago I took that approach for the mpc5200 documentation[1].
> >> Take a look at that organization and let me know what you think.
> > 
> > I don't think this is a good idea. When a new SoC comes out we don't
> > know which components will be reused on the next SoC. This will cause a
> > lot of bikeshedding when it actually is reused and then has to be moved.
> > 
> > Also I would find it quite inconsistent if I had to lookup some devices
> > in a SoC file and most bindings in subsystem specific files. So when
> > searching for a binding I would first have to know if the hardware is
> > unique to the SoC or not.
> 
> I agree that as new SoC come out its better to have these things split
> out.  Also for IP that is sold by vendors like Synopsys/Designware
> that get used on a lot of SoCs its makes it easier to ensure
> consistency by having the binding split out for the IP and not the
> SoC.

After looking at how many files we're now creating under
Documentation/devicetree/bindings, I'm starting to think we need to go
entirely the other way and put whole the whole family of an SoC into a
single binding file, particularly for the SoC integration level stuff
that isn't likely to be shared with SoCs from other vendors.

A decision on this can be deferred as we're working on schema tools.
Some of that may fall out when we figure out what the structure of the
schema files should be.

> Also, by having bindings for similar devices for different SoCs kept together it becomes easier for us to see patterns across SoCs as we might come up with a more generic binding in the future.  Far more difficult if things are SoC oriented.

The downside is bindings for a single SoC get distributed among a lot of
files which makes it more difficult to follow how the SoC is put
together and configured.

g.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv3][ 1/5] fbdev: Add the lacking FB_SYNC_* for matching the DISPLAY_FLAGS_*
  2013-10-23 12:43 ` Denis Carikli
  (?)
@ 2013-10-29 10:35   ` Tomi Valkeinen
  -1 siblings, 0 replies; 29+ messages in thread
From: Tomi Valkeinen @ 2013-10-29 10:35 UTC (permalink / raw)
  To: Denis Carikli, Sascha Hauer
  Cc: Mark Rutland, devicetree, linux-fbdev, Pawel Moll,
	Stephen Warren, Ian Campbell, Rob Herring, Eric Bénard,
	Shawn Guo, Jean-Christophe Plagniol-Villard, linux-arm-kernel


[-- Attachment #1.1: Type: text/plain, Size: 2393 bytes --]

On 23/10/13 15:43, Denis Carikli wrote:
> Without that fix, drivers using the fb_videomode_from_videomode
>   function will not be able to get certain information because
>   some DISPLAY_FLAGS_* have no corresponding FB_SYNC_*.
> 
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: devicetree@vger.kernel.org
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: Eric Bénard <eric@eukrea.com>
> Signed-off-by: Denis Carikli <denis@eukrea.com>
> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
> ChangeLog v2->v3:
> - Added Jean-Christophe PLAGNIOL-VILLARD's ACK.
> ---
>  drivers/video/fbmon.c   |    4 ++++
>  include/uapi/linux/fb.h |    2 ++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
> index 6103fa6..29a9ed0 100644
> --- a/drivers/video/fbmon.c
> +++ b/drivers/video/fbmon.c
> @@ -1402,6 +1402,10 @@ int fb_videomode_from_videomode(const struct videomode *vm,
>  		fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
>  	if (vm->flags & DISPLAY_FLAGS_VSYNC_HIGH)
>  		fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
> +	if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
> +		fbmode->sync |= FB_SYNC_DE_HIGH_ACT;
> +	if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> +		fbmode->sync |= FB_SYNC_PIXDAT_HIGH_ACT;
>  	if (vm->flags & DISPLAY_FLAGS_INTERLACED)
>  		fbmode->vmode |= FB_VMODE_INTERLACED;
>  	if (vm->flags & DISPLAY_FLAGS_DOUBLESCAN)
> diff --git a/include/uapi/linux/fb.h b/include/uapi/linux/fb.h
> index fb795c3..30487df 100644
> --- a/include/uapi/linux/fb.h
> +++ b/include/uapi/linux/fb.h
> @@ -215,6 +215,8 @@ struct fb_bitfield {
>  					/* vtotal = 144d/288n/576i => PAL  */
>  					/* vtotal = 121d/242n/484i => NTSC */
>  #define FB_SYNC_ON_GREEN	32	/* sync on green */
> +#define FB_SYNC_DE_HIGH_ACT	64	/* data enable high active */
> +#define FB_SYNC_PIXDAT_HIGH_ACT	64	/* data enable high active */

This can't be right. You map both flags to value 64. And the comment is
the same for both.

 Tomi



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCHv3][ 1/5] fbdev: Add the lacking FB_SYNC_* for matching the DISPLAY_FLAGS_*
@ 2013-10-29 10:35   ` Tomi Valkeinen
  0 siblings, 0 replies; 29+ messages in thread
From: Tomi Valkeinen @ 2013-10-29 10:35 UTC (permalink / raw)
  To: linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 2393 bytes --]

On 23/10/13 15:43, Denis Carikli wrote:
> Without that fix, drivers using the fb_videomode_from_videomode
>   function will not be able to get certain information because
>   some DISPLAY_FLAGS_* have no corresponding FB_SYNC_*.
> 
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: devicetree@vger.kernel.org
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: Eric Bénard <eric@eukrea.com>
> Signed-off-by: Denis Carikli <denis@eukrea.com>
> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
> ChangeLog v2->v3:
> - Added Jean-Christophe PLAGNIOL-VILLARD's ACK.
> ---
>  drivers/video/fbmon.c   |    4 ++++
>  include/uapi/linux/fb.h |    2 ++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
> index 6103fa6..29a9ed0 100644
> --- a/drivers/video/fbmon.c
> +++ b/drivers/video/fbmon.c
> @@ -1402,6 +1402,10 @@ int fb_videomode_from_videomode(const struct videomode *vm,
>  		fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
>  	if (vm->flags & DISPLAY_FLAGS_VSYNC_HIGH)
>  		fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
> +	if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
> +		fbmode->sync |= FB_SYNC_DE_HIGH_ACT;
> +	if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> +		fbmode->sync |= FB_SYNC_PIXDAT_HIGH_ACT;
>  	if (vm->flags & DISPLAY_FLAGS_INTERLACED)
>  		fbmode->vmode |= FB_VMODE_INTERLACED;
>  	if (vm->flags & DISPLAY_FLAGS_DOUBLESCAN)
> diff --git a/include/uapi/linux/fb.h b/include/uapi/linux/fb.h
> index fb795c3..30487df 100644
> --- a/include/uapi/linux/fb.h
> +++ b/include/uapi/linux/fb.h
> @@ -215,6 +215,8 @@ struct fb_bitfield {
>  					/* vtotal = 144d/288n/576i => PAL  */
>  					/* vtotal = 121d/242n/484i => NTSC */
>  #define FB_SYNC_ON_GREEN	32	/* sync on green */
> +#define FB_SYNC_DE_HIGH_ACT	64	/* data enable high active */
> +#define FB_SYNC_PIXDAT_HIGH_ACT	64	/* data enable high active */

This can't be right. You map both flags to value 64. And the comment is
the same for both.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCHv3][ 1/5] fbdev: Add the lacking FB_SYNC_* for matching the DISPLAY_FLAGS_*
@ 2013-10-29 10:35   ` Tomi Valkeinen
  0 siblings, 0 replies; 29+ messages in thread
From: Tomi Valkeinen @ 2013-10-29 10:35 UTC (permalink / raw)
  To: linux-arm-kernel

On 23/10/13 15:43, Denis Carikli wrote:
> Without that fix, drivers using the fb_videomode_from_videomode
>   function will not be able to get certain information because
>   some DISPLAY_FLAGS_* have no corresponding FB_SYNC_*.
> 
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev at vger.kernel.org
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: devicetree at vger.kernel.org
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: Eric B?nard <eric@eukrea.com>
> Signed-off-by: Denis Carikli <denis@eukrea.com>
> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
> ChangeLog v2->v3:
> - Added Jean-Christophe PLAGNIOL-VILLARD's ACK.
> ---
>  drivers/video/fbmon.c   |    4 ++++
>  include/uapi/linux/fb.h |    2 ++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
> index 6103fa6..29a9ed0 100644
> --- a/drivers/video/fbmon.c
> +++ b/drivers/video/fbmon.c
> @@ -1402,6 +1402,10 @@ int fb_videomode_from_videomode(const struct videomode *vm,
>  		fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
>  	if (vm->flags & DISPLAY_FLAGS_VSYNC_HIGH)
>  		fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
> +	if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
> +		fbmode->sync |= FB_SYNC_DE_HIGH_ACT;
> +	if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> +		fbmode->sync |= FB_SYNC_PIXDAT_HIGH_ACT;
>  	if (vm->flags & DISPLAY_FLAGS_INTERLACED)
>  		fbmode->vmode |= FB_VMODE_INTERLACED;
>  	if (vm->flags & DISPLAY_FLAGS_DOUBLESCAN)
> diff --git a/include/uapi/linux/fb.h b/include/uapi/linux/fb.h
> index fb795c3..30487df 100644
> --- a/include/uapi/linux/fb.h
> +++ b/include/uapi/linux/fb.h
> @@ -215,6 +215,8 @@ struct fb_bitfield {
>  					/* vtotal = 144d/288n/576i => PAL  */
>  					/* vtotal = 121d/242n/484i => NTSC */
>  #define FB_SYNC_ON_GREEN	32	/* sync on green */
> +#define FB_SYNC_DE_HIGH_ACT	64	/* data enable high active */
> +#define FB_SYNC_PIXDAT_HIGH_ACT	64	/* data enable high active */

This can't be right. You map both flags to value 64. And the comment is
the same for both.

 Tomi


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131029/6985d4b0/attachment.sig>

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2013-10-29 10:35 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-23 12:43 [PATCHv3][ 1/5] fbdev: Add the lacking FB_SYNC_* for matching the DISPLAY_FLAGS_* Denis Carikli
2013-10-23 12:43 ` Denis Carikli
2013-10-23 12:43 ` Denis Carikli
     [not found] ` <1382532229-32755-1-git-send-email-denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
2013-10-23 12:43   ` [PATCHv3][ 2/5] dma: ipu: Add devicetree support Denis Carikli
2013-10-23 12:43     ` Denis Carikli
2013-10-23 12:43   ` [PATCHv3][ 3/5] video: mx3fb: Add device tree suport Denis Carikli
2013-10-23 12:43     ` Denis Carikli
2013-10-23 12:43     ` Denis Carikli
2013-10-25 19:50     ` Grant Likely
2013-10-25 19:50       ` Grant Likely
     [not found]       ` <20131025195040.0CCC3C404DA-WNowdnHR2B42iJbIjFUEsiwD8/FfD2ys@public.gmane.org>
2013-10-26  0:18         ` Sascha Hauer
2013-10-26  0:18           ` Sascha Hauer
2013-10-26  0:18           ` Sascha Hauer
     [not found]           ` <20131026001854.GE17135-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2013-10-26  6:43             ` Kumar Gala
2013-10-26  6:43               ` Kumar Gala
2013-10-26  6:43               ` Kumar Gala
2013-10-27 13:56               ` Grant Likely
2013-10-27 13:56                 ` Grant Likely
     [not found]     ` <1382532229-32755-3-git-send-email-denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
2013-10-26  6:40       ` Kumar Gala
2013-10-26  6:40         ` Kumar Gala
2013-10-26  6:40         ` Kumar Gala
2013-10-23 12:43   ` [PATCHv3][ 5/5] ARM: dts: mbimxsd35 Add video and displays support Denis Carikli
2013-10-23 12:43     ` Denis Carikli
2013-10-23 12:43     ` Denis Carikli
2013-10-23 12:43 ` [PATCHv3][ 4/5] video: mx3fb: Introduce regulator support Denis Carikli
2013-10-23 12:43   ` Denis Carikli
2013-10-29 10:35 ` [PATCHv3][ 1/5] fbdev: Add the lacking FB_SYNC_* for matching the DISPLAY_FLAGS_* Tomi Valkeinen
2013-10-29 10:35   ` Tomi Valkeinen
2013-10-29 10:35   ` Tomi Valkeinen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.