[PATCH v3 0/12] Various s3c-fb updates

[PATCH v3 0/12] Various s3c-fb updates

[Pawel Osciak]

Hello,

this series is rebased onto Ben Dook's framebuffer branch available at:
git://git.fluff.org/bjdooks/linux.git dev/s3c-fb

The main changes are the addition of an ability to wait for VSYNC and
display panning and a better support for S5PV210 SoCs. A number of bugs
has also been fixed.

The first patch attempts to fix some NULL pointer dereferences in case
of a failed framebuffer memory allocation attempt. This is a quick fix,
so please treat it more as an indication of what may be wrong with the
code in the probe function and not as something to merge.

Patches 3-4 add "new style" device name initialization for various
S3C/S5P devices. The 4th patch also separates S5PC100 and S5PV210 as
their framebuffer registers sets differ.

Patches 8, 10 add shadow register update locking (S3C6410+)
Patch 12 Adds support for DMA channel control found on S5PV210+

Patches 7, 9 and 11 are pure bugfixes.

Changes in v3:
- new patches:
  * fixing a section mismatch error
  * adding support for DMA channel control (enable/disable) found on S5PV210
  * disabling shadow register updates during set_par

- unified function for disabling register updates
- no need to initialize names for 64xx-type framebuffers (is being set as
  default anyway)
- updated to the new version of Ben's branch
- minor fixes

Changes in v2:
- added SHADOWCON register support found on S3C6410 and S5PC100 for disabling
  shadow register updates
- fixed osd and alpha register handling (different configurations for different
  machines)

The series includes:
[PATCH v3 01/12] s3c-fb: Fix various null references on framebuffer memory alloc failure
[PATCH v3 02/12] s3c-fb: Correct FRAMESEL1 bitfield defines for VIDINTCON0 register
[PATCH v3 03/12] s3c-fb: Separate S5PC100 and S5PV210 framebuffer driver data structures
[PATCH v3 04/12] s3c-fb: Add device name initialization
[PATCH v3 05/12] s3c-fb: Add support for display panning
[PATCH v3 06/12] s3c-fb: Add wait for VSYNC ioctl
[PATCH v3 07/12] s3c-fb: window 3 of 64xx+ does not have an osd_d register
[PATCH v3 08/12] s3c-fb: Add SHADOWCON shadow register locking support for S5PV210
[PATCH v3 09/12] s3c-fb: Correct window osd size and alpha register handling
[PATCH v3 10/12] s3c-fb: Protect window-specific registers during updates
[PATCH v3 11/12] s3c-fb: fix section mismatch
[PATCH v3 12/12] s3c-fb: Add support for DMA channel control on S5PV210

Best regards
--
Pawel Osciak
Linux Platform Group
Samsung Poland R&D Center

[PATCH v3 01/12] s3c-fb: Fix various null references on framebuffer

[Pawel Osciak]

The following problems were found in the above situation:

sfb->windows[win] was being assigned at the end of s3c_fb_probe_win only.
This resulted in passing a NULL to s3c_fb_release_win if probe_win returned
early and a memory leak.

dma_free_writecombine does not allow its third argument to be NULL.

fb_dealloc_cmap does not verify whether its argument is not NULL.

Signed-off-by: Pawel Osciak <p.osciak@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/video/s3c-fb.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
index b00c064..d998324 100644
--- a/drivers/video/s3c-fb.c
+++ b/drivers/video/s3c-fb.c
@@ -804,7 +804,8 @@ static void s3c_fb_free_memory(struct s3c_fb *sfb, struct s3c_fb_win *win)
 {
 	struct fb_info *fbi = win->fbinfo;
 
-	dma_free_writecombine(sfb->dev, PAGE_ALIGN(fbi->fix.smem_len),
+	if (fbi->screen_base)
+		dma_free_writecombine(sfb->dev, PAGE_ALIGN(fbi->fix.smem_len),
 			      fbi->screen_base, fbi->fix.smem_start);
 }
 
@@ -819,7 +820,8 @@ static void s3c_fb_release_win(struct s3c_fb *sfb, struct s3c_fb_win *win)
 {
 	if (win->fbinfo) {
 		unregister_framebuffer(win->fbinfo);
-		fb_dealloc_cmap(&win->fbinfo->cmap);
+		if (&win->fbinfo->cmap)
+			fb_dealloc_cmap(&win->fbinfo->cmap);
 		s3c_fb_free_memory(sfb, win);
 		framebuffer_release(win->fbinfo);
 	}
@@ -865,6 +867,7 @@ static int __devinit s3c_fb_probe_win(struct s3c_fb *sfb, unsigned int win_no,
 	WARN_ON(windata->win_mode.yres = 0);
 
 	win = fbinfo->par;
+	*res = win;
 	var = &fbinfo->var;
 	win->variant = *variant;
 	win->fbinfo = fbinfo;
@@ -939,7 +942,6 @@ static int __devinit s3c_fb_probe_win(struct s3c_fb *sfb, unsigned int win_no,
 		return ret;
 	}
 
-	*res = win;
 	dev_info(sfb->dev, "window %d: fb %s\n", win_no, fbinfo->fix.id);
 
 	return 0;
-- 
1.7.1.569.g6f426

[PATCH v3 02/12] s3c-fb: Correct FRAMESEL1 bitfield defines for

[Pawel Osciak]

FRAMESEL1 bitfield starts on 13th bit, not on 14th.

Signed-off-by: Pawel Osciak <p.osciak@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/plat-samsung/include/plat/regs-fb.h |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/plat-samsung/include/plat/regs-fb.h b/arch/arm/plat-samsung/include/plat/regs-fb.h
index f4259e5..ac10013 100644
--- a/arch/arm/plat-samsung/include/plat/regs-fb.h
+++ b/arch/arm/plat-samsung/include/plat/regs-fb.h
@@ -292,11 +292,11 @@
 #define VIDINTCON0_FRAMESEL0_ACTIVE		(0x2 << 15)
 #define VIDINTCON0_FRAMESEL0_FRONTPORCH		(0x3 << 15)
 
-#define VIDINTCON0_FRAMESEL1			(1 << 14)
-#define VIDINTCON0_FRAMESEL1_NONE		(0x0 << 14)
-#define VIDINTCON0_FRAMESEL1_BACKPORCH		(0x1 << 14)
-#define VIDINTCON0_FRAMESEL1_VSYNC		(0x2 << 14)
-#define VIDINTCON0_FRAMESEL1_FRONTPORCH		(0x3 << 14)
+#define VIDINTCON0_FRAMESEL1			(1 << 13)
+#define VIDINTCON0_FRAMESEL1_NONE		(0x0 << 13)
+#define VIDINTCON0_FRAMESEL1_BACKPORCH		(0x1 << 13)
+#define VIDINTCON0_FRAMESEL1_VSYNC		(0x2 << 13)
+#define VIDINTCON0_FRAMESEL1_FRONTPORCH		(0x3 << 13)
 
 #define VIDINTCON0_INT_FRAME			(1 << 12)
 #define VIDINTCON0_FIFIOSEL_MASK		(0x7f << 5)
-- 
1.7.1.569.g6f426

[PATCH v3 03/12] s3c-fb: Separate S5PC100 and S5PV210 framebuffer

[Pawel Osciak]

S5PC100 and S5PV210 framebuffer devices differ slightly in terms of
available registers and their driver data structures have to be separate.

Signed-off-by: Pawel Osciak <p.osciak@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/video/s3c-fb.c |   37 ++++++++++++++++++++++++++++++++++---
 1 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
index d998324..59ac76a 100644
--- a/drivers/video/s3c-fb.c
+++ b/drivers/video/s3c-fb.c
@@ -1251,7 +1251,35 @@ static struct s3c_fb_driverdata s3c_fb_data_64xx __devinitdata = {
 	.win[4]	= &s3c_fb_data_64xx_wins[4],
 };
 
-static struct s3c_fb_driverdata s3c_fb_data_s5p __devinitdata = {
+static struct s3c_fb_driverdata s3c_fb_data_s5pc100 __devinitdata = {
+	.variant = {
+		.nr_windows	= 5,
+		.vidtcon	= VIDTCON0,
+		.wincon		= WINCON(0),
+		.winmap		= WINxMAP(0),
+		.keycon		= WKEYCON,
+		.osd		= VIDOSD_BASE,
+		.osd_stride	= 16,
+		.buf_start	= VIDW_BUF_START(0),
+		.buf_size	= VIDW_BUF_SIZE(0),
+		.buf_end	= VIDW_BUF_END(0),
+
+		.palette = {
+			[0] = 0x2400,
+			[1] = 0x2800,
+			[2] = 0x2c00,
+			[3] = 0x3000,
+			[4] = 0x3400,
+		},
+	},
+	.win[0]	= &s3c_fb_data_64xx_wins[0],
+	.win[1]	= &s3c_fb_data_64xx_wins[1],
+	.win[2]	= &s3c_fb_data_64xx_wins[2],
+	.win[3]	= &s3c_fb_data_64xx_wins[3],
+	.win[4]	= &s3c_fb_data_64xx_wins[4],
+};
+
+static struct s3c_fb_driverdata s3c_fb_data_s5pv210 __devinitdata = {
 	.variant = {
 		.nr_windows	= 5,
 		.vidtcon	= VIDTCON0,
@@ -1319,8 +1347,11 @@ static struct platform_device_id s3c_fb_driver_ids[] = {
 		.name		= "s3c-fb",
 		.driver_data	= (unsigned long)&s3c_fb_data_64xx,
 	}, {
-		.name		= "s5p-fb",
-		.driver_data	= (unsigned long)&s3c_fb_data_s5p,
+		.name		= "s5pc100-fb",
+		.driver_data	= (unsigned long)&s3c_fb_data_s5pc100,
+	}, {
+		.name		= "s5pv210-fb",
+		.driver_data	= (unsigned long)&s3c_fb_data_s5pv210,
 	}, {
 		.name		= "s3c2443-fb",
 		.driver_data	= (unsigned long)&s3c_fb_data_s3c2443,
-- 
1.7.1.569.g6f426

[PATCH v3 04/12] s3c-fb: Add device name initialization

[Pawel Osciak]

Add framebuffer device name initialization calls for S3C2443, S3C64xx
and S5P machines.

Signed-off-by: Pawel Osciak <p.osciak@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/mach-s3c2416/s3c2416.c              |    3 +-
 arch/arm/mach-s3c2443/s3c2443.c              |    2 +
 arch/arm/mach-s5pc100/cpu.c                  |    3 ++
 arch/arm/mach-s5pv210/cpu.c                  |    7 ++---
 arch/arm/plat-samsung/include/plat/fb-core.h |   29 ++++++++++++++++++++++++++
 5 files changed, 39 insertions(+), 5 deletions(-)
 create mode 100644 arch/arm/plat-samsung/include/plat/fb-core.h

diff --git a/arch/arm/mach-s3c2416/s3c2416.c b/arch/arm/mach-s3c2416/s3c2416.c
index 35dabcc..bc30245 100644
--- a/arch/arm/mach-s3c2416/s3c2416.c
+++ b/arch/arm/mach-s3c2416/s3c2416.c
@@ -55,6 +55,7 @@
 #include <plat/cpu.h>
 
 #include <plat/iic-core.h>
+#include <plat/fb-core.h>
 
 static struct map_desc s3c2416_iodesc[] __initdata = {
 	IODESC_ENT(WATCHDOG),
@@ -90,7 +91,7 @@ int __init s3c2416_init(void)
 	s3c_i2c0_setname("s3c2440-i2c");
 	s3c_i2c1_setname("s3c2440-i2c");
 
-	s3c_device_fb.name = "s3c2443-fb";
+	s3c_fb_setname("s3c2443-fb");
 
 	return sysdev_register(&s3c2416_sysdev);
 }
diff --git a/arch/arm/mach-s3c2443/s3c2443.c b/arch/arm/mach-s3c2443/s3c2443.c
index ce2ec32..839b6b2 100644
--- a/arch/arm/mach-s3c2443/s3c2443.c
+++ b/arch/arm/mach-s3c2443/s3c2443.c
@@ -35,6 +35,7 @@
 #include <plat/s3c2443.h>
 #include <plat/devs.h>
 #include <plat/cpu.h>
+#include <plat/fb-core.h>
 
 static struct map_desc s3c2443_iodesc[] __initdata = {
 	IODESC_ENT(WATCHDOG),
@@ -62,6 +63,7 @@ int __init s3c2443_init(void)
 	s3c24xx_reset_hook = s3c2443_hard_reset;
 
 	s3c_device_nand.name = "s3c2412-nand";
+	s3c_fb_setname("s3c2443-fb");
 
 	/* change WDT IRQ number */
 	s3c_device_wdt.resource[1].start = IRQ_S3C2443_WDT;
diff --git a/arch/arm/mach-s5pc100/cpu.c b/arch/arm/mach-s5pc100/cpu.c
index 7b5bdbc..d10276a 100644
--- a/arch/arm/mach-s5pc100/cpu.c
+++ b/arch/arm/mach-s5pc100/cpu.c
@@ -41,6 +41,7 @@
 #include <plat/iic-core.h>
 #include <plat/sdhci.h>
 #include <plat/onenand-core.h>
+#include <plat/fb-core.h>
 
 #include <plat/s5pc100.h>
 
@@ -92,6 +93,8 @@ void __init s5pc100_map_io(void)
 	s3c_i2c1_setname("s3c2440-i2c");
 
 	s3c_onenand_setname("s5pc100-onenand");
+
+	s3c_fb_setname("s5pc100-fb");
 }
 
 void __init s5pc100_init_clocks(int xtal)
diff --git a/arch/arm/mach-s5pv210/cpu.c b/arch/arm/mach-s5pv210/cpu.c
index f42c507..4d3580a 100644
--- a/arch/arm/mach-s5pv210/cpu.c
+++ b/arch/arm/mach-s5pv210/cpu.c
@@ -34,6 +34,7 @@
 #include <plat/s5pv210.h>
 #include <plat/iic-core.h>
 #include <plat/sdhci.h>
+#include <plat/fb-core.h>
 
 /* Initial IO mappings */
 
@@ -80,10 +81,6 @@ void __init s5pv210_map_io(void)
 	s3c_device_adc.name	= "s3c64xx-adc";
 #endif
 
-#ifdef CONFIG_S3C_DEV_FB
-	s3c_device_fb.name	= "s5p-fb";
-#endif
-
 	iotable_init(s5pv210_iodesc, ARRAY_SIZE(s5pv210_iodesc));
 
 	/* initialise device information early */
@@ -95,6 +92,8 @@ void __init s5pv210_map_io(void)
 	s3c_i2c0_setname("s3c2440-i2c");
 	s3c_i2c1_setname("s3c2440-i2c");
 	s3c_i2c2_setname("s3c2440-i2c");
+
+	s3c_fb_setname("s5pv210-fb");
 }
 
 void __init s5pv210_init_clocks(int xtal)
diff --git a/arch/arm/plat-samsung/include/plat/fb-core.h b/arch/arm/plat-samsung/include/plat/fb-core.h
new file mode 100644
index 0000000..bca383e
--- /dev/null
+++ b/arch/arm/plat-samsung/include/plat/fb-core.h
@@ -0,0 +1,29 @@
+/*
+ * arch/arm/plat-samsung/include/plat/fb-core.h
+ *
+ * Copyright 2010 Samsung Electronics Co., Ltd.
+ *	Pawel Osciak <p.osciak@samsung.com>
+ *
+ * Samsung framebuffer driver core functions
+ *
+ * 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.
+ */
+#ifndef __ASM_PLAT_FB_CORE_H
+#define __ASM_PLAT_FB_CORE_H __FILE__
+
+/*
+ * These functions are only for use with the core support code, such as
+ * the CPU-specific initialization code.
+ */
+
+/* Re-define device name depending on support. */
+static inline void s3c_fb_setname(char *name)
+{
+#ifdef CONFIG_S3C_DEV_FB
+	s3c_device_fb.name = name;
+#endif
+}
+
+#endif /* __ASM_PLAT_FB_CORE_H */
-- 
1.7.1.569.g6f426

[PATCH v3 05/12] s3c-fb: Add support for display panning

[Pawel Osciak]

Supports all bpp modes.

The PRTCON register is used to disable in-hardware updates of registers
that store start and end addresses of framebuffer memory. This prevents
display corruption in case we do not make it before VSYNC with updating
them atomically. With this feature there is no need to wait for a VSYNC
interrupt before each such update.

Signed-off-by: Pawel Osciak <p.osciak@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/plat-samsung/include/plat/regs-fb.h |    5 ++
 drivers/video/s3c-fb.c                       |   71 ++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-samsung/include/plat/regs-fb.h b/arch/arm/plat-samsung/include/plat/regs-fb.h
index ac10013..f454e32 100644
--- a/arch/arm/plat-samsung/include/plat/regs-fb.h
+++ b/arch/arm/plat-samsung/include/plat/regs-fb.h
@@ -112,6 +112,11 @@
 #define VIDCON2_ORGYCbCr			(1 << 8)
 #define VIDCON2_YUVORDCrCb			(1 << 7)
 
+/* PRTCON (S3C6410, S5PC100) */
+
+#define PRTCON					(0x0c)
+#define PRTCON_PROTECT				(1 << 11)
+
 /* VIDTCON0 */
 
 #define VIDTCON0_VBPDE_MASK			(0xff << 24)
diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
index 59ac76a..4f3680d 100644
--- a/drivers/video/s3c-fb.c
+++ b/drivers/video/s3c-fb.c
@@ -71,6 +71,7 @@ struct s3c_fb;
  * @buf_end: Offset of buffer end registers.
  * @osd: The base for the OSD registers.
  * @palette: Address of palette memory, or 0 if none.
+ * @has_prtcon: Set if has PRTCON register.
  */
 struct s3c_fb_variant {
 	unsigned int	is_2443:1;
@@ -85,6 +86,8 @@ struct s3c_fb_variant {
 	unsigned short	osd;
 	unsigned short	osd_stride;
 	unsigned short	palette[S3C_FB_MAX_WIN];
+
+	unsigned int	has_prtcon:1;
 };
 
 /**
@@ -379,6 +382,9 @@ static int s3c_fb_set_par(struct fb_info *info)
 
 	info->fix.line_length = (var->xres_virtual * var->bits_per_pixel) / 8;
 
+	info->fix.xpanstep = info->var.xres_virtual > info->var.xres ? 1 : 0;
+	info->fix.ypanstep = info->var.yres_virtual > info->var.yres ? 1 : 0;
+
 	/* disable the window whilst we update it */
 	writel(0, regs + WINCON(win_no));
 
@@ -735,6 +741,66 @@ static int s3c_fb_blank(int blank_mode, struct fb_info *info)
 	return 0;
 }
 
+/**
+ * s3c_fb_pan_display() - Pan the display.
+ *
+ * Note that the offsets can be written to the device at any time, as their
+ * values are latched at each vsync automatically. This also means that only
+ * the last call to this function will have any effect on next vsync, but
+ * there is no need to sleep waiting for it to prevent tearing.
+ *
+ * @var: The screen information to verify.
+ * @info: The framebuffer device.
+ */
+static int s3c_fb_pan_display(struct fb_var_screeninfo *var,
+			      struct fb_info *info)
+{
+	struct s3c_fb_win *win	= info->par;
+	struct s3c_fb *sfb	= win->parent;
+	void __iomem *buf	= sfb->regs + win->index * 8;
+	unsigned int start_byte_offset, end_byte_offset;
+
+	/* Offset in bytes to the start of the displayed area */
+	start_byte_offset = var->yoffset * info->fix.line_length;
+	/* X offset depends on the current bpp */
+	if (info->var.bits_per_pixel >= 8) {
+		start_byte_offset ++			var->xoffset * (info->var.bits_per_pixel >> 3);
+	} else {
+		switch (info->var.bits_per_pixel) {
+		case 4:
+			start_byte_offset += var->xoffset >> 1;
+			break;
+		case 2:
+			start_byte_offset += var->xoffset >> 2;
+			break;
+		case 1:
+			start_byte_offset += var->xoffset >> 3;
+			break;
+		default:
+			dev_err(sfb->dev, "invalid bpp\n");
+			return -EINVAL;
+		}
+	}
+	/* Offset in bytes to the end of the displayed area */
+	end_byte_offset = start_byte_offset + var->yres * info->fix.line_length;
+
+	/* Temporarily turn off per-vsync update from shadow registers until
+	 * both start and end addresses are updated to prevent corruption */
+	if (sfb->variant.has_prtcon)
+		writel(PRTCON_PROTECT, sfb->regs + PRTCON);
+
+	writel(info->fix.smem_start + start_byte_offset,
+		buf + sfb->variant.buf_start);
+	writel(info->fix.smem_start + end_byte_offset,
+		buf + sfb->variant.buf_end);
+
+	if (sfb->variant.has_prtcon)
+		writel(0, sfb->regs + PRTCON);
+
+	return 0;
+}
+
 static struct fb_ops s3c_fb_ops = {
 	.owner		= THIS_MODULE,
 	.fb_check_var	= s3c_fb_check_var,
@@ -744,6 +810,7 @@ static struct fb_ops s3c_fb_ops = {
 	.fb_fillrect	= cfb_fillrect,
 	.fb_copyarea	= cfb_copyarea,
 	.fb_imageblit	= cfb_imageblit,
+	.fb_pan_display	= s3c_fb_pan_display,
 };
 
 /**
@@ -1243,6 +1310,8 @@ static struct s3c_fb_driverdata s3c_fb_data_64xx __devinitdata = {
 			[3] = 0x320,
 			[4] = 0x340,
 		},
+
+		.has_prtcon	= 1,
 	},
 	.win[0]	= &s3c_fb_data_64xx_wins[0],
 	.win[1]	= &s3c_fb_data_64xx_wins[1],
@@ -1271,6 +1340,8 @@ static struct s3c_fb_driverdata s3c_fb_data_s5pc100 __devinitdata = {
 			[3] = 0x3000,
 			[4] = 0x3400,
 		},
+
+		.has_prtcon	= 1,
 	},
 	.win[0]	= &s3c_fb_data_64xx_wins[0],
 	.win[1]	= &s3c_fb_data_64xx_wins[1],
-- 
1.7.1.569.g6f426

[PATCH v3 06/12] s3c-fb: Add wait for VSYNC ioctl

[Pawel Osciak]

Add VSYNC interrupt support and an ioctl that allows waiting for it.
Interrupts are turned on only when needed.

Signed-off-by: Pawel Osciak <p.osciak@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/plat-samsung/include/plat/regs-fb.h |    1 +
 drivers/video/s3c-fb.c                       |  167 +++++++++++++++++++++++++-
 2 files changed, 167 insertions(+), 1 deletions(-)

diff --git a/arch/arm/plat-samsung/include/plat/regs-fb.h b/arch/arm/plat-samsung/include/plat/regs-fb.h
index f454e32..5bcdd09 100644
--- a/arch/arm/plat-samsung/include/plat/regs-fb.h
+++ b/arch/arm/plat-samsung/include/plat/regs-fb.h
@@ -298,6 +298,7 @@
 #define VIDINTCON0_FRAMESEL0_FRONTPORCH		(0x3 << 15)
 
 #define VIDINTCON0_FRAMESEL1			(1 << 13)
+#define VIDINTCON0_FRAMESEL1_MASK		(0x3 << 13)
 #define VIDINTCON0_FRAMESEL1_NONE		(0x0 << 13)
 #define VIDINTCON0_FRAMESEL1_BACKPORCH		(0x1 << 13)
 #define VIDINTCON0_FRAMESEL1_VSYNC		(0x2 << 13)
diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
index 4f3680d..6131ebb 100644
--- a/drivers/video/s3c-fb.c
+++ b/drivers/video/s3c-fb.c
@@ -21,6 +21,8 @@
 #include <linux/clk.h>
 #include <linux/fb.h>
 #include <linux/io.h>
+#include <linux/uaccess.h>
+#include <linux/interrupt.h>
 
 #include <mach/map.h>
 #include <plat/regs-fb-v4.h>
@@ -48,6 +50,11 @@
 	__raw_writel(v, r); } while(0)
 #endif /* FB_S3C_DEBUG_REGWRITE */
 
+/* irq_flags bits */
+#define S3C_FB_VSYNC_IRQ_EN	0
+
+#define VSYNC_TIMEOUT_MSEC 50
+
 struct s3c_fb;
 
 #define VALID_BPP(x) (1 << ((x) - 1))
@@ -156,6 +163,16 @@ struct s3c_fb_win {
 };
 
 /**
+ * struct s3c_fb_vsync - vsync information
+ * @wait:	a queue for processes waiting for vsync
+ * @count:	vsync interrupt count
+ */
+struct s3c_fb_vsync {
+	wait_queue_head_t	wait;
+	unsigned int		count;
+};
+
+/**
  * struct s3c_fb - overall hardware state of the hardware
  * @dev: The device that we bound to, for printing, etc.
  * @regs_res: The resource we claimed for the IO registers.
@@ -165,6 +182,9 @@ struct s3c_fb_win {
  * @enabled: A bitmask of enabled hardware windows.
  * @pdata: The platform configuration data passed with the device.
  * @windows: The hardware windows that have been claimed.
+ * @irq_no: IRQ line number
+ * @irq_flags: irq flags
+ * @vsync_info: VSYNC-related information (count, queues...)
  */
 struct s3c_fb {
 	struct device		*dev;
@@ -177,6 +197,10 @@ struct s3c_fb {
 
 	struct s3c_fb_platdata	*pdata;
 	struct s3c_fb_win	*windows[S3C_FB_MAX_WIN];
+
+	int			 irq_no;
+	unsigned long		 irq_flags;
+	struct s3c_fb_vsync	 vsync_info;
 };
 
 /**
@@ -801,6 +825,124 @@ static int s3c_fb_pan_display(struct fb_var_screeninfo *var,
 	return 0;
 }
 
+/**
+ * s3c_fb_enable_irq() - enable framebuffer interrupts
+ * @sfb: main hardware state
+ */
+static void s3c_fb_enable_irq(struct s3c_fb *sfb)
+{
+	void __iomem *regs = sfb->regs;
+	u32 irq_ctrl_reg;
+
+	if (!test_and_set_bit(S3C_FB_VSYNC_IRQ_EN, &sfb->irq_flags)) {
+		/* IRQ disabled, enable it */
+		irq_ctrl_reg = readl(regs + VIDINTCON0);
+
+		irq_ctrl_reg |= VIDINTCON0_INT_ENABLE;
+		irq_ctrl_reg |= VIDINTCON0_INT_FRAME;
+
+		irq_ctrl_reg &= ~VIDINTCON0_FRAMESEL0_MASK;
+		irq_ctrl_reg |= VIDINTCON0_FRAMESEL0_VSYNC;
+		irq_ctrl_reg &= ~VIDINTCON0_FRAMESEL1_MASK;
+		irq_ctrl_reg |= VIDINTCON0_FRAMESEL1_NONE;
+
+		writel(irq_ctrl_reg, regs + VIDINTCON0);
+	}
+}
+
+/**
+ * s3c_fb_disable_irq() - disable framebuffer interrupts
+ * @sfb: main hardware state
+ */
+static void s3c_fb_disable_irq(struct s3c_fb *sfb)
+{
+	void __iomem *regs = sfb->regs;
+	u32 irq_ctrl_reg;
+
+	if (test_and_clear_bit(S3C_FB_VSYNC_IRQ_EN, &sfb->irq_flags)) {
+		/* IRQ enabled, disable it */
+		irq_ctrl_reg = readl(regs + VIDINTCON0);
+
+		irq_ctrl_reg &= ~VIDINTCON0_INT_FRAME;
+		irq_ctrl_reg &= ~VIDINTCON0_INT_ENABLE;
+
+		writel(irq_ctrl_reg, regs + VIDINTCON0);
+	}
+}
+
+static irqreturn_t s3c_fb_irq(int irq, void *dev_id)
+{
+	struct s3c_fb *sfb = dev_id;
+	void __iomem  *regs = sfb->regs;
+	u32 irq_sts_reg;
+
+	irq_sts_reg = readl(regs + VIDINTCON1);
+
+	if (irq_sts_reg & VIDINTCON1_INT_FRAME) {
+
+		/* VSYNC interrupt, accept it */
+		writel(VIDINTCON1_INT_FRAME, regs + VIDINTCON1);
+
+		sfb->vsync_info.count++;
+		wake_up_interruptible(&sfb->vsync_info.wait);
+	}
+
+	/* We only support waiting for VSYNC for now, so it's safe
+	 * to always disable irqs here.
+	 */
+	s3c_fb_disable_irq(sfb);
+
+	return IRQ_HANDLED;
+}
+
+/**
+ * s3c_fb_wait_for_vsync() - sleep until next VSYNC interrupt or timeout
+ * @sfb: main hardware state
+ * @crtc: head index.
+ */
+static int s3c_fb_wait_for_vsync(struct s3c_fb *sfb, u32 crtc)
+{
+	unsigned long count;
+	int ret;
+
+	if (crtc != 0)
+		return -ENODEV;
+
+	s3c_fb_enable_irq(sfb);
+	count = sfb->vsync_info.count;
+	ret = wait_event_interruptible_timeout(sfb->vsync_info.wait,
+				       count != sfb->vsync_info.count,
+				       msecs_to_jiffies(VSYNC_TIMEOUT_MSEC));
+	if (ret = 0)
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
+static int s3c_fb_ioctl(struct fb_info *info, unsigned int cmd,
+			unsigned long arg)
+{
+	struct s3c_fb_win *win = info->par;
+	struct s3c_fb *sfb = win->parent;
+	int ret;
+	u32 crtc;
+
+	switch (cmd) {
+	case FBIO_WAITFORVSYNC:
+		if (get_user(crtc, (u32 __user *)arg)) {
+			ret = -EFAULT;
+			break;
+		}
+
+		ret = s3c_fb_wait_for_vsync(sfb, crtc);
+		break;
+	default:
+		ret = -ENOTTY;
+	}
+
+	return ret;
+}
+
 static struct fb_ops s3c_fb_ops = {
 	.owner		= THIS_MODULE,
 	.fb_check_var	= s3c_fb_check_var,
@@ -811,6 +953,7 @@ static struct fb_ops s3c_fb_ops = {
 	.fb_copyarea	= cfb_copyarea,
 	.fb_imageblit	= cfb_imageblit,
 	.fb_pan_display	= s3c_fb_pan_display,
+	.fb_ioctl	= s3c_fb_ioctl,
 };
 
 /**
@@ -917,6 +1060,8 @@ static int __devinit s3c_fb_probe_win(struct s3c_fb *sfb, unsigned int win_no,
 
 	dev_dbg(sfb->dev, "probing window %d, variant %p\n", win_no, variant);
 
+	init_waitqueue_head(&sfb->vsync_info.wait);
+
 	palette_size = variant->palette_sz * 4;
 
 	fbinfo = framebuffer_alloc(sizeof(struct s3c_fb_win) +
@@ -1096,6 +1241,20 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)
 		goto err_req_region;
 	}
 
+	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (!res) {
+		dev_err(dev, "failed to acquire irq resource\n");
+		ret = -ENOENT;
+		goto err_ioremap;
+	}
+	sfb->irq_no = res->start;
+	ret = request_irq(sfb->irq_no, s3c_fb_irq,
+			  0, "s3c_fb", sfb);
+	if (ret) {
+		dev_err(dev, "irq request failed\n");
+		goto err_ioremap;
+	}
+
 	dev_dbg(dev, "got resources (regs %p), probing windows\n", sfb->regs);
 
 	/* setup gpio and output polarity controls */
@@ -1130,7 +1289,7 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)
 			dev_err(dev, "failed to create window %d\n", win);
 			for (; win >= 0; win--)
 				s3c_fb_release_win(sfb, sfb->windows[win]);
-			goto err_ioremap;
+			goto err_irq;
 		}
 	}
 
@@ -1138,6 +1297,9 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)
 
 	return 0;
 
+err_irq:
+	free_irq(sfb->irq_no, sfb);
+
 err_ioremap:
 	iounmap(sfb->regs);
 
@@ -1170,6 +1332,8 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)
 		if (sfb->windows[win])
 			s3c_fb_release_win(sfb, sfb->windows[win]);
 
+	free_irq(sfb->irq_no, sfb);
+
 	iounmap(sfb->regs);
 
 	clk_disable(sfb->bus_clk);
@@ -1370,6 +1534,7 @@ static struct s3c_fb_driverdata s3c_fb_data_s5pv210 __devinitdata = {
 			[3] = 0x3000,
 			[4] = 0x3400,
 		},
+
 	},
 	.win[0]	= &s3c_fb_data_64xx_wins[0],
 	.win[1]	= &s3c_fb_data_64xx_wins[1],
-- 
1.7.1.569.g6f426

[PATCH v3 07/12] s3c-fb: window 3 of 64xx+ does not have an osd_d

[Pawel Osciak]

Signed-off-by: Pawel Osciak <p.osciak@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/video/s3c-fb.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
index 6131ebb..0a93fca 100644
--- a/drivers/video/s3c-fb.c
+++ b/drivers/video/s3c-fb.c
@@ -1437,7 +1437,6 @@ static struct s3c_fb_win_variant s3c_fb_data_64xx_wins[] __devinitdata = {
 	},
 	[3] = {
 		.has_osd_c	= 1,
-		.has_osd_d	= 1,
 		.palette_sz	= 16,
 		.palette_16bpp	= 1,
 		.valid_bpp	= (VALID_BPP124  | VALID_BPP(16) |
-- 
1.7.1.569.g6f426

[PATCH v3 08/12] s3c-fb: Add SHADOWCON shadow register locking support

[Pawel Osciak]

S5PV210 allows per-window locking of register value updates from shadow
registers.

Signed-off-by: Pawel Osciak <p.osciak@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/plat-samsung/include/plat/regs-fb.h |    3 ++
 drivers/video/s3c-fb.c                       |   39 +++++++++++++++++++++++--
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/arch/arm/plat-samsung/include/plat/regs-fb.h b/arch/arm/plat-samsung/include/plat/regs-fb.h
index 5bcdd09..da54b64 100644
--- a/arch/arm/plat-samsung/include/plat/regs-fb.h
+++ b/arch/arm/plat-samsung/include/plat/regs-fb.h
@@ -218,6 +218,9 @@
 #define WINCON1_BPPMODE_25BPP_A1888		(0xd << 2)
 #define WINCON1_BPPMODE_28BPP_A4888		(0xd << 2)
 
+/* S5PV210 */
+#define SHADOWCON				(0x34)
+#define SHADOWCON_WINx_PROTECT(_win)		(1 << (10 + _win))
 
 #define VIDOSDxA_TOPLEFT_X_MASK			(0x7ff << 11)
 #define VIDOSDxA_TOPLEFT_X_SHIFT		(11)
diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
index 0a93fca..94423c5 100644
--- a/drivers/video/s3c-fb.c
+++ b/drivers/video/s3c-fb.c
@@ -79,6 +79,7 @@ struct s3c_fb;
  * @osd: The base for the OSD registers.
  * @palette: Address of palette memory, or 0 if none.
  * @has_prtcon: Set if has PRTCON register.
+ * @has_shadowcon: Set if has SHADOWCON register.
  */
 struct s3c_fb_variant {
 	unsigned int	is_2443:1;
@@ -95,6 +96,7 @@ struct s3c_fb_variant {
 	unsigned short	palette[S3C_FB_MAX_WIN];
 
 	unsigned int	has_prtcon:1;
+	unsigned int	has_shadowcon:1;
 };
 
 /**
@@ -363,6 +365,36 @@ static int s3c_fb_align_word(unsigned int bpp, unsigned int pix)
 }
 
 /**
+ * shadow_protect_win() - disable updating values from shadow registers at vsync
+ *
+ * @win: window to protect registers for
+ * @protect: 1 to protect (disable updates)
+ */
+static void shadow_protect_win(struct s3c_fb_win *win, int protect)
+{
+	struct s3c_fb *sfb = win->parent;
+	u32 reg;
+
+	if (protect) {
+		if (sfb->variant.has_prtcon) {
+			writel(PRTCON_PROTECT, sfb->regs + PRTCON);
+		} else if (sfb->variant.has_shadowcon) {
+			reg = readl(sfb->regs + SHADOWCON);
+			writel(reg | SHADOWCON_WINx_PROTECT(win->index),
+				sfb->regs + SHADOWCON);
+		}
+	} else {
+		if (sfb->variant.has_prtcon) {
+			writel(0, sfb->regs + PRTCON);
+		} else if (sfb->variant.has_shadowcon) {
+			reg = readl(sfb->regs + SHADOWCON);
+			writel(reg & ~SHADOWCON_WINx_PROTECT(win->index),
+				sfb->regs + SHADOWCON);
+		}
+	}
+}
+
+/**
  * s3c_fb_set_par() - framebuffer request to set new framebuffer state.
  * @info: The framebuffer to change.
  *
@@ -811,16 +843,14 @@ static int s3c_fb_pan_display(struct fb_var_screeninfo *var,
 
 	/* Temporarily turn off per-vsync update from shadow registers until
 	 * both start and end addresses are updated to prevent corruption */
-	if (sfb->variant.has_prtcon)
-		writel(PRTCON_PROTECT, sfb->regs + PRTCON);
+	shadow_protect_win(win, 1);
 
 	writel(info->fix.smem_start + start_byte_offset,
 		buf + sfb->variant.buf_start);
 	writel(info->fix.smem_start + end_byte_offset,
 		buf + sfb->variant.buf_end);
 
-	if (sfb->variant.has_prtcon)
-		writel(0, sfb->regs + PRTCON);
+	shadow_protect_win(win, 0);
 
 	return 0;
 }
@@ -1534,6 +1564,7 @@ static struct s3c_fb_driverdata s3c_fb_data_s5pv210 __devinitdata = {
 			[4] = 0x3400,
 		},
 
+		.has_shadowcon	= 1,
 	},
 	.win[0]	= &s3c_fb_data_64xx_wins[0],
 	.win[1]	= &s3c_fb_data_64xx_wins[1],
-- 
1.7.1.569.g6f426

[PATCH v3 09/12] s3c-fb: Correct window osd size and alpha register

[Pawel Osciak]

S3C64xx and S5P OSD registers for OSD size and alpha are as follows:
VIDOSDC: win 0 - size, win 1-4: alpha
VIDOSDD: win 1-2 - size; not present for windows 0, 3 and 4

Signed-off-by: Pawel Osciak <p.osciak@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/video/s3c-fb.c |   58 ++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 51 insertions(+), 7 deletions(-)

diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
index 94423c5..3b2c7fe 100644
--- a/drivers/video/s3c-fb.c
+++ b/drivers/video/s3c-fb.c
@@ -64,6 +64,9 @@ struct s3c_fb;
 #define VIDOSD_B(win, variant) (OSD_BASE(win, variant) + 0x04)
 #define VIDOSD_C(win, variant) (OSD_BASE(win, variant) + 0x08)
 #define VIDOSD_D(win, variant) (OSD_BASE(win, variant) + 0x0C)
+#define VIDOSD_SIZE(win, variant, win_variant) \
+	(OSD_BASE(win, variant) + (win_variant).osd_size_off)
+#define VIDOSD_ALPHA(win, variant, win_variant) VIDOSD_C(win, variant)
 
 /**
  * struct s3c_fb_variant - fb variant information
@@ -112,7 +115,10 @@ struct s3c_fb_variant {
 struct s3c_fb_win_variant {
 	unsigned int	has_osd_c:1;
 	unsigned int	has_osd_d:1;
+	unsigned int	has_osd_size:1;
+	unsigned int	has_osd_alpha:1;
 	unsigned int	palette_16bpp:1;
+	unsigned short	osd_size_off;
 	unsigned short	palette_sz;
 	u32		valid_bpp;
 };
@@ -365,6 +371,36 @@ static int s3c_fb_align_word(unsigned int bpp, unsigned int pix)
 }
 
 /**
+ * vidosd_set_size() - set OSD size for a window
+ *
+ * @win: the window to set OSD size for
+ * @size: OSD size register value
+ */
+static void vidosd_set_size(struct s3c_fb_win *win, u32 size)
+{
+	struct s3c_fb *sfb = win->parent;
+
+	if (win->variant.has_osd_size)
+		writel(size, sfb->regs + VIDOSD_SIZE(win->index, sfb->variant,
+							win->variant));
+}
+
+/**
+ * vidosd_set_alpha() - set alpha transparency for a window
+ *
+ * @win: the window to set OSD size for
+ * @alpha: alpha register value
+ */
+static void vidosd_set_alpha(struct s3c_fb_win *win, u32 alpha)
+{
+	struct s3c_fb *sfb = win->parent;
+
+	if (win->variant.has_osd_alpha)
+		writel(alpha, sfb->regs + VIDOSD_ALPHA(win->index,
+						sfb->variant, win->variant));
+}
+
+/**
  * shadow_protect_win() - disable updating values from shadow registers at vsync
  *
  * @win: window to protect registers for
@@ -408,7 +444,7 @@ static int s3c_fb_set_par(struct fb_info *info)
 	void __iomem *regs = sfb->regs;
 	void __iomem *buf = regs;
 	int win_no = win->index;
-	u32 osdc_data = 0;
+	u32 alpha = 0;
 	u32 data;
 	u32 pagewidth;
 	int clkdiv;
@@ -511,15 +547,12 @@ static int s3c_fb_set_par(struct fb_info *info)
 
 	data = var->xres * var->yres;
 
-	osdc_data = VIDISD14C_ALPHA1_R(0xf) |
+	alpha = VIDISD14C_ALPHA1_R(0xf) |
 		VIDISD14C_ALPHA1_G(0xf) |
 		VIDISD14C_ALPHA1_B(0xf);
 
-	if (win->variant.has_osd_d) {
-		writel(data, regs + VIDOSD_D(win_no, sfb->variant));
-		writel(osdc_data, regs + VIDOSD_C(win_no, sfb->variant));
-	} else
-		writel(data, regs + VIDOSD_C(win_no, sfb->variant));
+	vidosd_set_alpha(win, alpha);
+	vidosd_set_size(win, data);
 
 	data = WINCONx_ENWIN;
 
@@ -1445,12 +1478,17 @@ static int s3c_fb_resume(struct platform_device *pdev)
 static struct s3c_fb_win_variant s3c_fb_data_64xx_wins[] __devinitdata = {
 	[0] = {
 		.has_osd_c	= 1,
+		.has_osd_size	= 1,
+		.osd_size_off	= 0x8,
 		.palette_sz	= 256,
 		.valid_bpp	= VALID_BPP1248 | VALID_BPP(16) | VALID_BPP(24),
 	},
 	[1] = {
 		.has_osd_c	= 1,
 		.has_osd_d	= 1,
+		.has_osd_size	= 1,
+		.osd_size_off	= 0x12,
+		.has_osd_alpha	= 1,
 		.palette_sz	= 256,
 		.valid_bpp	= (VALID_BPP1248 | VALID_BPP(16) |
 				   VALID_BPP(18) | VALID_BPP(19) |
@@ -1459,6 +1497,9 @@ static struct s3c_fb_win_variant s3c_fb_data_64xx_wins[] __devinitdata = {
 	[2] = {
 		.has_osd_c	= 1,
 		.has_osd_d	= 1,
+		.has_osd_size	= 1,
+		.osd_size_off	= 0x12,
+		.has_osd_alpha	= 1,
 		.palette_sz	= 16,
 		.palette_16bpp	= 1,
 		.valid_bpp	= (VALID_BPP1248 | VALID_BPP(16) |
@@ -1467,6 +1508,7 @@ static struct s3c_fb_win_variant s3c_fb_data_64xx_wins[] __devinitdata = {
 	},
 	[3] = {
 		.has_osd_c	= 1,
+		.has_osd_alpha	= 1,
 		.palette_sz	= 16,
 		.palette_16bpp	= 1,
 		.valid_bpp	= (VALID_BPP124  | VALID_BPP(16) |
@@ -1475,6 +1517,7 @@ static struct s3c_fb_win_variant s3c_fb_data_64xx_wins[] __devinitdata = {
 	},
 	[4] = {
 		.has_osd_c	= 1,
+		.has_osd_alpha	= 1,
 		.palette_sz	= 4,
 		.palette_16bpp	= 1,
 		.valid_bpp	= (VALID_BPP(1) | VALID_BPP(2) |
@@ -1600,6 +1643,7 @@ static struct s3c_fb_driverdata s3c_fb_data_s3c2443 __devinitdata = {
 	},
 	.win[1] = &(struct s3c_fb_win_variant) {
 		.has_osd_c	= 1,
+		.has_osd_alpha	= 1,
 		.palette_sz	= 256,
 		.valid_bpp	= (VALID_BPP1248 | VALID_BPP(16) |
 				   VALID_BPP(18) | VALID_BPP(19) |
-- 
1.7.1.569.g6f426

[PATCH v3 10/12] s3c-fb: Protect window-specific registers during

[Pawel Osciak]

Newer hardware (S3C6410, S5P) have the ability to block updates from shadow
registers during reconfiguration. Add protect calls for set_par and clear
protection when resetting.

Signed-off-by: Pawel Osciak <p.osciak@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/video/s3c-fb.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
index 3b2c7fe..d461f1d 100644
--- a/drivers/video/s3c-fb.c
+++ b/drivers/video/s3c-fb.c
@@ -451,6 +451,8 @@ static int s3c_fb_set_par(struct fb_info *info)
 
 	dev_dbg(sfb->dev, "setting framebuffer parameters\n");
 
+	shadow_protect_win(win, 1);
+
 	switch (var->bits_per_pixel) {
 	case 32:
 	case 24:
@@ -633,6 +635,8 @@ static int s3c_fb_set_par(struct fb_info *info)
 	writel(data, regs + sfb->variant.wincon + (win_no * 4));
 	writel(0x0, regs + sfb->variant.winmap + (win_no * 4));
 
+	shadow_protect_win(win, 0);
+
 	return 0;
 }
 
@@ -1232,11 +1236,14 @@ static int __devinit s3c_fb_probe_win(struct s3c_fb *sfb, unsigned int win_no,
 static void s3c_fb_clear_win(struct s3c_fb *sfb, int win)
 {
 	void __iomem *regs = sfb->regs;
+	u32 reg;
 
 	writel(0, regs + sfb->variant.wincon + (win * 4));
 	writel(0, regs + VIDOSD_A(win, sfb->variant));
 	writel(0, regs + VIDOSD_B(win, sfb->variant));
 	writel(0, regs + VIDOSD_C(win, sfb->variant));
+	reg = readl(regs + SHADOWCON);
+	writel(reg & ~SHADOWCON_WINx_PROTECT(win), regs + SHADOWCON);
 }
 
 static int __devinit s3c_fb_probe(struct platform_device *pdev)
-- 
1.7.1.569.g6f426

[PATCH v3 11/12] s3c-fb: fix section mismatch

[Pawel Osciak]

From: Marek Szyprowski <m.szyprowski@samsung.com>

This patch fixes the following section mismatch errors:

WARNING: vmlinux.o(.data+0x20b40): Section mismatch in reference from the variable s3c_fb_driver_ids to the (unknown reference) .devinit.data:(unknown)
The variable s3c_fb_driver_ids references
the (unknown reference) __devinitdata (unknown)
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*driver, *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console,

WARNING: vmlinux.o(.data+0x20b58): Section mismatch in reference from the variable s3c_fb_driver_ids to the (unknown reference) .devinit.data:(unknown)
The variable s3c_fb_driver_ids references
the (unknown reference) __devinitdata (unknown)
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*driver, *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console,

WARNING: vmlinux.o(.data+0x20b70): Section mismatch in reference from the variable s3c_fb_driver_ids to the (unknown reference) .devinit.data:(unknown)
The variable s3c_fb_driver_ids references
the (unknown reference) __devinitdata (unknown)
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*driver, *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console,

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/video/s3c-fb.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
index d461f1d..de7334c 100644
--- a/drivers/video/s3c-fb.c
+++ b/drivers/video/s3c-fb.c
@@ -1482,7 +1482,7 @@ static int s3c_fb_resume(struct platform_device *pdev)
 #define VALID_BPP124 (VALID_BPP(1) | VALID_BPP(2) | VALID_BPP(4))
 #define VALID_BPP1248 (VALID_BPP124 | VALID_BPP(8))
 
-static struct s3c_fb_win_variant s3c_fb_data_64xx_wins[] __devinitdata = {
+static struct s3c_fb_win_variant s3c_fb_data_64xx_wins[] = {
 	[0] = {
 		.has_osd_c	= 1,
 		.has_osd_size	= 1,
@@ -1533,7 +1533,7 @@ static struct s3c_fb_win_variant s3c_fb_data_64xx_wins[] __devinitdata = {
 	},
 };
 
-static struct s3c_fb_driverdata s3c_fb_data_64xx __devinitdata = {
+static struct s3c_fb_driverdata s3c_fb_data_64xx = {
 	.variant = {
 		.nr_windows	= 5,
 		.vidtcon	= VIDTCON0,
@@ -1563,7 +1563,7 @@ static struct s3c_fb_driverdata s3c_fb_data_64xx __devinitdata = {
 	.win[4]	= &s3c_fb_data_64xx_wins[4],
 };
 
-static struct s3c_fb_driverdata s3c_fb_data_s5pc100 __devinitdata = {
+static struct s3c_fb_driverdata s3c_fb_data_s5pc100 = {
 	.variant = {
 		.nr_windows	= 5,
 		.vidtcon	= VIDTCON0,
@@ -1593,7 +1593,7 @@ static struct s3c_fb_driverdata s3c_fb_data_s5pc100 __devinitdata = {
 	.win[4]	= &s3c_fb_data_64xx_wins[4],
 };
 
-static struct s3c_fb_driverdata s3c_fb_data_s5pv210 __devinitdata = {
+static struct s3c_fb_driverdata s3c_fb_data_s5pv210 = {
 	.variant = {
 		.nr_windows	= 5,
 		.vidtcon	= VIDTCON0,
@@ -1624,7 +1624,7 @@ static struct s3c_fb_driverdata s3c_fb_data_s5pv210 __devinitdata = {
 };
 
 /* S3C2443/S3C2416 style hardware */
-static struct s3c_fb_driverdata s3c_fb_data_s3c2443 __devinitdata = {
+static struct s3c_fb_driverdata s3c_fb_data_s3c2443 = {
 	.variant = {
 		.nr_windows	= 2,
 		.is_2443	= 1,
-- 
1.7.1.569.g6f426

[PATCH v3 12/12] s3c-fb: Add support for DMA channel control on S5PV210

[Pawel Osciak]

S5PV210 SoCs allow enabling/disabling DMA channels per window. For a window
to display data from framebuffer memory, its channel has to be enabled.

Signed-off-by: Pawel Osciak <p.osciak@samsung.com>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/plat-samsung/include/plat/regs-fb.h |    4 ++++
 drivers/video/s3c-fb.c                       |   15 +++++++++++++++
 2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-samsung/include/plat/regs-fb.h b/arch/arm/plat-samsung/include/plat/regs-fb.h
index da54b64..f801a37 100644
--- a/arch/arm/plat-samsung/include/plat/regs-fb.h
+++ b/arch/arm/plat-samsung/include/plat/regs-fb.h
@@ -221,6 +221,10 @@
 /* S5PV210 */
 #define SHADOWCON				(0x34)
 #define SHADOWCON_WINx_PROTECT(_win)		(1 << (10 + _win))
+/* DMA channels (all windows) */
+#define SHADOWCON_CHx_ENABLE(_win)		(1 << (_win))
+/* Local input channels (windows 0-2) */
+#define SHADOWCON_CHx_LOCAL_ENABLE(_win)	(1 << (5 + (_win)))
 
 #define VIDOSDxA_TOPLEFT_X_MASK			(0x7ff << 11)
 #define VIDOSDxA_TOPLEFT_X_SHIFT		(11)
diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
index de7334c..4b3617c 100644
--- a/drivers/video/s3c-fb.c
+++ b/drivers/video/s3c-fb.c
@@ -635,6 +635,13 @@ static int s3c_fb_set_par(struct fb_info *info)
 	writel(data, regs + sfb->variant.wincon + (win_no * 4));
 	writel(0x0, regs + sfb->variant.winmap + (win_no * 4));
 
+	/* Enable DMA channel for this window */
+	if (sfb->variant.has_shadowcon) {
+		data = readl(sfb->regs + SHADOWCON);
+		data |= SHADOWCON_CHx_ENABLE(win_no);
+		writel(data, sfb->regs + SHADOWCON);
+	}
+
 	shadow_protect_win(win, 0);
 
 	return 0;
@@ -1095,7 +1102,15 @@ static void s3c_fb_free_memory(struct s3c_fb *sfb, struct s3c_fb_win *win)
  */
 static void s3c_fb_release_win(struct s3c_fb *sfb, struct s3c_fb_win *win)
 {
+	u32 data;
+
 	if (win->fbinfo) {
+		if (sfb->variant.has_shadowcon) {
+			data = readl(sfb->regs + SHADOWCON);
+			data &= ~SHADOWCON_CHx_ENABLE(win->index);
+			data &= ~SHADOWCON_CHx_LOCAL_ENABLE(win->index);
+			writel(data, sfb->regs + SHADOWCON);
+		}
 		unregister_framebuffer(win->fbinfo);
 		if (&win->fbinfo->cmap)
 			fb_dealloc_cmap(&win->fbinfo->cmap);
-- 
1.7.1.569.g6f426

Re: [PATCH v3 05/12] s3c-fb: Add support for display panning

[Maurus Cuelenaere]

 Op 28-06-10 10:08, Pawel Osciak schreef:
> Supports all bpp modes.
>
> The PRTCON register is used to disable in-hardware updates of registers
> that store start and end addresses of framebuffer memory. This prevents
> display corruption in case we do not make it before VSYNC with updating
> them atomically. With this feature there is no need to wait for a VSYNC
> interrupt before each such update.
>
> Signed-off-by: Pawel Osciak <p.osciak@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  arch/arm/plat-samsung/include/plat/regs-fb.h |    5 ++
>  drivers/video/s3c-fb.c                       |   71 ++++++++++++++++++++++++++
>  2 files changed, 76 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/plat-samsung/include/plat/regs-fb.h b/arch/arm/plat-samsung/include/plat/regs-fb.h
> index ac10013..f454e32 100644
> --- a/arch/arm/plat-samsung/include/plat/regs-fb.h
> +++ b/arch/arm/plat-samsung/include/plat/regs-fb.h
> @@ -112,6 +112,11 @@
>  #define VIDCON2_ORGYCbCr			(1 << 8)
>  #define VIDCON2_YUVORDCrCb			(1 << 7)
>  
> +/* PRTCON (S3C6410, S5PC100) */
> +
> +#define PRTCON					(0x0c)
> +#define PRTCON_PROTECT				(1 << 11)
> +
>  /* VIDTCON0 */
>  
>  #define VIDTCON0_VBPDE_MASK			(0xff << 24)
> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
> index 59ac76a..4f3680d 100644
> --- a/drivers/video/s3c-fb.c
> +++ b/drivers/video/s3c-fb.c
> @@ -71,6 +71,7 @@ struct s3c_fb;
>   * @buf_end: Offset of buffer end registers.
>   * @osd: The base for the OSD registers.
>   * @palette: Address of palette memory, or 0 if none.
> + * @has_prtcon: Set if has PRTCON register.
>   */
>  struct s3c_fb_variant {
>  	unsigned int	is_2443:1;
> @@ -85,6 +86,8 @@ struct s3c_fb_variant {
>  	unsigned short	osd;
>  	unsigned short	osd_stride;
>  	unsigned short	palette[S3C_FB_MAX_WIN];
> +
> +	unsigned int	has_prtcon:1;
>  };
>  
>  /**
> @@ -379,6 +382,9 @@ static int s3c_fb_set_par(struct fb_info *info)
>  
>  	info->fix.line_length = (var->xres_virtual * var->bits_per_pixel) / 8;
>  
> +	info->fix.xpanstep = info->var.xres_virtual > info->var.xres ? 1 : 0;
> +	info->fix.ypanstep = info->var.yres_virtual > info->var.yres ? 1 : 0;

No need for the '? 1 : 0'

> +
>  	/* disable the window whilst we update it */
>  	writel(0, regs + WINCON(win_no));
>  
> @@ -735,6 +741,66 @@ static int s3c_fb_blank(int blank_mode, struct fb_info *info)
>  	return 0;
>  }
>  
> +/**
> + * s3c_fb_pan_display() - Pan the display.
> + *
> + * Note that the offsets can be written to the device at any time, as their
> + * values are latched at each vsync automatically. This also means that only
> + * the last call to this function will have any effect on next vsync, but
> + * there is no need to sleep waiting for it to prevent tearing.
> + *
> + * @var: The screen information to verify.
> + * @info: The framebuffer device.
> + */
> +static int s3c_fb_pan_display(struct fb_var_screeninfo *var,
> +			      struct fb_info *info)
> +{
> +	struct s3c_fb_win *win	= info->par;
> +	struct s3c_fb *sfb	= win->parent;
> +	void __iomem *buf	= sfb->regs + win->index * 8;
> +	unsigned int start_byte_offset, end_byte_offset;
> +
> +	/* Offset in bytes to the start of the displayed area */
> +	start_byte_offset = var->yoffset * info->fix.line_length;
> +	/* X offset depends on the current bpp */
> +	if (info->var.bits_per_pixel >= 8) {
> +		start_byte_offset +> +			var->xoffset * (info->var.bits_per_pixel >> 3);
> +	} else {
> +		switch (info->var.bits_per_pixel) {
> +		case 4:
> +			start_byte_offset += var->xoffset >> 1;
> +			break;
> +		case 2:
> +			start_byte_offset += var->xoffset >> 2;
> +			break;
> +		case 1:
> +			start_byte_offset += var->xoffset >> 3;
> +			break;
> +		default:
> +			dev_err(sfb->dev, "invalid bpp\n");
> +			return -EINVAL;
> +		}
> +	}
> +	/* Offset in bytes to the end of the displayed area */
> +	end_byte_offset = start_byte_offset + var->yres * info->fix.line_length;
> +
> +	/* Temporarily turn off per-vsync update from shadow registers until
> +	 * both start and end addresses are updated to prevent corruption */
> +	if (sfb->variant.has_prtcon)
> +		writel(PRTCON_PROTECT, sfb->regs + PRTCON);
> +
> +	writel(info->fix.smem_start + start_byte_offset,
> +		buf + sfb->variant.buf_start);
> +	writel(info->fix.smem_start + end_byte_offset,
> +		buf + sfb->variant.buf_end);
> +
> +	if (sfb->variant.has_prtcon)
> +		writel(0, sfb->regs + PRTCON);
> +
> +	return 0;
> +}
> +
>  static struct fb_ops s3c_fb_ops = {
>  	.owner		= THIS_MODULE,
>  	.fb_check_var	= s3c_fb_check_var,
> @@ -744,6 +810,7 @@ static struct fb_ops s3c_fb_ops = {
>  	.fb_fillrect	= cfb_fillrect,
>  	.fb_copyarea	= cfb_copyarea,
>  	.fb_imageblit	= cfb_imageblit,
> +	.fb_pan_display	= s3c_fb_pan_display,
>  };
>  
>  /**
> @@ -1243,6 +1310,8 @@ static struct s3c_fb_driverdata s3c_fb_data_64xx __devinitdata = {
>  			[3] = 0x320,
>  			[4] = 0x340,
>  		},
> +
> +		.has_prtcon	= 1,
>  	},
>  	.win[0]	= &s3c_fb_data_64xx_wins[0],
>  	.win[1]	= &s3c_fb_data_64xx_wins[1],
> @@ -1271,6 +1340,8 @@ static struct s3c_fb_driverdata s3c_fb_data_s5pc100 __devinitdata = {
>  			[3] = 0x3000,
>  			[4] = 0x3400,
>  		},
> +
> +		.has_prtcon	= 1,
>  	},
>  	.win[0]	= &s3c_fb_data_64xx_wins[0],
>  	.win[1]	= &s3c_fb_data_64xx_wins[1],


-- 
Maurus Cuelenaere

Re: [PATCH v3 01/12] s3c-fb: Fix various null references on framebuffer

[Ben Dooks]

On 28/06/10 09:08, Pawel Osciak wrote:
> The following problems were found in the above situation:
> 
> sfb->windows[win] was being assigned at the end of s3c_fb_probe_win only.
> This resulted in passing a NULL to s3c_fb_release_win if probe_win returned
> early and a memory leak.
> 
> dma_free_writecombine does not allow its third argument to be NULL.
> 
> fb_dealloc_cmap does not verify whether its argument is not NULL.
> 
> Signed-off-by: Pawel Osciak <p.osciak@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/video/s3c-fb.c |    8 +++++---
>  1 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
> index b00c064..d998324 100644
> --- a/drivers/video/s3c-fb.c
> +++ b/drivers/video/s3c-fb.c
> @@ -804,7 +804,8 @@ static void s3c_fb_free_memory(struct s3c_fb *sfb, struct s3c_fb_win *win)
>  {
>  	struct fb_info *fbi = win->fbinfo;
>  
> -	dma_free_writecombine(sfb->dev, PAGE_ALIGN(fbi->fix.smem_len),
> +	if (fbi->screen_base)
> +		dma_free_writecombine(sfb->dev, PAGE_ALIGN(fbi->fix.smem_len),
>  			      fbi->screen_base, fbi->fix.smem_start);
>  }
>  
> @@ -819,7 +820,8 @@ static void s3c_fb_release_win(struct s3c_fb *sfb, struct s3c_fb_win *win)
>  {
>  	if (win->fbinfo) {
>  		unregister_framebuffer(win->fbinfo);
> -		fb_dealloc_cmap(&win->fbinfo->cmap);
> +		if (&win->fbinfo->cmap)
> +			fb_dealloc_cmap(&win->fbinfo->cmap);

did you really mean &win->fbinfo->cmap? surely that will end up
always evaluating to true?

>  		s3c_fb_free_memory(sfb, win);
>  		framebuffer_release(win->fbinfo);
>  	}
> @@ -865,6 +867,7 @@ static int __devinit s3c_fb_probe_win(struct s3c_fb *sfb, unsigned int win_no,
>  	WARN_ON(windata->win_mode.yres = 0);
>  
>  	win = fbinfo->par;
> +	*res = win;
>  	var = &fbinfo->var;
>  	win->variant = *variant;
>  	win->fbinfo = fbinfo;
> @@ -939,7 +942,6 @@ static int __devinit s3c_fb_probe_win(struct s3c_fb *sfb, unsigned int win_no,
>  		return ret;
>  	}
>  
> -	*res = win;
>  	dev_info(sfb->dev, "window %d: fb %s\n", win_no, fbinfo->fix.id);
>  
>  	return 0;

Re: [PATCH v3 02/12] s3c-fb: Correct FRAMESEL1 bitfield defines for

[Ben Dooks]

On 28/06/10 09:08, Pawel Osciak wrote:
> FRAMESEL1 bitfield starts on 13th bit, not on 14th.
> 
> Signed-off-by: Pawel Osciak <p.osciak@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Ben Dooks <ben-linux@fluff.org>

Re: [PATCH v3 03/12] s3c-fb: Separate S5PC100 and S5PV210 framebuffer

[Ben Dooks]

On 28/06/10 09:08, Pawel Osciak wrote:
> S5PC100 and S5PV210 framebuffer devices differ slightly in terms of
> available registers and their driver data structures have to be separate.

Would have been nice to note which ones changes, otherwise ok.

Re: [PATCH v3 04/12] s3c-fb: Add device name initialization

[Ben Dooks]

On 28/06/10 09:08, Pawel Osciak wrote:
> Add framebuffer device name initialization calls for S3C2443, S3C64xx
> and S5P machines.

ok, if we don't sort out a better system before the next
window then this should be ok.

Re: [PATCH v3 05/12] s3c-fb: Add support for display panning

[Ben Dooks]

On 28/06/10 09:08, Pawel Osciak wrote:
> Supports all bpp modes.
> 
> The PRTCON register is used to disable in-hardware updates of registers
> that store start and end addresses of framebuffer memory. This prevents
> display corruption in case we do not make it before VSYNC with updating
> them atomically. With this feature there is no need to wait for a VSYNC
> interrupt before each such update.
> 
> Signed-off-by: Pawel Osciak <p.osciak@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  arch/arm/plat-samsung/include/plat/regs-fb.h |    5 ++
>  drivers/video/s3c-fb.c                       |   71 ++++++++++++++++++++++++++
>  2 files changed, 76 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/plat-samsung/include/plat/regs-fb.h b/arch/arm/plat-samsung/include/plat/regs-fb.h
> index ac10013..f454e32 100644
> --- a/arch/arm/plat-samsung/include/plat/regs-fb.h
> +++ b/arch/arm/plat-samsung/include/plat/regs-fb.h
> @@ -112,6 +112,11 @@
>  #define VIDCON2_ORGYCbCr			(1 << 8)
>  #define VIDCON2_YUVORDCrCb			(1 << 7)
>  
> +/* PRTCON (S3C6410, S5PC100) */

Not listed in my S3C6410 manual?

> +#define PRTCON					(0x0c)
> +#define PRTCON_PROTECT				(1 << 11)
> +
>  /* VIDTCON0 */
>  
>  #define VIDTCON0_VBPDE_MASK			(0xff << 24)
> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
> index 59ac76a..4f3680d 100644
> --- a/drivers/video/s3c-fb.c
> +++ b/drivers/video/s3c-fb.c
> @@ -71,6 +71,7 @@ struct s3c_fb;
>   * @buf_end: Offset of buffer end registers.
>   * @osd: The base for the OSD registers.
>   * @palette: Address of palette memory, or 0 if none.
> + * @has_prtcon: Set if has PRTCON register.
>   */
>  struct s3c_fb_variant {
>  	unsigned int	is_2443:1;
> @@ -85,6 +86,8 @@ struct s3c_fb_variant {
>  	unsigned short	osd;
>  	unsigned short	osd_stride;
>  	unsigned short	palette[S3C_FB_MAX_WIN];
> +
> +	unsigned int	has_prtcon:1;
>  };
>  
>  /**
> @@ -379,6 +382,9 @@ static int s3c_fb_set_par(struct fb_info *info)
>  
>  	info->fix.line_length = (var->xres_virtual * var->bits_per_pixel) / 8;
>  
> +	info->fix.xpanstep = info->var.xres_virtual > info->var.xres ? 1 : 0;
> +	info->fix.ypanstep = info->var.yres_virtual > info->var.yres ? 1 : 0;
> +
>  	/* disable the window whilst we update it */
>  	writel(0, regs + WINCON(win_no));
>  
> @@ -735,6 +741,66 @@ static int s3c_fb_blank(int blank_mode, struct fb_info *info)
>  	return 0;
>  }
>  
> +/**
> + * s3c_fb_pan_display() - Pan the display.
> + *
> + * Note that the offsets can be written to the device at any time, as their
> + * values are latched at each vsync automatically. This also means that only
> + * the last call to this function will have any effect on next vsync, but
> + * there is no need to sleep waiting for it to prevent tearing.
> + *
> + * @var: The screen information to verify.
> + * @info: The framebuffer device.
> + */
> +static int s3c_fb_pan_display(struct fb_var_screeninfo *var,
> +			      struct fb_info *info)
> +{
> +	struct s3c_fb_win *win	= info->par;
> +	struct s3c_fb *sfb	= win->parent;
> +	void __iomem *buf	= sfb->regs + win->index * 8;
> +	unsigned int start_byte_offset, end_byte_offset;

could have shortened to start and end, might have made some of the
calculation easier.

> +	/* Offset in bytes to the start of the displayed area */
> +	start_byte_offset = var->yoffset * info->fix.line_length;
> +	/* X offset depends on the current bpp */
> +	if (info->var.bits_per_pixel >= 8) {
> +		start_byte_offset +> +			var->xoffset * (info->var.bits_per_pixel >> 3);
> +	} else {
> +		switch (info->var.bits_per_pixel) {
> +		case 4:
> +			start_byte_offset += var->xoffset >> 1;
> +			break;
> +		case 2:
> +			start_byte_offset += var->xoffset >> 2;
> +			break;
> +		case 1:
> +			start_byte_offset += var->xoffset >> 3;
> +			break;
> +		default:
> +			dev_err(sfb->dev, "invalid bpp\n");
> +			return -EINVAL;
> +		}
> +	}

this could have been done by changing it to
start_byte_offset += var->xoffset / (8 / info->var.bits_per_pixel);

note, you always set pan step to 1, evne if the case of bpp less than 8.

> +	/* Offset in bytes to the end of the displayed area */
> +	end_byte_offset = start_byte_offset + var->yres * info->fix.line_length;
> +
> +	/* Temporarily turn off per-vsync update from shadow registers until
> +	 * both start and end addresses are updated to prevent corruption */
> +	if (sfb->variant.has_prtcon)
> +		writel(PRTCON_PROTECT, sfb->regs + PRTCON);
> +
> +	writel(info->fix.smem_start + start_byte_offset,
> +		buf + sfb->variant.buf_start);
> +	writel(info->fix.smem_start + end_byte_offset,
> +		buf + sfb->variant.buf_end);
> +
> +	if (sfb->variant.has_prtcon)
> +		writel(0, sfb->regs + PRTCON);
> +
> +	return 0;
> +}
> +
>  static struct fb_ops s3c_fb_ops = {
>  	.owner		= THIS_MODULE,
>  	.fb_check_var	= s3c_fb_check_var,
> @@ -744,6 +810,7 @@ static struct fb_ops s3c_fb_ops = {
>  	.fb_fillrect	= cfb_fillrect,
>  	.fb_copyarea	= cfb_copyarea,
>  	.fb_imageblit	= cfb_imageblit,
> +	.fb_pan_display	= s3c_fb_pan_display,
>  };
>  
>  /**
> @@ -1243,6 +1310,8 @@ static struct s3c_fb_driverdata s3c_fb_data_64xx __devinitdata = {
>  			[3] = 0x320,
>  			[4] = 0x340,
>  		},
> +
> +		.has_prtcon	= 1,
>  	},
>  	.win[0]	= &s3c_fb_data_64xx_wins[0],
>  	.win[1]	= &s3c_fb_data_64xx_wins[1],
> @@ -1271,6 +1340,8 @@ static struct s3c_fb_driverdata s3c_fb_data_s5pc100 __devinitdata = {
>  			[3] = 0x3000,
>  			[4] = 0x3400,
>  		},
> +
> +		.has_prtcon	= 1,
>  	},
>  	.win[0]	= &s3c_fb_data_64xx_wins[0],
>  	.win[1]	= &s3c_fb_data_64xx_wins[1],

Re: [PATCH v3 05/12] s3c-fb: Add support for display panning

[Ben Dooks]

On 28/06/10 12:28, Maurus Cuelenaere wrote:
>  Op 28-06-10 10:08, Pawel Osciak schreef:
>> Supports all bpp modes.
>>
>> The PRTCON register is used to disable in-hardware updates of registers
>> that store start and end addresses of framebuffer memory. This prevents
>> display corruption in case we do not make it before VSYNC with updating
>> them atomically. With this feature there is no need to wait for a VSYNC
>> interrupt before each such update.
>>
>> Signed-off-by: Pawel Osciak <p.osciak@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>>  arch/arm/plat-samsung/include/plat/regs-fb.h |    5 ++
>>  drivers/video/s3c-fb.c                       |   71 ++++++++++++++++++++++++++
>>  2 files changed, 76 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/plat-samsung/include/plat/regs-fb.h b/arch/arm/plat-samsung/include/plat/regs-fb.h
>> index ac10013..f454e32 100644
>> --- a/arch/arm/plat-samsung/include/plat/regs-fb.h
>> +++ b/arch/arm/plat-samsung/include/plat/regs-fb.h
>> @@ -112,6 +112,11 @@
>>  #define VIDCON2_ORGYCbCr			(1 << 8)
>>  #define VIDCON2_YUVORDCrCb			(1 << 7)
>>  
>> +/* PRTCON (S3C6410, S5PC100) */
>> +
>> +#define PRTCON					(0x0c)
>> +#define PRTCON_PROTECT				(1 << 11)
>> +
>>  /* VIDTCON0 */
>>  
>>  #define VIDTCON0_VBPDE_MASK			(0xff << 24)
>> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
>> index 59ac76a..4f3680d 100644
>> --- a/drivers/video/s3c-fb.c
>> +++ b/drivers/video/s3c-fb.c
>> @@ -71,6 +71,7 @@ struct s3c_fb;
>>   * @buf_end: Offset of buffer end registers.
>>   * @osd: The base for the OSD registers.
>>   * @palette: Address of palette memory, or 0 if none.
>> + * @has_prtcon: Set if has PRTCON register.
>>   */
>>  struct s3c_fb_variant {
>>  	unsigned int	is_2443:1;
>> @@ -85,6 +86,8 @@ struct s3c_fb_variant {
>>  	unsigned short	osd;
>>  	unsigned short	osd_stride;
>>  	unsigned short	palette[S3C_FB_MAX_WIN];
>> +
>> +	unsigned int	has_prtcon:1;
>>  };
>>  
>>  /**
>> @@ -379,6 +382,9 @@ static int s3c_fb_set_par(struct fb_info *info)
>>  
>>  	info->fix.line_length = (var->xres_virtual * var->bits_per_pixel) / 8;
>>  
>> +	info->fix.xpanstep = info->var.xres_virtual > info->var.xres ? 1 : 0;
>> +	info->fix.ypanstep = info->var.yres_virtual > info->var.yres ? 1 : 0;
> 
> No need for the '? 1 : 0'

I tihnk there is, IIRC there is no neccessity for a compiler to produce
one for true, just that the result be !0

Re: [PATCH v3 05/12] s3c-fb: Add support for display panning

[Maurus Cuelenaere]

 Op 02-07-10 13:25, Ben Dooks schreef:
> On 28/06/10 12:28, Maurus Cuelenaere wrote:
>>  Op 28-06-10 10:08, Pawel Osciak schreef:
>>> Supports all bpp modes.
>>>
>>> The PRTCON register is used to disable in-hardware updates of registers
>>> that store start and end addresses of framebuffer memory. This prevents
>>> display corruption in case we do not make it before VSYNC with updating
>>> them atomically. With this feature there is no need to wait for a VSYNC
>>> interrupt before each such update.
>>>
>>> Signed-off-by: Pawel Osciak <p.osciak@samsung.com>
>>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>>> ---
>>>  arch/arm/plat-samsung/include/plat/regs-fb.h |    5 ++
>>>  drivers/video/s3c-fb.c                       |   71 ++++++++++++++++++++++++++
>>>  2 files changed, 76 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/arm/plat-samsung/include/plat/regs-fb.h b/arch/arm/plat-samsung/include/plat/regs-fb.h
>>> index ac10013..f454e32 100644
>>> --- a/arch/arm/plat-samsung/include/plat/regs-fb.h
>>> +++ b/arch/arm/plat-samsung/include/plat/regs-fb.h
>>> @@ -112,6 +112,11 @@
>>>  #define VIDCON2_ORGYCbCr			(1 << 8)
>>>  #define VIDCON2_YUVORDCrCb			(1 << 7)
>>>  
>>> +/* PRTCON (S3C6410, S5PC100) */
>>> +
>>> +#define PRTCON					(0x0c)
>>> +#define PRTCON_PROTECT				(1 << 11)
>>> +
>>>  /* VIDTCON0 */
>>>  
>>>  #define VIDTCON0_VBPDE_MASK			(0xff << 24)
>>> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
>>> index 59ac76a..4f3680d 100644
>>> --- a/drivers/video/s3c-fb.c
>>> +++ b/drivers/video/s3c-fb.c
>>> @@ -71,6 +71,7 @@ struct s3c_fb;
>>>   * @buf_end: Offset of buffer end registers.
>>>   * @osd: The base for the OSD registers.
>>>   * @palette: Address of palette memory, or 0 if none.
>>> + * @has_prtcon: Set if has PRTCON register.
>>>   */
>>>  struct s3c_fb_variant {
>>>  	unsigned int	is_2443:1;
>>> @@ -85,6 +86,8 @@ struct s3c_fb_variant {
>>>  	unsigned short	osd;
>>>  	unsigned short	osd_stride;
>>>  	unsigned short	palette[S3C_FB_MAX_WIN];
>>> +
>>> +	unsigned int	has_prtcon:1;
>>>  };
>>>  
>>>  /**
>>> @@ -379,6 +382,9 @@ static int s3c_fb_set_par(struct fb_info *info)
>>>  
>>>  	info->fix.line_length = (var->xres_virtual * var->bits_per_pixel) / 8;
>>>  
>>> +	info->fix.xpanstep = info->var.xres_virtual > info->var.xres ? 1 : 0;
>>> +	info->fix.ypanstep = info->var.yres_virtual > info->var.yres ? 1 : 0;
>> No need for the '? 1 : 0'
> I tihnk there is, IIRC there is no neccessity for a compiler to produce
> one for true, just that the result be !0

Hmm ok, I only checked this using GCC 4.4.3 and this:

#include <stdio.h>
#include <stdbool.h>

int main(void) {
    printf("%d %d\n", (int)(1 > 2), (int)(1 < 2));   // gives 0 1
    printf("%d %d\n", (bool)(1 > 2), (bool)(1 < 2)); // gives 0 1

    return 0;
}

-- 
Maurus Cuelenaere

Re: [PATCH v3 06/12] s3c-fb: Add wait for VSYNC ioctl

[Ben Dooks]

On 28/06/10 09:08, Pawel Osciak wrote:
> Add VSYNC interrupt support and an ioctl that allows waiting for it.
> Interrupts are turned on only when needed.
> 
> Signed-off-by: Pawel Osciak <p.osciak@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  arch/arm/plat-samsung/include/plat/regs-fb.h |    1 +
>  drivers/video/s3c-fb.c                       |  167 +++++++++++++++++++++++++-
>  2 files changed, 167 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/plat-samsung/include/plat/regs-fb.h b/arch/arm/plat-samsung/include/plat/regs-fb.h
> index f454e32..5bcdd09 100644
> --- a/arch/arm/plat-samsung/include/plat/regs-fb.h
> +++ b/arch/arm/plat-samsung/include/plat/regs-fb.h
> @@ -298,6 +298,7 @@
>  #define VIDINTCON0_FRAMESEL0_FRONTPORCH		(0x3 << 15)
>  
>  #define VIDINTCON0_FRAMESEL1			(1 << 13)
> +#define VIDINTCON0_FRAMESEL1_MASK		(0x3 << 13)
>  #define VIDINTCON0_FRAMESEL1_NONE		(0x0 << 13)
>  #define VIDINTCON0_FRAMESEL1_BACKPORCH		(0x1 << 13)
>  #define VIDINTCON0_FRAMESEL1_VSYNC		(0x2 << 13)
> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
> index 4f3680d..6131ebb 100644
> --- a/drivers/video/s3c-fb.c
> +++ b/drivers/video/s3c-fb.c
> @@ -21,6 +21,8 @@
>  #include <linux/clk.h>
>  #include <linux/fb.h>
>  #include <linux/io.h>
> +#include <linux/uaccess.h>
> +#include <linux/interrupt.h>
>  
>  #include <mach/map.h>
>  #include <plat/regs-fb-v4.h>
> @@ -48,6 +50,11 @@
>  	__raw_writel(v, r); } while(0)
>  #endif /* FB_S3C_DEBUG_REGWRITE */
>  
> +/* irq_flags bits */
> +#define S3C_FB_VSYNC_IRQ_EN	0
> +
> +#define VSYNC_TIMEOUT_MSEC 50
> +
>  struct s3c_fb;
>  
>  #define VALID_BPP(x) (1 << ((x) - 1))
> @@ -156,6 +163,16 @@ struct s3c_fb_win {
>  };
>  
>  /**
> + * struct s3c_fb_vsync - vsync information
> + * @wait:	a queue for processes waiting for vsync
> + * @count:	vsync interrupt count
> + */
> +struct s3c_fb_vsync {
> +	wait_queue_head_t	wait;
> +	unsigned int		count;
> +};
> +
> +/**
>   * struct s3c_fb - overall hardware state of the hardware
>   * @dev: The device that we bound to, for printing, etc.
>   * @regs_res: The resource we claimed for the IO registers.
> @@ -165,6 +182,9 @@ struct s3c_fb_win {
>   * @enabled: A bitmask of enabled hardware windows.
>   * @pdata: The platform configuration data passed with the device.
>   * @windows: The hardware windows that have been claimed.
> + * @irq_no: IRQ line number
> + * @irq_flags: irq flags
> + * @vsync_info: VSYNC-related information (count, queues...)
>   */
>  struct s3c_fb {
>  	struct device		*dev;
> @@ -177,6 +197,10 @@ struct s3c_fb {
>  
>  	struct s3c_fb_platdata	*pdata;
>  	struct s3c_fb_win	*windows[S3C_FB_MAX_WIN];
> +
> +	int			 irq_no;
> +	unsigned long		 irq_flags;
> +	struct s3c_fb_vsync	 vsync_info;
>  };
>  
>  /**
> @@ -801,6 +825,124 @@ static int s3c_fb_pan_display(struct fb_var_screeninfo *var,
>  	return 0;
>  }
>  
> +/**
> + * s3c_fb_enable_irq() - enable framebuffer interrupts
> + * @sfb: main hardware state
> + */
> +static void s3c_fb_enable_irq(struct s3c_fb *sfb)
> +{
> +	void __iomem *regs = sfb->regs;
> +	u32 irq_ctrl_reg;
> +
> +	if (!test_and_set_bit(S3C_FB_VSYNC_IRQ_EN, &sfb->irq_flags)) {
> +		/* IRQ disabled, enable it */
> +		irq_ctrl_reg = readl(regs + VIDINTCON0);
> +
> +		irq_ctrl_reg |= VIDINTCON0_INT_ENABLE;
> +		irq_ctrl_reg |= VIDINTCON0_INT_FRAME;
> +
> +		irq_ctrl_reg &= ~VIDINTCON0_FRAMESEL0_MASK;
> +		irq_ctrl_reg |= VIDINTCON0_FRAMESEL0_VSYNC;
> +		irq_ctrl_reg &= ~VIDINTCON0_FRAMESEL1_MASK;
> +		irq_ctrl_reg |= VIDINTCON0_FRAMESEL1_NONE;
> +
> +		writel(irq_ctrl_reg, regs + VIDINTCON0);
> +	}
> +}

there should probably be some form of locking so that an irq
doesn't come through and disable this. possibly in the call
that queues the request to reduce the likelyhood of any races.

> +
> +/**
> + * s3c_fb_disable_irq() - disable framebuffer interrupts
> + * @sfb: main hardware state
> + */
> +static void s3c_fb_disable_irq(struct s3c_fb *sfb)
> +{
> +	void __iomem *regs = sfb->regs;
> +	u32 irq_ctrl_reg;
> +
> +	if (test_and_clear_bit(S3C_FB_VSYNC_IRQ_EN, &sfb->irq_flags)) {
> +		/* IRQ enabled, disable it */
> +		irq_ctrl_reg = readl(regs + VIDINTCON0);
> +
> +		irq_ctrl_reg &= ~VIDINTCON0_INT_FRAME;
> +		irq_ctrl_reg &= ~VIDINTCON0_INT_ENABLE;
> +
> +		writel(irq_ctrl_reg, regs + VIDINTCON0);
> +	}
> +}
> +
> +static irqreturn_t s3c_fb_irq(int irq, void *dev_id)
> +{
> +	struct s3c_fb *sfb = dev_id;
> +	void __iomem  *regs = sfb->regs;
> +	u32 irq_sts_reg;
> +
> +	irq_sts_reg = readl(regs + VIDINTCON1);
> +
> +	if (irq_sts_reg & VIDINTCON1_INT_FRAME) {
> +
> +		/* VSYNC interrupt, accept it */
> +		writel(VIDINTCON1_INT_FRAME, regs + VIDINTCON1);
> +
> +		sfb->vsync_info.count++;
> +		wake_up_interruptible(&sfb->vsync_info.wait);
> +	}
> +
> +	/* We only support waiting for VSYNC for now, so it's safe
> +	 * to always disable irqs here.
> +	 */
> +	s3c_fb_disable_irq(sfb);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +/**
> + * s3c_fb_wait_for_vsync() - sleep until next VSYNC interrupt or timeout
> + * @sfb: main hardware state
> + * @crtc: head index.
> + */
> +static int s3c_fb_wait_for_vsync(struct s3c_fb *sfb, u32 crtc)
> +{
> +	unsigned long count;
> +	int ret;
> +
> +	if (crtc != 0)
> +		return -ENODEV;
> +
> +	s3c_fb_enable_irq(sfb);
> +	count = sfb->vsync_info.count;

possibly doing count before enabling the irq.

> +	ret = wait_event_interruptible_timeout(sfb->vsync_info.wait,
> +				       count != sfb->vsync_info.count,
> +				       msecs_to_jiffies(VSYNC_TIMEOUT_MSEC));
> +	if (ret = 0)
> +		return -ETIMEDOUT;
> +
> +	return 0;
> +}
> +
> +static int s3c_fb_ioctl(struct fb_info *info, unsigned int cmd,
> +			unsigned long arg)
> +{
> +	struct s3c_fb_win *win = info->par;
> +	struct s3c_fb *sfb = win->parent;
> +	int ret;
> +	u32 crtc;
> +
> +	switch (cmd) {
> +	case FBIO_WAITFORVSYNC:
> +		if (get_user(crtc, (u32 __user *)arg)) {
> +			ret = -EFAULT;
> +			break;
> +		}

can't find any info on what the argument is meant to be.

> +
> +		ret = s3c_fb_wait_for_vsync(sfb, crtc);
> +		break;
> +	default:
> +		ret = -ENOTTY;

Can someone else confirm this is the correct err?

> +	}
> +
> +	return ret;
> +}
> +
>  static struct fb_ops s3c_fb_ops = {
>  	.owner		= THIS_MODULE,
>  	.fb_check_var	= s3c_fb_check_var,
> @@ -811,6 +953,7 @@ static struct fb_ops s3c_fb_ops = {
>  	.fb_copyarea	= cfb_copyarea,
>  	.fb_imageblit	= cfb_imageblit,
>  	.fb_pan_display	= s3c_fb_pan_display,
> +	.fb_ioctl	= s3c_fb_ioctl,
>  };
>  
>  /**
> @@ -917,6 +1060,8 @@ static int __devinit s3c_fb_probe_win(struct s3c_fb *sfb, unsigned int win_no,
>  
>  	dev_dbg(sfb->dev, "probing window %d, variant %p\n", win_no, variant);
>  
> +	init_waitqueue_head(&sfb->vsync_info.wait);
> +
>  	palette_size = variant->palette_sz * 4;
>  
>  	fbinfo = framebuffer_alloc(sizeof(struct s3c_fb_win) +
> @@ -1096,6 +1241,20 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)
>  		goto err_req_region;
>  	}
>  
> +	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +	if (!res) {
> +		dev_err(dev, "failed to acquire irq resource\n");
> +		ret = -ENOENT;
> +		goto err_ioremap;
> +	}
> +	sfb->irq_no = res->start;
> +	ret = request_irq(sfb->irq_no, s3c_fb_irq,
> +			  0, "s3c_fb", sfb);
> +	if (ret) {
> +		dev_err(dev, "irq request failed\n");
> +		goto err_ioremap;
> +	}
> +
>  	dev_dbg(dev, "got resources (regs %p), probing windows\n", sfb->regs);
>  
>  	/* setup gpio and output polarity controls */
> @@ -1130,7 +1289,7 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)
>  			dev_err(dev, "failed to create window %d\n", win);
>  			for (; win >= 0; win--)
>  				s3c_fb_release_win(sfb, sfb->windows[win]);
> -			goto err_ioremap;
> +			goto err_irq;
>  		}
>  	}
>  
> @@ -1138,6 +1297,9 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)
>  
>  	return 0;
>  
> +err_irq:
> +	free_irq(sfb->irq_no, sfb);
> +
>  err_ioremap:
>  	iounmap(sfb->regs);
>  
> @@ -1170,6 +1332,8 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)
>  		if (sfb->windows[win])
>  			s3c_fb_release_win(sfb, sfb->windows[win]);
>  
> +	free_irq(sfb->irq_no, sfb);
> +
>  	iounmap(sfb->regs);
>  
>  	clk_disable(sfb->bus_clk);
> @@ -1370,6 +1534,7 @@ static struct s3c_fb_driverdata s3c_fb_data_s5pv210 __devinitdata = {
>  			[3] = 0x3000,
>  			[4] = 0x3400,
>  		},
> +
>  	},

oops, added whitespace.

>  	.win[0]	= &s3c_fb_data_64xx_wins[0],
>  	.win[1]	= &s3c_fb_data_64xx_wins[1],

Re: [PATCH v3 05/12] s3c-fb: Add support for display panning

[Mark Brown]

On Fri, Jul 02, 2010 at 12:25:33PM +0100, Ben Dooks wrote:
> On 28/06/10 12:28, Maurus Cuelenaere wrote:

> > No need for the '? 1 : 0'

> I tihnk there is, IIRC there is no neccessity for a compiler to produce
> one for true, just that the result be !0

The compiler will *produce* 1 for true, but it will accept any non-zero
value as true.

RE: [PATCH v3 01/12] s3c-fb: Fix various null references on

[Pawel Osciak]

>Ben Dooks <ben@simtec.co.uk> wrote:
>On 28/06/10 09:08, Pawel Osciak wrote:
>> @@ -819,7 +820,8 @@ static void s3c_fb_release_win(struct s3c_fb *sfb,
>struct s3c_fb_win *win)
>>  {
>>  	if (win->fbinfo) {
>>  		unregister_framebuffer(win->fbinfo);
>> -		fb_dealloc_cmap(&win->fbinfo->cmap);
>> +		if (&win->fbinfo->cmap)
>> +			fb_dealloc_cmap(&win->fbinfo->cmap);
>
>did you really mean &win->fbinfo->cmap? surely that will end up
>always evaluating to true?
>

It should have actually been:

+		if (win->fbinfo->cmap.len)
+			fb_dealloc_cmap(&win->fbinfo->cmap);

Thanks, will post a fix for this patch.


Best regards
--
Pawel Osciak
Linux Platform Group
Samsung Poland R&D Center

RE: [PATCH v3 03/12] s3c-fb: Separate S5PC100 and S5PV210 framebuffer

[Pawel Osciak]

> Ben Dooks <ben@simtec.co.uk> wrote:
>On 28/06/10 09:08, Pawel Osciak wrote:
>> S5PC100 and S5PV210 framebuffer devices differ slightly in terms of
>> available registers and their driver data structures have to be separate.
>
>Would have been nice to note which ones changes, otherwise ok.

For this series, the difference is the existence of PRTCON and SHADOWCON
registers. There are more AFAIR. I am not sure if it'd be a good place
to list all of them though...

Best regards
--
Pawel Osciak
Linux Platform Group
Samsung Poland R&D Center

Re: [PATCH v3 08/12] s3c-fb: Add SHADOWCON shadow register locking

[Ben Dooks]

On 28/06/10 09:08, Pawel Osciak wrote:
> S5PV210 allows per-window locking of register value updates from shadow
> registers.
> 
> Signed-off-by: Pawel Osciak <p.osciak@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  arch/arm/plat-samsung/include/plat/regs-fb.h |    3 ++
>  drivers/video/s3c-fb.c                       |   39 +++++++++++++++++++++++--
>  2 files changed, 38 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/plat-samsung/include/plat/regs-fb.h b/arch/arm/plat-samsung/include/plat/regs-fb.h
> index 5bcdd09..da54b64 100644
> --- a/arch/arm/plat-samsung/include/plat/regs-fb.h
> +++ b/arch/arm/plat-samsung/include/plat/regs-fb.h
> @@ -218,6 +218,9 @@
>  #define WINCON1_BPPMODE_25BPP_A1888		(0xd << 2)
>  #define WINCON1_BPPMODE_28BPP_A4888		(0xd << 2)
>  
> +/* S5PV210 */
> +#define SHADOWCON				(0x34)
> +#define SHADOWCON_WINx_PROTECT(_win)		(1 << (10 + _win))

you should have () around the _win arg.

>  
>  #define VIDOSDxA_TOPLEFT_X_MASK			(0x7ff << 11)
>  #define VIDOSDxA_TOPLEFT_X_SHIFT		(11)
> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
> index 0a93fca..94423c5 100644
> --- a/drivers/video/s3c-fb.c
> +++ b/drivers/video/s3c-fb.c
> @@ -79,6 +79,7 @@ struct s3c_fb;
>   * @osd: The base for the OSD registers.
>   * @palette: Address of palette memory, or 0 if none.
>   * @has_prtcon: Set if has PRTCON register.
> + * @has_shadowcon: Set if has SHADOWCON register.
>   */
>  struct s3c_fb_variant {
>  	unsigned int	is_2443:1;
> @@ -95,6 +96,7 @@ struct s3c_fb_variant {
>  	unsigned short	palette[S3C_FB_MAX_WIN];
>  
>  	unsigned int	has_prtcon:1;
> +	unsigned int	has_shadowcon:1;
>  };
>  
>  /**
> @@ -363,6 +365,36 @@ static int s3c_fb_align_word(unsigned int bpp, unsigned int pix)
>  }
>  
>  /**
> + * shadow_protect_win() - disable updating values from shadow registers at vsync
> + *
> + * @win: window to protect registers for
> + * @protect: 1 to protect (disable updates)
> + */
> +static void shadow_protect_win(struct s3c_fb_win *win, int protect)

technically, protect should be of type 'bool'/

Re: [PATCH v3 09/12] s3c-fb: Correct window osd size and alpha register

[Ben Dooks]

On 28/06/10 09:08, Pawel Osciak wrote:
> S3C64xx and S5P OSD registers for OSD size and alpha are as follows:
> VIDOSDC: win 0 - size, win 1-4: alpha
> VIDOSDD: win 1-2 - size; not present for windows 0, 3 and 4
> 
> Signed-off-by: Pawel Osciak <p.osciak@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/video/s3c-fb.c |   58 ++++++++++++++++++++++++++++++++++++++++++-----
>  1 files changed, 51 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
> index 94423c5..3b2c7fe 100644
> --- a/drivers/video/s3c-fb.c
> +++ b/drivers/video/s3c-fb.c
> @@ -64,6 +64,9 @@ struct s3c_fb;
>  #define VIDOSD_B(win, variant) (OSD_BASE(win, variant) + 0x04)
>  #define VIDOSD_C(win, variant) (OSD_BASE(win, variant) + 0x08)
>  #define VIDOSD_D(win, variant) (OSD_BASE(win, variant) + 0x0C)
> +#define VIDOSD_SIZE(win, variant, win_variant) \
> +	(OSD_BASE(win, variant) + (win_variant).osd_size_off)
> +#define VIDOSD_ALPHA(win, variant, win_variant) VIDOSD_C(win, variant)

hmm, this is becoming a bit complicated. if we have a function to
set it then maybe we should just calculate it there.

>  /**
>   * struct s3c_fb_variant - fb variant information
> @@ -112,7 +115,10 @@ struct s3c_fb_variant {
>  struct s3c_fb_win_variant {
>  	unsigned int	has_osd_c:1;
>  	unsigned int	has_osd_d:1;
> +	unsigned int	has_osd_size:1;
> +	unsigned int	has_osd_alpha:1;
>  	unsigned int	palette_16bpp:1;
> +	unsigned short	osd_size_off;
>  	unsigned short	palette_sz;
>  	u32		valid_bpp;
>  };
> @@ -365,6 +371,36 @@ static int s3c_fb_align_word(unsigned int bpp, unsigned int pix)
>  }
>  
>  /**
> + * vidosd_set_size() - set OSD size for a window
> + *
> + * @win: the window to set OSD size for
> + * @size: OSD size register value
> + */
> +static void vidosd_set_size(struct s3c_fb_win *win, u32 size)
> +{
> +	struct s3c_fb *sfb = win->parent;
> +
> +	if (win->variant.has_osd_size)
> +		writel(size, sfb->regs + VIDOSD_SIZE(win->index, sfb->variant,
> +							win->variant));
> +}
> +
> +/**
> + * vidosd_set_alpha() - set alpha transparency for a window
> + *
> + * @win: the window to set OSD size for
> + * @alpha: alpha register value
> + */
> +static void vidosd_set_alpha(struct s3c_fb_win *win, u32 alpha)
> +{
> +	struct s3c_fb *sfb = win->parent;
> +
> +	if (win->variant.has_osd_alpha)
> +		writel(alpha, sfb->regs + VIDOSD_ALPHA(win->index,
> +						sfb->variant, win->variant));
> +}
> +
> +/**
>   * shadow_protect_win() - disable updating values from shadow registers at vsync
>   *
>   * @win: window to protect registers for
> @@ -408,7 +444,7 @@ static int s3c_fb_set_par(struct fb_info *info)
>  	void __iomem *regs = sfb->regs;
>  	void __iomem *buf = regs;
>  	int win_no = win->index;
> -	u32 osdc_data = 0;
> +	u32 alpha = 0;
>  	u32 data;
>  	u32 pagewidth;
>  	int clkdiv;
> @@ -511,15 +547,12 @@ static int s3c_fb_set_par(struct fb_info *info)
>  
>  	data = var->xres * var->yres;
>  
> -	osdc_data = VIDISD14C_ALPHA1_R(0xf) |
> +	alpha = VIDISD14C_ALPHA1_R(0xf) |
>  		VIDISD14C_ALPHA1_G(0xf) |
>  		VIDISD14C_ALPHA1_B(0xf);
>  
> -	if (win->variant.has_osd_d) {
> -		writel(data, regs + VIDOSD_D(win_no, sfb->variant));
> -		writel(osdc_data, regs + VIDOSD_C(win_no, sfb->variant));
> -	} else
> -		writel(data, regs + VIDOSD_C(win_no, sfb->variant));
> +	vidosd_set_alpha(win, alpha);
> +	vidosd_set_size(win, data);
>  
>  	data = WINCONx_ENWIN;
>  
> @@ -1445,12 +1478,17 @@ static int s3c_fb_resume(struct platform_device *pdev)
>  static struct s3c_fb_win_variant s3c_fb_data_64xx_wins[] __devinitdata = {
>  	[0] = {
>  		.has_osd_c	= 1,
> +		.has_osd_size	= 1,
> +		.osd_size_off	= 0x8,
>  		.palette_sz	= 256,
>  		.valid_bpp	= VALID_BPP1248 | VALID_BPP(16) | VALID_BPP(24),
>  	},
>  	[1] = {
>  		.has_osd_c	= 1,
>  		.has_osd_d	= 1,
> +		.has_osd_size	= 1,
> +		.osd_size_off	= 0x12,
> +		.has_osd_alpha	= 1,

how about osd_size_off !=0 => has_osd_size ?

do we need to change the osd c and d definitions?

RE: [PATCH v3 05/12] s3c-fb: Add support for display panning

[Pawel Osciak]

>Ben Dooks <ben@simtec.co.uk> wrote:
>On 28/06/10 09:08, Pawel Osciak wrote:

>> diff --git a/arch/arm/plat-samsung/include/plat/regs-fb.h b/arch/arm/plat-
>samsung/include/plat/regs-fb.h
>> index ac10013..f454e32 100644
>> --- a/arch/arm/plat-samsung/include/plat/regs-fb.h
>> +++ b/arch/arm/plat-samsung/include/plat/regs-fb.h
>> @@ -112,6 +112,11 @@
>>  #define VIDCON2_ORGYCbCr			(1 << 8)
>>  #define VIDCON2_YUVORDCrCb			(1 << 7)
>>
>> +/* PRTCON (S3C6410, S5PC100) */
>
>Not listed in my S3C6410 manual?
>

Not in my either. It is there in the S5PC100 docs though. This is not an
"official statement", but I've been testing panning on my 6410 *very*
intensively and experienced a lot of crashes without it. Not a single one
with it though. So it's either almost impossible luck or it is there,
undocumented. There is a hole at this address anyway, so this shouldn't
hurt anything.

>> @@ -735,6 +741,66 @@ static int s3c_fb_blank(int blank_mode, struct fb_info
>*info)
>>  	return 0;
>>  }
>>
>> +/**
>> + * s3c_fb_pan_display() - Pan the display.
>> + *
>> + * Note that the offsets can be written to the device at any time, as
>their
>> + * values are latched at each vsync automatically. This also means that
>only
>> + * the last call to this function will have any effect on next vsync, but
>> + * there is no need to sleep waiting for it to prevent tearing.
>> + *
>> + * @var: The screen information to verify.
>> + * @info: The framebuffer device.
>> + */
>> +static int s3c_fb_pan_display(struct fb_var_screeninfo *var,
>> +			      struct fb_info *info)
>> +{
>> +	struct s3c_fb_win *win	= info->par;
>> +	struct s3c_fb *sfb	= win->parent;
>> +	void __iomem *buf	= sfb->regs + win->index * 8;
>> +	unsigned int start_byte_offset, end_byte_offset;
>
>could have shortened to start and end, might have made some of the
>calculation easier.
>

Ok, will fix.

>> +	/* Offset in bytes to the start of the displayed area */
>> +	start_byte_offset = var->yoffset * info->fix.line_length;
>> +	/* X offset depends on the current bpp */
>> +	if (info->var.bits_per_pixel >= 8) {
>> +		start_byte_offset +>> +			var->xoffset * (info->var.bits_per_pixel >> 3);
>> +	} else {
>> +		switch (info->var.bits_per_pixel) {
>> +		case 4:
>> +			start_byte_offset += var->xoffset >> 1;
>> +			break;
>> +		case 2:
>> +			start_byte_offset += var->xoffset >> 2;
>> +			break;
>> +		case 1:
>> +			start_byte_offset += var->xoffset >> 3;
>> +			break;
>> +		default:
>> +			dev_err(sfb->dev, "invalid bpp\n");
>> +			return -EINVAL;
>> +		}
>> +	}
>
>this could have been done by changing it to
>start_byte_offset += var->xoffset / (8 / info->var.bits_per_pixel);
>

Good point, although I wanted to have no division here... Whichever you
prefer then...

>note, you always set pan step to 1, evne if the case of bpp less than 8.
>

Hmm... so I guess I should be changing xpanstep on bpp switch in fb_set_par
if needed?


Best regards
--
Pawel Osciak
Linux Platform Group
Samsung Poland R&D Center

RE: [PATCH v3 06/12] s3c-fb: Add wait for VSYNC ioctl

[Pawel Osciak]

>Ben Dooks <mailto:ben@simtec.co.uk> wrote:
>On 28/06/10 09:08, Pawel Osciak wrote:
>> @@ -801,6 +825,124 @@ static int s3c_fb_pan_display(struct fb_var_screeninfo *var,
>>  	return 0;
>>  }
>>
>> +/**
>> + * s3c_fb_enable_irq() - enable framebuffer interrupts
>> + * @sfb: main hardware state
>> + */
>> +static void s3c_fb_enable_irq(struct s3c_fb *sfb)
>> +{
>> +	void __iomem *regs = sfb->regs;
>> +	u32 irq_ctrl_reg;
>> +
>> +	if (!test_and_set_bit(S3C_FB_VSYNC_IRQ_EN, &sfb->irq_flags)) {
>> +		/* IRQ disabled, enable it */
>> +		irq_ctrl_reg = readl(regs + VIDINTCON0);
>> +
>> +		irq_ctrl_reg |= VIDINTCON0_INT_ENABLE;
>> +		irq_ctrl_reg |= VIDINTCON0_INT_FRAME;
>> +
>> +		irq_ctrl_reg &= ~VIDINTCON0_FRAMESEL0_MASK;
>> +		irq_ctrl_reg |= VIDINTCON0_FRAMESEL0_VSYNC;
>> +		irq_ctrl_reg &= ~VIDINTCON0_FRAMESEL1_MASK;
>> +		irq_ctrl_reg |= VIDINTCON0_FRAMESEL1_NONE;
>> +
>> +		writel(irq_ctrl_reg, regs + VIDINTCON0);
>> +	}
>> +}
>
>there should probably be some form of locking so that an irq
>doesn't come through and disable this. possibly in the call
>that queues the request to reduce the likelyhood of any races.
>

Swapping the count and enable_irq below will fix this.

>> +
>> +/**
>> + * s3c_fb_disable_irq() - disable framebuffer interrupts
>> + * @sfb: main hardware state
>> + */
>> +static void s3c_fb_disable_irq(struct s3c_fb *sfb)
>> +{
>> +	void __iomem *regs = sfb->regs;
>> +	u32 irq_ctrl_reg;
>> +
>> +	if (test_and_clear_bit(S3C_FB_VSYNC_IRQ_EN, &sfb->irq_flags)) {
>> +		/* IRQ enabled, disable it */
>> +		irq_ctrl_reg = readl(regs + VIDINTCON0);
>> +
>> +		irq_ctrl_reg &= ~VIDINTCON0_INT_FRAME;
>> +		irq_ctrl_reg &= ~VIDINTCON0_INT_ENABLE;
>> +
>> +		writel(irq_ctrl_reg, regs + VIDINTCON0);
>> +	}
>> +}
>> +
>> +static irqreturn_t s3c_fb_irq(int irq, void *dev_id)
>> +{
>> +	struct s3c_fb *sfb = dev_id;
>> +	void __iomem  *regs = sfb->regs;
>> +	u32 irq_sts_reg;
>> +
>> +	irq_sts_reg = readl(regs + VIDINTCON1);
>> +
>> +	if (irq_sts_reg & VIDINTCON1_INT_FRAME) {
>> +
>> +		/* VSYNC interrupt, accept it */
>> +		writel(VIDINTCON1_INT_FRAME, regs + VIDINTCON1);
>> +
>> +		sfb->vsync_info.count++;
>> +		wake_up_interruptible(&sfb->vsync_info.wait);
>> +	}
>> +
>> +	/* We only support waiting for VSYNC for now, so it's safe
>> +	 * to always disable irqs here.
>> +	 */
>> +	s3c_fb_disable_irq(sfb);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +/**
>> + * s3c_fb_wait_for_vsync() - sleep until next VSYNC interrupt or timeout
>> + * @sfb: main hardware state
>> + * @crtc: head index.
>> + */
>> +static int s3c_fb_wait_for_vsync(struct s3c_fb *sfb, u32 crtc)
>> +{
>> +	unsigned long count;
>> +	int ret;
>> +
>> +	if (crtc != 0)
>> +		return -ENODEV;
>> +
>> +	s3c_fb_enable_irq(sfb);
>> +	count = sfb->vsync_info.count;
>
>possibly doing count before enabling the irq.
>

yes... can't get my head around to what I've been thinking when writing this,
it's been a year ago... I remember being quite sure that it'd work that way
then though, as ridiculous as it looks right now...
Will swap them.

>> +	ret = wait_event_interruptible_timeout(sfb->vsync_info.wait,
>> +				       count != sfb->vsync_info.count,
>> +				       msecs_to_jiffies(VSYNC_TIMEOUT_MSEC));
>> +	if (ret = 0)
>> +		return -ETIMEDOUT;
>> +
>> +	return 0;
>> +}
>> +
>> +static int s3c_fb_ioctl(struct fb_info *info, unsigned int cmd,
>> +			unsigned long arg)
>> +{
>> +	struct s3c_fb_win *win = info->par;
>> +	struct s3c_fb *sfb = win->parent;
>> +	int ret;
>> +	u32 crtc;
>> +
>> +	switch (cmd) {
>> +	case FBIO_WAITFORVSYNC:
>> +		if (get_user(crtc, (u32 __user *)arg)) {
>> +			ret = -EFAULT;
>> +			break;
>> +		}
>
>can't find any info on what the argument is meant to be.
>

Head number, i.e. crt no I believe. Not sure though.

>> +
>> +		ret = s3c_fb_wait_for_vsync(sfb, crtc);
>> +		break;
>> +	default:
>> +		ret = -ENOTTY;
>
>Can someone else confirm this is the correct err?
>

Linux Device Drivers 3, page 140, or see the online version here:
http://www.makelinux.net/ldd3/chp-6-sect-1.shtml
section 6.1.2. The Return Value

man ioctl:
ENOTTY 
The specified request does not apply to the kind of object that the
descriptor d references.

>> @@ -1370,6 +1534,7 @@ static struct s3c_fb_driverdata s3c_fb_data_s5pv210
>__devinitdata = {
>>  			[3] = 0x3000,
>>  			[4] = 0x3400,
>>  		},
>> +
>>  	},
>
>oops, added whitespace.
>

will fix.


Best regards
--
Pawel Osciak
Linux Platform Group
Samsung Poland R&D Center

RE: [PATCH v3 08/12] s3c-fb: Add SHADOWCON shadow register locking

[Pawel Osciak]

>Ben Dooks <ben@simtec.co.uk> wrote:
>On 28/06/10 09:08, Pawel Osciak wrote:
>> @@ -218,6 +218,9 @@
>>  #define WINCON1_BPPMODE_25BPP_A1888		(0xd << 2)
>>  #define WINCON1_BPPMODE_28BPP_A4888		(0xd << 2)
>>
>> +/* S5PV210 */
>> +#define SHADOWCON				(0x34)
>> +#define SHADOWCON_WINx_PROTECT(_win)		(1 << (10 + _win))
>
>you should have () around the _win arg.
>

Good catch, thanks.

>>
>>  #define VIDOSDxA_TOPLEFT_X_MASK			(0x7ff << 11)
>>  #define VIDOSDxA_TOPLEFT_X_SHIFT		(11)
>> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
>> index 0a93fca..94423c5 100644
>> --- a/drivers/video/s3c-fb.c
>> +++ b/drivers/video/s3c-fb.c
>> @@ -79,6 +79,7 @@ struct s3c_fb;
>>   * @osd: The base for the OSD registers.
>>   * @palette: Address of palette memory, or 0 if none.
>>   * @has_prtcon: Set if has PRTCON register.
>> + * @has_shadowcon: Set if has SHADOWCON register.
>>   */
>>  struct s3c_fb_variant {
>>  	unsigned int	is_2443:1;
>> @@ -95,6 +96,7 @@ struct s3c_fb_variant {
>>  	unsigned short	palette[S3C_FB_MAX_WIN];
>>
>>  	unsigned int	has_prtcon:1;
>> +	unsigned int	has_shadowcon:1;
>>  };
>>
>>  /**
>> @@ -363,6 +365,36 @@ static int s3c_fb_align_word(unsigned int bpp,
>unsigned int pix)
>>  }
>>
>>  /**
>> + * shadow_protect_win() - disable updating values from shadow registers at
>vsync
>> + *
>> + * @win: window to protect registers for
>> + * @protect: 1 to protect (disable updates)
>> + */
>> +static void shadow_protect_win(struct s3c_fb_win *win, int protect)
>
>technically, protect should be of type 'bool'

Well... ok, I guess it's the "new" trend ;)


Best regards
--
Pawel Osciak
Linux Platform Group
Samsung Poland R&D Center

RE: [PATCH v3 09/12] s3c-fb: Correct window osd size and alpha

[Pawel Osciak]

>Ben Dooks <ben@simtec.co.uk> wrote:
>On 28/06/10 09:08, Pawel Osciak wrote:
>> S3C64xx and S5P OSD registers for OSD size and alpha are as follows:
>> VIDOSDC: win 0 - size, win 1-4: alpha
>> VIDOSDD: win 1-2 - size; not present for windows 0, 3 and 4
>>
>> Signed-off-by: Pawel Osciak <p.osciak@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>>  drivers/video/s3c-fb.c |   58 ++++++++++++++++++++++++++++++++++++++++++--
>---
>>  1 files changed, 51 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
>> index 94423c5..3b2c7fe 100644
>> --- a/drivers/video/s3c-fb.c
>> +++ b/drivers/video/s3c-fb.c
>> @@ -64,6 +64,9 @@ struct s3c_fb;
>>  #define VIDOSD_B(win, variant) (OSD_BASE(win, variant) + 0x04)
>>  #define VIDOSD_C(win, variant) (OSD_BASE(win, variant) + 0x08)
>>  #define VIDOSD_D(win, variant) (OSD_BASE(win, variant) + 0x0C)
>> +#define VIDOSD_SIZE(win, variant, win_variant) \
>> +	(OSD_BASE(win, variant) + (win_variant).osd_size_off)
>> +#define VIDOSD_ALPHA(win, variant, win_variant) VIDOSD_C(win, variant)
>
>hmm, this is becoming a bit complicated. if we have a function to
>set it then maybe we should just calculate it there.
>

>> @@ -1445,12 +1478,17 @@ static int s3c_fb_resume(struct platform_device
>*pdev)
>>  static struct s3c_fb_win_variant s3c_fb_data_64xx_wins[] __devinitdata = {
>>  	[0] = {
>>  		.has_osd_c	= 1,
>> +		.has_osd_size	= 1,
>> +		.osd_size_off	= 0x8,
>>  		.palette_sz	= 256,
>>  		.valid_bpp	= VALID_BPP1248 | VALID_BPP(16) | VALID_BPP(24),
>>  	},
>>  	[1] = {
>>  		.has_osd_c	= 1,
>>  		.has_osd_d	= 1,
>> +		.has_osd_size	= 1,
>> +		.osd_size_off	= 0x12,
>> +		.has_osd_alpha	= 1,
>
>how about osd_size_off !=0 => has_osd_size ?
>

Could be done that way, yes... I'm not aware of any SoC with osd_size_off = 0
till now.

>do we need to change the osd c and d definitions?

I'd prefer not using "OSD_C" and "OSD_D" names at all. How they are changing
their usage across hw versions and windows is confusing enough. Maybe we should
only be using osd_sizeand osd_alpha instead? Would you agree?


Best regards
--
Pawel Osciak
Linux Platform Group
Samsung Poland R&D Center

Re: [PATCH v3 05/12] s3c-fb: Add support for display panning

[Jamie Lokier]

Pawel Osciak wrote:
> >Ben Dooks <ben@simtec.co.uk> wrote:
> >On 28/06/10 09:08, Pawel Osciak wrote:
> 
> >> diff --git a/arch/arm/plat-samsung/include/plat/regs-fb.h b/arch/arm/plat-
> >samsung/include/plat/regs-fb.h
> >> index ac10013..f454e32 100644
> >> --- a/arch/arm/plat-samsung/include/plat/regs-fb.h
> >> +++ b/arch/arm/plat-samsung/include/plat/regs-fb.h
> >> @@ -112,6 +112,11 @@
> >>  #define VIDCON2_ORGYCbCr			(1 << 8)
> >>  #define VIDCON2_YUVORDCrCb			(1 << 7)
> >>
> >> +/* PRTCON (S3C6410, S5PC100) */
> >
> >Not listed in my S3C6410 manual?
> >
> 
> Not in my either. It is there in the S5PC100 docs though. This is not an
> "official statement", but I've been testing panning on my 6410 *very*
> intensively and experienced a lot of crashes without it. Not a single one
> with it though. So it's either almost impossible luck or it is there,
> undocumented. There is a hole at this address anyway, so this shouldn't
> hurt anything.

Probably a good idea to put that in a comment, for the person who
looks at it in 5 years time and wonders if the code is mistaken.

-- Jamie

Re: [PATCH v3 01/12] s3c-fb: Fix various null references on framebuffer

[James Simmons]

> On 28/06/10 09:08, Pawel Osciak wrote:
> > The following problems were found in the above situation:
> > 
> > sfb->windows[win] was being assigned at the end of s3c_fb_probe_win only.
> > This resulted in passing a NULL to s3c_fb_release_win if probe_win returned
> > early and a memory leak.
> > 
> > dma_free_writecombine does not allow its third argument to be NULL.
> > 
> > fb_dealloc_cmap does not verify whether its argument is not NULL.
> > 
> > Signed-off-by: Pawel Osciak <p.osciak@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> >  drivers/video/s3c-fb.c |    8 +++++---
> >  1 files changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
> > index b00c064..d998324 100644
> > --- a/drivers/video/s3c-fb.c
> > +++ b/drivers/video/s3c-fb.c
> > @@ -804,7 +804,8 @@ static void s3c_fb_free_memory(struct s3c_fb *sfb, struct s3c_fb_win *win)
> >  {
> >  	struct fb_info *fbi = win->fbinfo;
> >  
> > -	dma_free_writecombine(sfb->dev, PAGE_ALIGN(fbi->fix.smem_len),
> > +	if (fbi->screen_base)
> > +		dma_free_writecombine(sfb->dev, PAGE_ALIGN(fbi->fix.smem_len),
> >  			      fbi->screen_base, fbi->fix.smem_start);
> >  }
> >  
> > @@ -819,7 +820,8 @@ static void s3c_fb_release_win(struct s3c_fb *sfb, struct s3c_fb_win *win)
> >  {
> >  	if (win->fbinfo) {
> >  		unregister_framebuffer(win->fbinfo);
> > -		fb_dealloc_cmap(&win->fbinfo->cmap);
> > +		if (&win->fbinfo->cmap)
> > +			fb_dealloc_cmap(&win->fbinfo->cmap);
> 
> did you really mean &win->fbinfo->cmap? surely that will end up
> always evaluating to true?

Ouch. If you fail to allocate a color map at boot time no fbdev apps will 
work. Including fbcon.
 
> >  		s3c_fb_free_memory(sfb, win);
> >  		framebuffer_release(win->fbinfo);
> >  	}
> > @@ -865,6 +867,7 @@ static int __devinit s3c_fb_probe_win(struct s3c_fb *sfb, unsigned int win_no,
> >  	WARN_ON(windata->win_mode.yres = 0);
> >  
> >  	win = fbinfo->par;
> > +	*res = win;
> >  	var = &fbinfo->var;
> >  	win->variant = *variant;
> >  	win->fbinfo = fbinfo;
> > @@ -939,7 +942,6 @@ static int __devinit s3c_fb_probe_win(struct s3c_fb *sfb, unsigned int win_no,
> >  		return ret;
> >  	}
> >  
> > -	*res = win;
> >  	dev_info(sfb->dev, "window %d: fb %s\n", win_no, fbinfo->fix.id);
> >  
> >  	return 0;
> 
> --
> 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
>