Re: [PATCHv3][ 3/5] video: mx3fb: Add device tree suport.

[Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]

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