[PATCH v3 0/2] video: imxfb DT support

[PATCH v3 0/2] video: imxfb DT support

[Markus Pargmann]

Hi,

This series adds DT support for imxfb. Changes in v3 are
described in the notes of patch 2.

Regards,

Markus

[PATCH v3 1/2] imxfb: Set alpha value of the framebuffer

[Markus Pargmann]

From: Christian Hemp <c.hemp@phytec.de>

Based on Sascha Hauer's patch i.MX27 clock: Do not disable lcd clocks during
startup.
This patch gives a interface to chance the alphavalue of the framebuffer.

Signed-off-by: Christian Hemp <c.hemp@phytec.de>

rebased to 3.7
Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 drivers/video/imxfb.c                     | 35 +++++++++++++++++++++++++++++++
 include/linux/platform_data/video-imxfb.h |  3 +++
 2 files changed, 38 insertions(+)

diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index 0abf2bf..ef2b587 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -31,6 +31,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/io.h>
 #include <linux/math64.h>
+#include <linux/uaccess.h>
 
 #include <linux/platform_data/video-imxfb.h>
 
@@ -112,6 +113,10 @@
 #define LCDISR_EOF	(1<<1)
 #define LCDISR_BOF	(1<<0)
 
+#define LCDC_LGWCR	0x64
+#define LGWCR_GWAV(alpha)	(((alpha) & 0xff) << 24)
+#define LGWCR_GWE	(1 << 22)
+
 /* Used fb-mode. Can be set on kernel command line, therefore file-static. */
 static const char *fb_mode;
 
@@ -610,6 +615,35 @@ static int imxfb_blank(int blank, struct fb_info *info)
 	return 0;
 }
 
+static int imxfb_ioctl(struct fb_info *info, unsigned int cmd,
+			unsigned long arg)
+{
+	struct imxfb_info *fbi = info->par;
+	int alpha, ret = 0;
+	unsigned int tmp;
+
+	switch (cmd) {
+	case IMXFB_ALPHA:
+		if (get_user(alpha, (int __user *)arg)) {
+			ret = -EFAULT;
+		} else {
+			tmp = readl(fbi->regs + LCDC_LGWCR);
+			tmp &= ~LGWCR_GWAV(0xff);
+			tmp |= LGWCR_GWAV(alpha);
+			if (!alpha)
+				tmp &= ~LGWCR_GWE;
+			else
+				tmp |= LGWCR_GWE;
+			writel(tmp , fbi->regs + LCDC_LGWCR);
+		}
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
 static struct fb_ops imxfb_ops = {
 	.owner		= THIS_MODULE,
 	.fb_check_var	= imxfb_check_var,
@@ -619,6 +653,7 @@ static struct fb_ops imxfb_ops = {
 	.fb_copyarea	= cfb_copyarea,
 	.fb_imageblit	= cfb_imageblit,
 	.fb_blank	= imxfb_blank,
+	.fb_ioctl	= imxfb_ioctl,
 };
 
 /*
diff --git a/include/linux/platform_data/video-imxfb.h b/include/linux/platform_data/video-imxfb.h
index 9de8f06..ce3875f 100644
--- a/include/linux/platform_data/video-imxfb.h
+++ b/include/linux/platform_data/video-imxfb.h
@@ -51,6 +51,9 @@
 #define DMACR_HM(x)	(((x) & 0xf) << 16)
 #define DMACR_TM(x)	((x) & 0xf)
 
+#define IMXFB_IOW(num, dtype)	_IOW('I', num, dtype)
+#define IMXFB_ALPHA		IMXFB_IOW(31, int)
+
 struct imx_fb_videomode {
 	struct fb_videomode mode;
 	u32 pcr;
-- 
1.8.1.5

[PATCH v3 2/2] video: imxfb: Add DT support

[Markus Pargmann]

Add devicetree support for imx framebuffer driver. It uses the generic
display bindings and helper functions.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Mark Rutland <mark.rutland@arm.com>
---
 .../devicetree/bindings/video/fsl,imx-fb.txt       |  49 ++++++
 drivers/video/imxfb.c                              | 192 +++++++++++++++++----
 2 files changed, 207 insertions(+), 34 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/video/fsl,imx-fb.txt

diff --git a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
new file mode 100644
index 0000000..bde9c77
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
@@ -0,0 +1,49 @@
+Freescale imx21 Framebuffer
+
+This framebuffer driver supports devices imx1, imx21, imx25, and imx27.
+
+Required properties:
+- compatible : "fsl,<chip>-fb", chip should be imx1 or imx21
+- reg : Should contain 1 register ranges(address and length)
+- interrupts : One interrupt of the fb dev
+
+Required nodes:
+- display: Phandle to a display node as described in
+	Documentation/devicetree/bindings/video/display-timing.txt
+	Additional, the display node has to define properties:
+	- fsl,bpp: Bits per pixel
+	- fsl,pcr: LCDC PCR value
+
+Optional properties:
+- dmacr-eukrea: Should be set for eukrea boards.
+
+Example:
+
+	imxfb: fb at 10021000 {
+		compatible = "fsl,imx27-fb", "fsl,imx21-fb";
+		interrupts = <61>;
+		reg = <0x10021000 0x1000>;
+		display = <&display0>;
+	};
+
+	...
+
+	display0: display0 {
+		model = "Primeview-PD050VL1";
+		native-mode = <&timing_disp0>;
+		fsl,bpp = <16>;		/* non-standard but required */
+		fsl,pcr = <0xf0c88080>;	/* non-standard but required */
+		display-timings {
+			timing_disp0: 640x480 {
+				hactive = <640>;
+				vactive = <480>;
+				hback-porch = <112>;
+				hfront-porch = <36>;
+				hsync-len = <32>;
+				vback-porch = <33>;
+				vfront-porch = <33>;
+				vsync-len = <2>;
+				clock-frequency = <25000000>;
+			};
+		};
+	};
diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index ef2b587..5a9bc598 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -32,6 +32,12 @@
 #include <linux/io.h>
 #include <linux/math64.h>
 #include <linux/uaccess.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+
+#include <video/of_display_timing.h>
+#include <video/of_videomode.h>
+#include <video/videomode.h>
 
 #include <linux/platform_data/video-imxfb.h>
 
@@ -117,10 +123,13 @@
 #define LGWCR_GWAV(alpha)	(((alpha) & 0xff) << 24)
 #define LGWCR_GWE	(1 << 22)
 
+#define IMXFB_LSCR1_DEFAULT 0x00120300
+#define IMXFB_DMACR_DEFAULT 0x00020010
+#define IMXFB_DMACR_EUKREA_DEFAULT 0x00040060
+
 /* Used fb-mode. Can be set on kernel command line, therefore file-static. */
 static const char *fb_mode;
 
-
 /*
  * These are the bitfields for each
  * display depth that we support.
@@ -192,6 +201,19 @@ static struct platform_device_id imxfb_devtype[] = {
 };
 MODULE_DEVICE_TABLE(platform, imxfb_devtype);
 
+static struct of_device_id imxfb_of_dev_id[] = {
+	{
+		.compatible = "fsl,imx1-fb",
+		.data = &imxfb_devtype[IMX1_FB],
+	}, {
+		.compatible = "fsl,imx21-fb",
+		.data = &imxfb_devtype[IMX21_FB],
+	}, {
+		/* sentinel */
+	}
+};
+MODULE_DEVICE_TABLE(of, imxfb_of_dev_id);
+
 static inline int is_imx1_fb(struct imxfb_info *fbi)
 {
 	return fbi->devtype == IMX1_FB;
@@ -324,6 +346,9 @@ static const struct imx_fb_videomode *imxfb_find_mode(struct imxfb_info *fbi)
 	struct imx_fb_videomode *m;
 	int i;
 
+	if (!fb_mode)
+		return &fbi->mode[0];
+
 	for (i = 0, m = &fbi->mode[0]; i < fbi->num_modes; i++, m++) {
 		if (!strcmp(m->mode.name, fb_mode))
 			return m;
@@ -479,6 +504,9 @@ static int imxfb_bl_update_status(struct backlight_device *bl)
 	struct imxfb_info *fbi = bl_get_data(bl);
 	int brightness = bl->props.brightness;
 
+	if (!fbi->pwmr)
+		return 0;
+
 	if (bl->props.power != FB_BLANK_UNBLANK)
 		brightness = 0;
 	if (bl->props.fb_blank != FB_BLANK_UNBLANK)
@@ -719,7 +747,8 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf
 
 	writel(fbi->pcr, fbi->regs + LCDC_PCR);
 #ifndef PWMR_BACKLIGHT_AVAILABLE
-	writel(fbi->pwmr, fbi->regs + LCDC_PWMR);
+	if (fbi->pwmr)
+		writel(fbi->pwmr, fbi->regs + LCDC_PWMR);
 #endif
 	writel(fbi->lscr1, fbi->regs + LCDC_LSCR1);
 	writel(fbi->dmacr, fbi->regs + LCDC_DMACR);
@@ -758,13 +787,12 @@ static int imxfb_resume(struct platform_device *dev)
 #define imxfb_resume	NULL
 #endif
 
-static int __init imxfb_init_fbinfo(struct platform_device *pdev)
+static int imxfb_init_fbinfo(struct platform_device *pdev)
 {
 	struct imx_fb_platform_data *pdata = pdev->dev.platform_data;
 	struct fb_info *info = dev_get_drvdata(&pdev->dev);
 	struct imxfb_info *fbi = info->par;
-	struct imx_fb_videomode *m;
-	int i;
+	struct device_node *np;
 
 	pr_debug("%s\n",__func__);
 
@@ -795,41 +823,96 @@ static int __init imxfb_init_fbinfo(struct platform_device *pdev)
 	info->fbops			= &imxfb_ops;
 	info->flags			= FBINFO_FLAG_DEFAULT |
 					  FBINFO_READS_FAST;
-	info->var.grayscale		= pdata->cmap_greyscale;
-	fbi->cmap_inverse		= pdata->cmap_inverse;
-	fbi->cmap_static		= pdata->cmap_static;
-	fbi->lscr1			= pdata->lscr1;
-	fbi->dmacr			= pdata->dmacr;
-	fbi->pwmr			= pdata->pwmr;
-	fbi->lcd_power			= pdata->lcd_power;
-	fbi->backlight_power		= pdata->backlight_power;
-
-	for (i = 0, m = &pdata->mode[0]; i < pdata->num_modes; i++, m++)
-		info->fix.smem_len = max_t(size_t, info->fix.smem_len,
-				m->mode.xres * m->mode.yres * m->bpp / 8);
+	if (pdata) {
+		info->var.grayscale		= pdata->cmap_greyscale;
+		fbi->cmap_inverse		= pdata->cmap_inverse;
+		fbi->cmap_static		= pdata->cmap_static;
+		fbi->lscr1			= pdata->lscr1;
+		fbi->dmacr			= pdata->dmacr;
+		fbi->pwmr			= pdata->pwmr;
+		fbi->lcd_power			= pdata->lcd_power;
+		fbi->backlight_power		= pdata->backlight_power;
+	} else {
+		np = pdev->dev.of_node;
+		info->var.grayscale = of_property_read_bool(np,
+						"cmap-greyscale");
+		fbi->cmap_inverse = of_property_read_bool(np, "cmap-inverse");
+		fbi->cmap_static = of_property_read_bool(np, "cmap-static");
+
+		fbi->lscr1 = IMXFB_LSCR1_DEFAULT;
+		if (of_property_read_bool(np, "dmacr-eukrea"))
+			fbi->dmacr = IMXFB_DMACR_EUKREA_DEFAULT;
+		else
+			fbi->dmacr = IMXFB_DMACR_DEFAULT;
+
+		/* These two function pointers could be used by some specific
+		 * platforms. */
+		fbi->lcd_power = NULL;
+		fbi->backlight_power = NULL;
+	}
+
+	return 0;
+}
+
+static int imxfb_of_read_mode(struct device *dev, struct device_node *np,
+		struct imx_fb_videomode *imxfb_mode)
+{
+	int ret;
+	struct fb_videomode *of_mode = &imxfb_mode->mode;
+	u32 bpp;
+	u32 pcr;
+
+	ret = of_property_read_string(np, "model", &of_mode->name);
+	if (ret)
+		of_mode->name = NULL;
+
+	ret = of_get_fb_videomode(np, of_mode, OF_USE_NATIVE_MODE);
+	if (ret) {
+		dev_err(dev, "Failed to get videomode from DT\n");
+		return ret;
+	}
+
+	ret = of_property_read_u32(np, "fsl,bpp", &bpp);
+	ret |= of_property_read_u32(np, "fsl,pcr", &pcr);
+
+	if (ret) {
+		dev_err(dev, "Failed to read bpp and pcr from DT\n");
+		return -EINVAL;
+	}
+
+	if (bpp < 1 || bpp > 255) {
+		dev_err(dev, "Bits per pixel have to be between 1 and 255\n");
+		return -EINVAL;
+	}
+
+	imxfb_mode->bpp = bpp;
+	imxfb_mode->pcr = pcr;
 
 	return 0;
 }
 
-static int __init imxfb_probe(struct platform_device *pdev)
+static int imxfb_probe(struct platform_device *pdev)
 {
 	struct imxfb_info *fbi;
 	struct fb_info *info;
 	struct imx_fb_platform_data *pdata;
 	struct resource *res;
+	struct imx_fb_videomode *m;
+	const struct of_device_id *of_id;
 	int ret, i;
+	int bytes_per_pixel;
 
 	dev_info(&pdev->dev, "i.MX Framebuffer driver\n");
 
+	of_id = of_match_device(imxfb_of_dev_id, &pdev->dev);
+	if (of_id)
+		pdev->id_entry = of_id->data;
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res)
 		return -ENODEV;
 
 	pdata = pdev->dev.platform_data;
-	if (!pdata) {
-		dev_err(&pdev->dev,"No platform_data available\n");
-		return -ENOMEM;
-	}
 
 	info = framebuffer_alloc(sizeof(struct imxfb_info), &pdev->dev);
 	if (!info)
@@ -837,15 +920,55 @@ static int __init imxfb_probe(struct platform_device *pdev)
 
 	fbi = info->par;
 
-	if (!fb_mode)
-		fb_mode = pdata->mode[0].mode.name;
-
 	platform_set_drvdata(pdev, info);
 
 	ret = imxfb_init_fbinfo(pdev);
 	if (ret < 0)
 		goto failed_init;
 
+	if (pdata) {
+		if (!fb_mode)
+			fb_mode = pdata->mode[0].mode.name;
+
+		fbi->mode = pdata->mode;
+		fbi->num_modes = pdata->num_modes;
+	} else {
+		struct device_node *display_np;
+		fb_mode = NULL;
+
+		display_np = of_parse_phandle(pdev->dev.of_node, "display", 0);
+		if (!display_np) {
+			dev_err(&pdev->dev, "No display defined in devicetree\n");
+			ret = -EINVAL;
+			goto failed_of_parse;
+		}
+
+		/*
+		 * imxfb does not support more modes, we choose only the native
+		 * mode.
+		 */
+		fbi->num_modes = 1;
+
+		fbi->mode = devm_kzalloc(&pdev->dev,
+				sizeof(struct imx_fb_videomode), GFP_KERNEL);
+		if (!fbi->mode) {
+			ret = -ENOMEM;
+			goto failed_of_parse;
+		}
+
+		ret = imxfb_of_read_mode(&pdev->dev, display_np, fbi->mode);
+		if (ret)
+			goto failed_of_parse;
+	}
+
+	/* Calculate maximum bytes used per pixel. In most cases this should
+	 * be the same as m->bpp/8 */
+	m = &fbi->mode[0];
+	bytes_per_pixel = (m->bpp + 7) / 8;
+	for (i = 0; i < fbi->num_modes; i++, m++)
+		info->fix.smem_len = max_t(size_t, info->fix.smem_len,
+				m->mode.xres * m->mode.yres * bytes_per_pixel);
+
 	res = request_mem_region(res->start, resource_size(res),
 				DRIVER_NAME);
 	if (!res) {
@@ -878,7 +1001,8 @@ static int __init imxfb_probe(struct platform_device *pdev)
 		goto failed_ioremap;
 	}
 
-	if (!pdata->fixed_screen_cpu) {
+	/* Seems not being used by anyone, so no support for oftree */
+	if (!pdata || !pdata->fixed_screen_cpu) {
 		fbi->map_size = PAGE_ALIGN(info->fix.smem_len);
 		fbi->map_cpu = dma_alloc_writecombine(&pdev->dev,
 				fbi->map_size, &fbi->map_dma, GFP_KERNEL);
@@ -903,18 +1027,16 @@ static int __init imxfb_probe(struct platform_device *pdev)
 		info->fix.smem_start = fbi->screen_dma;
 	}
 
-	if (pdata->init) {
+	if (pdata && pdata->init) {
 		ret = pdata->init(fbi->pdev);
 		if (ret)
 			goto failed_platform_init;
 	}
 
-	fbi->mode = pdata->mode;
-	fbi->num_modes = pdata->num_modes;
 
 	INIT_LIST_HEAD(&info->modelist);
-	for (i = 0; i < pdata->num_modes; i++)
-		fb_add_videomode(&pdata->mode[i].mode, &info->modelist);
+	for (i = 0; i < fbi->num_modes; i++)
+		fb_add_videomode(&fbi->mode[i].mode, &info->modelist);
 
 	/*
 	 * This makes sure that our colour bitfield
@@ -944,10 +1066,10 @@ static int __init imxfb_probe(struct platform_device *pdev)
 failed_register:
 	fb_dealloc_cmap(&info->cmap);
 failed_cmap:
-	if (pdata->exit)
+	if (pdata && pdata->exit)
 		pdata->exit(fbi->pdev);
 failed_platform_init:
-	if (!pdata->fixed_screen_cpu)
+	if (pdata && !pdata->fixed_screen_cpu)
 		dma_free_writecombine(&pdev->dev,fbi->map_size,fbi->map_cpu,
 			fbi->map_dma);
 failed_map:
@@ -956,6 +1078,7 @@ failed_ioremap:
 failed_getclock:
 	release_mem_region(res->start, resource_size(res));
 failed_req:
+failed_of_parse:
 	kfree(info->pseudo_palette);
 failed_init:
 	platform_set_drvdata(pdev, NULL);
@@ -980,7 +1103,7 @@ static int imxfb_remove(struct platform_device *pdev)
 	unregister_framebuffer(info);
 
 	pdata = pdev->dev.platform_data;
-	if (pdata->exit)
+	if (pdata && pdata->exit)
 		pdata->exit(fbi->pdev);
 
 	fb_dealloc_cmap(&info->cmap);
@@ -1009,6 +1132,7 @@ static struct platform_driver imxfb_driver = {
 	.shutdown	= imxfb_shutdown,
 	.driver		= {
 		.name	= DRIVER_NAME,
+		.of_match_table = imxfb_of_dev_id,
 	},
 	.id_table	= imxfb_devtype,
 };
-- 
1.8.1.5

[PATCH v3 2/2] video: imxfb: Add DT support

[Jean-Christophe PLAGNIOL-VILLARD]

On 12:31 Fri 05 Apr     , Markus Pargmann wrote:
> Add devicetree support for imx framebuffer driver. It uses the generic
> display bindings and helper functions.
> 
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> ---
>  .../devicetree/bindings/video/fsl,imx-fb.txt       |  49 ++++++
>  drivers/video/imxfb.c                              | 192 +++++++++++++++++----
>  2 files changed, 207 insertions(+), 34 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> 
> diff --git a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> new file mode 100644
> index 0000000..bde9c77
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> @@ -0,0 +1,49 @@
> +Freescale imx21 Framebuffer
> +
> +This framebuffer driver supports devices imx1, imx21, imx25, and imx27.
> +
> +Required properties:
> +- compatible : "fsl,<chip>-fb", chip should be imx1 or imx21
> +- reg : Should contain 1 register ranges(address and length)
> +- interrupts : One interrupt of the fb dev
> +
> +Required nodes:
> +- display: Phandle to a display node as described in
> +	Documentation/devicetree/bindings/video/display-timing.txt
> +	Additional, the display node has to define properties:
> +	- fsl,bpp: Bits per pixel
> +	- fsl,pcr: LCDC PCR value
> +
> +Optional properties:
> +- dmacr-eukrea: Should be set for eukrea boards.
why ?

and This should be prefix by yhe Vendor here Eukrea
> +
> +Example:
> +
> +	imxfb: fb at 10021000 {
> +		compatible = "fsl,imx27-fb", "fsl,imx21-fb";
> +		interrupts = <61>;
> +		reg = <0x10021000 0x1000>;
> +		display = <&display0>;
> +	};
> +
> +	...
> +
> +	display0: display0 {
> +		model = "Primeview-PD050VL1";
> +		native-mode = <&timing_disp0>;
> +		fsl,bpp = <16>;		/* non-standard but required */
> +		fsl,pcr = <0xf0c88080>;	/* non-standard but required */
> +		display-timings {
> +			timing_disp0: 640x480 {
> +				hactive = <640>;
> +				vactive = <480>;
> +				hback-porch = <112>;
> +				hfront-porch = <36>;
> +				hsync-len = <32>;
> +				vback-porch = <33>;
> +				vfront-porch = <33>;
> +				vsync-len = <2>;
> +				clock-frequency = <25000000>;
> +			};
> +		};
> +	};
> diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> index ef2b587..5a9bc598 100644
> --- a/drivers/video/imxfb.c
> +++ b/drivers/video/imxfb.c
> @@ -32,6 +32,12 @@
>  #include <linux/io.h>
>  #include <linux/math64.h>
>  #include <linux/uaccess.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +
> +#include <video/of_display_timing.h>
> +#include <video/of_videomode.h>
> +#include <video/videomode.h>
>  
>  #include <linux/platform_data/video-imxfb.h>
>  
> @@ -117,10 +123,13 @@
>  #define LGWCR_GWAV(alpha)	(((alpha) & 0xff) << 24)
>  #define LGWCR_GWE	(1 << 22)
>  
> +#define IMXFB_LSCR1_DEFAULT 0x00120300
> +#define IMXFB_DMACR_DEFAULT 0x00020010
> +#define IMXFB_DMACR_EUKREA_DEFAULT 0x00040060
> +
>  /* Used fb-mode. Can be set on kernel command line, therefore file-static. */
>  static const char *fb_mode;
>  
> -
>  /*
>   * These are the bitfields for each
>   * display depth that we support.
> @@ -192,6 +201,19 @@ static struct platform_device_id imxfb_devtype[] = {
>  };
>  MODULE_DEVICE_TABLE(platform, imxfb_devtype);
>  
> +static struct of_device_id imxfb_of_dev_id[] = {
> +	{
> +		.compatible = "fsl,imx1-fb",
> +		.data = &imxfb_devtype[IMX1_FB],
> +	}, {
> +		.compatible = "fsl,imx21-fb",
> +		.data = &imxfb_devtype[IMX21_FB],
> +	}, {
> +		/* sentinel */
> +	}
> +};
> +MODULE_DEVICE_TABLE(of, imxfb_of_dev_id);
> +
>  static inline int is_imx1_fb(struct imxfb_info *fbi)
>  {
>  	return fbi->devtype == IMX1_FB;
> @@ -324,6 +346,9 @@ static const struct imx_fb_videomode *imxfb_find_mode(struct imxfb_info *fbi)
>  	struct imx_fb_videomode *m;
>  	int i;
>  
> +	if (!fb_mode)
> +		return &fbi->mode[0];
> +
>  	for (i = 0, m = &fbi->mode[0]; i < fbi->num_modes; i++, m++) {
>  		if (!strcmp(m->mode.name, fb_mode))
>  			return m;
> @@ -479,6 +504,9 @@ static int imxfb_bl_update_status(struct backlight_device *bl)
>  	struct imxfb_info *fbi = bl_get_data(bl);
>  	int brightness = bl->props.brightness;
>  
> +	if (!fbi->pwmr)
> +		return 0;
> +
>  	if (bl->props.power != FB_BLANK_UNBLANK)
>  		brightness = 0;
>  	if (bl->props.fb_blank != FB_BLANK_UNBLANK)
> @@ -719,7 +747,8 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf
>  
>  	writel(fbi->pcr, fbi->regs + LCDC_PCR);
>  #ifndef PWMR_BACKLIGHT_AVAILABLE
> -	writel(fbi->pwmr, fbi->regs + LCDC_PWMR);
> +	if (fbi->pwmr)
> +		writel(fbi->pwmr, fbi->regs + LCDC_PWMR);
>  #endif
>  	writel(fbi->lscr1, fbi->regs + LCDC_LSCR1);
>  	writel(fbi->dmacr, fbi->regs + LCDC_DMACR);
> @@ -758,13 +787,12 @@ static int imxfb_resume(struct platform_device *dev)
>  #define imxfb_resume	NULL
>  #endif
>  
> -static int __init imxfb_init_fbinfo(struct platform_device *pdev)
> +static int imxfb_init_fbinfo(struct platform_device *pdev)
>  {
>  	struct imx_fb_platform_data *pdata = pdev->dev.platform_data;
>  	struct fb_info *info = dev_get_drvdata(&pdev->dev);
>  	struct imxfb_info *fbi = info->par;
> -	struct imx_fb_videomode *m;
> -	int i;
> +	struct device_node *np;
>  
>  	pr_debug("%s\n",__func__);
>  
> @@ -795,41 +823,96 @@ static int __init imxfb_init_fbinfo(struct platform_device *pdev)
>  	info->fbops			= &imxfb_ops;
>  	info->flags			= FBINFO_FLAG_DEFAULT |
>  					  FBINFO_READS_FAST;
> -	info->var.grayscale		= pdata->cmap_greyscale;
> -	fbi->cmap_inverse		= pdata->cmap_inverse;
> -	fbi->cmap_static		= pdata->cmap_static;
> -	fbi->lscr1			= pdata->lscr1;
> -	fbi->dmacr			= pdata->dmacr;
> -	fbi->pwmr			= pdata->pwmr;
> -	fbi->lcd_power			= pdata->lcd_power;
> -	fbi->backlight_power		= pdata->backlight_power;
> -
> -	for (i = 0, m = &pdata->mode[0]; i < pdata->num_modes; i++, m++)
> -		info->fix.smem_len = max_t(size_t, info->fix.smem_len,
> -				m->mode.xres * m->mode.yres * m->bpp / 8);
> +	if (pdata) {
> +		info->var.grayscale		= pdata->cmap_greyscale;
> +		fbi->cmap_inverse		= pdata->cmap_inverse;
> +		fbi->cmap_static		= pdata->cmap_static;
> +		fbi->lscr1			= pdata->lscr1;
> +		fbi->dmacr			= pdata->dmacr;
> +		fbi->pwmr			= pdata->pwmr;
> +		fbi->lcd_power			= pdata->lcd_power;
> +		fbi->backlight_power		= pdata->backlight_power;
> +	} else {
> +		np = pdev->dev.of_node;
> +		info->var.grayscale = of_property_read_bool(np,
> +						"cmap-greyscale");
> +		fbi->cmap_inverse = of_property_read_bool(np, "cmap-inverse");
> +		fbi->cmap_static = of_property_read_bool(np, "cmap-static");
> +
> +		fbi->lscr1 = IMXFB_LSCR1_DEFAULT;
> +		if (of_property_read_bool(np, "dmacr-eukrea"))
> +			fbi->dmacr = IMXFB_DMACR_EUKREA_DEFAULT;
> +		else
> +			fbi->dmacr = IMXFB_DMACR_DEFAULT;
> +
> +		/* These two function pointers could be used by some specific
> +		 * platforms. */
> +		fbi->lcd_power = NULL;
> +		fbi->backlight_power = NULL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int imxfb_of_read_mode(struct device *dev, struct device_node *np,
> +		struct imx_fb_videomode *imxfb_mode)
> +{
> +	int ret;
> +	struct fb_videomode *of_mode = &imxfb_mode->mode;
> +	u32 bpp;
> +	u32 pcr;
> +
> +	ret = of_property_read_string(np, "model", &of_mode->name);
> +	if (ret)
> +		of_mode->name = NULL;
> +
> +	ret = of_get_fb_videomode(np, of_mode, OF_USE_NATIVE_MODE);
> +	if (ret) {
> +		dev_err(dev, "Failed to get videomode from DT\n");
> +		return ret;
> +	}
> +
> +	ret = of_property_read_u32(np, "fsl,bpp", &bpp);
> +	ret |= of_property_read_u32(np, "fsl,pcr", &pcr);
> +
> +	if (ret) {
> +		dev_err(dev, "Failed to read bpp and pcr from DT\n");
> +		return -EINVAL;
> +	}
> +
> +	if (bpp < 1 || bpp > 255) {
> +		dev_err(dev, "Bits per pixel have to be between 1 and 255\n");
> +		return -EINVAL;
> +	}
> +
> +	imxfb_mode->bpp = bpp;
> +	imxfb_mode->pcr = pcr;
>  
>  	return 0;
>  }
>  
> -static int __init imxfb_probe(struct platform_device *pdev)
> +static int imxfb_probe(struct platform_device *pdev)
>  {
>  	struct imxfb_info *fbi;
>  	struct fb_info *info;
>  	struct imx_fb_platform_data *pdata;
>  	struct resource *res;
> +	struct imx_fb_videomode *m;
> +	const struct of_device_id *of_id;
>  	int ret, i;
> +	int bytes_per_pixel;
>  
>  	dev_info(&pdev->dev, "i.MX Framebuffer driver\n");
>  
> +	of_id = of_match_device(imxfb_of_dev_id, &pdev->dev);
> +	if (of_id)
> +		pdev->id_entry = of_id->data;
> +
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!res)
>  		return -ENODEV;
>  
>  	pdata = pdev->dev.platform_data;
> -	if (!pdata) {
> -		dev_err(&pdev->dev,"No platform_data available\n");
> -		return -ENOMEM;
> -	}
>  
>  	info = framebuffer_alloc(sizeof(struct imxfb_info), &pdev->dev);
>  	if (!info)
> @@ -837,15 +920,55 @@ static int __init imxfb_probe(struct platform_device *pdev)
>  
>  	fbi = info->par;
>  
> -	if (!fb_mode)
> -		fb_mode = pdata->mode[0].mode.name;
> -
>  	platform_set_drvdata(pdev, info);
>  
>  	ret = imxfb_init_fbinfo(pdev);
>  	if (ret < 0)
>  		goto failed_init;
>  
> +	if (pdata) {
> +		if (!fb_mode)
> +			fb_mode = pdata->mode[0].mode.name;
> +
> +		fbi->mode = pdata->mode;
> +		fbi->num_modes = pdata->num_modes;
> +	} else {
> +		struct device_node *display_np;
> +		fb_mode = NULL;
> +
> +		display_np = of_parse_phandle(pdev->dev.of_node, "display", 0);
> +		if (!display_np) {
> +			dev_err(&pdev->dev, "No display defined in devicetree\n");
> +			ret = -EINVAL;
> +			goto failed_of_parse;
> +		}
> +
> +		/*
> +		 * imxfb does not support more modes, we choose only the native
> +		 * mode.
> +		 */
> +		fbi->num_modes = 1;
> +
> +		fbi->mode = devm_kzalloc(&pdev->dev,
> +				sizeof(struct imx_fb_videomode), GFP_KERNEL);
> +		if (!fbi->mode) {
> +			ret = -ENOMEM;
> +			goto failed_of_parse;
> +		}
> +
> +		ret = imxfb_of_read_mode(&pdev->dev, display_np, fbi->mode);
> +		if (ret)
> +			goto failed_of_parse;
> +	}
> +
> +	/* Calculate maximum bytes used per pixel. In most cases this should
> +	 * be the same as m->bpp/8 */
> +	m = &fbi->mode[0];
> +	bytes_per_pixel = (m->bpp + 7) / 8;
> +	for (i = 0; i < fbi->num_modes; i++, m++)
> +		info->fix.smem_len = max_t(size_t, info->fix.smem_len,
> +				m->mode.xres * m->mode.yres * bytes_per_pixel);
> +
>  	res = request_mem_region(res->start, resource_size(res),
>  				DRIVER_NAME);
>  	if (!res) {
> @@ -878,7 +1001,8 @@ static int __init imxfb_probe(struct platform_device *pdev)
>  		goto failed_ioremap;
>  	}
>  
> -	if (!pdata->fixed_screen_cpu) {
> +	/* Seems not being used by anyone, so no support for oftree */
> +	if (!pdata || !pdata->fixed_screen_cpu) {
>  		fbi->map_size = PAGE_ALIGN(info->fix.smem_len);
>  		fbi->map_cpu = dma_alloc_writecombine(&pdev->dev,
>  				fbi->map_size, &fbi->map_dma, GFP_KERNEL);
> @@ -903,18 +1027,16 @@ static int __init imxfb_probe(struct platform_device *pdev)
>  		info->fix.smem_start = fbi->screen_dma;
>  	}
>  
> -	if (pdata->init) {
> +	if (pdata && pdata->init) {
>  		ret = pdata->init(fbi->pdev);
>  		if (ret)
>  			goto failed_platform_init;
>  	}
>  
> -	fbi->mode = pdata->mode;
> -	fbi->num_modes = pdata->num_modes;
>  
>  	INIT_LIST_HEAD(&info->modelist);
> -	for (i = 0; i < pdata->num_modes; i++)
> -		fb_add_videomode(&pdata->mode[i].mode, &info->modelist);
> +	for (i = 0; i < fbi->num_modes; i++)
> +		fb_add_videomode(&fbi->mode[i].mode, &info->modelist);
>  
>  	/*
>  	 * This makes sure that our colour bitfield
> @@ -944,10 +1066,10 @@ static int __init imxfb_probe(struct platform_device *pdev)
>  failed_register:
>  	fb_dealloc_cmap(&info->cmap);
>  failed_cmap:
> -	if (pdata->exit)
> +	if (pdata && pdata->exit)
>  		pdata->exit(fbi->pdev);
>  failed_platform_init:
> -	if (!pdata->fixed_screen_cpu)
> +	if (pdata && !pdata->fixed_screen_cpu)
>  		dma_free_writecombine(&pdev->dev,fbi->map_size,fbi->map_cpu,
>  			fbi->map_dma);
>  failed_map:
> @@ -956,6 +1078,7 @@ failed_ioremap:
>  failed_getclock:
>  	release_mem_region(res->start, resource_size(res));
>  failed_req:
> +failed_of_parse:
>  	kfree(info->pseudo_palette);
>  failed_init:
>  	platform_set_drvdata(pdev, NULL);
> @@ -980,7 +1103,7 @@ static int imxfb_remove(struct platform_device *pdev)
>  	unregister_framebuffer(info);
>  
>  	pdata = pdev->dev.platform_data;
> -	if (pdata->exit)
> +	if (pdata && pdata->exit)
>  		pdata->exit(fbi->pdev);
>  
>  	fb_dealloc_cmap(&info->cmap);
> @@ -1009,6 +1132,7 @@ static struct platform_driver imxfb_driver = {
>  	.shutdown	= imxfb_shutdown,
>  	.driver		= {
>  		.name	= DRIVER_NAME,
> +		.of_match_table = imxfb_of_dev_id,
>  	},
>  	.id_table	= imxfb_devtype,
>  };
> -- 
> 1.8.1.5
> 
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

[PATCH v3 2/2] video: imxfb: Add DT support

[Markus Pargmann]

On Mon, Apr 08, 2013 at 09:57:42AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 12:31 Fri 05 Apr     , Markus Pargmann wrote:
> > Add devicetree support for imx framebuffer driver. It uses the generic
> > display bindings and helper functions.
> > 
> > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> > Cc: Fabio Estevam <festevam@gmail.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > ---
> >  .../devicetree/bindings/video/fsl,imx-fb.txt       |  49 ++++++
> >  drivers/video/imxfb.c                              | 192 +++++++++++++++++----
> >  2 files changed, 207 insertions(+), 34 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > new file mode 100644
> > index 0000000..bde9c77
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > @@ -0,0 +1,49 @@
> > +Freescale imx21 Framebuffer
> > +
> > +This framebuffer driver supports devices imx1, imx21, imx25, and imx27.
> > +
> > +Required properties:
> > +- compatible : "fsl,<chip>-fb", chip should be imx1 or imx21
> > +- reg : Should contain 1 register ranges(address and length)
> > +- interrupts : One interrupt of the fb dev
> > +
> > +Required nodes:
> > +- display: Phandle to a display node as described in
> > +	Documentation/devicetree/bindings/video/display-timing.txt
> > +	Additional, the display node has to define properties:
> > +	- fsl,bpp: Bits per pixel
> > +	- fsl,pcr: LCDC PCR value
> > +
> > +Optional properties:
> > +- dmacr-eukrea: Should be set for eukrea boards.
> why ?

Because eukrea boards have a different dmacr then all other boards using
imxfb. The dmacr address is hardcoded as defaults in the code. I could
also search for the board name in the DT, but there are no eukrea boards
with DT at the moment, so I thought a bool property may be better for
the moment.

> 
> and This should be prefix by yhe Vendor here Eukrea

Okay.

Regards,

Markus

> > +
> > +Example:
> > +
> > +	imxfb: fb at 10021000 {
> > +		compatible = "fsl,imx27-fb", "fsl,imx21-fb";
> > +		interrupts = <61>;
> > +		reg = <0x10021000 0x1000>;
> > +		display = <&display0>;
> > +	};
> > +
> > +	...
> > +
> > +	display0: display0 {
> > +		model = "Primeview-PD050VL1";
> > +		native-mode = <&timing_disp0>;
> > +		fsl,bpp = <16>;		/* non-standard but required */
> > +		fsl,pcr = <0xf0c88080>;	/* non-standard but required */
> > +		display-timings {
> > +			timing_disp0: 640x480 {
> > +				hactive = <640>;
> > +				vactive = <480>;
> > +				hback-porch = <112>;
> > +				hfront-porch = <36>;
> > +				hsync-len = <32>;
> > +				vback-porch = <33>;
> > +				vfront-porch = <33>;
> > +				vsync-len = <2>;
> > +				clock-frequency = <25000000>;
> > +			};
> > +		};
> > +	};
> > diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> > index ef2b587..5a9bc598 100644
> > --- a/drivers/video/imxfb.c
> > +++ b/drivers/video/imxfb.c
> > @@ -32,6 +32,12 @@
> >  #include <linux/io.h>
> >  #include <linux/math64.h>
> >  #include <linux/uaccess.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +
> > +#include <video/of_display_timing.h>
> > +#include <video/of_videomode.h>
> > +#include <video/videomode.h>
> >  
> >  #include <linux/platform_data/video-imxfb.h>
> >  
> > @@ -117,10 +123,13 @@
> >  #define LGWCR_GWAV(alpha)	(((alpha) & 0xff) << 24)
> >  #define LGWCR_GWE	(1 << 22)
> >  
> > +#define IMXFB_LSCR1_DEFAULT 0x00120300
> > +#define IMXFB_DMACR_DEFAULT 0x00020010
> > +#define IMXFB_DMACR_EUKREA_DEFAULT 0x00040060
> > +
> >  /* Used fb-mode. Can be set on kernel command line, therefore file-static. */
> >  static const char *fb_mode;
> >  
> > -
> >  /*
> >   * These are the bitfields for each
> >   * display depth that we support.
> > @@ -192,6 +201,19 @@ static struct platform_device_id imxfb_devtype[] = {
> >  };
> >  MODULE_DEVICE_TABLE(platform, imxfb_devtype);
> >  
> > +static struct of_device_id imxfb_of_dev_id[] = {
> > +	{
> > +		.compatible = "fsl,imx1-fb",
> > +		.data = &imxfb_devtype[IMX1_FB],
> > +	}, {
> > +		.compatible = "fsl,imx21-fb",
> > +		.data = &imxfb_devtype[IMX21_FB],
> > +	}, {
> > +		/* sentinel */
> > +	}
> > +};
> > +MODULE_DEVICE_TABLE(of, imxfb_of_dev_id);
> > +
> >  static inline int is_imx1_fb(struct imxfb_info *fbi)
> >  {
> >  	return fbi->devtype == IMX1_FB;
> > @@ -324,6 +346,9 @@ static const struct imx_fb_videomode *imxfb_find_mode(struct imxfb_info *fbi)
> >  	struct imx_fb_videomode *m;
> >  	int i;
> >  
> > +	if (!fb_mode)
> > +		return &fbi->mode[0];
> > +
> >  	for (i = 0, m = &fbi->mode[0]; i < fbi->num_modes; i++, m++) {
> >  		if (!strcmp(m->mode.name, fb_mode))
> >  			return m;
> > @@ -479,6 +504,9 @@ static int imxfb_bl_update_status(struct backlight_device *bl)
> >  	struct imxfb_info *fbi = bl_get_data(bl);
> >  	int brightness = bl->props.brightness;
> >  
> > +	if (!fbi->pwmr)
> > +		return 0;
> > +
> >  	if (bl->props.power != FB_BLANK_UNBLANK)
> >  		brightness = 0;
> >  	if (bl->props.fb_blank != FB_BLANK_UNBLANK)
> > @@ -719,7 +747,8 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf
> >  
> >  	writel(fbi->pcr, fbi->regs + LCDC_PCR);
> >  #ifndef PWMR_BACKLIGHT_AVAILABLE
> > -	writel(fbi->pwmr, fbi->regs + LCDC_PWMR);
> > +	if (fbi->pwmr)
> > +		writel(fbi->pwmr, fbi->regs + LCDC_PWMR);
> >  #endif
> >  	writel(fbi->lscr1, fbi->regs + LCDC_LSCR1);
> >  	writel(fbi->dmacr, fbi->regs + LCDC_DMACR);
> > @@ -758,13 +787,12 @@ static int imxfb_resume(struct platform_device *dev)
> >  #define imxfb_resume	NULL
> >  #endif
> >  
> > -static int __init imxfb_init_fbinfo(struct platform_device *pdev)
> > +static int imxfb_init_fbinfo(struct platform_device *pdev)
> >  {
> >  	struct imx_fb_platform_data *pdata = pdev->dev.platform_data;
> >  	struct fb_info *info = dev_get_drvdata(&pdev->dev);
> >  	struct imxfb_info *fbi = info->par;
> > -	struct imx_fb_videomode *m;
> > -	int i;
> > +	struct device_node *np;
> >  
> >  	pr_debug("%s\n",__func__);
> >  
> > @@ -795,41 +823,96 @@ static int __init imxfb_init_fbinfo(struct platform_device *pdev)
> >  	info->fbops			= &imxfb_ops;
> >  	info->flags			= FBINFO_FLAG_DEFAULT |
> >  					  FBINFO_READS_FAST;
> > -	info->var.grayscale		= pdata->cmap_greyscale;
> > -	fbi->cmap_inverse		= pdata->cmap_inverse;
> > -	fbi->cmap_static		= pdata->cmap_static;
> > -	fbi->lscr1			= pdata->lscr1;
> > -	fbi->dmacr			= pdata->dmacr;
> > -	fbi->pwmr			= pdata->pwmr;
> > -	fbi->lcd_power			= pdata->lcd_power;
> > -	fbi->backlight_power		= pdata->backlight_power;
> > -
> > -	for (i = 0, m = &pdata->mode[0]; i < pdata->num_modes; i++, m++)
> > -		info->fix.smem_len = max_t(size_t, info->fix.smem_len,
> > -				m->mode.xres * m->mode.yres * m->bpp / 8);
> > +	if (pdata) {
> > +		info->var.grayscale		= pdata->cmap_greyscale;
> > +		fbi->cmap_inverse		= pdata->cmap_inverse;
> > +		fbi->cmap_static		= pdata->cmap_static;
> > +		fbi->lscr1			= pdata->lscr1;
> > +		fbi->dmacr			= pdata->dmacr;
> > +		fbi->pwmr			= pdata->pwmr;
> > +		fbi->lcd_power			= pdata->lcd_power;
> > +		fbi->backlight_power		= pdata->backlight_power;
> > +	} else {
> > +		np = pdev->dev.of_node;
> > +		info->var.grayscale = of_property_read_bool(np,
> > +						"cmap-greyscale");
> > +		fbi->cmap_inverse = of_property_read_bool(np, "cmap-inverse");
> > +		fbi->cmap_static = of_property_read_bool(np, "cmap-static");
> > +
> > +		fbi->lscr1 = IMXFB_LSCR1_DEFAULT;
> > +		if (of_property_read_bool(np, "dmacr-eukrea"))
> > +			fbi->dmacr = IMXFB_DMACR_EUKREA_DEFAULT;
> > +		else
> > +			fbi->dmacr = IMXFB_DMACR_DEFAULT;
> > +
> > +		/* These two function pointers could be used by some specific
> > +		 * platforms. */
> > +		fbi->lcd_power = NULL;
> > +		fbi->backlight_power = NULL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int imxfb_of_read_mode(struct device *dev, struct device_node *np,
> > +		struct imx_fb_videomode *imxfb_mode)
> > +{
> > +	int ret;
> > +	struct fb_videomode *of_mode = &imxfb_mode->mode;
> > +	u32 bpp;
> > +	u32 pcr;
> > +
> > +	ret = of_property_read_string(np, "model", &of_mode->name);
> > +	if (ret)
> > +		of_mode->name = NULL;
> > +
> > +	ret = of_get_fb_videomode(np, of_mode, OF_USE_NATIVE_MODE);
> > +	if (ret) {
> > +		dev_err(dev, "Failed to get videomode from DT\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = of_property_read_u32(np, "fsl,bpp", &bpp);
> > +	ret |= of_property_read_u32(np, "fsl,pcr", &pcr);
> > +
> > +	if (ret) {
> > +		dev_err(dev, "Failed to read bpp and pcr from DT\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (bpp < 1 || bpp > 255) {
> > +		dev_err(dev, "Bits per pixel have to be between 1 and 255\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	imxfb_mode->bpp = bpp;
> > +	imxfb_mode->pcr = pcr;
> >  
> >  	return 0;
> >  }
> >  
> > -static int __init imxfb_probe(struct platform_device *pdev)
> > +static int imxfb_probe(struct platform_device *pdev)
> >  {
> >  	struct imxfb_info *fbi;
> >  	struct fb_info *info;
> >  	struct imx_fb_platform_data *pdata;
> >  	struct resource *res;
> > +	struct imx_fb_videomode *m;
> > +	const struct of_device_id *of_id;
> >  	int ret, i;
> > +	int bytes_per_pixel;
> >  
> >  	dev_info(&pdev->dev, "i.MX Framebuffer driver\n");
> >  
> > +	of_id = of_match_device(imxfb_of_dev_id, &pdev->dev);
> > +	if (of_id)
> > +		pdev->id_entry = of_id->data;
> > +
> >  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >  	if (!res)
> >  		return -ENODEV;
> >  
> >  	pdata = pdev->dev.platform_data;
> > -	if (!pdata) {
> > -		dev_err(&pdev->dev,"No platform_data available\n");
> > -		return -ENOMEM;
> > -	}
> >  
> >  	info = framebuffer_alloc(sizeof(struct imxfb_info), &pdev->dev);
> >  	if (!info)
> > @@ -837,15 +920,55 @@ static int __init imxfb_probe(struct platform_device *pdev)
> >  
> >  	fbi = info->par;
> >  
> > -	if (!fb_mode)
> > -		fb_mode = pdata->mode[0].mode.name;
> > -
> >  	platform_set_drvdata(pdev, info);
> >  
> >  	ret = imxfb_init_fbinfo(pdev);
> >  	if (ret < 0)
> >  		goto failed_init;
> >  
> > +	if (pdata) {
> > +		if (!fb_mode)
> > +			fb_mode = pdata->mode[0].mode.name;
> > +
> > +		fbi->mode = pdata->mode;
> > +		fbi->num_modes = pdata->num_modes;
> > +	} else {
> > +		struct device_node *display_np;
> > +		fb_mode = NULL;
> > +
> > +		display_np = of_parse_phandle(pdev->dev.of_node, "display", 0);
> > +		if (!display_np) {
> > +			dev_err(&pdev->dev, "No display defined in devicetree\n");
> > +			ret = -EINVAL;
> > +			goto failed_of_parse;
> > +		}
> > +
> > +		/*
> > +		 * imxfb does not support more modes, we choose only the native
> > +		 * mode.
> > +		 */
> > +		fbi->num_modes = 1;
> > +
> > +		fbi->mode = devm_kzalloc(&pdev->dev,
> > +				sizeof(struct imx_fb_videomode), GFP_KERNEL);
> > +		if (!fbi->mode) {
> > +			ret = -ENOMEM;
> > +			goto failed_of_parse;
> > +		}
> > +
> > +		ret = imxfb_of_read_mode(&pdev->dev, display_np, fbi->mode);
> > +		if (ret)
> > +			goto failed_of_parse;
> > +	}
> > +
> > +	/* Calculate maximum bytes used per pixel. In most cases this should
> > +	 * be the same as m->bpp/8 */
> > +	m = &fbi->mode[0];
> > +	bytes_per_pixel = (m->bpp + 7) / 8;
> > +	for (i = 0; i < fbi->num_modes; i++, m++)
> > +		info->fix.smem_len = max_t(size_t, info->fix.smem_len,
> > +				m->mode.xres * m->mode.yres * bytes_per_pixel);
> > +
> >  	res = request_mem_region(res->start, resource_size(res),
> >  				DRIVER_NAME);
> >  	if (!res) {
> > @@ -878,7 +1001,8 @@ static int __init imxfb_probe(struct platform_device *pdev)
> >  		goto failed_ioremap;
> >  	}
> >  
> > -	if (!pdata->fixed_screen_cpu) {
> > +	/* Seems not being used by anyone, so no support for oftree */
> > +	if (!pdata || !pdata->fixed_screen_cpu) {
> >  		fbi->map_size = PAGE_ALIGN(info->fix.smem_len);
> >  		fbi->map_cpu = dma_alloc_writecombine(&pdev->dev,
> >  				fbi->map_size, &fbi->map_dma, GFP_KERNEL);
> > @@ -903,18 +1027,16 @@ static int __init imxfb_probe(struct platform_device *pdev)
> >  		info->fix.smem_start = fbi->screen_dma;
> >  	}
> >  
> > -	if (pdata->init) {
> > +	if (pdata && pdata->init) {
> >  		ret = pdata->init(fbi->pdev);
> >  		if (ret)
> >  			goto failed_platform_init;
> >  	}
> >  
> > -	fbi->mode = pdata->mode;
> > -	fbi->num_modes = pdata->num_modes;
> >  
> >  	INIT_LIST_HEAD(&info->modelist);
> > -	for (i = 0; i < pdata->num_modes; i++)
> > -		fb_add_videomode(&pdata->mode[i].mode, &info->modelist);
> > +	for (i = 0; i < fbi->num_modes; i++)
> > +		fb_add_videomode(&fbi->mode[i].mode, &info->modelist);
> >  
> >  	/*
> >  	 * This makes sure that our colour bitfield
> > @@ -944,10 +1066,10 @@ static int __init imxfb_probe(struct platform_device *pdev)
> >  failed_register:
> >  	fb_dealloc_cmap(&info->cmap);
> >  failed_cmap:
> > -	if (pdata->exit)
> > +	if (pdata && pdata->exit)
> >  		pdata->exit(fbi->pdev);
> >  failed_platform_init:
> > -	if (!pdata->fixed_screen_cpu)
> > +	if (pdata && !pdata->fixed_screen_cpu)
> >  		dma_free_writecombine(&pdev->dev,fbi->map_size,fbi->map_cpu,
> >  			fbi->map_dma);
> >  failed_map:
> > @@ -956,6 +1078,7 @@ failed_ioremap:
> >  failed_getclock:
> >  	release_mem_region(res->start, resource_size(res));
> >  failed_req:
> > +failed_of_parse:
> >  	kfree(info->pseudo_palette);
> >  failed_init:
> >  	platform_set_drvdata(pdev, NULL);
> > @@ -980,7 +1103,7 @@ static int imxfb_remove(struct platform_device *pdev)
> >  	unregister_framebuffer(info);
> >  
> >  	pdata = pdev->dev.platform_data;
> > -	if (pdata->exit)
> > +	if (pdata && pdata->exit)
> >  		pdata->exit(fbi->pdev);
> >  
> >  	fb_dealloc_cmap(&info->cmap);
> > @@ -1009,6 +1132,7 @@ static struct platform_driver imxfb_driver = {
> >  	.shutdown	= imxfb_shutdown,
> >  	.driver		= {
> >  		.name	= DRIVER_NAME,
> > +		.of_match_table = imxfb_of_dev_id,
> >  	},
> >  	.id_table	= imxfb_devtype,
> >  };
> > -- 
> > 1.8.1.5
> > 
> > _______________________________________________
> > devicetree-discuss mailing list
> > devicetree-discuss at lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/devicetree-discuss
> 

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

[PATCH v3 2/2] video: imxfb: Add DT support

[Jean-Christophe PLAGNIOL-VILLARD]

On 16:31 Sun 14 Apr     , Markus Pargmann wrote:
> On Mon, Apr 08, 2013 at 09:57:42AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 12:31 Fri 05 Apr     , Markus Pargmann wrote:
> > > Add devicetree support for imx framebuffer driver. It uses the generic
> > > display bindings and helper functions.
> > > 
> > > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> > > Cc: Fabio Estevam <festevam@gmail.com>
> > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > ---
> > >  .../devicetree/bindings/video/fsl,imx-fb.txt       |  49 ++++++
> > >  drivers/video/imxfb.c                              | 192 +++++++++++++++++----
> > >  2 files changed, 207 insertions(+), 34 deletions(-)
> > >  create mode 100644 Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > > new file mode 100644
> > > index 0000000..bde9c77
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > > @@ -0,0 +1,49 @@
> > > +Freescale imx21 Framebuffer
> > > +
> > > +This framebuffer driver supports devices imx1, imx21, imx25, and imx27.
> > > +
> > > +Required properties:
> > > +- compatible : "fsl,<chip>-fb", chip should be imx1 or imx21
> > > +- reg : Should contain 1 register ranges(address and length)
> > > +- interrupts : One interrupt of the fb dev
> > > +
> > > +Required nodes:
> > > +- display: Phandle to a display node as described in
> > > +	Documentation/devicetree/bindings/video/display-timing.txt
> > > +	Additional, the display node has to define properties:
> > > +	- fsl,bpp: Bits per pixel
> > > +	- fsl,pcr: LCDC PCR value
> > > +
> > > +Optional properties:
> > > +- dmacr-eukrea: Should be set for eukrea boards.
> > why ?
> 
> Because eukrea boards have a different dmacr then all other boards using
> imxfb. The dmacr address is hardcoded as defaults in the code. I could
> also search for the board name in the DT, but there are no eukrea boards
> with DT at the moment, so I thought a bool property may be better for
> the moment.
so no if an other board come will have an other property no

add an optionnal option to pass the dmacr
> 
> > 
> > and This should be prefix by yhe Vendor here Eukrea
> 
> Okay.
> 
> Regards,
> 
> Markus
> 
> > > +
> > > +Example:
> > > +
> > > +	imxfb: fb at 10021000 {
> > > +		compatible = "fsl,imx27-fb", "fsl,imx21-fb";
> > > +		interrupts = <61>;
> > > +		reg = <0x10021000 0x1000>;
> > > +		display = <&display0>;
> > > +	};
> > > +
> > > +	...
> > > +
> > > +	display0: display0 {
> > > +		model = "Primeview-PD050VL1";
> > > +		native-mode = <&timing_disp0>;
> > > +		fsl,bpp = <16>;		/* non-standard but required */
> > > +		fsl,pcr = <0xf0c88080>;	/* non-standard but required */
> > > +		display-timings {
> > > +			timing_disp0: 640x480 {
> > > +				hactive = <640>;
> > > +				vactive = <480>;
> > > +				hback-porch = <112>;
> > > +				hfront-porch = <36>;
> > > +				hsync-len = <32>;
> > > +				vback-porch = <33>;
> > > +				vfront-porch = <33>;
> > > +				vsync-len = <2>;
> > > +				clock-frequency = <25000000>;
> > > +			};
> > > +		};
> > > +	};
> > > diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> > > index ef2b587..5a9bc598 100644
> > > --- a/drivers/video/imxfb.c
> > > +++ b/drivers/video/imxfb.c
> > > @@ -32,6 +32,12 @@
> > >  #include <linux/io.h>
> > >  #include <linux/math64.h>
> > >  #include <linux/uaccess.h>
> > > +#include <linux/of.h>
> > > +#include <linux/of_device.h>
> > > +
> > > +#include <video/of_display_timing.h>
> > > +#include <video/of_videomode.h>
> > > +#include <video/videomode.h>
> > >  
> > >  #include <linux/platform_data/video-imxfb.h>
> > >  
> > > @@ -117,10 +123,13 @@
> > >  #define LGWCR_GWAV(alpha)	(((alpha) & 0xff) << 24)
> > >  #define LGWCR_GWE	(1 << 22)
> > >  
> > > +#define IMXFB_LSCR1_DEFAULT 0x00120300
> > > +#define IMXFB_DMACR_DEFAULT 0x00020010
> > > +#define IMXFB_DMACR_EUKREA_DEFAULT 0x00040060
> > > +
> > >  /* Used fb-mode. Can be set on kernel command line, therefore file-static. */
> > >  static const char *fb_mode;
> > >  
> > > -
> > >  /*
> > >   * These are the bitfields for each
> > >   * display depth that we support.
> > > @@ -192,6 +201,19 @@ static struct platform_device_id imxfb_devtype[] = {
> > >  };
> > >  MODULE_DEVICE_TABLE(platform, imxfb_devtype);
> > >  
> > > +static struct of_device_id imxfb_of_dev_id[] = {
> > > +	{
> > > +		.compatible = "fsl,imx1-fb",
> > > +		.data = &imxfb_devtype[IMX1_FB],
> > > +	}, {
> > > +		.compatible = "fsl,imx21-fb",
> > > +		.data = &imxfb_devtype[IMX21_FB],
> > > +	}, {
> > > +		/* sentinel */
> > > +	}
> > > +};
> > > +MODULE_DEVICE_TABLE(of, imxfb_of_dev_id);
> > > +
> > >  static inline int is_imx1_fb(struct imxfb_info *fbi)
> > >  {
> > >  	return fbi->devtype == IMX1_FB;
> > > @@ -324,6 +346,9 @@ static const struct imx_fb_videomode *imxfb_find_mode(struct imxfb_info *fbi)
> > >  	struct imx_fb_videomode *m;
> > >  	int i;
> > >  
> > > +	if (!fb_mode)
> > > +		return &fbi->mode[0];
> > > +
> > >  	for (i = 0, m = &fbi->mode[0]; i < fbi->num_modes; i++, m++) {
> > >  		if (!strcmp(m->mode.name, fb_mode))
> > >  			return m;
> > > @@ -479,6 +504,9 @@ static int imxfb_bl_update_status(struct backlight_device *bl)
> > >  	struct imxfb_info *fbi = bl_get_data(bl);
> > >  	int brightness = bl->props.brightness;
> > >  
> > > +	if (!fbi->pwmr)
> > > +		return 0;
> > > +
> > >  	if (bl->props.power != FB_BLANK_UNBLANK)
> > >  		brightness = 0;
> > >  	if (bl->props.fb_blank != FB_BLANK_UNBLANK)
> > > @@ -719,7 +747,8 @@ static int imxfb_activate_var(struct fb_var_screeninfo *var, struct fb_info *inf
> > >  
> > >  	writel(fbi->pcr, fbi->regs + LCDC_PCR);
> > >  #ifndef PWMR_BACKLIGHT_AVAILABLE
> > > -	writel(fbi->pwmr, fbi->regs + LCDC_PWMR);
> > > +	if (fbi->pwmr)
> > > +		writel(fbi->pwmr, fbi->regs + LCDC_PWMR);
> > >  #endif
> > >  	writel(fbi->lscr1, fbi->regs + LCDC_LSCR1);
> > >  	writel(fbi->dmacr, fbi->regs + LCDC_DMACR);
> > > @@ -758,13 +787,12 @@ static int imxfb_resume(struct platform_device *dev)
> > >  #define imxfb_resume	NULL
> > >  #endif
> > >  
> > > -static int __init imxfb_init_fbinfo(struct platform_device *pdev)
> > > +static int imxfb_init_fbinfo(struct platform_device *pdev)
> > >  {
> > >  	struct imx_fb_platform_data *pdata = pdev->dev.platform_data;
> > >  	struct fb_info *info = dev_get_drvdata(&pdev->dev);
> > >  	struct imxfb_info *fbi = info->par;
> > > -	struct imx_fb_videomode *m;
> > > -	int i;
> > > +	struct device_node *np;
> > >  
> > >  	pr_debug("%s\n",__func__);
> > >  
> > > @@ -795,41 +823,96 @@ static int __init imxfb_init_fbinfo(struct platform_device *pdev)
> > >  	info->fbops			= &imxfb_ops;
> > >  	info->flags			= FBINFO_FLAG_DEFAULT |
> > >  					  FBINFO_READS_FAST;
> > > -	info->var.grayscale		= pdata->cmap_greyscale;
> > > -	fbi->cmap_inverse		= pdata->cmap_inverse;
> > > -	fbi->cmap_static		= pdata->cmap_static;
> > > -	fbi->lscr1			= pdata->lscr1;
> > > -	fbi->dmacr			= pdata->dmacr;
> > > -	fbi->pwmr			= pdata->pwmr;
> > > -	fbi->lcd_power			= pdata->lcd_power;
> > > -	fbi->backlight_power		= pdata->backlight_power;
> > > -
> > > -	for (i = 0, m = &pdata->mode[0]; i < pdata->num_modes; i++, m++)
> > > -		info->fix.smem_len = max_t(size_t, info->fix.smem_len,
> > > -				m->mode.xres * m->mode.yres * m->bpp / 8);
> > > +	if (pdata) {
> > > +		info->var.grayscale		= pdata->cmap_greyscale;
> > > +		fbi->cmap_inverse		= pdata->cmap_inverse;
> > > +		fbi->cmap_static		= pdata->cmap_static;
> > > +		fbi->lscr1			= pdata->lscr1;
> > > +		fbi->dmacr			= pdata->dmacr;
> > > +		fbi->pwmr			= pdata->pwmr;
> > > +		fbi->lcd_power			= pdata->lcd_power;
> > > +		fbi->backlight_power		= pdata->backlight_power;
> > > +	} else {
> > > +		np = pdev->dev.of_node;
> > > +		info->var.grayscale = of_property_read_bool(np,
> > > +						"cmap-greyscale");
> > > +		fbi->cmap_inverse = of_property_read_bool(np, "cmap-inverse");
> > > +		fbi->cmap_static = of_property_read_bool(np, "cmap-static");
> > > +
> > > +		fbi->lscr1 = IMXFB_LSCR1_DEFAULT;
> > > +		if (of_property_read_bool(np, "dmacr-eukrea"))
> > > +			fbi->dmacr = IMXFB_DMACR_EUKREA_DEFAULT;
> > > +		else
> > > +			fbi->dmacr = IMXFB_DMACR_DEFAULT;
> > > +
> > > +		/* These two function pointers could be used by some specific
> > > +		 * platforms. */
> > > +		fbi->lcd_power = NULL;
> > > +		fbi->backlight_power = NULL;
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int imxfb_of_read_mode(struct device *dev, struct device_node *np,
> > > +		struct imx_fb_videomode *imxfb_mode)
> > > +{
> > > +	int ret;
> > > +	struct fb_videomode *of_mode = &imxfb_mode->mode;
> > > +	u32 bpp;
> > > +	u32 pcr;
> > > +
> > > +	ret = of_property_read_string(np, "model", &of_mode->name);
> > > +	if (ret)
> > > +		of_mode->name = NULL;
> > > +
> > > +	ret = of_get_fb_videomode(np, of_mode, OF_USE_NATIVE_MODE);
> > > +	if (ret) {
> > > +		dev_err(dev, "Failed to get videomode from DT\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	ret = of_property_read_u32(np, "fsl,bpp", &bpp);
> > > +	ret |= of_property_read_u32(np, "fsl,pcr", &pcr);
> > > +
> > > +	if (ret) {
> > > +		dev_err(dev, "Failed to read bpp and pcr from DT\n");
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	if (bpp < 1 || bpp > 255) {
> > > +		dev_err(dev, "Bits per pixel have to be between 1 and 255\n");
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	imxfb_mode->bpp = bpp;
> > > +	imxfb_mode->pcr = pcr;
> > >  
> > >  	return 0;
> > >  }
> > >  
> > > -static int __init imxfb_probe(struct platform_device *pdev)
> > > +static int imxfb_probe(struct platform_device *pdev)
> > >  {
> > >  	struct imxfb_info *fbi;
> > >  	struct fb_info *info;
> > >  	struct imx_fb_platform_data *pdata;
> > >  	struct resource *res;
> > > +	struct imx_fb_videomode *m;
> > > +	const struct of_device_id *of_id;
> > >  	int ret, i;
> > > +	int bytes_per_pixel;
> > >  
> > >  	dev_info(&pdev->dev, "i.MX Framebuffer driver\n");
> > >  
> > > +	of_id = of_match_device(imxfb_of_dev_id, &pdev->dev);
> > > +	if (of_id)
> > > +		pdev->id_entry = of_id->data;
> > > +
> > >  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > >  	if (!res)
> > >  		return -ENODEV;
> > >  
> > >  	pdata = pdev->dev.platform_data;
> > > -	if (!pdata) {
> > > -		dev_err(&pdev->dev,"No platform_data available\n");
> > > -		return -ENOMEM;
> > > -	}
> > >  
> > >  	info = framebuffer_alloc(sizeof(struct imxfb_info), &pdev->dev);
> > >  	if (!info)
> > > @@ -837,15 +920,55 @@ static int __init imxfb_probe(struct platform_device *pdev)
> > >  
> > >  	fbi = info->par;
> > >  
> > > -	if (!fb_mode)
> > > -		fb_mode = pdata->mode[0].mode.name;
> > > -
> > >  	platform_set_drvdata(pdev, info);
> > >  
> > >  	ret = imxfb_init_fbinfo(pdev);
> > >  	if (ret < 0)
> > >  		goto failed_init;
> > >  
> > > +	if (pdata) {
> > > +		if (!fb_mode)
> > > +			fb_mode = pdata->mode[0].mode.name;
> > > +
> > > +		fbi->mode = pdata->mode;
> > > +		fbi->num_modes = pdata->num_modes;
> > > +	} else {
> > > +		struct device_node *display_np;
> > > +		fb_mode = NULL;
> > > +
> > > +		display_np = of_parse_phandle(pdev->dev.of_node, "display", 0);
> > > +		if (!display_np) {
> > > +			dev_err(&pdev->dev, "No display defined in devicetree\n");
> > > +			ret = -EINVAL;
> > > +			goto failed_of_parse;
> > > +		}
> > > +
> > > +		/*
> > > +		 * imxfb does not support more modes, we choose only the native
> > > +		 * mode.
> > > +		 */
> > > +		fbi->num_modes = 1;
> > > +
> > > +		fbi->mode = devm_kzalloc(&pdev->dev,
> > > +				sizeof(struct imx_fb_videomode), GFP_KERNEL);
> > > +		if (!fbi->mode) {
> > > +			ret = -ENOMEM;
> > > +			goto failed_of_parse;
> > > +		}
> > > +
> > > +		ret = imxfb_of_read_mode(&pdev->dev, display_np, fbi->mode);
> > > +		if (ret)
> > > +			goto failed_of_parse;
> > > +	}
> > > +
> > > +	/* Calculate maximum bytes used per pixel. In most cases this should
> > > +	 * be the same as m->bpp/8 */
> > > +	m = &fbi->mode[0];
> > > +	bytes_per_pixel = (m->bpp + 7) / 8;
> > > +	for (i = 0; i < fbi->num_modes; i++, m++)
> > > +		info->fix.smem_len = max_t(size_t, info->fix.smem_len,
> > > +				m->mode.xres * m->mode.yres * bytes_per_pixel);
> > > +
> > >  	res = request_mem_region(res->start, resource_size(res),
> > >  				DRIVER_NAME);
> > >  	if (!res) {
> > > @@ -878,7 +1001,8 @@ static int __init imxfb_probe(struct platform_device *pdev)
> > >  		goto failed_ioremap;
> > >  	}
> > >  
> > > -	if (!pdata->fixed_screen_cpu) {
> > > +	/* Seems not being used by anyone, so no support for oftree */
> > > +	if (!pdata || !pdata->fixed_screen_cpu) {
> > >  		fbi->map_size = PAGE_ALIGN(info->fix.smem_len);
> > >  		fbi->map_cpu = dma_alloc_writecombine(&pdev->dev,
> > >  				fbi->map_size, &fbi->map_dma, GFP_KERNEL);
> > > @@ -903,18 +1027,16 @@ static int __init imxfb_probe(struct platform_device *pdev)
> > >  		info->fix.smem_start = fbi->screen_dma;
> > >  	}
> > >  
> > > -	if (pdata->init) {
> > > +	if (pdata && pdata->init) {
> > >  		ret = pdata->init(fbi->pdev);
> > >  		if (ret)
> > >  			goto failed_platform_init;
> > >  	}
> > >  
> > > -	fbi->mode = pdata->mode;
> > > -	fbi->num_modes = pdata->num_modes;
> > >  
> > >  	INIT_LIST_HEAD(&info->modelist);
> > > -	for (i = 0; i < pdata->num_modes; i++)
> > > -		fb_add_videomode(&pdata->mode[i].mode, &info->modelist);
> > > +	for (i = 0; i < fbi->num_modes; i++)
> > > +		fb_add_videomode(&fbi->mode[i].mode, &info->modelist);
> > >  
> > >  	/*
> > >  	 * This makes sure that our colour bitfield
> > > @@ -944,10 +1066,10 @@ static int __init imxfb_probe(struct platform_device *pdev)
> > >  failed_register:
> > >  	fb_dealloc_cmap(&info->cmap);
> > >  failed_cmap:
> > > -	if (pdata->exit)
> > > +	if (pdata && pdata->exit)
> > >  		pdata->exit(fbi->pdev);
> > >  failed_platform_init:
> > > -	if (!pdata->fixed_screen_cpu)
> > > +	if (pdata && !pdata->fixed_screen_cpu)
> > >  		dma_free_writecombine(&pdev->dev,fbi->map_size,fbi->map_cpu,
> > >  			fbi->map_dma);
> > >  failed_map:
> > > @@ -956,6 +1078,7 @@ failed_ioremap:
> > >  failed_getclock:
> > >  	release_mem_region(res->start, resource_size(res));
> > >  failed_req:
> > > +failed_of_parse:
> > >  	kfree(info->pseudo_palette);
> > >  failed_init:
> > >  	platform_set_drvdata(pdev, NULL);
> > > @@ -980,7 +1103,7 @@ static int imxfb_remove(struct platform_device *pdev)
> > >  	unregister_framebuffer(info);
> > >  
> > >  	pdata = pdev->dev.platform_data;
> > > -	if (pdata->exit)
> > > +	if (pdata && pdata->exit)
> > >  		pdata->exit(fbi->pdev);
> > >  
> > >  	fb_dealloc_cmap(&info->cmap);
> > > @@ -1009,6 +1132,7 @@ static struct platform_driver imxfb_driver = {
> > >  	.shutdown	= imxfb_shutdown,
> > >  	.driver		= {
> > >  		.name	= DRIVER_NAME,
> > > +		.of_match_table = imxfb_of_dev_id,
> > >  	},
> > >  	.id_table	= imxfb_devtype,
> > >  };
> > > -- 
> > > 1.8.1.5
> > > 
> > > _______________________________________________
> > > devicetree-discuss mailing list
> > > devicetree-discuss at lists.ozlabs.org
> > > https://lists.ozlabs.org/listinfo/devicetree-discuss
> > 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v3 2/2] video: imxfb: Add DT support

[Eric Bénard]

Hi,

Le Mon, 15 Apr 2013 14:43:21 +0200,
Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> a ?crit :

> On 16:31 Sun 14 Apr     , Markus Pargmann wrote:
> > On Mon, Apr 08, 2013 at 09:57:42AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > On 12:31 Fri 05 Apr     , Markus Pargmann wrote:
> > > > Add devicetree support for imx framebuffer driver. It uses the generic
> > > > display bindings and helper functions.
> > > > 
> > > > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> > > > Cc: Fabio Estevam <festevam@gmail.com>
> > > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > > ---
> > > >  .../devicetree/bindings/video/fsl,imx-fb.txt       |  49 ++++++
> > > >  drivers/video/imxfb.c                              | 192 +++++++++++++++++----
> > > >  2 files changed, 207 insertions(+), 34 deletions(-)
> > > >  create mode 100644 Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > > > new file mode 100644
> > > > index 0000000..bde9c77
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > > > @@ -0,0 +1,49 @@
> > > > +Freescale imx21 Framebuffer
> > > > +
> > > > +This framebuffer driver supports devices imx1, imx21, imx25, and imx27.
> > > > +
> > > > +Required properties:
> > > > +- compatible : "fsl,<chip>-fb", chip should be imx1 or imx21
> > > > +- reg : Should contain 1 register ranges(address and length)
> > > > +- interrupts : One interrupt of the fb dev
> > > > +
> > > > +Required nodes:
> > > > +- display: Phandle to a display node as described in
> > > > +	Documentation/devicetree/bindings/video/display-timing.txt
> > > > +	Additional, the display node has to define properties:
> > > > +	- fsl,bpp: Bits per pixel
> > > > +	- fsl,pcr: LCDC PCR value
> > > > +
> > > > +Optional properties:
> > > > +- dmacr-eukrea: Should be set for eukrea boards.
> > > why ?
> > 
> > Because eukrea boards have a different dmacr then all other boards using
> > imxfb. The dmacr address is hardcoded as defaults in the code. I could
> > also search for the board name in the DT, but there are no eukrea boards
> > with DT at the moment, so I thought a bool property may be better for
> > the moment.
> so no if an other board come will have an other property no
> 
> add an optionnal option to pass the dmacr

I didn't follow the thread but dmacr is not specific to our boards :
$ grep -rn dmacr arch/arm/mach-imx/
arch/arm/mach-imx/mach-mx27_3ds.c:446:	.dmacr		= 0x00020010,
arch/arm/mach-imx/mach-mx21ads.c:231:	.dmacr		= 0x00020008,
arch/arm/mach-imx/mach-mxt_td60.c:210:	.dmacr		= 0x00020010,
arch/arm/mach-imx/eukrea_mbimx27-baseboard.c:192:	.dmacr		= 0x00040060,
arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:165:	.dmacr		= 0x00040060,
arch/arm/mach-imx/mach-mx27ads.c:241:	.dmacr		= 0x00020010,
arch/arm/mach-imx/mach-pca100.c:351:	.dmacr		= 0x00020010,
arch/arm/mach-imx/pcm970-baseboard.c:185:	.dmacr		= 0x00020010,
arch/arm/mach-imx/mach-mx25_3ds.c:173:	.dmacr		= 0x00020010,

Best regards,
Eric

[PATCH v3 2/2] video: imxfb: Add DT support

[Jean-Christophe PLAGNIOL-VILLARD]

On 15:13 Mon 15 Apr     , Eric B?nard wrote:
> Hi,
> 
> Le Mon, 15 Apr 2013 14:43:21 +0200,
> Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> a ?crit :
> 
> > On 16:31 Sun 14 Apr     , Markus Pargmann wrote:
> > > On Mon, Apr 08, 2013 at 09:57:42AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > > On 12:31 Fri 05 Apr     , Markus Pargmann wrote:
> > > > > Add devicetree support for imx framebuffer driver. It uses the generic
> > > > > display bindings and helper functions.
> > > > > 
> > > > > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> > > > > Cc: Fabio Estevam <festevam@gmail.com>
> > > > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > > > ---
> > > > >  .../devicetree/bindings/video/fsl,imx-fb.txt       |  49 ++++++
> > > > >  drivers/video/imxfb.c                              | 192 +++++++++++++++++----
> > > > >  2 files changed, 207 insertions(+), 34 deletions(-)
> > > > >  create mode 100644 Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > > > > new file mode 100644
> > > > > index 0000000..bde9c77
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > > > > @@ -0,0 +1,49 @@
> > > > > +Freescale imx21 Framebuffer
> > > > > +
> > > > > +This framebuffer driver supports devices imx1, imx21, imx25, and imx27.
> > > > > +
> > > > > +Required properties:
> > > > > +- compatible : "fsl,<chip>-fb", chip should be imx1 or imx21
> > > > > +- reg : Should contain 1 register ranges(address and length)
> > > > > +- interrupts : One interrupt of the fb dev
> > > > > +
> > > > > +Required nodes:
> > > > > +- display: Phandle to a display node as described in
> > > > > +	Documentation/devicetree/bindings/video/display-timing.txt
> > > > > +	Additional, the display node has to define properties:
> > > > > +	- fsl,bpp: Bits per pixel
> > > > > +	- fsl,pcr: LCDC PCR value
> > > > > +
> > > > > +Optional properties:
> > > > > +- dmacr-eukrea: Should be set for eukrea boards.
> > > > why ?
> > > 
> > > Because eukrea boards have a different dmacr then all other boards using
> > > imxfb. The dmacr address is hardcoded as defaults in the code. I could
> > > also search for the board name in the DT, but there are no eukrea boards
> > > with DT at the moment, so I thought a bool property may be better for
> > > the moment.
> > so no if an other board come will have an other property no
> > 
> > add an optionnal option to pass the dmacr
> 
> I didn't follow the thread but dmacr is not specific to our boards :
> $ grep -rn dmacr arch/arm/mach-imx/
> arch/arm/mach-imx/mach-mx27_3ds.c:446:	.dmacr		= 0x00020010,
> arch/arm/mach-imx/mach-mx21ads.c:231:	.dmacr		= 0x00020008,
> arch/arm/mach-imx/mach-mxt_td60.c:210:	.dmacr		= 0x00020010,
> arch/arm/mach-imx/eukrea_mbimx27-baseboard.c:192:	.dmacr		= 0x00040060,
> arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:165:	.dmacr		= 0x00040060,
> arch/arm/mach-imx/mach-mx27ads.c:241:	.dmacr		= 0x00020010,
> arch/arm/mach-imx/mach-pca100.c:351:	.dmacr		= 0x00020010,
> arch/arm/mach-imx/pcm970-baseboard.c:185:	.dmacr		= 0x00020010,
> arch/arm/mach-imx/mach-mx25_3ds.c:173:	.dmacr		= 0x00020010,

with this it even more clear nack on the dmacr-eukrea property

use an optional property where we can specify the value to use

Best Regsrds,
J.

[PATCH v3 2/2] video: imxfb: Add DT support

[Sascha Hauer]

On Mon, Apr 15, 2013 at 03:13:16PM +0200, Eric B?nard wrote:
> Hi,
> 
> 
> I didn't follow the thread but dmacr is not specific to our boards :
> $ grep -rn dmacr arch/arm/mach-imx/
> arch/arm/mach-imx/mach-mx27_3ds.c:446:	.dmacr		= 0x00020010,
> arch/arm/mach-imx/mach-mx21ads.c:231:	.dmacr		= 0x00020008,
> arch/arm/mach-imx/mach-mxt_td60.c:210:	.dmacr		= 0x00020010,
> arch/arm/mach-imx/eukrea_mbimx27-baseboard.c:192:	.dmacr		= 0x00040060,
> arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:165:	.dmacr		= 0x00040060,
> arch/arm/mach-imx/mach-mx27ads.c:241:	.dmacr		= 0x00020010,
> arch/arm/mach-imx/mach-pca100.c:351:	.dmacr		= 0x00020010,
> arch/arm/mach-imx/pcm970-baseboard.c:185:	.dmacr		= 0x00020010,
> arch/arm/mach-imx/mach-mx25_3ds.c:173:	.dmacr		= 0x00020010,

The reference manual has this about the dmacr register:

The dynamic mode is recommend, and also the TM and HM reset value are
recommend in the dynamic mode. The same recommendation is for the
graphic DMA control.

So maybe we should ask whether the boards really have to change this
value or if the reset value is suitable for all.

Sascha

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

[PATCH v3 2/2] video: imxfb: Add DT support

[Markus Pargmann]

On Mon, Apr 15, 2013 at 03:23:22PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 15:13 Mon 15 Apr     , Eric B?nard wrote:
> > Hi,
> > 
> > Le Mon, 15 Apr 2013 14:43:21 +0200,
> > Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> a ?crit :
> > 
> > > On 16:31 Sun 14 Apr     , Markus Pargmann wrote:
> > > > On Mon, Apr 08, 2013 at 09:57:42AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > > > On 12:31 Fri 05 Apr     , Markus Pargmann wrote:
> > > > > > Add devicetree support for imx framebuffer driver. It uses the generic
> > > > > > display bindings and helper functions.
> > > > > > 
> > > > > > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> > > > > > Cc: Fabio Estevam <festevam@gmail.com>
> > > > > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > > > > ---
> > > > > >  .../devicetree/bindings/video/fsl,imx-fb.txt       |  49 ++++++
> > > > > >  drivers/video/imxfb.c                              | 192 +++++++++++++++++----
> > > > > >  2 files changed, 207 insertions(+), 34 deletions(-)
> > > > > >  create mode 100644 Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > > > > > 
> > > > > > diff --git a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > > > > > new file mode 100644
> > > > > > index 0000000..bde9c77
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > > > > > @@ -0,0 +1,49 @@
> > > > > > +Freescale imx21 Framebuffer
> > > > > > +
> > > > > > +This framebuffer driver supports devices imx1, imx21, imx25, and imx27.
> > > > > > +
> > > > > > +Required properties:
> > > > > > +- compatible : "fsl,<chip>-fb", chip should be imx1 or imx21
> > > > > > +- reg : Should contain 1 register ranges(address and length)
> > > > > > +- interrupts : One interrupt of the fb dev
> > > > > > +
> > > > > > +Required nodes:
> > > > > > +- display: Phandle to a display node as described in
> > > > > > +	Documentation/devicetree/bindings/video/display-timing.txt
> > > > > > +	Additional, the display node has to define properties:
> > > > > > +	- fsl,bpp: Bits per pixel
> > > > > > +	- fsl,pcr: LCDC PCR value
> > > > > > +
> > > > > > +Optional properties:
> > > > > > +- dmacr-eukrea: Should be set for eukrea boards.
> > > > > why ?
> > > > 
> > > > Because eukrea boards have a different dmacr then all other boards using
> > > > imxfb. The dmacr address is hardcoded as defaults in the code. I could
> > > > also search for the board name in the DT, but there are no eukrea boards
> > > > with DT at the moment, so I thought a bool property may be better for
> > > > the moment.
> > > so no if an other board come will have an other property no
> > > 
> > > add an optionnal option to pass the dmacr
> > 
> > I didn't follow the thread but dmacr is not specific to our boards :
> > $ grep -rn dmacr arch/arm/mach-imx/
> > arch/arm/mach-imx/mach-mx27_3ds.c:446:	.dmacr		= 0x00020010,
> > arch/arm/mach-imx/mach-mx21ads.c:231:	.dmacr		= 0x00020008,
> > arch/arm/mach-imx/mach-mxt_td60.c:210:	.dmacr		= 0x00020010,
> > arch/arm/mach-imx/eukrea_mbimx27-baseboard.c:192:	.dmacr		= 0x00040060,
> > arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:165:	.dmacr		= 0x00040060,
> > arch/arm/mach-imx/mach-mx27ads.c:241:	.dmacr		= 0x00020010,
> > arch/arm/mach-imx/mach-pca100.c:351:	.dmacr		= 0x00020010,
> > arch/arm/mach-imx/pcm970-baseboard.c:185:	.dmacr		= 0x00020010,
> > arch/arm/mach-imx/mach-mx25_3ds.c:173:	.dmacr		= 0x00020010,
> 
> with this it even more clear nack on the dmacr-eukrea property
> 
> use an optional property where we can specify the value to use
> 
> Best Regsrds,
> J.
> 

v4 will contain a optional fsl,dmacr property now. By default the reset
value of dmacr register is not changed.

Regards,

Markus

-- 
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 |