* Re: [PATCH RESEND] MXSFB: Fix driver registration
From: Shawn Guo @ 2011-12-19 22:54 UTC (permalink / raw)
To: Marek Vasut
Cc: linux-kernel, linux-fbdev, FlorianSchandinat, Wolfgang Denk,
Stefano Babic, Huang Shijie, Axel Lin, Sascha Hauer
In-Reply-To: <1324309079-10739-1-git-send-email-marek.vasut@gmail.com>
On 19 December 2011 23:37, Marek Vasut <marek.vasut@gmail.com> wrote:
> The driver should be registered with mxsfb_driver, not with mxsfb_devtype.
> This caused obvious null pointer dereference and crash.
>
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Huang Shijie <b32955@freescale.com>
> Cc: Axel Lin <axel.lin@gmail.com>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Acked-by: Axel Lin <axel.lin@gmail.com>
> Acked-by: Sascha Hauer <s.hauer@pengutronix.de>
Acked-by: Shawn Guo <shawn.guo@linaro.org>
--
Regards,
Shawn
^ permalink raw reply
* [PATCH] s3fb: fix Virge/VX
From: Ondrej Zary @ 2011-12-19 22:39 UTC (permalink / raw)
To: Florian Tobias Schandinat
Cc: Paul Mundt, Ondrej Zajicek, linux-fbdev, Kernel development list
Add memory size detection for Virge/VX and small delay in mode setting (same
as in X.org driver) to fix blank screen problem.
Also adjust DTPC position to fix garbled screen in some modes (tested that
this adjustment does not break other cards - at least Trio32, Trio64V+,
Trio64V2/DX, Virge, Virge/DX).
Tested on ELSA Winner 2000AVI/3D.
Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
--- linux-3.1-orig/drivers/video/s3fb.c 2011-10-24 09:10:05.000000000 +0200
+++ linux-3.1/drivers/video/s3fb.c 2011-12-19 23:28:34.000000000 +0100
@@ -727,7 +727,7 @@
if (par->chip = CHIP_988_VIRGE_VX) {
vga_wcrt(par->state.vgabase, 0x50, 0x00);
vga_wcrt(par->state.vgabase, 0x67, 0x50);
-
+ msleep(10); /* screen remains blank sometimes without this */
vga_wcrt(par->state.vgabase, 0x63, (mode <= 2) ? 0x90 : 0x09);
vga_wcrt(par->state.vgabase, 0x66, 0x90);
}
@@ -901,7 +901,8 @@
/* Set Data Transfer Position */
hsstart = ((info->var.xres + info->var.right_margin) * hmul) / 8;
- value = clamp((htotal + hsstart + 1) / 2, hsstart + 4, htotal + 1);
+ /* + 2 is needed for Virge/VX, does no harm on other cards */
+ value = clamp((htotal + hsstart + 1) / 2 + 2, hsstart + 4, htotal + 1);
svga_wcrt_multi(par->state.vgabase, s3_dtpc_regs, value);
memset_io(info->screen_base, 0x00, screen_size);
@@ -1215,6 +1216,31 @@
info->screen_size = 2 << 20;
break;
}
+ } else if (par->chip = CHIP_988_VIRGE_VX) {
+ switch ((regval & 0x60) >> 5) {
+ case 0: /* 2MB */
+ info->screen_size = 2 << 20;
+ break;
+ case 1: /* 4MB */
+ info->screen_size = 4 << 20;
+ break;
+ case 2: /* 6MB */
+ info->screen_size = 6 << 20;
+ break;
+ case 3: /* 8MB */
+ info->screen_size = 8 << 20;
+ break;
+ }
+ /* off-screen memory */
+ regval = vga_rcrt(par->state.vgabase, 0x37);
+ switch ((regval & 0x60) >> 5) {
+ case 1: /* 4MB */
+ info->screen_size -= 4 << 20;
+ break;
+ case 2: /* 2MB */
+ info->screen_size -= 2 << 20;
+ break;
+ }
} else
info->screen_size = s3_memsizes[regval >> 5] << 10;
info->fix.smem_len = info->screen_size;
--
Ondrej Zary
^ permalink raw reply
* [PATCH 5/5] drivers/video: fsl-diu-fb: merge fsl_diu_alloc() into map_video_memory()
From: Timur Tabi @ 2011-12-19 22:26 UTC (permalink / raw)
To: linux-fbdev
Functions fsl_diu_alloc() and fsl_diu_free() were only being called by
map_video_memory() and unmap_video_memory(), respectively.
Signed-off-by: Timur Tabi <timur@freescale.com>
---
drivers/video/fsl-diu-fb.c | 47 ++++++++++---------------------------------
1 files changed, 11 insertions(+), 36 deletions(-)
diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index e2f4f32..acf292b 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -461,37 +461,6 @@ static enum fsl_diu_monitor_port fsl_diu_name_to_port(const char *s)
return diu_ops.valid_monitor_port(port);
}
-/**
- * fsl_diu_alloc - allocate memory for the DIU
- * @size: number of bytes to allocate
- * @param: returned physical address of memory
- *
- * This function allocates a physically-contiguous block of memory.
- */
-static void *fsl_diu_alloc(size_t size, phys_addr_t *phys)
-{
- void *virt;
-
- virt = alloc_pages_exact(size, GFP_DMA | __GFP_ZERO);
- if (virt)
- *phys = virt_to_phys(virt);
-
- return virt;
-}
-
-/**
- * fsl_diu_free - release DIU memory
- * @virt: pointer returned by fsl_diu_alloc()
- * @size: number of bytes allocated by fsl_diu_alloc()
- *
- * This function releases memory allocated by fsl_diu_alloc().
- */
-static void fsl_diu_free(void *virt, size_t size)
-{
- if (virt && size)
- free_pages_exact(virt, size);
-}
-
/*
* Workaround for failed writing desc register of planes.
* Needed with MPC5121 DIU rev 2.0 silicon.
@@ -875,16 +844,17 @@ static void update_lcdc(struct fb_info *info)
static int map_video_memory(struct fb_info *info)
{
- phys_addr_t phys;
u32 smem_len = info->fix.line_length * info->var.yres_virtual;
+ void *p;
- info->screen_base = fsl_diu_alloc(smem_len, &phys);
- if (info->screen_base = NULL) {
+ p = alloc_pages_exact(smem_len, GFP_DMA | __GFP_ZERO);
+ if (!p) {
dev_err(info->dev, "unable to allocate fb memory\n");
return -ENOMEM;
}
mutex_lock(&info->mm_lock);
- info->fix.smem_start = (unsigned long) phys;
+ info->screen_base = p;
+ info->fix.smem_start = virt_to_phys(info->screen_base);
info->fix.smem_len = smem_len;
mutex_unlock(&info->mm_lock);
info->screen_size = info->fix.smem_len;
@@ -894,12 +864,17 @@ static int map_video_memory(struct fb_info *info)
static void unmap_video_memory(struct fb_info *info)
{
- fsl_diu_free(info->screen_base, info->fix.smem_len);
+ void *p = info->screen_base;
+ size_t l = info->fix.smem_len;
+
mutex_lock(&info->mm_lock);
info->screen_base = NULL;
info->fix.smem_start = 0;
info->fix.smem_len = 0;
mutex_unlock(&info->mm_lock);
+
+ if (p)
+ free_pages_exact(p, l);
}
/*
--
1.7.3.4
^ permalink raw reply related
* [PATCH 4/5] drivers/video: fsl-diu-fb: add default platform ops functions
From: Timur Tabi @ 2011-12-19 22:26 UTC (permalink / raw)
To: linux-fbdev
The DIU driver requires some platform-specific functions to be defined,
but two them can be optional because most platforms implement them the
same way. Functions set_gamma_table() and get_pixel_format() are only
needed because of quirks in the Freescale MPC8610 HPCD reference board.
For other boards, a generic implementation works, so we shouldn't
require the platform code to define them.
Signed-off-by: Timur Tabi <timur@freescale.com>
---
drivers/video/fsl-diu-fb.c | 64 +++++++++++++++++++++++++++++++++++++++++--
1 files changed, 61 insertions(+), 3 deletions(-)
diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index d8d461d..e2f4f32 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -832,7 +832,8 @@ static void update_lcdc(struct fb_info *info)
for (j = 0; j <= 255; j++)
*gamma_table_base++ = j;
- diu_ops.set_gamma_table(data->monitor_port, data->gamma);
+ if (diu_ops.set_gamma_table)
+ diu_ops.set_gamma_table(data->monitor_port, data->gamma);
disable_lcdc(info);
@@ -917,6 +918,59 @@ static int fsl_diu_set_aoi(struct fb_info *info)
return 0;
}
+/**
+ * fsl_diu_get_pixel_format: return the pixel format for a given color depth
+ *
+ * The pixel format is a 32-bit value that determine which bits in each
+ * pixel are to be used for each color. This is the default function used
+ * if the platform does not define its own version.
+ */
+static u32 fsl_diu_get_pixel_format(unsigned int bits_per_pixel)
+{
+#define PF_BYTE_F 0x10000000
+#define PF_ALPHA_C_MASK 0x0E000000
+#define PF_ALPHA_C_SHIFT 25
+#define PF_BLUE_C_MASK 0x01800000
+#define PF_BLUE_C_SHIFT 23
+#define PF_GREEN_C_MASK 0x00600000
+#define PF_GREEN_C_SHIFT 21
+#define PF_RED_C_MASK 0x00180000
+#define PF_RED_C_SHIFT 19
+#define PF_PALETTE 0x00040000
+#define PF_PIXEL_S_MASK 0x00030000
+#define PF_PIXEL_S_SHIFT 16
+#define PF_COMP_3_MASK 0x0000F000
+#define PF_COMP_3_SHIFT 12
+#define PF_COMP_2_MASK 0x00000F00
+#define PF_COMP_2_SHIFT 8
+#define PF_COMP_1_MASK 0x000000F0
+#define PF_COMP_1_SHIFT 4
+#define PF_COMP_0_MASK 0x0000000F
+#define PF_COMP_0_SHIFT 0
+
+#define MAKE_PF(alpha, red, blue, green, size, c0, c1, c2, c3) \
+ cpu_to_le32(PF_BYTE_F | (alpha << PF_ALPHA_C_SHIFT) | \
+ (blue << PF_BLUE_C_SHIFT) | (green << PF_GREEN_C_SHIFT) | \
+ (red << PF_RED_C_SHIFT) | (c3 << PF_COMP_3_SHIFT) | \
+ (c2 << PF_COMP_2_SHIFT) | (c1 << PF_COMP_1_SHIFT) | \
+ (c0 << PF_COMP_0_SHIFT) | (size << PF_PIXEL_S_SHIFT))
+
+ switch (bits_per_pixel) {
+ case 32:
+ /* 0x88883316 */
+ return MAKE_PF(3, 2, 0, 1, 3, 8, 8, 8, 8);
+ case 24:
+ /* 0x88082219 */
+ return MAKE_PF(4, 0, 1, 2, 2, 0, 8, 8, 8);
+ case 16:
+ /* 0x65053118 */
+ return MAKE_PF(4, 2, 1, 0, 1, 5, 6, 5, 0);
+ default:
+ pr_err("fsl-diu: unsupported color depth %u\n", bits_per_pixel);
+ return 0;
+ }
+}
+
/*
* Using the fb_var_screeninfo in fb_info we set the resolution of this
* particular framebuffer. This function alters the fb_fix_screeninfo stored
@@ -952,8 +1006,12 @@ static int fsl_diu_set_par(struct fb_info *info)
}
}
- ad->pix_fmt = diu_ops.get_pixel_format(data->monitor_port,
- var->bits_per_pixel);
+ if (diu_ops.get_pixel_format)
+ ad->pix_fmt = diu_ops.get_pixel_format(data->monitor_port,
+ var->bits_per_pixel);
+ else
+ ad->pix_fmt = fsl_diu_get_pixel_format(var->bits_per_pixel);
+
ad->addr = cpu_to_le32(info->fix.smem_start);
ad->src_size_g_alpha = cpu_to_le32((var->yres_virtual << 12) |
var->xres_virtual) | mfbi->g_alpha;
--
1.7.3.4
^ permalink raw reply related
* [PATCH 3/5] drivers/video: fsl-diu-fb: remove broken reference count enabling the display
From: Timur Tabi @ 2011-12-19 22:26 UTC (permalink / raw)
To: linux-fbdev
The functions enable_lcdc() and disable_lcdc() used the variable fb_enable
to keep a reference count of when the display is enabled, but the code is
broken. The display is always disabled when disable_lcdc(), and it is
always enabled when enable_lcdc() is called, regardless of the value of
fb_enable. Obviously, we don't need to keep a reference count, so just
remove the variable.
Signed-off-by: Timur Tabi <timur@freescale.com>
---
drivers/video/fsl-diu-fb.c | 12 ++----------
1 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 231752e..d8d461d 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -352,7 +352,6 @@ struct mfb_info {
* @fsl_diu_info: fb_info objects, one per AOI
* @dev_attr: sysfs structure
* @irq: IRQ
- * @fb_enabled: TRUE if the DIU is enabled, FALSE if not
* @monitor_port: the monitor port this DIU is connected to
* @diu_reg: pointer to the DIU hardware registers
* @reg_lock: spinlock for register access
@@ -371,7 +370,6 @@ struct fsl_diu_data {
struct mfb_info mfb[NUM_AOIS];
struct device_attribute dev_attr;
unsigned int irq;
- int fb_enabled;
enum fsl_diu_monitor_port monitor_port;
struct diu __iomem *diu_reg;
spinlock_t reg_lock;
@@ -619,10 +617,7 @@ static void enable_lcdc(struct fb_info *info)
struct fsl_diu_data *data = mfbi->parent;
struct diu __iomem *hw = data->diu_reg;
- if (!data->fb_enabled) {
- out_be32(&hw->diu_mode, MFB_MODE1);
- data->fb_enabled++;
- }
+ out_be32(&hw->diu_mode, MFB_MODE1);
}
static void disable_lcdc(struct fb_info *info)
@@ -631,10 +626,7 @@ static void disable_lcdc(struct fb_info *info)
struct fsl_diu_data *data = mfbi->parent;
struct diu __iomem *hw = data->diu_reg;
- if (data->fb_enabled) {
- out_be32(&hw->diu_mode, 0);
- data->fb_enabled = 0;
- }
+ out_be32(&hw->diu_mode, 0);
}
static void adjust_aoi_size_position(struct fb_var_screeninfo *var,
--
1.7.3.4
^ permalink raw reply related
* [PATCH 2/5] drivers/video: fsl-diu-fb: set correct framebuffer flags
From: Timur Tabi @ 2011-12-19 22:26 UTC (permalink / raw)
To: linux-fbdev
The DIU uses system RAM for the framebuffer, so FBINFO_VIRTFB should be set.
Since the framebuffer is in system RAM, it can be read from more quickly
than written to, so FBINFO_READS_FAST should be set.
We can also set FBINFO_PARTIAL_PAN_OK for the same reasons.
Signed-off-by: Timur Tabi <timur@freescale.com>
---
drivers/video/fsl-diu-fb.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 411a7b3..231752e 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -1231,7 +1231,8 @@ static int __devinit install_fb(struct fb_info *info)
info->var.activate = FB_ACTIVATE_NOW;
info->fbops = &fsl_diu_ops;
- info->flags = FBINFO_DEFAULT;
+ info->flags = FBINFO_DEFAULT | FBINFO_VIRTFB | FBINFO_PARTIAL_PAN_OK |
+ FBINFO_READS_FAST;
info->pseudo_palette = mfbi->pseudo_palette;
rc = fb_alloc_cmap(&info->cmap, 16, 0);
--
1.7.3.4
^ permalink raw reply related
* [PATCH 1/5] drivers/video: fsl-diu-fb: merge init_fbinfo() into install_fb()
From: Timur Tabi @ 2011-12-19 22:26 UTC (permalink / raw)
To: linux-fbdev
Function init_fbinfo() is called only from install_fb(), and it's only a few
lines long. Plus, it ignores the return code from fb_alloc_cmap(). Merge
its contents into install_fb() and handle errors properly.
Signed-off-by: Timur Tabi <timur@freescale.com>
---
drivers/video/fsl-diu-fb.c | 25 ++++++++-----------------
1 files changed, 8 insertions(+), 17 deletions(-)
diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 408272c..411a7b3 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -1220,21 +1220,6 @@ static struct fb_ops fsl_diu_ops = {
.fb_release = fsl_diu_release,
};
-static int init_fbinfo(struct fb_info *info)
-{
- struct mfb_info *mfbi = info->par;
-
- info->device = NULL;
- info->var.activate = FB_ACTIVATE_NOW;
- info->fbops = &fsl_diu_ops;
- info->flags = FBINFO_FLAG_DEFAULT;
- info->pseudo_palette = &mfbi->pseudo_palette;
-
- /* Allocate colormap */
- fb_alloc_cmap(&info->cmap, 16, 0);
- return 0;
-}
-
static int __devinit install_fb(struct fb_info *info)
{
int rc;
@@ -1244,8 +1229,14 @@ static int __devinit install_fb(struct fb_info *info)
unsigned int dbsize = ARRAY_SIZE(fsl_diu_mode_db);
int has_default_mode = 1;
- if (init_fbinfo(info))
- return -EINVAL;
+ info->var.activate = FB_ACTIVATE_NOW;
+ info->fbops = &fsl_diu_ops;
+ info->flags = FBINFO_DEFAULT;
+ info->pseudo_palette = mfbi->pseudo_palette;
+
+ rc = fb_alloc_cmap(&info->cmap, 16, 0);
+ if (rc)
+ return rc;
if (mfbi->index = PLANE0) {
if (mfbi->edid_data) {
--
1.7.3.4
^ permalink raw reply related
* Re: [GIT PULL] ARM: amba: Enable module alias autogeneration for
From: Russell King - ARM Linux @ 2011-12-19 21:39 UTC (permalink / raw)
To: Dave Martin
Cc: Alessandro Rubini, patches, linux-kernel, linux-arm-kernel, alan,
a.zummo, alsa-devel, cjb, dan.j.williams, dmitry.torokhov,
grant.likely, perex, jassisinghbrar, julia, linus.walleij,
linux-fbdev, linux-input, linux-mmc, linux-serial, linux-watchdog,
lethal, Pawel.Moll, rtc-linux, spi-devel-general, tiwai,
vinod.koul, wim
In-Reply-To: <20111219132409.GB2031@linaro.org>
On Mon, Dec 19, 2011 at 01:24:09PM +0000, Dave Martin wrote:
> Hi Russell,
>
> This one isn't urgent, but I'm not seeing the amba modalias patches
> anywhere yet. Did you have any outstanding concerns which need to be
> resolved?
>
> If you can suggest when/if these are likely to merge that would be great.
>
> Since Alessandro is now expecting to have to rebase on top of the amba
> additions anyway, we shouldn't need to worry about conflicting with his
> patches.
>
> Of course, if you've already merged these somewhere, then there's no
> problem.
I have pulled it, I just haven't merged it in anywhere yet.
^ permalink raw reply
* Re: Should I use FBINFO_VIRTFB?
From: Timur Tabi @ 2011-12-19 16:08 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <4EEA5DA7.3010406@freescale.com>
Geert Uytterhoeven wrote:
>> > I'm a little confused about panning support in my driver (which I didn't write -- I'm just cleaning it up). There is .fb_pan_display function, but it only gets called early in the boot process, and only with xoffset=0 and yoffset=0. After that, it never seems to get called again. Under what circumstances is panning really used?
> Is yres_virtual larger than yres? Is the font size a multiple of ypanstep?
> If not, panning is disabled.
I can't seem to get partial panning to "activate", no matter what I do. I set 'flags' to FBINFO_DEFAULT | FBINFO_VIRTFB | FBINFO_PARTIAL_PAN_OK | FBINFO_READS_FAST. I have a 1280x1024 monitor. I set the virtual_size to 1280,1280 via sysfs. Then I set panning to "0,100", and I see the screen scroll. I tried sending a bunch of output to /dev/tty1, but no matter what I do, none of the SCROLL_PAN_REDRAW or SCROLL_PAN_MOVE case statements in fbcon_scroll() get executed.
At this point, should I just FBINFO_VIRTFB | FBINFO_PARTIAL_PAN_OK | FBINFO_READS_FAST and not worry about whether it really makes a difference?
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* [PATCH RESEND] MXSFB: Fix driver registration
From: Marek Vasut @ 2011-12-19 15:37 UTC (permalink / raw)
To: linux-kernel
Cc: linux-fbdev, FlorianSchandinat, Marek Vasut, Wolfgang Denk,
Stefano Babic, Shawn Guo, Huang Shijie, Axel Lin, Sascha Hauer
The driver should be registered with mxsfb_driver, not with mxsfb_devtype.
This caused obvious null pointer dereference and crash.
Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Huang Shijie <b32955@freescale.com>
Cc: Axel Lin <axel.lin@gmail.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Acked-by: Axel Lin <axel.lin@gmail.com>
Acked-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/video/mxsfb.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
index 18742c2..d29c7c0 100644
--- a/drivers/video/mxsfb.c
+++ b/drivers/video/mxsfb.c
@@ -902,7 +902,7 @@ static struct platform_driver mxsfb_driver = {
},
};
-module_platform_driver(mxsfb_devtype);
+module_platform_driver(mxsfb_driver);
MODULE_DESCRIPTION("Freescale mxs framebuffer driver");
MODULE_AUTHOR("Sascha Hauer, Pengutronix");
--
1.7.6.4
^ permalink raw reply related
* Re: [PATCH] MXSFB: Fix driver registration
From: Florian Tobias Schandinat @ 2011-12-19 15:18 UTC (permalink / raw)
To: Shawn Guo
Cc: Marek Vasut, linux-kernel, Wolfgang Denk, Stefano Babic,
Huang Shijie, Axel Lin, Sascha Hauer, linux-fbdev
In-Reply-To: <CAAQ0ZWSJ0Yzr2Gk9MBQ0nvVCWxr0yWF_RFTy3isqc0pgEDdRxw@mail.gmail.com>
Hi Marek,
can you please resend the patch to me and the fbdev mailing list, preferably
with all Ack's you received until now included.
On 12/19/2011 01:51 PM, Shawn Guo wrote:
> Cc-ed Florian and linux-fbdev@vger.kernel.org, as I guess Florian is
> the person who will take this patch.
Thanks, I will. I guess you wanted to Ack this patch?
Thanks,
Florian Tobias Schandinat
>
> -- Regards,Shawn
> On 18 December 2011 08:58, Marek Vasut <marek.vasut@gmail.com> wrote:
>> The driver should be registered with mxsfb_driver, not with mxsfb_devtype.
>> This caused obvious null pointer dereference and crash.
>>
>> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
>> Cc: Wolfgang Denk <wd@denx.de>
>> Cc: Stefano Babic <sbabic@denx.de>
>> Cc: Shawn Guo <shawn.guo@linaro.org>
>> Cc: Huang Shijie <b32955@freescale.com>
>> Cc: Axel Lin <axel.lin@gmail.com>
>> Cc: Sascha Hauer <s.hauer@pengutronix.de>
>> ---
>> drivers/video/mxsfb.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
>> index 18742c2..d29c7c0 100644
>> --- a/drivers/video/mxsfb.c
>> +++ b/drivers/video/mxsfb.c
>> @@ -902,7 +902,7 @@ static struct platform_driver mxsfb_driver = {
>> },
>> };
>>
>> -module_platform_driver(mxsfb_devtype);
>> +module_platform_driver(mxsfb_driver);
>>
>> MODULE_DESCRIPTION("Freescale mxs framebuffer driver");
>> MODULE_AUTHOR("Sascha Hauer, Pengutronix");
>> --
>> 1.7.6.4
>>
>
^ permalink raw reply
* Re: [PATCH] MXSFB: Fix driver registration
From: Shawn Guo @ 2011-12-19 13:51 UTC (permalink / raw)
To: Marek Vasut, Florian Tobias Schandinat
Cc: linux-kernel, Wolfgang Denk, Stefano Babic, Huang Shijie,
Axel Lin, Sascha Hauer, linux-fbdev
In-Reply-To: <1324169888-6687-1-git-send-email-marek.vasut@gmail.com>
Cc-ed Florian and linux-fbdev@vger.kernel.org, as I guess Florian is
the person who will take this patch.
-- Regards,Shawn
On 18 December 2011 08:58, Marek Vasut <marek.vasut@gmail.com> wrote:
> The driver should be registered with mxsfb_driver, not with mxsfb_devtype.
> This caused obvious null pointer dereference and crash.
>
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Huang Shijie <b32955@freescale.com>
> Cc: Axel Lin <axel.lin@gmail.com>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> drivers/video/mxsfb.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
> index 18742c2..d29c7c0 100644
> --- a/drivers/video/mxsfb.c
> +++ b/drivers/video/mxsfb.c
> @@ -902,7 +902,7 @@ static struct platform_driver mxsfb_driver = {
> },
> };
>
> -module_platform_driver(mxsfb_devtype);
> +module_platform_driver(mxsfb_driver);
>
> MODULE_DESCRIPTION("Freescale mxs framebuffer driver");
> MODULE_AUTHOR("Sascha Hauer, Pengutronix");
> --
> 1.7.6.4
>
^ permalink raw reply
* Re: [GIT PULL] ARM: amba: Enable module alias autogeneration for
From: Dave Martin @ 2011-12-19 13:24 UTC (permalink / raw)
To: Alessandro Rubini
Cc: linux, patches, linux-kernel, linux-arm-kernel, alan, a.zummo,
alsa-devel, cjb, dan.j.williams, dmitry.torokhov, grant.likely,
perex, jassisinghbrar, julia, linus.walleij, linux-fbdev,
linux-input, linux-mmc, linux-serial, linux-watchdog, lethal,
Pawel.Moll, rtc-linux, spi-devel-general, tiwai, vinod.koul, wim
In-Reply-To: <20111206161815.GD13769@linaro.org>
Hi Russell,
This one isn't urgent, but I'm not seeing the amba modalias patches
anywhere yet. Did you have any outstanding concerns which need to be
resolved?
If you can suggest when/if these are likely to merge that would be great.
Since Alessandro is now expecting to have to rebase on top of the amba
additions anyway, we shouldn't need to worry about conflicting with his
patches.
Of course, if you've already merged these somewhere, then there's no
problem.
Cheers
---Dave
^ permalink raw reply
* [PATCH V2 2/2] OMAPDSS: DISPC: Update Scaling Clock Logic
From: Chandrabhanu Mahapatra @ 2011-12-19 8:45 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
Clock requirements for scaling in OMAP2, OMAP3 and OMAP4 are different. In
OMAP2 and OMAP3 the required clock rate is a function of pixel clock, vertical
downscale ratio and horizontal downscale ratio whereas in OMAP4 it is a
function of pixel clock and horizontal downscale ratio only. Selection of 3-tap
vs 5-tap coefficients depends on clock rate line buffer width in OMAP3 whereas
in OMAP4 it is independent of clock rate and line buffer width. In OMAP2 3-tap
for vertical and 5-tap for horizontal scaling is used. In OMAP4 5-tap is used
both for horizontal and vertical scaling for better performance. Also, the
number and width of line buffers differs in OMAP3 and OMAP4.
So, clock functions have been fined tuned for OMAP3 and support has been added
added for OMAP4. This code has been tested on OMAP2, OMAP3 and OMAP4, and
scaling issues due to clock errors have been resolved.
Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
drivers/video/omap2/dss/dispc.c | 66 ++++++++++++++++++++++----------
drivers/video/omap2/dss/dss_features.c | 7 +++
drivers/video/omap2/dss/dss_features.h | 1 +
3 files changed, 54 insertions(+), 20 deletions(-)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index b981983..6c3feb4 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -1554,6 +1554,9 @@ static unsigned long calc_fclk_five_taps(enum omap_channel channel, u16 width,
u32 fclk = 0;
u64 tmp, pclk = dispc_mgr_pclk_rate(channel);
+ if (height <= out_height && width <= out_width)
+ return (unsigned long) pclk;
+
if (height > out_height) {
struct omap_dss_device *dssdev = dispc_mgr_get_device(channel);
unsigned int ppl = dssdev->panel.timings.x_res;
@@ -1608,7 +1611,16 @@ static unsigned long calc_fclk(enum omap_channel channel, u16 width,
else
vf = 1;
- return dispc_mgr_pclk_rate(channel) * vf * hf;
+ if (cpu_is_omap24xx()) {
+ if (vf > 1 && hf > 1)
+ return dispc_mgr_pclk_rate(channel) * 4;
+ else
+ return dispc_mgr_pclk_rate(channel) * 2;
+ } else if (cpu_is_omap34xx()) {
+ return dispc_mgr_pclk_rate(channel) * vf * hf;
+ } else {
+ return dispc_mgr_pclk_rate(channel) * hf;
+ }
}
static int dispc_ovl_calc_scaling(enum omap_plane plane,
@@ -1618,6 +1630,8 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
{
struct omap_overlay *ovl = omap_dss_get_overlay(plane);
const int maxdownscale = dss_feat_get_param_max(FEAT_PARAM_DOWNSCALE);
+ const int maxsinglelinewidth + dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
unsigned long fclk = 0;
if ((ovl->caps & OMAP_DSS_OVL_CAP_SCALE) = 0) {
@@ -1635,28 +1649,40 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
out_height > height * 8)
return -EINVAL;
- /* Must use 5-tap filter? */
- *five_taps = height > out_height * 2;
-
- if (!*five_taps) {
+ if (cpu_is_omap24xx()) {
+ if (width > maxsinglelinewidth)
+ DSSERR("Cannot scale max input width exceeded");
+ *five_taps = false;
+ fclk = calc_fclk(channel, width, height, out_width,
+ out_height);
+ } else if (cpu_is_omap34xx()) {
+ if (width > (maxsinglelinewidth * 2)) {
+ DSSERR("Cannot setup scaling");
+ DSSERR("width exceeds maximum width possible");
+ return -EINVAL;
+ }
+ fclk = calc_fclk_five_taps(channel, width, height, out_width,
+ out_height, color_mode);
+ if (width > maxsinglelinewidth) {
+ if (height > out_height && height < out_height * 2)
+ *five_taps = false;
+ else {
+ DSSERR("cannot setup scaling with five taps");
+ return -EINVAL;
+ }
+ }
+ if (!*five_taps)
+ fclk = calc_fclk(channel, width, height, out_width,
+ out_height);
+ } else {
+ if (width > maxsinglelinewidth) {
+ DSSERR("Cannot scale width exceeds max line width");
+ return -EINVAL;
+ }
fclk = calc_fclk(channel, width, height, out_width,
out_height);
-
- /* Try 5-tap filter if 3-tap fclk is too high */
- if (cpu_is_omap34xx() && height > out_height &&
- fclk > dispc_fclk_rate())
- *five_taps = true;
}
- if (width > (2048 >> *five_taps)) {
- DSSERR("failed to set up scaling, fclk too low\n");
- return -EINVAL;
- }
-
- if (*five_taps)
- fclk = calc_fclk_five_taps(channel, width, height,
- out_width, out_height, color_mode);
-
DSSDBG("required fclk rate = %lu Hz\n", fclk);
DSSDBG("current fclk rate = %lu Hz\n", dispc_fclk_rate());
@@ -1676,7 +1702,7 @@ int dispc_ovl_setup(enum omap_plane plane, struct omap_overlay_info *oi,
u32 fifo_low, u32 fifo_high)
{
struct omap_overlay *ovl = omap_dss_get_overlay(plane);
- bool five_taps = false;
+ bool five_taps = true;
bool fieldmode = 0;
int r, cconv = 0;
unsigned offset0, offset1;
diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c
index b402699..5e4b829 100644
--- a/drivers/video/omap2/dss/dss_features.c
+++ b/drivers/video/omap2/dss/dss_features.c
@@ -304,6 +304,11 @@ static const struct dss_param_range omap2_dss_param_range[] = {
[FEAT_PARAM_DSIPLL_FINT] = { 0, 0 },
[FEAT_PARAM_DSIPLL_LPDIV] = { 0, 0 },
[FEAT_PARAM_DOWNSCALE] = { 1, 2 },
+ /*
+ * Assuming the line width buffer to be 768 pixels as OMAP2 DISPC
+ * scaler cannot scale a image with width more than 768.
+ */
+ [FEAT_PARAM_LINEWIDTH] = { 1, 768 },
};
static const struct dss_param_range omap3_dss_param_range[] = {
@@ -316,6 +321,7 @@ static const struct dss_param_range omap3_dss_param_range[] = {
[FEAT_PARAM_DSIPLL_FINT] = { 750000, 2100000 },
[FEAT_PARAM_DSIPLL_LPDIV] = { 1, (1 << 13) - 1},
[FEAT_PARAM_DOWNSCALE] = { 1, 4 },
+ [FEAT_PARAM_LINEWIDTH] = { 1, 1024 },
};
static const struct dss_param_range omap4_dss_param_range[] = {
@@ -328,6 +334,7 @@ static const struct dss_param_range omap4_dss_param_range[] = {
[FEAT_PARAM_DSIPLL_FINT] = { 500000, 2500000 },
[FEAT_PARAM_DSIPLL_LPDIV] = { 0, (1 << 13) - 1 },
[FEAT_PARAM_DOWNSCALE] = { 1, 4 },
+ [FEAT_PARAM_LINEWIDTH] = { 1, 2048 },
};
/* OMAP2 DSS Features */
diff --git a/drivers/video/omap2/dss/dss_features.h b/drivers/video/omap2/dss/dss_features.h
index 6a6c05d..cd833bb 100644
--- a/drivers/video/omap2/dss/dss_features.h
+++ b/drivers/video/omap2/dss/dss_features.h
@@ -86,6 +86,7 @@ enum dss_range_param {
FEAT_PARAM_DSIPLL_FINT,
FEAT_PARAM_DSIPLL_LPDIV,
FEAT_PARAM_DOWNSCALE,
+ FEAT_PARAM_LINEWIDTH,
};
/* DSS Feature Functions */
--
1.7.1
^ permalink raw reply related
* [PATCH V2 1/2] OMAPDSS: DISPC: Update Fir Coefficients
From: Chandrabhanu Mahapatra @ 2011-12-19 8:45 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
The FIR coefficients present in kernel are being updated to new coefficients
consisting of 24 coefficient tables, with 12 each for 3 tap and 5 tap scenario,
which are chosen on the basis of DISPC up/downsampling filters M value. M is
the inverse of low pass cut off frequency of the sampling filter. For vertical
scaling 3 tap or 5 tap tables are used based on the clock rate and width of
the line buffer whereas in OMAP2 3 tap is always used. For horizontal scaling
however 5 tap tables are always used.
New coefficients and the corresponding logic have been tested on OMAP2, OMAP3
and OMAP4. Horizontal and vertical scaling worked fine except for some 3 tap
vs 5 tap issue during vertical upscaling and clock failing issues which is
acknowledged in the next patch. Vertical upscaling was found to perform better
under 5 taps. The 24 coefficient tables have been moved to another file
dispc_coefs.c for proper maintainance.
This code is written based on code written by Lajos Molnar <lajos@ti.com> in
Android Kernel for scaling. Lajos Molnar <lajos@ti.com> had fine tuned the FIR
coefficient selection process and reduced outliness and blockiness around
images when upscaling more than 2 times.
Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
drivers/video/omap2/dss/Makefile | 2 +-
drivers/video/omap2/dss/dispc.c | 135 ++------------
drivers/video/omap2/dss/dispc.h | 11 +
drivers/video/omap2/dss/dispc_coefs.c | 326 +++++++++++++++++++++++++++++++++
4 files changed, 356 insertions(+), 118 deletions(-)
create mode 100644 drivers/video/omap2/dss/dispc_coefs.c
diff --git a/drivers/video/omap2/dss/Makefile b/drivers/video/omap2/dss/Makefile
index bd34ac5..f10d56f 100644
--- a/drivers/video/omap2/dss/Makefile
+++ b/drivers/video/omap2/dss/Makefile
@@ -1,5 +1,5 @@
obj-$(CONFIG_OMAP2_DSS) += omapdss.o
-omapdss-y := core.o dss.o dss_features.o dispc.o display.o manager.o overlay.o
+omapdss-y := core.o dss.o dss_features.o dispc.o dispc_coefs.o display.o manager.o overlay.o
omapdss-$(CONFIG_OMAP2_DSS_DPI) += dpi.o
omapdss-$(CONFIG_OMAP2_DSS_RFBI) += rfbi.o
omapdss-$(CONFIG_OMAP2_DSS_VENC) += venc.o
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 6892cfd..b981983 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -63,22 +63,6 @@ struct omap_dispc_isr_data {
u32 mask;
};
-struct dispc_h_coef {
- s8 hc4;
- s8 hc3;
- u8 hc2;
- s8 hc1;
- s8 hc0;
-};
-
-struct dispc_v_coef {
- s8 vc22;
- s8 vc2;
- u8 vc1;
- s8 vc0;
- s8 vc00;
-};
-
enum omap_burst_size {
BURST_SIZE_X2 = 0,
BURST_SIZE_X4 = 1,
@@ -532,105 +516,27 @@ static void dispc_ovl_write_firv2_reg(enum omap_plane plane, int reg, u32 value)
dispc_write_reg(DISPC_OVL_FIR_COEF_V2(plane, reg), value);
}
-static void dispc_ovl_set_scale_coef(enum omap_plane plane, int hscaleup,
- int vscaleup, int five_taps,
- enum omap_color_component color_comp)
-{
- /* Coefficients for horizontal up-sampling */
- static const struct dispc_h_coef coef_hup[8] = {
- { 0, 0, 128, 0, 0 },
- { -1, 13, 124, -8, 0 },
- { -2, 30, 112, -11, -1 },
- { -5, 51, 95, -11, -2 },
- { 0, -9, 73, 73, -9 },
- { -2, -11, 95, 51, -5 },
- { -1, -11, 112, 30, -2 },
- { 0, -8, 124, 13, -1 },
- };
-
- /* Coefficients for vertical up-sampling */
- static const struct dispc_v_coef coef_vup_3tap[8] = {
- { 0, 0, 128, 0, 0 },
- { 0, 3, 123, 2, 0 },
- { 0, 12, 111, 5, 0 },
- { 0, 32, 89, 7, 0 },
- { 0, 0, 64, 64, 0 },
- { 0, 7, 89, 32, 0 },
- { 0, 5, 111, 12, 0 },
- { 0, 2, 123, 3, 0 },
- };
-
- static const struct dispc_v_coef coef_vup_5tap[8] = {
- { 0, 0, 128, 0, 0 },
- { -1, 13, 124, -8, 0 },
- { -2, 30, 112, -11, -1 },
- { -5, 51, 95, -11, -2 },
- { 0, -9, 73, 73, -9 },
- { -2, -11, 95, 51, -5 },
- { -1, -11, 112, 30, -2 },
- { 0, -8, 124, 13, -1 },
- };
-
- /* Coefficients for horizontal down-sampling */
- static const struct dispc_h_coef coef_hdown[8] = {
- { 0, 36, 56, 36, 0 },
- { 4, 40, 55, 31, -2 },
- { 8, 44, 54, 27, -5 },
- { 12, 48, 53, 22, -7 },
- { -9, 17, 52, 51, 17 },
- { -7, 22, 53, 48, 12 },
- { -5, 27, 54, 44, 8 },
- { -2, 31, 55, 40, 4 },
- };
-
- /* Coefficients for vertical down-sampling */
- static const struct dispc_v_coef coef_vdown_3tap[8] = {
- { 0, 36, 56, 36, 0 },
- { 0, 40, 57, 31, 0 },
- { 0, 45, 56, 27, 0 },
- { 0, 50, 55, 23, 0 },
- { 0, 18, 55, 55, 0 },
- { 0, 23, 55, 50, 0 },
- { 0, 27, 56, 45, 0 },
- { 0, 31, 57, 40, 0 },
- };
-
- static const struct dispc_v_coef coef_vdown_5tap[8] = {
- { 0, 36, 56, 36, 0 },
- { 4, 40, 55, 31, -2 },
- { 8, 44, 54, 27, -5 },
- { 12, 48, 53, 22, -7 },
- { -9, 17, 52, 51, 17 },
- { -7, 22, 53, 48, 12 },
- { -5, 27, 54, 44, 8 },
- { -2, 31, 55, 40, 4 },
- };
-
- const struct dispc_h_coef *h_coef;
- const struct dispc_v_coef *v_coef;
+static void dispc_ovl_set_scale_coef(enum omap_plane plane, int fir_hinc,
+ int fir_vinc, int five_taps,
+ enum omap_color_component color_comp)
+{
+ const struct dispc_coef *h_coef, *v_coef;
int i;
- if (hscaleup)
- h_coef = coef_hup;
- else
- h_coef = coef_hdown;
-
- if (vscaleup)
- v_coef = five_taps ? coef_vup_5tap : coef_vup_3tap;
- else
- v_coef = five_taps ? coef_vdown_5tap : coef_vdown_3tap;
+ h_coef = dispc_ovl_get_scale_coef(fir_hinc, true);
+ v_coef = dispc_ovl_get_scale_coef(fir_vinc, five_taps);
for (i = 0; i < 8; i++) {
u32 h, hv;
- h = FLD_VAL(h_coef[i].hc0, 7, 0)
- | FLD_VAL(h_coef[i].hc1, 15, 8)
- | FLD_VAL(h_coef[i].hc2, 23, 16)
- | FLD_VAL(h_coef[i].hc3, 31, 24);
- hv = FLD_VAL(h_coef[i].hc4, 7, 0)
- | FLD_VAL(v_coef[i].vc0, 15, 8)
- | FLD_VAL(v_coef[i].vc1, 23, 16)
- | FLD_VAL(v_coef[i].vc2, 31, 24);
+ h = FLD_VAL(h_coef[i].hc0_vc00, 7, 0)
+ | FLD_VAL(h_coef[i].hc1_vc0, 15, 8)
+ | FLD_VAL(h_coef[i].hc2_vc1, 23, 16)
+ | FLD_VAL(h_coef[i].hc3_vc2, 31, 24);
+ hv = FLD_VAL(h_coef[i].hc4_vc22, 7, 0)
+ | FLD_VAL(v_coef[i].hc1_vc0, 15, 8)
+ | FLD_VAL(v_coef[i].hc2_vc1, 23, 16)
+ | FLD_VAL(v_coef[i].hc3_vc2, 31, 24);
if (color_comp = DISPC_COLOR_COMPONENT_RGB_Y) {
dispc_ovl_write_firh_reg(plane, i, h);
@@ -645,8 +551,8 @@ static void dispc_ovl_set_scale_coef(enum omap_plane plane, int hscaleup,
if (five_taps) {
for (i = 0; i < 8; i++) {
u32 v;
- v = FLD_VAL(v_coef[i].vc00, 7, 0)
- | FLD_VAL(v_coef[i].vc22, 15, 8);
+ v = FLD_VAL(v_coef[i].hc0_vc00, 7, 0)
+ | FLD_VAL(v_coef[i].hc4_vc22, 15, 8);
if (color_comp = DISPC_COLOR_COMPONENT_RGB_Y)
dispc_ovl_write_firv_reg(plane, i, v);
else
@@ -1168,17 +1074,12 @@ static void dispc_ovl_set_scale_param(enum omap_plane plane,
enum omap_color_component color_comp)
{
int fir_hinc, fir_vinc;
- int hscaleup, vscaleup;
-
- hscaleup = orig_width <= out_width;
- vscaleup = orig_height <= out_height;
-
- dispc_ovl_set_scale_coef(plane, hscaleup, vscaleup, five_taps,
- color_comp);
fir_hinc = 1024 * orig_width / out_width;
fir_vinc = 1024 * orig_height / out_height;
+ dispc_ovl_set_scale_coef(plane, fir_hinc, fir_vinc, five_taps,
+ color_comp);
dispc_ovl_set_fir(plane, fir_hinc, fir_vinc, color_comp);
}
diff --git a/drivers/video/omap2/dss/dispc.h b/drivers/video/omap2/dss/dispc.h
index c06efc3..5836bd1 100644
--- a/drivers/video/omap2/dss/dispc.h
+++ b/drivers/video/omap2/dss/dispc.h
@@ -97,6 +97,17 @@
#define DISPC_OVL_PRELOAD(n) (DISPC_OVL_BASE(n) + \
DISPC_PRELOAD_OFFSET(n))
+/* DISPC up/downsampling FIR filter coefficient structure */
+struct dispc_coef {
+ s8 hc4_vc22;
+ s8 hc3_vc2;
+ u8 hc2_vc1;
+ s8 hc1_vc0;
+ s8 hc0_vc00;
+};
+
+const struct dispc_coef *dispc_ovl_get_scale_coef(int inc, int five_taps);
+
/* DISPC manager/channel specific registers */
static inline u16 DISPC_DEFAULT_COLOR(enum omap_channel channel)
{
diff --git a/drivers/video/omap2/dss/dispc_coefs.c b/drivers/video/omap2/dss/dispc_coefs.c
new file mode 100644
index 0000000..069bccb
--- /dev/null
+++ b/drivers/video/omap2/dss/dispc_coefs.c
@@ -0,0 +1,326 @@
+/*
+ * linux/drivers/video/omap2/dss/dispc_coefs.c
+ *
+ * Copyright (C) 2011 Texas Instruments
+ * Author: Chandrabhanu Mahapatra <cmahapatra@ti.com>
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/kernel.h>
+#include <video/omapdss.h>
+#include "dispc.h"
+
+#define ARRAY_LEN(array) (sizeof(array) / sizeof(array[0]))
+
+static const struct dispc_coef coef3_M8[8] = {
+ { 0, 0, 128, 0, 0 },
+ { 0, -4, 123, 9, 0 },
+ { 0, -4, 108, 87, 0 },
+ { 0, -2, 87, 43, 0 },
+ { 0, 64, 64, 0, 0 },
+ { 0, 43, 87, -2, 0 },
+ { 0, 24, 108, -4, 0 },
+ { 0, 9, 123, -4, 0 },
+};
+
+static const struct dispc_coef coef3_M9[8] = {
+ { 0, 6, 116, 6, 0 },
+ { 0, 0, 112, 16, 0 },
+ { 0, -2, 100, 30, 0 },
+ { 0, -2, 83, 47, 0 },
+ { 0, 64, 64, 0, 0 },
+ { 0, 47, 83, -2, 0 },
+ { 0, 30, 100, -2, 0 },
+ { 0, 16, 112, 0, 0 },
+};
+
+static const struct dispc_coef coef3_M10[8] = {
+ { 0, 10, 108, 10, 0 },
+ { 0, 3, 104, 21, 0 },
+ { 0, 0, 94, 34, 0 },
+ { 0, -1, 80, 49, 0 },
+ { 0, 64, 64, 0, 0 },
+ { 0, 49, 80, -1, 0 },
+ { 0, 34, 94, 0, 0 },
+ { 0, 21, 104, 3, 0 },
+};
+
+static const struct dispc_coef coef3_M11[8] = {
+ { 0, 14, 100, 14, 0 },
+ { 0, 6, 98, 24, 0 },
+ { 0, 2, 90, 36, 0 },
+ { 0, 0, 78, 50, 0 },
+ { 0, 64, 64, 0, 0 },
+ { 0, 50, 78, 0, 0 },
+ { 0, 36, 90, 2, 0 },
+ { 0, 24, 98, 6, 0 },
+};
+
+static const struct dispc_coef coef3_M12[8] = {
+ { 0, 16, 96, 16, 0 },
+ { 0, 9, 93, 26, 0 },
+ { 0, 4, 86, 38, 0 },
+ { 0, 1, 76, 51, 0 },
+ { 0, 64, 64, 0, 0 },
+ { 0, 51, 76, 1, 0 },
+ { 0, 38, 86, 4, 0 },
+ { 0, 26, 93, 9, 0 },
+};
+
+static const struct dispc_coef coef3_M13[8] = {
+ { 0, 18, 92, 18, 0 },
+ { 0, 10, 90, 28, 0 },
+ { 0, 5, 83, 40, 0 },
+ { 0, 1, 75, 52, 0 },
+ { 0, 64, 64, 0, 0 },
+ { 0, 52, 75, 1, 0 },
+ { 0, 40, 83, 5, 0 },
+ { 0, 28, 90, 10, 0 },
+};
+
+static const struct dispc_coef coef3_M14[8] = {
+ { 0, 20, 88, 20, 0 },
+ { 0, 12, 86, 30, 0 },
+ { 0, 6, 81, 41, 0 },
+ { 0, 2, 74, 52, 0 },
+ { 0, 64, 64, 0, 0 },
+ { 0, 52, 74, 2, 0 },
+ { 0, 41, 81, 6, 0 },
+ { 0, 30, 86, 12, 0 },
+};
+
+static const struct dispc_coef coef3_M16[8] = {
+ { 0, 22, 84, 22, 0 },
+ { 0, 14, 82, 32, 0 },
+ { 0, 8, 78, 42, 0 },
+ { 0, 3, 72, 53, 0 },
+ { 0, 64, 64, 0, 0 },
+ { 0, 53, 72, 3, 0 },
+ { 0, 42, 78, 8, 0 },
+ { 0, 32, 82, 14, 0 },
+};
+
+static const struct dispc_coef coef3_M19[8] = {
+ { 0, 24, 80, 24, 0 },
+ { 0, 16, 79, 33, 0 },
+ { 0, 9, 76, 43, 0 },
+ { 0, 4, 70, 54, 0 },
+ { 0, 64, 64, 0, 0 },
+ { 0, 54, 70, 4, 0 },
+ { 0, 43, 76, 9, 0 },
+ { 0, 33, 79, 16, 0 },
+};
+
+static const struct dispc_coef coef3_M22[8] = {
+ { 0, 25, 78, 25, 0 },
+ { 0, 17, 77, 34, 0 },
+ { 0, 10, 74, 44, 0 },
+ { 0, 5, 69, 54, 0 },
+ { 0, 64, 64, 0, 0 },
+ { 0, 54, 69, 5, 0 },
+ { 0, 44, 74, 10, 0 },
+ { 0, 34, 77, 17, 0 },
+};
+
+static const struct dispc_coef coef3_M26[8] = {
+ { 0, 26, 76, 26, 0 },
+ { 0, 19, 74, 35, 0 },
+ { 0, 11, 72, 45, 0 },
+ { 0, 5, 69, 54, 0 },
+ { 0, 64, 64, 0, 0 },
+ { 0, 54, 69, 5, 0 },
+ { 0, 45, 72, 11, 0 },
+ { 0, 35, 74, 19, 0 },
+};
+
+static const struct dispc_coef coef3_M32[8] = {
+ { 0, 27, 74, 27, 0 },
+ { 0, 19, 73, 36, 0 },
+ { 0, 12, 71, 45, 0 },
+ { 0, 6, 68, 54, 0 },
+ { 0, 64, 64, 0, 0 },
+ { 0, 54, 68, 6, 0 },
+ { 0, 45, 71, 12, 0 },
+ { 0, 36, 73, 19, 0 },
+};
+
+static const struct dispc_coef coef5_M8[8] = {
+ { 0, 0, 128, 0, 0 },
+ { -2, 14, 125, -10, 1 },
+ { -6, 33, 114, -15, 2 },
+ { -10, 55, 98, -16, 1 },
+ { 0, -14, 78, 78, -14 },
+ { 1, -16, 98, 55, -10 },
+ { 2, -15, 114, 33, -6 },
+ { 1, -10, 125, 14, -2 },
+};
+
+static const struct dispc_coef coef5_M9[8] = {
+ { -3, 10, 114, 10, -3 },
+ { -6, 24, 110, 0, -1 },
+ { -8, 40, 103, -7, 0 },
+ { -11, 58, 91, -11, 1 },
+ { 0, -12, 76, 76, -12 },
+ { 1, -11, 91, 58, -11 },
+ { 0, -7, 103, 40, -8 },
+ { -1, 0, 111, 24, -6 },
+};
+
+static const struct dispc_coef coef5_M10[8] = {
+ { -4, 18, 100, 18, -4 },
+ { -6, 30, 99, 8, -3 },
+ { -8, 44, 93, 0, -1 },
+ { -9, 58, 84, -5, 0 },
+ { 0, -8, 72, 72, -8 },
+ { 0, -5, 84, 58, -9 },
+ { -1, 0, 93, 44, -8 },
+ { -3, 8, 99, 30, -6 },
+};
+
+static const struct dispc_coef coef5_M11[8] = {
+ { -5, 23, 92, 23, -5 },
+ { -6, 34, 90, 13, -3 },
+ { -6, 45, 85, 6, -2 },
+ { -6, 57, 78, 0, -1 },
+ { 0, -4, 68, 68, -4 },
+ { -1, 0, 78, 57, -6 },
+ { -2, 6, 85, 45, -6 },
+ { -3, 13, 90, 34, -6 },
+};
+
+static const struct dispc_coef coef5_M12[8] = {
+ { -4, 26, 84, 26, -4 },
+ { -5, 36, 82, 18, -3 },
+ { -4, 46, 78, 10, -2 },
+ { -3, 55, 72, 5, -1 },
+ { 0, 0, 64, 64, 0 },
+ { -1, 5, 72, 55, -3 },
+ { -2, 10, 78, 46, -4 },
+ { -3, 18, 82, 36, -5 },
+};
+
+static const struct dispc_coef coef5_M13[8] = {
+ { -3, 28, 78, 28, -3 },
+ { -3, 37, 76, 21, -3 },
+ { -2, 45, 73, 14, -2 },
+ { 0, 53, 68, 8, -1 },
+ { 0, 3, 61, 61, 3 },
+ { -1, 8, 68, 53, 0 },
+ { -2, 14, 73, 45, -2 },
+ { -3, 21, 76, 37, -3 },
+};
+
+static const struct dispc_coef coef5_M14[8] = {
+ { -2, 30, 72, 30, -2 },
+ { -1, 37, 71, 23, -2 },
+ { 0, 45, 69, 16, -2 },
+ { 3, 52, 64, 10, -1 },
+ { 0, 6, 58, 58, 6 },
+ { -1, 10, 64, 52, 3 },
+ { -2, 16, 69, 45, 0 },
+ { -2, 23, 71, 37, -1 },
+};
+
+static const struct dispc_coef coef5_M16[8] = {
+ { 0, 31, 66, 31, 0 },
+ { 1, 38, 65, 25, -1 },
+ { 3, 44, 62, 20, -1 },
+ { 6, 49, 59, 14, 0 },
+ { 0, 10, 54, 54, 10 },
+ { 0, 14, 59, 49, 6 },
+ { -1, 20, 62, 44, 3 },
+ { -1, 25, 65, 38, 1 },
+};
+
+static const struct dispc_coef coef5_M19[8] = {
+ { 3, 32, 58, 32, 3 },
+ { 4, 38, 58, 27, 1 },
+ { 7, 42, 55, 23, 1 },
+ { 10, 46, 54, 18, 0 },
+ { 0, 14, 50, 50, 14 },
+ { 0, 18, 54, 46, 10 },
+ { 1, 23, 55, 42, 7 },
+ { 1, 27, 58, 38, 4 },
+};
+
+static const struct dispc_coef coef5_M22[8] = {
+ { 4, 33, 54, 33, 4 },
+ { 6, 37, 54, 28, 3 },
+ { 9, 41, 53, 24, 1 },
+ { 12, 45, 51, 20, 0 },
+ { 0, 16, 48, 48, 16 },
+ { 0, 20, 51, 45, 12 },
+ { 1, 24, 53, 41, 9 },
+ { 3, 28, 54, 37, 6 },
+};
+
+static const struct dispc_coef coef5_M26[8] = {
+ { 6, 33, 50, 33, 6 },
+ { 8, 36, 51, 29, 4 },
+ { 11, 40, 50, 25, 2 },
+ { 14, 43, 48, 22, 1 },
+ { 0, 18, 46, 46, 18 },
+ { 1, 22, 48, 43, 14 },
+ { 2, 25, 50, 40, 11 },
+ { 4, 29, 51, 36, 8 },
+};
+
+static const struct dispc_coef coef5_M32[8] = {
+ { 7, 33, 48, 33, 7 },
+ { 10, 36, 48, 29, 5 },
+ { 13, 39, 47, 26, 3 },
+ { 16, 42, 46, 23, 1 },
+ { 0, 19, 45, 45, 19 },
+ { 1, 23, 46, 42, 16 },
+ { 3, 26, 47, 39, 13 },
+ { 5, 29, 48, 36, 10 },
+};
+
+const struct dispc_coef *dispc_ovl_get_scale_coef(int inc, int five_taps)
+{
+ int i;
+ static const struct {
+ int Mmin;
+ int Mmax;
+ const struct dispc_coef *coef_3;
+ const struct dispc_coef *coef_5;
+ } coefs[] = {
+ { 27, 32, coef3_M32, coef5_M32 },
+ { 23, 26, coef3_M26, coef5_M26 },
+ { 20, 22, coef3_M22, coef5_M22 },
+ { 17, 19, coef3_M19, coef5_M19 },
+ { 15, 16, coef3_M16, coef5_M16 },
+ { 14, 14, coef3_M14, coef5_M14 },
+ { 13, 13, coef3_M13, coef5_M13 },
+ { 12, 12, coef3_M12, coef5_M12 },
+ { 11, 11, coef3_M11, coef5_M11 },
+ { 10, 10, coef3_M10, coef5_M10 },
+ { 9, 9, coef3_M9, coef5_M9 },
+ { 4, 8, coef3_M8, coef5_M8 },
+ /*
+ * When upscaling more than two times, blockiness and outlines
+ * around the image are observed when M8 tables are used. M11,
+ * M16 and M19 tables are used to prevent this.
+ */
+ { 3, 3, coef3_M11, coef5_M11 },
+ { 2, 2, coef3_M16, coef5_M16 },
+ { 0, 1, coef3_M19, coef5_M19 },
+ };
+
+ inc /= 128;
+ for (i = 0; i < ARRAY_LEN(coefs); ++i)
+ if (inc >= coefs[i].Mmin && inc <= coefs[i].Mmax)
+ return five_taps ? coefs[i].coef_5 : coefs[i].coef_3;
+ return NULL;
+}
--
1.7.1
^ permalink raw reply related
* [PATCH V2 0/2] OMAPDSS: DISPC: Peformance improvement of DISPC Scaling
From: Chandrabhanu Mahapatra @ 2011-12-19 8:45 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
Hi everyone,
the following patch set directs to improve the scaling performance of DISPC
module which consists of two pacthes.
The first patch is based on code of Lajos Molnar <lajos@ti.com> from Android
Kernel, which updates the code with new set of coefficients to improve the
scaling performance.
The second patch is based on some very valuable suggestions by
Sebastien Fagard <s-fagard@ti.com> and directs to modify the clock
requirements for scaling to support OMAP4 and avoid clock failure issues.
I have tested these patches on OMAP2, OMAP3 AND OMAP4. To test on 2430SDP
board I have created a patch to enable the display which will follow later.
All your comments and suggestions are welcome.
Regards,
Chandrabhanu
Chandrabhanu Mahapatra (2):
OMAPDSS: DISPC: Update Fir Coefficients
OMAPDSS: DISPC: Update Scaling Clock Logic
drivers/video/omap2/dss/Makefile | 2 +-
drivers/video/omap2/dss/dispc.c | 203 +++++++--------------
drivers/video/omap2/dss/dispc.h | 11 +
drivers/video/omap2/dss/dispc_coefs.c | 326 ++++++++++++++++++++++++++++++++
drivers/video/omap2/dss/dss_features.c | 7 +
drivers/video/omap2/dss/dss_features.h | 1 +
6 files changed, 411 insertions(+), 139 deletions(-)
create mode 100644 drivers/video/omap2/dss/dispc_coefs.c
^ permalink raw reply
* RE: [PATCH V2] video: s3c-fb: modify runtime pm functions
From: Jingoo Han @ 2011-12-19 3:58 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <002801ccb2f7$926c0240$b74406c0$%han@samsung.com>
Hi Florian,
Florian Tobias Schandinat wrote:
> -----Original Message-----
> Subject: Re: [PATCH V2] video: s3c-fb: modify runtime pm functions
>
> On 12/19/2011 02:19 AM, Jingoo Han wrote:
> > Hi Florian,
> >
> > Florian Tobias Schandinat wrote:
> >> -----Original Message-----
> >> Subject: Re: [PATCH V2] video: s3c-fb: modify runtime pm functions
> >>
> >> Hi Jingoo,
> >>
> >> On 12/05/2011 02:42 AM, Jingoo Han wrote:
> >>> Runtime suspend and runtime resume are modified in order to
> >>> reduce the complexity and improve the usability of runtime pm.
> >>> After probe function, s3c-fb driver is not suspended until
> >>> suspend or remove is called.
> >>>
> >>> The scheme is changed as follows:
> >>> runtime_get is only called in probe and resume.
> >>> runtime_put is only called in remove and suspend.
> >>> open/close cannot call the runtime_get/put.
> >>>
> >>> Also, runtime_susepnd/resume are just called by runtime pm,
> >>> not doing suspend/resume routine any longer. This is because
> >>> open/close cannot call the runtime_get/put; the suspend/resume
> >>> routine in runtime_suspend/resume were previously used when
> >>> open and close were called.
> >>>
> >>> The name of s3c-fb dev_pm_ops is changed from s3cfb_pm_ops to
> >>> s3c_fb_pm_ops in order to use more consistent naming.
> >>>
> >>> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> >>
> >> the patch itself looks okay to me, but I'm wondering what changes will be
> >> visible to the user, if any?
> >> Does the behavior change for example, when no one has called open?
> > Previously, in order to display framebuffer to LCD panel, user should call
> > open after booting, because runtime_pm_put is called after prove function.
> >
> > However, now, after booting, framebuffer can be displayed to LCD panel
> > without calling open.
>
> As I thought. What is displayed in such a case? (no open called = no one put any
> image in the framebuffer)
Sorry, user should call open, too. There is no visible change for user.
Just runtime pm put & get are removed between probe and open.
> As the new behavior is probably okay for single framebuffer I'll apply this
> patch, I'm just asking as I'm planning to do quite the opposite in viafb to get
> rid of this situation in dual framebuffer mode where it is quite likely that one
> display is not used for some time and at the moment only displays rubbish.
OK, I'll check dual framebuffer mode.
Thank you.
Best regards,
Jingoo Han
>
> Best regards,
>
> Florian Tobias Schandinat
>
> >
> > Best regards,
> > Jingoo Han
> >>
> >>
> >> Best regards,
> >>
> >> Florian Tobias Schandinat
> >>
> >>> ---
> >>> v2: fix unaligned lines
> >>>
> >>> drivers/video/s3c-fb.c | 51 ++++++++++++++++++-----------------------------
> >>> 1 files changed, 20 insertions(+), 31 deletions(-)
> >>>
> >>> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
> >>> index e84677e..91e629a 100644
> >>> --- a/drivers/video/s3c-fb.c
> >>> +++ b/drivers/video/s3c-fb.c
> >>> @@ -1028,30 +1028,8 @@ static int s3c_fb_ioctl(struct fb_info *info, unsigned int cmd,
> >>> return ret;
> >>> }
> >>>
> >>> -static int s3c_fb_open(struct fb_info *info, int user)
> >>> -{
> >>> - struct s3c_fb_win *win = info->par;
> >>> - struct s3c_fb *sfb = win->parent;
> >>> -
> >>> - pm_runtime_get_sync(sfb->dev);
> >>> -
> >>> - return 0;
> >>> -}
> >>> -
> >>> -static int s3c_fb_release(struct fb_info *info, int user)
> >>> -{
> >>> - struct s3c_fb_win *win = info->par;
> >>> - struct s3c_fb *sfb = win->parent;
> >>> -
> >>> - pm_runtime_put_sync(sfb->dev);
> >>> -
> >>> - return 0;
> >>> -}
> >>> -
> >>> static struct fb_ops s3c_fb_ops = {
> >>> .owner = THIS_MODULE,
> >>> - .fb_open = s3c_fb_open,
> >>> - .fb_release = s3c_fb_release,
> >>> .fb_check_var = s3c_fb_check_var,
> >>> .fb_set_par = s3c_fb_set_par,
> >>> .fb_blank = s3c_fb_blank,
> >>> @@ -1458,7 +1436,6 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)
> >>> }
> >>>
> >>> platform_set_drvdata(pdev, sfb);
> >>> - pm_runtime_put_sync(sfb->dev);
> >>>
> >>> return 0;
> >>>
> >>> @@ -1498,8 +1475,6 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)
> >>> struct s3c_fb *sfb = platform_get_drvdata(pdev);
> >>> int win;
> >>>
> >>> - pm_runtime_get_sync(sfb->dev);
> >>> -
> >>> for (win = 0; win < S3C_FB_MAX_WIN; win++)
> >>> if (sfb->windows[win])
> >>> s3c_fb_release_win(sfb, sfb->windows[win]);
> >>> @@ -1525,7 +1500,7 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)
> >>> return 0;
> >>> }
> >>>
> >>> -#ifdef CONFIG_PM
> >>> +#ifdef CONFIG_PM_SLEEP
> >>> static int s3c_fb_suspend(struct device *dev)
> >>> {
> >>> struct platform_device *pdev = to_platform_device(dev);
> >>> @@ -1546,6 +1521,8 @@ static int s3c_fb_suspend(struct device *dev)
> >>> clk_disable(sfb->lcd_clk);
> >>>
> >>> clk_disable(sfb->bus_clk);
> >>> + pm_runtime_put_sync(sfb->dev);
> >>> +
> >>> return 0;
> >>> }
> >>>
> >>> @@ -1557,6 +1534,7 @@ static int s3c_fb_resume(struct device *dev)
> >>> struct s3c_fb_win *win;
> >>> int win_no;
> >>>
> >>> + pm_runtime_get_sync(sfb->dev);
> >>> clk_enable(sfb->bus_clk);
> >>>
> >>> if (!sfb->variant.has_clksel)
> >>> @@ -1590,11 +1568,19 @@ static int s3c_fb_resume(struct device *dev)
> >>>
> >>> return 0;
> >>> }
> >>> -#else
> >>> -#define s3c_fb_suspend NULL
> >>> -#define s3c_fb_resume NULL
> >>> #endif
> >>>
> >>> +#ifdef CONFIG_PM_RUNTIME
> >>> +static int s3c_fb_runtime_suspend(struct device *dev)
> >>> +{
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static int s3c_fb_runtime_resume(struct device *dev)
> >>> +{
> >>> + return 0;
> >>> +}
> >>> +#endif
> >>>
> >>> #define VALID_BPP124 (VALID_BPP(1) | VALID_BPP(2) | VALID_BPP(4))
> >>> #define VALID_BPP1248 (VALID_BPP124 | VALID_BPP(8))
> >>> @@ -1917,7 +1903,10 @@ static struct platform_device_id s3c_fb_driver_ids[] = {
> >>> };
> >>> MODULE_DEVICE_TABLE(platform, s3c_fb_driver_ids);
> >>>
> >>> -static UNIVERSAL_DEV_PM_OPS(s3cfb_pm_ops, s3c_fb_suspend, s3c_fb_resume, NULL);
> >>> +static const struct dev_pm_ops s3c_fb_pm_ops = {
> >>> + SET_SYSTEM_SLEEP_PM_OPS(s3c_fb_suspend, s3c_fb_resume)
> >>> + SET_RUNTIME_PM_OPS(s3c_fb_runtime_suspend, s3c_fb_runtime_resume, NULL)
> >>> +};
> >>>
> >>> static struct platform_driver s3c_fb_driver = {
> >>> .probe = s3c_fb_probe,
> >>> @@ -1926,7 +1915,7 @@ static struct platform_driver s3c_fb_driver = {
> >>> .driver = {
> >>> .name = "s3c-fb",
> >>> .owner = THIS_MODULE,
> >>> - .pm = &s3cfb_pm_ops,
> >>> + .pm = &s3c_fb_pm_ops,
> >>> },
> >>> };
> >>>
> >
> >
^ permalink raw reply
* Re: [PATCH V2] video: s3c-fb: modify runtime pm functions
From: Florian Tobias Schandinat @ 2011-12-19 2:35 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <002801ccb2f7$926c0240$b74406c0$%han@samsung.com>
On 12/19/2011 02:19 AM, Jingoo Han wrote:
> Hi Florian,
>
> Florian Tobias Schandinat wrote:
>> -----Original Message-----
>> Subject: Re: [PATCH V2] video: s3c-fb: modify runtime pm functions
>>
>> Hi Jingoo,
>>
>> On 12/05/2011 02:42 AM, Jingoo Han wrote:
>>> Runtime suspend and runtime resume are modified in order to
>>> reduce the complexity and improve the usability of runtime pm.
>>> After probe function, s3c-fb driver is not suspended until
>>> suspend or remove is called.
>>>
>>> The scheme is changed as follows:
>>> runtime_get is only called in probe and resume.
>>> runtime_put is only called in remove and suspend.
>>> open/close cannot call the runtime_get/put.
>>>
>>> Also, runtime_susepnd/resume are just called by runtime pm,
>>> not doing suspend/resume routine any longer. This is because
>>> open/close cannot call the runtime_get/put; the suspend/resume
>>> routine in runtime_suspend/resume were previously used when
>>> open and close were called.
>>>
>>> The name of s3c-fb dev_pm_ops is changed from s3cfb_pm_ops to
>>> s3c_fb_pm_ops in order to use more consistent naming.
>>>
>>> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
>>
>> the patch itself looks okay to me, but I'm wondering what changes will be
>> visible to the user, if any?
>> Does the behavior change for example, when no one has called open?
> Previously, in order to display framebuffer to LCD panel, user should call
> open after booting, because runtime_pm_put is called after prove function.
>
> However, now, after booting, framebuffer can be displayed to LCD panel
> without calling open.
As I thought. What is displayed in such a case? (no open called = no one put any
image in the framebuffer)
As the new behavior is probably okay for single framebuffer I'll apply this
patch, I'm just asking as I'm planning to do quite the opposite in viafb to get
rid of this situation in dual framebuffer mode where it is quite likely that one
display is not used for some time and at the moment only displays rubbish.
Best regards,
Florian Tobias Schandinat
>
> Best regards,
> Jingoo Han
>>
>>
>> Best regards,
>>
>> Florian Tobias Schandinat
>>
>>> ---
>>> v2: fix unaligned lines
>>>
>>> drivers/video/s3c-fb.c | 51 ++++++++++++++++++-----------------------------
>>> 1 files changed, 20 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
>>> index e84677e..91e629a 100644
>>> --- a/drivers/video/s3c-fb.c
>>> +++ b/drivers/video/s3c-fb.c
>>> @@ -1028,30 +1028,8 @@ static int s3c_fb_ioctl(struct fb_info *info, unsigned int cmd,
>>> return ret;
>>> }
>>>
>>> -static int s3c_fb_open(struct fb_info *info, int user)
>>> -{
>>> - struct s3c_fb_win *win = info->par;
>>> - struct s3c_fb *sfb = win->parent;
>>> -
>>> - pm_runtime_get_sync(sfb->dev);
>>> -
>>> - return 0;
>>> -}
>>> -
>>> -static int s3c_fb_release(struct fb_info *info, int user)
>>> -{
>>> - struct s3c_fb_win *win = info->par;
>>> - struct s3c_fb *sfb = win->parent;
>>> -
>>> - pm_runtime_put_sync(sfb->dev);
>>> -
>>> - return 0;
>>> -}
>>> -
>>> static struct fb_ops s3c_fb_ops = {
>>> .owner = THIS_MODULE,
>>> - .fb_open = s3c_fb_open,
>>> - .fb_release = s3c_fb_release,
>>> .fb_check_var = s3c_fb_check_var,
>>> .fb_set_par = s3c_fb_set_par,
>>> .fb_blank = s3c_fb_blank,
>>> @@ -1458,7 +1436,6 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)
>>> }
>>>
>>> platform_set_drvdata(pdev, sfb);
>>> - pm_runtime_put_sync(sfb->dev);
>>>
>>> return 0;
>>>
>>> @@ -1498,8 +1475,6 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)
>>> struct s3c_fb *sfb = platform_get_drvdata(pdev);
>>> int win;
>>>
>>> - pm_runtime_get_sync(sfb->dev);
>>> -
>>> for (win = 0; win < S3C_FB_MAX_WIN; win++)
>>> if (sfb->windows[win])
>>> s3c_fb_release_win(sfb, sfb->windows[win]);
>>> @@ -1525,7 +1500,7 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)
>>> return 0;
>>> }
>>>
>>> -#ifdef CONFIG_PM
>>> +#ifdef CONFIG_PM_SLEEP
>>> static int s3c_fb_suspend(struct device *dev)
>>> {
>>> struct platform_device *pdev = to_platform_device(dev);
>>> @@ -1546,6 +1521,8 @@ static int s3c_fb_suspend(struct device *dev)
>>> clk_disable(sfb->lcd_clk);
>>>
>>> clk_disable(sfb->bus_clk);
>>> + pm_runtime_put_sync(sfb->dev);
>>> +
>>> return 0;
>>> }
>>>
>>> @@ -1557,6 +1534,7 @@ static int s3c_fb_resume(struct device *dev)
>>> struct s3c_fb_win *win;
>>> int win_no;
>>>
>>> + pm_runtime_get_sync(sfb->dev);
>>> clk_enable(sfb->bus_clk);
>>>
>>> if (!sfb->variant.has_clksel)
>>> @@ -1590,11 +1568,19 @@ static int s3c_fb_resume(struct device *dev)
>>>
>>> return 0;
>>> }
>>> -#else
>>> -#define s3c_fb_suspend NULL
>>> -#define s3c_fb_resume NULL
>>> #endif
>>>
>>> +#ifdef CONFIG_PM_RUNTIME
>>> +static int s3c_fb_runtime_suspend(struct device *dev)
>>> +{
>>> + return 0;
>>> +}
>>> +
>>> +static int s3c_fb_runtime_resume(struct device *dev)
>>> +{
>>> + return 0;
>>> +}
>>> +#endif
>>>
>>> #define VALID_BPP124 (VALID_BPP(1) | VALID_BPP(2) | VALID_BPP(4))
>>> #define VALID_BPP1248 (VALID_BPP124 | VALID_BPP(8))
>>> @@ -1917,7 +1903,10 @@ static struct platform_device_id s3c_fb_driver_ids[] = {
>>> };
>>> MODULE_DEVICE_TABLE(platform, s3c_fb_driver_ids);
>>>
>>> -static UNIVERSAL_DEV_PM_OPS(s3cfb_pm_ops, s3c_fb_suspend, s3c_fb_resume, NULL);
>>> +static const struct dev_pm_ops s3c_fb_pm_ops = {
>>> + SET_SYSTEM_SLEEP_PM_OPS(s3c_fb_suspend, s3c_fb_resume)
>>> + SET_RUNTIME_PM_OPS(s3c_fb_runtime_suspend, s3c_fb_runtime_resume, NULL)
>>> +};
>>>
>>> static struct platform_driver s3c_fb_driver = {
>>> .probe = s3c_fb_probe,
>>> @@ -1926,7 +1915,7 @@ static struct platform_driver s3c_fb_driver = {
>>> .driver = {
>>> .name = "s3c-fb",
>>> .owner = THIS_MODULE,
>>> - .pm = &s3cfb_pm_ops,
>>> + .pm = &s3c_fb_pm_ops,
>>> },
>>> };
>>>
>
>
^ permalink raw reply
* Re: Re: [PATCH V2] video: s3c-fb: modify runtime pm functions
From: Jingoo Han @ 2011-12-19 2:19 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <002801ccb2f7$926c0240$b74406c0$%han@samsung.com>
Hi Florian,
Florian Tobias Schandinat wrote:
> -----Original Message-----
> Subject: Re: [PATCH V2] video: s3c-fb: modify runtime pm functions
>
> Hi Jingoo,
>
> On 12/05/2011 02:42 AM, Jingoo Han wrote:
> > Runtime suspend and runtime resume are modified in order to
> > reduce the complexity and improve the usability of runtime pm.
> > After probe function, s3c-fb driver is not suspended until
> > suspend or remove is called.
> >
> > The scheme is changed as follows:
> > runtime_get is only called in probe and resume.
> > runtime_put is only called in remove and suspend.
> > open/close cannot call the runtime_get/put.
> >
> > Also, runtime_susepnd/resume are just called by runtime pm,
> > not doing suspend/resume routine any longer. This is because
> > open/close cannot call the runtime_get/put; the suspend/resume
> > routine in runtime_suspend/resume were previously used when
> > open and close were called.
> >
> > The name of s3c-fb dev_pm_ops is changed from s3cfb_pm_ops to
> > s3c_fb_pm_ops in order to use more consistent naming.
> >
> > Signed-off-by: Jingoo Han <jg1.han@samsung.com>
>
> the patch itself looks okay to me, but I'm wondering what changes will be
> visible to the user, if any?
> Does the behavior change for example, when no one has called open?
Previously, in order to display framebuffer to LCD panel, user should call
open after booting, because runtime_pm_put is called after prove function.
However, now, after booting, framebuffer can be displayed to LCD panel
without calling open.
Best regards,
Jingoo Han
>
>
> Best regards,
>
> Florian Tobias Schandinat
>
> > ---
> > v2: fix unaligned lines
> >
> > drivers/video/s3c-fb.c | 51 ++++++++++++++++++-----------------------------
> > 1 files changed, 20 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
> > index e84677e..91e629a 100644
> > --- a/drivers/video/s3c-fb.c
> > +++ b/drivers/video/s3c-fb.c
> > @@ -1028,30 +1028,8 @@ static int s3c_fb_ioctl(struct fb_info *info, unsigned int cmd,
> > return ret;
> > }
> >
> > -static int s3c_fb_open(struct fb_info *info, int user)
> > -{
> > - struct s3c_fb_win *win = info->par;
> > - struct s3c_fb *sfb = win->parent;
> > -
> > - pm_runtime_get_sync(sfb->dev);
> > -
> > - return 0;
> > -}
> > -
> > -static int s3c_fb_release(struct fb_info *info, int user)
> > -{
> > - struct s3c_fb_win *win = info->par;
> > - struct s3c_fb *sfb = win->parent;
> > -
> > - pm_runtime_put_sync(sfb->dev);
> > -
> > - return 0;
> > -}
> > -
> > static struct fb_ops s3c_fb_ops = {
> > .owner = THIS_MODULE,
> > - .fb_open = s3c_fb_open,
> > - .fb_release = s3c_fb_release,
> > .fb_check_var = s3c_fb_check_var,
> > .fb_set_par = s3c_fb_set_par,
> > .fb_blank = s3c_fb_blank,
> > @@ -1458,7 +1436,6 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)
> > }
> >
> > platform_set_drvdata(pdev, sfb);
> > - pm_runtime_put_sync(sfb->dev);
> >
> > return 0;
> >
> > @@ -1498,8 +1475,6 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)
> > struct s3c_fb *sfb = platform_get_drvdata(pdev);
> > int win;
> >
> > - pm_runtime_get_sync(sfb->dev);
> > -
> > for (win = 0; win < S3C_FB_MAX_WIN; win++)
> > if (sfb->windows[win])
> > s3c_fb_release_win(sfb, sfb->windows[win]);
> > @@ -1525,7 +1500,7 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)
> > return 0;
> > }
> >
> > -#ifdef CONFIG_PM
> > +#ifdef CONFIG_PM_SLEEP
> > static int s3c_fb_suspend(struct device *dev)
> > {
> > struct platform_device *pdev = to_platform_device(dev);
> > @@ -1546,6 +1521,8 @@ static int s3c_fb_suspend(struct device *dev)
> > clk_disable(sfb->lcd_clk);
> >
> > clk_disable(sfb->bus_clk);
> > + pm_runtime_put_sync(sfb->dev);
> > +
> > return 0;
> > }
> >
> > @@ -1557,6 +1534,7 @@ static int s3c_fb_resume(struct device *dev)
> > struct s3c_fb_win *win;
> > int win_no;
> >
> > + pm_runtime_get_sync(sfb->dev);
> > clk_enable(sfb->bus_clk);
> >
> > if (!sfb->variant.has_clksel)
> > @@ -1590,11 +1568,19 @@ static int s3c_fb_resume(struct device *dev)
> >
> > return 0;
> > }
> > -#else
> > -#define s3c_fb_suspend NULL
> > -#define s3c_fb_resume NULL
> > #endif
> >
> > +#ifdef CONFIG_PM_RUNTIME
> > +static int s3c_fb_runtime_suspend(struct device *dev)
> > +{
> > + return 0;
> > +}
> > +
> > +static int s3c_fb_runtime_resume(struct device *dev)
> > +{
> > + return 0;
> > +}
> > +#endif
> >
> > #define VALID_BPP124 (VALID_BPP(1) | VALID_BPP(2) | VALID_BPP(4))
> > #define VALID_BPP1248 (VALID_BPP124 | VALID_BPP(8))
> > @@ -1917,7 +1903,10 @@ static struct platform_device_id s3c_fb_driver_ids[] = {
> > };
> > MODULE_DEVICE_TABLE(platform, s3c_fb_driver_ids);
> >
> > -static UNIVERSAL_DEV_PM_OPS(s3cfb_pm_ops, s3c_fb_suspend, s3c_fb_resume, NULL);
> > +static const struct dev_pm_ops s3c_fb_pm_ops = {
> > + SET_SYSTEM_SLEEP_PM_OPS(s3c_fb_suspend, s3c_fb_resume)
> > + SET_RUNTIME_PM_OPS(s3c_fb_runtime_suspend, s3c_fb_runtime_resume, NULL)
> > +};
> >
> > static struct platform_driver s3c_fb_driver = {
> > .probe = s3c_fb_probe,
> > @@ -1926,7 +1915,7 @@ static struct platform_driver s3c_fb_driver = {
> > .driver = {
> > .name = "s3c-fb",
> > .owner = THIS_MODULE,
> > - .pm = &s3cfb_pm_ops,
> > + .pm = &s3c_fb_pm_ops,
> > },
> > };
> >
^ permalink raw reply
* [PATCH] MAINTAINERS: add a maintainer for Samsung Framebuffer driver
From: Jingoo Han @ 2011-12-19 2:09 UTC (permalink / raw)
To: linux-fbdev
Add a maintainer for Samsung Framebuffer driver.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Cc: Ben Dooks <ben-linux@fluff.org>
---
MAINTAINERS | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 855afd4..a03e288 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5667,6 +5667,12 @@ L: alsa-devel@alsa-project.org (moderated for non-subscribers)
S: Supported
F: sound/soc/samsung
+SAMSUNG FRAMEBUFFER DRIVER
+M: Jingoo Han <jg1.han@samsung.com>
+L: linux-fbdev@vger.kernel.org
+S: Maintained
+F: drivers/video/s3c-fb.c
+
SERIAL DRIVERS
M: Alan Cox <alan@linux.intel.com>
L: linux-serial@vger.kernel.org
--
1.7.1
^ permalink raw reply related
* Re: [PATCH V2] video: s3c-fb: modify runtime pm functions
From: Florian Tobias Schandinat @ 2011-12-19 2:01 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <002801ccb2f7$926c0240$b74406c0$%han@samsung.com>
Hi Jingoo,
On 12/05/2011 02:42 AM, Jingoo Han wrote:
> Runtime suspend and runtime resume are modified in order to
> reduce the complexity and improve the usability of runtime pm.
> After probe function, s3c-fb driver is not suspended until
> suspend or remove is called.
>
> The scheme is changed as follows:
> runtime_get is only called in probe and resume.
> runtime_put is only called in remove and suspend.
> open/close cannot call the runtime_get/put.
>
> Also, runtime_susepnd/resume are just called by runtime pm,
> not doing suspend/resume routine any longer. This is because
> open/close cannot call the runtime_get/put; the suspend/resume
> routine in runtime_suspend/resume were previously used when
> open and close were called.
>
> The name of s3c-fb dev_pm_ops is changed from s3cfb_pm_ops to
> s3c_fb_pm_ops in order to use more consistent naming.
>
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
the patch itself looks okay to me, but I'm wondering what changes will be
visible to the user, if any?
Does the behavior change for example, when no one has called open?
Best regards,
Florian Tobias Schandinat
> ---
> v2: fix unaligned lines
>
> drivers/video/s3c-fb.c | 51 ++++++++++++++++++-----------------------------
> 1 files changed, 20 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
> index e84677e..91e629a 100644
> --- a/drivers/video/s3c-fb.c
> +++ b/drivers/video/s3c-fb.c
> @@ -1028,30 +1028,8 @@ static int s3c_fb_ioctl(struct fb_info *info, unsigned int cmd,
> return ret;
> }
>
> -static int s3c_fb_open(struct fb_info *info, int user)
> -{
> - struct s3c_fb_win *win = info->par;
> - struct s3c_fb *sfb = win->parent;
> -
> - pm_runtime_get_sync(sfb->dev);
> -
> - return 0;
> -}
> -
> -static int s3c_fb_release(struct fb_info *info, int user)
> -{
> - struct s3c_fb_win *win = info->par;
> - struct s3c_fb *sfb = win->parent;
> -
> - pm_runtime_put_sync(sfb->dev);
> -
> - return 0;
> -}
> -
> static struct fb_ops s3c_fb_ops = {
> .owner = THIS_MODULE,
> - .fb_open = s3c_fb_open,
> - .fb_release = s3c_fb_release,
> .fb_check_var = s3c_fb_check_var,
> .fb_set_par = s3c_fb_set_par,
> .fb_blank = s3c_fb_blank,
> @@ -1458,7 +1436,6 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)
> }
>
> platform_set_drvdata(pdev, sfb);
> - pm_runtime_put_sync(sfb->dev);
>
> return 0;
>
> @@ -1498,8 +1475,6 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)
> struct s3c_fb *sfb = platform_get_drvdata(pdev);
> int win;
>
> - pm_runtime_get_sync(sfb->dev);
> -
> for (win = 0; win < S3C_FB_MAX_WIN; win++)
> if (sfb->windows[win])
> s3c_fb_release_win(sfb, sfb->windows[win]);
> @@ -1525,7 +1500,7 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)
> return 0;
> }
>
> -#ifdef CONFIG_PM
> +#ifdef CONFIG_PM_SLEEP
> static int s3c_fb_suspend(struct device *dev)
> {
> struct platform_device *pdev = to_platform_device(dev);
> @@ -1546,6 +1521,8 @@ static int s3c_fb_suspend(struct device *dev)
> clk_disable(sfb->lcd_clk);
>
> clk_disable(sfb->bus_clk);
> + pm_runtime_put_sync(sfb->dev);
> +
> return 0;
> }
>
> @@ -1557,6 +1534,7 @@ static int s3c_fb_resume(struct device *dev)
> struct s3c_fb_win *win;
> int win_no;
>
> + pm_runtime_get_sync(sfb->dev);
> clk_enable(sfb->bus_clk);
>
> if (!sfb->variant.has_clksel)
> @@ -1590,11 +1568,19 @@ static int s3c_fb_resume(struct device *dev)
>
> return 0;
> }
> -#else
> -#define s3c_fb_suspend NULL
> -#define s3c_fb_resume NULL
> #endif
>
> +#ifdef CONFIG_PM_RUNTIME
> +static int s3c_fb_runtime_suspend(struct device *dev)
> +{
> + return 0;
> +}
> +
> +static int s3c_fb_runtime_resume(struct device *dev)
> +{
> + return 0;
> +}
> +#endif
>
> #define VALID_BPP124 (VALID_BPP(1) | VALID_BPP(2) | VALID_BPP(4))
> #define VALID_BPP1248 (VALID_BPP124 | VALID_BPP(8))
> @@ -1917,7 +1903,10 @@ static struct platform_device_id s3c_fb_driver_ids[] = {
> };
> MODULE_DEVICE_TABLE(platform, s3c_fb_driver_ids);
>
> -static UNIVERSAL_DEV_PM_OPS(s3cfb_pm_ops, s3c_fb_suspend, s3c_fb_resume, NULL);
> +static const struct dev_pm_ops s3c_fb_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(s3c_fb_suspend, s3c_fb_resume)
> + SET_RUNTIME_PM_OPS(s3c_fb_runtime_suspend, s3c_fb_runtime_resume, NULL)
> +};
>
> static struct platform_driver s3c_fb_driver = {
> .probe = s3c_fb_probe,
> @@ -1926,7 +1915,7 @@ static struct platform_driver s3c_fb_driver = {
> .driver = {
> .name = "s3c-fb",
> .owner = THIS_MODULE,
> - .pm = &s3cfb_pm_ops,
> + .pm = &s3c_fb_pm_ops,
> },
> };
>
^ permalink raw reply
* Re: [PATCH 27/57] fbdev: sh_mobile_lcdc: Pass a video mode to the notify callback
From: Laurent Pinchart @ 2011-12-19 1:29 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1323784972-24205-28-git-send-email-laurent.pinchart@ideasonboard.com>
Hi Guennadi,
On Friday 16 December 2011 11:33:55 Guennadi Liakhovetski wrote:
> On Fri, 16 Dec 2011, Laurent Pinchart wrote:
> > On Thursday 15 December 2011 20:01:54 Guennadi Liakhovetski wrote:
> > > On Tue, 13 Dec 2011, Laurent Pinchart wrote:
> > > > Pass pointers to struct fb_videomode and struct fb_monspecs instead
> > > > of struct fb_var_screeninfo to the notify callback.
> > > >
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > ---
> > > >
> > > > drivers/video/sh_mobile_hdmi.c | 59
> > > > +++++++++++++++++--------------------
> > > > drivers/video/sh_mobile_lcdcfb.c
> > > >
> > > > | 40 ++++++++++++++----------- drivers/video/sh_mobile_lcdcfb.h |
> > > >
> > > > 3 +-
> > > > 3 files changed, 51 insertions(+), 51 deletions(-)
> > >
> > > [snip]
> > >
> > > > diff --git a/drivers/video/sh_mobile_lcdcfb.c
> > > > b/drivers/video/sh_mobile_lcdcfb.c index 21e5f10..4ec216e 100644
> > > > --- a/drivers/video/sh_mobile_lcdcfb.c
> > > > +++ b/drivers/video/sh_mobile_lcdcfb.c
> > >
> > > [ditto]
> > >
> > > > @@ -433,12 +432,17 @@ static int sh_mobile_lcdc_display_notify(struct
> > > > sh_mobile_lcdc_chan *ch,
> > > >
> > > > }
> > > > break;
> > > >
> > > > - case SH_MOBILE_LCDC_EVENT_DISPLAY_MODE:
> > > > + case SH_MOBILE_LCDC_EVENT_DISPLAY_MODE: {
> > > > + struct fb_var_screeninfo var;
> > > > +
> > > >
> > > > /* Validate a proposed new mode */
> > > >
> > > > - var->bits_per_pixel = info->var.bits_per_pixel;
> > > > - ret = info->fbops->fb_check_var(var, info);
> > > > + fb_videomode_to_var(&var, mode);
> > > > + var.bits_per_pixel = info->var.bits_per_pixel;
> > > > + var.grayscale = info->var.grayscale;
> > > > + ret = info->fbops->fb_check_var(&var, info);
> > > >
> > > > break;
> > > >
> > > > }
> > > >
> > > > + }
> > >
> > > nitpick - please, realign:-)
> >
> > It's common in driver code not to increase the identation level twice in
> > a case statement if braces are needed. However, those "braced"
> > statements are usually not the last ones in the switch. In this
> > particular case this creates an alignment issue at the end, as your
> > correctly pointed out. However, I'd like to avoid increasing the
> > indentation of the whole block. Increasing the indentation of the
> > closing brace only also causes an alignment issue. I can add a default
> > case that returns an error to fix this, but that's adding code to fix a
> > cosmetic problem :-) What's your preference.
>
> Personally, I don't like just using braces without a related statement
> like "if," "do," etc. In such "case" cases I usually avoid adding own
> braces and just put any declarations, that are needed there in the
> enclosing block, e.g., the function.
Using braces with case statements is a common practice in the kernel, without
adding an extra indentation level.
In this particular case I can move the variable declaration at the top of the
function, as the generated code on ARM (at least with gcc ('cs2009q3-67-hard-
sb4') 4.4.1) doesn't differ.
> If you really don't want either, I would probably prefer a style like
>
> switch (x) {
> case X:
> {
> int y;
> my_statement(y);
> break;
> }
> }
>
> You could also do
>
> switch (x) {
> case X:
> do {
> int y;
> my_statement(y);
> break;
> } while (0);
> }
>
> So, choose your poison:-)
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 12/57] arm: mach-shmobile: Add LCDC tx_dev field to
From: Guennadi Liakhovetski @ 2011-12-19 0:27 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1323784972-24205-13-git-send-email-laurent.pinchart@ideasonboard.com>
On Fri, 16 Dec 2011, Laurent Pinchart wrote:
> Hi Guennadi,
>
> On Thursday 15 December 2011 15:01:27 Guennadi Liakhovetski wrote:
> > On Tue, 13 Dec 2011, Laurent Pinchart wrote:
> > > Make sure the transmitter devices get registered before the associated
> > > LCDC devices.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> [snip]
>
> > > diff --git a/arch/arm/mach-shmobile/board-ap4evb.c
> > > b/arch/arm/mach-shmobile/board-ap4evb.c index dca4860..e0a6b3d 100644
> > > --- a/arch/arm/mach-shmobile/board-ap4evb.c
> > > +++ b/arch/arm/mach-shmobile/board-ap4evb.c
>
> [snip]
>
> > > @@ -627,6 +553,86 @@ static struct platform_device *qhd_devices[]
> > > __initdata = {
> > >
> > > };
> > > #endif /* CONFIG_AP4EVB_QHD */
> > >
> > > +/* LCDC0 */
> > > +static const struct fb_videomode ap4evb_lcdc_modes[] = {
> >
> > Hm, I must be missing something. I thought you were moving all the structs
> > around to make them available for reference _without_ forward
> > declarations. But you _do_ end up using a forward declaration anyway. Here
> > and for HDMI below too. So, what's the point? Why not just forward declare
> > that one struct, that's needed and leave the rest where it currently is
> > in the file?
>
> As we have circular dependencies forward declaration can't be avoided. I found
> it cleaner to have devices data in registration order in the board file, but I
> can remove this if you prefer.
As a _general_ guidline I find smaller patches more pleasant than
bigger;-) but if you really see an important advantage in moving these
structs around - I can live with that too:-)
> > > + {
> > > +#ifdef CONFIG_AP4EVB_QHD
> > > + .name = "R63302(QHD)",
> > > + .xres = 544,
> > > + .yres = 961,
> > > + .left_margin = 72,
> > > + .right_margin = 600,
> > > + .hsync_len = 16,
> > > + .upper_margin = 8,
> > > + .lower_margin = 8,
> > > + .vsync_len = 2,
> > > + .sync = FB_SYNC_VERT_HIGH_ACT | FB_SYNC_HOR_HIGH_ACT,
> > > +#else
> > > + .name = "WVGA Panel",
> > > + .xres = 800,
> > > + .yres = 480,
> > > + .left_margin = 220,
> > > + .right_margin = 110,
> > > + .hsync_len = 70,
> > > + .upper_margin = 20,
> > > + .lower_margin = 5,
> > > + .vsync_len = 5,
> > > + .sync = 0,
> > > +#endif
> > > + },
> > > +};
>
> [snip]
>
> > > diff --git a/arch/arm/mach-shmobile/board-mackerel.c
> > > b/arch/arm/mach-shmobile/board-mackerel.c index 4ed0138..ccdd9fc 100644
> > > --- a/arch/arm/mach-shmobile/board-mackerel.c
> > > +++ b/arch/arm/mach-shmobile/board-mackerel.c
>
> [snip]
>
> > > @@ -1313,8 +1317,8 @@ static struct platform_device *mackerel_devices[]
> > > __initdata = {
> > > &sh_mmcif_device,
> > > &ceu_device,
> > > &mackerel_camera,
> > > - &hdmi_lcdc_device,
> > > &hdmi_device,
> > > + &hdmi_lcdc_device,
> > > &meram_device,
> > > };
> >
> > Sorry, could you elaborate, why this is needed? If this is really
> > important, then I'd prefer to test this before committing. If OTOH this is
> > unimportant, and my assumption, that normally it's the platform driver
> > registration order, that's important, not device registration order, is
> > correct, then you don't have to swap the order?
>
> If both the transmitter and LCDC drivers are built-in, the device probe order
> is the device registration order. As we need the transmitter to be probed
> first, it needs to be registered first.
Even there I'm not quite sure about - I do remember having to fiddle with
the Makefiles to change the probe order, which followed the linkage order.
Could be, that this has changed since I last checked, but I'd really
double-check this.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply
* Re: [PATCH 24/57] fbdev: sh_mobile_lcdc: Return display connection state in display_on
From: Laurent Pinchart @ 2011-12-19 0:21 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1323784972-24205-25-git-send-email-laurent.pinchart@ideasonboard.com>
Hi Guennadi,
On Thursday 15 December 2011 18:11:03 Guennadi Liakhovetski wrote:
> On Tue, 13 Dec 2011, Laurent Pinchart wrote:
> > Return true if the display is connected and false otherwise. Set the fb
> > info state to FBINFO_STATE_SUSPENDED in the sh_mobile_lcdc driver when
> > the display is not connected.
>
> Hmm... I'm not sure I like functions, that return either a negative error,
> or _several_ non-negative success values. How about returning -ENODEV?
That would be returning a negative error on success. Is that better ? :-)
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH v4] Resurrect Intel740 driver: i740fb
From: Andrew Morton @ 2011-12-17 1:12 UTC (permalink / raw)
To: Ondrej Zary
Cc: linux-fbdev, Florian Tobias Schandinat, Paul Mundt,
Kernel development list
In-Reply-To: <201112080024.24383.linux@rainbow-software.org>
On Thu, 8 Dec 2011 00:24:18 +0100
Ondrej Zary <linux@rainbow-software.org> wrote:
> This is a resurrection of an old (like 2.4.19) out-of-tree driver for
> Intel740 graphics cards and modify it for recent kernels. The old driver is
> located at: http://sourceforge.net/projects/i740fbdev/files/
>
> This is a new driver based on skeletonfb, using most of the low level HW code
> from the old driver. The DDC code is completely new.
>
> The driver was tested on two 8MB cards: Protac AG240D and Diamond Stealth II
> G460.
>
> Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
The original driver appears to have been written by Andrey Ulanov. It
would be nice to mention that in the changelog and it is desirable that
you seek his Signed-off-by:, please.
> Changes in v4:
> - shortened many lines to fit 80-char limit
> - removed useless inb_p and outb_p functions
> - converted DDC (and some other) code to i740outreg_mask
> - removed useless wm initialization in i740_calc_fifo()
> - ALIGN macro is used instead of manual alignment
> - removed inactive code
> - simplified i740_setcolreg()
> - info->var used for panning
> - fixed out-of-video-memory checking
It still generates a large number of checkpatch errors. If you've
reviewed those and made the decision to ignore them then OK(ish).
Otherwise, please check.
>
> ...
>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/errno.h>
> +#include <linux/string.h>
> +#include <linux/mm.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/fb.h>
> +#include <linux/init.h>
> +#include <linux/pci.h>
> +#include <linux/pci_ids.h>
> +#include <linux/i2c.h>
> +#include <linux/i2c-algo-bit.h>
Should there have been a dependency on i2c in the Kconfig entry? If
not, has the driver been tested with is2 disabled?
> +#include <linux/console.h>
> +#include <video/vga.h>
> +
>
> ...
>
> +#define I740_RFREQ (1e6)
> +#define TARGET_MAX_N 30
> +
> +#define I740_FFIX 8
> +#define I740_REF_FREQ (u32) (66.66666666667 * (1 << I740_FFIX) + 0.5)
> +#define I740_MAX_VCO_FREQ (u32)(450.00000000000 * (1 << I740_FFIX) + 0.5)
There should be no floating point in the kernel, normally. Given the
casts, these will obviously not get any further than the compiler. But
if these could be redone to avoid the float, that would be nice.
> +#define I740_CALC_VCLKfix(m, n, p, d) ((((((m) * I740_REF_FREQ * (4 << ((d) << 1)))) / (n)) + ((1 << (p)) / 2)) / (1 << (p)))
This macro should be dragged out and shot. And it will cause bugs if
invoked using an expression with side-effects. There's no need to do
this to ourselves - please turn it into a nice, lower-case-named C
function with proper identifiers, code comments, etc.
> +static void i740_calc_vclk(u32 freq_hz, struct i740fb_par *par)
> +{
> + u32 freq = freq_hz / (u32)(1e6 / I740_RFREQ);
> + const u32 err_max > + freq / (u32)(I740_RFREQ / 0.005 / (1 << I740_FFIX) + 0.5);
> + const u32 err_target > + freq / (u32)(I740_RFREQ / 0.001 / (1 << I740_FFIX) + 0.5);
> + u32 err_best = (u32)(512.0 * (1 << I740_FFIX));
> + u32 f_err, f_vco;
> + int m_best = 0, n_best = 0, p_best = 0, d_best = 0;
> + int m, n, i;
> +
> + /* find log2(MAX_VCO_FREQ/f_target) */
> + for (i = 0; i < 16; i++)
> + if ((I740_MAX_VCO_FREQ) / (1 << i) < freq / (u32)(I740_RFREQ / (1 << I740_FFIX)))
Use the existing ilog2()?
> + break;
> + i--;
> + p_best = i;
> +
> + d_best = 0;
> + f_vco = (freq * (1 << p_best)) / (u32)(I740_RFREQ / (1 << I740_FFIX));
> + freq = freq / (u32)(I740_RFREQ / (1 << I740_FFIX));
> +
> + n = 2;
> + do {
> + n++;
> + m = ((f_vco * n) / I740_REF_FREQ + 2) / 4;
> +
> + if (m < 3)
> + m = 3;
> +
> + {
> + u32 f_out = I740_CALC_VCLKfix(m, n, p_best, d_best);
> +
> + f_err = (freq - f_out);
> +
> + if (abs(f_err) < err_max) {
> + m_best = m;
> + n_best = n;
> + err_best = f_err;
> + }
> + }
> + } while ((abs(f_err) >= err_target) &&
> + ((n <= TARGET_MAX_N) || (abs(err_best) > err_max)));
> +
> + if (abs(f_err) < err_target) {
> + m_best = m;
> + n_best = n;
> + }
> +
> + par->video_clk2_m = (m_best-2) & 0xFF;
> + par->video_clk2_n = (n_best-2) & 0xFF;
> + par->video_clk2_mn_msbs = ((((n_best-2) >> 4) & VCO_N_MSBS)
> + | (((m_best-2) >> 8) & VCO_M_MSBS));
> + par->video_clk2_div_sel > + ((p_best << 4) | (d_best ? 4 : 0) | REF_DIV_1);
> +}
> +
>
> ...
>
> +#ifndef MODULE
> +static int __init i740fb_setup(char *options)
> +{
> + char *opt;
> +
> + if (!options || !*options)
> + return 0;
> +
> + while ((opt = strsep(&options, ",")) != NULL) {
> + if (!*opt)
> + continue;
> +#ifdef CONFIG_MTRR
> + else if (!strncmp(opt, "mtrr:", 5))
> + mtrr = simple_strtoul(opt + 5, NULL, 0);
> +#endif
> + else
> + mode_option = opt;
> + }
> +
> + return 0;
> +}
> +#endif
> +
> +int __init i740fb_init(void)
> +{
> +#ifndef MODULE
> + char *option = NULL;
> +
> + if (fb_get_options("i740fb", &option))
> + return -ENODEV;
> + i740fb_setup(option);
> +#endif
> +
> + return pci_register_driver(&i740fb_driver);
> +}
hm, the "ifdef MODULE" stuff looks old-fashioned, but quite a few fbdev
drivers appear to be doing it.
>
> ...
>
Should we also be adding a documentation file for thei driver? Tell
people what the various module options do?
^ 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