Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* RE: [PATCH v1] ARM: i.mx: mx3fb: add overlay support
From: Alex Gershgorin @ 2012-04-20 15:38 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Florian Tobias Schandinat, s.hauer@pengutronix.de,
	laurent.pinchart@ideasonboard.com, linux-fbdev@vger.kernel.org,
	linux-media@vger.kernel.org
In-Reply-To: <Pine.LNX.4.64.1204181952580.30514@axis700.grange>

Hi Guennadi,

Thanks for your review.

> > Thanks for reviving, fixing and submitting this code!

I think that this code is beneficial  :-)

> > 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@meprolight.com>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

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

I would like to thank all the authors of original code.
unfortunately I can't thank for each one of you separately by name, i hope
that you understand and accept it.

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

This option is more suitable, I just correct the description of the patch,
and leave your signature (if you have any objections?) since 2008 patch version.

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

 This is descibed bellow... it can will move into struct mx3fb_info ...

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

     Already fixed
>
>       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.

I will do it, list-head will go inoto 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?

 Unfortunately yes ...

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

It wiil be nice... but I think for this will need a separate patch?


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

This makes a clear separation between the framebuffer and overlay
channels during initializing, but if you have any ideas welcome, please
send, I could do a test on my hardware :-)

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

I checked, it is not necessary

> +
> +#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.

Yes, I can remove something.

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

Thanks
Alex

^ permalink raw reply

* RE: [PATCH v1] ARM: i.mx: mx3fb: add overlay support
From: Guennadi Liakhovetski @ 2012-04-20 15:54 UTC (permalink / raw)
  To: Alex Gershgorin
  Cc: Florian Tobias Schandinat, s.hauer@pengutronix.de,
	laurent.pinchart@ideasonboard.com, linux-fbdev@vger.kernel.org,
	linux-media@vger.kernel.org
In-Reply-To: <4875438356E7CA4A8F2145FCD3E61C0B2CC952548B@MEP-EXCH.meprolight.com>

Hi Alex

On Fri, 20 Apr 2012, Alex Gershgorin wrote:

[snip]

> > Signed-off-by: Alex Gershgorin <alexg@meprolight.com>
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> 
> > > 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,
> 
> I would like to thank all the authors of original code.
> unfortunately I can't thank for each one of you separately by name, i hope
> that you understand and accept it.
> 
> >>  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.
> 
> This option is more suitable, I just correct the description of the patch,
> and leave your signature (if you have any objections?) since 2008 patch version.

Well, if you wish so...:-) To me it looks like a new patch from you, 
that's just vaguely based on my previous patch, that was copying some 
previous work, so, my contribution to this code isn't huge;-) But if you 
insist - you can keep my Sob, but at least put it above yours.

[snip]

> > @@ -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...
> 
> This makes a clear separation between the framebuffer and overlay
> channels during initializing, but if you have any ideas welcome, please
> send, I could do a test on my hardware :-)

Sorry, I didn't mean any functional change, just a pure formatting issue: 
you put a "switch-case" statement above, but didn't add an indentation 
level to the following code. While reducing the patch size by avoiding 
unnecessary changes is good, I think, following the coding style and 
improving readability are more important arguments here, so, I would go 
and do that "unnecessary" change and indent the code.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply

* Re: [PATCH 2/4] misc: add LM3533 ambient light sensor driver
From: Greg Kroah-Hartman @ 2012-04-20 15:57 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Richard Purdie, Samuel Ortiz, Arnd Bergmann,
	Florian Tobias Schandinat, Andrew Morton, linux-kernel,
	linux-fbdev
In-Reply-To: <1334935826-12527-3-git-send-email-jhovold@gmail.com>

On Fri, Apr 20, 2012 at 05:30:24PM +0200, Johan Hovold wrote:
> Add sub-driver for the ambient light sensor in National Semiconductor /
> TI LM3533 lighting power chips.
> 
> Raw ADC values as well as current ALS zone can be retrieved through
> sysfs. The ALS zone can also be read using a character device
> (/dev/lm3533-als) which is updated on zone changes (interrupt driven or
> polled).
> 
> The driver provides a configuration interface through sysfs.

Which seems to not be documented at all :(

What about using the iio interface for this instead?  Doesn't that
already provide this standard interface you are looking for?

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH 3/4] leds: add LM3533 LED driver
From: Arnd Bergmann @ 2012-04-20 16:10 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Richard Purdie, Samuel Ortiz, Greg Kroah-Hartman,
	Florian Tobias Schandinat, Andrew Morton, linux-kernel,
	linux-fbdev
In-Reply-To: <1334935826-12527-4-git-send-email-jhovold@gmail.com>

On Friday 20 April 2012, Johan Hovold wrote:
> Add sub-driver for the LEDs in National Semiconductor / TI LM3533
> lighting power chips.
> 
> The chip provides 256 brightness levels, hardware accelerated blinking
> as well as ambient-light-sensor and pwm input control.
> 
> Signed-off-by: Johan Hovold <jhovold@gmail.com>

I notice that there is already driver for lm3530, which sounds related.
Is there an opportunity to share code between these, or are they completely
different devices?

> +
> +#define show_ctrlbank_attr(_name)					\
> +static ssize_t show_##_name(struct device *dev,				\
> +				struct device_attribute *attr,		\
> +				char *buf)				\
> +{									\
> +	struct led_classdev *led_cdev = dev_get_drvdata(dev);		\
> +	struct lm3533_led *led = to_lm3533_led(led_cdev);		\
> +	u8 val;								\
> +	int ret;							\
> +									\
> +	ret = lm3533_ctrlbank_get_##_name(&led->cb, &val);		\
> +	if (ret)							\
> +		return ret;						\
> +									\
> +	return scnprintf(buf, PAGE_SIZE, "%d\n", val);			\
> +}

IMHO this macro adds more in terms of complexity than it saves in terms
of lines of code, and it would be better to open-code the two instances.
If you need more than two or three instances, I would recommend creating
keying the number off of the attribute pointer, either by comparing the
pointer or by adding a data structure derived from device_attribute and
using container_of to get at the other data.

	Arnd

^ permalink raw reply

* Re: [PATCH 3/4] leds: add LM3533 LED driver
From: Johan Hovold @ 2012-04-20 16:45 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Richard Purdie, Samuel Ortiz, Greg Kroah-Hartman,
	Florian Tobias Schandinat, Andrew Morton, linux-kernel,
	linux-fbdev
In-Reply-To: <201204201610.15758.arnd@arndb.de>

On Fri, Apr 20, 2012 at 04:10:15PM +0000, Arnd Bergmann wrote:
> On Friday 20 April 2012, Johan Hovold wrote:
> > Add sub-driver for the LEDs in National Semiconductor / TI LM3533
> > lighting power chips.
> > 
> > The chip provides 256 brightness levels, hardware accelerated blinking
> > as well as ambient-light-sensor and pwm input control.
> > 
> > Signed-off-by: Johan Hovold <jhovold@gmail.com>
> 
> I notice that there is already driver for lm3530, which sounds related.
> Is there an opportunity to share code between these, or are they completely
> different devices?

Unfortunately not. They are really very different devices despite
similar naming and terminology.

> > +
> > +#define show_ctrlbank_attr(_name)					\
> > +static ssize_t show_##_name(struct device *dev,				\
> > +				struct device_attribute *attr,		\
> > +				char *buf)				\
> > +{									\
> > +	struct led_classdev *led_cdev = dev_get_drvdata(dev);		\
> > +	struct lm3533_led *led = to_lm3533_led(led_cdev);		\
> > +	u8 val;								\
> > +	int ret;							\
> > +									\
> > +	ret = lm3533_ctrlbank_get_##_name(&led->cb, &val);		\
> > +	if (ret)							\
> > +		return ret;						\
> > +									\
> > +	return scnprintf(buf, PAGE_SIZE, "%d\n", val);			\
> > +}
> 
> IMHO this macro adds more in terms of complexity than it saves in terms
> of lines of code, and it would be better to open-code the two instances.
> If you need more than two or three instances, I would recommend creating
> keying the number off of the attribute pointer, either by comparing the
> pointer or by adding a data structure derived from device_attribute and
> using container_of to get at the other data.

Agreed. I'll simply open-code them for now, and do the same with the
equivalent instances in the backlight driver.

Thanks,
Johan

^ permalink raw reply

* Re: [PATCH 2/4] misc: add LM3533 ambient light sensor driver
From: Johan Hovold @ 2012-04-20 17:28 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Richard Purdie, Samuel Ortiz, Arnd Bergmann,
	Florian Tobias Schandinat, Andrew Morton, linux-kernel,
	linux-fbdev
In-Reply-To: <20120420155734.GE8327@kroah.com>

On Fri, Apr 20, 2012 at 08:57:34AM -0700, Greg Kroah-Hartman wrote:
> On Fri, Apr 20, 2012 at 05:30:24PM +0200, Johan Hovold wrote:
> > Add sub-driver for the ambient light sensor in National Semiconductor /
> > TI LM3533 lighting power chips.
> > 
> > Raw ADC values as well as current ALS zone can be retrieved through
> > sysfs. The ALS zone can also be read using a character device
> > (/dev/lm3533-als) which is updated on zone changes (interrupt driven or
> > polled).
> > 
> > The driver provides a configuration interface through sysfs.
> 
> Which seems to not be documented at all :(

There are the following sysfs entries for configuring ALS control:

boundary0_high
boundary0_low
boundary1_high
boundary1_low
boundary2_high
boundary2_low
boundary3_high
boundary3_low

gain

target1_0
target1_1
target1_2
target1_3
target1_4

target2_0
target2_1
target2_2
target2_3
target2_4

target3_0
target3_1
target3_2
target3_3
target3_4

These define the "five light zones and three sets of corresponding
brightness target levels" mentioned in the Kconfig entry and provides a
gain setting.

Each entry also corresponds to an 8-bit register, which is documented
along with the overall ALS functionality in the datasheets (which will
be published on the TI web page soon). So I think anyone integrating
this IC (or anyone who has access to the datasheets) will have no
problem with this interface, but I'd be happy to write something to put
under Documentation as well.

The end-customer insisted on sysfs configurability, but I'll probably
add these settings to the platform data later as well. 

> What about using the iio interface for this instead?  Doesn't that
> already provide this standard interface you are looking for?

I had a look at iio last fall and decided not to use it at the time. I
can't remember exactly what the reasons were right now, so I'll have
to get back to you on this.

Thanks,
Johan

^ permalink raw reply

* Re: [PATCH 2/4] misc: add LM3533 ambient light sensor driver
From: Greg Kroah-Hartman @ 2012-04-20 17:37 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Richard Purdie, Samuel Ortiz, Arnd Bergmann,
	Florian Tobias Schandinat, Andrew Morton, linux-kernel,
	linux-fbdev
In-Reply-To: <20120420172849.GB12376@localhost>

On Fri, Apr 20, 2012 at 07:28:49PM +0200, Johan Hovold wrote:
> On Fri, Apr 20, 2012 at 08:57:34AM -0700, Greg Kroah-Hartman wrote:
> > On Fri, Apr 20, 2012 at 05:30:24PM +0200, Johan Hovold wrote:
> > > Add sub-driver for the ambient light sensor in National Semiconductor /
> > > TI LM3533 lighting power chips.
> > > 
> > > Raw ADC values as well as current ALS zone can be retrieved through
> > > sysfs. The ALS zone can also be read using a character device
> > > (/dev/lm3533-als) which is updated on zone changes (interrupt driven or
> > > polled).
> > > 
> > > The driver provides a configuration interface through sysfs.
> > 
> > Which seems to not be documented at all :(
> 
> There are the following sysfs entries for configuring ALS control:

<snip>

That's fine, but you need a Documentation/ABI entry for any new sysfs
file you create.
> > What about using the iio interface for this instead?  Doesn't that
> > already provide this standard interface you are looking for?
> 
> I had a look at iio last fall and decided not to use it at the time. I
> can't remember exactly what the reasons were right now, so I'll have
> to get back to you on this.

Please look into this, the iio framework is about to move out of the
staging tree for 3.5, so there should not be any reason for you to
create yet-another user api for this type of thing.

thanks,

greg k-h

^ permalink raw reply

* [PATCH] video/smscufx: fix line counting in fb_write
From: Alexander Holler @ 2012-04-20 22:11 UTC (permalink / raw)
  To: linux-kernel; +Cc: Steve Glendinning, linux-fbdev, Alexander Holler

Line 0 and 1 were both written to line 0 (on the display) and all subsequent
lines had an offset of -1. The result was that the last line on the display
was never overwritten by writes to /dev/fbN.

The origin of this bug seems to have been udlfb.

Signed-off-by: Alexander Holler <holler@ahsoftware.de>
---
 drivers/video/smscufx.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/video/smscufx.c b/drivers/video/smscufx.c
index ccbfef5..1e1e2d2 100644
--- a/drivers/video/smscufx.c
+++ b/drivers/video/smscufx.c
@@ -904,7 +904,7 @@ static ssize_t ufx_ops_write(struct fb_info *info, const char __user *buf,
 	result = fb_sys_write(info, buf, count, ppos);
 
 	if (result > 0) {
-		int start = max((int)(offset / info->fix.line_length) - 1, 0);
+		int start = max((int)(offset / info->fix.line_length), 0);
 		int lines = min((u32)((result / info->fix.line_length) + 1),
 				(u32)info->var.yres);
 
-- 
1.7.6.5


^ permalink raw reply related

* Re: [PATCH] video/udlfb: fix line counting in fb_write
From: Alexander Holler @ 2012-04-21 11:26 UTC (permalink / raw)
  To: Alexander Holler; +Cc: linux-kernel, Bernie Thompson, linux-fbdev
In-Reply-To: <1334959844-3665-1-git-send-email-holler@ahsoftware.de>

Hello,

sorry, I forgot to mention that this is a candidate for all stable trees 
2.6.37 and above. Not sure how to announce that, but I think I should 
have added the below Cc: to the patch.

Regards,

Alexander

Am 21.04.2012 00:10, schrieb Alexander Holler:
> Line 0 and 1 were both written to line 0 (on the display) and all subsequent
> lines had an offset of -1. The result was that the last line on the display
> was never overwritten by writes to /dev/fbN.
>
> Signed-off-by: Alexander Holler<holler@ahsoftware.de>

Cc: stable@vger.kernel.org

> ---
>   drivers/video/udlfb.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/video/udlfb.c b/drivers/video/udlfb.c
> index a159b63..85d8110 100644
> --- a/drivers/video/udlfb.c
> +++ b/drivers/video/udlfb.c
> @@ -647,7 +647,7 @@ static ssize_t dlfb_ops_write(struct fb_info *info, const char __user *buf,
>   	result = fb_sys_write(info, buf, count, ppos);
>
>   	if (result>  0) {
> -		int start = max((int)(offset / info->fix.line_length) - 1, 0);
> +		int start = max((int)(offset / info->fix.line_length), 0);
>   		int lines = min((u32)((result / info->fix.line_length) + 1),
>   				(u32)info->var.yres);
>


^ permalink raw reply

* Re: [PATCH] video/smscufx: fix line counting in fb_write
From: Alexander Holler @ 2012-04-21 11:26 UTC (permalink / raw)
  To: Alexander Holler; +Cc: linux-kernel, linux-fbdev
In-Reply-To: <1334959867-3719-1-git-send-email-holler@ahsoftware.de>

Hello,

as for the patch for udlfb, I forgot to mention that this is a candidate 
for all stable trees 3.2 and above.

Btw., the address of the maintainer doesn't seem to be valid anymore.

Regards,

Alexander

Am 21.04.2012 00:11, schrieb Alexander Holler:
> Line 0 and 1 were both written to line 0 (on the display) and all subsequent
> lines had an offset of -1. The result was that the last line on the display
> was never overwritten by writes to /dev/fbN.
>
> The origin of this bug seems to have been udlfb.
>
> Signed-off-by: Alexander Holler<holler@ahsoftware.de>

Cc: stable@vger.kernel.org

> ---
>   drivers/video/smscufx.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/video/smscufx.c b/drivers/video/smscufx.c
> index ccbfef5..1e1e2d2 100644
> --- a/drivers/video/smscufx.c
> +++ b/drivers/video/smscufx.c
> @@ -904,7 +904,7 @@ static ssize_t ufx_ops_write(struct fb_info *info, const char __user *buf,
>   	result = fb_sys_write(info, buf, count, ppos);
>
>   	if (result>  0) {
> -		int start = max((int)(offset / info->fix.line_length) - 1, 0);
> +		int start = max((int)(offset / info->fix.line_length), 0);
>   		int lines = min((u32)((result / info->fix.line_length) + 1),
>   				(u32)info->var.yres);
>


^ permalink raw reply

* [PATCH] m68k/video: Create <asm/vga.h>
From: Geert Uytterhoeven @ 2012-04-21 20:16 UTC (permalink / raw)
  To: linux-m68k; +Cc: linux-kernel, Geert Uytterhoeven, linux-fbdev, dri-devel

For now, it just contains the hack for cirrusfb on Amiga, which is moved
out of <video/vga.h> with some slight modifications (use raw_*() instead of
z_*(), which are defined on all m68k platforms).

This makes it safe to include <video/vga.h> in all contexts. Before it
could fail to compile with

include/video/vga.h: In function ‘vga_mm_r’:
include/video/vga.h:242: error: implicit declaration of function ‘z_readb’
include/video/vga.h: In function ‘vga_mm_w’:
include/video/vga.h:247: error: implicit declaration of function ‘z_writeb’
include/video/vga.h: In function ‘vga_mm_w_fast’:
include/video/vga.h:253: error: implicit declaration of function ‘z_writew’

or

include/video/vga.h:23:21: error: asm/vga.h: No such file or directory

depending on the value of CONFIG_AMIGA.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: linux-fbdev@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
---
Will queue for 3.5, cfr.
http://kisskb.ellerman.id.au/kisskb/buildresult/6127136/
---
 arch/m68k/include/asm/vga.h |   27 +++++++++++++++++++++++++++
 include/video/vga.h         |   22 ----------------------
 2 files changed, 27 insertions(+), 22 deletions(-)
 create mode 100644 arch/m68k/include/asm/vga.h

diff --git a/arch/m68k/include/asm/vga.h b/arch/m68k/include/asm/vga.h
new file mode 100644
index 0000000..d3aa140
--- /dev/null
+++ b/arch/m68k/include/asm/vga.h
@@ -0,0 +1,27 @@
+#ifndef _ASM_M68K_VGA_H
+#define _ASM_M68K_VGA_H
+
+#include <asm/raw_io.h>
+
+/*
+ * FIXME
+ * Ugh, we don't have PCI space, so map readb() and friends to use raw I/O
+ * accessors, which are identical to the z_*() Zorro bus accessors.
+ * This should make cirrusfb work again on Amiga
+ */
+#undef inb_p
+#undef inw_p
+#undef outb_p
+#undef outw
+#undef readb
+#undef writeb
+#undef writew
+#define inb_p(port)		0
+#define inw_p(port)		0
+#define outb_p(port, val)	do { } while (0)
+#define outw(port, val)		do { } while (0)
+#define readb			raw_inb
+#define writeb			raw_outb
+#define writew			raw_outw
+
+#endif /* _ASM_M68K_VGA_H */
diff --git a/include/video/vga.h b/include/video/vga.h
index 2b8691f..cac567f 100644
--- a/include/video/vga.h
+++ b/include/video/vga.h
@@ -19,29 +19,7 @@
 
 #include <linux/types.h>
 #include <asm/io.h>
-#ifndef CONFIG_AMIGA
 #include <asm/vga.h>
-#else
-/*
- * FIXME
- * Ugh, we don't have PCI space, so map readb() and friends to use Zorro space
- * for MMIO accesses. This should make cirrusfb work again on Amiga
- */
-#undef inb_p
-#undef inw_p
-#undef outb_p
-#undef outw
-#undef readb
-#undef writeb
-#undef writew
-#define inb_p(port)	0
-#define inw_p(port)	0
-#define outb_p(port, val)	do { } while (0)
-#define outw(port, val)		do { } while (0)
-#define readb		z_readb
-#define writeb		z_writeb
-#define writew		z_writew
-#endif
 #include <asm/byteorder.h>
 
 
-- 
1.7.0.4


^ permalink raw reply related

* Dear Sir/Madam
From: Mr. Alan Johnson @ 2012-04-22  3:40 UTC (permalink / raw)





Dear Sir/Madam,

We welcome you to a whole new brand of secured lending establishment
Guaranteed Trust Loan Firm Malaysia, We are Loan specialists helping
theneedy to gain financial stability grant debt consolidation loans,
businessloans project loans, private loans,interested contact us via
e-mail:
guaranteed_my@yahoo.co.jp

Mr. Alan Johnson.
Officer, Applicant inquiries Unit
at GTLF.


^ permalink raw reply

* [PATCH] drivers/video/xen-fbfront.c: add missing cleanup code
From: Julia Lawall @ 2012-04-22  9:57 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: kernel-janitors, Jeremy Fitzhardinge, Florian Tobias Schandinat,
	xen-devel, virtualization, linux-fbdev, linux-kernel

From: Julia Lawall <Julia.Lawall@lip6.fr>

The operations in the subsequent error-handling code appear to be also
useful here.

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
Not tested.

 drivers/video/xen-fbfront.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/video/xen-fbfront.c b/drivers/video/xen-fbfront.c
index cb4529c..b0bd59c 100644
--- a/drivers/video/xen-fbfront.c
+++ b/drivers/video/xen-fbfront.c
@@ -458,8 +458,13 @@ static int __devinit xenfb_probe(struct xenbus_device *dev,
 	xenfb_init_shared_page(info, fb_info);
 
 	ret = xenfb_connect_backend(dev, info);
-	if (ret < 0)
+	if (ret < 0) {
+		fb_deferred_io_cleanup(fb_info);
+		fb_dealloc_cmap(&fb_info->cmap);
+		framebuffer_release(fb_info);
+		xenbus_dev_fatal(dev, ret, "xenfb_connect_backend");
 		goto error;
+	}
 
 	ret = register_framebuffer(fb_info);
 	if (ret) {


^ permalink raw reply related

* [PATCH v2] ARM: i.mx: mx3fb: add overlay support
From: Alex Gershgorin @ 2012-04-22 13:31 UTC (permalink / raw)
  To: Florian Tobias Schandinat
  Cc: g.liakhovetski, s.hauer, laurent.pinchart, linux-fbdev,
	linux-media, Alex Gershgorin

This work is based on some earlier patch series
"i.MX31: dmaengine and framebuffer drivers" from 2008
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: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Signed-off-by: Alex Gershgorin <alexg@meprolight.com>
---
Applies to v3.4-rc4

Changes since v1:
  *Some fixes after review
  *Added ioctl for setting overlay windows position
---
 drivers/video/Kconfig |    7 +
 drivers/video/mx3fb.c |  346 ++++++++++++++++++++++++++++++++++++++++++++----
 include/linux/mxcfb.h |   40 ++++++
 3 files changed, 364 insertions(+), 29 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..09d7885 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 {
@@ -268,10 +273,19 @@ struct mx3fb_info {
 	struct dma_async_tx_descriptor	*txd;
 	dma_cookie_t			cookie;
 	struct scatterlist		sg[2];
+	struct list_head		ovl_list; /* overlay buffer list */
 
 	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;
+};
+
 static void mx3fb_dma_done(void *);
 
 /* Used fb-mode and bpp. Can be set on kernel command line, therefore file-static. */
@@ -303,7 +317,13 @@ 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;
+		INIT_LIST_HEAD(&fbi->ovl_list);
+	}
 
 	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,33 @@ 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);
+/*
+   If the overlay is defined, we just need to swap
+   the window position register relative to DMAC channel ID
+*/
+	switch (channel) {
+#ifndef CONFIG_FB_MX3_OVERLAY
+	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;
+#else
+	case IDMAC_SDC_0:
+		mx3fb_write_reg(mx3fb, (x_pos << 16) | y_pos, SDC_FG_POS);
+		break;
+	case IDMAC_SDC_1:
+		mx3fb_write_reg(mx3fb, (x_pos << 16) | y_pos, SDC_BG_POS);
+		break;
+#endif
+	default:
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
@@ -482,14 +533,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 +617,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 +630,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 +717,16 @@ 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;
 
+#ifndef CONFIG_FB_MX3_OVERLAY
 	strncpy(fix->id, "DISP3 BG", 8);
+#else
+	if (mx3_fbi->ipu_ch = IDMAC_SDC_1)
+		strncpy(fix->id, "DISP3 BG", 8);
+	else
+		strncpy(fix->id, "DISP3 FG", 8);
+#endif
 
 	fix->line_length = var->xres_virtual * var->bits_per_pixel / 8;
 
@@ -689,13 +746,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 +1220,157 @@ 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;
+	int retval = 0;
+	int __user *argp = (void __user *)arg;
+	struct mx3fb_alloc_list *mem;
+	int size;
+	unsigned long offset;
+	union {
+		struct mxcfb_pos pos;
+		struct mxcfb_gbl_alpha ga;
+		struct mxcfb_color_key key;
+	} s;
+
+	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, &mx3_fbi->ovl_list);
+		mutex_unlock(&mx3_fbi->mutex);
+
+		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, &mx3_fbi->ovl_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:
+		if (copy_from_user(&s.ga, (void *)arg, sizeof(s.ga)))
+			retval = -EFAULT;
+
+		sdc_set_global_alpha(mx3fb, (bool)s.ga.enable, s.ga.alpha);
+		dev_dbg(fbi->device, "Set global alpha to %d\n", s.ga.alpha);
+
+		break;
+	case MXCFB_SET_CLR_KEY:
+		if (copy_from_user(&s.key, (void *)arg, sizeof(s.key)))
+			retval = -EFAULT;
+
+		sdc_set_color_key(mx3fb, (bool)s.key.enable, s.key.color_key);
+		dev_dbg(fbi->device, "Set color key to %d\n", s.key.color_key);
+
+		break;
+	case MXCFB_SET_OVERLAY_POS:
+		if (copy_from_user(&s.pos, (void *)arg, sizeof(s.pos)))
+			retval = -EFAULT;
+
+		sdc_set_window_pos(mx3fb, mx3_fbi->ipu_ch, s.pos.x, s.pos.y);
+		dev_dbg(fbi->device, "Set win position to posx=%d, posy=%d\n",
+			s.pos.x, s.pos.y);
+
+		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 +1384,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 +1409,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 +1563,9 @@ 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);
 	if (!fbi)
@@ -1375,7 +1606,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 +1645,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 +1749,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 +1798,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..b5b681f
--- /dev/null
+++ b/include/linux/mxcfb.h
@@ -0,0 +1,40 @@
+/*
+ * 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__
+
+struct mxcfb_gbl_alpha {
+	int enable;
+	int alpha;
+};
+
+struct mxcfb_color_key {
+	int enable;
+	__u32 color_key;
+};
+
+struct mxcfb_pos {
+	__u16 x;
+	__u16 y;
+};
+
+/* IOCTL commands. */
+
+#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)
+
+#endif
+
-- 
1.7.0.4


^ permalink raw reply related

* Re: [PATCH V3 3/3] OMAPDSS: DISPC: Correct DISPC functional clock usage
From: Mahapatra, Chandrabhanu @ 2012-04-23  4:47 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1334932153.2058.40.camel@deskari>

On Fri, Apr 20, 2012 at 7:59 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Fri, 2012-04-20 at 19:01 +0530, Chandrabhanu Mahapatra wrote:
>> DISPC_FCLK is incorrectly used as functional clock of DISPC in scaling
>> calculations. So, DISPC_CORE_CLK replaces as functional clock of DISPC.
>> DISPC_CORE_CLK is derived from DISPC_FCLK divided by an independent DISPC
>> divisor LCD.
>>
>> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
>> ---
>>  drivers/video/omap2/dss/dispc.c |   25 +++++++++++++++++++------
>>  drivers/video/omap2/dss/dss.h   |    1 +
>>  2 files changed, 20 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
>> index 17ffa71..0417423 100644
>> --- a/drivers/video/omap2/dss/dispc.c
>> +++ b/drivers/video/omap2/dss/dispc.c
>> @@ -1855,7 +1855,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
>>                       fclk = calc_fclk(channel, in_width, in_height,
>>                                       out_width, out_height);
>>                       error = (in_width > maxsinglelinewidth || !fclk ||
>> -                             fclk > dispc_fclk_rate());
>> +                             fclk > dispc_core_clk_rate());
>
> The patch is now otherwise fine, but I think it needs some more
> renaming. Now the code mixes fclk and core-clk names, which is rather
> confusing. I guess the calc_fclk should actually be calc_req_core_clk?
> And the fclk variable core_clk (or cclk or something)?
>
>  Tomi
>

As per TRM dispc_core_clk is also a functional clock, so I think above
code should be fine. But, to avoid confusion of names the renaming of
fclk, calc_fclk and calc_fclk_five_taps can be done.


-- 
Chandrabhanu Mahapatra
Texas Instruments India Pvt. Ltd.

^ permalink raw reply

* Re: [PATCH] video: s3c-fb: add runtime_get/put to suspend/resume
From: Florian Tobias Schandinat @ 2012-04-23  6:15 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <001201cd1230$3c424a80$b4c6df80$%han@samsung.com>

On 04/04/2012 06:57 AM, Jingoo Han wrote:
> This patch adds runtime_get/put to suspend/resume, which should be
> necessary to prevent the problem to access the fimd register
> without block power on.
> 
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>

Applied.


Thanks,

Florian Tobias Schandinat

> ---
>  drivers/video/s3c-fb.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
> index f310516..76f3cf2 100644
> --- a/drivers/video/s3c-fb.c
> +++ b/drivers/video/s3c-fb.c
> @@ -1564,6 +1564,8 @@ static int s3c_fb_suspend(struct device *dev)
>  	struct s3c_fb_win *win;
>  	int win_no;
>  
> +	pm_runtime_get_sync(sfb->dev);
> +
>  	for (win_no = S3C_FB_MAX_WIN - 1; win_no >= 0; win_no--) {
>  		win = sfb->windows[win_no];
>  		if (!win)
> @@ -1577,6 +1579,9 @@ static int s3c_fb_suspend(struct device *dev)
>  		clk_disable(sfb->lcd_clk);
>  
>  	clk_disable(sfb->bus_clk);
> +
> +	pm_runtime_put_sync(sfb->dev);
> +
>  	return 0;
>  }
>  
> @@ -1589,6 +1594,8 @@ static int s3c_fb_resume(struct device *dev)
>  	int win_no;
>  	u32 reg;
>  
> +	pm_runtime_get_sync(sfb->dev);
> +
>  	clk_enable(sfb->bus_clk);
>  
>  	if (!sfb->variant.has_clksel)
> @@ -1633,6 +1640,8 @@ static int s3c_fb_resume(struct device *dev)
>  		s3c_fb_set_par(win->fbinfo);
>  	}
>  
> +	pm_runtime_put_sync(sfb->dev);
> +
>  	return 0;
>  }
>  #endif


^ permalink raw reply

* Re: [PATCH] video: exynos_dp: check DP PLL Lock status
From: Florian Tobias Schandinat @ 2012-04-23  6:15 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <001401cd1230$7846abc0$68d40340$%han@samsung.com>

On 04/04/2012 06:59 AM, Jingoo Han wrote:
> DP PLL Lock status should be checked in order to  prevent unlocked PLL.
> 
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>

Applied.


Thanks,

Florian Tobias Schandinat

> ---
>  drivers/video/exynos/exynos_dp_reg.c |   13 ++++++++++++-
>  1 files changed, 12 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
> index d9c66fd..661d010 100644
> --- a/drivers/video/exynos/exynos_dp_reg.c
> +++ b/drivers/video/exynos/exynos_dp_reg.c
> @@ -285,6 +285,7 @@ void exynos_dp_set_analog_power_down(struct exynos_dp_device *dp,
>  void exynos_dp_init_analog_func(struct exynos_dp_device *dp)
>  {
>  	u32 reg;
> +	int timeout_loop = 0;
>  
>  	exynos_dp_set_analog_power_down(dp, POWER_ALL, 0);
>  
> @@ -296,9 +297,19 @@ void exynos_dp_init_analog_func(struct exynos_dp_device *dp)
>  	writel(reg, dp->reg_base + EXYNOS_DP_DEBUG_CTL);
>  
>  	/* Power up PLL */
> -	if (exynos_dp_get_pll_lock_status(dp) = PLL_UNLOCKED)
> +	if (exynos_dp_get_pll_lock_status(dp) = PLL_UNLOCKED) {
>  		exynos_dp_set_pll_power_down(dp, 0);
>  
> +		while (exynos_dp_get_pll_lock_status(dp) = PLL_UNLOCKED) {
> +			timeout_loop++;
> +			if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) {
> +				dev_err(dp->dev, "failed to get pll lock status\n");
> +				return;
> +			}
> +			usleep_range(10, 20);
> +		}
> +	}
> +
>  	/* Enable Serdes FIFO function and Link symbol clock domain module */
>  	reg = readl(dp->reg_base + EXYNOS_DP_FUNC_EN_2);
>  	reg &= ~(SERDES_FIFO_FUNC_EN_N | LS_CLK_DOMAIN_FUNC_EN_N


^ permalink raw reply

* Re: [PATCH] video: exynos_dp: replace char pointer with char array for adjust_request variable
From: Florian Tobias Schandinat @ 2012-04-23  6:16 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <001501cd1230$8d4211e0$a7c635a0$%han@samsung.com>

On 04/04/2012 07:00 AM, Jingoo Han wrote:
> The char pointer for adjust_request variable is replaced with char array
> to fix possible null pointer dereference when clock recovery is failed.
> 
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>

Applied.


Thanks,

Florian Tobias Schandinat

> ---
>  drivers/video/exynos/exynos_dp_core.c |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> index 2a4481c..6e9f3ce 100644
> --- a/drivers/video/exynos/exynos_dp_core.c
> +++ b/drivers/video/exynos/exynos_dp_core.c
> @@ -478,7 +478,7 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
>  	int lane_count;
>  	u8 buf[5];
>  
> -	u8 *adjust_request;
> +	u8 adjust_request[2];
>  	u8 voltage_swing;
>  	u8 pre_emphasis;
>  	u8 training_lane;
> @@ -493,8 +493,8 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
>  		/* set training pattern 2 for EQ */
>  		exynos_dp_set_training_pattern(dp, TRAINING_PTN2);
>  
> -		adjust_request = link_status + (DPCD_ADDR_ADJUST_REQUEST_LANE0_1
> -						- DPCD_ADDR_LANE0_1_STATUS);
> +		adjust_request[0] = link_status[4];
> +		adjust_request[1] = link_status[5];
>  
>  		exynos_dp_get_adjust_train(dp, adjust_request);
>  
> @@ -566,7 +566,7 @@ static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
>  	u8 buf[5];
>  	u32 reg;
>  
> -	u8 *adjust_request;
> +	u8 adjust_request[2];
>  
>  	udelay(400);
>  
> @@ -575,8 +575,8 @@ static int exynos_dp_process_equalizer_training(struct exynos_dp_device *dp)
>  	lane_count = dp->link_train.lane_count;
>  
>  	if (exynos_dp_clock_recovery_ok(link_status, lane_count) = 0) {
> -		adjust_request = link_status + (DPCD_ADDR_ADJUST_REQUEST_LANE0_1
> -						- DPCD_ADDR_LANE0_1_STATUS);
> +		adjust_request[0] = link_status[4];
> +		adjust_request[1] = link_status[5];
>  
>  		if (exynos_dp_channel_eq_ok(link_status, lane_count) = 0) {
>  			/* traing pattern Set to Normal */


^ permalink raw reply

* Re: [PATCH 1/2] video: msm: Fix section mismatches in mddi.c
From: Florian Tobias Schandinat @ 2012-04-23  6:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1334256334-8234-1-git-send-email-davidb@codeaurora.org>

On 04/12/2012 06:45 PM, David Brown wrote:
> The change
>     commit 461cbe77d0a4f887c33a3a95ea68a7daf23b4302
>     Author: Gregory Bean <gbean@codeaurora.org>
>     Date:   Wed Jul 28 10:22:13 2010 -0700
> 
>         video: msm: Fix section mismatch in mddi.c.
> 
> fixes a section mismatch between the board file and the driver's probe
> function, however, it misses the additional mismatches between the
> probe function and some routines it calls.  Fix these up as well.
> 
> Signed-off-by: David Brown <davidb@codeaurora.org>

Applied.


Thanks,

Florian Tobias Schandinat

> ---
>  drivers/video/msm/mddi.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/video/msm/mddi.c b/drivers/video/msm/mddi.c
> index 4527cbf..b061d70 100644
> --- a/drivers/video/msm/mddi.c
> +++ b/drivers/video/msm/mddi.c
> @@ -420,7 +420,7 @@ static void mddi_resume(struct msm_mddi_client_data *cdata)
>  	mddi_set_auto_hibernate(&mddi->client_data, 1);
>  }
>  
> -static int __init mddi_get_client_caps(struct mddi_info *mddi)
> +static int __devinit mddi_get_client_caps(struct mddi_info *mddi)
>  {
>  	int i, j;
>  
> @@ -622,9 +622,9 @@ uint32_t mddi_remote_read(struct msm_mddi_client_data *cdata, uint32_t reg)
>  
>  static struct mddi_info mddi_info[2];
>  
> -static int __init mddi_clk_setup(struct platform_device *pdev,
> -				 struct mddi_info *mddi,
> -				 unsigned long clk_rate)
> +static int __devinit mddi_clk_setup(struct platform_device *pdev,
> +				    struct mddi_info *mddi,
> +				    unsigned long clk_rate)
>  {
>  	int ret;
>  


^ permalink raw reply

* Re: [PATCH v2] video: exynos_dp: add analog and pll control setting
From: Florian Tobias Schandinat @ 2012-04-23  6:17 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <000e01cd1b68$81562e70$84028b50$%han@samsung.com>

On 04/16/2012 12:33 AM, Jingoo Han wrote:
> This patch adds analog and pll control setting. This control setting
> is used for DP TX PHY block to set the values as below. It is beneficial
> to improve analog characteristics.
>  - TX terminal registor is 50 Ohm.
>  - Reference clock of PHY is 24 MHz.
>  - Power source for TX digital logic is 1.0625 V.
>  - Power source for internal clock driver is 1.0625 V.
>  - PLL VCO range setting is 600 uA.
>  - Power down ring osc is turned off.
>  - AUX terminal resistor is 50 Ohm.
>  - AUX channel current is 8 mA and multiplied by 2.
>  - TX channel output amplitude is 400 mV.
> 
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>

Applied.


Thanks,

Florian Tobias Schandinat

> ---
>  drivers/video/exynos/exynos_dp_core.h |    1 +
>  drivers/video/exynos/exynos_dp_reg.c  |   23 +++++++++++++++++++++++
>  drivers/video/exynos/exynos_dp_reg.h  |   29 +++++++++++++++++++++++++++++
>  3 files changed, 53 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/video/exynos/exynos_dp_core.h b/drivers/video/exynos/exynos_dp_core.h
> index 90ceaca..519c3a6 100644
> --- a/drivers/video/exynos/exynos_dp_core.h
> +++ b/drivers/video/exynos/exynos_dp_core.h
> @@ -39,6 +39,7 @@ struct exynos_dp_device {
>  void exynos_dp_enable_video_mute(struct exynos_dp_device *dp, bool enable);
>  void exynos_dp_stop_video(struct exynos_dp_device *dp);
>  void exynos_dp_lane_swap(struct exynos_dp_device *dp, bool enable);
> +void exynos_dp_init_analog_param(struct exynos_dp_device *dp);
>  void exynos_dp_init_interrupt(struct exynos_dp_device *dp);
>  void exynos_dp_reset(struct exynos_dp_device *dp);
>  void exynos_dp_config_interrupt(struct exynos_dp_device *dp);
> diff --git a/drivers/video/exynos/exynos_dp_reg.c b/drivers/video/exynos/exynos_dp_reg.c
> index 6548afa..38177d0 100644
> --- a/drivers/video/exynos/exynos_dp_reg.c
> +++ b/drivers/video/exynos/exynos_dp_reg.c
> @@ -65,6 +65,28 @@ void exynos_dp_lane_swap(struct exynos_dp_device *dp, bool enable)
>  	writel(reg, dp->reg_base + EXYNOS_DP_LANE_MAP);
>  }
>  
> +void exynos_dp_init_analog_param(struct exynos_dp_device *dp)
> +{
> +	u32 reg;
> +
> +	reg = TX_TERMINAL_CTRL_50_OHM;
> +	writel(reg, dp->reg_base + EXYNOS_DP_ANALOG_CTL_1);
> +
> +	reg = SEL_24M | TX_DVDD_BIT_1_0625V;
> +	writel(reg, dp->reg_base + EXYNOS_DP_ANALOG_CTL_2);
> +
> +	reg = DRIVE_DVDD_BIT_1_0625V | VCO_BIT_600_MICRO;
> +	writel(reg, dp->reg_base + EXYNOS_DP_ANALOG_CTL_3);
> +
> +	reg = PD_RING_OSC | AUX_TERMINAL_CTRL_50_OHM |
> +		TX_CUR1_2X | TX_CUR_8_MA;
> +	writel(reg, dp->reg_base + EXYNOS_DP_PLL_FILTER_CTL_1);
> +
> +	reg = CH3_AMP_400_MV | CH2_AMP_400_MV |
> +		CH1_AMP_400_MV | CH0_AMP_400_MV;
> +	writel(reg, dp->reg_base + EXYNOS_DP_TX_AMP_TUNING_CTL);
> +}
> +
>  void exynos_dp_init_interrupt(struct exynos_dp_device *dp)
>  {
>  	/* Set interrupt pin assertion polarity as high */
> @@ -131,6 +153,7 @@ void exynos_dp_reset(struct exynos_dp_device *dp)
>  
>  	writel(0x00000101, dp->reg_base + EXYNOS_DP_SOC_GENERAL_CTL);
>  
> +	exynos_dp_init_analog_param(dp);
>  	exynos_dp_init_interrupt(dp);
>  }
>  
> diff --git a/drivers/video/exynos/exynos_dp_reg.h b/drivers/video/exynos/exynos_dp_reg.h
> index 42f608e..125b27c 100644
> --- a/drivers/video/exynos/exynos_dp_reg.h
> +++ b/drivers/video/exynos/exynos_dp_reg.h
> @@ -24,6 +24,12 @@
>  
>  #define EXYNOS_DP_LANE_MAP			0x35C
>  
> +#define EXYNOS_DP_ANALOG_CTL_1			0x370
> +#define EXYNOS_DP_ANALOG_CTL_2			0x374
> +#define EXYNOS_DP_ANALOG_CTL_3			0x378
> +#define EXYNOS_DP_PLL_FILTER_CTL_1		0x37C
> +#define EXYNOS_DP_TX_AMP_TUNING_CTL		0x380
> +
>  #define EXYNOS_DP_AUX_HW_RETRY_CTL		0x390
>  
>  #define EXYNOS_DP_COMMON_INT_STA_1		0x3C4
> @@ -166,6 +172,29 @@
>  #define LANE0_MAP_LOGIC_LANE_2			(0x2 << 0)
>  #define LANE0_MAP_LOGIC_LANE_3			(0x3 << 0)
>  
> +/* EXYNOS_DP_ANALOG_CTL_1 */
> +#define TX_TERMINAL_CTRL_50_OHM			(0x1 << 4)
> +
> +/* EXYNOS_DP_ANALOG_CTL_2 */
> +#define SEL_24M					(0x1 << 3)
> +#define TX_DVDD_BIT_1_0625V			(0x4 << 0)
> +
> +/* EXYNOS_DP_ANALOG_CTL_3 */
> +#define DRIVE_DVDD_BIT_1_0625V			(0x4 << 5)
> +#define VCO_BIT_600_MICRO			(0x5 << 0)
> +
> +/* EXYNOS_DP_PLL_FILTER_CTL_1 */
> +#define PD_RING_OSC				(0x1 << 6)
> +#define AUX_TERMINAL_CTRL_50_OHM		(0x2 << 4)
> +#define TX_CUR1_2X				(0x1 << 2)
> +#define TX_CUR_8_MA				(0x2 << 0)
> +
> +/* EXYNOS_DP_TX_AMP_TUNING_CTL */
> +#define CH3_AMP_400_MV				(0x0 << 24)
> +#define CH2_AMP_400_MV				(0x0 << 16)
> +#define CH1_AMP_400_MV				(0x0 << 8)
> +#define CH0_AMP_400_MV				(0x0 << 0)
> +
>  /* EXYNOS_DP_AUX_HW_RETRY_CTL */
>  #define AUX_BIT_PERIOD_EXPECTED_DELAY(x)	(((x) & 0x7) << 8)
>  #define AUX_HW_RETRY_INTERVAL_MASK		(0x3 << 3)


^ permalink raw reply

* Re: [PATCH V3 3/3] OMAPDSS: DISPC: Correct DISPC functional clock usage
From: Tomi Valkeinen @ 2012-04-23  6:19 UTC (permalink / raw)
  To: Mahapatra, Chandrabhanu; +Cc: linux-omap, linux-fbdev
In-Reply-To: <CAF0AtAu6xhQmQZLbW9keTMjcSfPwsVUoWa7e4fsSpHXms+pVSQ@mail.gmail.com>

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

On Mon, 2012-04-23 at 10:05 +0530, Mahapatra, Chandrabhanu wrote:
> On Fri, Apr 20, 2012 at 7:59 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

> > The patch is now otherwise fine, but I think it needs some more
> > renaming. Now the code mixes fclk and core-clk names, which is rather
> > confusing. I guess the calc_fclk should actually be calc_req_core_clk?
> > And the fclk variable core_clk (or cclk or something)?
> >
> >  Tomi
> >
> 
> As per TRM dispc_core_clk is also a functional clock, so I think above
> code should be fine. But, to avoid confusion of names the renaming of
> fclk, calc_fclk and calc_fclk_five_taps can be done.

Well, that is true is a sense, if you consider a "functional clock"
something that is used to drive some function of the HW. But then we
could also call the pixel clock a functional clock, as it also drives
some functionality. And true, TRM seems to refer to the core clock as
"dispc internal functional clock" at times.

But in this case, looking at the clock tree, DISPC_FCLK is the clock
coming to DISPC, which is then divided by the logic clock divisor to get
the logic clock (or core clock, or DISPC internal functional clock). And
I think the TRM, at least the OMAP2/3 versions, mostly speak of the
DISPC_FCLK as the functional clock, and that's what the driver also has
meant with "functional clock".

So to avoid any confusion, I'd suggest renaming them. At least I don't
see any benefit in having multiple clocks called "functional clock" for
dispc...

That said, I wonder if the DISPC_FCLK, LCD1_CLK and LCD2_CLK are used at
all before they are divided with the logic clock divider. If they are
not, then it could make sense to speak of the logic clock as the
functional clock, and call the incoming clocks something else.

But for the time being, I think we should continue calling the internal
fclk as core-clock.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH] drivers/video/ep93xx-fb.c: clean up error-handling code
From: Florian Tobias Schandinat @ 2012-04-23  6:19 UTC (permalink / raw)
  To: Julia Lawall; +Cc: kernel-janitors, linux-fbdev, linux-kernel
In-Reply-To: <1334777828-3557-1-git-send-email-Julia.Lawall@lip6.fr>

On 04/18/2012 07:37 PM, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> There were two problems in this code: failure of the setup function should
> free locally allocated resources like other nearby failures, and the test
> if (&info->cmap) can never be false.  To generally clean things up, this
> patch reorders the error handling code at the failed label and adds labels
> so that the conditionals are not necessary.
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

Applied.


Thanks,

Florian Tobias Schandinat

> 
> ---
> Not tested.
> 
>  drivers/video/ep93xx-fb.c |   32 +++++++++++++++++---------------
>  1 file changed, 17 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/video/ep93xx-fb.c b/drivers/video/ep93xx-fb.c
> index f8babbe..345d962 100644
> --- a/drivers/video/ep93xx-fb.c
> +++ b/drivers/video/ep93xx-fb.c
> @@ -507,16 +507,16 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
>  
>  	err = fb_alloc_cmap(&info->cmap, 256, 0);
>  	if (err)
> -		goto failed;
> +		goto failed_cmap;
>  
>  	err = ep93xxfb_alloc_videomem(info);
>  	if (err)
> -		goto failed;
> +		goto failed_videomem;
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!res) {
>  		err = -ENXIO;
> -		goto failed;
> +		goto failed_resource;
>  	}
>  
>  	/*
> @@ -532,7 +532,7 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
>  	fbi->mmio_base = ioremap(res->start, resource_size(res));
>  	if (!fbi->mmio_base) {
>  		err = -ENXIO;
> -		goto failed;
> +		goto failed_resource;
>  	}
>  
>  	strcpy(info->fix.id, pdev->name);
> @@ -553,24 +553,24 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
>  	if (err = 0) {
>  		dev_err(info->dev, "No suitable video mode found\n");
>  		err = -EINVAL;
> -		goto failed;
> +		goto failed_mode;
>  	}
>  
>  	if (mach_info->setup) {
>  		err = mach_info->setup(pdev);
>  		if (err)
> -			return err;
> +			goto failed_mode;
>  	}
>  
>  	err = ep93xxfb_check_var(&info->var, info);
>  	if (err)
> -		goto failed;
> +		goto failed_check;
>  
>  	fbi->clk = clk_get(info->dev, NULL);
>  	if (IS_ERR(fbi->clk)) {
>  		err = PTR_ERR(fbi->clk);
>  		fbi->clk = NULL;
> -		goto failed;
> +		goto failed_check;
>  	}
>  
>  	ep93xxfb_set_par(info);
> @@ -585,15 +585,17 @@ static int __devinit ep93xxfb_probe(struct platform_device *pdev)
>  	return 0;
>  
>  failed:
> -	if (fbi->clk)
> -		clk_put(fbi->clk);
> -	if (fbi->mmio_base)
> -		iounmap(fbi->mmio_base);
> -	ep93xxfb_dealloc_videomem(info);
> -	if (&info->cmap)
> -		fb_dealloc_cmap(&info->cmap);
> +	clk_put(fbi->clk);
> +failed_check:
>  	if (fbi->mach_info->teardown)
>  		fbi->mach_info->teardown(pdev);
> +failed_mode:
> +	iounmap(fbi->mmio_base);
> +failed_resource:
> +	ep93xxfb_dealloc_videomem(info);
> +failed_videomem:
> +	fb_dealloc_cmap(&info->cmap);
> +failed_cmap:
>  	kfree(info);
>  	platform_set_drvdata(pdev, NULL);
>  
> 
> 


^ permalink raw reply

* Re: [PATCH] drivers/video/mbx/mbxfb.c: correct ioremap_nocache test
From: Florian Tobias Schandinat @ 2012-04-23  6:19 UTC (permalink / raw)
  To: Julia Lawall; +Cc: kernel-janitors, linux-fbdev, linux-kernel
In-Reply-To: <1334779631-4361-1-git-send-email-Julia.Lawall@lip6.fr>

On 04/18/2012 08:07 PM, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
> 
> The test tested not the most recently allocated value but a previously
> allocated one.
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

Applied.


Thanks,

Florian Tobias Schandinat

> 
> ---
>  drivers/video/mbx/mbxfb.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/video/mbx/mbxfb.c b/drivers/video/mbx/mbxfb.c
> index 55bf619..ab0a8e5 100644
> --- a/drivers/video/mbx/mbxfb.c
> +++ b/drivers/video/mbx/mbxfb.c
> @@ -950,7 +950,7 @@ static int __devinit mbxfb_probe(struct platform_device *dev)
>  
>  	mfbi->fb_virt_addr = ioremap_nocache(mfbi->fb_phys_addr,
>  					     res_size(mfbi->fb_req));
> -	if (!mfbi->reg_virt_addr) {
> +	if (!mfbi->fb_virt_addr) {
>  		dev_err(&dev->dev, "failed to ioremap frame buffer\n");
>  		ret = -EINVAL;
>  		goto err4;
> 
> 


^ permalink raw reply

* Re: [PATCH V2] i.MX28: Shut down the LCD controller to avoid BootROM sampling bug
From: Florian Tobias Schandinat @ 2012-04-23  6:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1334860263-9389-1-git-send-email-marex@denx.de>

On 04/19/2012 06:31 PM, Marek Vasut wrote:
> From: Marek Vasut <marek.vasut@gmail.com>
> 
> If there's some traffic on the LCD controller pads, the BootROM has trouble with
> sampling the bootmode from these pads. The BootROM usually ends in a loop.
> 
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> Cc: Chen Peter-B29397 <B29397@freescale.com>
> Cc: Detlev Zundel <dzu@denx.de>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
> Cc: Li Frank-B20596 <B20596@freescale.com>
> Cc: Lin Tony-B19295 <B19295@freescale.com>
> Cc: Linux FBDEV <linux-fbdev@vger.kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Shawn Guo <shawn.guo@freescale.com>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Subodh Nijsure <snijsure@grid-net.com>
> Cc: Tony Lin <tony.lin@freescale.com>
> Cc: Wolfgang Denk <wd@denx.de>
> Acked-by: Shawn Guo <shawn.guo@linaro.org>
> Acked-by: Wolfgang Denk <wd@denx.de>
> Acked-by: Wolfram Sang <w.sang@pengutronix.de>
> Tested-by: Wolfgang Denk <wd@denx.de>

Applied.


Thanks,

Florian Tobias Schandinat

> ---
>  drivers/video/mxsfb.c |   13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> V2: Make shutdown() call static
> 
> diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
> index 4a89f88..dcf29bf 100644
> --- a/drivers/video/mxsfb.c
> +++ b/drivers/video/mxsfb.c
> @@ -880,6 +880,18 @@ static int __devexit mxsfb_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static void mxsfb_shutdown(struct platform_device *pdev)
> +{
> +	struct fb_info *fb_info = platform_get_drvdata(pdev);
> +	struct mxsfb_info *host = to_imxfb_host(fb_info);
> +
> +	/*
> +	 * Force stop the LCD controller as keeping it running during reboot
> +	 * might interfere with the BootROM's boot mode pads sampling.
> +	 */
> +	writel(CTRL_RUN, host->base + LCDC_CTRL + REG_CLR);
> +}
> +
>  static struct platform_device_id mxsfb_devtype[] = {
>  	{
>  		.name = "imx23-fb",
> @@ -896,6 +908,7 @@ MODULE_DEVICE_TABLE(platform, mxsfb_devtype);
>  static struct platform_driver mxsfb_driver = {
>  	.probe = mxsfb_probe,
>  	.remove = __devexit_p(mxsfb_remove),
> +	.shutdown = mxsfb_shutdown,
>  	.id_table = mxsfb_devtype,
>  	.driver = {
>  		   .name = DRIVER_NAME,


^ permalink raw reply

* [PATCH V3 3/3] OMAPDSS: DISPC: Correct DISPC functional clock usage
From: Chandrabhanu Mahapatra @ 2012-04-23  6:58 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <1333379598-11544-4-git-send-email-cmahapatra@ti.com>

DISPC_FCLK is incorrectly used as functional clock of DISPC in scaling
calculations. So, DISPC_CORE_CLK replaces as functional clock of DISPC.
DISPC_CORE_CLK is derived from DISPC_FCLK divided by an independent DISPC
divisor LCD.

Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
 drivers/video/omap2/dss/dispc.c |   67 +++++++++++++++++++++++---------------
 drivers/video/omap2/dss/dss.h   |    1 +
 2 files changed, 41 insertions(+), 27 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 17ffa71..f1be23f 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -1721,11 +1721,11 @@ static int check_horiz_timing_omap3(enum omap_channel channel, u16 pos_x,
 	return 0;
 }
 
-static unsigned long calc_fclk_five_taps(enum omap_channel channel, u16 width,
-		u16 height, u16 out_width, u16 out_height,
+static unsigned long calc_core_clk_five_taps(enum omap_channel channel,
+		u16 width, u16 height, u16 out_width, u16 out_height,
 		enum omap_color_mode color_mode)
 {
-	u32 fclk = 0;
+	u32 core_clk = 0;
 	u64 tmp, pclk = dispc_mgr_pclk_rate(channel);
 
 	if (height <= out_height && width <= out_width)
@@ -1737,7 +1737,7 @@ static unsigned long calc_fclk_five_taps(enum omap_channel channel, u16 width,
 
 		tmp = pclk * height * out_width;
 		do_div(tmp, 2 * out_height * ppl);
-		fclk = tmp;
+		core_clk = tmp;
 
 		if (height > 2 * out_height) {
 			if (ppl = out_width)
@@ -1745,23 +1745,23 @@ static unsigned long calc_fclk_five_taps(enum omap_channel channel, u16 width,
 
 			tmp = pclk * (height - 2 * out_height) * out_width;
 			do_div(tmp, 2 * out_height * (ppl - out_width));
-			fclk = max(fclk, (u32) tmp);
+			core_clk = max_t(u32, core_clk, tmp);
 		}
 	}
 
 	if (width > out_width) {
 		tmp = pclk * width;
 		do_div(tmp, out_width);
-		fclk = max(fclk, (u32) tmp);
+		core_clk = max_t(u32, core_clk, tmp);
 
 		if (color_mode = OMAP_DSS_COLOR_RGB24U)
-			fclk <<= 1;
+			core_clk <<= 1;
 	}
 
-	return fclk;
+	return core_clk;
 }
 
-static unsigned long calc_fclk(enum omap_channel channel, u16 width,
+static unsigned long calc_core_clk(enum omap_channel channel, u16 width,
 		u16 height, u16 out_width, u16 out_height)
 {
 	unsigned int hf, vf;
@@ -1812,7 +1812,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 	const int maxsinglelinewidth  				dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
 	const int max_decim_limit = 16;
-	unsigned long fclk = 0;
+	unsigned long core_clk = 0;
 	int decim_x, decim_y, error, min_factor;
 	u16 in_width, in_height, in_width_max = 0;
 
@@ -1852,10 +1852,10 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 		do {
 			in_height = DIV_ROUND_UP(height, decim_y);
 			in_width = DIV_ROUND_UP(width, decim_x);
-			fclk = calc_fclk(channel, in_width, in_height,
+			core_clk = calc_core_clk(channel, in_width, in_height,
 					out_width, out_height);
-			error = (in_width > maxsinglelinewidth || !fclk ||
-				fclk > dispc_fclk_rate());
+			error = (in_width > maxsinglelinewidth || !core_clk ||
+				core_clk > dispc_core_clk_rate());
 			if (error) {
 				if (decim_x = decim_y) {
 					decim_x = min_factor;
@@ -1878,8 +1878,8 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 		do {
 			in_height = DIV_ROUND_UP(height, decim_y);
 			in_width = DIV_ROUND_UP(width, decim_x);
-			fclk = calc_fclk_five_taps(channel, in_width, in_height,
-				out_width, out_height, color_mode);
+			core_clk = calc_core_clk_five_taps(channel, in_width,
+				in_height, out_width, out_height, color_mode);
 
 			error = check_horiz_timing_omap3(channel, pos_x,
 				in_width, in_height, out_width, out_height);
@@ -1889,11 +1889,11 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 					in_height < out_height * 2)
 					*five_taps = false;
 			if (!*five_taps)
-				fclk = calc_fclk(channel, in_width, in_height,
-					out_width, out_height);
+				core_clk = calc_core_clk(channel, in_width,
+					in_height, out_width, out_height);
 			error = (error || in_width > maxsinglelinewidth * 2 ||
 				(in_width > maxsinglelinewidth && *five_taps) ||
-				!fclk || fclk > dispc_fclk_rate());
+				!core_clk || core_clk > dispc_core_clk_rate());
 			if (error) {
 				if (decim_x = decim_y) {
 					decim_x = min_factor;
@@ -1926,7 +1926,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 	} else {
 		int decim_x_min = decim_x;
 		in_height = DIV_ROUND_UP(height, decim_y);
-		in_width_max = dispc_fclk_rate() /
+		in_width_max = dispc_core_clk_rate() /
 				DIV_ROUND_UP(dispc_mgr_pclk_rate(channel),
 						out_width);
 		decim_x = DIV_ROUND_UP(width, in_width_max);
@@ -1945,18 +1945,18 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
 			return -EINVAL;
 		}
 
-		fclk = calc_fclk(channel, in_width, in_height, out_width,
-				out_height);
+		core_clk = calc_core_clk(channel, in_width, in_height,
+				out_width, out_height);
 	}
 
-	DSSDBG("required fclk rate = %lu Hz\n", fclk);
-	DSSDBG("current fclk rate = %lu Hz\n", dispc_fclk_rate());
+	DSSDBG("required core clk rate = %lu Hz\n", core_clk);
+	DSSDBG("current core clk rate = %lu Hz\n", dispc_core_clk_rate());
 
-	if (!fclk || fclk > dispc_fclk_rate()) {
+	if (!core_clk || core_clk > dispc_core_clk_rate()) {
 		DSSERR("failed to set up scaling, "
-			"required fclk rate = %lu Hz, "
-			"current fclk rate = %lu Hz\n",
-			fclk, dispc_fclk_rate());
+			"required core clk rate = %lu Hz, "
+			"current core clk rate = %lu Hz\n",
+			core_clk, dispc_core_clk_rate());
 		return -EINVAL;
 	}
 
@@ -2646,6 +2646,19 @@ unsigned long dispc_mgr_pclk_rate(enum omap_channel channel)
 	}
 }
 
+unsigned long dispc_core_clk_rate(void)
+{
+	int lcd;
+	unsigned long fclk = dispc_fclk_rate();
+
+	if (dss_has_feature(FEAT_CORE_CLK_DIV))
+		lcd = REG_GET(DISPC_DIVISOR, 23, 16);
+	else
+		lcd = REG_GET(DISPC_DIVISORo(OMAP_DSS_CHANNEL_LCD), 23, 16);
+
+	return fclk / lcd;
+}
+
 void dispc_dump_clocks(struct seq_file *s)
 {
 	int lcd, pcd;
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index d4b3dff..1bde34c 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -451,6 +451,7 @@ void dispc_mgr_set_pol_freq(enum omap_channel channel,
 		enum omap_panel_config config, u8 acbi, u8 acb);
 unsigned long dispc_mgr_lclk_rate(enum omap_channel channel);
 unsigned long dispc_mgr_pclk_rate(enum omap_channel channel);
+unsigned long dispc_core_clk_rate(void);
 int dispc_mgr_set_clock_div(enum omap_channel channel,
 		struct dispc_clock_info *cinfo);
 int dispc_mgr_get_clock_div(enum omap_channel channel,
-- 
1.7.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox