Re: [PATCH v1] ARM: i.mx: mx3fb: add overlay support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Alex

Thanks for reviving, fixing and submitting this code!

On Wed, 18 Apr 2012, Alex Gershgorin wrote:

> This patch is based on the original version submitted by Guennadi Liakhovetski,
> the patch initializes overlay channel, adds ioctl for configuring
> transparency of the overlay and graphics planes, CONFIG_FB_MX3_OVERLAY
> is also supported.
> 
> In case that CONFIG_FB_MX3_OVERLAY is not defined, mx3fb is completely
> backward compatible.
> 
> Blend mode, only global alpha blending has been tested.
> 
> Signed-off-by: Alex Gershgorin <alexg@xxxxxxxxxxxxxx>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>

Thanks for the credit (;-)), but no, putting my Sob after yours means, 
that I took your patch and forwarded it on to the next maintainer, which 
is clearly not the case here:-) The original i.MX31 framebuffer overlay 
code from my old patches also clearly wasn't written by me, since I didn't 
have a chance to test it. So, if you like, you can try to trace back 
original authors of that code and ask them, how they want to be credited 
here, otherwise just mentioning, that this work is based on some earlier 
patch series "i.MX31: dmaengine and framebuffer drivers" from 2008 by ... 
should be enough.

I don't think I can review this patch in sufficient depth, just a couple 
of minor comments below

> ---
> 
> Applies to v3.4-rc3
> ---
>  drivers/video/Kconfig |    7 +
>  drivers/video/mx3fb.c |  318 ++++++++++++++++++++++++++++++++++++++++++++-----
>  include/linux/mxcfb.h |   93 ++++++++++++++
>  3 files changed, 388 insertions(+), 30 deletions(-)
>  create mode 100644 include/linux/mxcfb.h
> 
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index a290be5..acbfccc 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -2368,6 +2368,13 @@ config FB_MX3
>  	  far only synchronous displays are supported. If you plan to use
>  	  an LCD display with your i.MX31 system, say Y here.
>  
> +config FB_MX3_OVERLAY
> +	bool "MX3 Overlay support"
> +	default n
> +	depends on FB_MX3
> +	---help---
> +	  Say Y here to enable overlay support
> +
>  config FB_BROADSHEET
>  	tristate "E-Ink Broadsheet/Epson S1D13521 controller support"
>  	depends on FB
> diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
> index eec0d7b..0fb8a72 100644
> --- a/drivers/video/mx3fb.c
> +++ b/drivers/video/mx3fb.c
> @@ -26,6 +26,7 @@
>  #include <linux/console.h>
>  #include <linux/clk.h>
>  #include <linux/mutex.h>
> +#include <linux/mxcfb.h>
>  
>  #include <mach/dma.h>
>  #include <mach/hardware.h>
> @@ -238,6 +239,7 @@ static const struct fb_videomode mx3fb_modedb[] = {
>  
>  struct mx3fb_data {
>  	struct fb_info		*fbi;
> +	struct fb_info		*fbi_ovl;
>  	int			backlight_level;
>  	void __iomem		*reg_base;
>  	spinlock_t		lock;
> @@ -246,6 +248,9 @@ struct mx3fb_data {
>  	uint32_t		h_start_width;
>  	uint32_t		v_start_width;
>  	enum disp_data_mapping	disp_data_fmt;
> +
> +	/* IDMAC / dmaengine interface */
> +	struct idmac_channel	*idmac_channel[2];	/* We need 2 channels */
>  };
>  
>  struct dma_chan_request {
> @@ -272,6 +277,17 @@ struct mx3fb_info {
>  	u32				sync;	/* preserve var->sync flags */
>  };
>  
> +/* Allocated overlay buffer */
> +struct mx3fb_alloc_list {
> +	struct list_head	list;
> +	dma_addr_t		phy_addr;
> +	void			*cpu_addr;
> +	size_t			size;
> +};
> +
> +/* A list of overlay buffers */
> +static LIST_HEAD(fb_alloc_list);

Static variables are evil:-) Which you prove below by protecting this 
global list-head by a per-device mutex. No, I have no idea whether anyone 
ever comes up with an SoC with multiple instances of this device, but at 
least this seems inconsistent to me.

> +
>  static void mx3fb_dma_done(void *);
>  
>  /* Used fb-mode and bpp. Can be set on kernel command line, therefore file-static. */
> @@ -303,7 +319,11 @@ static void sdc_fb_init(struct mx3fb_info *fbi)
>  	struct mx3fb_data *mx3fb = fbi->mx3fb;
>  	uint32_t reg;
>  
> -	reg = mx3fb_read_reg(mx3fb, SDC_COM_CONF);
> +	reg = mx3fb_read_reg(mx3fb, SDC_COM_CONF) & ~SDC_COM_GWSEL;
> +
> +	/* Also enable foreground for overlay graphic window is foreground */
> +	if (mx3fb->fbi_ovl && fbi == mx3fb->fbi_ovl->par)
> +		reg |= (SDC_COM_FG_EN | SDC_COM_GWSEL);

Superfluous parenthesis.

>  
>  	mx3fb_write_reg(mx3fb, reg | SDC_COM_BG_EN, SDC_COM_CONF);
>  }
> @@ -312,13 +332,24 @@ static void sdc_fb_init(struct mx3fb_info *fbi)
>  static uint32_t sdc_fb_uninit(struct mx3fb_info *fbi)
>  {
>  	struct mx3fb_data *mx3fb = fbi->mx3fb;
> -	uint32_t reg;
> +	uint32_t reg, chan_mask;
>  
>  	reg = mx3fb_read_reg(mx3fb, SDC_COM_CONF);
>  
> -	mx3fb_write_reg(mx3fb, reg & ~SDC_COM_BG_EN, SDC_COM_CONF);
> +	/*
> +	 * Don't we have to automatically disable overlay when disabling
> +	 * background? Attention: cannot test mx3fb->fbi_ovl->par, must
> +	 * test mx3fb->fbi->par, because at the time this function is
> +	 * called for the first time fbi_ovl is not assigned yet.
> +	 */
> +	if (fbi == mx3fb->fbi->par)
> +		chan_mask = SDC_COM_BG_EN;
> +	else
> +		chan_mask = SDC_COM_FG_EN | SDC_COM_GWSEL;
> +
> +	mx3fb_write_reg(mx3fb, reg & ~chan_mask, SDC_COM_CONF);
>  
> -	return reg & SDC_COM_BG_EN;
> +	return reg & chan_mask;
>  }
>  
>  static void sdc_enable_channel(struct mx3fb_info *mx3_fbi)
> @@ -412,13 +443,20 @@ static void sdc_disable_channel(struct mx3fb_info *mx3_fbi)
>  static int sdc_set_window_pos(struct mx3fb_data *mx3fb, enum ipu_channel channel,
>  			      int16_t x_pos, int16_t y_pos)
>  {
> -	if (channel != IDMAC_SDC_0)
> -		return -EINVAL;
> -
>  	x_pos += mx3fb->h_start_width;
>  	y_pos += mx3fb->v_start_width;
>  
> -	mx3fb_write_reg(mx3fb, (x_pos << 16) | y_pos, SDC_BG_POS);
> +	switch (channel) {
> +	case IDMAC_SDC_0:
> +		mx3fb_write_reg(mx3fb, (x_pos << 16) | y_pos, SDC_BG_POS);
> +		break;
> +	case IDMAC_SDC_1:
> +		mx3fb_write_reg(mx3fb, (x_pos << 16) | y_pos, SDC_FG_POS);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -482,14 +520,17 @@ static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel,
>  	mx3fb->h_start_width = h_start_width;
>  	mx3fb->v_start_width = v_start_width;
>  
> +	reg = mx3fb_read_reg(mx3fb, SDC_COM_CONF);
> +
>  	switch (panel) {
>  	case IPU_PANEL_SHARP_TFT:
>  		mx3fb_write_reg(mx3fb, 0x00FD0102L, SDC_SHARP_CONF_1);
>  		mx3fb_write_reg(mx3fb, 0x00F500F4L, SDC_SHARP_CONF_2);
> -		mx3fb_write_reg(mx3fb, SDC_COM_SHARP | SDC_COM_TFT_COLOR, SDC_COM_CONF);
> +		mx3fb_write_reg(mx3fb, reg | SDC_COM_SHARP |
> +				SDC_COM_TFT_COLOR, SDC_COM_CONF);
>  		break;
>  	case IPU_PANEL_TFT:
> -		mx3fb_write_reg(mx3fb, SDC_COM_TFT_COLOR, SDC_COM_CONF);
> +		mx3fb_write_reg(mx3fb, reg | SDC_COM_TFT_COLOR, SDC_COM_CONF);
>  		break;
>  	default:
>  		return -EINVAL;
> @@ -563,13 +604,12 @@ static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel,
>  /**
>   * sdc_set_color_key() - set the transparent color key for SDC graphic plane.
>   * @mx3fb:	mx3fb context.
> - * @channel:	IPU DMAC channel ID.
>   * @enable:	boolean to enable or disable color keyl.
>   * @color_key:	24-bit RGB color to use as transparent color key.
>   * @return:	0 on success or negative error code on failure.
>   */
> -static int sdc_set_color_key(struct mx3fb_data *mx3fb, enum ipu_channel channel,
> -			     bool enable, uint32_t color_key)
> +static int sdc_set_color_key(struct mx3fb_data *mx3fb, bool enable,
> +				uint32_t color_key)
>  {
>  	uint32_t reg, sdc_conf;
>  	unsigned long lock_flags;
> @@ -577,10 +617,6 @@ static int sdc_set_color_key(struct mx3fb_data *mx3fb, enum ipu_channel channel,
>  	spin_lock_irqsave(&mx3fb->lock, lock_flags);
>  
>  	sdc_conf = mx3fb_read_reg(mx3fb, SDC_COM_CONF);
> -	if (channel == IDMAC_SDC_0)
> -		sdc_conf &= ~SDC_COM_GWSEL;
> -	else
> -		sdc_conf |= SDC_COM_GWSEL;
>  
>  	if (enable) {
>  		reg = mx3fb_read_reg(mx3fb, SDC_GW_CTRL) & 0xFF000000L;
> @@ -668,8 +704,12 @@ static int mx3fb_set_fix(struct fb_info *fbi)
>  {
>  	struct fb_fix_screeninfo *fix = &fbi->fix;
>  	struct fb_var_screeninfo *var = &fbi->var;
> +	struct mx3fb_info *mx3_fbi = fbi->par;
>  
> -	strncpy(fix->id, "DISP3 BG", 8);
> +	if (mx3_fbi->ipu_ch == IDMAC_SDC_1)
> +		strncpy(fix->id, "DISP3 FG", 8);
> +	else
> +		strncpy(fix->id, "DISP3 BG", 8);
>  
>  	fix->line_length = var->xres_virtual * var->bits_per_pixel / 8;
>  
> @@ -689,13 +729,25 @@ static void mx3fb_dma_done(void *arg)
>  	struct idmac_channel *ichannel = to_idmac_chan(chan);
>  	struct mx3fb_data *mx3fb = ichannel->client;
>  	struct mx3fb_info *mx3_fbi = mx3fb->fbi->par;
> +	struct mx3fb_info *mx3_fbi_cur;
> +	struct mx3fb_info *mx3_fbi_ovl = mx3fb->fbi_ovl ? mx3fb->fbi_ovl->par :
> +		NULL;
>  
>  	dev_dbg(mx3fb->dev, "irq %d callback\n", ichannel->eof_irq);
>  
> +	if (ichannel == mx3_fbi->idmac_channel) {
> +		mx3_fbi_cur = mx3_fbi;
> +	} else if (mx3_fbi_ovl && ichannel == mx3_fbi_ovl->idmac_channel) {
> +		mx3_fbi_cur = mx3_fbi_ovl;
> +	} else {
> +		WARN(1, "Cannot identify channel!\n");
> +		return;
> +	}
> +
>  	/* We only need one interrupt, it will be re-enabled as needed */
>  	disable_irq_nosync(ichannel->eof_irq);
>  
> -	complete(&mx3_fbi->flip_cmpl);
> +	complete(&mx3_fbi_cur->flip_cmpl);
>  }
>  
>  static int __set_par(struct fb_info *fbi, bool lock)
> @@ -1151,6 +1203,145 @@ static struct fb_ops mx3fb_ops = {
>  	.fb_blank = mx3fb_blank,
>  };
>  
> +#ifdef CONFIG_FB_MX3_OVERLAY
> +static int mx3fb_blank_ovl(int blank, struct fb_info *fbi)
> +{
> +	struct mx3fb_info *mx3_fbi = fbi->par;
> +
> +	dev_dbg(fbi->device, "ovl blank = %d\n", blank);
> +
> +	if (mx3_fbi->blank == blank)
> +		return 0;
> +
> +	mutex_lock(&mx3_fbi->mutex);
> +	mx3_fbi->blank = blank;
> +
> +	switch (blank) {
> +	case FB_BLANK_POWERDOWN:
> +	case FB_BLANK_VSYNC_SUSPEND:
> +	case FB_BLANK_HSYNC_SUSPEND:
> +	case FB_BLANK_NORMAL:
> +		sdc_disable_channel(mx3_fbi);
> +		break;
> +	case FB_BLANK_UNBLANK:
> +		sdc_enable_channel(mx3_fbi);
> +		break;
> +	}
> +	mutex_unlock(&mx3_fbi->mutex);
> +
> +	return 0;
> +}
> +
> +/*
> + * Function to handle custom ioctls for MX3 framebuffer.
> + *
> + *  @inode	inode struct
> + *  @file	file struct
> + *  @cmd	Ioctl command to handle
> + *  @arg	User pointer to command arguments
> + *  @fbi	framebuffer information pointer
> + */
> +static int mx3fb_ioctl_ovl(struct fb_info *fbi, unsigned int cmd,
> +			   unsigned long arg)
> +{
> +	struct mx3fb_info *mx3_fbi = fbi->par;
> +	struct mx3fb_data *mx3fb = mx3_fbi->mx3fb;
> +	struct mxcfb_gbl_alpha ga;
> +	struct mxcfb_color_key key;
> +	int retval = 0;
> +	int __user *argp = (void __user *)arg;
> +	struct mx3fb_alloc_list *mem;
> +	int size;
> +	unsigned long offset;
> +
> +	switch (cmd) {
> +	case FBIO_ALLOC:
> +		if (get_user(size, argp))
> +			return -EFAULT;
> +
> +		mem = kzalloc(sizeof(*mem), GFP_KERNEL);
> +		if (mem == NULL)
> +			return -ENOMEM;
> +
> +		mem->size = PAGE_ALIGN(size);
> +
> +		mem->cpu_addr = dma_alloc_coherent(fbi->device, size,
> +						   &mem->phy_addr,
> +						   GFP_DMA);
> +		if (mem->cpu_addr == NULL) {
> +			kfree(mem);
> +			return -ENOMEM;
> +		}
> +
> +		mutex_lock(&mx3_fbi->mutex);
> +		list_add(&mem->list, &fb_alloc_list);
> +		mutex_unlock(&mx3_fbi->mutex);

Here. At the very least you'd need a global mutex. Or put the list-head in 
struct mx3fb_info.

> +
> +		dev_dbg(fbi->device, "allocated %d bytes  <at>  0x%08X\n",
> +			mem->size, mem->phy_addr);
> +
> +		if (put_user(mem->phy_addr, argp))
> +			return -EFAULT;
> +
> +		break;
> +	case FBIO_FREE:
> +		if (get_user(offset, argp))
> +			return -EFAULT;
> +
> +		retval = -EINVAL;
> +		mutex_lock(&mx3_fbi->mutex);
> +		list_for_each_entry(mem, &fb_alloc_list, list) {
> +			if (mem->phy_addr == offset) {
> +				list_del(&mem->list);
> +				dma_free_coherent(fbi->device,
> +						  mem->size,
> +						  mem->cpu_addr,
> +						  mem->phy_addr);
> +				kfree(mem);
> +				retval = 0;
> +				break;
> +			}
> +		}
> +		mutex_unlock(&mx3_fbi->mutex);
> +
> +		break;
> +	case MXCFB_SET_GBL_ALPHA:

Are you using these proprietary ioctl()s? If not, I wouldn't implement 
them. If you do need them, maybe it would make sense to add such ioctl()s 
globally for fbdev?

> +		if (copy_from_user(&ga, (void *)arg, sizeof(ga)))
> +			retval = -EFAULT;
> +
> +		sdc_set_global_alpha(mx3fb, (bool)ga.enable, ga.alpha);
> +		dev_dbg(fbi->device, "Set global alpha to %d\n", ga.alpha);
> +
> +		break;
> +	case MXCFB_SET_CLR_KEY:
> +		if (copy_from_user(&key, (void *)arg, sizeof(key)))
> +			retval = -EFAULT;
> +
> +		sdc_set_color_key(mx3fb, (bool)key.enable, key.color_key);
> +		dev_dbg(fbi->device, "Set color key to %d\n", key.color_key);
> +
> +		break;
> +	default:
> +		retval = -EINVAL;
> +	}
> +
> +	return retval;
> +}
> +
> +static struct fb_ops mx3fb_ovl_ops = {
> +	.owner = THIS_MODULE,
> +	.fb_set_par = mx3fb_set_par,
> +	.fb_check_var = mx3fb_check_var,
> +	.fb_setcolreg = mx3fb_setcolreg,
> +	.fb_pan_display = mx3fb_pan_display,
> +	.fb_ioctl = mx3fb_ioctl_ovl,
> +	.fb_fillrect = cfb_fillrect,
> +	.fb_copyarea = cfb_copyarea,
> +	.fb_imageblit = cfb_imageblit,
> +	.fb_blank = mx3fb_blank_ovl,
> +};
> +#endif
> +
>  #ifdef CONFIG_PM
>  /*
>   * Power management hooks.      Note that we won't be called from IRQ context,
> @@ -1164,11 +1355,16 @@ static int mx3fb_suspend(struct platform_device *pdev, pm_message_t state)
>  {
>  	struct mx3fb_data *mx3fb = platform_get_drvdata(pdev);
>  	struct mx3fb_info *mx3_fbi = mx3fb->fbi->par;
> +	struct mx3fb_info *mx3_fbi_ovl = mx3fb->fbi_ovl->par;
>  
>  	console_lock();
>  	fb_set_suspend(mx3fb->fbi, 1);
> +	fb_set_suspend(mx3fb->fbi_ovl, 1);
>  	console_unlock();
>  
> +	if (mx3_fbi_ovl->blank == FB_BLANK_UNBLANK)
> +		sdc_disable_channel(mx3_fbi_ovl);
> +
>  	if (mx3_fbi->blank == FB_BLANK_UNBLANK) {
>  		sdc_disable_channel(mx3_fbi);
>  		sdc_set_brightness(mx3fb, 0);
> @@ -1184,14 +1380,19 @@ static int mx3fb_resume(struct platform_device *pdev)
>  {
>  	struct mx3fb_data *mx3fb = platform_get_drvdata(pdev);
>  	struct mx3fb_info *mx3_fbi = mx3fb->fbi->par;
> +	struct mx3fb_info *mx3_fbi_ovl = mx3fb->fbi_ovl->par;
>  
>  	if (mx3_fbi->blank == FB_BLANK_UNBLANK) {
>  		sdc_enable_channel(mx3_fbi);
>  		sdc_set_brightness(mx3fb, mx3fb->backlight_level);
>  	}
>  
> +	if (mx3_fbi_ovl->blank == FB_BLANK_UNBLANK)
> +		sdc_enable_channel(mx3_fbi_ovl);
> +
>  	console_lock();
>  	fb_set_suspend(mx3fb->fbi, 0);
> +	fb_set_suspend(mx3fb->fbi_ovl, 0);
>  	console_unlock();
>  
>  	return 0;
> @@ -1333,8 +1534,8 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
>  	ichan->client = mx3fb;
>  	irq = ichan->eof_irq;
>  
> -	if (ichan->dma_chan.chan_id != IDMAC_SDC_0)
> -		return -EINVAL;
> +	switch (ichan->dma_chan.chan_id) {
> +	case IDMAC_SDC_0:
>  
>  	fbi = mx3fb_init_fbinfo(dev, &mx3fb_ops);

I would bite the bullet and indent this case block...

>  	if (!fbi)
> @@ -1375,7 +1576,29 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
>  
>  	sdc_set_brightness(mx3fb, 255);
>  	sdc_set_global_alpha(mx3fb, true, 0xFF);
> -	sdc_set_color_key(mx3fb, IDMAC_SDC_0, false, 0);
> +	sdc_set_color_key(mx3fb, false, 0);
> +
> +	break;
> +#ifdef CONFIG_FB_MX3_OVERLAY
> +	case IDMAC_SDC_1:
> +
> +		/* We know, that background has been allocated already! */
> +		fbi = mx3fb_init_fbinfo(dev, &mx3fb_ovl_ops);
> +		if (!fbi)
> +			return -ENOMEM;
> +
> +		/* Default Y virtual size is 2x panel size */
> +		fbi->var = mx3fb->fbi->var;
> +		/* This shouldn't be necessary, it is already set up above */
> +		fbi->var.yres_virtual = mx3fb->fbi->var.yres * 2;
> +
> +		mx3fb->fbi_ovl = fbi;
> +
> +		break;
> +#endif
> +	default:
> +		return -EINVAL;
> +	}
>  
>  	mx3fbi			= fbi->par;
>  	mx3fbi->idmac_channel	= ichan;
> @@ -1392,9 +1615,13 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
>  	if (ret < 0)
>  		goto esetpar;
>  
> -	__blank(FB_BLANK_UNBLANK, fbi);
> +	/* Overlay stays blanked by default */
> +	if (ichan->dma_chan.chan_id == IDMAC_SDC_0) {
> +		mx3fb_blank(FB_BLANK_UNBLANK, fbi);
>  
> -	dev_info(dev, "registered, using mode %s\n", fb_mode);
> +		dev_info(dev, "mx3fb: fb registered, using mode %s [%c]\n",
> +		fb_mode, list_empty(&ichan->queue) ? '-' : '+');
> +	}
>  
>  	ret = register_framebuffer(fbi);
>  	if (ret < 0)
> @@ -1492,14 +1719,42 @@ static int mx3fb_probe(struct platform_device *pdev)
>  
>  	mx3fb->backlight_level = 255;
>  
> +	mx3fb->idmac_channel[0] = to_idmac_chan(chan);
> +	mx3fb->idmac_channel[0]->client = mx3fb;
> +
>  	ret = init_fb_chan(mx3fb, to_idmac_chan(chan));
>  	if (ret < 0)
>  		goto eisdc0;
>  
> +#ifdef CONFIG_FB_MX3_OVERLAY
> +	dma_cap_zero(mask);
> +	dma_cap_set(DMA_SLAVE, mask);
> +	dma_cap_set(DMA_PRIVATE, mask);
> +	rq.id = IDMAC_SDC_1;
> +	chan = dma_request_channel(mask, chan_filter, &rq);
> +	if (!chan) {
> +		ret = -EBUSY;
> +		goto ersdc1;
> +	}
> +
> +	mx3fb->idmac_channel[1] = to_idmac_chan(chan);
> +	mx3fb->idmac_channel[1]->client = mx3fb;
> +
> +	ret = init_fb_chan(mx3fb, to_idmac_chan(chan));
> +	if (ret < 0)
> +		goto eisdc1;
> +#endif
> +
>  	return 0;
>  
> +#ifdef CONFIG_FB_MX3_OVERLAY
> +eisdc1:
> +	dma_release_channel(&mx3fb->idmac_channel[1]->dma_chan);
> +ersdc1:
> +	release_fbi(mx3fb->fbi);
> +#endif
>  eisdc0:
> -	dma_release_channel(chan);
> +	dma_release_channel(&mx3fb->idmac_channel[0]->dma_chan);
>  ersdc0:
>  	dmaengine_put();
>  	iounmap(mx3fb->reg_base);
> @@ -1513,13 +1768,16 @@ static int mx3fb_remove(struct platform_device *dev)
>  {
>  	struct mx3fb_data *mx3fb = platform_get_drvdata(dev);
>  	struct fb_info *fbi = mx3fb->fbi;
> -	struct mx3fb_info *mx3_fbi = fbi->par;
> -	struct dma_chan *chan;
>  
> -	chan = &mx3_fbi->idmac_channel->dma_chan;
> -	release_fbi(fbi);
> +	if (fbi)
> +		release_fbi(fbi);
> +
> +	fbi = mx3fb->fbi_ovl;
> +	if (fbi)
> +		release_fbi(fbi);
>  
> -	dma_release_channel(chan);
> +	dma_release_channel(&mx3fb->idmac_channel[1]->dma_chan);
> +	dma_release_channel(&mx3fb->idmac_channel[0]->dma_chan);
>  	dmaengine_put();
>  
>  	iounmap(mx3fb->reg_base);
> diff --git a/include/linux/mxcfb.h b/include/linux/mxcfb.h
> new file mode 100644
> index 0000000..54b720d
> --- /dev/null
> +++ b/include/linux/mxcfb.h
> @@ -0,0 +1,93 @@
> +/*
> + * File: include/linux/mxcfb.h
> + * Global header file for the MXC Framebuffer
> + *
> + * Copyright 2004-2012 Freescale Semiconductor, Inc. All Rights Reserved.
> + *
> + * The code contained herein is licensed under the GNU Lesser General
> + * Public License.  You may obtain a copy of the GNU Lesser General
> + * Public License Version 2.1 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/lgpl-license.html
> + * http://www.gnu.org/copyleft/lgpl.html
> + */
> +
> +#ifndef __LINUX_MXCFB_H__
> +#define __LINUX_MXCFB_H__
> +
> +#include <linux/fb.h>

Why is this needed here?

> +
> +#define FB_SYNC_OE_LOW_ACT	0x80000000
> +#define FB_SYNC_CLK_LAT_FALL	0x40000000
> +#define FB_SYNC_DATA_INVERT	0x20000000
> +#define FB_SYNC_CLK_IDLE_EN	0x10000000
> +#define FB_SYNC_SHARP_MODE	0x08000000
> +#define FB_SYNC_SWAP_RGB	0x04000000
> +
> +struct mxcfb_gbl_alpha {
> +	int enable;
> +	int alpha;
> +};
> +
> +struct mxcfb_color_key {
> +	int enable;
> +	__u32 color_key;
> +};
> +
> +struct mxcfb_pos {
> +	__u16 x;
> +	__u16 y;
> +};
> +
> +struct mxcfb_gamma {
> +	int enable;
> +	int constk[16];
> +	int slopek[16];
> +};
> +
> +struct mxcfb_rect {
> +	__u32 top;
> +	__u32 left;
> +	__u32 width;
> +	__u32 height;
> +};
> +
> +/*
> + * Structure used to define waveform modes for driver
> + * Needed for driver to perform auto-waveform selection
> + */
> +struct mxcfb_waveform_modes {
> +	int mode_init;
> +	int mode_du;
> +	int mode_gc4;
> +	int mode_gc8;
> +	int mode_gc16;
> +	int mode_gc32;
> +};
> +
> +/* IOCTL commands. */
> +
> +#define MXCFB_WAIT_FOR_VSYNC		_IOW('F', 0x20, u_int32_t)
> +#define MXCFB_SET_GBL_ALPHA		_IOW('F', 0x21, struct mxcfb_gbl_alpha)
> +#define MXCFB_SET_CLR_KEY		_IOW('F', 0x22, struct mxcfb_color_key)
> +#define MXCFB_SET_OVERLAY_POS		_IOWR('F', 0x24, struct mxcfb_pos)
> +#define MXCFB_GET_FB_IPU_CHAN		_IOR('F', 0x25, u_int32_t)
> +#define MXCFB_SET_LOC_ALPHA		_IOWR('F', 0x26, struct mxcfb_loc_alpha)
> +#define MXCFB_SET_LOC_ALP_BUF		_IOW('F', 0x27, unsigned long)
> +#define MXCFB_SET_GAMMA			_IOW('F', 0x28, struct mxcfb_gamma)
> +#define MXCFB_GET_FB_IPU_DI		_IOR('F', 0x29, u_int32_t)
> +#define MXCFB_GET_DIFMT			_IOR('F', 0x2A, u_int32_t)
> +#define MXCFB_GET_FB_BLANK		_IOR('F', 0x2B, u_int32_t)

Please, don't add unused identifiers.

> +
> +#ifdef __KERNEL__
> +
> +enum {
> +	MXCFB_REFRESH_OFF,
> +	MXCFB_REFRESH_AUTO,
> +	MXCFB_REFRESH_PARTIAL,
> +};
> +
> +#endif
> +
> +#endif /* _MXCFB_H */
> +
> -- 
> 1.7.0.4
> 

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux