* Re: [PATCH] fb: avoid possible deadlock caused by fb_set_suspend
From: Bruno Prémont @ 2011-06-18 9:19 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: Florian Tobias Schandinat, lethal, linux-fbdev, francis.moro,
torvalds, linux-kernel, Herton Ronaldo Krzesinski, stable
In-Reply-To: <20110618104311.6d80ba50@neptune.home>
Guennadi, could you have a look at (completely untested) patch which avoids
possible deadlock due to inverted lock taking order on hotplug as well
as "readding" lock_fb_info() for fb_set_suspend() call after Herton's
patch to fb_set_suspend().
Thanks,
Bruno
On Sat, 18 June 2011 Bruno Prémont <bonbons@linux-vserver.org> wrote:
> On Fri, 17 June 2011 Florian Tobias Schandinat <FlorianSchandinat@gmx.de> wrote:
> > From: Herton Ronaldo Krzesinski <herton@mandriva.com.br>
> >
> > A lock ordering issue can cause deadlocks: in framebuffer/console code,
> > all needed struct fb_info locks are taken before acquire_console_sem(),
> > in places which need to take console semaphore.
> >
> > But fb_set_suspend is always called with console semaphore held, and
> > inside it we call lock_fb_info which gets the fb_info lock, inverse
> > locking order of what the rest of the code does. This causes a real
> > deadlock issue, when we write to state fb sysfs attribute (which calls
> > fb_set_suspend) while a framebuffer is being unregistered by
> > remove_conflicting_framebuffers, as can be shown by following show
> > blocked state trace on a test program which loads i915 and runs another
> > forked processes writing to state attribute:
> >
> > Test process with semaphore held and trying to get fb_info lock:
>
> ...
>
> > fb-test2 which reproduces above is available on kernel.org bug #26232.
> > To solve this issue, avoid calling lock_fb_info inside fb_set_suspend,
> > and move it out to where needed (callers of fb_set_suspend must call
> > lock_fb_info before if needed). So far, the only place which needs to
> > call lock_fb_info is store_fbstate, all other places which calls
> > fb_set_suspend are suspend/resume hooks that should not need the lock as
> > they should be run only when processes are already frozen in
> > suspend/resume.
>
> From a quick look through FB drivers in 2.6.39 I've found one that would need
> more work:
> - drivers/video/sh_mobile_hdmi.c: sh_hdmi_edid_work_fn()
> Should get changed to
> a) right locking order in case (hdmi->hp_state = HDMI_HOTPLUG_CONNECTED)
> b) lock fb_info in the other case
> For this one fb_set_suspend() does get call in a hotplug worker,
> thus independently on suspend/resume process.
>
> The rest does match the suspend/resume hook pattern mentioned.
>
> Bruno
>
>
> > References: https://bugzilla.kernel.org/show_bug.cgi?id&232
> > Signed-off-by: Herton Ronaldo Krzesinski <herton@mandriva.com.br>
> > Signed-off-by: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
> > Cc: stable@kernel.org
> > ---
> > drivers/video/fbmem.c | 3 ---
> > drivers/video/fbsysfs.c | 3 +++
> > 2 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> > index 5aac00e..ad93629 100644
> > --- a/drivers/video/fbmem.c
> > +++ b/drivers/video/fbmem.c
> > @@ -1738,8 +1738,6 @@ void fb_set_suspend(struct fb_info *info, int state)
> > {
> > struct fb_event event;
> >
> > - if (!lock_fb_info(info))
> > - return;
> > event.info = info;
> > if (state) {
> > fb_notifier_call_chain(FB_EVENT_SUSPEND, &event);
> > @@ -1748,7 +1746,6 @@ void fb_set_suspend(struct fb_info *info, int state)
> > info->state = FBINFO_STATE_RUNNING;
> > fb_notifier_call_chain(FB_EVENT_RESUME, &event);
> > }
> > - unlock_fb_info(info);
> > }
> >
> > /**
> > diff --git a/drivers/video/fbsysfs.c b/drivers/video/fbsysfs.c
> > index 04251ce..67afa9c 100644
> > --- a/drivers/video/fbsysfs.c
> > +++ b/drivers/video/fbsysfs.c
> > @@ -399,9 +399,12 @@ static ssize_t store_fbstate(struct device *device,
> >
> > state = simple_strtoul(buf, &last, 0);
> >
> > + if (!lock_fb_info(fb_info))
> > + return -ENODEV;
> > console_lock();
> > fb_set_suspend(fb_info, (int)state);
> > console_unlock();
> > + unlock_fb_info(fb_info);
> >
> > return count;
> > }
diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c
index 2b9e56a..b1a13ab 100644
--- a/drivers/video/sh_mobile_hdmi.c
+++ b/drivers/video/sh_mobile_hdmi.c
@@ -1151,27 +1151,27 @@ static void sh_hdmi_edid_work_fn(struct work_struct *work)
ch = info->par;
- console_lock();
+ if (lock_fb_info(info)) {
+ console_lock();
- /* HDMI plug in */
- if (!sh_hdmi_must_reconfigure(hdmi) &&
- info->state = FBINFO_STATE_RUNNING) {
- /*
- * First activation with the default monitor - just turn
- * on, if we run a resume here, the logo disappears
- */
- if (lock_fb_info(info)) {
+ /* HDMI plug in */
+ if (!sh_hdmi_must_reconfigure(hdmi) &&
+ info->state = FBINFO_STATE_RUNNING) {
+ /*
+ * First activation with the default monitor - just turn
+ * on, if we run a resume here, the logo disappears
+ */
info->var.width = hdmi->var.width;
info->var.height = hdmi->var.height;
sh_hdmi_display_on(hdmi, info);
- unlock_fb_info(info);
+ } else {
+ /* New monitor or have to wake up */
+ fb_set_suspend(info, 0);
}
- } else {
- /* New monitor or have to wake up */
- fb_set_suspend(info, 0);
- }
- console_unlock();
+ console_unlock();
+ unlock_fb_info(info);
+ }
} else {
ret = 0;
if (!hdmi->info)
@@ -1181,12 +1181,15 @@ static void sh_hdmi_edid_work_fn(struct work_struct *work)
fb_destroy_modedb(hdmi->monspec.modedb);
hdmi->monspec.modedb = NULL;
- console_lock();
+ if (lock_fb_info(info)) {
+ console_lock();
- /* HDMI disconnect */
- fb_set_suspend(hdmi->info, 1);
+ /* HDMI disconnect */
+ fb_set_suspend(hdmi->info, 1);
- console_unlock();
+ console_unlock();
+ unlock_fb_info(info);
+ }
pm_runtime_put(hdmi->dev);
}
^ permalink raw reply related
* Re: [PATCH] fb: avoid possible deadlock caused by fb_set_suspend
From: Bruno Prémont @ 2011-06-18 8:43 UTC (permalink / raw)
To: Florian Tobias Schandinat
Cc: lethal, linux-fbdev, francis.moro, torvalds, linux-kernel,
Herton Ronaldo Krzesinski, stable
In-Reply-To: <1308337359-3480-1-git-send-email-FlorianSchandinat@gmx.de>
On Fri, 17 June 2011 Florian Tobias Schandinat <FlorianSchandinat@gmx.de> wrote:
> From: Herton Ronaldo Krzesinski <herton@mandriva.com.br>
>
> A lock ordering issue can cause deadlocks: in framebuffer/console code,
> all needed struct fb_info locks are taken before acquire_console_sem(),
> in places which need to take console semaphore.
>
> But fb_set_suspend is always called with console semaphore held, and
> inside it we call lock_fb_info which gets the fb_info lock, inverse
> locking order of what the rest of the code does. This causes a real
> deadlock issue, when we write to state fb sysfs attribute (which calls
> fb_set_suspend) while a framebuffer is being unregistered by
> remove_conflicting_framebuffers, as can be shown by following show
> blocked state trace on a test program which loads i915 and runs another
> forked processes writing to state attribute:
>
> Test process with semaphore held and trying to get fb_info lock:
...
> fb-test2 which reproduces above is available on kernel.org bug #26232.
> To solve this issue, avoid calling lock_fb_info inside fb_set_suspend,
> and move it out to where needed (callers of fb_set_suspend must call
> lock_fb_info before if needed). So far, the only place which needs to
> call lock_fb_info is store_fbstate, all other places which calls
> fb_set_suspend are suspend/resume hooks that should not need the lock as
> they should be run only when processes are already frozen in
> suspend/resume.
From a quick look through FB drivers in 2.6.39 I've found one that would need
more work:
- drivers/video/sh_mobile_hdmi.c: sh_hdmi_edid_work_fn()
Should get changed to
a) right locking order in case (hdmi->hp_state = HDMI_HOTPLUG_CONNECTED)
b) lock fb_info in the other case
For this one fb_set_suspend() does get call in a hotplug worker,
thus independently on suspend/resume process.
The rest does match the suspend/resume hook pattern mentioned.
Bruno
> References: https://bugzilla.kernel.org/show_bug.cgi?id&232
> Signed-off-by: Herton Ronaldo Krzesinski <herton@mandriva.com.br>
> Signed-off-by: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
> Cc: stable@kernel.org
> ---
> drivers/video/fbmem.c | 3 ---
> drivers/video/fbsysfs.c | 3 +++
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> index 5aac00e..ad93629 100644
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -1738,8 +1738,6 @@ void fb_set_suspend(struct fb_info *info, int state)
> {
> struct fb_event event;
>
> - if (!lock_fb_info(info))
> - return;
> event.info = info;
> if (state) {
> fb_notifier_call_chain(FB_EVENT_SUSPEND, &event);
> @@ -1748,7 +1746,6 @@ void fb_set_suspend(struct fb_info *info, int state)
> info->state = FBINFO_STATE_RUNNING;
> fb_notifier_call_chain(FB_EVENT_RESUME, &event);
> }
> - unlock_fb_info(info);
> }
>
> /**
> diff --git a/drivers/video/fbsysfs.c b/drivers/video/fbsysfs.c
> index 04251ce..67afa9c 100644
> --- a/drivers/video/fbsysfs.c
> +++ b/drivers/video/fbsysfs.c
> @@ -399,9 +399,12 @@ static ssize_t store_fbstate(struct device *device,
>
> state = simple_strtoul(buf, &last, 0);
>
> + if (!lock_fb_info(fb_info))
> + return -ENODEV;
> console_lock();
> fb_set_suspend(fb_info, (int)state);
> console_unlock();
> + unlock_fb_info(fb_info);
>
> return count;
> }
^ permalink raw reply
* Re: Framebuffer driver for controller with indirect addressing?
From: Bernie Thompson @ 2011-06-18 0:18 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <0LlUqt-1Pye1R36EZ-00bCDF@mrelay.perfora.net>
On Fri, Jun 17, 2011 at 4:10 PM, Steve Strobel
<steve.strobel@link-comm.com> wrote:
> It does not have address lines, so the framebuffer memory
> is not directly accessible
>
> If someone could point me to a driver that works in a similar way, it would
> help me out a lot. Â Thanks.
Hi Steve - there are several drivers that handle this kind of case,
including the udlfb driver for DisplayLink USB graphics chips (in the
kernel tree /drivers/video/udlfb.c)
The main unsolved challenge is fbdev (in the memory mapped framebuffer
case) assumes writes to the framebuffer take effect immediately. For
hardware in which this isn't true (e.g. USB, eink controllers, etc.),
there are two common solutions. However both have problems:
1) Use of /driver/video/fb_defio.c to use the MMU to trigger fault
handlers on framebuffer writes, and transmit to hardware via delayed
processing of the dirty pages. However, this implementation still
appears to have some race conditions which can result in difficult to
debug problems. It's also a bit of MMU/cache load.
2) Use of an ioctl to allow the user mode client to inform the
framebuffer of changed pixels (usually xorg forwarding X DAMAGE
notifications, which are well matched for this purpose). This works
well and is simple, however we don't yet have a standardized ioctl, so
several drivers have different ioctls and thus custom x servers,
instead of being supported by the standard xf86-video-fbdev driver.
#2 would be the quickest way to give Linux a clean way of handling
remote (non-directly-addressable) framebuffers.
Others may have some additional suggestions.
Hope that helps. Best wishes,
Bernie
http://plugable.com/
^ permalink raw reply
* Framebuffer driver for controller with indirect addressing?
From: Steve Strobel @ 2011-06-17 23:10 UTC (permalink / raw)
To: linux-fbdev
I am trying to support a Tianma TM023KDH18 QVGA LCD display (1) via a
16-bit parallel bus interface on a BF537 Blackfin running the
distribution from blackfin.uclinux.org. Built into the display is a
ILITEK ILI9342 controller (2). The controller has built-in memory
for the framebuffer and handles refreshing the display
automatically. Changing the contents of the framebuffer is done by
sending commands to set the starting pixel position, then writing
repeatedly to the same address with the data for each successive
pixel. It does not have address lines, so the framebuffer memory is
not directly accessible as with the S1D13XXX chips. A single D/CX
pin, which we plan to treat like an address line, controls whether
each bus read/write operation is treated as a command or data.
I am wondering if there is an existing driver for something that
works in a similar way that I could use as a model. I think I am
going to need to create a shadow framebuffer in system memory (maybe
using the virtual framebuffer) and DMA it to the graphics controller
whenever the displayed information changes (or maybe on the
double-buffer flip). Ultimately I would like to use Qt, so building
on the existing framebuffer API seems like a good idea.
If someone could point me to a driver that works in a similar way, it
would help me out a lot. Thanks.
Steve
(1) Datasheet at
http://www.tianma-usa.com/web/uploads/spec/0906123532_TM023KDH18%20V1.0.pdf
(2) Datasheet at http://linkcomm.com/temp/ILI9342.pdf
---
Steve Strobel
Link Communications, Inc.
1035 Cerise Rd
Billings, MT 59101-7378
(406) 245-5002 ext 102
(406) 245-4889 (fax)
WWW: http://www.link-comm.com
MailTo:steve.strobel@link-comm.com
^ permalink raw reply
* [PATCH] fb: avoid possible deadlock caused by fb_set_suspend
From: Florian Tobias Schandinat @ 2011-06-17 18:46 UTC (permalink / raw)
To: lethal, linux-fbdev
Cc: francis.moro, torvalds, bonbons, linux-kernel,
Herton Ronaldo Krzesinski, Florian Tobias Schandinat, stable
In-Reply-To: <4DF7B0B4.4060002@gmx.de>
From: Herton Ronaldo Krzesinski <herton@mandriva.com.br>
A lock ordering issue can cause deadlocks: in framebuffer/console code,
all needed struct fb_info locks are taken before acquire_console_sem(),
in places which need to take console semaphore.
But fb_set_suspend is always called with console semaphore held, and
inside it we call lock_fb_info which gets the fb_info lock, inverse
locking order of what the rest of the code does. This causes a real
deadlock issue, when we write to state fb sysfs attribute (which calls
fb_set_suspend) while a framebuffer is being unregistered by
remove_conflicting_framebuffers, as can be shown by following show
blocked state trace on a test program which loads i915 and runs another
forked processes writing to state attribute:
Test process with semaphore held and trying to get fb_info lock:
..
fb-test2 D 0000000000000000 0 237 228 0x00000000
ffff8800774f3d68 0000000000000082 00000000000135c0 00000000000135c0
ffff880000000000 ffff8800774f3fd8 ffff8800774f3fd8 ffff880076ee4530
00000000000135c0 ffff8800774f3fd8 ffff8800774f2000 00000000000135c0
Call Trace:
[<ffffffff8141287a>] __mutex_lock_slowpath+0x11a/0x1e0
[<ffffffff814142f2>] ? _raw_spin_lock_irq+0x22/0x40
[<ffffffff814123d3>] mutex_lock+0x23/0x50
[<ffffffff8125dfc5>] lock_fb_info+0x25/0x60
[<ffffffff8125e3f0>] fb_set_suspend+0x20/0x80
[<ffffffff81263e2f>] store_fbstate+0x4f/0x70
[<ffffffff812e7f70>] dev_attr_store+0x20/0x30
[<ffffffff811c46b4>] sysfs_write_file+0xd4/0x160
[<ffffffff81155a26>] vfs_write+0xc6/0x190
[<ffffffff81155d51>] sys_write+0x51/0x90
[<ffffffff8100c012>] system_call_fastpath+0x16/0x1b
..
modprobe process stalled because has the fb_info lock (got inside
unregister_framebuffer) but waiting for the semaphore held by the
test process which is waiting to get the fb_info lock:
..
modprobe D 0000000000000000 0 230 218 0x00000000
ffff880077a4d618 0000000000000082 0000000000000001 0000000000000001
ffff880000000000 ffff880077a4dfd8 ffff880077a4dfd8 ffff8800775a2e20
00000000000135c0 ffff880077a4dfd8 ffff880077a4c000 00000000000135c0
Call Trace:
[<ffffffff81411fe5>] schedule_timeout+0x215/0x310
[<ffffffff81058051>] ? get_parent_ip+0x11/0x50
[<ffffffff814130dd>] __down+0x6d/0xb0
[<ffffffff81089f71>] down+0x41/0x50
[<ffffffff810629ac>] acquire_console_sem+0x2c/0x50
[<ffffffff812ca53d>] unbind_con_driver+0xad/0x2d0
[<ffffffff8126f5f7>] fbcon_event_notify+0x457/0x890
[<ffffffff814144ff>] ? _raw_spin_unlock_irqrestore+0x1f/0x50
[<ffffffff81058051>] ? get_parent_ip+0x11/0x50
[<ffffffff8141836d>] notifier_call_chain+0x4d/0x70
[<ffffffff8108a3b8>] __blocking_notifier_call_chain+0x58/0x80
[<ffffffff8108a3f6>] blocking_notifier_call_chain+0x16/0x20
[<ffffffff8125dabb>] fb_notifier_call_chain+0x1b/0x20
[<ffffffff8125e6ac>] unregister_framebuffer+0x7c/0x130
[<ffffffff8125e8b3>] remove_conflicting_framebuffers+0x153/0x180
[<ffffffff8125eef3>] register_framebuffer+0x93/0x2c0
[<ffffffffa0331112>] drm_fb_helper_single_fb_probe+0x252/0x2f0 [drm_kms_helper]
[<ffffffffa03314a3>] drm_fb_helper_initial_config+0x2f3/0x6d0 [drm_kms_helper]
[<ffffffffa03318dd>] ? drm_fb_helper_single_add_all_connectors+0x5d/0x1c0 [drm_kms_helper]
[<ffffffffa037b588>] intel_fbdev_init+0xa8/0x160 [i915]
[<ffffffffa0343d74>] i915_driver_load+0x854/0x12b0 [i915]
[<ffffffffa02f0e7e>] drm_get_pci_dev+0x19e/0x360 [drm]
[<ffffffff8141821d>] ? sub_preempt_count+0x9d/0xd0
[<ffffffffa0386f91>] i915_pci_probe+0x15/0x17 [i915]
[<ffffffff8124481f>] local_pci_probe+0x5f/0xd0
[<ffffffff81244f89>] pci_device_probe+0x119/0x120
[<ffffffff812eccaa>] ? driver_sysfs_add+0x7a/0xb0
[<ffffffff812ed003>] driver_probe_device+0xa3/0x290
[<ffffffff812ed1f0>] ? __driver_attach+0x0/0xb0
[<ffffffff812ed29b>] __driver_attach+0xab/0xb0
[<ffffffff812ed1f0>] ? __driver_attach+0x0/0xb0
[<ffffffff812ebd3e>] bus_for_each_dev+0x5e/0x90
[<ffffffff812ecc2e>] driver_attach+0x1e/0x20
[<ffffffff812ec6f2>] bus_add_driver+0xe2/0x320
[<ffffffffa03aa000>] ? i915_init+0x0/0x96 [i915]
[<ffffffff812ed536>] driver_register+0x76/0x140
[<ffffffffa03aa000>] ? i915_init+0x0/0x96 [i915]
[<ffffffff81245216>] __pci_register_driver+0x56/0xd0
[<ffffffffa02f1264>] drm_pci_init+0xe4/0xf0 [drm]
[<ffffffffa03aa000>] ? i915_init+0x0/0x96 [i915]
[<ffffffffa02e84a8>] drm_init+0x58/0x70 [drm]
[<ffffffffa03aa094>] i915_init+0x94/0x96 [i915]
[<ffffffff81002194>] do_one_initcall+0x44/0x190
[<ffffffff810a066b>] sys_init_module+0xcb/0x210
[<ffffffff8100c012>] system_call_fastpath+0x16/0x1b
..
fb-test2 which reproduces above is available on kernel.org bug #26232.
To solve this issue, avoid calling lock_fb_info inside fb_set_suspend,
and move it out to where needed (callers of fb_set_suspend must call
lock_fb_info before if needed). So far, the only place which needs to
call lock_fb_info is store_fbstate, all other places which calls
fb_set_suspend are suspend/resume hooks that should not need the lock as
they should be run only when processes are already frozen in
suspend/resume.
References: https://bugzilla.kernel.org/show_bug.cgi?id&232
Signed-off-by: Herton Ronaldo Krzesinski <herton@mandriva.com.br>
Signed-off-by: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: stable@kernel.org
---
drivers/video/fbmem.c | 3 ---
drivers/video/fbsysfs.c | 3 +++
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 5aac00e..ad93629 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1738,8 +1738,6 @@ void fb_set_suspend(struct fb_info *info, int state)
{
struct fb_event event;
- if (!lock_fb_info(info))
- return;
event.info = info;
if (state) {
fb_notifier_call_chain(FB_EVENT_SUSPEND, &event);
@@ -1748,7 +1746,6 @@ void fb_set_suspend(struct fb_info *info, int state)
info->state = FBINFO_STATE_RUNNING;
fb_notifier_call_chain(FB_EVENT_RESUME, &event);
}
- unlock_fb_info(info);
}
/**
diff --git a/drivers/video/fbsysfs.c b/drivers/video/fbsysfs.c
index 04251ce..67afa9c 100644
--- a/drivers/video/fbsysfs.c
+++ b/drivers/video/fbsysfs.c
@@ -399,9 +399,12 @@ static ssize_t store_fbstate(struct device *device,
state = simple_strtoul(buf, &last, 0);
+ if (!lock_fb_info(fb_info))
+ return -ENODEV;
console_lock();
fb_set_suspend(fb_info, (int)state);
console_unlock();
+ unlock_fb_info(fb_info);
return count;
}
--
1.6.3.2
^ permalink raw reply related
* [PATCH] hecubafb: add module_put on error path in hecubafb_probe()
From: Pavel Shved @ 2011-06-17 16:25 UTC (permalink / raw)
To: Paul Mundt; +Cc: Pavel Shved, linux-fbdev, linux-kernel, ldv-project
In-Reply-To: <1308327913-23220-1-git-send-email-shved@ispras.ru>
In hecubafb_probe(), after a successful try_module_get, vzalloc may
fail and make the hecubafb_probe return, but the module is not put on
this error path.
This patch adds an exit point that calls module_put in such situation.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Pavel Shved <shved@ispras.ru>
---
drivers/video/hecubafb.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/video/hecubafb.c b/drivers/video/hecubafb.c
index fbef15f..614251a 100644
--- a/drivers/video/hecubafb.c
+++ b/drivers/video/hecubafb.c
@@ -233,7 +233,7 @@ static int __devinit hecubafb_probe(struct platform_device *dev)
videomemory = vzalloc(videomemorysize);
if (!videomemory)
- return retval;
+ goto err_videomem_alloc;
info = framebuffer_alloc(sizeof(struct hecubafb_par), &dev->dev);
if (!info)
@@ -275,6 +275,7 @@ err_fbreg:
framebuffer_release(info);
err_fballoc:
vfree(videomemory);
+err_videomem_alloc:
module_put(board->owner);
return retval;
}
--
1.7.0.2
^ permalink raw reply related
* Re: [PATCH] gx1fb: Fix section mismatch warnings
From: Sedat Dilek @ 2011-06-17 7:15 UTC (permalink / raw)
To: Randy Dunlap; +Cc: Paul Mundt, linux-geode, linux-fbdev, linux-kernel
In-Reply-To: <20110616123119.6ca873fa.randy.dunlap@oracle.com>
On Thu, Jun 16, 2011 at 9:31 PM, Randy Dunlap <randy.dunlap@oracle.com> wrote:
> On Thu, 6 Jan 2011 15:39:07 +0900 Paul Mundt wrote:
>
>> The problem seems to be because gx1fb_probe is annotated __init. In the
>> PCI case you want it to be __devinit, and you're also going to want to
>> annotate the remove function as __devexit and wrap it up with a
>> __devexit_p().
>> --
>
> From: Randy Dunlap <randy.dunlap@oracle.com>
>
> Fix a chain of section mismatches in geode driver, beginning with:
>
> WARNING: drivers/video/geode/gx1fb.o(.data+0x70): Section mismatch in reference from the variable gx1fb_driver to the function .init.text:gx1fb_probe()
> The variable gx1fb_driver references
> the function __init gx1fb_probe()
> If the reference is valid then annotate the
> variable with __init* or __refdata (see linux/init.h) or name the variable:
> *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console
>
> Making the changes that Paul pointed out resulted in a few more
> changes being needed, so they are all included here.
>
> Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
> ---
Hi Randy,
thanks for taking care of this old issue!
It's around a week I did not compile a linux-next kernel.
I should again enable full section mismatch in my setup.
Regards,
- Sedat -
^ permalink raw reply
* [PATCH] sm501fb: fix section mismatch warning
From: Randy Dunlap @ 2011-06-16 19:32 UTC (permalink / raw)
To: linux-fbdev
From: Randy Dunlap <randy.dunlap@oracle.com>
Fix section mismatch warning in sm501fb:
WARNING: drivers/video/sm501fb.o(.text+0x21d6): Section mismatch in reference from the function sm501fb_init_fb() to the variable .devinit.data:sm501_default_mode
The function sm501fb_init_fb() references
the variable __devinitdata sm501_default_mode.
This is often because sm501fb_init_fb lacks a __devinitdata
annotation or the annotation of sm501_default_mode is wrong.
Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
---
drivers/video/sm501fb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- lnx-30-rc3.orig/drivers/video/sm501fb.c
+++ lnx-30-rc3/drivers/video/sm501fb.c
@@ -1664,7 +1664,7 @@ static void sm501fb_stop(struct sm501fb_
resource_size(info->regs_res));
}
-static int sm501fb_init_fb(struct fb_info *fb,
+static int __devinit sm501fb_init_fb(struct fb_info *fb,
enum sm501_controller head,
const char *fbname)
{
^ permalink raw reply
* [PATCH] gx1fb: Fix section mismatch warnings
From: Randy Dunlap @ 2011-06-16 19:31 UTC (permalink / raw)
To: Paul Mundt
Cc: Sedat Dilek, linux-geode, linux-fbdev, linux-kernel, Sedat Dilek
In-Reply-To: <20110106063906.GB15340@linux-sh.org>
On Thu, 6 Jan 2011 15:39:07 +0900 Paul Mundt wrote:
> The problem seems to be because gx1fb_probe is annotated __init. In the
> PCI case you want it to be __devinit, and you're also going to want to
> annotate the remove function as __devexit and wrap it up with a
> __devexit_p().
> --
From: Randy Dunlap <randy.dunlap@oracle.com>
Fix a chain of section mismatches in geode driver, beginning with:
WARNING: drivers/video/geode/gx1fb.o(.data+0x70): Section mismatch in reference from the variable gx1fb_driver to the function .init.text:gx1fb_probe()
The variable gx1fb_driver references
the function __init gx1fb_probe()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console
Making the changes that Paul pointed out resulted in a few more
changes being needed, so they are all included here.
Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
---
drivers/video/geode/gx1fb_core.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
--- lnx-30-rc3.orig/drivers/video/geode/gx1fb_core.c
+++ lnx-30-rc3/drivers/video/geode/gx1fb_core.c
@@ -29,7 +29,7 @@ static int crt_option = 1;
static char panel_option[32] = "";
/* Modes relevant to the GX1 (taken from modedb.c) */
-static const struct fb_videomode __initdata gx1_modedb[] = {
+static const struct fb_videomode __devinitdata gx1_modedb[] = {
/* 640x480-60 VESA */
{ NULL, 60, 640, 480, 39682, 48, 16, 33, 10, 96, 2,
0, FB_VMODE_NONINTERLACED, FB_MODE_IS_VESA },
@@ -195,7 +195,7 @@ static int gx1fb_blank(int blank_mode, s
return par->vid_ops->blank_display(info, blank_mode);
}
-static int __init gx1fb_map_video_memory(struct fb_info *info, struct pci_dev *dev)
+static int __devinit gx1fb_map_video_memory(struct fb_info *info, struct pci_dev *dev)
{
struct geodefb_par *par = info->par;
unsigned gx_base;
@@ -268,7 +268,7 @@ static struct fb_ops gx1fb_ops = {
.fb_imageblit = cfb_imageblit,
};
-static struct fb_info * __init gx1fb_init_fbinfo(struct device *dev)
+static struct fb_info * __devinit gx1fb_init_fbinfo(struct device *dev)
{
struct geodefb_par *par;
struct fb_info *info;
@@ -318,7 +318,7 @@ static struct fb_info * __init gx1fb_ini
return info;
}
-static int __init gx1fb_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+static int __devinit gx1fb_probe(struct pci_dev *pdev, const struct pci_device_id *id)
{
struct geodefb_par *par;
struct fb_info *info;
@@ -382,7 +382,7 @@ static int __init gx1fb_probe(struct pci
return ret;
}
-static void gx1fb_remove(struct pci_dev *pdev)
+static void __devexit gx1fb_remove(struct pci_dev *pdev)
{
struct fb_info *info = pci_get_drvdata(pdev);
struct geodefb_par *par = info->par;
@@ -441,7 +441,7 @@ static struct pci_driver gx1fb_driver =
.name = "gx1fb",
.id_table = gx1fb_id_table,
.probe = gx1fb_probe,
- .remove = gx1fb_remove,
+ .remove = __devexit_p(gx1fb_remove),
};
static int __init gx1fb_init(void)
@@ -456,7 +456,7 @@ static int __init gx1fb_init(void)
return pci_register_driver(&gx1fb_driver);
}
-static void __exit gx1fb_cleanup(void)
+static void __devexit gx1fb_cleanup(void)
{
pci_unregister_driver(&gx1fb_driver);
}
^ permalink raw reply
* Re: [PATCH V2] video: Add GRVGA framebuffer device driver
From: Kristoffer Glembo @ 2011-06-16 8:55 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1308139443-15661-1-git-send-email-kristoffer@gaisler.com>
Hi Paul,
Paul Mundt wrote:
> On Wed, Jun 15, 2011 at 02:04:03PM +0200, Kristoffer Glembo wrote:
>> +#define GRVGA_REGLOAD(a) (__raw_readl(&(a)))
>> +#define GRVGA_REGSAVE(a, v) (__raw_writel(v, &(a)))
>> +#define GRVGA_REGORIN(a, v) (GRVGA_REGSAVE(a, (GRVGA_REGLOAD(a) | (v))))
>> +#define GRVGA_REGANDIN(a, v) (GRVGA_REGSAVE(a, (GRVGA_REGLOAD(a) & (v))))
>> +
> I would recommend just getting rid of these completely, they don't really
> buy you much of anything, other than forcing people to scroll to the top
> to figure out what exactly it's supposed to be doing.
I find it quite convenient when doing a lot of register accesses and especially
a lot of ORing and ANDing (this driver is simple enough to not benefit a lot).
It also makes it very clear that we are accessing a register which I think is good.
Anyway, it should be readable to other people as well and since this driver is
so simple I can remove it.
>
>> +#define GRVGA_FBINFO_DEFAULT (FBINFO_DEFAULT \
>> + | FBINFO_PARTIAL_PAN_OK \
>> + | FBINFO_HWACCEL_YPAN)
>> +
> Same with this.
Ok.
>
>> + var->red = (struct fb_bitfield) {16, 8, 0};
>> + var->green = (struct fb_bitfield) {8, 8, 0};
>> + var->blue = (struct fb_bitfield) {0, 8, 0};
>> + var->transp = (struct fb_bitfield) {24, 8, 0};
>> + break;
>
> This is also a bit unconventional. var should already be zeroed out, so
> you can just establish the .length values for each one as necessary.
I think this looks better than setting length and offset for each one.
> If you have open firmware available then why do you have a need to have
> this bizarre "custom" command line option for parsing all of the geometry
> information?
Our OF typically only contains the address and interrupt of each device (it
is automatically generated in the boot loader from a ROM area in the chip).
> You would probably benefit from ripping all of this out and stashing it
> in a grvga_setup() function.
Ok.
>> + par->regs = of_ioremap(&dev->resource[0], 0,
>> + resource_size(&dev->resource[0]),
>> + "grlib-svgactrl regs");
>> +
>> + retval = fb_alloc_cmap(&info->cmap, 256, 0);
>> + if (retval < 0) {
>> + dev_err(&dev->dev, "failed to allocate mem with fb_alloc_cmap\n");
>> + retval = -ENOMEM;
>> + goto err1;
>> + }
>> +
> Can of_ioremap() fail?
I will add error checking.
>> +
>> + virtual_start = (unsigned long) __get_free_pages(GFP_ATOMIC | GFP_DMA,
>> + get_order(grvga_mem_size));
>
> GFP_ATOMIC?
Hmm yes that seems very unnecessary, will remove.
>> + physical_start = __pa(virtual_start);
>> +
>> + /* Set page reserved so that mmap will work. This is necessary
>> + * since we'll be remapping normal memory.
>> + */
>> + for (page = virtual_start;
>> + page < PAGE_ALIGN(virtual_start + grvga_mem_size);
>> + page += PAGE_SIZE) {
>> + SetPageReserved(virt_to_page(page));
>> + }
>> + }
>> +
> This all looks like a good candidate for dma_alloc_coherent().
That would make the whole framebuffer uncachable and won't reserve the page?
>> + if (info) {
>> + unregister_framebuffer(info);
>> + fb_dealloc_cmap(&info->cmap);
>> + of_iounmap(&device->resource[0], par->regs,
>> + resource_size(&device->resource[0]));
>> + framebuffer_release(info);
>> + dev_set_drvdata(&device->dev, NULL);
>> + }
>> +
> You seem to be missing a release_mem_region() in here.
Indeed.
>> +int __init grvga_init(void)
>> +{
>> + return platform_driver_register(&grvga_driver);
>> +}
>> +
> static?
It should be, yes.
Thanks,
Kristoffer Glembo
^ permalink raw reply
* Re: [PATCH] video: Add GRVGA framebuffer device driver
From: Kristoffer Glembo @ 2011-06-16 7:20 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1308128180-14645-1-git-send-email-kristoffer@gaisler.com>
Hi Konrad,
Konrad Rzeszutek Wilk wrote:
>> +#define GRVGA_REGLOAD(a) (__raw_readl(&(a)))
>> +#define GRVGA_REGSAVE(a, v) (__raw_writel(v, &(a)))
>
> writel?
>
I use __raw since I want native endianess. On a big endian system
writel/readl byte swap since they are for little endian (PCI).
>> +struct grvga_regs {
>> + u32 status; /* 0x00 */
>> + u32 video_length; /* 0x04 */
>> + u32 front_porch; /* 0x08 */
>> + u32 sync_length; /* 0x0C */
>> + u32 line_length; /* 0x10 */
>> + u32 fb_pos; /* 0x14 */
>> + u32 clk_vector[4]; /* 0x18 */
>> + u32 clut; /* 0x20 */
>> +};
>
> Would it make sense to add __packed__ here? It seems that you
> really depend on this ordering.
>
I depend on the ordering, but that is guaranteed by the compiler without __packed__.
Attribute __packed__ is dangerous here since it tells the compiler that it is ok
to do byte accesses.
>> +static struct fb_ops grvga_ops = {
>> + .owner = THIS_MODULE,
>> + .fb_check_var = grvga_check_var,
>> + .fb_set_par = grvga_set_par,
>> + .fb_setcolreg = grvga_setcolreg,
>> + .fb_pan_display = grvga_pan_display,
>> + .fb_fillrect = cfb_fillrect,
>> + .fb_copyarea = cfb_copyarea,
>> + .fb_imageblit = cfb_imageblit
>> +};
>
> Could you move this struct down and remove the declarations earlier?
Yes sure.
>> + }
>> + if (i != -1)
>> + par->clk_sel = i;
>
> How would i possibly become -1?
Refactoring error .. thanks!
>> +static int grvga_set_par(struct fb_info *info)
>> +{
>> +
>> + int func = 0;
>
> You probably want that to be u32.
Sure why not.
> No default case?
Will add ...
>> + physical_start = __pa(virtual_start);
>
> Yikes. That is one big assumption. You don't want to use the PCI/DMA API
> to map it? I thought that on SPARCs the Bus addresses are different
> from the physical addresses?
Yeah I should map it for cleanliness sake. It does not matter on this architecture (LEON) though.
>> +
>> + if (info) {
>> + unregister_framebuffer(info);
>> + fb_dealloc_cmap(&info->cmap);
>> + of_iounmap(&device->resource[0], par->regs,
>> + resource_size(&device->resource[0]));
>> + framebuffer_release(info);
>> + dev_set_drvdata(&device->dev, NULL);
>
> Shouldn't that be much earlier? Like right after unregister_framebuffer?
>
You mean I should set driver_data = NULL ASAP after unregistering the framebuffer?
Does it really matter? Please enlighten me.
Thanks a lot for your feedback.
Best regards,
Kristoffer Glembo
^ permalink raw reply
* [GIT PULL] fbdev fixes for 3.0-rc4
From: Paul Mundt @ 2011-06-16 6:53 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-fbdev, linux-kernel
This is just a pretty straightforward set of fixes. It doesn't yet
address the console locking / suspend/resume issue as that's still
pending feedback.
Please pull from:
master.kernel.org:/pub/scm/linux/kernel/git/lethal/fbdev-3.x.git fbdev-fixes-for-linus
Which contains:
Geert Uytterhoeven (1):
fbdev/atyfb: Fix 2 defined-but-not-used warnings
Guennadi Liakhovetski (1):
fbdev: sh_mobile_hdmi: fix regression: statically enable RTPM
Jingoo Han (3):
video: s3c-fb: fix misleading kfree in remove function
video: s3c-fb: fix virtual resolution checking
video: s3c-fb: move enabling channel for window
Wanlong Gao (1):
efifb: Fix call to wrong unregister function
drivers/video/aty/atyfb_base.c | 10 ++++------
drivers/video/efifb.c | 2 +-
drivers/video/s3c-fb.c | 22 ++++++++++------------
drivers/video/sh_mobile_hdmi.c | 18 +++++-------------
4 files changed, 20 insertions(+), 32 deletions(-)
^ permalink raw reply
* Re: [PATCH V2] video: Add GRVGA framebuffer device driver
From: Paul Mundt @ 2011-06-16 6:34 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1308139443-15661-1-git-send-email-kristoffer@gaisler.com>
On Wed, Jun 15, 2011 at 02:04:03PM +0200, Kristoffer Glembo wrote:
> +#define GRVGA_REGLOAD(a) (__raw_readl(&(a)))
> +#define GRVGA_REGSAVE(a, v) (__raw_writel(v, &(a)))
> +#define GRVGA_REGORIN(a, v) (GRVGA_REGSAVE(a, (GRVGA_REGLOAD(a) | (v))))
> +#define GRVGA_REGANDIN(a, v) (GRVGA_REGSAVE(a, (GRVGA_REGLOAD(a) & (v))))
> +
I would recommend just getting rid of these completely, they don't really
buy you much of anything, other than forcing people to scroll to the top
to figure out what exactly it's supposed to be doing.
> +#define GRVGA_FBINFO_DEFAULT (FBINFO_DEFAULT \
> + | FBINFO_PARTIAL_PAN_OK \
> + | FBINFO_HWACCEL_YPAN)
> +
Same with this.
> + switch (info->var.bits_per_pixel) {
> + case 8:
> + var->red = (struct fb_bitfield) {0, 8, 0}; /* offset, length, msb-right */
> + var->green = (struct fb_bitfield) {0, 8, 0};
> + var->blue = (struct fb_bitfield) {0, 8, 0};
> + var->transp = (struct fb_bitfield) {0, 0, 0};
> + break;
> + case 16:
> + var->red = (struct fb_bitfield) {11, 5, 0};
> + var->green = (struct fb_bitfield) {5, 6, 0};
> + var->blue = (struct fb_bitfield) {0, 5, 0};
> + var->transp = (struct fb_bitfield) {0, 0, 0};
> + break;
> + case 24:
> + case 32:
> + var->red = (struct fb_bitfield) {16, 8, 0};
> + var->green = (struct fb_bitfield) {8, 8, 0};
> + var->blue = (struct fb_bitfield) {0, 8, 0};
> + var->transp = (struct fb_bitfield) {24, 8, 0};
> + break;
This is also a bit unconventional. var should already be zeroed out, so
you can just establish the .length values for each one as necessary.
> +static int __init grvga_parse_custom(char *options,
> + struct fb_var_screeninfo *screendata)
> +{
> + char *this_opt;
> + int count = 0;
> + if (!options || !*options)
> + return -1;
> +
> + while ((this_opt = strsep(&options, " ")) != NULL) {
> + if (!*this_opt)
> + continue;
> +
> + switch (count) {
> + case 0:
> + screendata->pixclock = simple_strtoul(this_opt, NULL, 0);
> + count++;
> + break;
> + case 1:
> + screendata->xres = screendata->xres_virtual = simple_strtoul(this_opt, NULL, 0);
> + count++;
> + break;
If you have open firmware available then why do you have a need to have
this bizarre "custom" command line option for parsing all of the geometry
information?
> +static int __devinit grvga_probe(struct platform_device *dev)
> +{
> + struct fb_info *info;
> + int retval = -ENOMEM;
> + unsigned long virtual_start;
> + unsigned long grvga_fix_addr = 0;
> + unsigned long physical_start = 0;
> + unsigned long grvga_mem_size = 0;
> + struct grvga_par *par;
> + char *options = NULL, *mode_opt = NULL;
> +
> + info = framebuffer_alloc(sizeof(struct grvga_par), &dev->dev);
> + if (!info) {
> + dev_err(&dev->dev, "framebuffer_alloc failed\n");
> + return -ENOMEM;
> + }
> +
> + /* Expecting: "grvga: modestring, [addr:<framebuffer physical address>], [size:<framebuffer size>]
> + *
> + * If modestring is custom:<custom mode string> we parse the string which then contains all videoparameters
> + * If address is left out, we allocate memory,
> + * if size is left out we only allocate enough to support the given mode.
> + */
> + if (fb_get_options("grvga", &options)) {
> + retval = -ENODEV;
> + goto err;
> + }
> +
> + if (!options || !*options)
> + options = "640x480-8@60";
> +
> + while (1) {
> + char *this_opt = strsep(&options, ",");
> +
> + if (!this_opt)
> + break;
> +
> + if (!strncmp(this_opt, "custom", 6))
> + grvga_parse_custom(this_opt, &info->var);
> + else if (!strncmp(this_opt, "addr:", 5))
> + grvga_fix_addr = simple_strtoul(this_opt + 5, NULL, 16);
> + else if (!strncmp(this_opt, "size:", 5))
> + grvga_mem_size = simple_strtoul(this_opt + 5, NULL, 0);
> + else
> + mode_opt = this_opt;
> + }
> +
You would probably benefit from ripping all of this out and stashing it
in a grvga_setup() function.
> + par = info->par;
> + info->fbops = &grvga_ops;
> + info->fix = grvga_fix;
> + info->pseudo_palette = par->color_palette;
> + info->flags = GRVGA_FBINFO_DEFAULT;
> + info->fix.smem_len = grvga_mem_size;
> +
> + par->regs = of_ioremap(&dev->resource[0], 0,
> + resource_size(&dev->resource[0]),
> + "grlib-svgactrl regs");
> +
> + retval = fb_alloc_cmap(&info->cmap, 256, 0);
> + if (retval < 0) {
> + dev_err(&dev->dev, "failed to allocate mem with fb_alloc_cmap\n");
> + retval = -ENOMEM;
> + goto err1;
> + }
> +
Can of_ioremap() fail?
> + if (mode_opt) {
> + retval = fb_find_mode(&info->var, info, mode_opt,
> + grvga_modedb, sizeof(grvga_modedb), &grvga_modedb[0], 8);
> + if (!retval || retval = 4) {
> + retval = -EINVAL;
> + goto err2;
> + }
> + }
> +
> + if (!grvga_mem_size)
> + grvga_mem_size = info->var.xres_virtual * info->var.yres_virtual * info->var.bits_per_pixel/8;
> +
> + if (grvga_fix_addr) {
> + /* Got framebuffer base address from argument list */
> +
> + physical_start = grvga_fix_addr;
> +
> + if (!request_mem_region(physical_start, grvga_mem_size, dev->name)) {
> + dev_err(&dev->dev, "request mem region failed\n");
> + retval = -ENOMEM;
> + goto err2;
> + }
> +
> + virtual_start = (unsigned long) ioremap(physical_start, grvga_mem_size);
> +
> + if (!virtual_start) {
> + dev_err(&dev->dev, "error mapping memory\n");
> + retval = -ENOMEM;
> + goto err3;
> + }
> + } else { /* Allocate frambuffer memory */
> +
> + unsigned long page;
> +
> + virtual_start = (unsigned long) __get_free_pages(GFP_ATOMIC | GFP_DMA,
> + get_order(grvga_mem_size));
GFP_ATOMIC?
> + if (!virtual_start) {
> + dev_err(&dev->dev,
> + "unable to allocate framebuffer memory (%lu bytes)\n",
> + grvga_mem_size);
> + retval = -ENOMEM;
> + goto err2;
> + }
> +
> +
> + physical_start = __pa(virtual_start);
> +
> + /* Set page reserved so that mmap will work. This is necessary
> + * since we'll be remapping normal memory.
> + */
> + for (page = virtual_start;
> + page < PAGE_ALIGN(virtual_start + grvga_mem_size);
> + page += PAGE_SIZE) {
> + SetPageReserved(virt_to_page(page));
> + }
> + }
> +
This all looks like a good candidate for dma_alloc_coherent().
> +static int __devexit grvga_remove(struct platform_device *device)
> +{
> + struct fb_info *info = dev_get_drvdata(&device->dev);
> + struct grvga_par *par = info->par;
> +
> + if (info) {
> + unregister_framebuffer(info);
> + fb_dealloc_cmap(&info->cmap);
> + of_iounmap(&device->resource[0], par->regs,
> + resource_size(&device->resource[0]));
> + framebuffer_release(info);
> + dev_set_drvdata(&device->dev, NULL);
> + }
> +
You seem to be missing a release_mem_region() in here.
> + return 0;
> +}
> +
> +static struct of_device_id svgactrl_of_match[] = {
> + {
> + .name = "GAISLER_SVGACTRL",
> + },
> + {
> + .name = "01_063",
> + },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, svgactrl_of_match);
> +
> +static struct platform_driver grvga_driver = {
> + .driver = {
> + .name = "grlib-svgactrl",
> + .owner = THIS_MODULE,
> + .of_match_table = svgactrl_of_match,
> + },
> + .probe = grvga_probe,
> + .remove = __devexit_p(grvga_remove),
> +};
> +
> +
> +int __init grvga_init(void)
> +{
> + return platform_driver_register(&grvga_driver);
> +}
> +
static?
^ permalink raw reply
* Re: [PATCH] video: Add GRVGA framebuffer device driver
From: Konrad Rzeszutek Wilk @ 2011-06-15 14:56 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1308128180-14645-1-git-send-email-kristoffer@gaisler.com>
On Wed, Jun 15, 2011 at 10:56:20AM +0200, Kristoffer Glembo wrote:
> This patch adds support for the GRVGA framebuffer IP core from Aeroflex Gaisler.
> The device is used in LEON SPARCV8 based System on Chips. Documentation can
> be found here: www.gaisler.com/products/grlib/grip.pdf.
See comments below.
>
> Signed-off-by: Kristoffer Glembo <kristoffer@gaisler.com>
> ---
> drivers/video/Kconfig | 10 +
> drivers/video/Makefile | 1 +
> drivers/video/grvga.c | 559 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 570 insertions(+), 0 deletions(-)
> create mode 100644 drivers/video/grvga.c
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 549b960..18ee201 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -259,6 +259,16 @@ config FB_TILEBLITTING
> comment "Frame buffer hardware drivers"
> depends on FB
>
> +config FB_GRVGA
> + tristate "Aeroflex Gaisler framebuffer support"
> + depends on FB && SPARC
> + select FB_CFB_FILLRECT
> + select FB_CFB_COPYAREA
> + select FB_CFB_IMAGEBLIT
> + ---help---
> + This enables support for the SVGACTRL framebuffer in the GRLIB IP library from Aeroflex Gaisler.
> +
> +
> config FB_CIRRUS
> tristate "Cirrus Logic support"
> depends on FB && (ZORRO || PCI)
> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> index 8b83129..4cff5ec 100644
> --- a/drivers/video/Makefile
> +++ b/drivers/video/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_FB_DEFERRED_IO) += fb_defio.o
> obj-$(CONFIG_FB_WMT_GE_ROPS) += wmt_ge_rops.o
>
> # Hardware specific drivers go first
> +obj-$(CONFIG_FB_GRVGA) += grvga.o
> obj-$(CONFIG_FB_AMIGA) += amifb.o c2p_planar.o
> obj-$(CONFIG_FB_ARC) += arcfb.o
> obj-$(CONFIG_FB_CLPS711X) += clps711xfb.o
> diff --git a/drivers/video/grvga.c b/drivers/video/grvga.c
> new file mode 100644
> index 0000000..e1514f9
> --- /dev/null
> +++ b/drivers/video/grvga.c
> @@ -0,0 +1,559 @@
> +/*
> + * Driver for Aeroflex Gaisler SVGACTRL framebuffer device.
> + *
> + * 2011 (c) Aeroflex Gaisler AB
> + *
> + * Full documentation of the core can be found here:
> + * http://www.gaisler.com/products/grlib/grip.pdf
> + *
> + * 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.
> + *
> + * Contributors: Kristoffer Glembo <kristoffer@gaisler.com>
> + *
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_device.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/string.h>
> +#include <linux/delay.h>
> +#include <linux/errno.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/tty.h>
> +#include <linux/mm.h>
> +#include <linux/fb.h>
> +#include <linux/io.h>
> +
> +#define GRVGA_REGLOAD(a) (__raw_readl(&(a)))
> +#define GRVGA_REGSAVE(a, v) (__raw_writel(v, &(a)))
writel?
> +#define GRVGA_REGORIN(a, v) (GRVGA_REGSAVE(a, (GRVGA_REGLOAD(a) | (v))))
> +#define GRVGA_REGANDIN(a, v) (GRVGA_REGSAVE(a, (GRVGA_REGLOAD(a) & (v))))
> +
> +#define GRVGA_FBINFO_DEFAULT (FBINFO_DEFAULT \
> + | FBINFO_PARTIAL_PAN_OK \
> + | FBINFO_HWACCEL_YPAN)
> +
> +struct grvga_regs {
> + u32 status; /* 0x00 */
> + u32 video_length; /* 0x04 */
> + u32 front_porch; /* 0x08 */
> + u32 sync_length; /* 0x0C */
> + u32 line_length; /* 0x10 */
> + u32 fb_pos; /* 0x14 */
> + u32 clk_vector[4]; /* 0x18 */
> + u32 clut; /* 0x20 */
> +};
Would it make sense to add __packed__ here? It seems that you
really depend on this ordering.
> +
> +struct grvga_par {
> + struct grvga_regs *regs;
> + u32 color_palette[16]; /* 16 entry pseudo palette used by fbcon in true color mode */
> + int clk_sel;
> +};
> +
> +
> +static const struct fb_videomode grvga_modedb[] = {
> + {
> + /* 640x480 @ 60 Hz */
> + NULL, 60, 640, 480, 40000, 48, 16, 39, 11, 96, 2,
> + 0, FB_VMODE_NONINTERLACED
> + }, {
> + /* 800x600 @ 60 Hz */
> + NULL, 60, 800, 600, 25000, 88, 40, 23, 1, 128, 4,
> + 0, FB_VMODE_NONINTERLACED
> + }, {
> + /* 800x600 @ 72 Hz */
> + NULL, 72, 800, 600, 20000, 64, 56, 23, 37, 120, 6,
> + 0, FB_VMODE_NONINTERLACED
> + }, {
> + /* 1024x768 @ 60 Hz */
> + NULL, 60, 1024, 768, 15385, 160, 24, 29, 3, 136, 6,
> + 0, FB_VMODE_NONINTERLACED
> + }
> + };
> +
> +static struct fb_fix_screeninfo grvga_fix __initdata = {
> + .id = "AG SVGACTRL",
> + .type = FB_TYPE_PACKED_PIXELS,
> + .visual = FB_VISUAL_PSEUDOCOLOR,
> + .xpanstep = 0,
> + .ypanstep = 1,
> + .ywrapstep = 0,
> + .accel = FB_ACCEL_NONE,
> +};
> +
> +
> +static int grvga_check_var(struct fb_var_screeninfo *var,
> + struct fb_info *info);
> +static int grvga_set_par(struct fb_info *info);
> +static int grvga_setcolreg(unsigned regno,
> + unsigned red, unsigned green, unsigned blue, unsigned transp,
> + struct fb_info *info);
> +static int grvga_pan_display(struct fb_var_screeninfo *var, struct fb_info *info);
> +
> +static struct fb_ops grvga_ops = {
> + .owner = THIS_MODULE,
> + .fb_check_var = grvga_check_var,
> + .fb_set_par = grvga_set_par,
> + .fb_setcolreg = grvga_setcolreg,
> + .fb_pan_display = grvga_pan_display,
> + .fb_fillrect = cfb_fillrect,
> + .fb_copyarea = cfb_copyarea,
> + .fb_imageblit = cfb_imageblit
> +};
Could you move this struct down and remove the declarations earlier?
> +
> +static int grvga_check_var(struct fb_var_screeninfo *var,
> + struct fb_info *info)
> +{
> + struct grvga_par *par = info->par;
> + int i;
> +
> + if (!var->xres)
> + var->xres = 1;
> + if (!var->yres)
> + var->yres = 1;
> + if (var->bits_per_pixel <= 8)
> + var->bits_per_pixel = 8;
> + else if (var->bits_per_pixel <= 16)
> + var->bits_per_pixel = 16;
> + else if (var->bits_per_pixel <= 24)
> + var->bits_per_pixel = 24;
> + else if (var->bits_per_pixel <= 32)
> + var->bits_per_pixel = 32;
> + else
> + return -EINVAL;
> +
> + var->xres_virtual = var->xres;
> + var->yres_virtual = 2*var->yres;
> +
> + if (info->fix.smem_len) {
> + if ((var->yres_virtual*var->xres_virtual*var->bits_per_pixel/8) > info->fix.smem_len)
> + return -ENOMEM;
> + }
> +
> + /* Which clocks that are available can be read out in these registers */
> + for (i = 0; i <= 3 ; i++) {
> + if (var->pixclock = par->regs->clk_vector[i])
> + break;
> + }
> + if (i != -1)
> + par->clk_sel = i;
How would i possibly become -1?
> + else
> + return -EINVAL;
> +
> + switch (info->var.bits_per_pixel) {
> + case 8:
> + var->red = (struct fb_bitfield) {0, 8, 0}; /* offset, length, msb-right */
> + var->green = (struct fb_bitfield) {0, 8, 0};
> + var->blue = (struct fb_bitfield) {0, 8, 0};
> + var->transp = (struct fb_bitfield) {0, 0, 0};
> + break;
> + case 16:
> + var->red = (struct fb_bitfield) {11, 5, 0};
> + var->green = (struct fb_bitfield) {5, 6, 0};
> + var->blue = (struct fb_bitfield) {0, 5, 0};
> + var->transp = (struct fb_bitfield) {0, 0, 0};
> + break;
> + case 24:
> + case 32:
> + var->red = (struct fb_bitfield) {16, 8, 0};
> + var->green = (struct fb_bitfield) {8, 8, 0};
> + var->blue = (struct fb_bitfield) {0, 8, 0};
> + var->transp = (struct fb_bitfield) {24, 8, 0};
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int grvga_set_par(struct fb_info *info)
> +{
> +
> + int func = 0;
You probably want that to be u32.
> + struct grvga_par *par = info->par;
> +
> + GRVGA_REGSAVE(par->regs->video_length,
> + ((info->var.yres - 1) << 16) | (info->var.xres - 1));
> + GRVGA_REGSAVE(par->regs->front_porch,
> + (info->var.lower_margin << 16) | (info->var.right_margin));
> + GRVGA_REGSAVE(par->regs->sync_length,
> + (info->var.vsync_len << 16) | (info->var.hsync_len));
> + GRVGA_REGSAVE(par->regs->line_length,
> + ((info->var.yres + info->var.lower_margin + info->var.upper_margin + info->var.vsync_len - 1) << 16) |
> + (info->var.xres + info->var.right_margin + info->var.left_margin + info->var.hsync_len - 1));
> +
> + switch (info->var.bits_per_pixel) {
> + case 8:
> + info->fix.visual = FB_VISUAL_PSEUDOCOLOR;
> + func = 1;
> + break;
> + case 16:
> + info->fix.visual = FB_VISUAL_TRUECOLOR;
> + func = 2;
> + break;
> + case 24:
> + case 32:
> + info->fix.visual = FB_VISUAL_TRUECOLOR;
> + func = 3;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + GRVGA_REGSAVE(par->regs->status, ((par->clk_sel << 6) | (func << 4)) | 1);
Just in case in the future you modify the code and bitshift func more than you thought
and end up with a signed value in an unsigned register.
> +
> + info->fix.line_length = (info->var.xres_virtual*info->var.bits_per_pixel)/8;
> + return 0;
> +}
> +
> +static int grvga_setcolreg(unsigned regno, unsigned red, unsigned green, unsigned blue, unsigned transp, struct fb_info *info)
> +{
> + struct grvga_par *par;
> + par = info->par;
> +
> + if (regno >= 256) /* Size of CLUT */
> + return -EINVAL;
> +
> + if (info->var.grayscale) {
> + /* grayscale = 0.30*R + 0.59*G + 0.11*B */
> + red = green = blue = (red * 77 + green * 151 + blue * 28) >> 8;
> + }
> +
> +
> +
> +#define CNVT_TOHW(val, width) ((((val)<<(width))+0x7FFF-(val))>>16)
> +
> + red = CNVT_TOHW(red, info->var.red.length);
> + green = CNVT_TOHW(green, info->var.green.length);
> + blue = CNVT_TOHW(blue, info->var.blue.length);
> + transp = CNVT_TOHW(transp, info->var.transp.length);
> +
> +#undef CNVT_TOHW
> +
> + /* In PSEUDOCOLOR we use the hardware CLUT */
> + if (info->fix.visual = FB_VISUAL_PSEUDOCOLOR)
> + GRVGA_REGSAVE(par->regs->clut,
> + (regno << 24) | (red << 16) | (green << 8) | blue);
> +
> + /* Truecolor uses the pseudo palette */
> + else if (info->fix.visual = FB_VISUAL_TRUECOLOR) {
> + u32 v;
> + if (regno >= 16)
> + return -EINVAL;
> +
> +
> + v = (red << info->var.red.offset) |
> + (green << info->var.green.offset) |
> + (blue << info->var.blue.offset) |
> + (transp << info->var.transp.offset);
> +
> + ((u32 *) (info->pseudo_palette))[regno] = v;
> + }
> + return 0;
> +}
> +
> +static int grvga_pan_display(struct fb_var_screeninfo *var,
> + struct fb_info *info)
> +{
> + struct grvga_par *par = info->par;
> + struct fb_fix_screeninfo *fix = &info->fix;
> + u32 base_addr;
> +
> + if (var->xoffset != 0)
> + return -EINVAL;
> +
> + base_addr = fix->smem_start + (var->yoffset * fix->line_length);
> + base_addr &= ~3UL;
> +
> + /* Set framebuffer base address */
> + GRVGA_REGSAVE(par->regs->fb_pos, base_addr);
> +
> + return 0;
> +}
> +
> +static int __init grvga_parse_custom(char *options,
> + struct fb_var_screeninfo *screendata)
> +{
> + char *this_opt;
> + int count = 0;
> + if (!options || !*options)
> + return -1;
> +
> + while ((this_opt = strsep(&options, " ")) != NULL) {
> + if (!*this_opt)
> + continue;
> +
> + switch (count) {
> + case 0:
> + screendata->pixclock = simple_strtoul(this_opt, NULL, 0);
> + count++;
> + break;
> + case 1:
> + screendata->xres = screendata->xres_virtual = simple_strtoul(this_opt, NULL, 0);
> + count++;
> + break;
> + case 2:
> + screendata->right_margin = simple_strtoul(this_opt, NULL, 0);
> + count++;
> + break;
> + case 3:
> + screendata->hsync_len = simple_strtoul(this_opt, NULL, 0);
> + count++;
> + break;
> + case 4:
> + screendata->left_margin = simple_strtoul(this_opt, NULL, 0);
> + count++;
> + break;
> + case 5:
> + screendata->yres = screendata->yres_virtual = simple_strtoul(this_opt, NULL, 0);
> + count++;
> + break;
> + case 6:
> + screendata->lower_margin = simple_strtoul(this_opt, NULL, 0);
> + count++;
> + break;
> + case 7:
> + screendata->vsync_len = simple_strtoul(this_opt, NULL, 0);
> + count++;
> + break;
> + case 8:
> + screendata->upper_margin = simple_strtoul(this_opt, NULL, 0);
> + count++;
> + break;
> + case 9:
> + screendata->bits_per_pixel = simple_strtoul(this_opt, NULL, 0);
> + count++;
> + break;
No default case?
> + }
> + }
> + screendata->activate = FB_ACTIVATE_NOW;
> + screendata->vmode = FB_VMODE_NONINTERLACED;
> + return 0;
> +}
> +
> +static int __devinit grvga_probe(struct platform_device *dev)
> +{
> + struct fb_info *info;
> + int retval = -ENOMEM;
> + unsigned long virtual_start;
> + unsigned long grvga_fix_addr = 0;
> + unsigned long physical_start = 0;
> + unsigned long grvga_mem_size = 0;
> + struct grvga_par *par;
> + char *options = NULL, *mode_opt = NULL;
> +
> + info = framebuffer_alloc(sizeof(struct grvga_par), &dev->dev);
> + if (!info) {
> + dev_err(&dev->dev, "framebuffer_alloc failed\n");
> + return -ENOMEM;
> + }
> +
> + /* Expecting: "grvga: modestring, [addr:<framebuffer physical address>], [size:<framebuffer size>]
> + *
> + * If modestring is custom:<custom mode string> we parse the string which then contains all videoparameters
> + * If address is left out, we allocate memory,
> + * if size is left out we only allocate enough to support the given mode.
> + */
> + if (fb_get_options("grvga", &options)) {
> + retval = -ENODEV;
> + goto err;
> + }
> +
> + if (!options || !*options)
> + options = "640x480-8@60";
> +
> + while (1) {
> + char *this_opt = strsep(&options, ",");
> +
> + if (!this_opt)
> + break;
> +
> + if (!strncmp(this_opt, "custom", 6))
> + grvga_parse_custom(this_opt, &info->var);
> + else if (!strncmp(this_opt, "addr:", 5))
> + grvga_fix_addr = simple_strtoul(this_opt + 5, NULL, 16);
> + else if (!strncmp(this_opt, "size:", 5))
> + grvga_mem_size = simple_strtoul(this_opt + 5, NULL, 0);
> + else
> + mode_opt = this_opt;
> + }
> +
> + par = info->par;
> + info->fbops = &grvga_ops;
> + info->fix = grvga_fix;
> + info->pseudo_palette = par->color_palette;
> + info->flags = GRVGA_FBINFO_DEFAULT;
> + info->fix.smem_len = grvga_mem_size;
> +
> + par->regs = of_ioremap(&dev->resource[0], 0,
> + resource_size(&dev->resource[0]),
> + "grlib-svgactrl regs");
> +
> + retval = fb_alloc_cmap(&info->cmap, 256, 0);
> + if (retval < 0) {
> + dev_err(&dev->dev, "failed to allocate mem with fb_alloc_cmap\n");
> + retval = -ENOMEM;
> + goto err1;
> + }
> +
> + if (mode_opt) {
> + retval = fb_find_mode(&info->var, info, mode_opt,
> + grvga_modedb, sizeof(grvga_modedb), &grvga_modedb[0], 8);
> + if (!retval || retval = 4) {
> + retval = -EINVAL;
> + goto err2;
> + }
> + }
> +
> + if (!grvga_mem_size)
> + grvga_mem_size = info->var.xres_virtual * info->var.yres_virtual * info->var.bits_per_pixel/8;
Should you update the info->fix.smem_len ?
> +
> + if (grvga_fix_addr) {
> + /* Got framebuffer base address from argument list */
> +
> + physical_start = grvga_fix_addr;
> +
> + if (!request_mem_region(physical_start, grvga_mem_size, dev->name)) {
> + dev_err(&dev->dev, "request mem region failed\n");
requested..
> + retval = -ENOMEM;
> + goto err2;
> + }
> +
> + virtual_start = (unsigned long) ioremap(physical_start, grvga_mem_size);
> +
> + if (!virtual_start) {
> + dev_err(&dev->dev, "error mapping memory\n");
> + retval = -ENOMEM;
> + goto err3;
> + }
> + } else { /* Allocate frambuffer memory */
> +
> + unsigned long page;
> +
> + virtual_start = (unsigned long) __get_free_pages(GFP_ATOMIC | GFP_DMA,
> + get_order(grvga_mem_size));
> + if (!virtual_start) {
> + dev_err(&dev->dev,
> + "unable to allocate framebuffer memory (%lu bytes)\n",
> + grvga_mem_size);
> + retval = -ENOMEM;
> + goto err2;
> + }
> +
> +
> + physical_start = __pa(virtual_start);
Yikes. That is one big assumption. You don't want to use the PCI/DMA API
to map it? I thought that on SPARCs the Bus addresses are different
from the physical addresses?
> +
> + /* Set page reserved so that mmap will work. This is necessary
> + * since we'll be remapping normal memory.
> + */
> + for (page = virtual_start;
> + page < PAGE_ALIGN(virtual_start + grvga_mem_size);
> + page += PAGE_SIZE) {
> + SetPageReserved(virt_to_page(page));
> + }
> + }
> +
> + memset((unsigned long *) virtual_start, 0, grvga_mem_size);
> +
> + info->screen_base = (char __iomem *) virtual_start;
> + info->fix.smem_start = physical_start;
> + info->fix.smem_len = grvga_mem_size;
Ah, you update it here. OK.
> +
> + dev_set_drvdata(&dev->dev, info);
> +
> + dev_info(&dev->dev,
> + "Aeroflex Gaisler framebuffer device (fb%d), %dx%d-%d, using %luK of video memory @ 0x%x\n",
> + info->node, info->var.xres, info->var.yres, info->var.bits_per_pixel,
> + grvga_mem_size >> 10, (unsigned int) info->screen_base);
> +
> + retval = register_framebuffer(info);
> + if (retval < 0) {
> + dev_err(&dev->dev, "failed to register framebuffer\n");
> + if (grvga_fix_addr)
> + goto err3;
> + else {
> + kfree((void *)virtual_start);
> + goto err2;
> + }
> + }
> +
> + GRVGA_REGSAVE(par->regs->fb_pos, physical_start);
> + GRVGA_REGORIN(par->regs->status, 1); /* Enable framebuffer */
> +
> + return 0;
> +
> +err3:
> + release_mem_region(physical_start, grvga_mem_size);
> +err2:
> + fb_dealloc_cmap(&info->cmap);
> +err1:
> + of_iounmap(&dev->resource[0], par->regs,
> + resource_size(&dev->resource[0]));
> +err:
> + framebuffer_release(info);
> +
> + return retval;
> +}
> +
> +static int __devexit grvga_remove(struct platform_device *device)
> +{
> + struct fb_info *info = dev_get_drvdata(&device->dev);
> + struct grvga_par *par = info->par;
> +
> + if (info) {
> + unregister_framebuffer(info);
> + fb_dealloc_cmap(&info->cmap);
> + of_iounmap(&device->resource[0], par->regs,
> + resource_size(&device->resource[0]));
> + framebuffer_release(info);
> + dev_set_drvdata(&device->dev, NULL);
Shouldn't that be much earlier? Like right after unregister_framebuffer?
> + }
> +
> + return 0;
> +}
> +
> +static struct of_device_id svgactrl_of_match[] = {
> + {
> + .name = "GAISLER_SVGACTRL",
> + },
> + {
> + .name = "01_063",
> + },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, svgactrl_of_match);
> +
> +static struct platform_driver grvga_driver = {
> + .driver = {
> + .name = "grlib-svgactrl",
> + .owner = THIS_MODULE,
> + .of_match_table = svgactrl_of_match,
> + },
> + .probe = grvga_probe,
> + .remove = __devexit_p(grvga_remove),
> +};
> +
> +
> +int __init grvga_init(void)
> +{
> + return platform_driver_register(&grvga_driver);
> +}
> +
> +static void __exit grvga_exit(void)
> +{
> + platform_driver_unregister(&grvga_driver);
> +}
> +
> +module_init(grvga_init);
> +module_exit(grvga_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Aeroflex Gaisler");
> +MODULE_DESCRIPTION("Aeroflex Gaisler framebuffer device driver");
> --
> 1.6.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] video: Add GRVGA framebuffer device driver
From: Geert Uytterhoeven @ 2011-06-15 12:09 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1308128180-14645-1-git-send-email-kristoffer@gaisler.com>
On Wed, Jun 15, 2011 at 13:52, Kristoffer Glembo <kristoffer@gaisler.com> wrote:
> Geert Uytterhoeven wrote:
>> At first sight, nothing in this driver seems to be SPARC-specific, so
>> perhaps this can be relaxed
>> to e.g. depends on OF_DEVICE?
>
> It uses of_ioremap/unmap which are SPARC-specific. I have been bitten by things like this before
> when thinking I was portable so this time I just depended on SPARC. The core is not used on any
> other platforms and is not very likely to be used either.
Ah, your're right. I just thought all OF-capable platforms had of_ioremap().
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
^ permalink raw reply
* [PATCH V2] video: Add GRVGA framebuffer device driver
From: Kristoffer Glembo @ 2011-06-15 12:04 UTC (permalink / raw)
To: linux-fbdev
This patch adds support for the GRVGA framebuffer IP core from Aeroflex Gaisler.
The device is used in LEON SPARCV8 based System on Chips. Documentation can
be found here: www.gaisler.com/products/grlib/grip.pdf.
Signed-off-by: Kristoffer Glembo <kristoffer@gaisler.com>
---
drivers/video/Kconfig | 10 +
drivers/video/Makefile | 1 +
drivers/video/grvga.c | 559 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 570 insertions(+), 0 deletions(-)
create mode 100644 drivers/video/grvga.c
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 549b960..18ee201 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -259,6 +259,16 @@ config FB_TILEBLITTING
comment "Frame buffer hardware drivers"
depends on FB
+config FB_GRVGA
+ tristate "Aeroflex Gaisler framebuffer support"
+ depends on FB && SPARC
+ select FB_CFB_FILLRECT
+ select FB_CFB_COPYAREA
+ select FB_CFB_IMAGEBLIT
+ ---help---
+ This enables support for the SVGACTRL framebuffer in the GRLIB IP library from Aeroflex Gaisler.
+
+
config FB_CIRRUS
tristate "Cirrus Logic support"
depends on FB && (ZORRO || PCI)
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 8b83129..4cff5ec 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_FB_DEFERRED_IO) += fb_defio.o
obj-$(CONFIG_FB_WMT_GE_ROPS) += wmt_ge_rops.o
# Hardware specific drivers go first
+obj-$(CONFIG_FB_GRVGA) += grvga.o
obj-$(CONFIG_FB_AMIGA) += amifb.o c2p_planar.o
obj-$(CONFIG_FB_ARC) += arcfb.o
obj-$(CONFIG_FB_CLPS711X) += clps711xfb.o
diff --git a/drivers/video/grvga.c b/drivers/video/grvga.c
new file mode 100644
index 0000000..aa1f9ab
--- /dev/null
+++ b/drivers/video/grvga.c
@@ -0,0 +1,559 @@
+/*
+ * Driver for Aeroflex Gaisler SVGACTRL framebuffer device.
+ *
+ * 2011 (c) Aeroflex Gaisler AB
+ *
+ * Full documentation of the core can be found here:
+ * http://www.gaisler.com/products/grlib/grip.pdf
+ *
+ * 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.
+ *
+ * Contributors: Kristoffer Glembo <kristoffer@gaisler.com>
+ *
+ */
+
+#include <linux/platform_device.h>
+#include <linux/of_platform.h>
+#include <linux/of_device.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/string.h>
+#include <linux/delay.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/tty.h>
+#include <linux/mm.h>
+#include <linux/fb.h>
+#include <linux/io.h>
+
+#define GRVGA_REGLOAD(a) (__raw_readl(&(a)))
+#define GRVGA_REGSAVE(a, v) (__raw_writel(v, &(a)))
+#define GRVGA_REGORIN(a, v) (GRVGA_REGSAVE(a, (GRVGA_REGLOAD(a) | (v))))
+#define GRVGA_REGANDIN(a, v) (GRVGA_REGSAVE(a, (GRVGA_REGLOAD(a) & (v))))
+
+#define GRVGA_FBINFO_DEFAULT (FBINFO_DEFAULT \
+ | FBINFO_PARTIAL_PAN_OK \
+ | FBINFO_HWACCEL_YPAN)
+
+struct grvga_regs {
+ u32 status; /* 0x00 */
+ u32 video_length; /* 0x04 */
+ u32 front_porch; /* 0x08 */
+ u32 sync_length; /* 0x0C */
+ u32 line_length; /* 0x10 */
+ u32 fb_pos; /* 0x14 */
+ u32 clk_vector[4]; /* 0x18 */
+ u32 clut; /* 0x20 */
+};
+
+struct grvga_par {
+ struct grvga_regs *regs;
+ u32 color_palette[16]; /* 16 entry pseudo palette used by fbcon in true color mode */
+ int clk_sel;
+};
+
+
+static const struct fb_videomode grvga_modedb[] = {
+ {
+ /* 640x480 @ 60 Hz */
+ NULL, 60, 640, 480, 40000, 48, 16, 39, 11, 96, 2,
+ 0, FB_VMODE_NONINTERLACED
+ }, {
+ /* 800x600 @ 60 Hz */
+ NULL, 60, 800, 600, 25000, 88, 40, 23, 1, 128, 4,
+ 0, FB_VMODE_NONINTERLACED
+ }, {
+ /* 800x600 @ 72 Hz */
+ NULL, 72, 800, 600, 20000, 64, 56, 23, 37, 120, 6,
+ 0, FB_VMODE_NONINTERLACED
+ }, {
+ /* 1024x768 @ 60 Hz */
+ NULL, 60, 1024, 768, 15385, 160, 24, 29, 3, 136, 6,
+ 0, FB_VMODE_NONINTERLACED
+ }
+ };
+
+static struct fb_fix_screeninfo grvga_fix __initdata = {
+ .id = "AG SVGACTRL",
+ .type = FB_TYPE_PACKED_PIXELS,
+ .visual = FB_VISUAL_PSEUDOCOLOR,
+ .xpanstep = 0,
+ .ypanstep = 1,
+ .ywrapstep = 0,
+ .accel = FB_ACCEL_NONE,
+};
+
+
+static int grvga_check_var(struct fb_var_screeninfo *var,
+ struct fb_info *info);
+static int grvga_set_par(struct fb_info *info);
+static int grvga_setcolreg(unsigned regno,
+ unsigned red, unsigned green, unsigned blue, unsigned transp,
+ struct fb_info *info);
+static int grvga_pan_display(struct fb_var_screeninfo *var, struct fb_info *info);
+
+static struct fb_ops grvga_ops = {
+ .owner = THIS_MODULE,
+ .fb_check_var = grvga_check_var,
+ .fb_set_par = grvga_set_par,
+ .fb_setcolreg = grvga_setcolreg,
+ .fb_pan_display = grvga_pan_display,
+ .fb_fillrect = cfb_fillrect,
+ .fb_copyarea = cfb_copyarea,
+ .fb_imageblit = cfb_imageblit
+};
+
+static int grvga_check_var(struct fb_var_screeninfo *var,
+ struct fb_info *info)
+{
+ struct grvga_par *par = info->par;
+ int i;
+
+ if (!var->xres)
+ var->xres = 1;
+ if (!var->yres)
+ var->yres = 1;
+ if (var->bits_per_pixel <= 8)
+ var->bits_per_pixel = 8;
+ else if (var->bits_per_pixel <= 16)
+ var->bits_per_pixel = 16;
+ else if (var->bits_per_pixel <= 24)
+ var->bits_per_pixel = 24;
+ else if (var->bits_per_pixel <= 32)
+ var->bits_per_pixel = 32;
+ else
+ return -EINVAL;
+
+ var->xres_virtual = var->xres;
+ var->yres_virtual = 2*var->yres;
+
+ if (info->fix.smem_len) {
+ if ((var->yres_virtual*var->xres_virtual*var->bits_per_pixel/8) > info->fix.smem_len)
+ return -ENOMEM;
+ }
+
+ /* Which clocks that are available can be read out in these registers */
+ for (i = 0; i <= 3 ; i++) {
+ if (var->pixclock = par->regs->clk_vector[i])
+ break;
+ }
+ if (i != -1)
+ par->clk_sel = i;
+ else
+ return -EINVAL;
+
+ switch (info->var.bits_per_pixel) {
+ case 8:
+ var->red = (struct fb_bitfield) {0, 8, 0}; /* offset, length, msb-right */
+ var->green = (struct fb_bitfield) {0, 8, 0};
+ var->blue = (struct fb_bitfield) {0, 8, 0};
+ var->transp = (struct fb_bitfield) {0, 0, 0};
+ break;
+ case 16:
+ var->red = (struct fb_bitfield) {11, 5, 0};
+ var->green = (struct fb_bitfield) {5, 6, 0};
+ var->blue = (struct fb_bitfield) {0, 5, 0};
+ var->transp = (struct fb_bitfield) {0, 0, 0};
+ break;
+ case 24:
+ case 32:
+ var->red = (struct fb_bitfield) {16, 8, 0};
+ var->green = (struct fb_bitfield) {8, 8, 0};
+ var->blue = (struct fb_bitfield) {0, 8, 0};
+ var->transp = (struct fb_bitfield) {24, 8, 0};
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int grvga_set_par(struct fb_info *info)
+{
+
+ int func = 0;
+ struct grvga_par *par = info->par;
+
+ GRVGA_REGSAVE(par->regs->video_length,
+ ((info->var.yres - 1) << 16) | (info->var.xres - 1));
+ GRVGA_REGSAVE(par->regs->front_porch,
+ (info->var.lower_margin << 16) | (info->var.right_margin));
+ GRVGA_REGSAVE(par->regs->sync_length,
+ (info->var.vsync_len << 16) | (info->var.hsync_len));
+ GRVGA_REGSAVE(par->regs->line_length,
+ ((info->var.yres + info->var.lower_margin + info->var.upper_margin + info->var.vsync_len - 1) << 16) |
+ (info->var.xres + info->var.right_margin + info->var.left_margin + info->var.hsync_len - 1));
+
+ switch (info->var.bits_per_pixel) {
+ case 8:
+ info->fix.visual = FB_VISUAL_PSEUDOCOLOR;
+ func = 1;
+ break;
+ case 16:
+ info->fix.visual = FB_VISUAL_TRUECOLOR;
+ func = 2;
+ break;
+ case 24:
+ case 32:
+ info->fix.visual = FB_VISUAL_TRUECOLOR;
+ func = 3;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ GRVGA_REGSAVE(par->regs->status, ((par->clk_sel << 6) | (func << 4)) | 1);
+
+ info->fix.line_length = (info->var.xres_virtual*info->var.bits_per_pixel)/8;
+ return 0;
+}
+
+static int grvga_setcolreg(unsigned regno, unsigned red, unsigned green, unsigned blue, unsigned transp, struct fb_info *info)
+{
+ struct grvga_par *par;
+ par = info->par;
+
+ if (regno >= 256) /* Size of CLUT */
+ return -EINVAL;
+
+ if (info->var.grayscale) {
+ /* grayscale = 0.30*R + 0.59*G + 0.11*B */
+ red = green = blue = (red * 77 + green * 151 + blue * 28) >> 8;
+ }
+
+
+
+#define CNVT_TOHW(val, width) ((((val)<<(width))+0x7FFF-(val))>>16)
+
+ red = CNVT_TOHW(red, info->var.red.length);
+ green = CNVT_TOHW(green, info->var.green.length);
+ blue = CNVT_TOHW(blue, info->var.blue.length);
+ transp = CNVT_TOHW(transp, info->var.transp.length);
+
+#undef CNVT_TOHW
+
+ /* In PSEUDOCOLOR we use the hardware CLUT */
+ if (info->fix.visual = FB_VISUAL_PSEUDOCOLOR)
+ GRVGA_REGSAVE(par->regs->clut,
+ (regno << 24) | (red << 16) | (green << 8) | blue);
+
+ /* Truecolor uses the pseudo palette */
+ else if (info->fix.visual = FB_VISUAL_TRUECOLOR) {
+ u32 v;
+ if (regno >= 16)
+ return -EINVAL;
+
+
+ v = (red << info->var.red.offset) |
+ (green << info->var.green.offset) |
+ (blue << info->var.blue.offset) |
+ (transp << info->var.transp.offset);
+
+ ((u32 *) (info->pseudo_palette))[regno] = v;
+ }
+ return 0;
+}
+
+static int grvga_pan_display(struct fb_var_screeninfo *var,
+ struct fb_info *info)
+{
+ struct grvga_par *par = info->par;
+ struct fb_fix_screeninfo *fix = &info->fix;
+ u32 base_addr;
+
+ if (var->xoffset != 0)
+ return -EINVAL;
+
+ base_addr = fix->smem_start + (var->yoffset * fix->line_length);
+ base_addr &= ~3UL;
+
+ /* Set framebuffer base address */
+ GRVGA_REGSAVE(par->regs->fb_pos, base_addr);
+
+ return 0;
+}
+
+static int __init grvga_parse_custom(char *options,
+ struct fb_var_screeninfo *screendata)
+{
+ char *this_opt;
+ int count = 0;
+ if (!options || !*options)
+ return -1;
+
+ while ((this_opt = strsep(&options, " ")) != NULL) {
+ if (!*this_opt)
+ continue;
+
+ switch (count) {
+ case 0:
+ screendata->pixclock = simple_strtoul(this_opt, NULL, 0);
+ count++;
+ break;
+ case 1:
+ screendata->xres = screendata->xres_virtual = simple_strtoul(this_opt, NULL, 0);
+ count++;
+ break;
+ case 2:
+ screendata->right_margin = simple_strtoul(this_opt, NULL, 0);
+ count++;
+ break;
+ case 3:
+ screendata->hsync_len = simple_strtoul(this_opt, NULL, 0);
+ count++;
+ break;
+ case 4:
+ screendata->left_margin = simple_strtoul(this_opt, NULL, 0);
+ count++;
+ break;
+ case 5:
+ screendata->yres = screendata->yres_virtual = simple_strtoul(this_opt, NULL, 0);
+ count++;
+ break;
+ case 6:
+ screendata->lower_margin = simple_strtoul(this_opt, NULL, 0);
+ count++;
+ break;
+ case 7:
+ screendata->vsync_len = simple_strtoul(this_opt, NULL, 0);
+ count++;
+ break;
+ case 8:
+ screendata->upper_margin = simple_strtoul(this_opt, NULL, 0);
+ count++;
+ break;
+ case 9:
+ screendata->bits_per_pixel = simple_strtoul(this_opt, NULL, 0);
+ count++;
+ break;
+ }
+ }
+ screendata->activate = FB_ACTIVATE_NOW;
+ screendata->vmode = FB_VMODE_NONINTERLACED;
+ return 0;
+}
+
+static int __devinit grvga_probe(struct platform_device *dev)
+{
+ struct fb_info *info;
+ int retval = -ENOMEM;
+ unsigned long virtual_start;
+ unsigned long grvga_fix_addr = 0;
+ unsigned long physical_start = 0;
+ unsigned long grvga_mem_size = 0;
+ struct grvga_par *par;
+ char *options = NULL, *mode_opt = NULL;
+
+ info = framebuffer_alloc(sizeof(struct grvga_par), &dev->dev);
+ if (!info) {
+ dev_err(&dev->dev, "framebuffer_alloc failed\n");
+ return -ENOMEM;
+ }
+
+ /* Expecting: "grvga: modestring, [addr:<framebuffer physical address>], [size:<framebuffer size>]
+ *
+ * If modestring is custom:<custom mode string> we parse the string which then contains all videoparameters
+ * If address is left out, we allocate memory,
+ * if size is left out we only allocate enough to support the given mode.
+ */
+ if (fb_get_options("grvga", &options)) {
+ retval = -ENODEV;
+ goto err;
+ }
+
+ if (!options || !*options)
+ options = "640x480-8@60";
+
+ while (1) {
+ char *this_opt = strsep(&options, ",");
+
+ if (!this_opt)
+ break;
+
+ if (!strncmp(this_opt, "custom", 6))
+ grvga_parse_custom(this_opt, &info->var);
+ else if (!strncmp(this_opt, "addr:", 5))
+ grvga_fix_addr = simple_strtoul(this_opt + 5, NULL, 16);
+ else if (!strncmp(this_opt, "size:", 5))
+ grvga_mem_size = simple_strtoul(this_opt + 5, NULL, 0);
+ else
+ mode_opt = this_opt;
+ }
+
+ par = info->par;
+ info->fbops = &grvga_ops;
+ info->fix = grvga_fix;
+ info->pseudo_palette = par->color_palette;
+ info->flags = GRVGA_FBINFO_DEFAULT;
+ info->fix.smem_len = grvga_mem_size;
+
+ par->regs = of_ioremap(&dev->resource[0], 0,
+ resource_size(&dev->resource[0]),
+ "grlib-svgactrl regs");
+
+ retval = fb_alloc_cmap(&info->cmap, 256, 0);
+ if (retval < 0) {
+ dev_err(&dev->dev, "failed to allocate mem with fb_alloc_cmap\n");
+ retval = -ENOMEM;
+ goto err1;
+ }
+
+ if (mode_opt) {
+ retval = fb_find_mode(&info->var, info, mode_opt,
+ grvga_modedb, sizeof(grvga_modedb), &grvga_modedb[0], 8);
+ if (!retval || retval = 4) {
+ retval = -EINVAL;
+ goto err2;
+ }
+ }
+
+ if (!grvga_mem_size)
+ grvga_mem_size = info->var.xres_virtual * info->var.yres_virtual * info->var.bits_per_pixel/8;
+
+ if (grvga_fix_addr) {
+ /* Got framebuffer base address from argument list */
+
+ physical_start = grvga_fix_addr;
+
+ if (!request_mem_region(physical_start, grvga_mem_size, dev->name)) {
+ dev_err(&dev->dev, "request mem region failed\n");
+ retval = -ENOMEM;
+ goto err2;
+ }
+
+ virtual_start = (unsigned long) ioremap(physical_start, grvga_mem_size);
+
+ if (!virtual_start) {
+ dev_err(&dev->dev, "error mapping memory\n");
+ retval = -ENOMEM;
+ goto err3;
+ }
+ } else { /* Allocate frambuffer memory */
+
+ unsigned long page;
+
+ virtual_start = (unsigned long) __get_free_pages(GFP_ATOMIC | GFP_DMA,
+ get_order(grvga_mem_size));
+ if (!virtual_start) {
+ dev_err(&dev->dev,
+ "unable to allocate framebuffer memory (%lu bytes)\n",
+ grvga_mem_size);
+ retval = -ENOMEM;
+ goto err2;
+ }
+
+
+ physical_start = __pa(virtual_start);
+
+ /* Set page reserved so that mmap will work. This is necessary
+ * since we'll be remapping normal memory.
+ */
+ for (page = virtual_start;
+ page < PAGE_ALIGN(virtual_start + grvga_mem_size);
+ page += PAGE_SIZE) {
+ SetPageReserved(virt_to_page(page));
+ }
+ }
+
+ memset((unsigned long *) virtual_start, 0, grvga_mem_size);
+
+ info->screen_base = (char __iomem *) virtual_start;
+ info->fix.smem_start = physical_start;
+ info->fix.smem_len = grvga_mem_size;
+
+ dev_set_drvdata(&dev->dev, info);
+
+ dev_info(&dev->dev,
+ "Aeroflex Gaisler framebuffer device (fb%d), %dx%d-%d, using %luK of video memory @ %p\n",
+ info->node, info->var.xres, info->var.yres, info->var.bits_per_pixel,
+ grvga_mem_size >> 10, info->screen_base);
+
+ retval = register_framebuffer(info);
+ if (retval < 0) {
+ dev_err(&dev->dev, "failed to register framebuffer\n");
+ if (grvga_fix_addr)
+ goto err3;
+ else {
+ kfree((void *)virtual_start);
+ goto err2;
+ }
+ }
+
+ GRVGA_REGSAVE(par->regs->fb_pos, physical_start);
+ GRVGA_REGORIN(par->regs->status, 1); /* Enable framebuffer */
+
+ return 0;
+
+err3:
+ release_mem_region(physical_start, grvga_mem_size);
+err2:
+ fb_dealloc_cmap(&info->cmap);
+err1:
+ of_iounmap(&dev->resource[0], par->regs,
+ resource_size(&dev->resource[0]));
+err:
+ framebuffer_release(info);
+
+ return retval;
+}
+
+static int __devexit grvga_remove(struct platform_device *device)
+{
+ struct fb_info *info = dev_get_drvdata(&device->dev);
+ struct grvga_par *par = info->par;
+
+ if (info) {
+ unregister_framebuffer(info);
+ fb_dealloc_cmap(&info->cmap);
+ of_iounmap(&device->resource[0], par->regs,
+ resource_size(&device->resource[0]));
+ framebuffer_release(info);
+ dev_set_drvdata(&device->dev, NULL);
+ }
+
+ return 0;
+}
+
+static struct of_device_id svgactrl_of_match[] = {
+ {
+ .name = "GAISLER_SVGACTRL",
+ },
+ {
+ .name = "01_063",
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, svgactrl_of_match);
+
+static struct platform_driver grvga_driver = {
+ .driver = {
+ .name = "grlib-svgactrl",
+ .owner = THIS_MODULE,
+ .of_match_table = svgactrl_of_match,
+ },
+ .probe = grvga_probe,
+ .remove = __devexit_p(grvga_remove),
+};
+
+
+int __init grvga_init(void)
+{
+ return platform_driver_register(&grvga_driver);
+}
+
+static void __exit grvga_exit(void)
+{
+ platform_driver_unregister(&grvga_driver);
+}
+
+module_init(grvga_init);
+module_exit(grvga_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Aeroflex Gaisler");
+MODULE_DESCRIPTION("Aeroflex Gaisler framebuffer device driver");
--
1.6.4.1
^ permalink raw reply related
* Re: [PATCH] video: Add GRVGA framebuffer device driver
From: Kristoffer Glembo @ 2011-06-15 11:52 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1308128180-14645-1-git-send-email-kristoffer@gaisler.com>
Hi,
Geert Uytterhoeven wrote:
> At first sight, nothing in this driver seems to be SPARC-specific, so
> perhaps this can be relaxed
> to e.g. depends on OF_DEVICE?
>
It uses of_ioremap/unmap which are SPARC-specific. I have been bitten by things like this before
when thinking I was portable so this time I just depended on SPARC. The core is not used on any
other platforms and is not very likely to be used either.
> Please remove the cast and use %p to format the address.
Will do.
Thanks,
Kristoffer Glembo
^ permalink raw reply
* Re: [PATCH] video: Add GRVGA framebuffer device driver
From: Geert Uytterhoeven @ 2011-06-15 11:32 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1308128180-14645-1-git-send-email-kristoffer@gaisler.com>
On Wed, Jun 15, 2011 at 10:56, Kristoffer Glembo <kristoffer@gaisler.com> wrote:
> This patch adds support for the GRVGA framebuffer IP core from Aeroflex Gaisler.
> The device is used in LEON SPARCV8 based System on Chips. Documentation can
> be found here: www.gaisler.com/products/grlib/grip.pdf.
>
> Signed-off-by: Kristoffer Glembo <kristoffer@gaisler.com>
> ---
> drivers/video/Kconfig | 10 +
> drivers/video/Makefile | 1 +
> drivers/video/grvga.c | 559 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 570 insertions(+), 0 deletions(-)
> create mode 100644 drivers/video/grvga.c
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 549b960..18ee201 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -259,6 +259,16 @@ config FB_TILEBLITTING
> comment "Frame buffer hardware drivers"
> depends on FB
>
> +config FB_GRVGA
> + tristate "Aeroflex Gaisler framebuffer support"
> + depends on FB && SPARC
At first sight, nothing in this driver seems to be SPARC-specific, so
perhaps this can be relaxed
to e.g. depends on OF_DEVICE?
> + select FB_CFB_FILLRECT
> + select FB_CFB_COPYAREA
> + select FB_CFB_IMAGEBLIT
> + ---help---
> + This enables support for the SVGACTRL framebuffer in the GRLIB IP library from Aeroflex Gaisler.
> +
> +
> config FB_CIRRUS
> tristate "Cirrus Logic support"
> depends on FB && (ZORRO || PCI)
> --- /dev/null
> +++ b/drivers/video/grvga.c
> + dev_info(&dev->dev,
> + "Aeroflex Gaisler framebuffer device (fb%d), %dx%d-%d, using %luK of video memory @ 0x%x\n",
> + info->node, info->var.xres, info->var.yres, info->var.bits_per_pixel,
> + grvga_mem_size >> 10, (unsigned int) info->screen_base);
^^^^^^^^^^^^^^
Please remove the cast and use %p to format the address.
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
^ permalink raw reply
* Re: [PATCHv2 15/28] OMAP4: HWMOD: Modify DSS opt clocks
From: Tomi Valkeinen @ 2011-06-15 11:23 UTC (permalink / raw)
To: b-cousson; +Cc: linux-fbdev, paul, khilman, linux-omap mailing list
In-Reply-To: <1307627810-3768-16-git-send-email-tomi.valkeinen@ti.com>
On Thu, 2011-06-09 at 16:56 +0300, Tomi Valkeinen wrote:
> Add missing DSS optional clocks to HWMOD data for OMAP4xxx.
>
> Add HWMOD_CONTROL_OPT_CLKS_IN_RESET flag for dispc to fix dispc reset.
>
> Cc: Benoit Cousson <b-cousson@ti.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Benoit (and/or Paul/Kevin), can you ack this and the other HWMOD
patches, or give comments?
Tomi
^ permalink raw reply
* Re: Possible deadlock when suspending framebuffer
From: Bruno Prémont @ 2011-06-15 10:20 UTC (permalink / raw)
To: Francis Moreau
Cc: Wanlong Gao, Paul Mundt, linux-fbdev, Linux Kernel Mailing List,
Linus Torvalds
In-Reply-To: <BANLkTinE+EZ9nr1qGKC0VY0bVHuF8U-R0w@mail.gmail.com>
On Wed, 15 Jun 2011 09:12:46 Francis Moreau wrote:
> On Wed, Jun 15, 2011 at 7:58 AM, Bruno Prémont wrote:
> Well, sorry for the dumb question but the fb/fbcon code is pretty hard
> to follow for me.
Certainly not just for you
> Why does store_fbstate() and any fb driver's suspsend methods acquire
> the console lock at all ?
From my understanding, fbcon currently has very loose binding with
framebuffers in general instead of just with those few framebuffers
it is effectively mapped to (and active on!).
James Simmons started a complete rework of fbcon/tty code which is
expected to get things more fine-grained, until then console semaphore
(console_lock) remains a kind of big kernel lock in the
console/framebuffer area.
As such any state change of framebuffer may influence/race against
fbcon.
e.g. for the suspend state you want to avoid fbcon to fiddle with
your framebuffer while it is suspending (and have fbcon know about the
suspended state). This way fbcon can unsuspend framebuffer if needed
but also stop accessing it when it should not.
Bruno
^ permalink raw reply
* Re: [PATCHv2 01/28] OMAP: change get_context_loss_count ret value
From: Rajendra Nayak @ 2011-06-15 9:19 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Paul Walmsley, linux-fbdev, b-cousson, khilman,
linux-omap mailing list
In-Reply-To: <4DF76830.8070108@ti.com>
On 6/14/2011 7:24 PM, Rajendra Nayak wrote:
> On 6/14/2011 12:54 PM, Tomi Valkeinen wrote:
>> On Tue, 2011-06-14 at 01:13 -0600, Paul Walmsley wrote:
>>> Hi Tomi
>>>
>>> On Mon, 13 Jun 2011, Tomi Valkeinen wrote:
>>>
>>>> Paul, can you take this patch and queue it for an rc?
>>>
>>> Generally I only queue regressions or fixes for major problems (crashes,
>>> corruption, etc.) for -rc series. So probably this one should go in via
>>> the normal merge window, unless it's been causing major disruptions?
>>
>> No, only disruptions for me as the DSS pm_runtime patches depend on this
>> one to function correctly. So merge window is ok, I'll handle the DSS
>> side somehow.
>
> Hi Paul/Kevin,
>
> I had a query, not directly related to this patch, but to the way
> the omap_pm_get_dev_context_loss_count() api is implemented, which
> this patch is trying to fix in some ways.
> I see that the api relies on the pwrdm level state counters, which
> in-turn seem to be getting updated only in the cpuidle/suspend path.
> How are domains like DSS which can independently transition outside
> of the cpuidle path handled?
Thinking some more on this, maybe I now understand how this worked
on OMAP3. We always had the 'autodeps' on OMAP3 which made sure no
clkdm idle's while MPU is not in standby, and hence all transitions
would always happen between the pwrdm_pre_transition() and
pwrdm_post_transition() (where the pwrdm level state counters get
cleared/updated) callbacks. So there were really no domain
transitions outside of this on OMAP3.
My questions were popping out from the work I was trying to do to
support this on OMAP4, and with no 'autodeps' on OMAP4 there will
be transitions outside of cpuidle/suspend where counters need
to be updated/cleared which at this point I have no clue how to
handle :(
I will start a separate discussion/thread on this since
this is probably not the right place to discuss on how to do
this on OMAP4.
Thanks,
Rajendra
> What I mean is, if DSS on disabling its clocks transitions to OFF
> state (it being an independent powerdomain) and tries to use this api
> to know if it lost context the next time it is re-enabling clocks and
> all this happens while there was no cpuidle being scheduled, where do
> the pwrdm level state counters get updated, which tell DSS it did lose
> context?
>
> On another note, i was wondering if it even made any sense to drivers
> like DSS, which have an independent power domain of its own on OMAP to
> try and do a restore-only-if-needed kind of an implementation.
> Would'nt it always lose context the moment it run-time idle's?
>
> regards,
> Rajendra
>
>>
>> Tomi
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* [PATCH] video: Add GRVGA framebuffer device driver
From: Kristoffer Glembo @ 2011-06-15 8:56 UTC (permalink / raw)
To: linux-fbdev
This patch adds support for the GRVGA framebuffer IP core from Aeroflex Gaisler.
The device is used in LEON SPARCV8 based System on Chips. Documentation can
be found here: www.gaisler.com/products/grlib/grip.pdf.
Signed-off-by: Kristoffer Glembo <kristoffer@gaisler.com>
---
drivers/video/Kconfig | 10 +
drivers/video/Makefile | 1 +
drivers/video/grvga.c | 559 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 570 insertions(+), 0 deletions(-)
create mode 100644 drivers/video/grvga.c
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 549b960..18ee201 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -259,6 +259,16 @@ config FB_TILEBLITTING
comment "Frame buffer hardware drivers"
depends on FB
+config FB_GRVGA
+ tristate "Aeroflex Gaisler framebuffer support"
+ depends on FB && SPARC
+ select FB_CFB_FILLRECT
+ select FB_CFB_COPYAREA
+ select FB_CFB_IMAGEBLIT
+ ---help---
+ This enables support for the SVGACTRL framebuffer in the GRLIB IP library from Aeroflex Gaisler.
+
+
config FB_CIRRUS
tristate "Cirrus Logic support"
depends on FB && (ZORRO || PCI)
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 8b83129..4cff5ec 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_FB_DEFERRED_IO) += fb_defio.o
obj-$(CONFIG_FB_WMT_GE_ROPS) += wmt_ge_rops.o
# Hardware specific drivers go first
+obj-$(CONFIG_FB_GRVGA) += grvga.o
obj-$(CONFIG_FB_AMIGA) += amifb.o c2p_planar.o
obj-$(CONFIG_FB_ARC) += arcfb.o
obj-$(CONFIG_FB_CLPS711X) += clps711xfb.o
diff --git a/drivers/video/grvga.c b/drivers/video/grvga.c
new file mode 100644
index 0000000..e1514f9
--- /dev/null
+++ b/drivers/video/grvga.c
@@ -0,0 +1,559 @@
+/*
+ * Driver for Aeroflex Gaisler SVGACTRL framebuffer device.
+ *
+ * 2011 (c) Aeroflex Gaisler AB
+ *
+ * Full documentation of the core can be found here:
+ * http://www.gaisler.com/products/grlib/grip.pdf
+ *
+ * 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.
+ *
+ * Contributors: Kristoffer Glembo <kristoffer@gaisler.com>
+ *
+ */
+
+#include <linux/platform_device.h>
+#include <linux/of_platform.h>
+#include <linux/of_device.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/string.h>
+#include <linux/delay.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/tty.h>
+#include <linux/mm.h>
+#include <linux/fb.h>
+#include <linux/io.h>
+
+#define GRVGA_REGLOAD(a) (__raw_readl(&(a)))
+#define GRVGA_REGSAVE(a, v) (__raw_writel(v, &(a)))
+#define GRVGA_REGORIN(a, v) (GRVGA_REGSAVE(a, (GRVGA_REGLOAD(a) | (v))))
+#define GRVGA_REGANDIN(a, v) (GRVGA_REGSAVE(a, (GRVGA_REGLOAD(a) & (v))))
+
+#define GRVGA_FBINFO_DEFAULT (FBINFO_DEFAULT \
+ | FBINFO_PARTIAL_PAN_OK \
+ | FBINFO_HWACCEL_YPAN)
+
+struct grvga_regs {
+ u32 status; /* 0x00 */
+ u32 video_length; /* 0x04 */
+ u32 front_porch; /* 0x08 */
+ u32 sync_length; /* 0x0C */
+ u32 line_length; /* 0x10 */
+ u32 fb_pos; /* 0x14 */
+ u32 clk_vector[4]; /* 0x18 */
+ u32 clut; /* 0x20 */
+};
+
+struct grvga_par {
+ struct grvga_regs *regs;
+ u32 color_palette[16]; /* 16 entry pseudo palette used by fbcon in true color mode */
+ int clk_sel;
+};
+
+
+static const struct fb_videomode grvga_modedb[] = {
+ {
+ /* 640x480 @ 60 Hz */
+ NULL, 60, 640, 480, 40000, 48, 16, 39, 11, 96, 2,
+ 0, FB_VMODE_NONINTERLACED
+ }, {
+ /* 800x600 @ 60 Hz */
+ NULL, 60, 800, 600, 25000, 88, 40, 23, 1, 128, 4,
+ 0, FB_VMODE_NONINTERLACED
+ }, {
+ /* 800x600 @ 72 Hz */
+ NULL, 72, 800, 600, 20000, 64, 56, 23, 37, 120, 6,
+ 0, FB_VMODE_NONINTERLACED
+ }, {
+ /* 1024x768 @ 60 Hz */
+ NULL, 60, 1024, 768, 15385, 160, 24, 29, 3, 136, 6,
+ 0, FB_VMODE_NONINTERLACED
+ }
+ };
+
+static struct fb_fix_screeninfo grvga_fix __initdata = {
+ .id = "AG SVGACTRL",
+ .type = FB_TYPE_PACKED_PIXELS,
+ .visual = FB_VISUAL_PSEUDOCOLOR,
+ .xpanstep = 0,
+ .ypanstep = 1,
+ .ywrapstep = 0,
+ .accel = FB_ACCEL_NONE,
+};
+
+
+static int grvga_check_var(struct fb_var_screeninfo *var,
+ struct fb_info *info);
+static int grvga_set_par(struct fb_info *info);
+static int grvga_setcolreg(unsigned regno,
+ unsigned red, unsigned green, unsigned blue, unsigned transp,
+ struct fb_info *info);
+static int grvga_pan_display(struct fb_var_screeninfo *var, struct fb_info *info);
+
+static struct fb_ops grvga_ops = {
+ .owner = THIS_MODULE,
+ .fb_check_var = grvga_check_var,
+ .fb_set_par = grvga_set_par,
+ .fb_setcolreg = grvga_setcolreg,
+ .fb_pan_display = grvga_pan_display,
+ .fb_fillrect = cfb_fillrect,
+ .fb_copyarea = cfb_copyarea,
+ .fb_imageblit = cfb_imageblit
+};
+
+static int grvga_check_var(struct fb_var_screeninfo *var,
+ struct fb_info *info)
+{
+ struct grvga_par *par = info->par;
+ int i;
+
+ if (!var->xres)
+ var->xres = 1;
+ if (!var->yres)
+ var->yres = 1;
+ if (var->bits_per_pixel <= 8)
+ var->bits_per_pixel = 8;
+ else if (var->bits_per_pixel <= 16)
+ var->bits_per_pixel = 16;
+ else if (var->bits_per_pixel <= 24)
+ var->bits_per_pixel = 24;
+ else if (var->bits_per_pixel <= 32)
+ var->bits_per_pixel = 32;
+ else
+ return -EINVAL;
+
+ var->xres_virtual = var->xres;
+ var->yres_virtual = 2*var->yres;
+
+ if (info->fix.smem_len) {
+ if ((var->yres_virtual*var->xres_virtual*var->bits_per_pixel/8) > info->fix.smem_len)
+ return -ENOMEM;
+ }
+
+ /* Which clocks that are available can be read out in these registers */
+ for (i = 0; i <= 3 ; i++) {
+ if (var->pixclock = par->regs->clk_vector[i])
+ break;
+ }
+ if (i != -1)
+ par->clk_sel = i;
+ else
+ return -EINVAL;
+
+ switch (info->var.bits_per_pixel) {
+ case 8:
+ var->red = (struct fb_bitfield) {0, 8, 0}; /* offset, length, msb-right */
+ var->green = (struct fb_bitfield) {0, 8, 0};
+ var->blue = (struct fb_bitfield) {0, 8, 0};
+ var->transp = (struct fb_bitfield) {0, 0, 0};
+ break;
+ case 16:
+ var->red = (struct fb_bitfield) {11, 5, 0};
+ var->green = (struct fb_bitfield) {5, 6, 0};
+ var->blue = (struct fb_bitfield) {0, 5, 0};
+ var->transp = (struct fb_bitfield) {0, 0, 0};
+ break;
+ case 24:
+ case 32:
+ var->red = (struct fb_bitfield) {16, 8, 0};
+ var->green = (struct fb_bitfield) {8, 8, 0};
+ var->blue = (struct fb_bitfield) {0, 8, 0};
+ var->transp = (struct fb_bitfield) {24, 8, 0};
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int grvga_set_par(struct fb_info *info)
+{
+
+ int func = 0;
+ struct grvga_par *par = info->par;
+
+ GRVGA_REGSAVE(par->regs->video_length,
+ ((info->var.yres - 1) << 16) | (info->var.xres - 1));
+ GRVGA_REGSAVE(par->regs->front_porch,
+ (info->var.lower_margin << 16) | (info->var.right_margin));
+ GRVGA_REGSAVE(par->regs->sync_length,
+ (info->var.vsync_len << 16) | (info->var.hsync_len));
+ GRVGA_REGSAVE(par->regs->line_length,
+ ((info->var.yres + info->var.lower_margin + info->var.upper_margin + info->var.vsync_len - 1) << 16) |
+ (info->var.xres + info->var.right_margin + info->var.left_margin + info->var.hsync_len - 1));
+
+ switch (info->var.bits_per_pixel) {
+ case 8:
+ info->fix.visual = FB_VISUAL_PSEUDOCOLOR;
+ func = 1;
+ break;
+ case 16:
+ info->fix.visual = FB_VISUAL_TRUECOLOR;
+ func = 2;
+ break;
+ case 24:
+ case 32:
+ info->fix.visual = FB_VISUAL_TRUECOLOR;
+ func = 3;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ GRVGA_REGSAVE(par->regs->status, ((par->clk_sel << 6) | (func << 4)) | 1);
+
+ info->fix.line_length = (info->var.xres_virtual*info->var.bits_per_pixel)/8;
+ return 0;
+}
+
+static int grvga_setcolreg(unsigned regno, unsigned red, unsigned green, unsigned blue, unsigned transp, struct fb_info *info)
+{
+ struct grvga_par *par;
+ par = info->par;
+
+ if (regno >= 256) /* Size of CLUT */
+ return -EINVAL;
+
+ if (info->var.grayscale) {
+ /* grayscale = 0.30*R + 0.59*G + 0.11*B */
+ red = green = blue = (red * 77 + green * 151 + blue * 28) >> 8;
+ }
+
+
+
+#define CNVT_TOHW(val, width) ((((val)<<(width))+0x7FFF-(val))>>16)
+
+ red = CNVT_TOHW(red, info->var.red.length);
+ green = CNVT_TOHW(green, info->var.green.length);
+ blue = CNVT_TOHW(blue, info->var.blue.length);
+ transp = CNVT_TOHW(transp, info->var.transp.length);
+
+#undef CNVT_TOHW
+
+ /* In PSEUDOCOLOR we use the hardware CLUT */
+ if (info->fix.visual = FB_VISUAL_PSEUDOCOLOR)
+ GRVGA_REGSAVE(par->regs->clut,
+ (regno << 24) | (red << 16) | (green << 8) | blue);
+
+ /* Truecolor uses the pseudo palette */
+ else if (info->fix.visual = FB_VISUAL_TRUECOLOR) {
+ u32 v;
+ if (regno >= 16)
+ return -EINVAL;
+
+
+ v = (red << info->var.red.offset) |
+ (green << info->var.green.offset) |
+ (blue << info->var.blue.offset) |
+ (transp << info->var.transp.offset);
+
+ ((u32 *) (info->pseudo_palette))[regno] = v;
+ }
+ return 0;
+}
+
+static int grvga_pan_display(struct fb_var_screeninfo *var,
+ struct fb_info *info)
+{
+ struct grvga_par *par = info->par;
+ struct fb_fix_screeninfo *fix = &info->fix;
+ u32 base_addr;
+
+ if (var->xoffset != 0)
+ return -EINVAL;
+
+ base_addr = fix->smem_start + (var->yoffset * fix->line_length);
+ base_addr &= ~3UL;
+
+ /* Set framebuffer base address */
+ GRVGA_REGSAVE(par->regs->fb_pos, base_addr);
+
+ return 0;
+}
+
+static int __init grvga_parse_custom(char *options,
+ struct fb_var_screeninfo *screendata)
+{
+ char *this_opt;
+ int count = 0;
+ if (!options || !*options)
+ return -1;
+
+ while ((this_opt = strsep(&options, " ")) != NULL) {
+ if (!*this_opt)
+ continue;
+
+ switch (count) {
+ case 0:
+ screendata->pixclock = simple_strtoul(this_opt, NULL, 0);
+ count++;
+ break;
+ case 1:
+ screendata->xres = screendata->xres_virtual = simple_strtoul(this_opt, NULL, 0);
+ count++;
+ break;
+ case 2:
+ screendata->right_margin = simple_strtoul(this_opt, NULL, 0);
+ count++;
+ break;
+ case 3:
+ screendata->hsync_len = simple_strtoul(this_opt, NULL, 0);
+ count++;
+ break;
+ case 4:
+ screendata->left_margin = simple_strtoul(this_opt, NULL, 0);
+ count++;
+ break;
+ case 5:
+ screendata->yres = screendata->yres_virtual = simple_strtoul(this_opt, NULL, 0);
+ count++;
+ break;
+ case 6:
+ screendata->lower_margin = simple_strtoul(this_opt, NULL, 0);
+ count++;
+ break;
+ case 7:
+ screendata->vsync_len = simple_strtoul(this_opt, NULL, 0);
+ count++;
+ break;
+ case 8:
+ screendata->upper_margin = simple_strtoul(this_opt, NULL, 0);
+ count++;
+ break;
+ case 9:
+ screendata->bits_per_pixel = simple_strtoul(this_opt, NULL, 0);
+ count++;
+ break;
+ }
+ }
+ screendata->activate = FB_ACTIVATE_NOW;
+ screendata->vmode = FB_VMODE_NONINTERLACED;
+ return 0;
+}
+
+static int __devinit grvga_probe(struct platform_device *dev)
+{
+ struct fb_info *info;
+ int retval = -ENOMEM;
+ unsigned long virtual_start;
+ unsigned long grvga_fix_addr = 0;
+ unsigned long physical_start = 0;
+ unsigned long grvga_mem_size = 0;
+ struct grvga_par *par;
+ char *options = NULL, *mode_opt = NULL;
+
+ info = framebuffer_alloc(sizeof(struct grvga_par), &dev->dev);
+ if (!info) {
+ dev_err(&dev->dev, "framebuffer_alloc failed\n");
+ return -ENOMEM;
+ }
+
+ /* Expecting: "grvga: modestring, [addr:<framebuffer physical address>], [size:<framebuffer size>]
+ *
+ * If modestring is custom:<custom mode string> we parse the string which then contains all videoparameters
+ * If address is left out, we allocate memory,
+ * if size is left out we only allocate enough to support the given mode.
+ */
+ if (fb_get_options("grvga", &options)) {
+ retval = -ENODEV;
+ goto err;
+ }
+
+ if (!options || !*options)
+ options = "640x480-8@60";
+
+ while (1) {
+ char *this_opt = strsep(&options, ",");
+
+ if (!this_opt)
+ break;
+
+ if (!strncmp(this_opt, "custom", 6))
+ grvga_parse_custom(this_opt, &info->var);
+ else if (!strncmp(this_opt, "addr:", 5))
+ grvga_fix_addr = simple_strtoul(this_opt + 5, NULL, 16);
+ else if (!strncmp(this_opt, "size:", 5))
+ grvga_mem_size = simple_strtoul(this_opt + 5, NULL, 0);
+ else
+ mode_opt = this_opt;
+ }
+
+ par = info->par;
+ info->fbops = &grvga_ops;
+ info->fix = grvga_fix;
+ info->pseudo_palette = par->color_palette;
+ info->flags = GRVGA_FBINFO_DEFAULT;
+ info->fix.smem_len = grvga_mem_size;
+
+ par->regs = of_ioremap(&dev->resource[0], 0,
+ resource_size(&dev->resource[0]),
+ "grlib-svgactrl regs");
+
+ retval = fb_alloc_cmap(&info->cmap, 256, 0);
+ if (retval < 0) {
+ dev_err(&dev->dev, "failed to allocate mem with fb_alloc_cmap\n");
+ retval = -ENOMEM;
+ goto err1;
+ }
+
+ if (mode_opt) {
+ retval = fb_find_mode(&info->var, info, mode_opt,
+ grvga_modedb, sizeof(grvga_modedb), &grvga_modedb[0], 8);
+ if (!retval || retval = 4) {
+ retval = -EINVAL;
+ goto err2;
+ }
+ }
+
+ if (!grvga_mem_size)
+ grvga_mem_size = info->var.xres_virtual * info->var.yres_virtual * info->var.bits_per_pixel/8;
+
+ if (grvga_fix_addr) {
+ /* Got framebuffer base address from argument list */
+
+ physical_start = grvga_fix_addr;
+
+ if (!request_mem_region(physical_start, grvga_mem_size, dev->name)) {
+ dev_err(&dev->dev, "request mem region failed\n");
+ retval = -ENOMEM;
+ goto err2;
+ }
+
+ virtual_start = (unsigned long) ioremap(physical_start, grvga_mem_size);
+
+ if (!virtual_start) {
+ dev_err(&dev->dev, "error mapping memory\n");
+ retval = -ENOMEM;
+ goto err3;
+ }
+ } else { /* Allocate frambuffer memory */
+
+ unsigned long page;
+
+ virtual_start = (unsigned long) __get_free_pages(GFP_ATOMIC | GFP_DMA,
+ get_order(grvga_mem_size));
+ if (!virtual_start) {
+ dev_err(&dev->dev,
+ "unable to allocate framebuffer memory (%lu bytes)\n",
+ grvga_mem_size);
+ retval = -ENOMEM;
+ goto err2;
+ }
+
+
+ physical_start = __pa(virtual_start);
+
+ /* Set page reserved so that mmap will work. This is necessary
+ * since we'll be remapping normal memory.
+ */
+ for (page = virtual_start;
+ page < PAGE_ALIGN(virtual_start + grvga_mem_size);
+ page += PAGE_SIZE) {
+ SetPageReserved(virt_to_page(page));
+ }
+ }
+
+ memset((unsigned long *) virtual_start, 0, grvga_mem_size);
+
+ info->screen_base = (char __iomem *) virtual_start;
+ info->fix.smem_start = physical_start;
+ info->fix.smem_len = grvga_mem_size;
+
+ dev_set_drvdata(&dev->dev, info);
+
+ dev_info(&dev->dev,
+ "Aeroflex Gaisler framebuffer device (fb%d), %dx%d-%d, using %luK of video memory @ 0x%x\n",
+ info->node, info->var.xres, info->var.yres, info->var.bits_per_pixel,
+ grvga_mem_size >> 10, (unsigned int) info->screen_base);
+
+ retval = register_framebuffer(info);
+ if (retval < 0) {
+ dev_err(&dev->dev, "failed to register framebuffer\n");
+ if (grvga_fix_addr)
+ goto err3;
+ else {
+ kfree((void *)virtual_start);
+ goto err2;
+ }
+ }
+
+ GRVGA_REGSAVE(par->regs->fb_pos, physical_start);
+ GRVGA_REGORIN(par->regs->status, 1); /* Enable framebuffer */
+
+ return 0;
+
+err3:
+ release_mem_region(physical_start, grvga_mem_size);
+err2:
+ fb_dealloc_cmap(&info->cmap);
+err1:
+ of_iounmap(&dev->resource[0], par->regs,
+ resource_size(&dev->resource[0]));
+err:
+ framebuffer_release(info);
+
+ return retval;
+}
+
+static int __devexit grvga_remove(struct platform_device *device)
+{
+ struct fb_info *info = dev_get_drvdata(&device->dev);
+ struct grvga_par *par = info->par;
+
+ if (info) {
+ unregister_framebuffer(info);
+ fb_dealloc_cmap(&info->cmap);
+ of_iounmap(&device->resource[0], par->regs,
+ resource_size(&device->resource[0]));
+ framebuffer_release(info);
+ dev_set_drvdata(&device->dev, NULL);
+ }
+
+ return 0;
+}
+
+static struct of_device_id svgactrl_of_match[] = {
+ {
+ .name = "GAISLER_SVGACTRL",
+ },
+ {
+ .name = "01_063",
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, svgactrl_of_match);
+
+static struct platform_driver grvga_driver = {
+ .driver = {
+ .name = "grlib-svgactrl",
+ .owner = THIS_MODULE,
+ .of_match_table = svgactrl_of_match,
+ },
+ .probe = grvga_probe,
+ .remove = __devexit_p(grvga_remove),
+};
+
+
+int __init grvga_init(void)
+{
+ return platform_driver_register(&grvga_driver);
+}
+
+static void __exit grvga_exit(void)
+{
+ platform_driver_unregister(&grvga_driver);
+}
+
+module_init(grvga_init);
+module_exit(grvga_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Aeroflex Gaisler");
+MODULE_DESCRIPTION("Aeroflex Gaisler framebuffer device driver");
--
1.6.4.1
^ permalink raw reply related
* Re: Possible deadlock when suspending framebuffer
From: Francis Moreau @ 2011-06-15 7:12 UTC (permalink / raw)
To: Bruno Prémont
Cc: Wanlong Gao, Paul Mundt, linux-fbdev, Linux Kernel Mailing List,
Linus Torvalds
In-Reply-To: <20110615075803.3348b4ec@pluto.restena.lu>
On Wed, Jun 15, 2011 at 7:58 AM, Bruno Prémont
<bonbons@linux-vserver.org> wrote:
> Hi,
>
> On Wed, 15 Jun 2011 09:09:24 Wanlong Gao <wanlong.gao@gmail.com> wrote:
>> <snip>
>> Hi Francis:
>> can you test this patch?
>
> Do you have a deadlock trace which you are trying to fix?
>
> It's either the caller of unregister_framebuffer() which must be
> changed to not call unregister_framebuffer with info's lock held or
> the code reacting on the notification that must not try to acquire the
> lock again.
>
> The interesting par is if console semaphore has some relation to this
> deadlock as the order for taking both varies... It could be
> lock_fb_info(); console_lock() versus console_lock(); lock_fb_info()
>
> Bruno
>
>
>> Thanks
>>
>> From fe026c42af4cbdce053460a428a445e99071586a Mon Sep 17 00:00:00 2001
>> From: Wanlong Gao <wanlong.gao@gmail.com>
>> Date: Wed, 15 Jun 2011 09:03:41 +0800
>> Subject: [PATCH] test
>>
>>
>>
>> Signed-off-by: Wanlong Gao <wanlong.gao@gmail.com>
>> ---
>> drivers/video/fbmem.c | 3 ---
>> 1 files changed, 0 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
>> index 5aac00e..6e6cef3 100644
>> --- a/drivers/video/fbmem.c
>> +++ b/drivers/video/fbmem.c
>> @@ -1642,11 +1642,8 @@ static int do_unregister_framebuffer(struct
>> fb_info *fb_info)
>> if (i < 0 || i >= FB_MAX || registered_fb[i] != fb_info)
>> return -EINVAL;
>>
>> - if (!lock_fb_info(fb_info))
>> - return -ENODEV;
>> event.info = fb_info;
>> ret = fb_notifier_call_chain(FB_EVENT_FB_UNBIND, &event);
>> - unlock_fb_info(fb_info);
>
> Not a good idea to stop taking fb_lock here.
> Pretty all calls of fb_notifier_call_chain are protected by info's
> lock, except the one for FB_EVENT_FB_UNREGISTERED a few lines further.
>
> IMHO it wou make sense to add the lock around that last one so all
> notifier chain calls are handled the same.
>
>> if (ret)
>> return -EINVAL;
>
>
Well, sorry for the dumb question but the fb/fbcon code is pretty hard
to follow for me.
Why does store_fbstate() and any fb driver's suspsend methods acquire
the console lock at all ?
Thanks
--
Francis
^ permalink raw reply
* Re: Possible deadlock when suspending framebuffer
From: Américo Wang @ 2011-06-15 7:04 UTC (permalink / raw)
To: wanlong.gao
Cc: Bruno Prémont, Francis Moreau, Paul Mundt, linux-fbdev,
Linux Kernel Mailing List, Linus Torvalds
In-Reply-To: <BANLkTimiv5T6cgRjkDfPwjh-c5AivYzhmA@mail.gmail.com>
On Wed, Jun 15, 2011 at 2:22 PM, Wanlong Gao <wanlong.gao@gmail.com> wrote:
> On Wed, Jun 15, 2011 at 1:58 PM, Bruno Prémont
> <bonbons@linux-vserver.org> wrote:
>>
>> Hi,
>>
>> On Wed, 15 Jun 2011 09:09:24 Wanlong Gao <wanlong.gao@gmail.com> wrote:
>> > <snip>
>> > Hi Francis:
>> > can you test this patch?
>>
>> Do you have a deadlock trace which you are trying to fix?
> No, I just look at the code and try to fix this but I'm not sure.
> Can you teach me how to have a deadlock trace here?
CONFIG_LOCKDEP=y
^ permalink raw reply
* Re: Possible deadlock when suspending framebuffer
From: Wanlong Gao @ 2011-06-15 6:22 UTC (permalink / raw)
To: Bruno Prémont
Cc: Francis Moreau, Paul Mundt, linux-fbdev,
Linux Kernel Mailing List, Linus Torvalds
In-Reply-To: <20110615075803.3348b4ec@pluto.restena.lu>
On Wed, Jun 15, 2011 at 1:58 PM, Bruno Prémont
<bonbons@linux-vserver.org> wrote:
>
> Hi,
>
> On Wed, 15 Jun 2011 09:09:24 Wanlong Gao <wanlong.gao@gmail.com> wrote:
> > <snip>
> > Hi Francis:
> > can you test this patch?
>
> Do you have a deadlock trace which you are trying to fix?
No, I just look at the code and try to fix this but I'm not sure.
Can you teach me how to have a deadlock trace here?
Thanks
>
> It's either the caller of unregister_framebuffer() which must be
> changed to not call unregister_framebuffer with info's lock held or
> the code reacting on the notification that must not try to acquire the
> lock again.
>
> The interesting par is if console semaphore has some relation to this
> deadlock as the order for taking both varies... It could be
> lock_fb_info(); console_lock() versus console_lock(); lock_fb_info()
>
I see, thanks
> Bruno
>
>
> > Thanks
> >
><snip>
>
> Not a good idea to stop taking fb_lock here.
> Pretty all calls of fb_notifier_call_chain are protected by info's
> lock, except the one for FB_EVENT_FB_UNREGISTERED a few lines further.
Yup, thanks
>
> IMHO it wou make sense to add the lock around that last one so all
> notifier chain calls are handled the same.
>
> <snip>
>
--
Best regards
Wanlong Gao
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox