* Re: [RESEND][PATCH 09/10] cobalt_lcdfb: LCD panel framebuffer support for SEAD-3 platform.
From: Florian Tobias Schandinat @ 2012-04-18 16:48 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1334758309-3986-1-git-send-email-sjhill@mips.com>
[Cc'ing Yoichi Yuasa and linux-fbdev]
On 04/18/2012 02:13 PM, Steven J. Hill wrote:
> From: "Steven J. Hill" <sjhill@mips.com>
>
> Add support for LCD panel on MIPS SEAD-3 development platform.
>
> Signed-off-by: Douglas Leung <douglas@mips.com>
> Signed-off-by: Chris Dearman <chris@mips.com>
> Signed-off-by: Steven J. Hill <sjhill@mips.com>
Looks good to me. I will apply it in a few days if nobody is against it.
Best regards,
Florian Tobias Schandinat
> ---
> drivers/video/Kconfig | 2 +-
> drivers/video/cobalt_lcdfb.c | 45 +++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index a8a897a..e921a45 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -2210,7 +2210,7 @@ config FB_XILINX
>
> config FB_COBALT
> tristate "Cobalt server LCD frame buffer support"
> - depends on FB && MIPS_COBALT
> + depends on FB && (MIPS_COBALT || MIPS_SEAD3)
>
> config FB_SH7760
> bool "SH7760/SH7763/SH7720/SH7721 LCDC support"
> diff --git a/drivers/video/cobalt_lcdfb.c b/drivers/video/cobalt_lcdfb.c
> index f56699d..eae46f6 100644
> --- a/drivers/video/cobalt_lcdfb.c
> +++ b/drivers/video/cobalt_lcdfb.c
> @@ -1,7 +1,8 @@
> /*
> - * Cobalt server LCD frame buffer driver.
> + * Cobalt/SEAD3 LCD frame buffer driver.
> *
> * Copyright (C) 2008 Yoichi Yuasa <yuasa@linux-mips.org>
> + * Copyright (C) 2012 MIPS Technologies, Inc.
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License as published by
> @@ -62,6 +63,7 @@
> #define LCD_CUR_POS(x) ((x) & LCD_CUR_POS_MASK)
> #define LCD_TEXT_POS(x) ((x) | LCD_TEXT_MODE)
>
> +#ifdef CONFIG_MIPS_COBALT
> static inline void lcd_write_control(struct fb_info *info, u8 control)
> {
> writel((u32)control << 24, info->screen_base);
> @@ -81,6 +83,47 @@ static inline u8 lcd_read_data(struct fb_info *info)
> {
> return readl(info->screen_base + LCD_DATA_REG_OFFSET) >> 24;
> }
> +#else
> +
> +#define LCD_CTL 0x00
> +#define LCD_DATA 0x08
> +#define CPLD_STATUS 0x10
> +#define CPLD_DATA 0x18
> +
> +static inline void cpld_wait(struct fb_info *info)
> +{
> + do {
> + } while (readl(info->screen_base + CPLD_STATUS) & 1);
> +}
> +
> +static inline void lcd_write_control(struct fb_info *info, u8 control)
> +{
> + cpld_wait(info);
> + writel(control, info->screen_base + LCD_CTL);
> +}
> +
> +static inline u8 lcd_read_control(struct fb_info *info)
> +{
> + cpld_wait(info);
> + readl(info->screen_base + LCD_CTL);
> + cpld_wait(info);
> + return readl(info->screen_base + CPLD_DATA) & 0xff;
> +}
> +
> +static inline void lcd_write_data(struct fb_info *info, u8 data)
> +{
> + cpld_wait(info);
> + writel(data, info->screen_base + LCD_DATA);
> +}
> +
> +static inline u8 lcd_read_data(struct fb_info *info)
> +{
> + cpld_wait(info);
> + readl(info->screen_base + LCD_DATA);
> + cpld_wait(info);
> + return readl(info->screen_base + CPLD_DATA) & 0xff;
> +}
> +#endif
>
> static int lcd_busy_wait(struct fb_info *info)
> {
^ permalink raw reply
* [PATCH v1] ARM: i.mx: mx3fb: add overlay support
From: Alex Gershgorin @ 2012-04-18 17:38 UTC (permalink / raw)
To: Florian Tobias Schandinat
Cc: g.liakhovetski, s.hauer, laurent.pinchart, linux-fbdev,
linux-media, Alex Gershgorin
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>
---
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 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);
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);
+
+ 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:
+ 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);
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>
+
+#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)
+
+#ifdef __KERNEL__
+
+enum {
+ MXCFB_REFRESH_OFF,
+ MXCFB_REFRESH_AUTO,
+ MXCFB_REFRESH_PARTIAL,
+};
+
+#endif
+
+#endif /* _MXCFB_H */
+
--
1.7.0.4
^ permalink raw reply related
* [PATCH] drivers/video/ep93xx-fb.c: clean up error-handling code
From: Julia Lawall @ 2012-04-18 19:37 UTC (permalink / raw)
To: Florian Tobias Schandinat; +Cc: kernel-janitors, linux-fbdev, linux-kernel
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>
---
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 related
* [PATCH] drivers/video/mbx/mbxfb.c: correct ioremap_nocache test
From: Julia Lawall @ 2012-04-18 20:07 UTC (permalink / raw)
To: Florian Tobias Schandinat; +Cc: kernel-janitors, linux-fbdev, linux-kernel
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>
---
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 related
* RE: [PATCH] drivers/video/ep93xx-fb.c: clean up error-handling code
From: H Hartley Sweeten @ 2012-04-18 20:35 UTC (permalink / raw)
To: Julia Lawall, Florian Tobias Schandinat
Cc: kernel-janitors@vger.kernel.org, linux-fbdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1334777828-3557-1-git-send-email-Julia.Lawall@lip6.fr>
On Wednesday, April 18, 2012 12: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>
>
Thanks!
Acked-by: H Hartley Sweeten <hsweeten@visionengravers.com>
^ permalink raw reply
* Re: [PATCH v1] ARM: i.mx: mx3fb: add overlay support
From: Guennadi Liakhovetski @ 2012-04-18 22:40 UTC (permalink / raw)
To: Alex Gershgorin
Cc: Florian Tobias Schandinat, s.hauer, laurent.pinchart, linux-fbdev,
linux-media
In-Reply-To: <1334770715-31064-1-git-send-email-alexg@meprolight.com>
Hi Alex
Thanks for reviving, fixing and submitting this code!
On Wed, 18 Apr 2012, Alex Gershgorin wrote:
> This patch is based on the original version submitted by Guennadi Liakhovetski,
> the patch initializes overlay channel, adds ioctl for configuring
> transparency of the overlay and graphics planes, CONFIG_FB_MX3_OVERLAY
> is also supported.
>
> In case that CONFIG_FB_MX3_OVERLAY is not defined, mx3fb is completely
> backward compatible.
>
> Blend mode, only global alpha blending has been tested.
>
> Signed-off-by: Alex Gershgorin <alexg@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, otherwise just mentioning, that this work is based on some earlier
patch series "i.MX31: dmaengine and framebuffer drivers" from 2008 by ...
should be enough.
I don't think I can review this patch in sufficient depth, just a couple
of minor comments below
> ---
>
> Applies to v3.4-rc3
> ---
> drivers/video/Kconfig | 7 +
> drivers/video/mx3fb.c | 318 ++++++++++++++++++++++++++++++++++++++++++++-----
> include/linux/mxcfb.h | 93 ++++++++++++++
> 3 files changed, 388 insertions(+), 30 deletions(-)
> create mode 100644 include/linux/mxcfb.h
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index a290be5..acbfccc 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -2368,6 +2368,13 @@ config FB_MX3
> far only synchronous displays are supported. If you plan to use
> an LCD display with your i.MX31 system, say Y here.
>
> +config FB_MX3_OVERLAY
> + bool "MX3 Overlay support"
> + default n
> + depends on FB_MX3
> + ---help---
> + Say Y here to enable overlay support
> +
> config FB_BROADSHEET
> tristate "E-Ink Broadsheet/Epson S1D13521 controller support"
> depends on FB
> diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
> index eec0d7b..0fb8a72 100644
> --- a/drivers/video/mx3fb.c
> +++ b/drivers/video/mx3fb.c
> @@ -26,6 +26,7 @@
> #include <linux/console.h>
> #include <linux/clk.h>
> #include <linux/mutex.h>
> +#include <linux/mxcfb.h>
>
> #include <mach/dma.h>
> #include <mach/hardware.h>
> @@ -238,6 +239,7 @@ static const struct fb_videomode mx3fb_modedb[] = {
>
> struct mx3fb_data {
> struct fb_info *fbi;
> + struct fb_info *fbi_ovl;
> int backlight_level;
> void __iomem *reg_base;
> spinlock_t lock;
> @@ -246,6 +248,9 @@ struct mx3fb_data {
> uint32_t h_start_width;
> uint32_t v_start_width;
> enum disp_data_mapping disp_data_fmt;
> +
> + /* IDMAC / dmaengine interface */
> + struct idmac_channel *idmac_channel[2]; /* We need 2 channels */
> };
>
> struct dma_chan_request {
> @@ -272,6 +277,17 @@ struct mx3fb_info {
> u32 sync; /* preserve var->sync flags */
> };
>
> +/* Allocated overlay buffer */
> +struct mx3fb_alloc_list {
> + struct list_head list;
> + dma_addr_t phy_addr;
> + void *cpu_addr;
> + size_t size;
> +};
> +
> +/* A list of overlay buffers */
> +static LIST_HEAD(fb_alloc_list);
Static variables are evil:-) Which you prove below by protecting this
global list-head by a per-device mutex. No, I have no idea whether anyone
ever comes up with an SoC with multiple instances of this device, but at
least this seems inconsistent to me.
> +
> static void mx3fb_dma_done(void *);
>
> /* Used fb-mode and bpp. Can be set on kernel command line, therefore file-static. */
> @@ -303,7 +319,11 @@ static void sdc_fb_init(struct mx3fb_info *fbi)
> struct mx3fb_data *mx3fb = fbi->mx3fb;
> uint32_t reg;
>
> - reg = mx3fb_read_reg(mx3fb, SDC_COM_CONF);
> + reg = mx3fb_read_reg(mx3fb, SDC_COM_CONF) & ~SDC_COM_GWSEL;
> +
> + /* Also enable foreground for overlay graphic window is foreground */
> + if (mx3fb->fbi_ovl && fbi = mx3fb->fbi_ovl->par)
> + reg |= (SDC_COM_FG_EN | SDC_COM_GWSEL);
Superfluous parenthesis.
>
> mx3fb_write_reg(mx3fb, reg | SDC_COM_BG_EN, SDC_COM_CONF);
> }
> @@ -312,13 +332,24 @@ static void sdc_fb_init(struct mx3fb_info *fbi)
> static uint32_t sdc_fb_uninit(struct mx3fb_info *fbi)
> {
> struct mx3fb_data *mx3fb = fbi->mx3fb;
> - uint32_t reg;
> + uint32_t reg, chan_mask;
>
> reg = mx3fb_read_reg(mx3fb, SDC_COM_CONF);
>
> - mx3fb_write_reg(mx3fb, reg & ~SDC_COM_BG_EN, SDC_COM_CONF);
> + /*
> + * Don't we have to automatically disable overlay when disabling
> + * background? Attention: cannot test mx3fb->fbi_ovl->par, must
> + * test mx3fb->fbi->par, because at the time this function is
> + * called for the first time fbi_ovl is not assigned yet.
> + */
> + if (fbi = mx3fb->fbi->par)
> + chan_mask = SDC_COM_BG_EN;
> + else
> + chan_mask = SDC_COM_FG_EN | SDC_COM_GWSEL;
> +
> + mx3fb_write_reg(mx3fb, reg & ~chan_mask, SDC_COM_CONF);
>
> - return reg & SDC_COM_BG_EN;
> + return reg & chan_mask;
> }
>
> static void sdc_enable_channel(struct mx3fb_info *mx3_fbi)
> @@ -412,13 +443,20 @@ static void sdc_disable_channel(struct mx3fb_info *mx3_fbi)
> static int sdc_set_window_pos(struct mx3fb_data *mx3fb, enum ipu_channel channel,
> int16_t x_pos, int16_t y_pos)
> {
> - if (channel != IDMAC_SDC_0)
> - return -EINVAL;
> -
> x_pos += mx3fb->h_start_width;
> y_pos += mx3fb->v_start_width;
>
> - mx3fb_write_reg(mx3fb, (x_pos << 16) | y_pos, SDC_BG_POS);
> + switch (channel) {
> + case IDMAC_SDC_0:
> + mx3fb_write_reg(mx3fb, (x_pos << 16) | y_pos, SDC_BG_POS);
> + break;
> + case IDMAC_SDC_1:
> + mx3fb_write_reg(mx3fb, (x_pos << 16) | y_pos, SDC_FG_POS);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> return 0;
> }
>
> @@ -482,14 +520,17 @@ static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel,
> mx3fb->h_start_width = h_start_width;
> mx3fb->v_start_width = v_start_width;
>
> + reg = mx3fb_read_reg(mx3fb, SDC_COM_CONF);
> +
> switch (panel) {
> case IPU_PANEL_SHARP_TFT:
> mx3fb_write_reg(mx3fb, 0x00FD0102L, SDC_SHARP_CONF_1);
> mx3fb_write_reg(mx3fb, 0x00F500F4L, SDC_SHARP_CONF_2);
> - mx3fb_write_reg(mx3fb, SDC_COM_SHARP | SDC_COM_TFT_COLOR, SDC_COM_CONF);
> + mx3fb_write_reg(mx3fb, reg | SDC_COM_SHARP |
> + SDC_COM_TFT_COLOR, SDC_COM_CONF);
> break;
> case IPU_PANEL_TFT:
> - mx3fb_write_reg(mx3fb, SDC_COM_TFT_COLOR, SDC_COM_CONF);
> + mx3fb_write_reg(mx3fb, reg | SDC_COM_TFT_COLOR, SDC_COM_CONF);
> break;
> default:
> return -EINVAL;
> @@ -563,13 +604,12 @@ static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel,
> /**
> * sdc_set_color_key() - set the transparent color key for SDC graphic plane.
> * @mx3fb: mx3fb context.
> - * @channel: IPU DMAC channel ID.
> * @enable: boolean to enable or disable color keyl.
> * @color_key: 24-bit RGB color to use as transparent color key.
> * @return: 0 on success or negative error code on failure.
> */
> -static int sdc_set_color_key(struct mx3fb_data *mx3fb, enum ipu_channel channel,
> - bool enable, uint32_t color_key)
> +static int sdc_set_color_key(struct mx3fb_data *mx3fb, bool enable,
> + uint32_t color_key)
> {
> uint32_t reg, sdc_conf;
> unsigned long lock_flags;
> @@ -577,10 +617,6 @@ static int sdc_set_color_key(struct mx3fb_data *mx3fb, enum ipu_channel channel,
> spin_lock_irqsave(&mx3fb->lock, lock_flags);
>
> sdc_conf = mx3fb_read_reg(mx3fb, SDC_COM_CONF);
> - if (channel = IDMAC_SDC_0)
> - sdc_conf &= ~SDC_COM_GWSEL;
> - else
> - sdc_conf |= SDC_COM_GWSEL;
>
> if (enable) {
> reg = mx3fb_read_reg(mx3fb, SDC_GW_CTRL) & 0xFF000000L;
> @@ -668,8 +704,12 @@ static int mx3fb_set_fix(struct fb_info *fbi)
> {
> struct fb_fix_screeninfo *fix = &fbi->fix;
> struct fb_var_screeninfo *var = &fbi->var;
> + struct mx3fb_info *mx3_fbi = fbi->par;
>
> - strncpy(fix->id, "DISP3 BG", 8);
> + if (mx3_fbi->ipu_ch = IDMAC_SDC_1)
> + strncpy(fix->id, "DISP3 FG", 8);
> + else
> + strncpy(fix->id, "DISP3 BG", 8);
>
> fix->line_length = var->xres_virtual * var->bits_per_pixel / 8;
>
> @@ -689,13 +729,25 @@ static void mx3fb_dma_done(void *arg)
> struct idmac_channel *ichannel = to_idmac_chan(chan);
> struct mx3fb_data *mx3fb = ichannel->client;
> struct mx3fb_info *mx3_fbi = mx3fb->fbi->par;
> + struct mx3fb_info *mx3_fbi_cur;
> + struct mx3fb_info *mx3_fbi_ovl = mx3fb->fbi_ovl ? mx3fb->fbi_ovl->par :
> + NULL;
>
> dev_dbg(mx3fb->dev, "irq %d callback\n", ichannel->eof_irq);
>
> + if (ichannel = mx3_fbi->idmac_channel) {
> + mx3_fbi_cur = mx3_fbi;
> + } else if (mx3_fbi_ovl && ichannel = mx3_fbi_ovl->idmac_channel) {
> + mx3_fbi_cur = mx3_fbi_ovl;
> + } else {
> + WARN(1, "Cannot identify channel!\n");
> + return;
> + }
> +
> /* We only need one interrupt, it will be re-enabled as needed */
> disable_irq_nosync(ichannel->eof_irq);
>
> - complete(&mx3_fbi->flip_cmpl);
> + complete(&mx3_fbi_cur->flip_cmpl);
> }
>
> static int __set_par(struct fb_info *fbi, bool lock)
> @@ -1151,6 +1203,145 @@ static struct fb_ops mx3fb_ops = {
> .fb_blank = mx3fb_blank,
> };
>
> +#ifdef CONFIG_FB_MX3_OVERLAY
> +static int mx3fb_blank_ovl(int blank, struct fb_info *fbi)
> +{
> + struct mx3fb_info *mx3_fbi = fbi->par;
> +
> + dev_dbg(fbi->device, "ovl blank = %d\n", blank);
> +
> + if (mx3_fbi->blank = blank)
> + return 0;
> +
> + mutex_lock(&mx3_fbi->mutex);
> + mx3_fbi->blank = blank;
> +
> + switch (blank) {
> + case FB_BLANK_POWERDOWN:
> + case FB_BLANK_VSYNC_SUSPEND:
> + case FB_BLANK_HSYNC_SUSPEND:
> + case FB_BLANK_NORMAL:
> + sdc_disable_channel(mx3_fbi);
> + break;
> + case FB_BLANK_UNBLANK:
> + sdc_enable_channel(mx3_fbi);
> + break;
> + }
> + mutex_unlock(&mx3_fbi->mutex);
> +
> + return 0;
> +}
> +
> +/*
> + * Function to handle custom ioctls for MX3 framebuffer.
> + *
> + * @inode inode struct
> + * @file file struct
> + * @cmd Ioctl command to handle
> + * @arg User pointer to command arguments
> + * @fbi framebuffer information pointer
> + */
> +static int mx3fb_ioctl_ovl(struct fb_info *fbi, unsigned int cmd,
> + unsigned long arg)
> +{
> + struct mx3fb_info *mx3_fbi = fbi->par;
> + struct mx3fb_data *mx3fb = mx3_fbi->mx3fb;
> + struct mxcfb_gbl_alpha ga;
> + struct mxcfb_color_key key;
> + int retval = 0;
> + int __user *argp = (void __user *)arg;
> + struct mx3fb_alloc_list *mem;
> + int size;
> + unsigned long offset;
> +
> + switch (cmd) {
> + case FBIO_ALLOC:
> + if (get_user(size, argp))
> + return -EFAULT;
> +
> + mem = kzalloc(sizeof(*mem), GFP_KERNEL);
> + if (mem = NULL)
> + return -ENOMEM;
> +
> + mem->size = PAGE_ALIGN(size);
> +
> + mem->cpu_addr = dma_alloc_coherent(fbi->device, size,
> + &mem->phy_addr,
> + GFP_DMA);
> + if (mem->cpu_addr = NULL) {
> + kfree(mem);
> + return -ENOMEM;
> + }
> +
> + mutex_lock(&mx3_fbi->mutex);
> + list_add(&mem->list, &fb_alloc_list);
> + mutex_unlock(&mx3_fbi->mutex);
Here. At the very least you'd need a global mutex. Or put the list-head in
struct mx3fb_info.
> +
> + dev_dbg(fbi->device, "allocated %d bytes <at> 0x%08X\n",
> + mem->size, mem->phy_addr);
> +
> + if (put_user(mem->phy_addr, argp))
> + return -EFAULT;
> +
> + break;
> + case FBIO_FREE:
> + if (get_user(offset, argp))
> + return -EFAULT;
> +
> + retval = -EINVAL;
> + mutex_lock(&mx3_fbi->mutex);
> + list_for_each_entry(mem, &fb_alloc_list, list) {
> + if (mem->phy_addr = offset) {
> + list_del(&mem->list);
> + dma_free_coherent(fbi->device,
> + mem->size,
> + mem->cpu_addr,
> + mem->phy_addr);
> + kfree(mem);
> + retval = 0;
> + break;
> + }
> + }
> + mutex_unlock(&mx3_fbi->mutex);
> +
> + break;
> + case MXCFB_SET_GBL_ALPHA:
Are you using these proprietary ioctl()s? If not, I wouldn't implement
them. If you do need them, maybe it would make sense to add such ioctl()s
globally for fbdev?
> + if (copy_from_user(&ga, (void *)arg, sizeof(ga)))
> + retval = -EFAULT;
> +
> + sdc_set_global_alpha(mx3fb, (bool)ga.enable, ga.alpha);
> + dev_dbg(fbi->device, "Set global alpha to %d\n", ga.alpha);
> +
> + break;
> + case MXCFB_SET_CLR_KEY:
> + if (copy_from_user(&key, (void *)arg, sizeof(key)))
> + retval = -EFAULT;
> +
> + sdc_set_color_key(mx3fb, (bool)key.enable, key.color_key);
> + dev_dbg(fbi->device, "Set color key to %d\n", key.color_key);
> +
> + break;
> + default:
> + retval = -EINVAL;
> + }
> +
> + return retval;
> +}
> +
> +static struct fb_ops mx3fb_ovl_ops = {
> + .owner = THIS_MODULE,
> + .fb_set_par = mx3fb_set_par,
> + .fb_check_var = mx3fb_check_var,
> + .fb_setcolreg = mx3fb_setcolreg,
> + .fb_pan_display = mx3fb_pan_display,
> + .fb_ioctl = mx3fb_ioctl_ovl,
> + .fb_fillrect = cfb_fillrect,
> + .fb_copyarea = cfb_copyarea,
> + .fb_imageblit = cfb_imageblit,
> + .fb_blank = mx3fb_blank_ovl,
> +};
> +#endif
> +
> #ifdef CONFIG_PM
> /*
> * Power management hooks. Note that we won't be called from IRQ context,
> @@ -1164,11 +1355,16 @@ static int mx3fb_suspend(struct platform_device *pdev, pm_message_t state)
> {
> struct mx3fb_data *mx3fb = platform_get_drvdata(pdev);
> struct mx3fb_info *mx3_fbi = mx3fb->fbi->par;
> + struct mx3fb_info *mx3_fbi_ovl = mx3fb->fbi_ovl->par;
>
> console_lock();
> fb_set_suspend(mx3fb->fbi, 1);
> + fb_set_suspend(mx3fb->fbi_ovl, 1);
> console_unlock();
>
> + if (mx3_fbi_ovl->blank = FB_BLANK_UNBLANK)
> + sdc_disable_channel(mx3_fbi_ovl);
> +
> if (mx3_fbi->blank = FB_BLANK_UNBLANK) {
> sdc_disable_channel(mx3_fbi);
> sdc_set_brightness(mx3fb, 0);
> @@ -1184,14 +1380,19 @@ static int mx3fb_resume(struct platform_device *pdev)
> {
> struct mx3fb_data *mx3fb = platform_get_drvdata(pdev);
> struct mx3fb_info *mx3_fbi = mx3fb->fbi->par;
> + struct mx3fb_info *mx3_fbi_ovl = mx3fb->fbi_ovl->par;
>
> if (mx3_fbi->blank = FB_BLANK_UNBLANK) {
> sdc_enable_channel(mx3_fbi);
> sdc_set_brightness(mx3fb, mx3fb->backlight_level);
> }
>
> + if (mx3_fbi_ovl->blank = FB_BLANK_UNBLANK)
> + sdc_enable_channel(mx3_fbi_ovl);
> +
> console_lock();
> fb_set_suspend(mx3fb->fbi, 0);
> + fb_set_suspend(mx3fb->fbi_ovl, 0);
> console_unlock();
>
> return 0;
> @@ -1333,8 +1534,8 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
> ichan->client = mx3fb;
> irq = ichan->eof_irq;
>
> - if (ichan->dma_chan.chan_id != IDMAC_SDC_0)
> - return -EINVAL;
> + switch (ichan->dma_chan.chan_id) {
> + case IDMAC_SDC_0:
>
> fbi = mx3fb_init_fbinfo(dev, &mx3fb_ops);
I would bite the bullet and indent this case block...
> if (!fbi)
> @@ -1375,7 +1576,29 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
>
> sdc_set_brightness(mx3fb, 255);
> sdc_set_global_alpha(mx3fb, true, 0xFF);
> - sdc_set_color_key(mx3fb, IDMAC_SDC_0, false, 0);
> + sdc_set_color_key(mx3fb, false, 0);
> +
> + break;
> +#ifdef CONFIG_FB_MX3_OVERLAY
> + case IDMAC_SDC_1:
> +
> + /* We know, that background has been allocated already! */
> + fbi = mx3fb_init_fbinfo(dev, &mx3fb_ovl_ops);
> + if (!fbi)
> + return -ENOMEM;
> +
> + /* Default Y virtual size is 2x panel size */
> + fbi->var = mx3fb->fbi->var;
> + /* This shouldn't be necessary, it is already set up above */
> + fbi->var.yres_virtual = mx3fb->fbi->var.yres * 2;
> +
> + mx3fb->fbi_ovl = fbi;
> +
> + break;
> +#endif
> + default:
> + return -EINVAL;
> + }
>
> mx3fbi = fbi->par;
> mx3fbi->idmac_channel = ichan;
> @@ -1392,9 +1615,13 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
> if (ret < 0)
> goto esetpar;
>
> - __blank(FB_BLANK_UNBLANK, fbi);
> + /* Overlay stays blanked by default */
> + if (ichan->dma_chan.chan_id = IDMAC_SDC_0) {
> + mx3fb_blank(FB_BLANK_UNBLANK, fbi);
>
> - dev_info(dev, "registered, using mode %s\n", fb_mode);
> + dev_info(dev, "mx3fb: fb registered, using mode %s [%c]\n",
> + fb_mode, list_empty(&ichan->queue) ? '-' : '+');
> + }
>
> ret = register_framebuffer(fbi);
> if (ret < 0)
> @@ -1492,14 +1719,42 @@ static int mx3fb_probe(struct platform_device *pdev)
>
> mx3fb->backlight_level = 255;
>
> + mx3fb->idmac_channel[0] = to_idmac_chan(chan);
> + mx3fb->idmac_channel[0]->client = mx3fb;
> +
> ret = init_fb_chan(mx3fb, to_idmac_chan(chan));
> if (ret < 0)
> goto eisdc0;
>
> +#ifdef CONFIG_FB_MX3_OVERLAY
> + dma_cap_zero(mask);
> + dma_cap_set(DMA_SLAVE, mask);
> + dma_cap_set(DMA_PRIVATE, mask);
> + rq.id = IDMAC_SDC_1;
> + chan = dma_request_channel(mask, chan_filter, &rq);
> + if (!chan) {
> + ret = -EBUSY;
> + goto ersdc1;
> + }
> +
> + mx3fb->idmac_channel[1] = to_idmac_chan(chan);
> + mx3fb->idmac_channel[1]->client = mx3fb;
> +
> + ret = init_fb_chan(mx3fb, to_idmac_chan(chan));
> + if (ret < 0)
> + goto eisdc1;
> +#endif
> +
> return 0;
>
> +#ifdef CONFIG_FB_MX3_OVERLAY
> +eisdc1:
> + dma_release_channel(&mx3fb->idmac_channel[1]->dma_chan);
> +ersdc1:
> + release_fbi(mx3fb->fbi);
> +#endif
> eisdc0:
> - dma_release_channel(chan);
> + dma_release_channel(&mx3fb->idmac_channel[0]->dma_chan);
> ersdc0:
> dmaengine_put();
> iounmap(mx3fb->reg_base);
> @@ -1513,13 +1768,16 @@ static int mx3fb_remove(struct platform_device *dev)
> {
> struct mx3fb_data *mx3fb = platform_get_drvdata(dev);
> struct fb_info *fbi = mx3fb->fbi;
> - struct mx3fb_info *mx3_fbi = fbi->par;
> - struct dma_chan *chan;
>
> - chan = &mx3_fbi->idmac_channel->dma_chan;
> - release_fbi(fbi);
> + if (fbi)
> + release_fbi(fbi);
> +
> + fbi = mx3fb->fbi_ovl;
> + if (fbi)
> + release_fbi(fbi);
>
> - dma_release_channel(chan);
> + dma_release_channel(&mx3fb->idmac_channel[1]->dma_chan);
> + dma_release_channel(&mx3fb->idmac_channel[0]->dma_chan);
> dmaengine_put();
>
> iounmap(mx3fb->reg_base);
> diff --git a/include/linux/mxcfb.h b/include/linux/mxcfb.h
> new file mode 100644
> index 0000000..54b720d
> --- /dev/null
> +++ b/include/linux/mxcfb.h
> @@ -0,0 +1,93 @@
> +/*
> + * File: include/linux/mxcfb.h
> + * Global header file for the MXC Framebuffer
> + *
> + * Copyright 2004-2012 Freescale Semiconductor, Inc. All Rights Reserved.
> + *
> + * The code contained herein is licensed under the GNU Lesser General
> + * Public License. You may obtain a copy of the GNU Lesser General
> + * Public License Version 2.1 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/lgpl-license.html
> + * http://www.gnu.org/copyleft/lgpl.html
> + */
> +
> +#ifndef __LINUX_MXCFB_H__
> +#define __LINUX_MXCFB_H__
> +
> +#include <linux/fb.h>
Why is this needed here?
> +
> +#define FB_SYNC_OE_LOW_ACT 0x80000000
> +#define FB_SYNC_CLK_LAT_FALL 0x40000000
> +#define FB_SYNC_DATA_INVERT 0x20000000
> +#define FB_SYNC_CLK_IDLE_EN 0x10000000
> +#define FB_SYNC_SHARP_MODE 0x08000000
> +#define FB_SYNC_SWAP_RGB 0x04000000
> +
> +struct mxcfb_gbl_alpha {
> + int enable;
> + int alpha;
> +};
> +
> +struct mxcfb_color_key {
> + int enable;
> + __u32 color_key;
> +};
> +
> +struct mxcfb_pos {
> + __u16 x;
> + __u16 y;
> +};
> +
> +struct mxcfb_gamma {
> + int enable;
> + int constk[16];
> + int slopek[16];
> +};
> +
> +struct mxcfb_rect {
> + __u32 top;
> + __u32 left;
> + __u32 width;
> + __u32 height;
> +};
> +
> +/*
> + * Structure used to define waveform modes for driver
> + * Needed for driver to perform auto-waveform selection
> + */
> +struct mxcfb_waveform_modes {
> + int mode_init;
> + int mode_du;
> + int mode_gc4;
> + int mode_gc8;
> + int mode_gc16;
> + int mode_gc32;
> +};
> +
> +/* IOCTL commands. */
> +
> +#define MXCFB_WAIT_FOR_VSYNC _IOW('F', 0x20, u_int32_t)
> +#define MXCFB_SET_GBL_ALPHA _IOW('F', 0x21, struct mxcfb_gbl_alpha)
> +#define MXCFB_SET_CLR_KEY _IOW('F', 0x22, struct mxcfb_color_key)
> +#define MXCFB_SET_OVERLAY_POS _IOWR('F', 0x24, struct mxcfb_pos)
> +#define MXCFB_GET_FB_IPU_CHAN _IOR('F', 0x25, u_int32_t)
> +#define MXCFB_SET_LOC_ALPHA _IOWR('F', 0x26, struct mxcfb_loc_alpha)
> +#define MXCFB_SET_LOC_ALP_BUF _IOW('F', 0x27, unsigned long)
> +#define MXCFB_SET_GAMMA _IOW('F', 0x28, struct mxcfb_gamma)
> +#define MXCFB_GET_FB_IPU_DI _IOR('F', 0x29, u_int32_t)
> +#define MXCFB_GET_DIFMT _IOR('F', 0x2A, u_int32_t)
> +#define MXCFB_GET_FB_BLANK _IOR('F', 0x2B, u_int32_t)
Please, don't add unused identifiers.
> +
> +#ifdef __KERNEL__
> +
> +enum {
> + MXCFB_REFRESH_OFF,
> + MXCFB_REFRESH_AUTO,
> + MXCFB_REFRESH_PARTIAL,
> +};
> +
> +#endif
> +
> +#endif /* _MXCFB_H */
> +
> --
> 1.7.0.4
>
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply
* Re: [PATCH] drivers/video/ep93xx-fb.c: clean up error-handling code
From: Ryan Mallon @ 2012-04-19 0:15 UTC (permalink / raw)
To: Julia Lawall
Cc: Florian Tobias Schandinat, kernel-janitors, linux-fbdev,
linux-kernel
In-Reply-To: <1334777828-3557-1-git-send-email-Julia.Lawall@lip6.fr>
On 19/04/12 05:37, 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>
Reviewed-by: Ryan Mallon <rmallon@gmail.com>
Oddly, scripts/get_maintainer.pl on this file doesn't return me, even
though, according to git blame, I am the author of 90% of the commits.
Should I have an entry in the MAINTAINERS file, or is
scripts/get_maintainer.pl not working properly?
~Ryan
> ---
> 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);
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* RE: [PATCH] drivers/video/ep93xx-fb.c: clean up error-handling code
From: Jingoo Han @ 2012-04-19 2:16 UTC (permalink / raw)
To: 'Ryan Mallon'
Cc: 'Florian Tobias Schandinat', kernel-janitors, linux-fbdev,
linux-kernel, 'Julia Lawall', 'Jingoo Han'
In-Reply-To: <4F8F5938.2000207@gmail.com>
> -----Original Message-----
> From: Ryan Mallon
> Sent: Thursday, April 19, 2012 9:16 AM
> To: Julia Lawall
> Cc: Florian Tobias Schandinat; kernel-janitors@vger.kernel.org; linux-fbdev@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH] drivers/video/ep93xx-fb.c: clean up error-handling code
>
> On 19/04/12 05:37, 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>
>
> Reviewed-by: Ryan Mallon <rmallon@gmail.com>
>
> Oddly, scripts/get_maintainer.pl on this file doesn't return me, even
> though, according to git blame, I am the author of 90% of the commits.
> Should I have an entry in the MAINTAINERS file, or is
> scripts/get_maintainer.pl not working properly?
There are optional differences in using scripts/get_maintainer.pl.
If you use './' ahead of file path, you will see your name.
Without './' ahead of 'drivers/video/ep93xx-fb.c':
./scripts/get_maintainer.pl --file drivers/video/ep93xx-fb.c
Florian Tobias Schandinat <FlorianSchandinat@gmx.de> (maintainer:FRAMEBUFFER LAYER)
linux-fbdev@vger.kernel.org (open list:FRAMEBUFFER LAYER)
linux-kernel@vger.kernel.org (open list)
With './' ahead of 'drivers/video/ep93xx-fb.c':
./scripts/get_maintainer.pl --file ./drivers/video/ep93xx-fb.c
Ryan Mallon <rmallon@gmail.com> (commit_signer:2/3g%)
Paul Gortmaker <paul.gortmaker@windriver.com> (commit_signer:1/33%)
H Hartley Sweeten <hsweeten@visionengravers.com> (commit_signer:1/33%)
Jesper Juhl <jj@chaosbits.net> (commit_signer:1/33%)
Jiri Kosina <jkosina@suse.cz> (commit_signer:1/33%)
linux-kernel@vger.kernel.org (open list)
Best regards,
Jingoo Han
>
> ~Ryan
>
> > ---
> > 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);
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] blackfin: fix compile error in bfin-lq035q1-fb.c
From: Paul Gortmaker @ 2012-04-19 2:43 UTC (permalink / raw)
To: linux-fbdev
This file has an implicit dependency on GPIO stuff, showing
up as the following build failure:
drivers/video/bfin-lq035q1-fb.c:369:6: error: 'GPIOF_OUT_INIT_LOW' undeclared
Other more global bfin build issues prevent an automated bisect, but
it really doesn't matter - simply add in the appropriate header.
Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
diff --git a/drivers/video/bfin-lq035q1-fb.c b/drivers/video/bfin-lq035q1-fb.c
index 86922ac..353c02f 100644
--- a/drivers/video/bfin-lq035q1-fb.c
+++ b/drivers/video/bfin-lq035q1-fb.c
@@ -13,6 +13,7 @@
#include <linux/errno.h>
#include <linux/string.h>
#include <linux/fb.h>
+#include <linux/gpio.h>
#include <linux/slab.h>
#include <linux/init.h>
#include <linux/types.h>
--
1.7.9.1
^ permalink raw reply related
* Re: [PATCH] blackfin: fix compile error in bfin-lq035q1-fb.c
From: Mike Frysinger @ 2012-04-19 2:49 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1334803401-26780-1-git-send-email-paul.gortmaker@windriver.com>
[-- Attachment #1: Type: Text/Plain, Size: 447 bytes --]
On Wednesday 18 April 2012 22:43:21 Paul Gortmaker wrote:
> This file has an implicit dependency on GPIO stuff, showing
> up as the following build failure:
>
> drivers/video/bfin-lq035q1-fb.c:369:6: error: 'GPIOF_OUT_INIT_LOW'
> undeclared
>
> Other more global bfin build issues prevent an automated bisect, but
> it really doesn't matter - simply add in the appropriate header.
Acked-by: Mike Frysinger <vapier@gentoo.org>
-mike
[-- 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: Julia Lawall @ 2012-04-19 5:14 UTC (permalink / raw)
To: Jingoo Han
Cc: 'Ryan Mallon', 'Florian Tobias Schandinat',
kernel-janitors, linux-fbdev, linux-kernel,
'Julia Lawall', joe
In-Reply-To: <000401cd1dd2$5f8ee3b0$1eacab10$%han@samsung.com>
I don't know thw impact of the ./. I use the options
--nokeywords --nogit-fallback --subsystem --norolestats -f
julia
On Thu, 19 Apr 2012, Jingoo Han wrote:
>> -----Original Message-----
>> From: Ryan Mallon
>> Sent: Thursday, April 19, 2012 9:16 AM
>> To: Julia Lawall
>> Cc: Florian Tobias Schandinat; kernel-janitors@vger.kernel.org; linux-fbdev@vger.kernel.org; linux-
>> kernel@vger.kernel.org
>> Subject: Re: [PATCH] drivers/video/ep93xx-fb.c: clean up error-handling code
>>
>> On 19/04/12 05:37, 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>
>>
>> Reviewed-by: Ryan Mallon <rmallon@gmail.com>
>>
>> Oddly, scripts/get_maintainer.pl on this file doesn't return me, even
>> though, according to git blame, I am the author of 90% of the commits.
>> Should I have an entry in the MAINTAINERS file, or is
>> scripts/get_maintainer.pl not working properly?
>
> There are optional differences in using scripts/get_maintainer.pl.
> If you use './' ahead of file path, you will see your name.
>
> Without './' ahead of 'drivers/video/ep93xx-fb.c':
> ./scripts/get_maintainer.pl --file drivers/video/ep93xx-fb.c
> Florian Tobias Schandinat <FlorianSchandinat@gmx.de> (maintainer:FRAMEBUFFER LAYER)
> linux-fbdev@vger.kernel.org (open list:FRAMEBUFFER LAYER)
> linux-kernel@vger.kernel.org (open list)
>
> With './' ahead of 'drivers/video/ep93xx-fb.c':
> ./scripts/get_maintainer.pl --file ./drivers/video/ep93xx-fb.c
> Ryan Mallon <rmallon@gmail.com> (commit_signer:2/3g%)
> Paul Gortmaker <paul.gortmaker@windriver.com> (commit_signer:1/33%)
> H Hartley Sweeten <hsweeten@visionengravers.com> (commit_signer:1/33%)
> Jesper Juhl <jj@chaosbits.net> (commit_signer:1/33%)
> Jiri Kosina <jkosina@suse.cz> (commit_signer:1/33%)
> linux-kernel@vger.kernel.org (open list)
>
>
> Best regards,
> Jingoo Han
>
>>
>> ~Ryan
>>
>>> ---
>>> 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);
>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at http://www.tux.org/lkml/
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH 4/6] OMAPDSS: MANAGER: Make DISPC timings a manager_info parameter
From: Archit Taneja @ 2012-04-19 6:25 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: Archit Taneja, linux-omap, linux-fbdev
In-Reply-To: <1334761105.2027.67.camel@deskari>
On Wednesday 18 April 2012 08:28 PM, Tomi Valkeinen wrote:
> On Mon, 2012-04-16 at 12:53 +0530, Archit Taneja wrote:
>> DISPC manager size and DISPC manager blanking parameters(for LCD managers)
>> follow the shadow register programming model. Currently, they are programmed
>> directly by the interface drivers.
>>
>> Make timings(omap_video_timing struct) an overlay_manager_info member, they are
>> now programmed via the apply mechanism used for programming shadow registers.
>>
>> The interface driver now call the function dss_mgr_set_timings() which applies
>> the new timing parameters, rather than directly writing to DISPC registers.
>
> I don't think that works correctly. The omap_overlay_manager_info is
> supposed to be set with set_manager_info() by the user of omapdss, to
> configure the manager's features. The timings are not supposed to be set
> via that mechanism, but with dssdev->set_timings().
>
> This is similar to the info and extra_info for overlay. info has stuff
> that omapdss doesn't change, it just uses what the user gives.
> extra_info, on the other hand, has omapdss private stuff that the user
> does not see. Timings are clearly private stuff in this sense, because
> they are set via dssdev->set_timings().
>
> One reason for this is the programming model we use. If the user of
> omapdss does get_info() for two overlays, changes the infos, and then
> calls set_info() for both overlays and finally apply() for the manager,
> we don't do any locking there because omapdss presumes the info is
> handled by one user. If, say, the dpi.c would change the info and call
> apply at the same time, the configuration could go badly wrong.
I think I get your point. So even though get_info() and set_info() fn's
are spinlock protected, if there are 2 users setting the info, it
doesn't mean that the info they finally written is correct. Is this
example the same thing as what you mean ? :
In order of time:
-user 1 gets an overlay's info
-user 2 gets an overlay's info
-user 1 modifies and sets overlay info
-user 2 sets overlay info without the knowledge of what user 1 did.
So even though we ensure these events happen sequentially, we don't
protect the info across multiple users.
>
> So I think what should be done is to add similar "extra" flags and code
> to mgr_priv_data that we have for ovl_priv_data, and add
> omap_video_timings to mgr_priv_data (the same way as we have, say,
> fifo_low for ovl_priv_data).
>
> And then add similar function to dss_ovl_write_regs_extra() for manager,
> and a function like dss_apply_ovl_fifo_thresholds() for timings. And
> finally a non-static function to set the timings (used by dpi.c etc),
> which calls the similar function to dss_apply_ovl_fifo_thresholds(), and
> dss_write_regs() and dss_set_go_bits().
Okay, I'll work on it along these lines.
>
> Tomi
>
^ permalink raw reply
* Re: [PATCH 4/6] OMAPDSS: MANAGER: Make DISPC timings a manager_info parameter
From: Tomi Valkeinen @ 2012-04-19 6:37 UTC (permalink / raw)
To: Archit Taneja; +Cc: Archit Taneja, linux-omap, linux-fbdev
In-Reply-To: <4F8FAD01.3090406@ti.com>
[-- Attachment #1: Type: text/plain, Size: 3139 bytes --]
On Thu, 2012-04-19 at 11:43 +0530, Archit Taneja wrote:
> On Wednesday 18 April 2012 08:28 PM, Tomi Valkeinen wrote:
> > On Mon, 2012-04-16 at 12:53 +0530, Archit Taneja wrote:
> >> DISPC manager size and DISPC manager blanking parameters(for LCD managers)
> >> follow the shadow register programming model. Currently, they are programmed
> >> directly by the interface drivers.
> >>
> >> Make timings(omap_video_timing struct) an overlay_manager_info member, they are
> >> now programmed via the apply mechanism used for programming shadow registers.
> >>
> >> The interface driver now call the function dss_mgr_set_timings() which applies
> >> the new timing parameters, rather than directly writing to DISPC registers.
> >
> > I don't think that works correctly. The omap_overlay_manager_info is
> > supposed to be set with set_manager_info() by the user of omapdss, to
> > configure the manager's features. The timings are not supposed to be set
> > via that mechanism, but with dssdev->set_timings().
> >
> > This is similar to the info and extra_info for overlay. info has stuff
> > that omapdss doesn't change, it just uses what the user gives.
> > extra_info, on the other hand, has omapdss private stuff that the user
> > does not see. Timings are clearly private stuff in this sense, because
> > they are set via dssdev->set_timings().
> >
> > One reason for this is the programming model we use. If the user of
> > omapdss does get_info() for two overlays, changes the infos, and then
> > calls set_info() for both overlays and finally apply() for the manager,
> > we don't do any locking there because omapdss presumes the info is
> > handled by one user. If, say, the dpi.c would change the info and call
> > apply at the same time, the configuration could go badly wrong.
>
> I think I get your point. So even though get_info() and set_info() fn's
> are spinlock protected, if there are 2 users setting the info, it
> doesn't mean that the info they finally written is correct. Is this
> example the same thing as what you mean ? :
>
> In order of time:
>
> -user 1 gets an overlay's info
>
> -user 2 gets an overlay's info
>
> -user 1 modifies and sets overlay info
>
> -user 2 sets overlay info without the knowledge of what user 1 did.
>
> So even though we ensure these events happen sequentially, we don't
> protect the info across multiple users.
Yes. The spinlocks ensure that the info is "whole", so we don't get a
few fields from user1 and a few fields from user2. But they don't
protect us from the case you described above.
For that we would need a "dss lock" that the user would acquire before
using get_info and set_info. But I don't want to go to that direction,
because we really only support one user anyway.
The problem in this particular case is that omapdss itself becomes
another user if it uses get_info & set_info. And that can be easily
avoided by splitting the configuration into public (the "info") and
internal ("extra_info"). The users of omapdss never touch the
extra_info, and omapdss never touches the info.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] backlight: Add LMS501KF03 LCD panel driver
From: Sachin Kamat @ 2012-04-19 7:52 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1334558406-22576-1-git-send-email-sachin.kamat@linaro.org>
On 18/04/2012, Florian Tobias Schandinat <FlorianSchandinat@gmx.de> wrote:
> On 04/16/2012 06:40 AM, Sachin Kamat wrote:
>> LMS501KF03 is a 480x800 LCD module with brightness control.
>> The driver uses 3-wired SPI inteface.
>>
>> Signed-off-by: Ilho Lee <Ilho215.lee@samsung.com>
>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>> ---
>
>> diff --git a/drivers/video/backlight/lms501kf03.c
>> b/drivers/video/backlight/lms501kf03.c
>> new file mode 100644
>> index 0000000..3dc85d4
>> --- /dev/null
>> +++ b/drivers/video/backlight/lms501kf03.c
>> +static int lms501kf03_ldi_enable(struct lms501kf03 *lcd)
>> +{
>> + int ret, i;
>> + const unsigned short *init_seq[] = {
>> + SEQ_DISPLAY_ON,
>> + };
>
> Is this array expected to grow at some point?
> Otherwise I'd suggest to get rid of it and simplify the code below.
OK. I will simplify the code.
>
>> +
>> + for (i = 0; i < ARRAY_SIZE(init_seq); i++) {
>> + ret = lms501kf03_panel_send_sequence(lcd, init_seq[i]);
>> + if (ret)
>> + break;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static int lms501kf03_ldi_disable(struct lms501kf03 *lcd)
>> +{
>> + int ret, i;
>> +
>> + const unsigned short *init_seq[] = {
>> + SEQ_DISPLAY_OFF,
>> + };
>
> dito
OK.
>
>> +
>> + for (i = 0; i < ARRAY_SIZE(init_seq); i++) {
>> + ret = lms501kf03_panel_send_sequence(lcd, init_seq[i]);
>> + if (ret)
>> + break;
>> + }
>> +
>> + return ret;
>> +}
>
>
Thank you for reviewing the code. I will re-send the patch with above
suggested changes.
> Best regards,
>
> Florian Tobias Schandinat
>
--
With warm regards,
Sachin
^ permalink raw reply
* Re: [PATCH 4/6] OMAPDSS: MANAGER: Make DISPC timings a manager_info parameter
From: Archit Taneja @ 2012-04-19 10:20 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1334817446.1521.57.camel@lappy>
On Thursday 19 April 2012 12:07 PM, Tomi Valkeinen wrote:
> On Thu, 2012-04-19 at 11:43 +0530, Archit Taneja wrote:
>> On Wednesday 18 April 2012 08:28 PM, Tomi Valkeinen wrote:
>>> On Mon, 2012-04-16 at 12:53 +0530, Archit Taneja wrote:
>>>> DISPC manager size and DISPC manager blanking parameters(for LCD managers)
>>>> follow the shadow register programming model. Currently, they are programmed
>>>> directly by the interface drivers.
>>>>
>>>> Make timings(omap_video_timing struct) an overlay_manager_info member, they are
>>>> now programmed via the apply mechanism used for programming shadow registers.
>>>>
>>>> The interface driver now call the function dss_mgr_set_timings() which applies
>>>> the new timing parameters, rather than directly writing to DISPC registers.
>>>
>>> I don't think that works correctly. The omap_overlay_manager_info is
>>> supposed to be set with set_manager_info() by the user of omapdss, to
>>> configure the manager's features. The timings are not supposed to be set
>>> via that mechanism, but with dssdev->set_timings().
>>>
>>> This is similar to the info and extra_info for overlay. info has stuff
>>> that omapdss doesn't change, it just uses what the user gives.
>>> extra_info, on the other hand, has omapdss private stuff that the user
>>> does not see. Timings are clearly private stuff in this sense, because
>>> they are set via dssdev->set_timings().
>>>
>>> One reason for this is the programming model we use. If the user of
>>> omapdss does get_info() for two overlays, changes the infos, and then
>>> calls set_info() for both overlays and finally apply() for the manager,
>>> we don't do any locking there because omapdss presumes the info is
>>> handled by one user. If, say, the dpi.c would change the info and call
>>> apply at the same time, the configuration could go badly wrong.
>>
>> I think I get your point. So even though get_info() and set_info() fn's
>> are spinlock protected, if there are 2 users setting the info, it
>> doesn't mean that the info they finally written is correct. Is this
>> example the same thing as what you mean ? :
>>
>> In order of time:
>>
>> -user 1 gets an overlay's info
>>
>> -user 2 gets an overlay's info
>>
>> -user 1 modifies and sets overlay info
>>
>> -user 2 sets overlay info without the knowledge of what user 1 did.
>>
>> So even though we ensure these events happen sequentially, we don't
>> protect the info across multiple users.
>
> Yes. The spinlocks ensure that the info is "whole", so we don't get a
> few fields from user1 and a few fields from user2. But they don't
> protect us from the case you described above.
>
> For that we would need a "dss lock" that the user would acquire before
> using get_info and set_info. But I don't want to go to that direction,
> because we really only support one user anyway.
>
> The problem in this particular case is that omapdss itself becomes
> another user if it uses get_info& set_info. And that can be easily
> avoided by splitting the configuration into public (the "info") and
> internal ("extra_info"). The users of omapdss never touch the
> extra_info, and omapdss never touches the info.
omapdss touches info via sysfs, so if we use sysfs(in a fast way, using
a it in scripts, for example) while fb uses DSS2, then we might hit this
issue.
Archit
>
> Tomi
>
^ permalink raw reply
* Re: [PATCH 4/6] OMAPDSS: MANAGER: Make DISPC timings a manager_info parameter
From: Tomi Valkeinen @ 2012-04-19 11:37 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <4F8FE42D.6030105@ti.com>
[-- Attachment #1: Type: text/plain, Size: 3861 bytes --]
On Thu, 2012-04-19 at 15:38 +0530, Archit Taneja wrote:
> On Thursday 19 April 2012 12:07 PM, Tomi Valkeinen wrote:
> > On Thu, 2012-04-19 at 11:43 +0530, Archit Taneja wrote:
> >> On Wednesday 18 April 2012 08:28 PM, Tomi Valkeinen wrote:
> >>> On Mon, 2012-04-16 at 12:53 +0530, Archit Taneja wrote:
> >>>> DISPC manager size and DISPC manager blanking parameters(for LCD managers)
> >>>> follow the shadow register programming model. Currently, they are programmed
> >>>> directly by the interface drivers.
> >>>>
> >>>> Make timings(omap_video_timing struct) an overlay_manager_info member, they are
> >>>> now programmed via the apply mechanism used for programming shadow registers.
> >>>>
> >>>> The interface driver now call the function dss_mgr_set_timings() which applies
> >>>> the new timing parameters, rather than directly writing to DISPC registers.
> >>>
> >>> I don't think that works correctly. The omap_overlay_manager_info is
> >>> supposed to be set with set_manager_info() by the user of omapdss, to
> >>> configure the manager's features. The timings are not supposed to be set
> >>> via that mechanism, but with dssdev->set_timings().
> >>>
> >>> This is similar to the info and extra_info for overlay. info has stuff
> >>> that omapdss doesn't change, it just uses what the user gives.
> >>> extra_info, on the other hand, has omapdss private stuff that the user
> >>> does not see. Timings are clearly private stuff in this sense, because
> >>> they are set via dssdev->set_timings().
> >>>
> >>> One reason for this is the programming model we use. If the user of
> >>> omapdss does get_info() for two overlays, changes the infos, and then
> >>> calls set_info() for both overlays and finally apply() for the manager,
> >>> we don't do any locking there because omapdss presumes the info is
> >>> handled by one user. If, say, the dpi.c would change the info and call
> >>> apply at the same time, the configuration could go badly wrong.
> >>
> >> I think I get your point. So even though get_info() and set_info() fn's
> >> are spinlock protected, if there are 2 users setting the info, it
> >> doesn't mean that the info they finally written is correct. Is this
> >> example the same thing as what you mean ? :
> >>
> >> In order of time:
> >>
> >> -user 1 gets an overlay's info
> >>
> >> -user 2 gets an overlay's info
> >>
> >> -user 1 modifies and sets overlay info
> >>
> >> -user 2 sets overlay info without the knowledge of what user 1 did.
> >>
> >> So even though we ensure these events happen sequentially, we don't
> >> protect the info across multiple users.
> >
> > Yes. The spinlocks ensure that the info is "whole", so we don't get a
> > few fields from user1 and a few fields from user2. But they don't
> > protect us from the case you described above.
> >
> > For that we would need a "dss lock" that the user would acquire before
> > using get_info and set_info. But I don't want to go to that direction,
> > because we really only support one user anyway.
> >
> > The problem in this particular case is that omapdss itself becomes
> > another user if it uses get_info& set_info. And that can be easily
> > avoided by splitting the configuration into public (the "info") and
> > internal ("extra_info"). The users of omapdss never touch the
> > extra_info, and omapdss never touches the info.
>
> omapdss touches info via sysfs, so if we use sysfs(in a fast way, using
> a it in scripts, for example) while fb uses DSS2, then we might hit this
> issue.
The sysfs calls come from the user, so in that sense it's not omapdss
that is doing the change.
But it's true that with sysfs we may have two users for omapdss, for
example the scripts using sysfs and omapfb. Hopefully we can deprecate
the sysfs files at some point...
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH v2] backlight: Add LMS501KF03 LCD panel driver
From: Sachin Kamat @ 2012-04-19 11:45 UTC (permalink / raw)
To: linux-fbdev
LMS501KF03 is a 480x800 LCD module with brightness control.
The driver uses 3-wired SPI inteface.
Signed-off-by: Ilho Lee <Ilho215.lee@samsung.com>
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
Changes since v1:
Incorporated review comments from Florian Tobias Schandinat
<FlorianSchandinat@gmx.de> - Simplied lms501kf03_ldi_enable and
lms501kf03_ldi_disable function implementations.
---
drivers/video/backlight/Kconfig | 8 +
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/lms501kf03.c | 525 ++++++++++++++++++++++++++++++++++
3 files changed, 534 insertions(+), 0 deletions(-)
create mode 100644 drivers/video/backlight/lms501kf03.c
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index af16884..e216f77 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -125,6 +125,14 @@ config LCD_AMS369FG06
If you have an AMS369FG06 AMOLED Panel, say Y to enable its
LCD control driver.
+config LCD_LMS501KF03
+ tristate "LMS501KF03 AMOLED LCD Driver"
+ depends on SPI_GPIO && BACKLIGHT_CLASS_DEVICE
+ default n
+ help
+ If you have an 5.01" LMS501KF03 AMOLED Panel, say Y to enable its
+ LCD control driver.
+
endif # LCD_CLASS_DEVICE
#
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index 36855ae..1b1e62a 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_LCD_TOSA) += tosa_lcd.o
obj-$(CONFIG_LCD_S6E63M0) += s6e63m0.o
obj-$(CONFIG_LCD_LD9040) += ld9040.o
obj-$(CONFIG_LCD_AMS369FG06) += ams369fg06.o
+obj-$(CONFIG_LCD_LMS501KF03) += lms501kf03.o
obj-$(CONFIG_BACKLIGHT_CLASS_DEVICE) += backlight.o
obj-$(CONFIG_BACKLIGHT_ATMEL_PWM) += atmel-pwm-bl.o
diff --git a/drivers/video/backlight/lms501kf03.c b/drivers/video/backlight/lms501kf03.c
new file mode 100644
index 0000000..af25532
--- /dev/null
+++ b/drivers/video/backlight/lms501kf03.c
@@ -0,0 +1,525 @@
+/*
+ * Copyright (c) 2012 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com/
+ *
+ * LMS501KF03 5.01" LCD module driver
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+*/
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/fb.h>
+#include <linux/gpio.h>
+#include <linux/lcd.h>
+#include <linux/module.h>
+#include <linux/spi/spi.h>
+#include <linux/wait.h>
+
+#define ENDDEF 0xFF00
+#define COMMAND_ONLY 0x00
+#define DATA_ONLY 0x01
+
+#define MIN_BRIGHTNESS 0
+#define MAX_BRIGHTNESS 255
+#define DEFAULT_BRIGHTNESS 150
+
+#define POWER_IS_ON(power) ((power) <= FB_BLANK_NORMAL)
+
+struct lms501kf03 {
+ struct device *dev;
+ struct spi_device *spi;
+ unsigned int power;
+ struct lcd_device *ld;
+ struct backlight_device *bd;
+ struct lcd_platform_data *lcd_pd;
+};
+
+const unsigned short SEQ_PASSWORD[] = {
+ 0xb9, 0xff, 0x83, 0x69,
+ ENDDEF
+};
+
+const unsigned short SEQ_POWER[] = {
+ 0xb1, 0x01, 0x00, 0x34, 0x06, 0x00, 0x14, 0x14, 0x20, 0x28,
+ 0x12, 0x12, 0x17, 0x0a, 0x01, 0xe6, 0xe6, 0xe6, 0xe6, 0xe6,
+ ENDDEF
+};
+
+const unsigned short SEQ_DISPLAY[] = {
+ 0xb2, 0x00, 0x2b, 0x03, 0x03, 0x70, 0x00, 0xff, 0x00, 0x00,
+ 0x00, 0x00, 0x03, 0x03, 0x00, 0x01,
+ ENDDEF
+};
+
+const unsigned short SEQ_RGB_IF[] = {
+ 0xb3, 0x09,
+ ENDDEF
+};
+
+const unsigned short SEQ_DISPLAY_INV[] = {
+ 0xb4, 0x01, 0x08, 0x77, 0x0e, 0x06,
+ ENDDEF
+};
+
+const unsigned short SEQ_VCOM[] = {
+ 0xb6, 0x4c, 0x2e,
+ ENDDEF
+};
+
+const unsigned short SEQ_GATE[] = {
+ 0xd5, 0x00, 0x05, 0x03, 0x29, 0x01, 0x07, 0x17, 0x68, 0x13,
+ 0x37, 0x20, 0x31, 0x8a, 0x46, 0x9b, 0x57, 0x13, 0x02, 0x75,
+ 0xb9, 0x64, 0xa8, 0x07, 0x0f, 0x04, 0x07,
+ ENDDEF
+};
+
+const unsigned short SEQ_PANEL[] = {
+ 0xcc, 0x02,
+ ENDDEF
+};
+
+const unsigned short SEQ_COL_MOD[] = {
+ 0x3a, 0x77,
+ ENDDEF
+};
+
+const unsigned short SEQ_W_GAMMA[] = {
+ 0xe0, 0x00, 0x04, 0x09, 0x0f, 0x1f, 0x3f, 0x1f, 0x2f, 0x0a,
+ 0x0f, 0x10, 0x16, 0x18, 0x16, 0x17, 0x0d, 0x15, 0x00, 0x04,
+ 0x09, 0x0f, 0x38, 0x3f, 0x20, 0x39, 0x0a, 0x0f, 0x10, 0x16,
+ 0x18, 0x16, 0x17, 0x0d, 0x15,
+ ENDDEF
+};
+
+const unsigned short SEQ_RGB_GAMMA[] = {
+ 0xc1, 0x01, 0x03, 0x07, 0x0f, 0x1a, 0x22, 0x2c, 0x33, 0x3c,
+ 0x46, 0x4f, 0x58, 0x60, 0x69, 0x71, 0x79, 0x82, 0x89, 0x92,
+ 0x9a, 0xa1, 0xa9, 0xb1, 0xb9, 0xc1, 0xc9, 0xcf, 0xd6, 0xde,
+ 0xe5, 0xec, 0xf3, 0xf9, 0xff, 0xdd, 0x39, 0x07, 0x1c, 0xcb,
+ 0xab, 0x5f, 0x49, 0x80, 0x03, 0x07, 0x0f, 0x19, 0x20, 0x2a,
+ 0x31, 0x39, 0x42, 0x4b, 0x53, 0x5b, 0x63, 0x6b, 0x73, 0x7b,
+ 0x83, 0x8a, 0x92, 0x9b, 0xa2, 0xaa, 0xb2, 0xba, 0xc2, 0xca,
+ 0xd0, 0xd8, 0xe1, 0xe8, 0xf0, 0xf8, 0xff, 0xf7, 0xd8, 0xbe,
+ 0xa7, 0x39, 0x40, 0x85, 0x8c, 0xc0, 0x04, 0x07, 0x0c, 0x17,
+ 0x1c, 0x23, 0x2b, 0x34, 0x3b, 0x43, 0x4c, 0x54, 0x5b, 0x63,
+ 0x6a, 0x73, 0x7a, 0x82, 0x8a, 0x91, 0x98, 0xa1, 0xa8, 0xb0,
+ 0xb7, 0xc1, 0xc9, 0xcf, 0xd9, 0xe3, 0xea, 0xf4, 0xff, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ ENDDEF
+};
+
+const unsigned short SEQ_UP_DN[] = {
+ 0x36, 0x10,
+ ENDDEF
+};
+
+const unsigned short SEQ_SLEEP_IN[] = {
+ 0x10,
+ ENDDEF
+};
+
+const unsigned short SEQ_SLEEP_OUT[] = {
+ 0x11,
+ ENDDEF
+};
+
+const unsigned short SEQ_DISPLAY_ON[] = {
+ 0x29,
+ ENDDEF
+};
+
+const unsigned short SEQ_DISPLAY_OFF[] = {
+ 0x10,
+ ENDDEF
+};
+
+static int lms501kf03_spi_write_byte(struct lms501kf03 *lcd, int addr, int data)
+{
+ u16 buf[1];
+ struct spi_message msg;
+
+ struct spi_transfer xfer = {
+ .len = 2,
+ .tx_buf = buf,
+ };
+
+ buf[0] = (addr << 8) | data;
+
+ spi_message_init(&msg);
+ spi_message_add_tail(&xfer, &msg);
+
+ return spi_sync(lcd->spi, &msg);
+}
+
+static int lms501kf03_spi_write(struct lms501kf03 *lcd, unsigned char address,
+ unsigned char command)
+{
+ int ret = 0;
+
+ ret = lms501kf03_spi_write_byte(lcd, address, command);
+
+ return ret;
+}
+
+static int lms501kf03_panel_send_sequence(struct lms501kf03 *lcd,
+ const unsigned short *wbuf)
+{
+ int ret = 0, i = 0;
+
+ while (wbuf[i] != ENDDEF) {
+ if (i = 0)
+ ret = lms501kf03_spi_write(lcd, COMMAND_ONLY, wbuf[i]);
+ else
+ ret = lms501kf03_spi_write(lcd, DATA_ONLY, wbuf[i]);
+ if (ret)
+ break;
+
+ udelay(100);
+ i += 1;
+ }
+ return ret;
+}
+
+static int lms501kf03_ldi_init(struct lms501kf03 *lcd)
+{
+ int ret, i;
+ const unsigned short *init_seq[] = {
+ SEQ_PASSWORD,
+ SEQ_POWER,
+ SEQ_DISPLAY,
+ SEQ_RGB_IF,
+ SEQ_DISPLAY_INV,
+ SEQ_VCOM,
+ SEQ_GATE,
+ SEQ_PANEL,
+ SEQ_COL_MOD,
+ SEQ_W_GAMMA,
+ SEQ_RGB_GAMMA,
+ SEQ_SLEEP_OUT,
+ };
+
+ for (i = 0; i < ARRAY_SIZE(init_seq); i++) {
+ ret = lms501kf03_panel_send_sequence(lcd, init_seq[i]);
+ if (ret)
+ break;
+ }
+ mdelay(120);
+
+ return ret;
+}
+
+static int lms501kf03_ldi_enable(struct lms501kf03 *lcd)
+{
+ return lms501kf03_panel_send_sequence(lcd, SEQ_DISPLAY_ON);
+}
+
+static int lms501kf03_ldi_disable(struct lms501kf03 *lcd)
+{
+ return lms501kf03_panel_send_sequence(lcd, SEQ_DISPLAY_OFF);
+}
+
+static int lms501kf03_power_on(struct lms501kf03 *lcd)
+{
+ int ret = 0;
+ struct lcd_platform_data *pd = NULL;
+ struct backlight_device *bd = NULL;
+
+ pd = lcd->lcd_pd;
+ if (!pd) {
+ dev_err(lcd->dev, "platform data is NULL.\n");
+ return -EFAULT;
+ }
+
+ bd = lcd->bd;
+ if (!bd) {
+ dev_err(lcd->dev, "backlight device is NULL.\n");
+ return -EFAULT;
+ }
+
+ if (!pd->power_on) {
+ dev_err(lcd->dev, "power_on is NULL.\n");
+ return -EFAULT;
+ } else {
+ pd->power_on(lcd->ld, 1);
+ mdelay(pd->power_on_delay);
+ }
+
+ if (!pd->reset) {
+ dev_err(lcd->dev, "reset is NULL.\n");
+ return -EFAULT;
+ } else {
+ pd->reset(lcd->ld);
+ mdelay(pd->reset_delay);
+ }
+
+ ret = lms501kf03_ldi_init(lcd);
+ if (ret) {
+ dev_err(lcd->dev, "failed to initialize ldi.\n");
+ return ret;
+ }
+
+ ret = lms501kf03_ldi_enable(lcd);
+ if (ret) {
+ dev_err(lcd->dev, "failed to enable ldi.\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int lms501kf03_power_off(struct lms501kf03 *lcd)
+{
+ int ret = 0;
+ struct lcd_platform_data *pd = NULL;
+
+ pd = lcd->lcd_pd;
+ if (!pd) {
+ dev_err(lcd->dev, "platform data is NULL\n");
+ return -EFAULT;
+ }
+
+ ret = lms501kf03_ldi_disable(lcd);
+ if (ret) {
+ dev_err(lcd->dev, "lcd setting failed.\n");
+ return -EIO;
+ }
+
+ mdelay(pd->power_off_delay);
+
+ if (!pd->power_on) {
+ dev_err(lcd->dev, "power_on is NULL.\n");
+ return -EFAULT;
+ } else
+ pd->power_on(lcd->ld, 0);
+
+ return 0;
+}
+
+static int lms501kf03_power(struct lms501kf03 *lcd, int power)
+{
+ int ret = 0;
+
+ if (POWER_IS_ON(power) && !POWER_IS_ON(lcd->power))
+ ret = lms501kf03_power_on(lcd);
+ else if (!POWER_IS_ON(power) && POWER_IS_ON(lcd->power))
+ ret = lms501kf03_power_off(lcd);
+
+ if (!ret)
+ lcd->power = power;
+
+ return ret;
+}
+
+static int lms501kf03_get_power(struct lcd_device *ld)
+{
+ struct lms501kf03 *lcd = lcd_get_data(ld);
+
+ return lcd->power;
+}
+
+static int lms501kf03_set_power(struct lcd_device *ld, int power)
+{
+ struct lms501kf03 *lcd = lcd_get_data(ld);
+
+ if (power != FB_BLANK_UNBLANK && power != FB_BLANK_POWERDOWN &&
+ power != FB_BLANK_NORMAL) {
+ dev_err(lcd->dev, "power value should be 0, 1 or 4.\n");
+ return -EINVAL;
+ }
+
+ return lms501kf03_power(lcd, power);
+}
+
+static int lms501kf03_get_brightness(struct backlight_device *bd)
+{
+ return bd->props.brightness;
+}
+
+static int lms501kf03_set_brightness(struct backlight_device *bd)
+{
+ int ret = 0;
+ int brightness = bd->props.brightness;
+
+ if (brightness < MIN_BRIGHTNESS ||
+ brightness > bd->props.max_brightness) {
+ dev_err(&bd->dev, "lcd brightness should be %d to %d.\n",
+ MIN_BRIGHTNESS, MAX_BRIGHTNESS);
+ return -EINVAL;
+ }
+
+ return ret;
+}
+
+static struct lcd_ops lms501kf03_lcd_ops = {
+ .get_power = lms501kf03_get_power,
+ .set_power = lms501kf03_set_power,
+};
+
+static const struct backlight_ops lms501kf03_backlight_ops = {
+ .get_brightness = lms501kf03_get_brightness,
+ .update_status = lms501kf03_set_brightness,
+};
+
+static int __devinit lms501kf03_probe(struct spi_device *spi)
+{
+ struct lms501kf03 *lcd = NULL;
+ struct lcd_device *ld = NULL;
+ struct backlight_device *bd = NULL;
+ struct backlight_properties props;
+ int ret = 0;
+
+ lcd = kzalloc(sizeof(struct lms501kf03), GFP_KERNEL);
+ if (!lcd)
+ return -ENOMEM;
+
+ /* lms501kf03 lcd panel uses 3-wire 9-bit SPI Mode. */
+ spi->bits_per_word = 9;
+
+ ret = spi_setup(spi);
+ if (ret < 0) {
+ dev_err(&spi->dev, "spi setup failed.\n");
+ goto out_free_lcd;
+ }
+
+ lcd->spi = spi;
+ lcd->dev = &spi->dev;
+
+ lcd->lcd_pd = (struct lcd_platform_data *)spi->dev.platform_data;
+ if (!lcd->lcd_pd) {
+ dev_err(&spi->dev, "platform data is NULL\n");
+ goto out_free_lcd;
+ }
+
+ ld = lcd_device_register("lms501kf03", &spi->dev, lcd,
+ &lms501kf03_lcd_ops);
+ if (IS_ERR(ld)) {
+ ret = PTR_ERR(ld);
+ goto out_free_lcd;
+ }
+
+ lcd->ld = ld;
+
+ memset(&props, 0, sizeof(struct backlight_properties));
+ props.type = BACKLIGHT_RAW;
+ props.max_brightness = MAX_BRIGHTNESS;
+
+ bd = backlight_device_register("lms501kf03-bl", &spi->dev, lcd,
+ &lms501kf03_backlight_ops, &props);
+ if (IS_ERR(bd)) {
+ ret = PTR_ERR(bd);
+ goto out_lcd_unregister;
+ }
+
+ bd->props.brightness = DEFAULT_BRIGHTNESS;
+ lcd->bd = bd;
+
+ if (!lcd->lcd_pd->lcd_enabled) {
+ /*
+ * if lcd panel was off from bootloader then
+ * current lcd status is powerdown and then
+ * it enables lcd panel.
+ */
+ lcd->power = FB_BLANK_POWERDOWN;
+
+ lms501kf03_power(lcd, FB_BLANK_UNBLANK);
+ } else
+ lcd->power = FB_BLANK_UNBLANK;
+
+ dev_set_drvdata(&spi->dev, lcd);
+
+ dev_info(&spi->dev, "lms501kf03 panel driver has been probed.\n");
+
+ return 0;
+
+out_lcd_unregister:
+ lcd_device_unregister(ld);
+out_free_lcd:
+ kfree(lcd);
+ return ret;
+}
+
+static int __devexit lms501kf03_remove(struct spi_device *spi)
+{
+ struct lms501kf03 *lcd = dev_get_drvdata(&spi->dev);
+
+ lms501kf03_power(lcd, FB_BLANK_POWERDOWN);
+ lcd_device_unregister(lcd->ld);
+ kfree(lcd);
+
+ return 0;
+}
+
+#if defined(CONFIG_PM)
+unsigned int before_power;
+
+static int lms501kf03_suspend(struct spi_device *spi, pm_message_t mesg)
+{
+ int ret = 0;
+ struct lms501kf03 *lcd = dev_get_drvdata(&spi->dev);
+
+ dev_dbg(&spi->dev, "lcd->power = %d\n", lcd->power);
+
+ before_power = lcd->power;
+
+ /*
+ * when lcd panel is suspend, lcd panel becomes off
+ * regardless of status.
+ */
+ ret = lms501kf03_power(lcd, FB_BLANK_POWERDOWN);
+
+ return ret;
+}
+
+static int lms501kf03_resume(struct spi_device *spi)
+{
+ int ret = 0;
+ struct lms501kf03 *lcd = dev_get_drvdata(&spi->dev);
+
+ /*
+ * after suspended, if lcd panel status is FB_BLANK_UNBLANK
+ * (at that time, before_power is FB_BLANK_UNBLANK) then
+ * it changes that status to FB_BLANK_POWERDOWN to get lcd on.
+ */
+ if (before_power = FB_BLANK_UNBLANK)
+ lcd->power = FB_BLANK_POWERDOWN;
+
+ dev_dbg(&spi->dev, "before_power = %d\n", before_power);
+
+ ret = lms501kf03_power(lcd, before_power);
+
+ return ret;
+}
+#else
+#define lms501kf03_suspend NULL
+#define lms501kf03_resume NULL
+#endif
+
+void lms501kf03_shutdown(struct spi_device *spi)
+{
+ struct lms501kf03 *lcd = dev_get_drvdata(&spi->dev);
+
+ lms501kf03_power(lcd, FB_BLANK_POWERDOWN);
+}
+
+static struct spi_driver lms501kf03_driver = {
+ .driver = {
+ .name = "lms501kf03",
+ .bus = &spi_bus_type,
+ .owner = THIS_MODULE,
+ },
+ .probe = lms501kf03_probe,
+ .remove = __devexit_p(lms501kf03_remove),
+ .shutdown = lms501kf03_shutdown,
+ .suspend = lms501kf03_suspend,
+ .resume = lms501kf03_resume,
+};
+
+module_spi_driver(lms501kf03_driver);
+
+MODULE_AUTHOR("Ilho Lee <Ilho215.lee@samsung.com>");
+MODULE_AUTHOR("Sachin Kamat <sachin.kamat@samsung.com>");
+MODULE_DESCRIPTION("LMS501KF03 LCD Driver");
+MODULE_LICENSE("GPL");
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH 0/6] OMAPDSS: APPLY: Treat overlay manager timings as shadow registers
From: Tomi Valkeinen @ 2012-04-19 11:48 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1334561027-28569-1-git-send-email-archit@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1263 bytes --]
On Mon, 2012-04-16 at 12:53 +0530, Archit Taneja wrote:
> An overlay manager's timings (the manager size, and blanking parameters if an
> LCD manager) are DISPC shadow registers, and they should hence follow the
> correct programming model.
>
> This set makes the timings a manager_info parameter. The interface drivers now
> set the timings in manager_info instead of directly writing to registers.
>
> This change also prevents the need to use display resolution for overlay
> checks, hence making some of the APPLY functions less dependent on the display.
>
> These patches apply over:
>
> git://gitorious.org/linux-omap-dss2/linux.git dev
>
> Archit Taneja (6):
> OMAPDSS: DISPC/RFBI: Use dispc_mgr_set_lcd_timings() for setting lcd
> size
> OMAPDSS: DISPC: Use a common function to set manager timings
> OMAPDSS: DISPC: Clean up manager timing/size functions
> OMAPDSS: MANAGER: Make DISPC timings a manager_info parameter
> OMAPDSS: MANAGER: Check validity of manager timings
> OMAPDSS: APPLY: Remove display dependency from overlay and manager
> checks
The patches 1, 2 and 3 in this series look like independent cleanups.
Should I apply those, or do you think there may be changes required?
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 0/6] OMAPDSS: APPLY: Treat overlay manager timings as shadow registers
From: Tomi Valkeinen @ 2012-04-19 12:00 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <4F8FFDD5.5030302@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1799 bytes --]
On Thu, 2012-04-19 at 17:28 +0530, Archit Taneja wrote:
> On Thursday 19 April 2012 05:18 PM, Tomi Valkeinen wrote:
> > On Mon, 2012-04-16 at 12:53 +0530, Archit Taneja wrote:
> >> An overlay manager's timings (the manager size, and blanking parameters if an
> >> LCD manager) are DISPC shadow registers, and they should hence follow the
> >> correct programming model.
> >>
> >> This set makes the timings a manager_info parameter. The interface drivers now
> >> set the timings in manager_info instead of directly writing to registers.
> >>
> >> This change also prevents the need to use display resolution for overlay
> >> checks, hence making some of the APPLY functions less dependent on the display.
> >>
> >> These patches apply over:
> >>
> >> git://gitorious.org/linux-omap-dss2/linux.git dev
> >>
> >> Archit Taneja (6):
> >> OMAPDSS: DISPC/RFBI: Use dispc_mgr_set_lcd_timings() for setting lcd
> >> size
> >> OMAPDSS: DISPC: Use a common function to set manager timings
> >> OMAPDSS: DISPC: Clean up manager timing/size functions
> >> OMAPDSS: MANAGER: Make DISPC timings a manager_info parameter
> >> OMAPDSS: MANAGER: Check validity of manager timings
> >> OMAPDSS: APPLY: Remove display dependency from overlay and manager
> >> checks
> >
> > The patches 1, 2 and 3 in this series look like independent cleanups.
> > Should I apply those, or do you think there may be changes required?
>
> I think you can pull these 3, they were sort of a prerequisite for the
> apply-manager-timings stuff. I don't think they need to change.
>
> I'll post another series for applying the manager timings correctly(i.e,
> the fixed version of the last 3 patches).
Ok, I applied the first three patches to my dev branch.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 0/6] OMAPDSS: APPLY: Treat overlay manager timings as shadow registers
From: Archit Taneja @ 2012-04-19 12:10 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1334836135.1911.40.camel@deskari>
On Thursday 19 April 2012 05:18 PM, Tomi Valkeinen wrote:
> On Mon, 2012-04-16 at 12:53 +0530, Archit Taneja wrote:
>> An overlay manager's timings (the manager size, and blanking parameters if an
>> LCD manager) are DISPC shadow registers, and they should hence follow the
>> correct programming model.
>>
>> This set makes the timings a manager_info parameter. The interface drivers now
>> set the timings in manager_info instead of directly writing to registers.
>>
>> This change also prevents the need to use display resolution for overlay
>> checks, hence making some of the APPLY functions less dependent on the display.
>>
>> These patches apply over:
>>
>> git://gitorious.org/linux-omap-dss2/linux.git dev
>>
>> Archit Taneja (6):
>> OMAPDSS: DISPC/RFBI: Use dispc_mgr_set_lcd_timings() for setting lcd
>> size
>> OMAPDSS: DISPC: Use a common function to set manager timings
>> OMAPDSS: DISPC: Clean up manager timing/size functions
>> OMAPDSS: MANAGER: Make DISPC timings a manager_info parameter
>> OMAPDSS: MANAGER: Check validity of manager timings
>> OMAPDSS: APPLY: Remove display dependency from overlay and manager
>> checks
>
> The patches 1, 2 and 3 in this series look like independent cleanups.
> Should I apply those, or do you think there may be changes required?
I think you can pull these 3, they were sort of a prerequisite for the
apply-manager-timings stuff. I don't think they need to change.
I'll post another series for applying the manager timings correctly(i.e,
the fixed version of the last 3 patches).
Archit
>
> Tomi
>
^ permalink raw reply
* Re: [PATCH V3 3/3] OMAPDSS: DISPC: Correct DISPC functional clock usage
From: Tomi Valkeinen @ 2012-04-19 13:11 UTC (permalink / raw)
To: Chandrabhanu Mahapatra, Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1333379598-11544-4-git-send-email-cmahapatra@ti.com>
[-- Attachment #1: Type: text/plain, Size: 4271 bytes --]
On Mon, 2012-04-02 at 20:43 +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 | 28 ++++++++++++++++++++++------
> drivers/video/omap2/dss/dss.h | 1 +
> 2 files changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index 17ffa71..cfde674 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -1813,6 +1813,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
> dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
> const int max_decim_limit = 16;
> unsigned long fclk = 0;
> + unsigned long dispc_core_clk = dispc_core_clk_rate(channel);
> int decim_x, decim_y, error, min_factor;
> u16 in_width, in_height, in_width_max = 0;
>
> @@ -1855,7 +1856,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);
> if (error) {
> if (decim_x == decim_y) {
> decim_x = min_factor;
> @@ -1893,7 +1894,7 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
> out_width, out_height);
> error = (error || in_width > maxsinglelinewidth * 2 ||
> (in_width > maxsinglelinewidth && *five_taps) ||
> - !fclk || fclk > dispc_fclk_rate());
> + !fclk || fclk > dispc_core_clk);
> if (error) {
> if (decim_x == decim_y) {
> decim_x = min_factor;
> @@ -1926,7 +1927,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 /
> DIV_ROUND_UP(dispc_mgr_pclk_rate(channel),
> out_width);
> decim_x = DIV_ROUND_UP(width, in_width_max);
> @@ -1950,13 +1951,13 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
> }
>
> DSSDBG("required fclk rate = %lu Hz\n", fclk);
> - DSSDBG("current fclk rate = %lu Hz\n", dispc_fclk_rate());
> + DSSDBG("current fclk rate = %lu Hz\n", dispc_core_clk);
>
> - if (!fclk || fclk > dispc_fclk_rate()) {
> + if (!fclk || fclk > dispc_core_clk) {
> DSSERR("failed to set up scaling, "
> "required fclk rate = %lu Hz, "
> "current fclk rate = %lu Hz\n",
> - fclk, dispc_fclk_rate());
> + fclk, dispc_core_clk);
> return -EINVAL;
> }
>
> @@ -2646,6 +2647,21 @@ unsigned long dispc_mgr_pclk_rate(enum omap_channel channel)
> }
> }
>
> +unsigned long dispc_core_clk_rate(enum omap_channel channel)
> +{
> + int lcd = 1;
> + unsigned long r = dispc_fclk_rate();
> +
> + if (dss_has_feature(FEAT_CORE_CLK_DIV)) {
> + lcd = REG_GET(DISPC_DIVISOR, 23, 16);
> + } else {
> + if (dispc_mgr_is_lcd(channel))
> + lcd = REG_GET(DISPC_DIVISORo(channel), 23, 16);
> + }
> +
> + return r / lcd ;
> +}
> +
I wonder if this is correct. "channel" for dispc core clock doesn't make
sense, there's no channel related to that. At least on OMAP4.
If I'm not mistaken, in omap2/3 case (i.e.
dss_has_feature(FEAT_CORE_CLK_DIV) == false) we can just use channel 0
to get the lcd divisor. Although that would mean that LCD output's
divisor affects the tv-out's scaling calculations, which feels a bit
strange...
So... I don't know... Do we really have two different dispc core clocks
int omap2/3 (for lcd output and for tv output), but only one in omap4?
If so, then the above code is ok. Have you found explanations from TRM
which say what clock is required and where?
In any case, please remove the initialization of lcd variable, and add:
else
lcd = 1;
I think that's much clearer. And "r" variable is commonly used as a
return value. I would rename the variable to something else, say, "fck".
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] i.MX28: Shut down the LCD controller to avoid BootROM sampling bug
From: Shawn Guo @ 2012-04-19 14:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1334767680-12688-1-git-send-email-marex@denx.de>
On Wed, Apr 18, 2012 at 06:48:00PM +0200, Marek Vasut wrote:
...
> +void mxsfb_shutdown(struct platform_device *pdev)
static?
Otherwise,
Acked-by: Shawn Guo <shawn.guo@linaro.org>
> +{
> + 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,
> --
> 1.7.9.5
>
^ permalink raw reply
* [PATCH] drivers/video/intelfb/intelfbdrv.c: add missing agp_backend_release
From: Julia Lawall @ 2012-04-19 16:55 UTC (permalink / raw)
To: Maik Broemme
Cc: kernel-janitors, Florian Tobias Schandinat, linux-fbdev,
linux-kernel
From: Julia Lawall <Julia.Lawall@lip6.fr>
Release bridge, as done on all other paths out of the function.
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
drivers/video/intelfb/intelfbdrv.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/video/intelfb/intelfbdrv.c b/drivers/video/intelfb/intelfbdrv.c
index 02fd226..bdcbfba 100644
--- a/drivers/video/intelfb/intelfbdrv.c
+++ b/drivers/video/intelfb/intelfbdrv.c
@@ -680,6 +680,7 @@ static int __devinit intelfb_pci_register(struct pci_dev *pdev,
+ dinfo->fb.size);
if (!dinfo->aperture.virtual) {
ERR_MSG("Cannot remap FB region.\n");
+ agp_backend_release(bridge);
cleanup(dinfo);
return -ENODEV;
}
@@ -689,6 +690,7 @@ static int __devinit intelfb_pci_register(struct pci_dev *pdev,
INTEL_REG_SIZE);
if (!dinfo->mmio_base) {
ERR_MSG("Cannot remap MMIO region.\n");
+ agp_backend_release(bridge);
cleanup(dinfo);
return -ENODEV;
}
^ permalink raw reply related
* [PATCH V2] i.MX28: Shut down the LCD controller to avoid BootROM sampling bug
From: Marek Vasut @ 2012-04-19 18:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1334767680-12688-1-git-send-email-marex@denx.de>
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>
---
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,
--
1.7.9.5
^ permalink raw reply related
* RE: [PATCH] blackfin: fix compile error in bfin-lq035q1-fb.c
From: Jingoo Han @ 2012-04-20 4:52 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1334803401-26780-1-git-send-email-paul.gortmaker@windriver.com>
> -----Original Message-----
> From: linux-fbdev-owner@vger.kernel.org [mailto:linux-fbdev-owner@vger.kernel.org] On Behalf Of Paul
> Gortmaker
> Sent: Thursday, April 19, 2012 11:43 AM
> To: linux-fbdev@vger.kernel.org
> Cc: uclinux-dist-devel@blackfin.uclinux.org; Paul Gortmaker; Florian Tobias Schandinat; Mike Frysinger
> Subject: [PATCH] blackfin: fix compile error in bfin-lq035q1-fb.c
>
> This file has an implicit dependency on GPIO stuff, showing
> up as the following build failure:
>
> drivers/video/bfin-lq035q1-fb.c:369:6: error: 'GPIOF_OUT_INIT_LOW' undeclared
>
> Other more global bfin build issues prevent an automated bisect, but
> it really doesn't matter - simply add in the appropriate header.
>
> Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
> Cc: Mike Frysinger <vapier@gentoo.org>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Acked-by: Jingoo Han <jg1.han@samsung.com>
It looks good.
This header file 'linux/gpio.h' is necessary for using 'GPIOF_OUT_INIT_LOW'.
>
> diff --git a/drivers/video/bfin-lq035q1-fb.c b/drivers/video/bfin-lq035q1-fb.c
> index 86922ac..353c02f 100644
> --- a/drivers/video/bfin-lq035q1-fb.c
> +++ b/drivers/video/bfin-lq035q1-fb.c
> @@ -13,6 +13,7 @@
> #include <linux/errno.h>
> #include <linux/string.h>
> #include <linux/fb.h>
> +#include <linux/gpio.h>
> #include <linux/slab.h>
> #include <linux/init.h>
> #include <linux/types.h>
> --
> 1.7.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox