[PATCH] video: imxfb DT support

[PATCH] video: imxfb DT support

[Markus Pargmann]

Hi,

the first patch adds support to set alpha values. The second one
adds DT support using the generic DT display bindings.

Markus

[PATCH 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.4

[PATCH 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>
---
 .../devicetree/bindings/video/fsl,imx-fb.txt       |  30 ++++
 drivers/video/imxfb.c                              | 176 +++++++++++++++++----
 2 files changed, 173 insertions(+), 33 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..fd88d26
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
@@ -0,0 +1,30 @@
+Freescale imx21 Framebuffer
+
+This framebuffer driver supports chips imx1 and imx21.
+
+Required properties:
+- compatible : "fsl,<chip>-fb"
+- reg : Should contain 1 register ranges(address and length)
+- interrupts : One interrupt of the fb dev
+
+Required nodes:
+- display: a display node is required to initialize the lcd panel
+	This should be in the board dts. See definition in
+	Documentation/devicetree/bindings/video/via,vt8500-fb.txt
+- default-mode: a videomode node as specified in
+	Documentation/devicetree/bindings/video/via,vt8500-fb.txt
+
+Optional properties:
+- pwmr: Address of pwmr register
+- lscr1: Address of lscr1 register
+- dmacr: Address of dmacr register
+
+Example:
+
+	imxfb: fb@10021000 {
+		compatible = "fsl,imx27-fb", "fsl,imx21-fb";
+		interrupts = <61>;
+		reg = <0x10021000 0x1000>;
+		display = <&display0>;
+		default-mode = <&mode0>;
+	};
diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index ef2b587..6f1b04c 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>
 
@@ -120,7 +126,6 @@
 /* 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 +197,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 +342,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;
@@ -758,13 +779,13 @@ 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;
+	int ret;
 
 	pr_debug("%s\n",__func__);
 
@@ -795,41 +816,93 @@ 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");
+
+		ret = of_property_read_u32(np, "lscr1", &fbi->lscr1);
+		if (ret)
+			fbi->lscr1 = 0;
+		ret = of_property_read_u32(np, "dmacr", &fbi->dmacr);
+		if (ret)
+			fbi->dmacr = 0;
+		ret = of_property_read_u32(np, "pwmr", &fbi->pwmr);
+		if (ret)
+			fbi->pwmr = 0;
+
+		/* These two function pointers could be used by some specific
+		 * platforms. */
+		fbi->lcd_power = NULL;
+		fbi->backlight_power = NULL;
+	}
 
 	return 0;
 }
 
-static int __init imxfb_probe(struct platform_device *pdev)
+static int imxfb_of_read_mode(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)
+		return ret;
+
+	ret = of_property_read_u32(np, "bpp", &bpp);
+	ret |= of_property_read_u32(np, "pcr", &pcr);
+
+	if (ret)
+		return ret;
+
+	if (bpp > 255)
+		return -EINVAL;
+
+	imxfb_mode->bpp = bpp;
+	imxfb_mode->pcr = pcr;
+
+	return ret;
+}
+
+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;
 
 	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 +910,51 @@ 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(display_np, fbi->mode);
+		if (ret)
+			goto failed_of_parse;
+	}
+
+	for (i = 0, m = &fbi->mode[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 * m->bpp / 8);
+
 	res = request_mem_region(res->start, resource_size(res),
 				DRIVER_NAME);
 	if (!res) {
@@ -878,7 +987,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 +1013,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 +1052,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 +1064,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 +1089,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 +1118,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.4

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

[Fabio Estevam]

On Fri, Mar 1, 2013 at 12:41 PM, Markus Pargmann <mpa@pengutronix.de> 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>
> ---
>  .../devicetree/bindings/video/fsl,imx-fb.txt       |  30 ++++
>  drivers/video/imxfb.c                              | 176 +++++++++++++++++----
>  2 files changed, 173 insertions(+), 33 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..fd88d26
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> @@ -0,0 +1,30 @@
> +Freescale imx21 Framebuffer
> +
> +This framebuffer driver supports chips imx1 and imx21.

and mx25/mx27 as well.

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

[Sascha Hauer]

On Fri, Mar 01, 2013 at 04:41:07PM +0100, 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>
> ---
>  .../devicetree/bindings/video/fsl,imx-fb.txt       |  30 ++++
>  drivers/video/imxfb.c                              | 176 +++++++++++++++++----
>  2 files changed, 173 insertions(+), 33 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..fd88d26
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> @@ -0,0 +1,30 @@
> +Freescale imx21 Framebuffer
> +
> +This framebuffer driver supports chips imx1 and imx21.
> +
> +Required properties:
> +- compatible : "fsl,<chip>-fb"
> +- reg : Should contain 1 register ranges(address and length)
> +- interrupts : One interrupt of the fb dev
> +
> +Required nodes:
> +- display: a display node is required to initialize the lcd panel
> +	This should be in the board dts. See definition in
> +	Documentation/devicetree/bindings/video/via,vt8500-fb.txt
> +- default-mode: a videomode node as specified in
> +	Documentation/devicetree/bindings/video/via,vt8500-fb.txt

Since the of_videomode helpers didn't add a binding in itself, they
don't have a binding description in Documentation/devicetree/.
It's forseeable that drivers will use this binding in the near
future, so we should probably add a separate Documentation file
for it instead of referencing some driver which already implements
the binding.

> +
> +Optional properties:
> +- pwmr: Address of pwmr register

This describes the imxfb internal PWM controller. Please drop this for
now or add a proper PWM driver for it.

> +- lscr1: Address of lscr1 register
> +- dmacr: Address of dmacr register

I think we shouldn't expose these to the devicetree. With platform_data
this hasn't been nice, but ok. With devicetree this becomes an API, so
we should think of something better. Can't we make up sensible values
during runtime?

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 |

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

[Markus Pargmann]

On Mon, Mar 04, 2013 at 08:48:51AM +0100, Sascha Hauer wrote:
> On Fri, Mar 01, 2013 at 04:41:07PM +0100, 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>
> > ---
> >  .../devicetree/bindings/video/fsl,imx-fb.txt       |  30 ++++
> >  drivers/video/imxfb.c                              | 176 +++++++++++++++++----
> >  2 files changed, 173 insertions(+), 33 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..fd88d26
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > @@ -0,0 +1,30 @@
> > +Freescale imx21 Framebuffer
> > +
> > +This framebuffer driver supports chips imx1 and imx21.
> > +
> > +Required properties:
> > +- compatible : "fsl,<chip>-fb"
> > +- reg : Should contain 1 register ranges(address and length)
> > +- interrupts : One interrupt of the fb dev
> > +
> > +Required nodes:
> > +- display: a display node is required to initialize the lcd panel
> > +	This should be in the board dts. See definition in
> > +	Documentation/devicetree/bindings/video/via,vt8500-fb.txt
> > +- default-mode: a videomode node as specified in
> > +	Documentation/devicetree/bindings/video/via,vt8500-fb.txt
> 
> Since the of_videomode helpers didn't add a binding in itself, they
> don't have a binding description in Documentation/devicetree/.
> It's forseeable that drivers will use this binding in the near
> future, so we should probably add a separate Documentation file
> for it instead of referencing some driver which already implements
> the binding.

I checked again, there is a binding description at
Documentation/devicetree/bindings/video/display-timing.txt . I replaced
the reference to the other driver. That was actually from a previous
version of this patch.

> 
> > +
> > +Optional properties:
> > +- pwmr: Address of pwmr register
> 
> This describes the imxfb internal PWM controller. Please drop this for
> now or add a proper PWM driver for it.

Okay, removed.

> 
> > +- lscr1: Address of lscr1 register
> > +- dmacr: Address of dmacr register
> 
> I think we shouldn't expose these to the devicetree. With platform_data
> this hasn't been nice, but ok. With devicetree this becomes an API, so
> we should think of something better. Can't we make up sensible values
> during runtime?

I checked the definitions of the platform-data. It seems that all
definitions use the same lscr1 value. So I simply droped that property
and introduced a default value.
For pwmr1 exist two different values, one for eukrea boards and another
for all other boards, so I added two defaults here. The different value
can be selected by setting a bool property 'dmacr-eukrea'.

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 |