Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: Linux 2.6.38-rc6
From: Anca Emanuel @ 2011-02-24 13:20 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Dave Airlie, linux-fbdev, Ben Skeggs, dri-devel, Borislav Petkov,
	Linux Kernel Mailing List
In-Reply-To: <AANLkTinD2cC9dGg2eBtZQhWvhSsuH2BcWDj8byLhWWso@mail.gmail.com>

On Thu, Feb 24, 2011 at 2:28 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Wed, Feb 23, 2011 at 9:16 AM, Anca Emanuel <anca.emanuel@gmail.com> wrote:
>>>
>>> Looks like nouveafb took over from vesafb. Did you do anything special
>>> to trigger this?
>>
>> No. Just boot the system.
>
> Every boot?

Yes.

>
> And just out of interest, what happens if you don't have the vesafb
> driver at all?
>
>                          Linus
>

I used 'e' option from grub, removed the 'set gfxpayload = $linux_gfx_mode'
and it works.

dmesg: http://pastebin.com/JAZsk4vD

^ permalink raw reply

* Re: Linux 2.6.38-rc6
From: Linus Torvalds @ 2011-02-24 16:37 UTC (permalink / raw)
  To: Anca Emanuel
  Cc: Dave Airlie, linux-fbdev, Ben Skeggs, dri-devel, Borislav Petkov,
	Linux Kernel Mailing List
In-Reply-To: <AANLkTin8+F003-jFoQ0pd9OjN1oT2iqyNjd58SDJZuMK@mail.gmail.com>

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

On Thu, Feb 24, 2011 at 5:20 AM, Anca Emanuel <anca.emanuel@gmail.com> wrote:
>>
>> Every boot?
>
> Yes.
>
>> And just out of interest, what happens if you don't have the vesafb
>> driver at all?
>
> I used 'e' option from grub, removed the 'set gfxpayload = $linux_gfx_mode'
> and it works.
>
> dmesg: http://pastebin.com/JAZsk4vD

Hmm. So it definitely seems to be the hand-over.

Does this patch make any difference? When we unregister the old
framebuffer, we still leave it in the registered_fb[] array, which
looks wrong. But it would also be interesting to hear if setting
CONFIG_SLUB_DEBUG_ON or CONFIG_DEBUG_PAGEALLOC makes any difference
(they'd help detect accesses to free'd data structures).

                          Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 497 bytes --]

 drivers/video/fbmem.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index e2bf953..e8f8925 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1511,6 +1511,7 @@ void remove_conflicting_framebuffers(struct apertures_struct *a,
 			       "%s vs %s - removing generic driver\n",
 			       name, registered_fb[i]->fix.id);
 			unregister_framebuffer(registered_fb[i]);
+			registered_fb[i] = NULL;
 		}
 	}
 }

^ permalink raw reply related

* Re: [PATCH v6] OMAP2/3/4: DSS2: Enable Display SubSystem as modules
From: Tony Lindgren @ 2011-02-24 21:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1298536312.16119.23.camel@deskari>

* Tomi Valkeinen <tomi.valkeinen@ti.com> [110224 00:30]:
> On Thu, 2011-02-24 at 00:26 -0600, Nilofer, Samreen wrote:
> > Enabling all the display interface options to be built as module
> > And enabling all the display panels to be built as modules.
> > 
> > Signed-off-by: Samreen <samreen@ti.com>
> 
> This looks good to me.
> 
> Tony, want to ack this? This enables all DSS features as modules. This
> should go through dss tree, as this doesn't work without a fix that is
> there.

Sounds good to me:

Acked-by: Tony Lindgren <tony@atomide.com>

^ permalink raw reply

* Re: [PATCH] drivers: ld9040 amoled driver support
From: Andrew Morton @ 2011-02-24 23:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1294893628-16998-1-git-send-email-dh09.lee@samsung.com>

On Thu, 13 Jan 2011 13:40:28 +0900
Donghwa Lee <dh09.lee@samsung.com> wrote:

> This patch is ld9040 amoled panel driver.
> I resend because there was no comment.
> 
> It is similar to s6e63m0 panel driver and use spi gpio to send panel
> information.
> 
>
> ...
>
> +#define POWER_IS_ON(pwr)	((pwr) <= FB_BLANK_NORMAL)

This could and should be implemented as a regular lower-case C function.

> +struct ld9040 {
> +	struct device			*dev;
> +	struct spi_device		*spi;
> +	unsigned int			power;
> +	unsigned int			gamma_mode;
> +	unsigned int			current_brightness;
> +	unsigned int			gamma_table_count;
> +
> +	struct lcd_device		*ld;
> +	struct backlight_device		*bd;
> +	struct lcd_platform_data	*lcd_pd;
> +};
> +
> +static const unsigned short SEQ_SWRESET[] = {
> +	0x01, COMMAND_ONLY,
> +	ENDDEF, 0x00
> +};

It's strange that all these tables have upper-case names.  Unless
there's some special reason for doing it this way, please make them
regular lower-case identifiers.

>
> ...
>
> +static int ld9040_ldi_init(struct ld9040 *lcd)
> +{
> +	int ret, i;
> +	const unsigned short *init_seq[] = {
> +		SEQ_USER_SETTING,
> +		SEQ_PANEL_CONDITION,
> +		SEQ_DISPCTL,
> +		SEQ_MANPWR,
> +		SEQ_PWR_CTRL,
> +		SEQ_ELVSS_ON,
> +		SEQ_GTCON,
> +		SEQ_GAMMA_SET1,
> +		SEQ_GAMMA_CTRL,
> +		SEQ_SLPOUT,
> +	};

Make this table static so the compiler doesn't rebuild it on the stack
each time the function is called.  The compiler probably already
performs that optimisation, but it doesn't hurt to be explicit.


> +	for (i = 0; i < ARRAY_SIZE(init_seq); i++) {
> +		ret = ld9040_panel_send_sequence(lcd, init_seq[i]);
> +		/* workaround: minimum delay time for transferring CMD */
> +		udelay(300);
> +		if (ret)
> +			break;
> +	}
> +
> +	return ret;
> +}
> +
>
> ...
>
> +static ssize_t ld9040_sysfs_show_gamma_mode(struct device *dev,
> +				      struct device_attribute *attr, char *buf)
> +{
> +	struct ld9040 *lcd = dev_get_drvdata(dev);
> +	char temp[10];
> +
> +	switch (lcd->gamma_mode) {
> +	case 0:
> +		sprintf(temp, "2.2 mode\n");
> +		strcat(buf, temp);

Could do sprintf(buf + strlen(buf)) and eliminate temp[].

> +		break;
> +	default:
> +		dev_info(dev, "gamma mode could be 0:2.2\n");
> +		break;
> +	}
> +
> +	return strlen(buf);
> +}
> +
> +static ssize_t ld9040_sysfs_store_gamma_mode(struct device *dev,
> +				       struct device_attribute *attr,
> +				       const char *buf, size_t len)
> +{
> +	struct ld9040 *lcd = dev_get_drvdata(dev);
> +	struct backlight_device *bd = NULL;
> +	int brightness, rc;
> +
> +	rc = strict_strtoul(buf, 0, (unsigned long *)&lcd->gamma_mode);

That's a bug.  gamma_mode has type int, which is 4 bytes, but you're
telling strict_strtoul() to write eight bytes - it will corrupt *lcd on
a 64-bit machine.  Use a temporary.


> +	if (rc < 0)
> +		return rc;
> +
> +	bd = lcd->bd;
> +
> +	brightness = bd->props.brightness;
> +
> +	switch (lcd->gamma_mode) {
> +	case 0:
> +		_ld9040_gamma_ctl(lcd, gamma_table.gamma_22_table[brightness]);
> +		break;
> +	default:
> +		dev_info(dev, "gamma mode could be 0:2.2\n");
> +		_ld9040_gamma_ctl(lcd, gamma_table.gamma_22_table[brightness]);
> +		break;
> +	}
> +	return len;
> +}
> +
> +static DEVICE_ATTR(gamma_mode, 0644,
> +		ld9040_sysfs_show_gamma_mode, ld9040_sysfs_store_gamma_mode);
> +
> +static ssize_t ld9040_sysfs_show_gamma_table(struct device *dev,
> +				      struct device_attribute *attr, char *buf)
> +{
> +	struct ld9040 *lcd = dev_get_drvdata(dev);
> +	char temp[3];
> +
> +	sprintf(temp, "%d\n", lcd->gamma_table_count);
> +	strcpy(buf, temp);

sprintf(buf + strlen(buf), remove temp[].

> +	return strlen(buf);
> +}
> +
> +static DEVICE_ATTR(gamma_table, 0644,
> +		ld9040_sysfs_show_gamma_table, NULL);
> +
> +static int ld9040_probe(struct spi_device *spi)
> +{
> +	int ret = 0;
> +	struct ld9040 *lcd = NULL;
> +	struct lcd_device *ld = NULL;
> +	struct backlight_device *bd = NULL;
> +
> +	lcd = kzalloc(sizeof(struct ld9040), GFP_KERNEL);
> +	if (!lcd)
> +		return -ENOMEM;
> +
> +	/* ld9040 lcd panel uses 3-wire 9bits SPI Mode. */
> +	spi->bits_per_word = 9;
> +
> +	ret = spi_setup(spi);
> +	if (ret < 0) {
> +		dev_err(&spi->dev, "spi setup failed.\n");
> +		goto out_free_lcd;
> +	}
> +
> +	lcd->spi = spi;
> +	lcd->dev = &spi->dev;
> +
> +	lcd->lcd_pd = (struct lcd_platform_data *)spi->dev.platform_data;

Unneeded and undesirable typecast.

> +	if (!lcd->lcd_pd) {
> +		dev_err(&spi->dev, "platform data is NULL.\n");
> +		goto out_free_lcd;
> +	}
> +
> +	ld = lcd_device_register("ld9040", &spi->dev, lcd, &ld9040_lcd_ops);
> +	if (IS_ERR(ld)) {
> +		ret = PTR_ERR(ld);
> +		goto out_free_lcd;
> +	}
> +
> +	lcd->ld = ld;
> +
> +	bd = backlight_device_register("ld9040-bl", &spi->dev,
> +		lcd, &ld9040_backlight_ops, NULL);
> +	if (IS_ERR(ld)) {
> +		ret = PTR_ERR(ld);
> +		goto out_free_lcd;
> +	}
> +
> +	bd->props.max_brightness = MAX_BRIGHTNESS;
> +	bd->props.brightness = MAX_BRIGHTNESS;
> +	lcd->bd = bd;
> +
> +	/*
> +	 * it gets gamma table count available so it gets user
> +	 * know that.
> +	 */
> +	lcd->gamma_table_count > +	    sizeof(gamma_table) / (MAX_GAMMA_LEVEL * sizeof(int));
> +
> +	ret = device_create_file(&(spi->dev), &dev_attr_gamma_mode);
> +	if (ret < 0)
> +		dev_err(&(spi->dev), "failed to add sysfs entries\n");
> +
> +	ret = device_create_file(&(spi->dev), &dev_attr_gamma_table);
> +	if (ret < 0)
> +		dev_err(&(spi->dev), "failed to add sysfs entries\n");

These errors are just ignored.  It would be better to perform cleanup
and to then return the correct errno.

> +	/*
> +	 * if lcd panel was on from bootloader like u-boot then
> +	 * do not lcd on.
> +	 */
> +	if (!lcd->lcd_pd->lcd_enabled) {
> +		/*
> +		 * if lcd panel was off from bootloader then
> +		 * current lcd status is powerdown and then
> +		 * it enables lcd panel.
> +		 */
> +		lcd->power = FB_BLANK_POWERDOWN;
> +
> +		ld9040_power(lcd, FB_BLANK_UNBLANK);
> +	} else
> +		lcd->power = FB_BLANK_UNBLANK;
> +
> +	dev_set_drvdata(&spi->dev, lcd);
> +
> +	dev_info(&spi->dev, "ld9040 panel driver has been probed.\n");
> +	return 0;
> +
> +out_free_lcd:
> +	kfree(lcd);
> +	return ret;
> +}
> +
> +static int __devexit ld9040_remove(struct spi_device *spi)
> +{
> +	struct ld9040 *lcd = dev_get_drvdata(&spi->dev);
> +
> +	ld9040_power(lcd, FB_BLANK_POWERDOWN);
> +	lcd_device_unregister(lcd->ld);
> +	kfree(lcd);
> +
> +	return 0;
> +}
> +
> +#if defined(CONFIG_PM)
> +unsigned int beforepower;

This should be static.

Also, it shouldn't exist.  Its presence assumes that there will only be
one device controlled by this driver.  It should be moved into the
per-device data area.

>
> ...
>
> +struct ld9040_gamma {
> +	unsigned int *gamma_22_table[MAX_GAMMA_LEVEL];
> +};
> +
> +static struct ld9040_gamma gamma_table = {

Could do

static struct ld9040_gamma {
	unsigned int *gamma_22_table[MAX_GAMMA_LEVEL];
} gamma_table = {

here.

> +	.gamma_22_table[0] = (unsigned int *)&ld9040_22_50,
> +	.gamma_22_table[1] = (unsigned int *)&ld9040_22_70,
> +	.gamma_22_table[2] = (unsigned int *)&ld9040_22_80,
> +	.gamma_22_table[3] = (unsigned int *)&ld9040_22_90,
> +	.gamma_22_table[4] = (unsigned int *)&ld9040_22_100,
> +	.gamma_22_table[5] = (unsigned int *)&ld9040_22_110,
> +	.gamma_22_table[6] = (unsigned int *)&ld9040_22_120,
> +	.gamma_22_table[7] = (unsigned int *)&ld9040_22_130,
> +	.gamma_22_table[8] = (unsigned int *)&ld9040_22_140,
> +	.gamma_22_table[9] = (unsigned int *)&ld9040_22_150,
> +	.gamma_22_table[10] = (unsigned int *)&ld9040_22_160,
> +	.gamma_22_table[11] = (unsigned int *)&ld9040_22_170,
> +	.gamma_22_table[12] = (unsigned int *)&ld9040_22_180,
> +	.gamma_22_table[13] = (unsigned int *)&ld9040_22_190,
> +	.gamma_22_table[14] = (unsigned int *)&ld9040_22_200,
> +	.gamma_22_table[15] = (unsigned int *)&ld9040_22_210,
> +	.gamma_22_table[16] = (unsigned int *)&ld9040_22_220,
> +	.gamma_22_table[17] = (unsigned int *)&ld9040_22_230,
> +	.gamma_22_table[18] = (unsigned int *)&ld9040_22_240,
> +	.gamma_22_table[19] = (unsigned int *)&ld9040_22_250,
> +	.gamma_22_table[20] = (unsigned int *)&ld9040_22_260,
> +	.gamma_22_table[21] = (unsigned int *)&ld9040_22_270,
> +	.gamma_22_table[22] = (unsigned int *)&ld9040_22_280,
> +	.gamma_22_table[23] = (unsigned int *)&ld9040_22_290,
> +	.gamma_22_table[24] = (unsigned int *)&ld9040_22_300,
> +};
> +
> +#endif

Defining static tables in a header file is strange.  It would be very
wrong if that header was ever included by more than a single .c file,
so why not just eliminate this file and move its contents into that .c
file?


^ permalink raw reply

* Re: Linux 2.6.38-rc6
From: Anca Emanuel @ 2011-02-25  0:48 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-fbdev, Herton Ronaldo Krzesinski, Linux Kernel Mailing List,
	dri-devel, Ben Skeggs, Borislav Petkov, Dave Airlie
In-Reply-To: <AANLkTinzse1ZfEvAuWfuZ_8VDypcnD5poU6GwOdYTgWh@mail.gmail.com>

On Thu, Feb 24, 2011 at 6:37 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Thu, Feb 24, 2011 at 5:20 AM, Anca Emanuel <anca.emanuel@gmail.com> wrote:
>>>
>>> Every boot?
>>
>> Yes.
>>
>>> And just out of interest, what happens if you don't have the vesafb
>>> driver at all?
>>
>> I used 'e' option from grub, removed the 'set gfxpayload = $linux_gfx_mode'
>> and it works.
>>
>> dmesg: http://pastebin.com/JAZsk4vD
>
> Hmm. So it definitely seems to be the hand-over.
>
> Does this patch make any difference? When we unregister the old
> framebuffer, we still leave it in the registered_fb[] array, which
> looks wrong. But it would also be interesting to hear if setting
> CONFIG_SLUB_DEBUG_ON or CONFIG_DEBUG_PAGEALLOC makes any difference
> (they'd help detect accesses to free'd data structures).
>
>                          Linus
>
 drivers/video/fbmem.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index e2bf953..e8f8925 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1511,6 +1511,7 @@ void remove_conflicting_framebuffers(struct
apertures_struct *a,
 			       "%s vs %s - removing generic driver\n",
 			       name, registered_fb[i]->fix.id);
 			unregister_framebuffer(registered_fb[i]);
+			registered_fb[i] = NULL;
 		}
 	}
 }


Tested the patch, and now I get this:
dmesg: http://pastebin.com/ieMNrA7C

[   12.252328] BUG: unable to handle kernel NULL pointer dereference
at 00000000000003b8
[   12.252342] IP: [<ffffffff81311178>] fb_mmap+0x58/0x1d0
[   12.252354] PGD 78e6c067 PUD 78e6d067 PMD 0
[   12.252360] Oops: 0000 [#1] SMP
[   12.252364] last sysfs file: /sys/module/snd/initstate
[   12.252370] CPU 0
[   12.252372] Modules linked in: nouveau(+) snd ttm drm_kms_helper
psmouse serio_raw drm soundcore lp snd_page_alloc i2c_algo_bit video
parport pata_marvell ahci r8169 libahci
[   12.252393]
[   12.252397] Pid: 244, comm: plymouthd Not tainted
2.6.38-rc6-git3-patch-linus+ #2 MICRO-STAR INTERNATIONAL CO.,LTD
MS-7360/MS-7360
[   12.252407] RIP: 0010:[<ffffffff81311178>]  [<ffffffff81311178>]
fb_mmap+0x58/0x1d0
[   12.252414] RSP: 0018:ffff880078e8fd88  EFLAGS: 00010293
[   12.252418] RAX: 00000000ffffffea RBX: ffff88007047d228 RCX: 0000000000000000
[   12.252423] RDX: 000fffffffffffff RSI: ffff88007047d228 RDI: ffff880078f5d840
[   12.252428] RBP: ffff880078e8fdc8 R08: 0000000000000000 R09: ffff88007047d228
[   12.252432] R10: ffff88006f9d9cf0 R11: ffff88006f9d9d28 R12: ffff880037363800
[   12.252437] R13: 0000000000000000 R14: 0000000000000000 R15: ffff88007047d228
[   12.252442] FS:  00007fb5fbaa4720(0000) GS:ffff88007fc00000(0000)
knlGS:0000000000000000
[   12.252448] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   12.252453] CR2: 00000000000003b8 CR3: 0000000078e6b000 CR4: 00000000000006f0
[   12.252458] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   12.252463] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[   12.252468] Process plymouthd (pid: 244, threadinfo
ffff880078e8e000, task ffff88003737ad80)
[   12.252473] Stack:
[   12.252476]  ffff880037363800 00000000000000b8 ffff880078e8fdd8
ffffffffffffffea
[   12.252484]  ffff880037363800 00000000000006bb 00000000006bb000
ffff88007047d228
[   12.252491]  ffff880078e8fe98 ffffffff81130543 ffff880078f5d840
0000000000000000
[   12.252499] Call Trace:
[   12.252507]  [<ffffffff81130543>] mmap_region+0x3c3/0x500
[   12.252514]  [<ffffffff81010d7e>] ?
arch_get_unmapped_area_topdown+0x1ce/0x2f0
[   12.252521]  [<ffffffff811309c4>] do_mmap_pgoff+0x344/0x380
[   12.252528]  [<ffffffff810524f1>] ? finish_task_switch+0x41/0xe0
[   12.252535]  [<ffffffff815ac0c3>] ? schedule+0x403/0xa00
[   12.252541]  [<ffffffff81130bfe>] sys_mmap_pgoff+0x1fe/0x230
[   12.252546]  [<ffffffff810108c9>] sys_mmap+0x29/0x30
[   12.252551]  [<ffffffff8100bf02>] system_call_fastpath+0x16/0x1b
[   12.252556] Code: ba ff ff ff ff ff ff 0f 00 48 89 f3 48 8b 40 30
8b 80 b8 00 00 00 25 ff ff 0f 00 49 39 d6 4c 8b 2c c5 c0 cf aa 81 b8
ea ff ff ff <4d> 8b bd b8 03 00 00 76 1f 48 8b 5d d8 4c 8b 65 e0 4c 8b
6d e8
[   12.252603] RIP  [<ffffffff81311178>] fb_mmap+0x58/0x1d0
[   12.252608]  RSP <ffff880078e8fd88>
[   12.252611] CR2: 00000000000003b8
[   12.252616] ---[ end trace 381165bafe65d748 ]---

^ permalink raw reply related

* Re: Linux 2.6.38-rc6
From: Linus Torvalds @ 2011-02-25  0:54 UTC (permalink / raw)
  To: Anca Emanuel
  Cc: Dave Airlie, linux-fbdev, Ben Skeggs, dri-devel, Borislav Petkov,
	Herton Ronaldo Krzesinski, Linux Kernel Mailing List
In-Reply-To: <AANLkTi=wbtcAOmYE8UF7ynPZ_nR3ZXxVRBcuMVGNK4u0@mail.gmail.com>

On Thu, Feb 24, 2011 at 4:48 PM, Anca Emanuel <anca.emanuel@gmail.com> wrote:
>
> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> index e2bf953..e8f8925 100644
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -1511,6 +1511,7 @@ void remove_conflicting_framebuffers(struct
> apertures_struct *a,
>                               "%s vs %s - removing generic driver\n",
>                               name, registered_fb[i]->fix.id);
>                        unregister_framebuffer(registered_fb[i]);
> +                       registered_fb[i] = NULL;
>
> Tested the patch, and now I get this:
> dmesg: http://pastebin.com/ieMNrA7C
>
> [   12.252328] BUG: unable to handle kernel NULL pointer dereference
> at 00000000000003b8
> [   12.252342] IP: [<ffffffff81311178>] fb_mmap+0x58/0x1d0

Ok, goodie.

Or not so goodie, but it does make it clear that yeah, the fb code
seems to be using stale pointers from that registered_fb[] array, and
the whole unregistration process is just racing with people using it.

Herton had that much bigger patch, can you test it?

                         Linus

^ permalink raw reply

* Re: Linux 2.6.38-rc6
From: Dave Airlie @ 2011-02-25  1:14 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Anca Emanuel, linux-fbdev, Ben Skeggs, dri-devel, Borislav Petkov,
	Herton Ronaldo Krzesinski, Linux Kernel Mailing List
In-Reply-To: <AANLkTin7+AHAqKz5T-1_=N6tqPytyT4ooqyJuhHC6ET=@mail.gmail.com>

On Thu, 2011-02-24 at 16:54 -0800, Linus Torvalds wrote:
> On Thu, Feb 24, 2011 at 4:48 PM, Anca Emanuel <anca.emanuel@gmail.com> wrote:
> >
> > diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> > index e2bf953..e8f8925 100644
> > --- a/drivers/video/fbmem.c
> > +++ b/drivers/video/fbmem.c
> > @@ -1511,6 +1511,7 @@ void remove_conflicting_framebuffers(struct
> > apertures_struct *a,
> >                               "%s vs %s - removing generic driver\n",
> >                               name, registered_fb[i]->fix.id);
> >                        unregister_framebuffer(registered_fb[i]);
> > +                       registered_fb[i] = NULL;
> >
> > Tested the patch, and now I get this:
> > dmesg: http://pastebin.com/ieMNrA7C
> >
> > [   12.252328] BUG: unable to handle kernel NULL pointer dereference
> > at 00000000000003b8
> > [   12.252342] IP: [<ffffffff81311178>] fb_mmap+0x58/0x1d0
> 
> Ok, goodie.
> 
> Or not so goodie, but it does make it clear that yeah, the fb code
> seems to be using stale pointers from that registered_fb[] array, and
> the whole unregistration process is just racing with people using it.
> 
> Herton had that much bigger patch, can you test it?

I think Andy's patch worked, not sure why it fell between the cracks,
either didn't appear on lkml or in my inbox at all.

if we can get Herton to repost it properly + a tested by I'm happy for
it to go in.

Dave.


^ permalink raw reply

* Re: Linux 2.6.38-rc6
From: Anca Emanuel @ 2011-02-25  1:47 UTC (permalink / raw)
  To: Dave Airlie
  Cc: Linus Torvalds, linux-fbdev, Ben Skeggs, dri-devel,
	Borislav Petkov, Herton Ronaldo Krzesinski,
	Linux Kernel Mailing List
In-Reply-To: <1298596499.10585.27.camel@clockmaker-el6>

On Fri, Feb 25, 2011 at 3:14 AM, Dave Airlie <airlied@redhat.com> wrote:
> On Thu, 2011-02-24 at 16:54 -0800, Linus Torvalds wrote:
>> On Thu, Feb 24, 2011 at 4:48 PM, Anca Emanuel <anca.emanuel@gmail.com> wrote:
>> >
>> > diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
>> > index e2bf953..e8f8925 100644
>> > --- a/drivers/video/fbmem.c
>> > +++ b/drivers/video/fbmem.c
>> > @@ -1511,6 +1511,7 @@ void remove_conflicting_framebuffers(struct
>> > apertures_struct *a,
>> >                               "%s vs %s - removing generic driver\n",
>> >                               name, registered_fb[i]->fix.id);
>> >                        unregister_framebuffer(registered_fb[i]);
>> > +                       registered_fb[i] = NULL;
>> >
>> > Tested the patch, and now I get this:
>> > dmesg: http://pastebin.com/ieMNrA7C
>> >
>> > [   12.252328] BUG: unable to handle kernel NULL pointer dereference
>> > at 00000000000003b8
>> > [   12.252342] IP: [<ffffffff81311178>] fb_mmap+0x58/0x1d0
>>
>> Ok, goodie.
>>
>> Or not so goodie, but it does make it clear that yeah, the fb code
>> seems to be using stale pointers from that registered_fb[] array, and
>> the whole unregistration process is just racing with people using it.
>>
>> Herton had that much bigger patch, can you test it?
>
> I think Andy's patch worked, not sure why it fell between the cracks,
> either didn't appear on lkml or in my inbox at all.
>
> if we can get Herton to repost it properly + a tested by I'm happy for
> it to go in.
>
> Dave.
>
>

Tested Andy's patch and it works !
http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-natty.git;a=commit;hÅa742b5f78e161d6a13853a7e3e6e1dfa429e69

Tested-by: Anca Emanuel <anca.emanuel@gmail.com>

^ permalink raw reply

* Re: Linux 2.6.38-rc6
From: Anca Emanuel @ 2011-02-25  1:56 UTC (permalink / raw)
  To: Dave Airlie
  Cc: Linus Torvalds, linux-fbdev, Ben Skeggs, dri-devel,
	Borislav Petkov, Herton Ronaldo Krzesinski,
	Linux Kernel Mailing List
In-Reply-To: <AANLkTikQOBGfe8dGv05M8YD=CK+ToUQ4J8s_n9FncfPA@mail.gmail.com>

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

On Fri, Feb 25, 2011 at 3:47 AM, Anca Emanuel <anca.emanuel@gmail.com> wrote:
> On Fri, Feb 25, 2011 at 3:14 AM, Dave Airlie <airlied@redhat.com> wrote:
>> On Thu, 2011-02-24 at 16:54 -0800, Linus Torvalds wrote:
>>> On Thu, Feb 24, 2011 at 4:48 PM, Anca Emanuel <anca.emanuel@gmail.com> wrote:
>>> >
>>> > diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
>>> > index e2bf953..e8f8925 100644
>>> > --- a/drivers/video/fbmem.c
>>> > +++ b/drivers/video/fbmem.c
>>> > @@ -1511,6 +1511,7 @@ void remove_conflicting_framebuffers(struct
>>> > apertures_struct *a,
>>> >                               "%s vs %s - removing generic driver\n",
>>> >                               name, registered_fb[i]->fix.id);
>>> >                        unregister_framebuffer(registered_fb[i]);
>>> > +                       registered_fb[i] = NULL;
>>> >
>>> > Tested the patch, and now I get this:
>>> > dmesg: http://pastebin.com/ieMNrA7C
>>> >
>>> > [   12.252328] BUG: unable to handle kernel NULL pointer dereference
>>> > at 00000000000003b8
>>> > [   12.252342] IP: [<ffffffff81311178>] fb_mmap+0x58/0x1d0
>>>
>>> Ok, goodie.
>>>
>>> Or not so goodie, but it does make it clear that yeah, the fb code
>>> seems to be using stale pointers from that registered_fb[] array, and
>>> the whole unregistration process is just racing with people using it.
>>>
>>> Herton had that much bigger patch, can you test it?
>>
>> I think Andy's patch worked, not sure why it fell between the cracks,
>> either didn't appear on lkml or in my inbox at all.
>>
>> if we can get Herton to repost it properly + a tested by I'm happy for
>> it to go in.
>>
>> Dave.
>>
>>
>
> Tested Andy's patch and it works !
> http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-natty.git;a=commit;h=c5a742b5f78e161d6a13853a7e3e6e1dfa429e69
>
> Tested-by: Anca Emanuel <anca.emanuel@gmail.com>
>

link to patch: http://is.gd/otIfGc

[-- Attachment #2: patch --]
[-- Type: application/octet-stream, Size: 9935 bytes --]

From c5a742b5f78e161d6a13853a7e3e6e1dfa429e69 Mon Sep 17 00:00:00 2001
From: Andy Whitcroft <apw@canonical.com>
Date: Thu, 29 Jul 2010 16:48:20 +0100
Subject: [PATCH 1/1] UBUNTU: SAUCE: fbcon -- fix race between open and removal of framebuffers

Currently there is no locking for updates to the registered_fb list.
This allows an open through /dev/fbN to pick up a registered framebuffer
pointer in parallel with it being released, as happens when a conflicting
framebuffer is ejected or on module unload.  There is also no reference
counting on the framebuffer descriptor which is referenced from all open
files, leading to references to released or reused memory to persist on
these open files.

This patch adds a reference count to the framebuffer descriptor to prevent
it from being released until after all pending opens are closed.  This
allows the pending opens to detect the closed status and unmap themselves.
It also adds locking to the framebuffer lookup path, locking it against
the removal path such that it is possible to atomically lookup and take a
reference to the descriptor.  It also adds locking to the read and write
paths which currently could access the framebuffer descriptor after it
has been freed.  Finally it moves the device to FBINFO_STATE_REMOVED to
indicate that all access should be errored for this device.

Signed-off-by: Andy Whitcroft <apw@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
Signed-off-by: Leann Ogasawara <leann.ogasawara@canonical.com>
---
 drivers/video/fbmem.c |  132 ++++++++++++++++++++++++++++++++++++++-----------
 include/linux/fb.h    |    2 +
 2 files changed, 105 insertions(+), 29 deletions(-)

diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index e2bf953..877200b 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -42,6 +42,8 @@
 
 #define FBPIXMAPSIZE	(1024 * 8)
 
+/* Protects the registered framebuffer list and count. */
+static DEFINE_SPINLOCK(registered_lock);
 struct fb_info *registered_fb[FB_MAX] __read_mostly;
 int num_registered_fb __read_mostly;
 
@@ -694,9 +696,7 @@ static ssize_t
 fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 {
 	unsigned long p = *ppos;
-	struct inode *inode = file->f_path.dentry->d_inode;
-	int fbidx = iminor(inode);
-	struct fb_info *info = registered_fb[fbidx];
+	struct fb_info *info = file->private_data;
 	u8 *buffer, *dst;
 	u8 __iomem *src;
 	int c, cnt = 0, err = 0;
@@ -705,19 +705,28 @@ fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 	if (!info || ! info->screen_base)
 		return -ENODEV;
 
-	if (info->state != FBINFO_STATE_RUNNING)
-		return -EPERM;
+	if (!lock_fb_info(info))
+		return -ENODEV;
+
+	if (info->state != FBINFO_STATE_RUNNING) {
+		err = -EPERM;
+		goto out_fb_info;
+	}
 
-	if (info->fbops->fb_read)
-		return info->fbops->fb_read(info, buf, count, ppos);
+	if (info->fbops->fb_read) {
+		err = info->fbops->fb_read(info, buf, count, ppos);
+		goto out_fb_info;
+	}
 	
 	total_size = info->screen_size;
 
 	if (total_size == 0)
 		total_size = info->fix.smem_len;
 
-	if (p >= total_size)
-		return 0;
+	if (p >= total_size) {
+		err = 0;
+		goto out_fb_info;
+	}
 
 	if (count >= total_size)
 		count = total_size;
@@ -727,8 +736,10 @@ fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 
 	buffer = kmalloc((count > PAGE_SIZE) ? PAGE_SIZE : count,
 			 GFP_KERNEL);
-	if (!buffer)
-		return -ENOMEM;
+	if (!buffer) {
+		err = -ENOMEM;
+		goto out_fb_info;
+	}
 
 	src = (u8 __iomem *) (info->screen_base + p);
 
@@ -751,19 +762,21 @@ fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 		cnt += c;
 		count -= c;
 	}
+	if (!err)
+		err = cnt;
 
 	kfree(buffer);
+out_fb_info:
+	unlock_fb_info(info);
 
-	return (err) ? err : cnt;
+	return err;
 }
 
 static ssize_t
 fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 {
 	unsigned long p = *ppos;
-	struct inode *inode = file->f_path.dentry->d_inode;
-	int fbidx = iminor(inode);
-	struct fb_info *info = registered_fb[fbidx];
+	struct fb_info *info = file->private_data;
 	u8 *buffer, *src;
 	u8 __iomem *dst;
 	int c, cnt = 0, err = 0;
@@ -772,8 +785,13 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 	if (!info || !info->screen_base)
 		return -ENODEV;
 
-	if (info->state != FBINFO_STATE_RUNNING)
-		return -EPERM;
+	if (!lock_fb_info(info))
+		return -ENODEV;
+
+	if (info->state != FBINFO_STATE_RUNNING) {
+		err = -EPERM;
+		goto out_fb_info;
+	}
 
 	if (info->fbops->fb_write)
 		return info->fbops->fb_write(info, buf, count, ppos);
@@ -783,8 +801,10 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 	if (total_size == 0)
 		total_size = info->fix.smem_len;
 
-	if (p > total_size)
-		return -EFBIG;
+	if (p > total_size) {
+		err = -EFBIG;
+		goto out_fb_info;
+	}
 
 	if (count > total_size) {
 		err = -EFBIG;
@@ -800,8 +820,10 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 
 	buffer = kmalloc((count > PAGE_SIZE) ? PAGE_SIZE : count,
 			 GFP_KERNEL);
-	if (!buffer)
-		return -ENOMEM;
+	if (!buffer) {
+		err = -ENOMEM;
+		goto out_fb_info;
+	}
 
 	dst = (u8 __iomem *) (info->screen_base + p);
 
@@ -825,10 +847,14 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 		cnt += c;
 		count -= c;
 	}
+	if (cnt)
+		err = cnt;
 
 	kfree(buffer);
+out_fb_info:
+	unlock_fb_info(info);
 
-	return (cnt) ? cnt : err;
+	return err;
 }
 
 int
@@ -1303,8 +1329,7 @@ static long fb_compat_ioctl(struct file *file, unsigned int cmd,
 static int
 fb_mmap(struct file *file, struct vm_area_struct * vma)
 {
-	int fbidx = iminor(file->f_path.dentry->d_inode);
-	struct fb_info *info = registered_fb[fbidx];
+	struct fb_info * const info = file->private_data;
 	struct fb_ops *fb = info->fbops;
 	unsigned long off;
 	unsigned long start;
@@ -1316,6 +1341,11 @@ fb_mmap(struct file *file, struct vm_area_struct * vma)
 	if (!fb)
 		return -ENODEV;
 	mutex_lock(&info->mm_lock);
+	if (info->state == FBINFO_STATE_REMOVED) {
+		mutex_unlock(&info->mm_lock);
+		return -ENODEV;
+	}
+
 	if (fb->fb_mmap) {
 		int res;
 		res = fb->fb_mmap(info, vma);
@@ -1352,6 +1382,34 @@ fb_mmap(struct file *file, struct vm_area_struct * vma)
 	return 0;
 }
 
+static struct fb_info *get_framebuffer_info(int idx)
+__acquires(&registered_lock)
+__releases(&registered_lock)
+{
+	struct fb_info *fb_info;
+
+	spin_lock(&registered_lock);
+	fb_info = registered_fb[idx];
+	fb_info->ref_count++;
+	spin_unlock(&registered_lock);
+
+	return fb_info;
+}
+
+static void put_framebuffer_info(struct fb_info *fb_info)
+__acquires(&registered_lock)
+__releases(&registered_lock)
+{
+	int keep;
+
+	spin_lock(&registered_lock);
+	keep = --fb_info->ref_count;
+	spin_unlock(&registered_lock);
+	
+	if (!keep && fb_info->fbops->fb_destroy)
+		fb_info->fbops->fb_destroy(fb_info);
+}
+
 static int
 fb_open(struct inode *inode, struct file *file)
 __acquires(&info->lock)
@@ -1363,13 +1421,17 @@ __releases(&info->lock)
 
 	if (fbidx >= FB_MAX)
 		return -ENODEV;
-	info = registered_fb[fbidx];
+	info = get_framebuffer_info(fbidx);
 	if (!info)
 		request_module("fb%d", fbidx);
-	info = registered_fb[fbidx];
+	info = get_framebuffer_info(fbidx);
 	if (!info)
 		return -ENODEV;
 	mutex_lock(&info->lock);
+	if (info->state == FBINFO_STATE_REMOVED) {
+		res = -ENODEV;
+		goto out;
+	}
 	if (!try_module_get(info->fbops->owner)) {
 		res = -ENODEV;
 		goto out;
@@ -1386,6 +1448,8 @@ __releases(&info->lock)
 #endif
 out:
 	mutex_unlock(&info->lock);
+	if (res)
+		put_framebuffer_info(info);
 	return res;
 }
 
@@ -1401,6 +1465,7 @@ __releases(&info->lock)
 		info->fbops->fb_release(info,1);
 	module_put(info->fbops->owner);
 	mutex_unlock(&info->lock);
+	put_framebuffer_info(info);
 	return 0;
 }
 
@@ -1549,6 +1614,7 @@ register_framebuffer(struct fb_info *fb_info)
 	fb_info->node = i;
 	mutex_init(&fb_info->lock);
 	mutex_init(&fb_info->mm_lock);
+	fb_info->ref_count = 1;
 
 	fb_info->dev = device_create(fb_class, fb_info->device,
 				     MKDEV(FB_MAJOR, i), NULL, "fb%d", i);
@@ -1592,7 +1658,6 @@ register_framebuffer(struct fb_info *fb_info)
 	return 0;
 }
 
-
 /**
  *	unregister_framebuffer - releases a frame buffer device
  *	@fb_info: frame buffer info structure
@@ -1627,6 +1692,16 @@ unregister_framebuffer(struct fb_info *fb_info)
 		return -ENODEV;
 	event.info = fb_info;
 	ret = fb_notifier_call_chain(FB_EVENT_FB_UNBIND, &event);
+	if (!ret) {
+		mutex_lock(&fb_info->mm_lock);
+		/*
+		 * We must prevent any operations for this transition, we
+		 * already have info->lock so grab the info->mm_lock to hold
+		 * the remainder.
+		 */
+		fb_info->state = FBINFO_STATE_REMOVED;
+		mutex_unlock(&fb_info->mm_lock);
+	}
 	unlock_fb_info(fb_info);
 
 	if (ret) {
@@ -1646,8 +1721,7 @@ unregister_framebuffer(struct fb_info *fb_info)
 	fb_notifier_call_chain(FB_EVENT_FB_UNREGISTERED, &event);
 
 	/* this may free fb info */
-	if (fb_info->fbops->fb_destroy)
-		fb_info->fbops->fb_destroy(fb_info);
+	put_framebuffer_info(fb_info);
 done:
 	return ret;
 }
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 68ba85a..1e8b785 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -832,6 +832,7 @@ struct fb_tile_ops {
 struct fb_info {
 	int node;
 	int flags;
+	int ref_count;
 	struct mutex lock;		/* Lock for open/release/ioctl funcs */
 	struct mutex mm_lock;		/* Lock for fb_mmap and smem_* fields */
 	struct fb_var_screeninfo var;	/* Current var */
@@ -871,6 +872,7 @@ struct fb_info {
 	void *pseudo_palette;		/* Fake palette of 16 colors */ 
 #define FBINFO_STATE_RUNNING	0
 #define FBINFO_STATE_SUSPENDED	1
+#define FBINFO_STATE_REMOVED	2
 	u32 state;			/* Hardware state i.e suspend */
 	void *fbcon_par;                /* fbcon use-only private area */
 	/* From here on everything is device dependent */
-- 
1.7.0.4

^ permalink raw reply related

* Re: [PATCH] drivers: ld9040 amoled driver support
From: Donghwa Lee @ 2011-02-25  2:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20110224154250.266b2688.akpm@linux-foundation.org>

 On Fri, 25 Feb 2011 8:42 @t, Andrew Morton wrote:
> On Thu, 13 Jan 2011 13:40:28 +0900
> Donghwa Lee <dh09.lee@samsung.com> wrote:
>
>>  +#define POWER_IS_ON(pwr)	((pwr) <= FB_BLANK_NORMAL)
> This could and should be implemented as a regular lower-case C function.
>
Ok, I will change to lower-case.
>> +
>> +static const unsigned short SEQ_SWRESET[] = {
>> +	0x01, COMMAND_ONLY,
>> +	ENDDEF, 0x00
>> +};
> It's strange that all these tables have upper-case names.  Unless
> there's some special reason for doing it this way, please make them
> regular lower-case identifiers.
>
It means specific register setting tables, but, on second thoughts, it is meaningless. I will change it.
>> ...
>>
>> +static int ld9040_ldi_init(struct ld9040 *lcd)
>> +{
>> +	int ret, i;
>> +	const unsigned short *init_seq[] = {
>> +		SEQ_USER_SETTING,
>> +		SEQ_PANEL_CONDITION,
>> +		SEQ_DISPCTL,
>> +		SEQ_MANPWR,
>> +		SEQ_PWR_CTRL,
>> +		SEQ_ELVSS_ON,
>> +		SEQ_GTCON,
>> +		SEQ_GAMMA_SET1,
>> +		SEQ_GAMMA_CTRL,
>> +		SEQ_SLPOUT,
>> +	};
> Make this table static so the compiler doesn't rebuild it on the stack
> each time the function is called.  The compiler probably already
> performs that optimisation, but it doesn't hurt to be explicit.
>
Ok, I will modify it.
>  +static ssize_t ld9040_sysfs_show_gamma_mode(struct device *dev,
> +				      struct device_attribute *attr, char *buf)
> +{
> +	struct ld9040 *lcd = dev_get_drvdata(dev);
> +	char temp[10];
> +
> +	switch (lcd->gamma_mode) {
> +	case 0:
> +		sprintf(temp, "2.2 mode\n");
> +		strcat(buf, temp);
> Could do sprintf(buf + strlen(buf)) and eliminate temp[].
Current gamma table is only one, so I remove all about gamma mode.
>> +static ssize_t ld9040_sysfs_store_gamma_mode(struct device *dev,
>> +				       struct device_attribute *attr,
>> +				       const char *buf, size_t len)
>> +{
>> +	struct ld9040 *lcd = dev_get_drvdata(dev);
>> +	struct backlight_device *bd = NULL;
>> +	int brightness, rc;
>> +
>> +	rc = strict_strtoul(buf, 0, (unsigned long *)&lcd->gamma_mode);
> That's a bug.  gamma_mode has type int, which is 4 bytes, but you're
> telling strict_strtoul() to write eight bytes - it will corrupt *lcd on
> a 64-bit machine.  Use a temporary.
>
Current gamma table is only one, so I remove all about gamma mode.
>> +
>> +static DEVICE_ATTR(gamma_mode, 0644,
>> +		ld9040_sysfs_show_gamma_mode, ld9040_sysfs_store_gamma_mode);
>> +
>> +static ssize_t ld9040_sysfs_show_gamma_table(struct device *dev,
>> +				      struct device_attribute *attr, char *buf)
>> +{
>> +	struct ld9040 *lcd = dev_get_drvdata(dev);
>> +	char temp[3];
>> +
>> +	sprintf(temp, "%d\n", lcd->gamma_table_count);
>> +	strcpy(buf, temp);
> sprintf(buf + strlen(buf), remove temp[].
>
Current gamma table is only one, so I remove all about gamma mode.
>> +	return strlen(buf);
>> +}
>> +
>> +static DEVICE_ATTR(gamma_table, 0644,
>> +		ld9040_sysfs_show_gamma_table, NULL);
>> +
>> +static int ld9040_probe(struct spi_device *spi)
>> +{
>> +	int ret = 0;
>> +	struct ld9040 *lcd = NULL;
>> +	struct lcd_device *ld = NULL;
>> +	struct backlight_device *bd = NULL;
>> +
>> +	lcd = kzalloc(sizeof(struct ld9040), GFP_KERNEL);
>> +	if (!lcd)
>> +		return -ENOMEM;
>> +
>> +	/* ld9040 lcd panel uses 3-wire 9bits SPI Mode. */
>> +	spi->bits_per_word = 9;
>> +
>> +	ret = spi_setup(spi);
>> +	if (ret < 0) {
>> +		dev_err(&spi->dev, "spi setup failed.\n");
>> +		goto out_free_lcd;
>> +	}
>> +
>> +	lcd->spi = spi;
>> +	lcd->dev = &spi->dev;
>> +
>> +	lcd->lcd_pd = (struct lcd_platform_data *)spi->dev.platform_data;
> Unneeded and undesirable typecast.
>
Ok, I will modify it.
>> +	/*
>> +	 * it gets gamma table count available so it gets user
>> +	 * know that.
>> +	 */
>> +	lcd->gamma_table_count >> +	    sizeof(gamma_table) / (MAX_GAMMA_LEVEL * sizeof(int));
>> +
>> +	ret = device_create_file(&(spi->dev), &dev_attr_gamma_mode);
>> +	if (ret < 0)
>> +		dev_err(&(spi->dev), "failed to add sysfs entries\n");
>> +
>> +	ret = device_create_file(&(spi->dev), &dev_attr_gamma_table);
>> +	if (ret < 0)
>> +		dev_err(&(spi->dev), "failed to add sysfs entries\n");
> These errors are just ignored.  It would be better to perform cleanup
> and to then return the correct errno.
>
Above, creating sysfs node is unneeded. I will remove it.
>> +
>> +#if defined(CONFIG_PM)
>> +unsigned int beforepower;
> This should be static.
>
> Also, it shouldn't exist.  Its presence assumes that there will only be
> one device controlled by this driver.  It should be moved into the
> per-device data area.
>
Yes, you're right. beforepower variable is unneeded, I will remove it.
>> ...
>>
>> +struct ld9040_gamma {
>> +	unsigned int *gamma_22_table[MAX_GAMMA_LEVEL];
>> +};
>> +
>> +static struct ld9040_gamma gamma_table = {
> Could do
>
> static struct ld9040_gamma {
> 	unsigned int *gamma_22_table[MAX_GAMMA_LEVEL];
> } gamma_table = {
>
> here.
>
Ok, I will move it.
>> +	.gamma_22_table[0] = (unsigned int *)&ld9040_22_50,
>> +	.gamma_22_table[1] = (unsigned int *)&ld9040_22_70,
>> +	.gamma_22_table[2] = (unsigned int *)&ld9040_22_80,
>> +	.gamma_22_table[3] = (unsigned int *)&ld9040_22_90,
>> +	.gamma_22_table[4] = (unsigned int *)&ld9040_22_100,
>> +	.gamma_22_table[5] = (unsigned int *)&ld9040_22_110,
>> +	.gamma_22_table[6] = (unsigned int *)&ld9040_22_120,
>> +	.gamma_22_table[7] = (unsigned int *)&ld9040_22_130,
>> +	.gamma_22_table[8] = (unsigned int *)&ld9040_22_140,
>> +	.gamma_22_table[9] = (unsigned int *)&ld9040_22_150,
>> +	.gamma_22_table[10] = (unsigned int *)&ld9040_22_160,
>> +	.gamma_22_table[11] = (unsigned int *)&ld9040_22_170,
>> +	.gamma_22_table[12] = (unsigned int *)&ld9040_22_180,
>> +	.gamma_22_table[13] = (unsigned int *)&ld9040_22_190,
>> +	.gamma_22_table[14] = (unsigned int *)&ld9040_22_200,
>> +	.gamma_22_table[15] = (unsigned int *)&ld9040_22_210,
>> +	.gamma_22_table[16] = (unsigned int *)&ld9040_22_220,
>> +	.gamma_22_table[17] = (unsigned int *)&ld9040_22_230,
>> +	.gamma_22_table[18] = (unsigned int *)&ld9040_22_240,
>> +	.gamma_22_table[19] = (unsigned int *)&ld9040_22_250,
>> +	.gamma_22_table[20] = (unsigned int *)&ld9040_22_260,
>> +	.gamma_22_table[21] = (unsigned int *)&ld9040_22_270,
>> +	.gamma_22_table[22] = (unsigned int *)&ld9040_22_280,
>> +	.gamma_22_table[23] = (unsigned int *)&ld9040_22_290,
>> +	.gamma_22_table[24] = (unsigned int *)&ld9040_22_300,
>> +};
>> +
>> +#endif
> Defining static tables in a header file is strange.  It would be very
> wrong if that header was ever included by more than a single .c file,
> so why not just eliminate this file and move its contents into that .c
> file?
>
But, if its contents moved into that .c file, ld9040.c  becomes very long file.
Ld9040.c file already had many register set data variable, I think readability is less than
before.


I'm very grateful to you for your review.
Thank you.

Donghwa Lee

^ permalink raw reply

* [PATCH v2] drivers: ld9040 amoled driver support
From: Donghwa Lee @ 2011-02-25  4:57 UTC (permalink / raw)
  To: linux-arm-kernel

This patch is ld9040 amoled panel driver.
I modify first patch and remove unneeded code.

Signed-off-by: Donghwa Lee <dh09.lee@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Inki Dae <inki.dae@samsung.com>
---
 drivers/video/backlight/Kconfig        |    8 +
 drivers/video/backlight/Makefile       |    1 +
 drivers/video/backlight/ld9040.c       |  820 ++++++++++++++++++++++++++++++++
 drivers/video/backlight/ld9040_gamma.h |  201 ++++++++
 4 files changed, 1030 insertions(+), 0 deletions(-)
 create mode 100644 drivers/video/backlight/ld9040.c
 create mode 100644 drivers/video/backlight/ld9040_gamma.h

diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index e54a337..39a4cd2 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -109,6 +109,14 @@ config LCD_S6E63M0
 	  If you have an S6E63M0 LCD Panel, say Y to enable its
 	  LCD control driver.
 
+config LCD_LD9040
+	tristate "LD9040 AMOLED LCD Driver"
+	depends on SPI && BACKLIGHT_CLASS_DEVICE
+	default n
+	help
+	  If you have an LD9040 Panel, say Y to enable its
+	  control driver.
+
 endif # LCD_CLASS_DEVICE
 
 #
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index 44c0f81..457996c 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_LCD_VGG2432A4)	   += vgg2432a4.o
 obj-$(CONFIG_LCD_TDO24M)	   += tdo24m.o
 obj-$(CONFIG_LCD_TOSA)		   += tosa_lcd.o
 obj-$(CONFIG_LCD_S6E63M0)	+= s6e63m0.o
+obj-$(CONFIG_LCD_LD9040)	+= ld9040.o
 
 obj-$(CONFIG_BACKLIGHT_CLASS_DEVICE) += backlight.o
 obj-$(CONFIG_BACKLIGHT_ATMEL_PWM)    += atmel-pwm-bl.o
diff --git a/drivers/video/backlight/ld9040.c b/drivers/video/backlight/ld9040.c
new file mode 100644
index 0000000..fbd4b19
--- /dev/null
+++ b/drivers/video/backlight/ld9040.c
@@ -0,0 +1,820 @@
+/*
+ * ld9040 AMOLED LCD panel driver.
+ *
+ * Copyright (c) 2011 Samsung Electronics
+ * Author: Donghwa Lee  <dh09.lee@samsung.com>
+ * Derived from drivers/video/backlight/s6e63m0.c
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
+ */
+
+#include <linux/wait.h>
+#include <linux/fb.h>
+#include <linux/delay.h>
+#include <linux/gpio.h>
+#include <linux/spi/spi.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/lcd.h>
+#include <linux/backlight.h>
+
+#include "ld9040_gamma.h"
+
+#define SLEEPMSEC		0x1000
+#define ENDDEF			0x2000
+#define	DEFMASK			0xFF00
+#define COMMAND_ONLY		0xFE
+#define DATA_ONLY		0xFF
+
+#define MIN_BRIGHTNESS		0
+#define MAX_BRIGHTNESS		24
+#define power_is_on(pwr)	((pwr) <= FB_BLANK_NORMAL)
+
+struct ld9040 {
+	struct device			*dev;
+	struct spi_device		*spi;
+	unsigned int			power;
+	unsigned int			current_brightness;
+
+	struct lcd_device		*ld;
+	struct backlight_device		*bd;
+	struct lcd_platform_data	*lcd_pd;
+};
+
+static const unsigned short seq_swreset[] = {
+	0x01, COMMAND_ONLY,
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_user_setting[] = {
+	0xF0, 0x5A,
+
+	DATA_ONLY, 0x5A,
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_elvss_on[] = {
+	0xB1, 0x0D,
+
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x16,
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_gtcon[] = {
+	0xF7, 0x09,
+
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x00,
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_panel_condition[] = {
+	0xF8, 0x05,
+
+	DATA_ONLY, 0x65,
+	DATA_ONLY, 0x96,
+	DATA_ONLY, 0x71,
+	DATA_ONLY, 0x7D,
+	DATA_ONLY, 0x19,
+	DATA_ONLY, 0x3B,
+	DATA_ONLY, 0x0D,
+	DATA_ONLY, 0x19,
+	DATA_ONLY, 0x7E,
+	DATA_ONLY, 0x0D,
+	DATA_ONLY, 0xE2,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x7E,
+	DATA_ONLY, 0x7D,
+	DATA_ONLY, 0x07,
+	DATA_ONLY, 0x07,
+	DATA_ONLY, 0x20,
+	DATA_ONLY, 0x20,
+	DATA_ONLY, 0x20,
+	DATA_ONLY, 0x02,
+	DATA_ONLY, 0x02,
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_gamma_set1[] = {
+	0xF9, 0x00,
+
+	DATA_ONLY, 0xA7,
+	DATA_ONLY, 0xB4,
+	DATA_ONLY, 0xAE,
+	DATA_ONLY, 0xBF,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x91,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0xB2,
+	DATA_ONLY, 0xB4,
+	DATA_ONLY, 0xAA,
+	DATA_ONLY, 0xBB,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0xAC,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0xB3,
+	DATA_ONLY, 0xB1,
+	DATA_ONLY, 0xAA,
+	DATA_ONLY, 0xBC,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0xB3,
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_gamma_ctrl[] = {
+	0xFB, 0x02,
+
+	DATA_ONLY, 0x5A,
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_gamma_start[] = {
+	0xF9, COMMAND_ONLY,
+
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_apon[] = {
+	0xF3, 0x00,
+
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x0A,
+	DATA_ONLY, 0x02,
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_display_ctrl[] = {
+	0xF2, 0x02,
+
+	DATA_ONLY, 0x08,
+	DATA_ONLY, 0x08,
+	DATA_ONLY, 0x10,
+	DATA_ONLY, 0x10,
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_manual_pwr[] = {
+	0xB0, 0x04,
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_pwr_ctrl[] = {
+	0xF4, 0x0A,
+
+	DATA_ONLY, 0x87,
+	DATA_ONLY, 0x25,
+	DATA_ONLY, 0x6A,
+	DATA_ONLY, 0x44,
+	DATA_ONLY, 0x02,
+	DATA_ONLY, 0x88,
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_sleep_out[] = {
+	0x11, COMMAND_ONLY,
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_sleep_in[] = {
+	0x10, COMMAND_ONLY,
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_display_on[] = {
+	0x29, COMMAND_ONLY,
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_display_off[] = {
+	0x28, COMMAND_ONLY,
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_vci1_1st_en[] = {
+	0xF3, 0x10,
+
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x02,
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_vl1_en[] = {
+	0xF3, 0x11,
+
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x02,
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_vl2_en[] = {
+	0xF3, 0x13,
+
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x02,
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_vci1_2nd_en[] = {
+	0xF3, 0x33,
+
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x02,
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_vl3_en[] = {
+	0xF3, 0x37,
+
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x02,
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_vreg1_amp_en[] = {
+	0xF3, 0x37,
+
+	DATA_ONLY, 0x01,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x02,
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_vgh_amp_en[] = {
+	0xF3, 0x37,
+
+	DATA_ONLY, 0x11,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x02,
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_vgl_amp_en[] = {
+	0xF3, 0x37,
+
+	DATA_ONLY, 0x31,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x02,
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_vmos_amp_en[] = {
+	0xF3, 0x37,
+
+	DATA_ONLY, 0xB1,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x03,
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_vint_amp_en[] = {
+	0xF3, 0x37,
+
+	DATA_ONLY, 0xF1,
+	/* DATA_ONLY, 0x71,	VMOS/VBL/VBH not used */
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x03,
+	/* DATA_ONLY, 0x02,	VMOS/VBL/VBH not used */
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_vbh_amp_en[] = {
+	0xF3, 0x37,
+
+	DATA_ONLY, 0xF9,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x03,
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_vbl_amp_en[] = {
+	0xF3, 0x37,
+
+	DATA_ONLY, 0xFD,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x03,
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_gam_amp_en[] = {
+	0xF3, 0x37,
+
+	DATA_ONLY, 0xFF,
+	/* DATA_ONLY, 0x73,	VMOS/VBL/VBH not used */
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x03,
+	/* DATA_ONLY, 0x02,	VMOS/VBL/VBH not used */
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_sd_amp_en[] = {
+	0xF3, 0x37,
+
+	DATA_ONLY, 0xFF,
+	/* DATA_ONLY, 0x73,	VMOS/VBL/VBH not used */
+	DATA_ONLY, 0x80,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x03,
+	/* DATA_ONLY, 0x02,	VMOS/VBL/VBH not used */
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_gls_en[] = {
+	0xF3, 0x37,
+
+	DATA_ONLY, 0xFF,
+	/* DATA_ONLY, 0x73,	VMOS/VBL/VBH not used */
+	DATA_ONLY, 0x81,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x03,
+	/* DATA_ONLY, 0x02,	VMOS/VBL/VBH not used */
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_els_en[] = {
+	0xF3, 0x37,
+
+	DATA_ONLY, 0xFF,
+	/* DATA_ONLY, 0x73,	VMOS/VBL/VBH not used */
+	DATA_ONLY, 0x83,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x03,
+	/* DATA_ONLY, 0x02,	VMOS/VBL/VBH not used */
+	ENDDEF, 0x00
+};
+
+static const unsigned short seq_el_on[] = {
+	0xF3, 0x37,
+
+	DATA_ONLY, 0xFF,
+	/* DATA_ONLY, 0x73,	VMOS/VBL/VBH not used */
+	DATA_ONLY, 0x87,
+	DATA_ONLY, 0x00,
+	DATA_ONLY, 0x03,
+	/* DATA_ONLY, 0x02,	VMOS/VBL/VBH not used */
+	ENDDEF, 0x00
+};
+
+static int ld9040_spi_write_byte(struct ld9040 *lcd, int addr, int data)
+{
+	u16 buf[1];
+	struct spi_message msg;
+
+	struct spi_transfer xfer = {
+		.len		= 2,
+		.tx_buf		= buf,
+	};
+
+	buf[0] = (addr << 8) | data;
+
+	spi_message_init(&msg);
+	spi_message_add_tail(&xfer, &msg);
+
+	return spi_sync(lcd->spi, &msg);
+}
+
+static int ld9040_spi_write(struct ld9040 *lcd, unsigned char address,
+	unsigned char command)
+{
+	int ret = 0;
+
+	if (address != DATA_ONLY)
+		ret = ld9040_spi_write_byte(lcd, 0x0, address);
+	if (command != COMMAND_ONLY)
+		ret = ld9040_spi_write_byte(lcd, 0x1, command);
+
+	return ret;
+}
+
+static int ld9040_panel_send_sequence(struct ld9040 *lcd,
+	const unsigned short *wbuf)
+{
+	int ret = 0, i = 0;
+
+	while ((wbuf[i] & DEFMASK) != ENDDEF) {
+		if ((wbuf[i] & DEFMASK) != SLEEPMSEC) {
+			ret = ld9040_spi_write(lcd, wbuf[i], wbuf[i+1]);
+			if (ret)
+				break;
+		} else
+			udelay(wbuf[i+1]*1000);
+		i += 2;
+	}
+
+	return ret;
+}
+
+static int _ld9040_gamma_ctl(struct ld9040 *lcd, const unsigned int *gamma)
+{
+	unsigned int i = 0;
+	int ret = 0;
+
+	/* start gamma table updating. */
+	ret = ld9040_panel_send_sequence(lcd, seq_gamma_start);
+	if (ret) {
+		dev_err(lcd->dev, "failed to disable gamma table updating.\n");
+		goto gamma_err;
+	}
+
+	for (i = 0 ; i < GAMMA_TABLE_COUNT; i++) {
+		ret = ld9040_spi_write(lcd, DATA_ONLY, gamma[i]);
+		if (ret) {
+			dev_err(lcd->dev, "failed to set gamma table.\n");
+			goto gamma_err;
+		}
+	}
+
+	/* update gamma table. */
+	ret = ld9040_panel_send_sequence(lcd, seq_gamma_ctrl);
+	if (ret)
+		dev_err(lcd->dev, "failed to update gamma table.\n");
+
+gamma_err:
+	return ret;
+}
+
+static int ld9040_gamma_ctl(struct ld9040 *lcd, int gamma)
+{
+	int ret = 0;
+
+	ret = _ld9040_gamma_ctl(lcd, gamma_table.gamma_22_table[gamma]);
+
+	return ret;
+}
+
+
+static int ld9040_ldi_init(struct ld9040 *lcd)
+{
+	int ret, i;
+	static const unsigned short *init_seq[] = {
+		seq_user_setting,
+		seq_panel_condition,
+		seq_display_ctrl,
+		seq_manual_pwr,
+		seq_elvss_on,
+		seq_gtcon,
+		seq_gamma_set1,
+		seq_gamma_ctrl,
+		seq_sleep_out,
+	};
+
+	for (i = 0; i < ARRAY_SIZE(init_seq); i++) {
+		ret = ld9040_panel_send_sequence(lcd, init_seq[i]);
+		/* workaround: minimum delay time for transferring CMD */
+		udelay(300);
+		if (ret)
+			break;
+	}
+
+	return ret;
+}
+
+static int ld9040_ldi_enable(struct ld9040 *lcd)
+{
+	int ret = 0;
+
+	ret = ld9040_panel_send_sequence(lcd, seq_display_on);
+
+	return ret;
+}
+
+static int ld9040_ldi_disable(struct ld9040 *lcd)
+{
+	int ret;
+
+	ret = ld9040_panel_send_sequence(lcd, seq_display_off);
+	ret = ld9040_panel_send_sequence(lcd, seq_sleep_in);
+
+	return ret;
+}
+
+static int ld9040_power_on(struct ld9040 *lcd)
+{
+	int ret = 0;
+	struct lcd_platform_data *pd = NULL;
+	pd = lcd->lcd_pd;
+	if (!pd) {
+		dev_err(lcd->dev, "platform data is NULL.\n");
+		return -EFAULT;
+	}
+
+	if (!pd->power_on) {
+		dev_err(lcd->dev, "power_on is NULL.\n");
+		return -EFAULT;
+	} else {
+		pd->power_on(lcd->ld, 1);
+		mdelay(pd->power_on_delay);
+	}
+
+	if (!pd->reset) {
+		dev_err(lcd->dev, "reset is NULL.\n");
+		return -EFAULT;
+	} else {
+		pd->reset(lcd->ld);
+		mdelay(pd->reset_delay);
+	}
+
+	ret = ld9040_ldi_init(lcd);
+	if (ret) {
+		dev_err(lcd->dev, "failed to initialize ldi.\n");
+		return ret;
+	}
+
+	ret = ld9040_ldi_enable(lcd);
+	if (ret) {
+		dev_err(lcd->dev, "failed to enable ldi.\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int ld9040_power_off(struct ld9040 *lcd)
+{
+	int ret = 0;
+	struct lcd_platform_data *pd = NULL;
+
+	pd = lcd->lcd_pd;
+	if (!pd) {
+		dev_err(lcd->dev, "platform data is NULL.\n");
+		return -EFAULT;
+	}
+
+	ret = ld9040_ldi_disable(lcd);
+	if (ret) {
+		dev_err(lcd->dev, "lcd setting failed.\n");
+		return -EIO;
+	}
+
+	mdelay(pd->power_off_delay);
+
+	if (!pd->power_on) {
+		dev_err(lcd->dev, "power_on is NULL.\n");
+		return -EFAULT;
+	} else
+		pd->power_on(lcd->ld, 0);
+
+	return 0;
+}
+
+static int ld9040_power(struct ld9040 *lcd, int power)
+{
+	int ret = 0;
+
+	if (power_is_on(power) && !power_is_on(lcd->power))
+		ret = ld9040_power_on(lcd);
+	else if (!power_is_on(power) && power_is_on(lcd->power))
+		ret = ld9040_power_off(lcd);
+
+	if (!ret)
+		lcd->power = power;
+
+	return ret;
+}
+
+static int ld9040_set_power(struct lcd_device *ld, int power)
+{
+	struct ld9040 *lcd = lcd_get_data(ld);
+
+	if (power != FB_BLANK_UNBLANK && power != FB_BLANK_POWERDOWN &&
+		power != FB_BLANK_NORMAL) {
+		dev_err(lcd->dev, "power value should be 0, 1 or 4.\n");
+		return -EINVAL;
+	}
+
+	return ld9040_power(lcd, power);
+}
+
+static int ld9040_get_power(struct lcd_device *ld)
+{
+	struct ld9040 *lcd = lcd_get_data(ld);
+
+	return lcd->power;
+}
+
+static int ld9040_get_brightness(struct backlight_device *bd)
+{
+	return bd->props.brightness;
+}
+
+static int ld9040_set_brightness(struct backlight_device *bd)
+{
+	int ret = 0, brightness = bd->props.brightness;
+	struct ld9040 *lcd = bl_get_data(bd);
+
+	if (brightness < MIN_BRIGHTNESS ||
+		brightness > bd->props.max_brightness) {
+		dev_err(&bd->dev, "lcd brightness should be %d to %d.\n",
+			MIN_BRIGHTNESS, MAX_BRIGHTNESS);
+		return -EINVAL;
+	}
+
+	ret = ld9040_gamma_ctl(lcd, bd->props.brightness);
+	if (ret) {
+		dev_err(&bd->dev, "lcd brightness setting failed.\n");
+		return -EIO;
+	}
+
+	return ret;
+}
+
+static struct lcd_ops ld9040_lcd_ops = {
+	.set_power = ld9040_set_power,
+	.get_power = ld9040_get_power,
+};
+
+static const struct backlight_ops ld9040_backlight_ops  = {
+	.get_brightness = ld9040_get_brightness,
+	.update_status = ld9040_set_brightness,
+};
+
+
+static int ld9040_probe(struct spi_device *spi)
+{
+	int ret = 0;
+	struct ld9040 *lcd = NULL;
+	struct lcd_device *ld = NULL;
+	struct backlight_device *bd = NULL;
+
+	lcd = kzalloc(sizeof(struct ld9040), GFP_KERNEL);
+	if (!lcd)
+		return -ENOMEM;
+
+	/* ld9040 lcd panel uses 3-wire 9bits SPI Mode. */
+	spi->bits_per_word = 9;
+
+	ret = spi_setup(spi);
+	if (ret < 0) {
+		dev_err(&spi->dev, "spi setup failed.\n");
+		goto out_free_lcd;
+	}
+
+	lcd->spi = spi;
+	lcd->dev = &spi->dev;
+
+	lcd->lcd_pd = spi->dev.platform_data;
+	if (!lcd->lcd_pd) {
+		dev_err(&spi->dev, "platform data is NULL.\n");
+		goto out_free_lcd;
+	}
+
+	ld = lcd_device_register("ld9040", &spi->dev, lcd, &ld9040_lcd_ops);
+	if (IS_ERR(ld)) {
+		ret = PTR_ERR(ld);
+		goto out_free_lcd;
+	}
+
+	lcd->ld = ld;
+
+	bd = backlight_device_register("ld9040-bl", &spi->dev,
+		lcd, &ld9040_backlight_ops, NULL);
+	if (IS_ERR(ld)) {
+		ret = PTR_ERR(ld);
+		goto out_free_lcd;
+	}
+
+	bd->props.max_brightness = MAX_BRIGHTNESS;
+	bd->props.brightness = MAX_BRIGHTNESS;
+	lcd->bd = bd;
+
+	/*
+	 * if lcd panel was on from bootloader like u-boot then
+	 * do not lcd on.
+	 */
+	if (!lcd->lcd_pd->lcd_enabled) {
+		/*
+		 * if lcd panel was off from bootloader then
+		 * current lcd status is powerdown and then
+		 * it enables lcd panel.
+		 */
+		lcd->power = FB_BLANK_POWERDOWN;
+
+		ld9040_power(lcd, FB_BLANK_UNBLANK);
+	} else
+		lcd->power = FB_BLANK_UNBLANK;
+
+	dev_set_drvdata(&spi->dev, lcd);
+
+	dev_info(&spi->dev, "ld9040 panel driver has been probed.\n");
+	return 0;
+
+out_free_lcd:
+	kfree(lcd);
+	return ret;
+}
+
+static int __devexit ld9040_remove(struct spi_device *spi)
+{
+	struct ld9040 *lcd = dev_get_drvdata(&spi->dev);
+
+	ld9040_power(lcd, FB_BLANK_POWERDOWN);
+	lcd_device_unregister(lcd->ld);
+	kfree(lcd);
+
+	return 0;
+}
+
+#if defined(CONFIG_PM)
+static int ld9040_suspend(struct spi_device *spi, pm_message_t mesg)
+{
+	int ret = 0;
+	struct ld9040 *lcd = dev_get_drvdata(&spi->dev);
+
+	dev_dbg(&spi->dev, "lcd->power = %d\n", lcd->power);
+
+	/*
+	 * when lcd panel is suspend, lcd panel becomes off
+	 * regardless of status.
+	 */
+	ret = ld9040_power(lcd, FB_BLANK_POWERDOWN);
+
+	return ret;
+}
+
+static int ld9040_resume(struct spi_device *spi)
+{
+	int ret = 0;
+	struct ld9040 *lcd = dev_get_drvdata(&spi->dev);
+
+	lcd->power = FB_BLANK_POWERDOWN;
+
+	ret = ld9040_power(lcd, FB_BLANK_UNBLANK);
+
+	return ret;
+}
+#else
+#define ld9040_suspend		NULL
+#define ld9040_resume		NULL
+#endif
+
+/* Power down all displays on reboot, poweroff or halt. */
+static void ld9040_shutdown(struct spi_device *spi)
+{
+	struct ld9040 *lcd = dev_get_drvdata(&spi->dev);
+
+	ld9040_power(lcd, FB_BLANK_POWERDOWN);
+}
+
+static struct spi_driver ld9040_driver = {
+	.driver = {
+		.name	= "ld9040",
+		.bus	= &spi_bus_type,
+		.owner	= THIS_MODULE,
+	},
+	.probe		= ld9040_probe,
+	.remove		= __devexit_p(ld9040_remove),
+	.shutdown	= ld9040_shutdown,
+	.suspend	= ld9040_suspend,
+	.resume		= ld9040_resume,
+};
+
+static int __init ld9040_init(void)
+{
+	return spi_register_driver(&ld9040_driver);
+}
+
+static void __exit ld9040_exit(void)
+{
+	spi_unregister_driver(&ld9040_driver);
+}
+
+module_init(ld9040_init);
+module_exit(ld9040_exit);
+
+MODULE_AUTHOR("Donghwa Lee <dh09.lee@samsung.com>");
+MODULE_DESCRIPTION("ld9040 LCD Driver");
+MODULE_LICENSE("GPL");
+
diff --git a/drivers/video/backlight/ld9040_gamma.h b/drivers/video/backlight/ld9040_gamma.h
new file mode 100644
index 0000000..167fca4
--- /dev/null
+++ b/drivers/video/backlight/ld9040_gamma.h
@@ -0,0 +1,201 @@
+/* 
+ * Gamma level definitions.
+ *
+ * Copyright (c) 2011 Samsung Electronics
+ * InKi Dae <inki.dae@samsung.com>
+ * Donghwa Lee <dh09.lee@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+*/
+
+#ifndef _LD9040_BRIGHTNESS_H
+#define _LD9040_BRIGHTNESS_H
+
+#define MAX_GAMMA_LEVEL		25
+#define GAMMA_TABLE_COUNT	21
+
+/* gamma value: 2.2 */
+static const unsigned int ld9040_22_300[] = {
+	0x00, 0xa7, 0xb4, 0xae, 0xbf, 0x00, 0x91,
+	0x00, 0xb2, 0xb4, 0xaa, 0xbb, 0x00, 0xac,
+	0x00, 0xb3, 0xb1, 0xaa, 0xbc, 0x00, 0xb3
+};
+
+static const unsigned int ld9040_22_290[] = {
+	0x00, 0xa9, 0xb7, 0xae, 0xbd, 0x00, 0x89,
+	0x00, 0xb7, 0xb6, 0xa8, 0xba, 0x00, 0xa4,
+	0x00, 0xb1, 0xb4, 0xaa, 0xbb, 0x00, 0xaa
+};
+
+static const unsigned int ld9040_22_280[] = {
+	0x00, 0xa9, 0xb6, 0xad, 0xbf, 0x00, 0x86,
+	0x00, 0xb8, 0xb5, 0xa8, 0xbc, 0x00, 0xa0,
+	0x00, 0xb3, 0xb3, 0xa9, 0xbc, 0x00, 0xa7
+};
+
+static const unsigned int ld9040_22_270[] = {
+	0x00, 0xa8, 0xb8, 0xae, 0xbe, 0x00, 0x84,
+	0x00, 0xb9, 0xb7, 0xa8, 0xbc, 0x00, 0x9d,
+	0x00, 0xb2, 0xb5, 0xaa, 0xbc, 0x00, 0xa4
+
+};
+static const unsigned int ld9040_22_260[] = {
+	0x00, 0xa4, 0xb8, 0xb0, 0xbf, 0x00, 0x80,
+	0x00, 0xb8, 0xb6, 0xaa, 0xbc, 0x00, 0x9a,
+	0x00, 0xb0, 0xb5, 0xab, 0xbd, 0x00, 0xa0
+};
+
+static const unsigned int ld9040_22_250[] = {
+	0x00, 0xa4, 0xb9, 0xaf, 0xc1, 0x00, 0x7d,
+	0x00, 0xb9, 0xb6, 0xaa, 0xbb, 0x00, 0x97,
+	0x00, 0xb1, 0xb5, 0xaa, 0xbf, 0x00, 0x9d
+};
+
+static const unsigned int ld9040_22_240[] = {
+	0x00, 0xa2, 0xb9, 0xaf, 0xc2, 0x00, 0x7a,
+	0x00, 0xb9, 0xb7, 0xaa, 0xbd, 0x00, 0x94,
+	0x00, 0xb0, 0xb5, 0xab, 0xbf, 0x00, 0x9a
+};
+
+static const unsigned int ld9040_22_230[] = {
+	0x00, 0xa0, 0xb9, 0xaf, 0xc3, 0x00, 0x77,
+	0x00, 0xb9, 0xb7, 0xab, 0xbe, 0x00, 0x90,
+	0x00, 0xb0, 0xb6, 0xab, 0xbf, 0x00, 0x97
+};
+
+static const unsigned int ld9040_22_220[] = {
+	0x00, 0x9e, 0xba, 0xb0, 0xc2, 0x00, 0x75,
+	0x00, 0xb9, 0xb8, 0xab, 0xbe, 0x00, 0x8e,
+	0x00, 0xb0, 0xb6, 0xac, 0xbf, 0x00, 0x94
+};
+
+static const unsigned int ld9040_22_210[] = {
+	0x00, 0x9c, 0xb9, 0xb0, 0xc4, 0x00, 0x72,
+	0x00, 0xb8, 0xb8, 0xac, 0xbf, 0x00, 0x8a,
+	0x00, 0xb0, 0xb6, 0xac, 0xc0, 0x00, 0x91
+};
+
+static const unsigned int ld9040_22_200[] = {
+	0x00, 0x9a, 0xba, 0xb1, 0xc4, 0x00, 0x6f,
+	0x00, 0xb8, 0xb8, 0xad, 0xc0, 0x00, 0x86,
+	0x00, 0xb0, 0xb7, 0xad, 0xc0, 0x00, 0x8d
+};
+
+static const unsigned int ld9040_22_190[] = {
+	0x00, 0x97, 0xba, 0xb2, 0xc5, 0x00, 0x6c,
+	0x00, 0xb8, 0xb8, 0xae, 0xc1, 0x00, 0x82,
+	0x00, 0xb0, 0xb6, 0xae, 0xc2, 0x00, 0x89
+};
+
+static const unsigned int ld9040_22_180[] = {
+	0x00, 0x93, 0xba, 0xb3, 0xc5, 0x00, 0x69,
+	0x00, 0xb8, 0xb9, 0xae, 0xc1, 0x00, 0x7f,
+	0x00, 0xb0, 0xb6, 0xae, 0xc3, 0x00, 0x85
+};
+
+static const unsigned int ld9040_22_170[] = {
+	0x00, 0x8b, 0xb9, 0xb3, 0xc7, 0x00, 0x65,
+	0x00, 0xb7, 0xb8, 0xaf, 0xc3, 0x00, 0x7a,
+	0x00, 0x80, 0xb6, 0xae, 0xc4, 0x00, 0x81
+};
+
+static const unsigned int ld9040_22_160[] = {
+	0x00, 0x89, 0xba, 0xb3, 0xc8, 0x00, 0x62,
+	0x00, 0xb6, 0xba, 0xaf, 0xc3, 0x00, 0x76,
+	0x00, 0xaf, 0xb7, 0xae, 0xc4, 0x00, 0x7e
+};
+
+static const unsigned int ld9040_22_150[] = {
+	0x00, 0x82, 0xba, 0xb4, 0xc7, 0x00, 0x5f,
+	0x00, 0xb5, 0xba, 0xb0, 0xc3, 0x00, 0x72,
+	0x00, 0xae, 0xb8, 0xb0, 0xc3, 0x00, 0x7a
+};
+
+static const unsigned int ld9040_22_140[] = {
+	0x00, 0x7b, 0xbb, 0xb4, 0xc8, 0x00, 0x5b,
+	0x00, 0xb5, 0xba, 0xb1, 0xc4, 0x00, 0x6e,
+	0x00, 0xae, 0xb9, 0xb0, 0xc5, 0x00, 0x75
+};
+
+static const unsigned int ld9040_22_130[] = {
+	0x00, 0x71, 0xbb, 0xb5, 0xc8, 0x00, 0x57,
+	0x00, 0xb5, 0xbb, 0xb0, 0xc5, 0x00, 0x6a,
+	0x00, 0xae, 0xb9, 0xb1, 0xc6, 0x00, 0x70
+};
+
+static const unsigned int ld9040_22_120[] = {
+	0x00, 0x47, 0xba, 0xb6, 0xca, 0x00, 0x53,
+	0x00, 0xb5, 0xbb, 0xb3, 0xc6, 0x00, 0x65,
+	0x00, 0xae, 0xb8, 0xb3, 0xc7, 0x00, 0x6c
+};
+
+static const unsigned int ld9040_22_110[] = {
+	0x00, 0x13, 0xbb, 0xb7, 0xca, 0x00, 0x4f,
+	0x00, 0xb4, 0xbb, 0xb3, 0xc7, 0x00, 0x60,
+	0x00, 0xad, 0xb8, 0xb4, 0xc7, 0x00, 0x67
+};
+
+static const unsigned int ld9040_22_100[] = {
+	0x00, 0x13, 0xba, 0xb8, 0xcb, 0x00, 0x4b,
+	0x00, 0xb3, 0xbc, 0xb4, 0xc7, 0x00, 0x5c,
+	0x00, 0xac, 0xb8, 0xb4, 0xc8, 0x00, 0x62
+};
+
+static const unsigned int ld9040_22_90[] = {
+	0x00, 0x13, 0xb9, 0xb8, 0xcd, 0x00, 0x46,
+	0x00, 0xb1, 0xbc, 0xb5, 0xc8, 0x00, 0x56,
+	0x00, 0xaa, 0xb8, 0xb4, 0xc9, 0x00, 0x5d
+};
+
+static const unsigned int ld9040_22_80[] = {
+	0x00, 0x13, 0xba, 0xb9, 0xcd, 0x00, 0x41,
+	0x00, 0xb0, 0xbe, 0xb5, 0xc9, 0x00, 0x51,
+	0x00, 0xa9, 0xb9, 0xb5, 0xca, 0x00, 0x57
+};
+
+static const unsigned int ld9040_22_70[] = {
+	0x00, 0x13, 0xb9, 0xb9, 0xd0, 0x00, 0x3c,
+	0x00, 0xaf, 0xbf, 0xb6, 0xcb, 0x00, 0x4b,
+	0x00, 0xa8, 0xb9, 0xb5, 0xcc, 0x00, 0x52
+};
+
+static const unsigned int ld9040_22_50[] = {
+	0x00, 0x13, 0xb2, 0xba, 0xd2, 0x00, 0x30,
+	0x00, 0xaf, 0xc0, 0xb8, 0xcd, 0x00, 0x3d,
+	0x00, 0xa8, 0xb8, 0xb7, 0xcd, 0x00, 0x44
+};
+
+struct ld9040_gamma {
+	unsigned int *gamma_22_table[MAX_GAMMA_LEVEL];
+} gamma_table = {
+	.gamma_22_table[0] = (unsigned int *)&ld9040_22_50,
+	.gamma_22_table[1] = (unsigned int *)&ld9040_22_70,
+	.gamma_22_table[2] = (unsigned int *)&ld9040_22_80,
+	.gamma_22_table[3] = (unsigned int *)&ld9040_22_90,
+	.gamma_22_table[4] = (unsigned int *)&ld9040_22_100,
+	.gamma_22_table[5] = (unsigned int *)&ld9040_22_110,
+	.gamma_22_table[6] = (unsigned int *)&ld9040_22_120,
+	.gamma_22_table[7] = (unsigned int *)&ld9040_22_130,
+	.gamma_22_table[8] = (unsigned int *)&ld9040_22_140,
+	.gamma_22_table[9] = (unsigned int *)&ld9040_22_150,
+	.gamma_22_table[10] = (unsigned int *)&ld9040_22_160,
+	.gamma_22_table[11] = (unsigned int *)&ld9040_22_170,
+	.gamma_22_table[12] = (unsigned int *)&ld9040_22_180,
+	.gamma_22_table[13] = (unsigned int *)&ld9040_22_190,
+	.gamma_22_table[14] = (unsigned int *)&ld9040_22_200,
+	.gamma_22_table[15] = (unsigned int *)&ld9040_22_210,
+	.gamma_22_table[16] = (unsigned int *)&ld9040_22_220,
+	.gamma_22_table[17] = (unsigned int *)&ld9040_22_230,
+	.gamma_22_table[18] = (unsigned int *)&ld9040_22_240,
+	.gamma_22_table[19] = (unsigned int *)&ld9040_22_250,
+	.gamma_22_table[20] = (unsigned int *)&ld9040_22_260,
+	.gamma_22_table[21] = (unsigned int *)&ld9040_22_270,
+	.gamma_22_table[22] = (unsigned int *)&ld9040_22_280,
+	.gamma_22_table[23] = (unsigned int *)&ld9040_22_290,
+	.gamma_22_table[24] = (unsigned int *)&ld9040_22_300,
+};
+
+#endif
+
-- 
1.6.0.4


^ permalink raw reply related

* Re: Linux 2.6.38-rc6
From: Herton Ronaldo Krzesinski @ 2011-02-25 14:49 UTC (permalink / raw)
  To: Anca Emanuel
  Cc: Dave Airlie, Linus Torvalds, linux-fbdev, Ben Skeggs, dri-devel,
	Borislav Petkov, Linux Kernel Mailing List, Andy Whitcroft
In-Reply-To: <AANLkTinq_gyAFKb_i_4j6qzkAmoSuAS+k_eCRFKFy5wL@mail.gmail.com>

On Fri, Feb 25, 2011 at 03:56:20AM +0200, Anca Emanuel wrote:
> On Fri, Feb 25, 2011 at 3:47 AM, Anca Emanuel <anca.emanuel@gmail.com> wrote:
> > On Fri, Feb 25, 2011 at 3:14 AM, Dave Airlie <airlied@redhat.com> wrote:
> >> On Thu, 2011-02-24 at 16:54 -0800, Linus Torvalds wrote:
> >>> On Thu, Feb 24, 2011 at 4:48 PM, Anca Emanuel <anca.emanuel@gmail.com> wrote:
> >>> >
> >>> > diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> >>> > index e2bf953..e8f8925 100644
> >>> > --- a/drivers/video/fbmem.c
> >>> > +++ b/drivers/video/fbmem.c
> >>> > @@ -1511,6 +1511,7 @@ void remove_conflicting_framebuffers(struct
> >>> > apertures_struct *a,
> >>> >                               "%s vs %s - removing generic driver\n",
> >>> >                               name, registered_fb[i]->fix.id);
> >>> >                        unregister_framebuffer(registered_fb[i]);
> >>> > +                       registered_fb[i] = NULL;
> >>> >
> >>> > Tested the patch, and now I get this:
> >>> > dmesg: http://pastebin.com/ieMNrA7C
> >>> >
> >>> > [   12.252328] BUG: unable to handle kernel NULL pointer dereference
> >>> > at 00000000000003b8
> >>> > [   12.252342] IP: [<ffffffff81311178>] fb_mmap+0x58/0x1d0
> >>>
> >>> Ok, goodie.
> >>>
> >>> Or not so goodie, but it does make it clear that yeah, the fb code
> >>> seems to be using stale pointers from that registered_fb[] array, and
> >>> the whole unregistration process is just racing with people using it.
> >>>
> >>> Herton had that much bigger patch, can you test it?
> >>
> >> I think Andy's patch worked, not sure why it fell between the cracks,
> >> either didn't appear on lkml or in my inbox at all.
> >>
> >> if we can get Herton to repost it properly + a tested by I'm happy for
> >> it to go in.
> >>
> >> Dave.
> >>
> >>
> >
> > Tested Andy's patch and it works !
> > http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-natty.git;a=commit;hÅa742b5f78e161d6a13853a7e3e6e1dfa429e69
> >
> > Tested-by: Anca Emanuel <anca.emanuel@gmail.com>
> >
> 
> link to patch: http://is.gd/otIfGc

Adding Andy on CC (btw he is away for today, may get some time to answer).

Andy, can you repost the patch?

--
[]'s
Herton

^ permalink raw reply

* [PATCH] video: msmfb: Put the partial update magic value into the fix_screen struct.
From: Carl Vanderlip @ 2011-02-25 21:05 UTC (permalink / raw)
  To: davidb
  Cc: Dima Zavin, open list:ARM/QUALCOMM MSM...,
	open list:FRAMEBUFFER LAYER, open list

From: Dima Zavin <dima@android.com>

This can then be tested by userspace to see if the capability is supported.
Userspace cannot rely on that value being left in var_screen, since userspace
itself can change it.

---
 drivers/video/msm/msm_fb.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/video/msm/msm_fb.c b/drivers/video/msm/msm_fb.c
index 5436aeb..bbf841e 100644
--- a/drivers/video/msm/msm_fb.c
+++ b/drivers/video/msm/msm_fb.c
@@ -469,6 +469,14 @@ static void setup_fb_info(struct msmfb_info *msmfb)
 	fb_info->var.yoffset = 0;
 
 	if (msmfb->panel->caps & MSMFB_CAP_PARTIAL_UPDATES) {
+		/* set the param in the fixed screen, so userspace can't
+		 * change it. This will be used to check for the
+		 * capability. */
+		fb_info->fix.reserved[0] = 0x5444;
+		fb_info->fix.reserved[1] = 0x5055;
+
+		/* This preloads the value so that if userspace doesn't
+		 * change it, it will be a full update */
 		fb_info->var.reserved[0] = 0x54445055;
 		fb_info->var.reserved[1] = 0;
 		fb_info->var.reserved[2] = (uint16_t)msmfb->xres |
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.


^ permalink raw reply related

* [PATCH] [ARM] msm: mdp: Add support for RGBX 8888 image format.
From: Carl Vanderlip @ 2011-02-25 21:11 UTC (permalink / raw)
  To: davidb
  Cc: Dima Zavin, open list:ARM/QUALCOMM MSM...,
	open list:FRAMEBUFFER LAYER, open list

From: Dima Zavin <dima@android.com>

---
 drivers/video/msm/mdp_hw.h  |    9 ++++++++-
 drivers/video/msm/mdp_ppp.c |    1 +
 include/linux/msm_mdp.h     |    1 +
 3 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/drivers/video/msm/mdp_hw.h b/drivers/video/msm/mdp_hw.h
index 4e3deb4..9e1e92e 100644
--- a/drivers/video/msm/mdp_hw.h
+++ b/drivers/video/msm/mdp_hw.h
@@ -449,6 +449,7 @@ int mdp_ppp_blit(const struct mdp_info *mdp, struct mdp_blit_req *req,
 #define PPP_CFG_MDP_XRGB_8888(dir) PPP_CFG_MDP_ARGB_8888(dir)
 #define PPP_CFG_MDP_RGBA_8888(dir) PPP_CFG_MDP_ARGB_8888(dir)
 #define PPP_CFG_MDP_BGRA_8888(dir) PPP_CFG_MDP_ARGB_8888(dir)
+#define PPP_CFG_MDP_RGBX_8888(dir) PPP_CFG_MDP_ARGB_8888(dir)
 
 #define PPP_CFG_MDP_Y_CBCR_H2V2(dir)   (PPP_##dir##_C2R_8BIT | \
 					PPP_##dir##_C0G_8BIT | \
@@ -494,6 +495,8 @@ int mdp_ppp_blit(const struct mdp_info *mdp, struct mdp_blit_req *req,
 	MDP_GET_PACK_PATTERN(CLR_ALPHA, CLR_B, CLR_G, CLR_R, 8)
 #define PPP_PACK_PATTERN_MDP_BGRA_8888 \
 	MDP_GET_PACK_PATTERN(CLR_ALPHA, CLR_R, CLR_G, CLR_B, 8)
+#define PPP_PACK_PATTERN_MDP_RGBX_8888 \
+	MDP_GET_PACK_PATTERN(CLR_ALPHA, CLR_B, CLR_G, CLR_R, 8)
 #define PPP_PACK_PATTERN_MDP_Y_CBCR_H2V1 \
 	MDP_GET_PACK_PATTERN(0, 0, CLR_CB, CLR_CR, 8)
 #define PPP_PACK_PATTERN_MDP_Y_CBCR_H2V2 PPP_PACK_PATTERN_MDP_Y_CBCR_H2V1
@@ -509,6 +512,7 @@ int mdp_ppp_blit(const struct mdp_info *mdp, struct mdp_blit_req *req,
 #define PPP_CHROMA_SAMP_MDP_ARGB_8888(dir) PPP_OP_##dir##_CHROMA_RGB
 #define PPP_CHROMA_SAMP_MDP_RGBA_8888(dir) PPP_OP_##dir##_CHROMA_RGB
 #define PPP_CHROMA_SAMP_MDP_BGRA_8888(dir) PPP_OP_##dir##_CHROMA_RGB
+#define PPP_CHROMA_SAMP_MDP_RGBX_8888(dir) PPP_OP_##dir##_CHROMA_RGB
 #define PPP_CHROMA_SAMP_MDP_Y_CBCR_H2V1(dir) PPP_OP_##dir##_CHROMA_H2V1
 #define PPP_CHROMA_SAMP_MDP_Y_CBCR_H2V2(dir) PPP_OP_##dir##_CHROMA_420
 #define PPP_CHROMA_SAMP_MDP_Y_CRCB_H2V1(dir) PPP_OP_##dir##_CHROMA_H2V1
@@ -523,6 +527,7 @@ int mdp_ppp_blit(const struct mdp_info *mdp, struct mdp_blit_req *req,
 	[MDP_ARGB_8888] = PPP_##name##_MDP_ARGB_8888,\
 	[MDP_RGBA_8888] = PPP_##name##_MDP_RGBA_8888,\
 	[MDP_BGRA_8888] = PPP_##name##_MDP_BGRA_8888,\
+	[MDP_RGBX_8888] = PPP_##name##_MDP_RGBX_8888,\
 	[MDP_Y_CBCR_H2V1] = PPP_##name##_MDP_Y_CBCR_H2V1,\
 	[MDP_Y_CBCR_H2V2] = PPP_##name##_MDP_Y_CBCR_H2V2,\
 	[MDP_Y_CRCB_H2V1] = PPP_##name##_MDP_Y_CRCB_H2V1,\
@@ -536,6 +541,7 @@ int mdp_ppp_blit(const struct mdp_info *mdp, struct mdp_blit_req *req,
 	[MDP_ARGB_8888] = PPP_##name##_MDP_ARGB_8888(dir),\
 	[MDP_RGBA_8888] = PPP_##name##_MDP_RGBA_8888(dir),\
 	[MDP_BGRA_8888] = PPP_##name##_MDP_BGRA_8888(dir),\
+	[MDP_RGBX_8888] = PPP_##name##_MDP_RGBX_8888(dir),\
 	[MDP_Y_CBCR_H2V1] = PPP_##name##_MDP_Y_CBCR_H2V1(dir),\
 	[MDP_Y_CBCR_H2V2] = PPP_##name##_MDP_Y_CBCR_H2V2(dir),\
 	[MDP_Y_CRCB_H2V1] = PPP_##name##_MDP_Y_CRCB_H2V1(dir),\
@@ -547,7 +553,8 @@ int mdp_ppp_blit(const struct mdp_info *mdp, struct mdp_blit_req *req,
 		       (img = MDP_YCRYCB_H2V1))
 #define IS_RGB(img) ((img = MDP_RGB_565) | (img = MDP_RGB_888) | \
 		     (img = MDP_ARGB_8888) | (img = MDP_RGBA_8888) | \
-		     (img = MDP_XRGB_8888) | (img = MDP_BGRA_8888))
+		     (img = MDP_XRGB_8888) | (img = MDP_BGRA_8888) | \
+		     (img = MDP_RGBX_8888))
 #define HAS_ALPHA(img) ((img = MDP_ARGB_8888) | (img = MDP_RGBA_8888) | \
 			(img = MDP_BGRA_8888))
 
diff --git a/drivers/video/msm/mdp_ppp.c b/drivers/video/msm/mdp_ppp.c
index 4ff001f..2b6564e 100644
--- a/drivers/video/msm/mdp_ppp.c
+++ b/drivers/video/msm/mdp_ppp.c
@@ -69,6 +69,7 @@ static uint32_t bytes_per_pixel[] = {
 	[MDP_ARGB_8888] = 4,
 	[MDP_RGBA_8888] = 4,
 	[MDP_BGRA_8888] = 4,
+	[MDP_RGBX_8888] = 4,
 	[MDP_Y_CBCR_H2V1] = 1,
 	[MDP_Y_CBCR_H2V2] = 1,
 	[MDP_Y_CRCB_H2V1] = 1,
diff --git a/include/linux/msm_mdp.h b/include/linux/msm_mdp.h
index d11fe0f..fe722c1 100644
--- a/include/linux/msm_mdp.h
+++ b/include/linux/msm_mdp.h
@@ -32,6 +32,7 @@ enum {
 	MDP_Y_CBCR_H2V1,	/* Y and CrCb, pseduo planar w/ Cr is in MSB */
 	MDP_RGBA_8888,		/* ARGB 888 */
 	MDP_BGRA_8888,		/* ABGR 888 */
+	MDP_RGBX_8888,		/* RGBX 888 */
 	MDP_IMGTYPE_LIMIT	/* Non valid image type after this enum */
 };
 
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.


^ permalink raw reply related

* Re: [PATCH] [ARM] msm: mdp: Add support for RGBX 8888 image format.
From: Carl Vanderlip @ 2011-02-25 21:28 UTC (permalink / raw)
  To: davidb
  Cc: Dima Zavin, open list:ARM/QUALCOMM MSM...,
	open list:FRAMEBUFFER LAYER, open list
In-Reply-To: <1298668281-22305-1-git-send-email-carlv@codeaurora.org>

Please disregard this patch.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.


^ permalink raw reply

* RE: [PATCH] video: msmfb: Put the partial update magic value into
From: Janorkar, Mayuresh @ 2011-02-26 12:31 UTC (permalink / raw)
  To: Carl Vanderlip, davidb@codeaurora.org
  Cc: Dima Zavin, open list:ARM/QUALCOMM MSM...,
	open list:FRAMEBUFFER LAYER, open list
In-Reply-To: <1298667947-22123-1-git-send-email-carlv@codeaurora.org>


> -----Original Message-----
> From: linux-fbdev-owner@vger.kernel.org [mailto:linux-fbdev-
> owner@vger.kernel.org] On Behalf Of Carl Vanderlip
> Sent: Saturday, February 26, 2011 2:36 AM
> To: davidb@codeaurora.org
> Cc: Dima Zavin; open list:ARM/QUALCOMM MSM...; open list:FRAMEBUFFER
> LAYER; open list
> Subject: [PATCH] video: msmfb: Put the partial update magic value into the
> fix_screen struct.
> 
> From: Dima Zavin <dima@android.com>
> 
> This can then be tested by userspace to see if the capability is
> supported.
> Userspace cannot rely on that value being left in var_screen, since
> userspace
> itself can change it.
> 
Missing Signed-off-by:
checkpatch would give error for this.
> ---
>  drivers/video/msm/msm_fb.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/video/msm/msm_fb.c b/drivers/video/msm/msm_fb.c
> index 5436aeb..bbf841e 100644
> --- a/drivers/video/msm/msm_fb.c
> +++ b/drivers/video/msm/msm_fb.c
> @@ -469,6 +469,14 @@ static void setup_fb_info(struct msmfb_info *msmfb)
>  	fb_info->var.yoffset = 0;
> 
>  	if (msmfb->panel->caps & MSMFB_CAP_PARTIAL_UPDATES) {
> +		/* set the param in the fixed screen, so userspace can't
> +		 * change it. This will be used to check for the
> +		 * capability. */
Multi-line comments?
> +		fb_info->fix.reserved[0] = 0x5444;
> +		fb_info->fix.reserved[1] = 0x5055;
> +
> +		/* This preloads the value so that if userspace doesn't
> +		 * change it, it will be a full update */
Multi-line comments?
>  		fb_info->var.reserved[0] = 0x54445055;
>  		fb_info->var.reserved[1] = 0;
>  		fb_info->var.reserved[2] = (uint16_t)msmfb->xres |
> --
> Sent by an employee of the Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 1/3] ARM: PXA: PXAFB: Fix double-free issue
From: Vasily Khoruzhick @ 2011-02-26 16:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <201102201946.20681.marek.vasut@gmail.com>

On Sunday 20 February 2011 20:46:20 Marek Vasut wrote:

> > @@ -629,6 +629,9 @@ static void overlay1fb_disable(struct pxafb_layer
> > *ofb)
> > 
> >  {
> >  
> >  	uint32_t lccr5 = lcd_readl(ofb->fbi, LCCR5);
> > 
> > +	if (!(lcd_readl(ofb->fbi, OVL1C1) & OVLxC1_OEN))
> > +		return;
> > +
> 
> Maybe you can even avoid reading LCCR5 above ;-)

Agreed.

Will resend soon.

^ permalink raw reply

* [PATCH] backlight/platform_lcd: change set power function parameter in
From: Donghwa Lee @ 2011-02-28  6:14 UTC (permalink / raw)
  To: linux-arm-kernel

In drivers/video/backlight/platform_lcd.c driver, to use lcd power
control function in platdata, it is used
set_power(struct plat_lcd_data *, unsigned int power) callback function.
set_power() function is consists of two parameters,
(struct plat_lcd_data *, unsigned int power).

But actually, there is no one to use (struct plat_lcd_data *), only use
second (unsigned int power) value to control lcd power on/off.

I don't know why (struct plat_lcd_data*) parameter is used in set_power()
callback function. Maybe I think it is unessesntial or changeable to another
meaningful variable. For example, struct lcd_device *, it can be used to
control regulator_get() function.

In mach-nuri.c, it has been patching by Minku Kang, (struct lcd_device*) can be
used to regulator lcd power on/off, and many other machines uses regulator
control to on/off lcd power.

So I think more meaningful variable that (struct lcd_device* ) can be
substituted for unessential variable that (struct plat_lcd_data *).

Thank you,
Donghwa Lee


Signed-off-by: Donghwa Lee <dh09.lee@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

---
 drivers/video/backlight/platform_lcd.c |    2 +-
 include/video/platform_lcd.h           |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/backlight/platform_lcd.c b/drivers/video/backlight/platform_lcd.c
index 302330a..cf23a3f 100644
--- a/drivers/video/backlight/platform_lcd.c
+++ b/drivers/video/backlight/platform_lcd.c
@@ -49,7 +49,7 @@ static int platform_lcd_set_power(struct lcd_device *lcd, int power)
 	if (power = FB_BLANK_POWERDOWN || plcd->suspended)
 		lcd_power = 0;
 
-	plcd->pdata->set_power(plcd->pdata, lcd_power);
+	plcd->pdata->set_power(lcd, lcd_power);
 	plcd->power = power;
 
 	return 0;
diff --git a/include/video/platform_lcd.h b/include/video/platform_lcd.h
index ad3bdfe..e92a039 100644
--- a/include/video/platform_lcd.h
+++ b/include/video/platform_lcd.h
@@ -15,7 +15,7 @@ struct plat_lcd_data;
 struct fb_info;
 
 struct plat_lcd_data {
-	void	(*set_power)(struct plat_lcd_data *, unsigned int power);
+	void	(*set_power)(struct lcd_device *, unsigned int power);
 	int	(*match_fb)(struct plat_lcd_data *, struct fb_info *);
 };
 
-- 
1.6.0.4


^ permalink raw reply related

* [PATCH 1/2] video: Add i.MX23/28 framebuffer driver
From: Sascha Hauer @ 2011-02-28  8:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1298880152-28913-1-git-send-email-s.hauer@pengutronix.de>

changes since v2:

- use v3 and v4 for specifying the ip version instead of i.MX23/28.
  This is a better namespace when future versions are added.
- rename mach/fb.h to mach/mxsfb.h

changes since v1:
- Add a LCDC_ prefix to the register names.
- use set/clear registers where appropriate
- protect call to mxsfb_disable_controller() in mxsfb_remove()
  with a (host->enabled) as suggested by Lothar Wassmann

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: linux-fbdev@vger.kernel.org
Cc: Shawn Guo <shawn.guo@freescale.com>
---
 arch/arm/mach-mxs/include/mach/mxsfb.h |   48 ++
 drivers/video/Kconfig                  |    9 +
 drivers/video/Makefile                 |    1 +
 drivers/video/mxsfb.c                  |  914 ++++++++++++++++++++++++++++++++
 4 files changed, 972 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-mxs/include/mach/mxsfb.h
 create mode 100644 drivers/video/mxsfb.c

diff --git a/arch/arm/mach-mxs/include/mach/mxsfb.h b/arch/arm/mach-mxs/include/mach/mxsfb.h
new file mode 100644
index 0000000..923f397
--- /dev/null
+++ b/arch/arm/mach-mxs/include/mach/mxsfb.h
@@ -0,0 +1,48 @@
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301, USA.
+ */
+
+#ifndef __MACH_FB_H
+#define __MACH_FB_H
+
+#include <linux/fb.h>
+
+#define STMLCDIF_8BIT 1	/** pixel data bus to the display is of 8 bit width */
+#define STMLCDIF_16BIT 0 /** pixel data bus to the display is of 16 bit width */
+#define STMLCDIF_18BIT 2 /** pixel data bus to the display is of 18 bit width */
+#define STMLCDIF_24BIT 3 /** pixel data bus to the display is of 24 bit width */
+
+#define FB_SYNC_DATA_ENABLE_HIGH_ACT	64
+
+struct mxsfb_platform_data {
+	struct fb_videomode *mode_list;
+	unsigned mode_count;
+
+	unsigned default_bpp;
+
+	unsigned dotclk_delay;	/* refer manual HW_LCDIF_VDCTRL4 register */
+	unsigned ld_intf_width;	/* refer STMLCDIF_* macros */
+
+	unsigned fb_size;	/* Size of the video memory. If zero a
+				 * default will be used
+				 */
+	unsigned long fb_phys;	/* physical address for the video memory. If
+				 * zero the framebuffer memory will be dynamically
+				 * allocated. If specified,fb_size must also be specified.
+				 * fb_phys must be unused by Linux.
+				 */
+};
+
+#endif /* __MACH_FB_H */
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 6bafb51b..e0ea23f 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2365,6 +2365,15 @@ config FB_JZ4740
 	help
 	  Framebuffer support for the JZ4740 SoC.
 
+config FB_MXS
+	tristate "MXS LCD framebuffer support"
+	depends on FB && ARCH_MXS
+	select FB_CFB_FILLRECT
+	select FB_CFB_COPYAREA
+	select FB_CFB_IMAGEBLIT
+	help
+	  Framebuffer support for the MXS SoC.
+
 source "drivers/video/omap/Kconfig"
 source "drivers/video/omap2/Kconfig"
 
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 8c8fabd..9a096ae 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -153,6 +153,7 @@ obj-$(CONFIG_FB_BFIN_T350MCQB)	  += bfin-t350mcqb-fb.o
 obj-$(CONFIG_FB_BFIN_7393)        += bfin_adv7393fb.o
 obj-$(CONFIG_FB_MX3)		  += mx3fb.o
 obj-$(CONFIG_FB_DA8XX)		  += da8xx-fb.o
+obj-$(CONFIG_FB_MXS)		  += mxsfb.o
 
 # the test framebuffer is last
 obj-$(CONFIG_FB_VIRTUAL)          += vfb.o
diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
new file mode 100644
index 0000000..a3b1179
--- /dev/null
+++ b/drivers/video/mxsfb.c
@@ -0,0 +1,914 @@
+/*
+ * Copyright (C) 2010 Juergen Beisert, Pengutronix
+ *
+ * This code is based on:
+ * Author: Vitaly Wool <vital@embeddedalley.com>
+ *
+ * Copyright 2008-2009 Freescale Semiconductor, Inc. All Rights Reserved.
+ * Copyright 2008 Embedded Alley Solutions, Inc All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#define DRIVER_NAME "mxsfb"
+
+/**
+ * @file
+ * @brief LCDIF driver for i.MX23 and i.MX28
+ *
+ * The LCDIF support four modes of operation
+ * - MPU interface (to drive smart displays) -> not supported yet
+ * - VSYNC interface (like MPU interface plus Vsync) -> not supported yet
+ * - Dotclock interface (to drive LC displays with RGB data and sync signals)
+ * - DVI (to drive ITU-R BT656)  -> not supported yet
+ *
+ * This driver depends on a correct setup of the pins used for this purpose
+ * (platform specific).
+ *
+ * For the developer: Don't forget to set the data bus width to the display
+ * in the imx_fb_videomode structure. You will else end up with ugly colours.
+ * If you fight against jitter you can vary the clock delay. This is a feature
+ * of the i.MX28 and you can vary it between 2 ns ... 8 ns in 2 ns steps. Give
+ * the required value in the imx_fb_videomode structure.
+ */
+
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/dma-mapping.h>
+#include <linux/io.h>
+#include <mach/mxsfb.h>
+
+#define REG_SET	4
+#define REG_CLR	8
+
+#define LCDC_CTRL			0x00
+#define LCDC_CTRL1			0x10
+#define LCDC_V4_CTRL2			0x20
+#define LCDC_V3_TRANSFER_COUNT	0x20
+#define LCDC_V4_TRANSFER_COUNT	0x30
+#define LCDC_V4_CUR_BUF		0x40
+#define LCDC_V4_NEXT_BUF		0x50
+#define LCDC_V3_CUR_BUF		0x30
+#define LCDC_V3_NEXT_BUF		0x40
+#define LCDC_TIMING			0x60
+#define LCDC_VDCTRL0			0x70
+#define LCDC_VDCTRL1			0x80
+#define LCDC_VDCTRL2			0x90
+#define LCDC_VDCTRL3			0xa0
+#define LCDC_VDCTRL4			0xb0
+#define LCDC_DVICTRL0			0xc0
+#define LCDC_DVICTRL1			0xd0
+#define LCDC_DVICTRL2			0xe0
+#define LCDC_DVICTRL3			0xf0
+#define LCDC_DVICTRL4			0x100
+#define LCDC_V4_DATA			0x180
+#define LCDC_V3_DATA			0x1b0
+#define LCDC_V4_DEBUG0		0x1d0
+#define LCDC_V3_DEBUG0		0x1f0
+
+#define CTRL_SFTRST			(1 << 31)
+#define CTRL_CLKGATE			(1 << 30)
+#define CTRL_BYPASS_COUNT		(1 << 19)
+#define CTRL_VSYNC_MODE			(1 << 18)
+#define CTRL_DOTCLK_MODE		(1 << 17)
+#define CTRL_DATA_SELECT		(1 << 16)
+#define CTRL_SET_BUS_WIDTH(x)		(((x) & 0x3) << 10)
+#define CTRL_GET_BUS_WIDTH(x)		(((x) >> 10) & 0x3)
+#define CTRL_SET_WORD_LENGTH(x)		(((x) & 0x3) << 8)
+#define CTRL_GET_WORD_LENGTH(x)		(((x) >> 8) & 0x3)
+#define CTRL_MASTER			(1 << 5)
+#define CTRL_DF16			(1 << 3)
+#define CTRL_DF18			(1 << 2)
+#define CTRL_DF24			(1 << 1)
+#define CTRL_RUN			(1 << 0)
+
+#define CTRL1_FIFO_CLEAR		(1 << 21)
+#define CTRL1_SET_BYTE_PACKAGING(x)	(((x) & 0xf) << 16)
+#define CTRL1_GET_BYTE_PACKAGING(x)	(((x) >> 16) & 0xf)
+
+#define TRANSFER_COUNT_SET_VCOUNT(x)	(((x) & 0xffff) << 16)
+#define TRANSFER_COUNT_GET_VCOUNT(x)	(((x) >> 16) & 0xffff)
+#define TRANSFER_COUNT_SET_HCOUNT(x)	((x) & 0xffff)
+#define TRANSFER_COUNT_GET_HCOUNT(x)	((x) & 0xffff)
+
+
+#define VDCTRL0_ENABLE_PRESENT		(1 << 28)
+#define VDCTRL0_VSYNC_ACT_HIGH		(1 << 27)
+#define VDCTRL0_HSYNC_ACT_HIGH		(1 << 26)
+#define VDCTRL0_DOTCLK_ACTIVE_HIGH	(1 << 25)
+#define VDCTRL0_POL_ACTIVE_HIGH		(1 << 24)
+#define VDCTRL0_VSYNC_PERIOD_UNIT	(1 << 21)
+#define VDCTRL0_VSYNC_PULSE_WIDTH_UNIT	(1 << 20)
+#define VDCTRL0_HALF_LINE		(1 << 19)
+#define VDCTRL0_HALF_LINE_MODE		(1 << 18)
+#define VDCTRL0_SET_VSYNC_PULSE_WIDTH(x) ((x) & 0x3ffff)
+#define VDCTRL0_GET_VSYNC_PULSE_WIDTH(x) ((x) & 0x3ffff)
+
+#define VDCTRL2_SET_HSYNC_PERIOD(x)	((x) & 0x3ffff)
+#define VDCTRL2_GET_HSYNC_PERIOD(x)	((x) & 0x3ffff)
+
+#define VDCTRL3_MUX_SYNC_SIGNALS	(1 << 29)
+#define VDCTRL3_VSYNC_ONLY		(1 << 28)
+#define SET_HOR_WAIT_CNT(x)		(((x) & 0xfff) << 16)
+#define GET_HOR_WAIT_CNT(x)		(((x) >> 16) & 0xfff)
+#define SET_VERT_WAIT_CNT(x)		((x) & 0xffff)
+#define GET_VERT_WAIT_CNT(x)		((x) & 0xffff)
+
+#define VDCTRL4_SET_DOTCLK_DLY(x)	(((x) & 0x7) << 29) /* v4 only */
+#define VDCTRL4_GET_DOTCLK_DLY(x)	(((x) >> 29) & 0x7) /* v4 only */
+#define VDCTRL4_SYNC_SIGNALS_ON		(1 << 18)
+#define SET_DOTCLK_H_VALID_DATA_CNT(x)	((x) & 0x3ffff)
+
+#define DEBUG0_HSYNC			(1 < 26)
+#define DEBUG0_VSYNC			(1 < 25)
+
+#define MIN_XRES			120
+#define MIN_YRES			120
+
+#define RED 0
+#define GREEN 1
+#define BLUE 2
+#define TRANSP 3
+
+enum mxsfb_devtype {
+	MXSFB_V3,
+	MXSFB_V4,
+};
+
+/* CPU dependent register offsets */
+struct mxsfb_devdata {
+	unsigned transfer_count;
+	unsigned cur_buf;
+	unsigned next_buf;
+	unsigned debug0;
+	unsigned hs_wdth_mask;
+	unsigned hs_wdth_shift;
+	unsigned ipversion;
+};
+
+struct mxsfb_info {
+	struct fb_info fb_info;
+	struct platform_device *pdev;
+	struct clk *clk;
+	void __iomem *base;	/* registers */
+	unsigned allocated_size;
+	int enabled;
+	unsigned ld_intf_width;
+	unsigned dotclk_delay;
+	const struct mxsfb_devdata *devdata;
+	int mapped;
+};
+
+#define mxsfb_is_v3(host) (host->devdata->ipversion = 3)
+#define mxsfb_is_v4(host) (host->devdata->ipversion = 4)
+
+static const struct mxsfb_devdata mxsfb_devdata[] = {
+	[MXSFB_V3] = {
+		.transfer_count = LCDC_V3_TRANSFER_COUNT,
+		.cur_buf = LCDC_V3_CUR_BUF,
+		.next_buf = LCDC_V3_NEXT_BUF,
+		.debug0 = LCDC_V3_DEBUG0,
+		.hs_wdth_mask = 0xff,
+		.hs_wdth_shift = 24,
+		.ipversion = 3,
+	},
+	[MXSFB_V4] = {
+		.transfer_count = LCDC_V4_TRANSFER_COUNT,
+		.cur_buf = LCDC_V4_CUR_BUF,
+		.next_buf = LCDC_V4_NEXT_BUF,
+		.debug0 = LCDC_V4_DEBUG0,
+		.hs_wdth_mask = 0x3fff,
+		.hs_wdth_shift = 18,
+		.ipversion = 4,
+	},
+};
+
+#define to_imxfb_host(x) (container_of(x, struct mxsfb_info, fb_info))
+
+/* mask and shift depends on architecture */
+static inline u32 set_hsync_pulse_width(struct mxsfb_info *host, unsigned val)
+{
+	return (val & host->devdata->hs_wdth_mask) <<
+		host->devdata->hs_wdth_shift;
+}
+
+static inline u32 get_hsync_pulse_width(struct mxsfb_info *host, unsigned val)
+{
+	return (val >> host->devdata->hs_wdth_shift) &
+		host->devdata->hs_wdth_mask;
+}
+
+static const struct fb_bitfield def_rgb565[] = {
+	[RED] = {
+		.offset = 11,
+		.length = 5,
+	},
+	[GREEN] = {
+		.offset = 5,
+		.length = 6,
+	},
+	[BLUE] = {
+		.offset = 0,
+		.length = 5,
+	},
+	[TRANSP] = {	/* no support for transparency */
+		.length = 0,
+	}
+};
+
+static const struct fb_bitfield def_rgb666[] = {
+	[RED] = {
+		.offset = 16,
+		.length = 6,
+	},
+	[GREEN] = {
+		.offset = 8,
+		.length = 6,
+	},
+	[BLUE] = {
+		.offset = 0,
+		.length = 6,
+	},
+	[TRANSP] = {	/* no support for transparency */
+		.length = 0,
+	}
+};
+
+static const struct fb_bitfield def_rgb888[] = {
+	[RED] = {
+		.offset = 16,
+		.length = 8,
+	},
+	[GREEN] = {
+		.offset = 8,
+		.length = 8,
+	},
+	[BLUE] = {
+		.offset = 0,
+		.length = 8,
+	},
+	[TRANSP] = {	/* no support for transparency */
+		.length = 0,
+	}
+};
+
+static inline unsigned chan_to_field(unsigned chan, struct fb_bitfield *bf)
+{
+	chan &= 0xffff;
+	chan >>= 16 - bf->length;
+	return chan << bf->offset;
+}
+
+static int mxsfb_check_var(struct fb_var_screeninfo *var,
+		struct fb_info *fb_info)
+{
+	struct mxsfb_info *host = to_imxfb_host(fb_info);
+	const struct fb_bitfield *rgb = NULL;
+
+	if (var->xres < MIN_XRES)
+		var->xres = MIN_XRES;
+	if (var->yres < MIN_YRES)
+		var->yres = MIN_YRES;
+
+	var->xres_virtual = var->xres;
+
+	var->yres_virtual = var->yres;
+
+	switch (var->bits_per_pixel) {
+	case 16:
+		/* always expect RGB 565 */
+		rgb = def_rgb565;
+		break;
+	case 32:
+		switch (host->ld_intf_width) {
+		case STMLCDIF_8BIT:
+			pr_debug("Unsupported LCD bus width mapping\n");
+			break;
+		case STMLCDIF_16BIT:
+		case STMLCDIF_18BIT:
+			/* 24 bit to 18 bit mapping */
+			rgb = def_rgb666;
+			break;
+		case STMLCDIF_24BIT:
+			/* real 24 bit */
+			rgb = def_rgb888;
+			break;
+		}
+		break;
+	default:
+		pr_debug("Unsupported colour depth: %u\n", var->bits_per_pixel);
+		return -EINVAL;
+	}
+
+	/*
+	 * Copy the RGB parameters for this display
+	 * from the machine specific parameters.
+	 */
+	var->red    = rgb[RED];
+	var->green  = rgb[GREEN];
+	var->blue   = rgb[BLUE];
+	var->transp = rgb[TRANSP];
+
+	return 0;
+}
+
+static void mxsfb_enable_controller(struct fb_info *fb_info)
+{
+	struct mxsfb_info *host = to_imxfb_host(fb_info);
+	u32 reg;
+
+	dev_dbg(&host->pdev->dev, "%s\n", __func__);
+
+	clk_enable(host->clk);
+	clk_set_rate(host->clk, PICOS2KHZ(fb_info->var.pixclock) * 1000U);
+
+	/* if it was disabled, re-enable the mode again */
+	writel(CTRL_DOTCLK_MODE, host->base + LCDC_CTRL + REG_SET);
+
+	/* enable the SYNC signals first, then the DMA engine */
+	reg = readl(host->base + LCDC_VDCTRL4);
+	reg |= VDCTRL4_SYNC_SIGNALS_ON;
+	writel(reg, host->base + LCDC_VDCTRL4);
+
+	writel(CTRL_RUN, host->base + LCDC_CTRL + REG_SET);
+
+	host->enabled = 1;
+}
+
+static void mxsfb_disable_controller(struct fb_info *fb_info)
+{
+	struct mxsfb_info *host = to_imxfb_host(fb_info);
+	unsigned loop;
+	u32 reg;
+
+	dev_dbg(&host->pdev->dev, "%s\n", __func__);
+
+	/*
+	 * Even if we disable the controller here, it will still continue
+	 * until its FIFOs are running out of data
+	 */
+	writel(CTRL_DOTCLK_MODE, host->base + LCDC_CTRL + REG_CLR);
+
+	loop = 1000;
+	while (loop) {
+		reg = readl(host->base + LCDC_CTRL);
+		if (!(reg & CTRL_RUN))
+			break;
+		loop--;
+	}
+
+	writel(VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4 + REG_CLR);
+
+	clk_disable(host->clk);
+
+	host->enabled = 0;
+}
+
+static int mxsfb_set_par(struct fb_info *fb_info)
+{
+	struct mxsfb_info *host = to_imxfb_host(fb_info);
+	u32 ctrl, vdctrl0, vdctrl4;
+	int line_size, fb_size;
+	int reenable = 0;
+
+	line_size =  fb_info->var.xres * (fb_info->var.bits_per_pixel >> 3);
+	fb_size = fb_info->var.yres_virtual * line_size;
+
+	if (fb_size > fb_info->fix.smem_len)
+		return -ENOMEM;
+
+	fb_info->fix.line_length = line_size;
+
+	/*
+	 * It seems, you can't re-program the controller if it is still running.
+	 * This may lead into shifted pictures (FIFO issue?).
+	 * So, first stop the controller and drain its FIFOs
+	 */
+	if (host->enabled) {
+		reenable = 1;
+		mxsfb_disable_controller(fb_info);
+	}
+
+	/* clear the FIFOs */
+	writel(CTRL1_FIFO_CLEAR, host->base + LCDC_CTRL1 + REG_SET);
+
+	ctrl = CTRL_BYPASS_COUNT | CTRL_MASTER |
+		CTRL_SET_BUS_WIDTH(host->ld_intf_width);;
+
+	switch (fb_info->var.bits_per_pixel) {
+	case 16:
+		dev_dbg(&host->pdev->dev, "Setting up RGB565 mode\n");
+		ctrl |= CTRL_SET_WORD_LENGTH(0);
+		writel(CTRL1_SET_BYTE_PACKAGING(0xf), host->base + LCDC_CTRL1);
+		break;
+	case 32:
+		dev_dbg(&host->pdev->dev, "Setting up RGB888/666 mode\n");
+		ctrl |= CTRL_SET_WORD_LENGTH(3);
+		switch (host->ld_intf_width) {
+		case STMLCDIF_8BIT:
+			dev_dbg(&host->pdev->dev,
+					"Unsupported LCD bus width mapping\n");
+			return -EINVAL;
+		case STMLCDIF_16BIT:
+		case STMLCDIF_18BIT:
+			/* 24 bit to 18 bit mapping */
+			ctrl |= CTRL_DF24; /* ignore the upper 2 bits in
+					    *  each colour component
+					    */
+			break;
+		case STMLCDIF_24BIT:
+			/* real 24 bit */
+			break;
+		}
+		/* do not use packed pixels = one pixel per word instead */
+		writel(CTRL1_SET_BYTE_PACKAGING(0x7), host->base + LCDC_CTRL1);
+		break;
+	default:
+		dev_dbg(&host->pdev->dev, "Unhandled color depth of %u\n",
+				fb_info->var.bits_per_pixel);
+		return -EINVAL;
+	}
+
+	writel(ctrl, host->base + LCDC_CTRL);
+
+	writel(TRANSFER_COUNT_SET_VCOUNT(fb_info->var.yres) |
+			TRANSFER_COUNT_SET_HCOUNT(fb_info->var.xres),
+			host->base + host->devdata->transfer_count);
+
+	vdctrl0 = VDCTRL0_ENABLE_PRESENT |	/* always in DOTCLOCK mode */
+		VDCTRL0_VSYNC_PERIOD_UNIT |
+		VDCTRL0_VSYNC_PULSE_WIDTH_UNIT |
+		VDCTRL0_SET_VSYNC_PULSE_WIDTH(fb_info->var.vsync_len);
+	if (fb_info->var.sync & FB_SYNC_HOR_HIGH_ACT)
+		vdctrl0 |= VDCTRL0_HSYNC_ACT_HIGH;
+	if (fb_info->var.sync & FB_SYNC_VERT_HIGH_ACT)
+		vdctrl0 |= VDCTRL0_VSYNC_ACT_HIGH;
+	if (fb_info->var.sync & FB_SYNC_DATA_ENABLE_HIGH_ACT)
+		vdctrl0 |= VDCTRL0_POL_ACTIVE_HIGH;
+
+	writel(vdctrl0, host->base + LCDC_VDCTRL0);
+
+	/* frame length in lines */
+	writel(fb_info->var.upper_margin + fb_info->var.vsync_len +
+		fb_info->var.lower_margin + fb_info->var.yres,
+		host->base + LCDC_VDCTRL1);
+
+	/* line length in units of clocks or pixels */
+	writel(set_hsync_pulse_width(host, fb_info->var.hsync_len) |
+		VDCTRL2_SET_HSYNC_PERIOD(fb_info->var.left_margin +
+		fb_info->var.hsync_len + fb_info->var.right_margin +
+		fb_info->var.xres),
+		host->base + LCDC_VDCTRL2);
+
+	writel(SET_HOR_WAIT_CNT(fb_info->var.left_margin +
+		fb_info->var.hsync_len) |
+		SET_VERT_WAIT_CNT(fb_info->var.upper_margin +
+			fb_info->var.vsync_len),
+		host->base + LCDC_VDCTRL3);
+
+	vdctrl4 = SET_DOTCLK_H_VALID_DATA_CNT(fb_info->var.xres);
+	if (mxsfb_is_v4(host))
+		vdctrl4 |= VDCTRL4_SET_DOTCLK_DLY(host->dotclk_delay);
+	writel(vdctrl4, host->base + LCDC_VDCTRL4);
+
+	writel(fb_info->fix.smem_start +
+			fb_info->fix.line_length * fb_info->var.yoffset,
+			host->base + host->devdata->next_buf);
+
+	if (reenable)
+		mxsfb_enable_controller(fb_info);
+
+	return 0;
+}
+
+static int mxsfb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
+		u_int transp, struct fb_info *fb_info)
+{
+	unsigned int val;
+	int ret = -EINVAL;
+
+	/*
+	 * If greyscale is true, then we convert the RGB value
+	 * to greyscale no matter what visual we are using.
+	 */
+	if (fb_info->var.grayscale)
+		red = green = blue = (19595 * red + 38470 * green +
+					7471 * blue) >> 16;
+
+	switch (fb_info->fix.visual) {
+	case FB_VISUAL_TRUECOLOR:
+		/*
+		 * 12 or 16-bit True Colour.  We encode the RGB value
+		 * according to the RGB bitfield information.
+		 */
+		if (regno < 16) {
+			u32 *pal = fb_info->pseudo_palette;
+
+			val  = chan_to_field(red, &fb_info->var.red);
+			val |= chan_to_field(green, &fb_info->var.green);
+			val |= chan_to_field(blue, &fb_info->var.blue);
+
+			pal[regno] = val;
+			ret = 0;
+		}
+		break;
+
+	case FB_VISUAL_STATIC_PSEUDOCOLOR:
+	case FB_VISUAL_PSEUDOCOLOR:
+		break;
+	}
+
+	return ret;
+}
+
+static int mxsfb_blank(int blank, struct fb_info *fb_info)
+{
+	struct mxsfb_info *host = to_imxfb_host(fb_info);
+
+	switch (blank) {
+	case FB_BLANK_POWERDOWN:
+	case FB_BLANK_VSYNC_SUSPEND:
+	case FB_BLANK_HSYNC_SUSPEND:
+	case FB_BLANK_NORMAL:
+		if (host->enabled)
+			mxsfb_disable_controller(fb_info);
+		break;
+
+	case FB_BLANK_UNBLANK:
+		if (!host->enabled)
+			mxsfb_enable_controller(fb_info);
+		break;
+	}
+	return 0;
+}
+
+static int mxsfb_pan_display(struct fb_var_screeninfo *var,
+		struct fb_info *fb_info)
+{
+	struct mxsfb_info *host = to_imxfb_host(fb_info);
+	unsigned offset;
+
+	if (var->xoffset != 0)
+		return -EINVAL;
+
+	offset = fb_info->fix.line_length * var->yoffset;
+
+	/* update on next VSYNC */
+	writel(fb_info->fix.smem_start + offset,
+			host->base + host->devdata->next_buf);
+
+	return 0;
+}
+
+static struct fb_ops mxsfb_ops = {
+	.owner = THIS_MODULE,
+	.fb_check_var = mxsfb_check_var,
+	.fb_set_par = mxsfb_set_par,
+	.fb_setcolreg = mxsfb_setcolreg,
+	.fb_blank = mxsfb_blank,
+	.fb_pan_display = mxsfb_pan_display,
+	.fb_fillrect = cfb_fillrect,
+	.fb_copyarea = cfb_copyarea,
+	.fb_imageblit = cfb_imageblit,
+};
+
+static int __devinit mxsfb_restore_mode(struct mxsfb_info *host)
+{
+	struct fb_info *fb_info = &host->fb_info;
+	unsigned line_count;
+	unsigned period;
+	unsigned long pa, fbsize;
+	int bits_per_pixel, ofs;
+	u32 transfer_count, vdctrl0, vdctrl2, vdctrl3, vdctrl4, ctrl;
+	struct fb_videomode vmode;
+
+	/* Only restore the mode when the controller is running */
+	ctrl = readl(host->base + LCDC_CTRL);
+	if (!(ctrl & CTRL_RUN))
+		return -EINVAL;
+
+	vdctrl0 = readl(host->base + LCDC_VDCTRL0);
+	vdctrl2 = readl(host->base + LCDC_VDCTRL2);
+	vdctrl3 = readl(host->base + LCDC_VDCTRL3);
+	vdctrl4 = readl(host->base + LCDC_VDCTRL4);
+
+	transfer_count = readl(host->base + host->devdata->transfer_count);
+
+	vmode.xres = TRANSFER_COUNT_GET_HCOUNT(transfer_count);
+	vmode.yres = TRANSFER_COUNT_GET_VCOUNT(transfer_count);
+
+	switch (CTRL_GET_WORD_LENGTH(ctrl)) {
+	case 0:
+		bits_per_pixel = 16;
+		break;
+	case 3:
+		bits_per_pixel = 32;
+	case 1:
+	default:
+		return -EINVAL;
+	}
+
+	fb_info->var.bits_per_pixel = bits_per_pixel;
+
+	vmode.pixclock = KHZ2PICOS(clk_get_rate(host->clk) / 1000U);
+	vmode.hsync_len = get_hsync_pulse_width(host, vdctrl2);
+	vmode.left_margin = GET_HOR_WAIT_CNT(vdctrl3) - vmode.hsync_len;
+	vmode.right_margin = VDCTRL2_GET_HSYNC_PERIOD(vdctrl2) - vmode.hsync_len -
+		vmode.left_margin - vmode.xres;
+	vmode.vsync_len = VDCTRL0_GET_VSYNC_PULSE_WIDTH(vdctrl0);
+	period = readl(host->base + LCDC_VDCTRL1);
+	vmode.upper_margin = GET_VERT_WAIT_CNT(vdctrl3) - vmode.vsync_len;
+	vmode.lower_margin = period - vmode.vsync_len - vmode.upper_margin - vmode.yres;
+
+	vmode.vmode = FB_VMODE_NONINTERLACED;
+
+	vmode.sync = 0;
+	if (vdctrl0 & VDCTRL0_HSYNC_ACT_HIGH)
+		vmode.sync |= FB_SYNC_HOR_HIGH_ACT;
+	if (vdctrl0 & VDCTRL0_VSYNC_ACT_HIGH)
+		vmode.sync |= FB_SYNC_VERT_HIGH_ACT;
+
+	pr_debug("Reconstructed video mode:\n");
+	pr_debug("%dx%d, hsync: %u left: %u, right: %u, vsync: %u, upper: %u, lower: %u\n",
+			vmode.xres, vmode.yres,
+			vmode.hsync_len, vmode.left_margin, vmode.right_margin,
+			vmode.vsync_len, vmode.upper_margin, vmode.lower_margin);
+	pr_debug("pixclk: %ldkHz\n", PICOS2KHZ(vmode.pixclock));
+
+	fb_add_videomode(&vmode, &fb_info->modelist);
+
+	host->ld_intf_width = CTRL_GET_BUS_WIDTH(ctrl);
+	host->dotclk_delay = VDCTRL4_GET_DOTCLK_DLY(vdctrl4);
+
+	fb_info->fix.line_length = vmode.xres * (bits_per_pixel >> 3);
+
+	pa = readl(host->base + host->devdata->cur_buf);
+	fbsize = fb_info->fix.line_length * vmode.yres;
+	if (pa < fb_info->fix.smem_start)
+		return -EINVAL;
+	if (pa + fbsize > fb_info->fix.smem_start + fb_info->fix.smem_len)
+		return -EINVAL;
+	ofs = pa - fb_info->fix.smem_start;
+	if (ofs) {
+		memmove(fb_info->screen_base, fb_info->screen_base + ofs, fbsize);
+		writel(fb_info->fix.smem_start, host->base + host->devdata->next_buf);
+	}
+
+	line_count = fb_info->fix.smem_len / fb_info->fix.line_length;
+	fb_info->fix.ypanstep = 1;
+
+	clk_enable(host->clk);
+	host->enabled = 1;
+
+	return 0;
+}
+
+static int __devinit mxsfb_init_fbinfo(struct mxsfb_info *host)
+{
+	struct fb_info *fb_info = &host->fb_info;
+	struct fb_var_screeninfo *var = &fb_info->var;
+	struct mxsfb_platform_data *pdata = host->pdev->dev.platform_data;
+	dma_addr_t fb_phys;
+	void *fb_virt;
+	unsigned fb_size = pdata->fb_size;
+
+	fb_info->fbops = &mxsfb_ops;
+	fb_info->flags = FBINFO_FLAG_DEFAULT | FBINFO_READS_FAST;
+	strlcpy(fb_info->fix.id, "mxs", sizeof(fb_info->fix.id));
+	fb_info->fix.type = FB_TYPE_PACKED_PIXELS;
+	fb_info->fix.ypanstep = 1;
+	fb_info->fix.visual = FB_VISUAL_TRUECOLOR,
+	fb_info->fix.accel = FB_ACCEL_NONE;
+
+	var->bits_per_pixel = pdata->default_bpp ? pdata->default_bpp : 16;
+	var->nonstd = 0;
+	var->activate = FB_ACTIVATE_NOW;
+	var->accel_flags = 0;
+	var->vmode = FB_VMODE_NONINTERLACED;
+
+	host->dotclk_delay = pdata->dotclk_delay;
+	host->ld_intf_width = pdata->ld_intf_width;
+
+	/* Memory allocation for framebuffer */
+	if (pdata->fb_phys) {
+		if (!fb_size)
+			return -EINVAL;
+
+		fb_phys = pdata->fb_phys;
+
+		if (!request_mem_region(fb_phys, fb_size, host->pdev->name))
+			return -ENOMEM;
+
+		fb_virt = ioremap(fb_phys, fb_size);
+		if (!fb_virt) {
+			release_mem_region(fb_phys, fb_size);
+			return -ENOMEM;
+		}
+		host->mapped = 1;
+	} else {
+		if (!fb_size)
+			fb_size = SZ_2M; /* default */
+		fb_virt = alloc_pages_exact(fb_size, GFP_DMA);
+		if (!fb_info->screen_base)
+			return -ENOMEM;
+
+		fb_phys = virt_to_phys(fb_virt);
+	}
+
+	fb_info->fix.smem_start = fb_phys;
+	fb_info->screen_base = fb_virt;
+	fb_info->screen_size = fb_info->fix.smem_len = fb_size;
+
+	if (mxsfb_restore_mode(host))
+		memset(fb_virt, 0, fb_size);
+
+	return 0;
+}
+
+static void __devexit mxsfb_free_videomem(struct mxsfb_info *host)
+{
+	struct fb_info *fb_info = &host->fb_info;
+
+	if (host->mapped) {
+		iounmap(fb_info->screen_base);
+		release_mem_region(fb_info->fix.smem_start,
+				fb_info->screen_size);
+	} else {
+		free_pages_exact(fb_info->screen_base, fb_info->fix.smem_len);
+	}
+}
+
+static int __devinit mxsfb_probe(struct platform_device *pdev)
+{
+	struct mxsfb_platform_data *pdata = pdev->dev.platform_data;
+	struct resource *res;
+	struct mxsfb_info *host;
+	struct fb_info *fb_info;
+	struct fb_modelist *modelist;
+	int i, ret;
+
+	if (!pdata) {
+		dev_err(&pdev->dev, "No platformdata. Giving up\n");
+		return -ENODEV;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "Cannot get memory IO resource\n");
+		return -ENODEV;
+	}
+
+	if (!request_mem_region(res->start, resource_size(res), pdev->name))
+		return -EBUSY;
+
+	fb_info = framebuffer_alloc(sizeof(struct mxsfb_info), &pdev->dev);
+	if (!fb_info) {
+		dev_err(&pdev->dev, "Failed to allocate fbdev\n");
+		ret = -ENOMEM;
+		goto error_alloc_info;
+	}
+
+	host = to_imxfb_host(fb_info);
+
+	host->base = ioremap(res->start, resource_size(res));
+	if (!host->base) {
+		dev_err(&pdev->dev, "ioremap failed\n");
+		ret = -ENOMEM;
+		goto error_ioremap;
+	}
+
+	host->pdev = pdev;
+	platform_set_drvdata(pdev, host);
+
+	host->devdata = &mxsfb_devdata[pdev->id_entry->driver_data];
+
+	host->clk = clk_get(&host->pdev->dev, NULL);
+	if (IS_ERR(host->clk)) {
+		ret = PTR_ERR(host->clk);
+		goto error_getclock;
+	}
+
+	fb_info->pseudo_palette = kmalloc(sizeof(u32) * 16, GFP_KERNEL);
+	if (!fb_info->pseudo_palette) {
+		ret = -ENOMEM;
+		goto error_pseudo_pallette;
+	}
+
+	INIT_LIST_HEAD(&fb_info->modelist);
+
+	ret = mxsfb_init_fbinfo(host);
+	if (ret != 0)
+		goto error_init_fb;
+
+	for (i = 0; i < pdata->mode_count; i++)
+		fb_add_videomode(&pdata->mode_list[i], &fb_info->modelist);
+
+	modelist = list_first_entry(&fb_info->modelist,
+			struct fb_modelist, list);
+	fb_videomode_to_var(&fb_info->var, &modelist->mode);
+
+	/* init the color fields */
+	mxsfb_check_var(&fb_info->var, fb_info);
+
+	platform_set_drvdata(pdev, fb_info);
+
+	ret = register_framebuffer(fb_info);
+	if (ret != 0) {
+		dev_err(&pdev->dev,"Failed to register framebuffer\n");
+		goto error_register;
+	}
+
+	if (!host->enabled) {
+		writel(0, host->base + LCDC_CTRL);
+		mxsfb_set_par(fb_info);
+		mxsfb_enable_controller(fb_info);
+	}
+
+	return 0;
+
+error_register:
+	if (host->enabled)
+		clk_disable(host->clk);
+	fb_destroy_modelist(&fb_info->modelist);
+error_init_fb:
+	kfree(fb_info->pseudo_palette);
+error_pseudo_pallette:
+	clk_put(host->clk);
+error_getclock:
+	iounmap(host->base);
+error_ioremap:
+	framebuffer_release(fb_info);
+error_alloc_info:
+	release_mem_region(res->start, resource_size(res));
+
+	return ret;
+}
+
+static int __devexit mxsfb_remove(struct platform_device *pdev)
+{
+	struct fb_info *fb_info = platform_get_drvdata(pdev);
+	struct mxsfb_info *host = to_imxfb_host(fb_info);
+	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	if (host->enabled)
+		mxsfb_disable_controller(fb_info);
+
+	unregister_framebuffer(fb_info);
+	kfree(fb_info->pseudo_palette);
+	mxsfb_free_videomem(host);
+	iounmap(host->base);
+	clk_put(host->clk);
+
+	framebuffer_release(fb_info);
+	release_mem_region(res->start, resource_size(res));
+
+	platform_set_drvdata(pdev, NULL);
+
+	return 0;
+}
+
+static struct platform_device_id mxsfb_devtype[] = {
+	{
+		.name = "imx23-fb",
+		.driver_data = MXSFB_V3,
+	}, {
+		.name = "imx28-fb",
+		.driver_data = MXSFB_V4,
+	}, {
+	},
+};
+MODULE_DEVICE_TABLE(platform, mxsfb_devtype);
+
+static struct platform_driver mxsfb_driver = {
+	.probe = mxsfb_probe,
+	.remove = __devexit_p(mxsfb_remove),
+	.id_table = mxsfb_devtype,
+	.driver = {
+		   .name = DRIVER_NAME,
+	},
+};
+
+static int __init mxsfb_init(void)
+{
+	return platform_driver_register(&mxsfb_driver);
+}
+
+static void __exit mxsfb_exit(void)
+{
+	platform_driver_unregister(&mxsfb_driver);
+}
+
+module_init(mxsfb_init);
+module_exit(mxsfb_exit);
+
+MODULE_DESCRIPTION("Freescale mxs framebuffer driver");
+MODULE_AUTHOR("Sascha Hauer, Pengutronix");
+MODULE_LICENSE("GPL");
-- 
1.7.2.3


^ permalink raw reply related

* [PATCH 1/8] fb: export fb mode db table
From: Sascha Hauer @ 2011-02-28 10:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1298887229-7987-1-git-send-email-s.hauer@pengutronix.de>

The different modes can be useful for drivers. Currently there is
no way to expose the modes to sysfs, so export them.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Cc: linux-kernel@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
Cc: Paul Mundt <lethal@linux-sh.org>
---
 drivers/video/modedb.c |    8 ++++++--
 include/linux/fb.h     |    3 +++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/video/modedb.c b/drivers/video/modedb.c
index 48c3ea8..942b44d 100644
--- a/drivers/video/modedb.c
+++ b/drivers/video/modedb.c
@@ -36,8 +36,7 @@ EXPORT_SYMBOL_GPL(fb_mode_option);
  *  Standard video mode definitions (taken from XFree86)
  */
 
-static const struct fb_videomode modedb[] = {
-
+const struct fb_videomode modedb[] = {
 	/* 640x400 @ 70 Hz, 31.5 kHz hsync */
 	{ NULL, 70, 640, 400, 39721, 40, 24, 39, 9, 96, 2, 0,
 		FB_VMODE_NONINTERLACED },
@@ -291,6 +290,11 @@ static const struct fb_videomode modedb[] = {
 		0, FB_VMODE_NONINTERLACED },
 };
 
+const struct fb_videomode *fb_modes = modedb;
+EXPORT_SYMBOL(fb_modes);
+const int num_fb_modes = ARRAY_SIZE(modedb);
+EXPORT_SYMBOL(num_fb_modes);
+
 #ifdef CONFIG_FB_MODE_HELPERS
 const struct fb_videomode cea_modes[64] = {
 	/* #1: 640x480p@59.94/60Hz */
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 68ba85a..41b0ce1 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -1154,6 +1154,9 @@ extern const char *fb_mode_option;
 extern const struct fb_videomode vesa_modes[];
 extern const struct fb_videomode cea_modes[64];
 
+extern const struct fb_videomode *fb_modes;
+extern const int num_fb_modes;
+
 struct fb_modelist {
 	struct list_head list;
 	struct fb_videomode mode;
-- 
1.7.2.3


^ permalink raw reply related

* [PATCH 4/8] Add i.MX5 framebuffer driver
From: Sascha Hauer @ 2011-02-28 10:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1298887229-7987-1-git-send-email-s.hauer@pengutronix.de>

This patch adds framebuffer support to the Freescale i.MX SoCs
equipped with an IPU v3, so far these are the i.MX51/53.

This driver has been tested on the i.MX51 babbage board with
both DVI and analog VGA in different resolutions and color depths.
It has also been tested on a custom i.MX51 board using a fixed
resolution panel.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Cc: linux-kernel@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: Samuel Ortiz <sameo@linux.intel.com>
---
 drivers/video/Kconfig  |   11 +
 drivers/video/Makefile |    1 +
 drivers/video/mx5fb.c  |  925 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 937 insertions(+), 0 deletions(-)
 create mode 100644 drivers/video/mx5fb.c

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index ffdb37a..eb00cfa 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2344,6 +2344,17 @@ config FB_MX3
 	  far only synchronous displays are supported. If you plan to use
 	  an LCD display with your i.MX31 system, say Y here.
 
+config FB_MX5
+	tristate "MX5 Framebuffer support"
+	depends on FB && FB_IMX_IPU_V3
+	select FB_CFB_FILLRECT
+	select FB_CFB_COPYAREA
+	select FB_CFB_IMAGEBLIT
+	select FB_MODE_HELPERS
+	help
+	  This is a framebuffer device for the i.MX51 LCD Controller. If you
+	  plan to use an LCD display with your i.MX51 system, say Y here.
+
 config FB_BROADSHEET
 	tristate "E-Ink Broadsheet/Epson S1D13521 controller support"
 	depends on FB
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index dd76680..2116376 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -152,6 +152,7 @@ obj-$(CONFIG_FB_BFIN_LQ035Q1)     += bfin-lq035q1-fb.o
 obj-$(CONFIG_FB_BFIN_T350MCQB)	  += bfin-t350mcqb-fb.o
 obj-$(CONFIG_FB_BFIN_7393)        += bfin_adv7393fb.o
 obj-$(CONFIG_FB_MX3)		  += mx3fb.o
+obj-$(CONFIG_FB_MX5)		  += mx5fb.o
 obj-$(CONFIG_FB_DA8XX)		  += da8xx-fb.o
 obj-$(CONFIG_FB_IMX_IPU_V3)	  += imx-ipu-v3/
 
diff --git a/drivers/video/mx5fb.c b/drivers/video/mx5fb.c
new file mode 100644
index 0000000..86c12d2
--- /dev/null
+++ b/drivers/video/mx5fb.c
@@ -0,0 +1,925 @@
+/*
+ * Copyright 2004-2009 Freescale Semiconductor, Inc. All Rights Reserved.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * Framebuffer Framebuffer Driver for SDC
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/errno.h>
+#include <linux/string.h>
+#include <linux/interrupt.h>
+#include <linux/slab.h>
+#include <linux/fb.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/dma-mapping.h>
+#include <linux/console.h>
+#include <video/imx-ipu-v3.h>
+#include <asm/uaccess.h>
+#include <mach/ipu-v3.h>
+
+#define DRIVER_NAME "imx-ipuv3-fb"
+
+struct imx_ipu_fb_info {
+	int			ipu_channel_num;
+	struct ipu_channel	*ipu_ch;
+	int			dc;
+	int			di_no;
+	u32			ipu_di_pix_fmt;
+	u32			ipu_in_pix_fmt;
+
+	u32			pseudo_palette[16];
+
+	struct ipu_dp		*dp;
+	struct dmfc_channel	*dmfc;
+	struct ipu_di		*di;
+	struct fb_info		*slave;
+	struct fb_info		*master;
+	bool			enabled;
+
+	/* overlay specific fields */
+	int			ovlxres, ovlyres;
+	int			usage;
+};
+
+static int imx_ipu_fb_set_fix(struct fb_info *info)
+{
+	struct fb_fix_screeninfo *fix = &info->fix;
+	struct fb_var_screeninfo *var = &info->var;
+
+	fix->line_length = var->xres_virtual * var->bits_per_pixel / 8;
+
+	fix->type = FB_TYPE_PACKED_PIXELS;
+	fix->accel = FB_ACCEL_NONE;
+	fix->visual = FB_VISUAL_TRUECOLOR;
+	fix->xpanstep = 1;
+	fix->ypanstep = 1;
+
+	return 0;
+}
+
+static int imx_ipu_fb_map_video_memory(struct fb_info *fbi)
+{
+	int size;
+
+	size = fbi->var.yres_virtual * fbi->fix.line_length;
+
+	if (fbi->screen_base) {
+		if (fbi->fix.smem_len >= size)
+			return 0;
+		else
+			return -ENOMEM;
+	}
+
+	fbi->screen_base = dma_alloc_writecombine(fbi->device,
+				size,
+				(dma_addr_t *)&fbi->fix.smem_start,
+				GFP_DMA);
+	if (fbi->screen_base = 0) {
+		dev_err(fbi->device, "Unable to allocate framebuffer memory (%d)\n",
+				fbi->fix.smem_len);
+		fbi->fix.smem_len = 0;
+		fbi->fix.smem_start = 0;
+		return -ENOMEM;
+	}
+
+	fbi->fix.smem_len = size;
+	fbi->screen_size = fbi->fix.smem_len;
+
+	dev_dbg(fbi->device, "allocated fb @ paddr=0x%08lx, size=%d\n",
+		fbi->fix.smem_start, fbi->fix.smem_len);
+
+	/* Clear the screen */
+	memset((char *)fbi->screen_base, 0, fbi->fix.smem_len);
+
+	return 0;
+}
+
+static void imx_ipu_fb_enable(struct fb_info *fbi)
+{
+	struct imx_ipu_fb_info *mxc_fbi = fbi->par;
+
+	if (mxc_fbi->enabled)
+		return;
+
+	ipu_di_enable(mxc_fbi->di);
+	ipu_dmfc_enable_channel(mxc_fbi->dmfc);
+	ipu_idmac_enable_channel(mxc_fbi->ipu_ch);
+	ipu_dc_enable_channel(mxc_fbi->dc);
+	ipu_dp_enable_channel(mxc_fbi->dp);
+	mxc_fbi->enabled = 1;
+}
+
+static void imx_ipu_fb_disable(struct fb_info *fbi)
+{
+	struct imx_ipu_fb_info *mxc_fbi = fbi->par;
+
+	if (!mxc_fbi->enabled)
+		return;
+
+	ipu_dp_disable_channel(mxc_fbi->dp);
+	ipu_dc_disable_channel(mxc_fbi->dc);
+	ipu_idmac_disable_channel(mxc_fbi->ipu_ch);
+	ipu_dmfc_disable_channel(mxc_fbi->dmfc);
+	ipu_di_disable(mxc_fbi->di);
+
+	mxc_fbi->enabled = 0;
+}
+
+static int calc_vref(struct fb_var_screeninfo *var)
+{
+	unsigned long htotal, vtotal;
+
+	htotal = var->xres + var->right_margin + var->hsync_len + var->left_margin;
+	vtotal = var->yres + var->lower_margin + var->vsync_len + var->upper_margin;
+
+	if (!htotal || !vtotal)
+		return 60;
+
+	return PICOS2KHZ(var->pixclock) * 1000 / vtotal / htotal;
+}
+
+static int calc_bandwidth(struct fb_var_screeninfo *var, unsigned int vref)
+{
+	return var->xres * var->yres * vref;
+}
+
+static int imx_ipu_fb_set_par(struct fb_info *fbi)
+{
+	int ret;
+	struct ipu_di_signal_cfg sig_cfg;
+	struct imx_ipu_fb_info *mxc_fbi = fbi->par;
+	u32 out_pixel_fmt;
+	int interlaced = 0;
+	struct fb_var_screeninfo *var = &fbi->var;
+	int enabled = mxc_fbi->enabled;
+
+	dev_dbg(fbi->device, "Reconfiguring framebuffer %dx%d-%d\n",
+		fbi->var.xres, fbi->var.yres, fbi->var.bits_per_pixel);
+
+	if (enabled)
+		imx_ipu_fb_disable(fbi);
+
+	fbi->fix.line_length = var->xres_virtual * var->bits_per_pixel / 8;
+
+	ret = imx_ipu_fb_map_video_memory(fbi);
+	if (ret)
+		return ret;
+
+	if (var->vmode & FB_VMODE_INTERLACED)
+		interlaced = 1;
+
+	memset(&sig_cfg, 0, sizeof(sig_cfg));
+	out_pixel_fmt = mxc_fbi->ipu_di_pix_fmt;
+
+	if (var->vmode & FB_VMODE_INTERLACED)
+		sig_cfg.interlaced = 1;
+	if (var->vmode & FB_VMODE_ODD_FLD_FIRST) /* PAL */
+		sig_cfg.odd_field_first = 1;
+	if (var->sync & FB_SYNC_HOR_HIGH_ACT)
+		sig_cfg.Hsync_pol = 1;
+	if (var->sync & FB_SYNC_VERT_HIGH_ACT)
+		sig_cfg.Vsync_pol = 1;
+	if (!(var->sync & FB_SYNC_CLK_LAT_FALL))
+		sig_cfg.clk_pol = 1;
+	if (var->sync & FB_SYNC_DATA_INVERT)
+		sig_cfg.data_pol = 1;
+	if (!(var->sync & FB_SYNC_OE_LOW_ACT))
+		sig_cfg.enable_pol = 1;
+	if (var->sync & FB_SYNC_CLK_IDLE_EN)
+		sig_cfg.clkidle_en = 1;
+
+	dev_dbg(fbi->device, "pixclock = %lu.%03lu MHz\n",
+		PICOS2KHZ(var->pixclock) / 1000,
+		PICOS2KHZ(var->pixclock) % 1000);
+
+	sig_cfg.width = var->xres;
+	sig_cfg.height = var->yres;
+	sig_cfg.pixel_fmt = out_pixel_fmt;
+	sig_cfg.h_start_width = var->left_margin;
+	sig_cfg.h_sync_width = var->hsync_len;
+	sig_cfg.h_end_width = var->right_margin;
+	sig_cfg.v_start_width = var->upper_margin;
+	sig_cfg.v_sync_width = var->vsync_len;
+	sig_cfg.v_end_width = var->lower_margin;
+	sig_cfg.v_to_h_sync = 0;
+
+	if (mxc_fbi->dp) {
+		ret = ipu_dp_setup_channel(mxc_fbi->dp, mxc_fbi->ipu_in_pix_fmt,
+				out_pixel_fmt, 1);
+		if (ret) {
+			dev_dbg(fbi->device, "initializing display processor failed with %d\n",
+				ret);
+			return ret;
+		}
+	}
+
+	ret = ipu_dc_init_sync(mxc_fbi->dc, mxc_fbi->di_no, interlaced,
+			out_pixel_fmt, fbi->var.xres);
+	if (ret) {
+		dev_dbg(fbi->device, "initializing display controller failed with %d\n",
+				ret);
+		return ret;
+	}
+
+	ret = ipu_di_init_sync_panel(mxc_fbi->di,
+				PICOS2KHZ(var->pixclock) * 1000UL,
+				&sig_cfg);
+	if (ret) {
+		dev_dbg(fbi->device, "initializing panel failed with %d\n",
+				ret);
+		return ret;
+	}
+
+	fbi->mode = (struct fb_videomode *)fb_match_mode(var, &fbi->modelist);
+	var->xoffset = var->yoffset = 0;
+
+	if (fbi->var.vmode & FB_VMODE_INTERLACED)
+		interlaced = 1;
+
+	ret = ipu_idmac_init_channel_buffer(mxc_fbi->ipu_ch,
+					mxc_fbi->ipu_in_pix_fmt,
+					var->xres, var->yres,
+					fbi->fix.line_length,
+					IPU_ROTATE_NONE,
+					fbi->fix.smem_start,
+					0,
+					0, 0, interlaced);
+	if (ret) {
+		dev_dbg(fbi->device, "init channel buffer failed with %d\n",
+				ret);
+		return ret;
+	}
+
+	ret = ipu_dmfc_init_channel(mxc_fbi->dmfc, var->xres);
+	if (ret) {
+		dev_dbg(fbi->device, "initializing dmfc channel failed with %d\n",
+				ret);
+		return ret;
+	}
+
+	ret = ipu_dmfc_alloc_bandwidth(mxc_fbi->dmfc, calc_bandwidth(var, calc_vref(var)));
+	if (ret) {
+		dev_dbg(fbi->device, "allocating dmfc bandwidth failed with %d\n",
+				ret);
+		return ret;
+	}
+
+	if (enabled)
+		imx_ipu_fb_enable(fbi);
+
+	return ret;
+}
+
+/*
+ * These are the bitfields for each
+ * display depth that we support.
+ */
+struct imxfb_rgb {
+	struct fb_bitfield	red;
+	struct fb_bitfield	green;
+	struct fb_bitfield	blue;
+	struct fb_bitfield	transp;
+};
+
+static struct imxfb_rgb def_rgb_8 = {
+	.red	= { .offset =  5, .length = 3, },
+	.green	= { .offset =  2, .length = 3, },
+	.blue	= { .offset =  0, .length = 2, },
+	.transp = { .offset =  0, .length = 0, },
+};
+
+static struct imxfb_rgb def_rgb_16 = {
+	.red	= { .offset = 11, .length = 5, },
+	.green	= { .offset =  5, .length = 6, },
+	.blue	= { .offset =  0, .length = 5, },
+	.transp = { .offset =  0, .length = 0, },
+};
+
+static struct imxfb_rgb def_rgb_24 = {
+	.red	= { .offset = 16, .length = 8, },
+	.green	= { .offset =  8, .length = 8, },
+	.blue	= { .offset =  0, .length = 8, },
+	.transp = { .offset =  0, .length = 0, },
+};
+
+static struct imxfb_rgb def_rgb_32 = {
+	.red	= { .offset = 16, .length = 8, },
+	.green	= { .offset =  8, .length = 8, },
+	.blue	= { .offset =  0, .length = 8, },
+	.transp = { .offset = 24, .length = 8, },
+};
+
+/*
+ * Check framebuffer variable parameters and adjust to valid values.
+ *
+ * @param       var      framebuffer variable parameters
+ *
+ * @param       info     framebuffer information pointer
+ */
+static int imx_ipu_fb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
+{
+	struct imx_ipu_fb_info *mxc_fbi = info->par;
+	struct imxfb_rgb *rgb;
+
+	/* we don't support xpan, force xres_virtual to be equal to xres */
+	var->xres_virtual = var->xres;
+
+	if (var->yres_virtual < var->yres)
+		var->yres_virtual = var->yres;
+
+	switch (var->bits_per_pixel) {
+	case 8:
+		rgb = &def_rgb_8;
+		break;
+	case 16:
+		rgb = &def_rgb_16;
+		mxc_fbi->ipu_in_pix_fmt = IPU_PIX_FMT_RGB565;
+		break;
+	case 24:
+		rgb = &def_rgb_24;
+		mxc_fbi->ipu_in_pix_fmt = IPU_PIX_FMT_BGR24;
+		break;
+	case 32:
+		rgb = &def_rgb_32;
+		mxc_fbi->ipu_in_pix_fmt = IPU_PIX_FMT_BGR32;
+		break;
+	default:
+		var->bits_per_pixel = 24;
+		rgb = &def_rgb_24;
+		mxc_fbi->ipu_in_pix_fmt = IPU_PIX_FMT_BGR24;
+	}
+
+	var->red    = rgb->red;
+	var->green  = rgb->green;
+	var->blue   = rgb->blue;
+	var->transp = rgb->transp;
+
+	return 0;
+}
+
+static inline unsigned int chan_to_field(u_int chan, struct fb_bitfield *bf)
+{
+	chan &= 0xffff;
+	chan >>= 16 - bf->length;
+	return chan << bf->offset;
+}
+
+static int imx_ipu_fb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
+			   u_int trans, struct fb_info *fbi)
+{
+	unsigned int val;
+	int ret = 1;
+
+	/*
+	 * If greyscale is true, then we convert the RGB value
+	 * to greyscale no matter what visual we are using.
+	 */
+	if (fbi->var.grayscale)
+		red = green = blue = (19595 * red + 38470 * green +
+				      7471 * blue) >> 16;
+	switch (fbi->fix.visual) {
+	case FB_VISUAL_TRUECOLOR:
+		/*
+		 * 16-bit True Colour.  We encode the RGB value
+		 * according to the RGB bitfield information.
+		 */
+		if (regno < 16) {
+			u32 *pal = fbi->pseudo_palette;
+
+			val = chan_to_field(red, &fbi->var.red);
+			val |= chan_to_field(green, &fbi->var.green);
+			val |= chan_to_field(blue, &fbi->var.blue);
+
+			pal[regno] = val;
+			ret = 0;
+		}
+		break;
+
+	case FB_VISUAL_STATIC_PSEUDOCOLOR:
+	case FB_VISUAL_PSEUDOCOLOR:
+		break;
+	}
+
+	return ret;
+}
+
+static void imx_ipu_fb_enable_overlay(struct fb_info *fbi);
+static void imx_ipu_fb_disable_overlay(struct fb_info *fbi);
+
+static int imx_ipu_fb_blank(int blank, struct fb_info *info)
+{
+	struct imx_ipu_fb_info *mxc_fbi = info->par;
+
+	dev_dbg(info->device, "blank = %d\n", blank);
+
+	switch (blank) {
+	case FB_BLANK_POWERDOWN:
+	case FB_BLANK_VSYNC_SUSPEND:
+	case FB_BLANK_HSYNC_SUSPEND:
+	case FB_BLANK_NORMAL:
+		if (mxc_fbi->slave)
+			imx_ipu_fb_disable_overlay(mxc_fbi->slave);
+		imx_ipu_fb_disable(info);
+		break;
+	case FB_BLANK_UNBLANK:
+		imx_ipu_fb_enable(info);
+		if (mxc_fbi->slave)
+			imx_ipu_fb_enable_overlay(mxc_fbi->slave);
+		break;
+	}
+
+	return 0;
+}
+
+static int imx_ipu_fb_pan_display(struct fb_var_screeninfo *var,
+		struct fb_info *info)
+{
+	struct imx_ipu_fb_info *mxc_fbi = info->par;
+	unsigned long base;
+	int ret;
+
+	if (info->var.yoffset = var->yoffset)
+		return 0;	/* No change, do nothing */
+
+	base = var->yoffset * var->xres_virtual * var->bits_per_pixel / 8;
+	base += info->fix.smem_start;
+
+	ret = ipu_wait_for_interrupt(IPU_IRQ_EOF(mxc_fbi->ipu_channel_num), 100);
+	if (ret)
+		return ret;
+
+	if (ipu_idmac_update_channel_buffer(mxc_fbi->ipu_ch, 0, base)) {
+		dev_err(info->device,
+			"Error updating SDC buf to address=0x%08lX\n", base);
+	}
+
+	info->var.yoffset = var->yoffset;
+
+	return 0;
+}
+
+static struct fb_ops imx_ipu_fb_ops = {
+	.owner		= THIS_MODULE,
+	.fb_set_par	= imx_ipu_fb_set_par,
+	.fb_check_var	= imx_ipu_fb_check_var,
+	.fb_setcolreg	= imx_ipu_fb_setcolreg,
+	.fb_pan_display	= imx_ipu_fb_pan_display,
+	.fb_fillrect	= cfb_fillrect,
+	.fb_copyarea	= cfb_copyarea,
+	.fb_imageblit	= cfb_imageblit,
+	.fb_blank	= imx_ipu_fb_blank,
+};
+
+/*
+ * Overlay functions
+ */
+static void imx_ipu_fb_enable_overlay(struct fb_info *ovl)
+{
+	struct imx_ipu_fb_info *mxc_ovl = ovl->par;
+
+	if (mxc_ovl->enabled)
+		return;
+
+	ipu_dmfc_enable_channel(mxc_ovl->dmfc);
+	ipu_idmac_enable_channel(mxc_ovl->ipu_ch);
+	ipu_dp_enable_fg(mxc_ovl->dp);
+	mxc_ovl->enabled = 1;
+}
+
+#define IPUV3_IRQ_DP_SF_END	451
+
+static void imx_ipu_fb_disable_overlay(struct fb_info *ovl)
+{
+	struct imx_ipu_fb_info *mxc_ovl = ovl->par;
+
+	if (!mxc_ovl->enabled)
+		return;
+
+	ipu_dp_disable_fg(mxc_ovl->dp);
+	ipu_wait_for_interrupt(IPUV3_IRQ_DP_SF_END, 100);
+	ipu_idmac_disable_channel(mxc_ovl->ipu_ch);
+	ipu_dmfc_disable_channel(mxc_ovl->dmfc);
+	mxc_ovl->enabled = 0;
+}
+
+#define NONSTD_TO_XPOS(x)	(((x) >> 0)  & 0xfff)
+#define NONSTD_TO_YPOS(x)	(((x) >> 12) & 0xfff)
+#define NONSTD_TO_ALPHA(x)	(((x) >> 24) & 0xff)
+
+static int imx_ipu_fb_check_var_overlay(struct fb_var_screeninfo *var,
+		struct fb_info *ovl)
+{
+	struct imx_ipu_fb_info *mxc_ovl = ovl->par;
+	struct fb_info *fbi_master = mxc_ovl->master;
+	struct fb_var_screeninfo *var_master = &fbi_master->var;
+	int ret;
+	static int xpos, ypos;
+
+	xpos = NONSTD_TO_XPOS(var->nonstd);
+	ypos = NONSTD_TO_YPOS(var->nonstd);
+
+	ret = imx_ipu_fb_check_var(var, ovl);
+	if (ret)
+		return ret;
+
+	if (var->xres + xpos > var_master->xres)
+		return -EINVAL;
+	if (var->yres + ypos > var_master->yres)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int imx_ipu_fb_set_par_overlay(struct fb_info *ovl)
+{
+	struct imx_ipu_fb_info *mxc_ovl = ovl->par;
+	struct fb_var_screeninfo *var = &ovl->var;
+	struct fb_info *fbi_master = mxc_ovl->master;
+	struct imx_ipu_fb_info *mxc_fbi_master = fbi_master->par;
+	struct fb_var_screeninfo *var_master = &fbi_master->var;
+	int ret;
+	int interlaced = 0;
+	int xpos, ypos, alpha;
+	int resolution_change;
+
+	dev_dbg(ovl->device, "Reconfiguring framebuffer %dx%d-%d\n",
+		ovl->var.xres, ovl->var.yres, ovl->var.bits_per_pixel);
+
+	resolution_change = mxc_ovl->ovlxres != var->xres ||
+		mxc_ovl->ovlyres != var->yres;
+
+	if (mxc_ovl->enabled && resolution_change)
+		imx_ipu_fb_disable_overlay(ovl);
+
+	ovl->fix.line_length = var->xres_virtual *
+                                  var->bits_per_pixel / 8;
+
+	xpos = NONSTD_TO_XPOS(var->nonstd);
+	ypos = NONSTD_TO_YPOS(var->nonstd);
+	alpha = NONSTD_TO_ALPHA(var->nonstd);
+
+	if (resolution_change) {
+		ret = imx_ipu_fb_map_video_memory(ovl);
+		if (ret)
+			return ret;
+	}
+
+	if (!resolution_change && mxc_ovl->enabled)
+		ipu_wait_for_interrupt(IPU_IRQ_EOF(mxc_fbi_master->ipu_channel_num), 100);
+
+	ipu_dp_set_window_pos(mxc_ovl->dp, xpos, ypos);
+	ipu_dp_set_global_alpha(mxc_ovl->dp, 1, alpha, 1);
+
+	var->xoffset = var->yoffset = 0;
+
+	if (resolution_change) {
+		if (var->vmode & FB_VMODE_INTERLACED)
+			interlaced = 1;
+
+		ret = ipu_idmac_init_channel_buffer(mxc_ovl->ipu_ch,
+					mxc_ovl->ipu_in_pix_fmt,
+					var->xres, var->yres,
+					ovl->fix.line_length,
+					IPU_ROTATE_NONE,
+					ovl->fix.smem_start,
+					0,
+					0, 0, interlaced);
+		if (ret) {
+			dev_dbg(ovl->device, "init channel buffer failed with %d\n",
+				ret);
+			return ret;
+		}
+
+		ret = ipu_dmfc_init_channel(mxc_ovl->dmfc, var->xres);
+		if (ret) {
+			dev_dbg(ovl->device, "initializing dmfc channel failed with %d\n",
+				ret);
+			return ret;
+		}
+
+		ret = ipu_dmfc_alloc_bandwidth(mxc_ovl->dmfc, calc_bandwidth(var, calc_vref(var_master)));
+		if (ret) {
+			dev_dbg(ovl->device, "allocating dmfc bandwidth failed with %d\n",
+				ret);
+			return ret;
+		}
+		mxc_ovl->ovlxres = var->xres;
+		mxc_ovl->ovlyres = var->yres;
+	}
+
+	if (mxc_fbi_master->enabled)
+		imx_ipu_fb_enable_overlay(ovl);
+
+	return ret;
+}
+
+static int imx_overlayfb_open(struct fb_info *ovl, int user)
+{
+	struct imx_ipu_fb_info *mxc_ovl = ovl->par;
+
+	if (mxc_ovl->usage++ = 0)
+		printk("enable ovl\n");
+
+	return 0;
+}
+
+static int imx_overlayfb_release(struct fb_info *ovl, int user)
+{
+	struct imx_ipu_fb_info *mxc_ovl = ovl->par;
+
+	if (--mxc_ovl->usage = 0) {
+		printk("disable ovl\n");
+
+		if (ovl->screen_base)
+			dma_free_writecombine(ovl->device, ovl->fix.smem_len,
+			      ovl->screen_base, ovl->fix.smem_start);
+		ovl->fix.smem_len = 0;
+		ovl->fix.smem_start = 0;
+		ovl->screen_base = NULL;
+		mxc_ovl->ovlxres = 0;
+		mxc_ovl->ovlyres = 0;
+	}
+
+	return 0;
+}
+
+static struct fb_ops imx_ipu_fb_overlay_ops = {
+	.owner		= THIS_MODULE,
+	.fb_set_par	= imx_ipu_fb_set_par_overlay,
+	.fb_check_var	= imx_ipu_fb_check_var_overlay,
+	.fb_setcolreg	= imx_ipu_fb_setcolreg,
+	.fb_pan_display	= imx_ipu_fb_pan_display,
+	.fb_fillrect	= cfb_fillrect,
+	.fb_copyarea	= cfb_copyarea,
+	.fb_imageblit	= cfb_imageblit,
+	.fb_open	= imx_overlayfb_open,
+	.fb_release	= imx_overlayfb_release,
+};
+
+static struct fb_info *imx_ipu_fb_init_fbinfo(struct device *dev, struct fb_ops *ops)
+{
+	struct fb_info *fbi;
+	struct imx_ipu_fb_info *mxc_fbi;
+
+	fbi = framebuffer_alloc(sizeof(struct imx_ipu_fb_info), dev);
+	if (!fbi)
+		return NULL;
+
+	BUG_ON(fbi->par = NULL);
+	mxc_fbi = fbi->par;
+
+	fbi->var.activate = FB_ACTIVATE_NOW;
+
+	fbi->fbops = ops;
+	fbi->flags = FBINFO_FLAG_DEFAULT;
+	fbi->pseudo_palette = mxc_fbi->pseudo_palette;
+
+	fb_alloc_cmap(&fbi->cmap, 16, 0);
+
+	return fbi;
+}
+
+static int imx_ipu_fb_init_overlay(struct platform_device *pdev,
+		struct fb_info *fbi_master, int ipu_channel)
+{
+	struct imx_ipu_fb_info *mxc_fbi_master = fbi_master->par;
+	struct fb_info *ovl;
+	struct imx_ipu_fb_info *mxc_ovl;
+	int ret;
+
+	ovl = imx_ipu_fb_init_fbinfo(&pdev->dev, &imx_ipu_fb_overlay_ops);
+	if (!ovl)
+		return -ENOMEM;
+
+	mxc_ovl = ovl->par;
+	mxc_ovl->ipu_ch = ipu_idmac_get(ipu_channel);
+	mxc_ovl->dmfc = ipu_dmfc_get(ipu_channel);
+	mxc_ovl->di = NULL;
+	mxc_ovl->dp = mxc_fbi_master->dp;
+	mxc_ovl->master = fbi_master;
+	mxc_fbi_master->slave = ovl;
+
+	imx_ipu_fb_check_var(&ovl->var, ovl);
+	imx_ipu_fb_set_fix(ovl);
+
+	ret = register_framebuffer(ovl);
+	if (ret) {
+		framebuffer_release(ovl);
+		return ret;
+	}
+
+	ipu_dp_set_global_alpha(mxc_ovl->dp, 0, 0, 1);
+	ipu_dp_set_color_key(mxc_ovl->dp, 1, 0x434343);
+
+	return 0;
+}
+
+static void imx_ipu_fb_exit_overlay(struct platform_device *pdev,
+		struct fb_info *fbi_master, int ipu_channel)
+{
+	struct imx_ipu_fb_info *mxc_fbi_master = fbi_master->par;
+	struct fb_info *ovl = mxc_fbi_master->slave;
+	struct imx_ipu_fb_info *mxc_ovl = ovl->par;
+
+	if (mxc_ovl->enabled)
+		imx_ipu_fb_disable_overlay(ovl);
+
+	unregister_framebuffer(ovl);
+
+	ipu_idmac_put(mxc_ovl->ipu_ch);
+	ipu_dmfc_free_bandwidth(mxc_ovl->dmfc);
+	ipu_dmfc_put(mxc_ovl->dmfc);
+
+	framebuffer_release(ovl);
+}
+
+static int imx_ipu_fb_find_mode(struct fb_info *fbi)
+{
+	int ret;
+	struct fb_videomode *mode_array;
+	struct fb_modelist *modelist;
+	struct fb_var_screeninfo *var = &fbi->var;
+	int i = 0;
+
+	list_for_each_entry(modelist, &fbi->modelist, list)
+		i++;
+
+	mode_array = kmalloc(sizeof (struct fb_modelist) * i, GFP_KERNEL);
+	if (!mode_array)
+		return -ENOMEM;
+
+	i = 0;
+	list_for_each_entry(modelist, &fbi->modelist, list)
+		mode_array[i++] = modelist->mode;
+
+	ret = fb_find_mode(&fbi->var, fbi, NULL, mode_array, i, NULL, 16);
+	if (ret = 0)
+		return -EINVAL;
+
+	dev_dbg(fbi->device, "found %dx%d-%d hs:%d:%d:%d vs:%d:%d:%d\n",
+			var->xres, var->yres, var->bits_per_pixel,
+			var->hsync_len, var->left_margin, var->right_margin,
+			var->vsync_len, var->upper_margin, var->lower_margin);
+
+	kfree(mode_array);
+
+	return 0;
+}
+
+static int __devinit imx_ipu_fb_probe(struct platform_device *pdev)
+{
+	struct fb_info *fbi;
+	struct imx_ipu_fb_info *mxc_fbi;
+	struct ipuv3_fb_platform_data *plat_data = pdev->dev.platform_data;
+	int ret = 0, i;
+
+	pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
+
+	fbi = imx_ipu_fb_init_fbinfo(&pdev->dev, &imx_ipu_fb_ops);
+	if (!fbi)
+		return -ENOMEM;
+
+	mxc_fbi = fbi->par;
+
+	mxc_fbi->ipu_channel_num = plat_data->ipu_channel_bg;
+	mxc_fbi->dc = plat_data->dc_channel;
+	mxc_fbi->ipu_di_pix_fmt = plat_data->interface_pix_fmt;
+	mxc_fbi->di_no = plat_data->display;
+
+	mxc_fbi->ipu_ch = ipu_idmac_get(plat_data->ipu_channel_bg);
+	if (IS_ERR(mxc_fbi->ipu_ch)) {
+		ret = PTR_ERR(mxc_fbi->ipu_ch);
+		goto failed_request_ipu;
+	}
+
+	mxc_fbi->dmfc = ipu_dmfc_get(plat_data->ipu_channel_bg);
+	if (IS_ERR(mxc_fbi->ipu_ch)) {
+		ret = PTR_ERR(mxc_fbi->ipu_ch);
+		goto failed_request_dmfc;
+	}
+
+	if (plat_data->dp_channel >= 0) {
+		mxc_fbi->dp = ipu_dp_get(plat_data->dp_channel);
+		if (IS_ERR(mxc_fbi->dp)) {
+			ret = PTR_ERR(mxc_fbi->ipu_ch);
+			goto failed_request_dp;
+		}
+	}
+
+	mxc_fbi->di = ipu_di_get(plat_data->display);
+	if (IS_ERR(mxc_fbi->di)) {
+		ret = PTR_ERR(mxc_fbi->di);
+		goto failed_request_di;
+	}
+
+	fbi->var.yres_virtual = fbi->var.yres;
+
+	INIT_LIST_HEAD(&fbi->modelist);
+	for (i = 0; i < plat_data->num_modes; i++)
+		fb_add_videomode(&plat_data->modes[i], &fbi->modelist);
+
+	if (plat_data->flags & IMX_IPU_FB_USE_MODEDB) {
+		for (i = 0; i < num_fb_modes; i++)
+			fb_add_videomode(&fb_modes[i], &fbi->modelist);
+	}
+
+	imx_ipu_fb_find_mode(fbi);
+
+	imx_ipu_fb_check_var(&fbi->var, fbi);
+	imx_ipu_fb_set_fix(fbi);
+	ret = register_framebuffer(fbi);
+	if (ret < 0)
+		goto failed_register;
+
+	imx_ipu_fb_set_par(fbi);
+	imx_ipu_fb_blank(FB_BLANK_UNBLANK, fbi);
+
+	if (plat_data->ipu_channel_fg >= 0 && plat_data->flags & IMX_IPU_FB_USE_OVERLAY)
+		imx_ipu_fb_init_overlay(pdev, fbi, plat_data->ipu_channel_fg);
+
+	platform_set_drvdata(pdev, fbi);
+
+	return 0;
+
+failed_register:
+	ipu_di_put(mxc_fbi->di);
+failed_request_di:
+	if (plat_data->dp_channel >= 0)
+		ipu_dp_put(mxc_fbi->dp);
+failed_request_dp:
+	ipu_dmfc_put(mxc_fbi->dmfc);
+failed_request_dmfc:
+	ipu_idmac_put(mxc_fbi->ipu_ch);
+failed_request_ipu:
+	fb_dealloc_cmap(&fbi->cmap);
+	framebuffer_release(fbi);
+
+	return ret;
+}
+
+static int __devexit imx_ipu_fb_remove(struct platform_device *pdev)
+{
+	struct fb_info *fbi = platform_get_drvdata(pdev);
+	struct imx_ipu_fb_info *mxc_fbi = fbi->par;
+	struct ipuv3_fb_platform_data *plat_data = pdev->dev.platform_data;
+
+	if (plat_data->ipu_channel_fg >= 0 && plat_data->flags & IMX_IPU_FB_USE_OVERLAY)
+		imx_ipu_fb_exit_overlay(pdev, fbi, plat_data->ipu_channel_fg);
+
+	imx_ipu_fb_blank(FB_BLANK_POWERDOWN, fbi);
+
+	dma_free_writecombine(fbi->device, fbi->fix.smem_len,
+			      fbi->screen_base, fbi->fix.smem_start);
+
+	if (&fbi->cmap)
+		fb_dealloc_cmap(&fbi->cmap);
+
+	unregister_framebuffer(fbi);
+
+	if (plat_data->dp_channel >= 0)
+		ipu_dp_put(mxc_fbi->dp);
+	ipu_dmfc_free_bandwidth(mxc_fbi->dmfc);
+	ipu_dmfc_put(mxc_fbi->dmfc);
+	ipu_di_put(mxc_fbi->di);
+	ipu_idmac_put(mxc_fbi->ipu_ch);
+
+	framebuffer_release(fbi);
+
+	return 0;
+}
+
+static struct platform_driver imx_ipu_fb_driver = {
+	.driver = {
+		.name = DRIVER_NAME,
+	},
+	.probe = imx_ipu_fb_probe,
+	.remove = __devexit_p(imx_ipu_fb_remove),
+};
+
+static int __init imx_ipu_fb_init(void)
+{
+	return platform_driver_register(&imx_ipu_fb_driver);
+}
+
+static void __exit imx_ipu_fb_exit(void)
+{
+	platform_driver_unregister(&imx_ipu_fb_driver);
+}
+
+module_init(imx_ipu_fb_init);
+module_exit(imx_ipu_fb_exit);
+
+MODULE_AUTHOR("Freescale Semiconductor, Inc.");
+MODULE_DESCRIPTION("i.MX framebuffer driver");
+MODULE_LICENSE("GPL");
+MODULE_SUPPORTED_DEVICE("fb");
-- 
1.7.2.3


^ permalink raw reply related

* (WARNING) possible deadlock in del_timer_sync, called from fbcon_del_cursor_timer
From: Clemens Ladisch @ 2011-02-28 14:41 UTC (permalink / raw)
  To: linux-fbdev; +Cc: linux-kernel

While using the FB console, I tried to change the console log level
with SysRq, and got this warning that del_timer_sync() is used in
an interrupt handler:

SysRq :
------------[ cut here ]------------
WARNING: at kernel/timer.c:983 del_timer_sync+0x93/0xae()
Hardware name: System Product Name
Modules linked in: soundcore firewire_ohci firewire_core ohci_hcd ehci_hcd
Pid: 0, comm: kworker/0:1 Not tainted 2.6.38-rc6+ #235
Call Trace:
 <IRQ>  [<ffffffff8103a6df>] ? warn_slowpath_common+0x80/0x98
 [<ffffffff8103a70c>] ? warn_slowpath_null+0x15/0x17
 [<ffffffff810452a7>] ? del_timer_sync+0x93/0xae
 [<ffffffff81045214>] ? del_timer_sync+0x0/0xae
 [<ffffffff8120725a>] ? fbcon_del_cursor_timer+0x32/0x3d
 [<ffffffff812077f0>] ? fbcon_cursor+0x9a/0x152
 [<ffffffff8125fd84>] ? hide_cursor+0x2c/0x7d
 [<ffffffff8126022b>] ? vt_console_print+0xd9/0x2f7
 [<ffffffff8103a891>] ? __call_console_drivers+0x67/0x79
 [<ffffffff8103a8fc>] ? _call_console_drivers+0x59/0x5d
 [<ffffffff8103ae3e>] ? console_unlock+0x160/0x1ce
 [<ffffffff8103b44a>] ? vprintk+0x358/0x38e
 [<ffffffff81466199>] ? printk+0x3c/0x3e
 [<ffffffff812594ce>] ? __handle_sysrq+0x23/0x157
 [<ffffffff812594f0>] ? __handle_sysrq+0x45/0x157
 [<ffffffff81259758>] ? sysrq_filter+0x116/0x17b
 [<ffffffff8139b9c5>] ? input_pass_event+0xbe/0x111
 [<ffffffff8139b907>] ? input_pass_event+0x0/0x111
 [<ffffffff8139cfb4>] ? input_handle_event+0x42a/0x439
 [<ffffffff8139d0e9>] ? input_event+0x5b/0x7a
 [<ffffffff813a37f3>] ? atkbd_interrupt+0x50f/0x5e0
 [<ffffffff8139735c>] ? serio_interrupt+0x40/0x7c
 [<ffffffff81398647>] ? i8042_interrupt+0x289/0x2a3
 [<ffffffff8107ae98>] ? handle_IRQ_event+0x20/0xa8
 [<ffffffff8107cd68>] ? handle_edge_irq+0x103/0x14f
 [<ffffffff81004a2b>] ? handle_irq+0x83/0x8c
 [<ffffffff81004067>] ? do_IRQ+0x48/0xaf
 [<ffffffff81469a93>] ? ret_from_intr+0x0/0x13
 <EOI>  [<ffffffff8105d49e>] ? tick_broadcast_oneshot_control+0x1a/0xfb
 [<ffffffff810098cc>] ? default_idle+0x27/0x43
 [<ffffffff810098ce>] ? default_idle+0x29/0x43
 [<ffffffff810098cc>] ? default_idle+0x27/0x43
 [<ffffffff81009a33>] ? c1e_idle+0xcd/0xf4
 [<ffffffff810012e5>] ? cpu_idle+0x5f/0x96
 [<ffffffff81462b7c>] ? start_secondary+0x1e3/0x1e5
---[ end trace 80565a42945fc993 ]---
Changing Loglevel
Loglevel set to 3


To add insult to injury, my cursor is configured not to blink.


Regards,
Clemens

^ permalink raw reply

* Re: [PATCH 3/8] Add a mfd IPUv3 driver
From: Arnd Bergmann @ 2011-02-28 17:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1298887229-7987-4-git-send-email-s.hauer@pengutronix.de>

Hi Sascha,

I've had a brief look around the driver. It looks reasonable in general,
but the division into subdrivers feels a bit ad-hoc. If all the components
are built into a single module, I don't see the need for separating the
data by functional units by file. It seems simple enough to turn this
into a layered driver with multiple sub-devices each derived from a 
platform_device on its own.

On Monday 28 February 2011, Sascha Hauer wrote:
>  arch/arm/plat-mxc/include/mach/ipu-v3.h |   49 +++
>  drivers/video/Kconfig                   |    2 +
>  drivers/video/Makefile                  |    1 +
>  drivers/video/imx-ipu-v3/Kconfig        |   10 +
>  drivers/video/imx-ipu-v3/Makefile       |    3 +
>  drivers/video/imx-ipu-v3/ipu-common.c   |  666 +++++++++++++++++++++++++++++++
>  drivers/video/imx-ipu-v3/ipu-cpmem.c    |  612 ++++++++++++++++++++++++++++
>  drivers/video/imx-ipu-v3/ipu-dc.c       |  364 +++++++++++++++++
>  drivers/video/imx-ipu-v3/ipu-di.c       |  550 +++++++++++++++++++++++++
>  drivers/video/imx-ipu-v3/ipu-dmfc.c     |  355 ++++++++++++++++
>  drivers/video/imx-ipu-v3/ipu-dp.c       |  476 ++++++++++++++++++++++
>  drivers/video/imx-ipu-v3/ipu-prv.h      |  216 ++++++++++
>  include/video/imx-ipu-v3.h              |  219 ++++++++++

I wonder if this is something that would fit better in drivers/gpu instead
of drivers/video. We recently discussed the benefits of KMS vs fb drivers,
and I think it would be good to be prepared for making this a KMS driver
in the long run, even if the immediate target has to be a simple frame buffer
driver.

> +#include "ipu-prv.h"
> +
> +static struct clk *ipu_clk;
> +static struct device *ipu_dev;
> +
> +static DEFINE_SPINLOCK(ipu_lock);
> +static DEFINE_MUTEX(ipu_channel_lock);
> +void __iomem *ipu_cm_reg;
> +void __iomem *ipu_idmac_reg;
> +
> +static int ipu_use_count;
> +
> +static struct ipu_channel channels[64];

Keeping these global limits you to just one ipu, and makes
understanding the code a little harder. How about moving
these variables into a struct ipu_data (or similar) that
is referenced from the platform_device?

> +EXPORT_SYMBOL(ipu_idmac_put);

Why not EXPORT_SYMBOL_GPL?

> +static LIST_HEAD(ipu_irq_handlers);
> +
> +static void ipu_irq_update_irq_mask(void)
> +{
> +	struct ipu_irq_handler *handler;
> +	int i;
> +
> +	DECLARE_IPU_IRQ_BITMAP(irqs);
> +
> +	bitmap_zero(irqs, IPU_IRQ_COUNT);
> +
> +	list_for_each_entry(handler, &ipu_irq_handlers, list)
> +		bitmap_or(irqs, irqs, handler->ipu_irqs, IPU_IRQ_COUNT);
> +
> +	for (i = 0; i < BITS_TO_LONGS(IPU_IRQ_COUNT); i++)
> +		ipu_cm_write(irqs[i], IPU_INT_CTRL(i + 1));
> +}
> +
> +int ipu_irq_add_handler(struct ipu_irq_handler *ipuirq)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&ipu_lock, flags);
> +
> +	list_add_tail(&ipuirq->list, &ipu_irq_handlers);
> +	ipu_irq_update_irq_mask();
> +
> +	spin_unlock_irqrestore(&ipu_lock, flags);
> +	return 0;
> +}
> +EXPORT_SYMBOL(ipu_irq_add_handler);

The interrupt logic needs some comments. What are you trying to achieve here?

> +int ipu_wait_for_interrupt(int interrupt, int timeout_ms)
> +{
> +	struct ipu_irq_handler handler;
> +	DECLARE_COMPLETION_ONSTACK(completion);
> +	int ret;
> +
> +	bitmap_zero(handler.ipu_irqs, IPU_IRQ_COUNT);
> +	bitmap_set(handler.ipu_irqs, interrupt, 1);
> +
> +	ipu_cm_write(1 << (interrupt % 32), IPU_INT_STAT(interrupt / 32 + 1));
> +
> +	handler.handler = ipu_completion_handler;
> +	handler.context = &completion;
> +	ipu_irq_add_handler(&handler);
> +
> +	ret = wait_for_completion_timeout(&completion,
> +			msecs_to_jiffies(timeout_ms));
> +
> +	ipu_irq_remove_handler(&handler);
> +
> +	if (ret > 0)
> +		ret = 0;
> +	else
> +		ret = -ETIMEDOUT;
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(ipu_wait_for_interrupt);

If I understand this correctly, this is a very complicated way to implement
something that could be done with a single wait_event_timeout() and
wake_up_interruptible_all() from the irq handler.

> +static irqreturn_t ipu_irq_handler(int irq, void *desc)
> +{
> +	DECLARE_IPU_IRQ_BITMAP(status);
> +	struct ipu_irq_handler *handler;
> +	int i;
> +
> +	for (i = 0; i < BITS_TO_LONGS(IPU_IRQ_COUNT); i++) {
> +		status[i] = ipu_cm_read(IPU_INT_STAT(i + 1));
> +		ipu_cm_write(status[i], IPU_INT_STAT(i + 1));
> +	}
> +
> +	list_for_each_entry(handler, &ipu_irq_handlers, list) {
> +		DECLARE_IPU_IRQ_BITMAP(tmp);
> +		if (bitmap_and(tmp, status, handler->ipu_irqs, IPU_IRQ_COUNT))
> +			handler->handler(tmp, handler->context);
> +	}
> +
> +	return IRQ_HANDLED;
> +}

This needs to take spin_lock_irqsave before walking the ipu_irq_handlers
list, in order to prevent another CPU from modifying the list
while you are in the handler.

> +static int ipu_reset(void)
> +{
> +	int timeout = 10000;
> +	u32 val;
> +
> +	/* hard reset the IPU */
> +	val = readl(MX51_IO_ADDRESS(MX51_SRC_BASE_ADDR));
> +	val |= 1 << 3;
> +	writel(val, MX51_IO_ADDRESS(MX51_SRC_BASE_ADDR));
> +
> +	ipu_cm_write(0x807FFFFF, IPU_MEM_RST);
> +
> +	while (ipu_cm_read(IPU_MEM_RST) & 0x80000000) {
> +		if (!timeout--)
> +			return -ETIME;
> +		udelay(100);
> +	}

You have a timeout of over one second with udelay, which
is not acceptable on many systems. AFAICT, the function 
can sleep, so you can replace udelay with msleep(1), and
you should use a better logic to determine the end of the
loop:

	unsigned long timeout = jiffies + msecs_to_jiffies(1000);

	while (ipu_cm_read(IPU_MEM_RST) & 0x80000000) {
		if (time_after(jiffies, timeout))
			return -ETIME;
		msleep(1);
	}

> +static u32 *ipu_cpmem_base;
> +static struct device *ipu_dev;
> +
> +struct ipu_ch_param_word {
> +	u32 data[5];
> +	u32 res[3];
> +};
> +
> +struct ipu_ch_param {
> +	struct ipu_ch_param_word word[2];
> +};

Same comment as for the previous file
> +
> +static void __iomem *ipu_dc_reg;
> +static void __iomem *ipu_dc_tmpl_reg;
> +static struct device *ipu_dev;
> +
> +struct ipu_dc {
> +	unsigned int di; /* The display interface number assigned to this dc channel */
> +	unsigned int channel_offset;
> +};
> +
> +static struct ipu_dc dc_channels[10];

And here again.

> +static void ipu_dc_link_event(int chan, int event, int addr, int priority)
> +{
> +	u32 reg;
> +
> +	reg = __raw_readl(DC_RL_CH(chan, event));
> +	reg &= ~(0xFFFF << (16 * (event & 0x1)));
> +	reg |= ((addr << 8) | priority) << (16 * (event & 0x1));
> +	__raw_writel(reg, DC_RL_CH(chan, event));
> +}

Better use readl/writel instead of __raw_readl/__raw_writel, throughout the
code.

> +int ipu_di_init(struct platform_device *pdev, int id, unsigned long base,
> +		u32 module, struct clk *ipu_clk);
> +void ipu_di_exit(struct platform_device *pdev, int id);
> +
> +int ipu_dmfc_init(struct platform_device *pdev, unsigned long base,
> +		struct clk *ipu_clk);
> +void ipu_dmfc_exit(struct platform_device *pdev);
> +
> +int ipu_dp_init(struct platform_device *pdev, unsigned long base);
> +void ipu_dp_exit(struct platform_device *pdev);
> +
> +int ipu_dc_init(struct platform_device *pdev, unsigned long base,
> +		unsigned long template_base);
> +void ipu_dc_exit(struct platform_device *pdev);
> +
> +int ipu_cpmem_init(struct platform_device *pdev, unsigned long base);
> +void ipu_cpmem_exit(struct platform_device *pdev);

If you make the main driver an mfd device, the sub-drivers could become
real platform drivers, which can structure the layering in a more modular
way.
E.g. instead of a single module init function, each subdriver can be
a module by itself and look at the resources associated with the
platform device it matches.

	Arnd

^ permalink raw reply

* Re: [PATCH 3/8] Add a mfd IPUv3 driver
From: Thomas Gleixner @ 2011-02-28 18:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1298887229-7987-4-git-send-email-s.hauer@pengutronix.de>

On Mon, 28 Feb 2011, Sascha Hauer wrote:
> +
> +static int ipu_use_count;
> +
> +static struct ipu_channel channels[64];
> +
> +struct ipu_channel *ipu_idmac_get(unsigned num)
> +{
> +	struct ipu_channel *channel;
> +
> +	dev_dbg(ipu_dev, "%s %d\n", __func__, num);
> +
> +	if (num > 63)

  >= ARRAY_SIZE(channels) or a sensible define please

> +		return ERR_PTR(-ENODEV);
> +
> +	mutex_lock(&ipu_channel_lock);
> +
> +	channel = &channels[num];
> +
> +	if (channel->busy) {
> +		channel = ERR_PTR(-EBUSY);
> +		goto out;
> +	}
> +
> +	channel->busy = 1;
> +	channel->num = num;
> +
> +out:
> +	mutex_unlock(&ipu_channel_lock);
> +
> +	return channel;
> +}
> +EXPORT_SYMBOL(ipu_idmac_get);

EXPORT_SYMBOL_GPL all over the place

> +void ipu_idmac_put(struct ipu_channel *channel)
> +{
> +	dev_dbg(ipu_dev, "%s %d\n", __func__, channel->num);

Do we really need this debug stuff in all these functions ?

> +	mutex_lock(&ipu_channel_lock);
> +
> +	channel->busy = 0;
> +
> +	mutex_unlock(&ipu_channel_lock);
> +}
> +EXPORT_SYMBOL(ipu_idmac_put);
> +

Also exported functions want a proper kerneldoc comment.

> +void ipu_idmac_set_double_buffer(struct ipu_channel *channel, bool doublebuffer)

> +static LIST_HEAD(ipu_irq_handlers);
> +
> +static void ipu_irq_update_irq_mask(void)
> +{
> +	struct ipu_irq_handler *handler;
> +	int i;
> +
> +	DECLARE_IPU_IRQ_BITMAP(irqs);

Why the hell do we need this? It's a bog standard bitmap, right ?

> +	bitmap_zero(irqs, IPU_IRQ_COUNT);
> +
> +	list_for_each_entry(handler, &ipu_irq_handlers, list)
> +		bitmap_or(irqs, irqs, handler->ipu_irqs, IPU_IRQ_COUNT);
> +
> +	for (i = 0; i < BITS_TO_LONGS(IPU_IRQ_COUNT); i++)
> +		ipu_cm_write(irqs[i], IPU_INT_CTRL(i + 1));
> +}

> +static void ipu_completion_handler(unsigned long *bitmask, void *context)
> +{
> +	struct completion *completion = context;
> +
> +	complete(completion);
> +}
> +
> +int ipu_wait_for_interrupt(int interrupt, int timeout_ms)
> +{
> +	struct ipu_irq_handler handler;
> +	DECLARE_COMPLETION_ONSTACK(completion);
> +	int ret;
> +
> +	bitmap_zero(handler.ipu_irqs, IPU_IRQ_COUNT);
> +	bitmap_set(handler.ipu_irqs, interrupt, 1);
> +
> +	ipu_cm_write(1 << (interrupt % 32), IPU_INT_STAT(interrupt / 32 + 1));
> +
> +	handler.handler = ipu_completion_handler;
> +	handler.context = &completion;
> +	ipu_irq_add_handler(&handler);
> +
> +	ret = wait_for_completion_timeout(&completion,
> +			msecs_to_jiffies(timeout_ms));
> +
> +	ipu_irq_remove_handler(&handler);
> +
> +	if (ret > 0)
> +		ret = 0;
> +	else
> +		ret = -ETIMEDOUT;
> +
> +	return ret;

  return ret > 0 ? 0 : -ETIMEDOUT;

  perhaps ?


> +}
> +EXPORT_SYMBOL(ipu_wait_for_interrupt);
> +
> +static irqreturn_t ipu_irq_handler(int irq, void *desc)
> +{
> +	DECLARE_IPU_IRQ_BITMAP(status);
> +	struct ipu_irq_handler *handler;
> +	int i;
> +
> +	for (i = 0; i < BITS_TO_LONGS(IPU_IRQ_COUNT); i++) {
> +		status[i] = ipu_cm_read(IPU_INT_STAT(i + 1));
> +		ipu_cm_write(status[i], IPU_INT_STAT(i + 1));
> +	}
> +
> +	list_for_each_entry(handler, &ipu_irq_handlers, list) {
> +		DECLARE_IPU_IRQ_BITMAP(tmp);
> +		if (bitmap_and(tmp, status, handler->ipu_irqs, IPU_IRQ_COUNT))
> +			handler->handler(tmp, handler->context);
> +	}

And what protects the list walk? Just the fact that this is a UP
machine?

> +void ipu_put(void)
> +{
> +	mutex_lock(&ipu_channel_lock);
> +
> +	ipu_use_count--;
> +
> +	if (ipu_use_count = 0)
> +		clk_disable(ipu_clk);
> +
> +	if (ipu_use_count < 0) {
> +		dev_err(ipu_dev, "ipu use count < 0\n");

This wants to be a WARN_ON(ipu_use_count < 0) so you get some
information which code is calling this.

> +		ipu_use_count = 0;
> +	}
> +
> +	mutex_unlock(&ipu_channel_lock);
> +}

> +static int __devinit ipu_probe(struct platform_device *pdev)
> +{
> +	struct resource *res;
> +	unsigned long ipu_base;
> +	int ret, irq1, irq2;
> +
> +	/* There can be only one */
> +	if (ipu_dev)
> +		return -EBUSY;
> +
> +	spin_lock_init(&ipu_lock);
> +
> +	ipu_dev = &pdev->dev;
> +
> +	irq1 = platform_get_irq(pdev, 0);
> +	irq2 = platform_get_irq(pdev, 1);
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> +	if (!res || irq1 < 0 || irq2 < 0)
> +		return -ENODEV;
> +
> +	ipu_base = res->start;
> +
> +	ipu_cm_reg = ioremap(ipu_base + IPU_CM_REG_BASE, PAGE_SIZE);
> +	if (!ipu_cm_reg) {
> +		ret = -ENOMEM;
> +		goto failed_ioremap1;
> +	}
> +
> +	ipu_idmac_reg = ioremap(ipu_base + IPU_IDMAC_REG_BASE, PAGE_SIZE);
> +	if (!ipu_idmac_reg) {
> +		ret = -ENOMEM;
> +		goto failed_ioremap2;
> +	}
> +
> +	ipu_clk = clk_get(&pdev->dev, "ipu");
> +	if (IS_ERR(ipu_clk)) {
> +		ret = PTR_ERR(ipu_clk);
> +		dev_err(&pdev->dev, "clk_get failed with %d", ret);
> +		goto failed_clk_get;
> +	}
> +
> +	ipu_get();
> +
> +	ret = request_irq(irq1, ipu_irq_handler, IRQF_DISABLED, pdev->name,
> +			&pdev->dev);

s/IRQF_DISABLED/0/ We run all handlers with interrupts disabled
nowadays.

> +	ret = ipu_submodules_init(pdev, ipu_base, ipu_clk);
> +	if (ret)
> +		goto failed_submodules_init;
> +
> +	/* Set sync refresh channels as high priority */
> +	ipu_idmac_write(0x18800000, IDMAC_CHA_PRI(0));

Hmm, this random prio setting here is odd.

> +	ret = ipu_add_client_devices(pdev);
> +        if (ret) {
> +                dev_err(&pdev->dev, "adding client devices failed with %d\n", ret);
> +		goto failed_add_clients;
> +        }

White space damage.

> +	ret = ipu_wait_for_interrupt(irq, 50);
> +	if (ret)
> +		return;
> +
> +	/* Wait for DC triple buffer to empty */
> +	if (dc_channels[dc_chan].di = 0)
> +		while ((__raw_readl(DC_STAT) & 0x00000002)
> +			!= 0x00000002) {
> +			msleep(2);
> +			timeout -= 2;
> +			if (timeout <= 0)
> +				break;

So we poll stuff which is updated from some other function ?

> +		}
> +	else if (dc_channels[dc_chan].di = 1)
> +		while ((__raw_readl(DC_STAT) & 0x00000020)
> +			!= 0x00000020) {
> +			msleep(2);
> +			timeout -= 2;
> +			if (timeout <= 0)
> +				break;

Ditto

> +	}

Thanks,

	tglx

^ permalink raw reply

* Re: [PATCH 1/2] fbdev: sh_mobile_lcdc: Add YUV input support
From: Damian @ 2011-03-01  3:13 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1298456210-26519-2-git-send-email-dhobsong@igel.co.jp>

Hi Geert,
On 2011/02/24 15:05, Geert Uytterhoeven wrote:
>>>
>>> My thinking behind separating this out was that I wanted this
>>> define to be accessible from user space.  The reason is so that
>>> an application can test the value of .nonstd against the flag to
>>> know for sure if it is dealing with a YUV framebuffer or not.
>>
>> Hm, but ideally we want something standard. How do you know the nonstd
>> flag is working as you expect from user space? All of a sudden you
>> have code that depends on what type of fbdev driver you are using.
>> This is ugly, but abstracting the nonstd interface does not make it
>> better IMO.
>>
>> The nonstd thing is a hack, but for now it is close enough. V4L2 has
>> standard NV12/NV16 support already, but I don't think there is any
>> such thing for fbdev. So i see your patches as a stop-gap, but I
>> really don't want to make it more complicated than it has to be. So
>> exporting nonstd values in a header file to user space seems too
>> complicated IMO.
>>
>> Please just live with the fact that nonstd is special for now. We need
>> to rework the entire LCDC/HDMI/DSI area to support multiple planes and
>> better PM anyway. Perhaps KMS is the way forward, or perhaps Media
>> Controller? Maybe both?
>>
>>> I was under the impression that only headers under include/linux/ should be
>>> accessed from user space, but to be honest, I'm not sure about that.
>>> If that is in fact not the case, then I totally agree that it can go
>>> into include/video/sh_mobile_lcdc.h.
>>
>> Please ditch the SH_FB_YUV constant all together. No need to build
>> some abstraction on top of a hackish interface. Just check if nonstd
>> is non-zero in the driver and assume that means YUV for now. That's
>> good enough.
>
> For YUV (do you mean YCbCr?), I'm inclined to suggest adding a new FB_VISUAL_*
> type instead, which indicates the fb_var_screeninfo.{red,green,blue} fields are
> YCbCr instead of RGB.
> Depending on the frame buffer organization, you also need new FB_TYPE_*/FB_AUX_*
> types.
I just wanted to clarify here.  Is your comment about these new flags in 
specific reference to this patch or to Magnus' "going forward" comment? 
  It seems like the beginnings of a method to standardize YCbCr support 
in fbdev across all platforms.
Also, do I understand correctly that FB_VISUAL_ would specify the 
colorspace (RGB, YCbCr), FB_TYPE_* would be a format specifier (i.e. 
planar, semiplanar, interleaved, etc)?  I'm not really sure what you are 
referring to with the FB_AUX_* however.
>
> Gr{oetje,eeting}s,
>
>                          Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                  -- Linus Torvalds
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Thanks very much,
Damian

^ permalink raw reply


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