* Re: [RFC][PATCH 0/3] MERAM support for LCDC
From: Magnus Damm @ 2011-05-09 16:22 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1301369758-18394-1-git-send-email-dhobsong@igel.co.jp>
Hi Damian,
On Mon, May 9, 2011 at 4:03 PM, Damian Hobson-Garcia
<dhobsong@igel.co.jp> wrote:
> Hi Magnus,
>
>> Please add Runtime PM support to the MERAM driver. The MSTP113 bit of
>> SMSTPCR1 should be dynamically controlled using pm_runtime_get_sync()
>> and pm_runtime_put_sync().
>
> So one thing that I noticed while adding in the runtime PM support is
> that the MERAM bit in RMSTPCR1 seems to be enabled as a power on default
> on the chip. (I determined this by dumping the register from u-boot and
> poking around in the u-boot code to verify that u-boot doesn't seem to
> be enabling this).
>
> Even if pm_runtime_get_sync() and pm_runtime_put_sync() correctly enable
> and disable the SMSTPCR1, with the RMSTPCR1 enabled, the clock doesn't
> actually turn off.
>
> While I still think that its a good idea to add the
> pm_runtime_get_sync() and pm_runtime_put_sync() calls to do their thing
> on the SMSTPCR1 side, with the current configuration we won't actually
> be saving any power.
Good catch, yes, the MSTP bits need to be disabled on both sides for
the clocks to be stopped. I recall the default state of each MSTP bit
to vary somehow. I think we simply should add code to initialize all
SH-side MSTP bits as disabled, that's the easiest.
Thanks,
/ magnus
^ permalink raw reply
* [bug] drmfb does not set physical screen dimensions
From: Roger Leigh @ 2011-05-09 18:08 UTC (permalink / raw)
To: dri-devel, linux-fbdev, David Airlie
[-- Attachment #1: Type: text/plain, Size: 3331 bytes --]
Hi,
drivers/gpu/drm/drm_fb_helper.c is not setting width and height
in struct fb_var_screeninfo. drm_fb_helper_fill_var sets them to -1
rather than using the real values:
info->var.height = -1;
info->var.width = -1;
Since the physical dimensions are most likely known from the monitor
EDID, it would be ideal if this could be set here. If not, it might
be nice to assume a default of 96dpi and compute the size based upon
the current resolution (so applications don't need to implement
fallbacks when unset). On my hardware the radeon driver certainly
does have this information.
This information is needed in order to do accurate font rendering and
drawing. It's available in X, and it would be great if it was also
available using the framebuffer.
I've also attached a small patch to fbset to allow reporting of the
current state in fb_var_screeninfo such as resolution, size, depth etc.
Could be extended to report more info if desired.
I'm not entirely sure who is best to submit this to, so my apologies
if this is not you.
I've included the dri and fbdev lists because I'm not sure if it's
specific to the drmfb code or the generic framebuffer code. Likewise
if you set a default 96dpi size, I'm not sure if it's a generic issue
or specific to drmfb.
Thanks,
Roger
diff -urN /tmp/fbset-2.1/fbset.c ./fbset.c
--- /tmp/fbset-2.1/fbset.c 2011-05-09 18:47:47.000000000 +0100
+++ ./fbset.c 2011-05-09 18:46:32.142945642 +0100
@@ -281,7 +281,8 @@
static struct VideoMode *FindVideoMode(const char *name);
static void ModifyVideoMode(struct VideoMode *vmode);
static void DisplayVModeInfo(struct VideoMode *vmode);
-static void DisplayFBInfo(struct fb_fix_screeninfo *fix);
+static void DisplayFBInfo(struct fb_fix_screeninfo *fix,
+ struct fb_var_screeninfo *var);
static int FillScanRates(struct VideoMode *vmode);
static void Usage(void) __attribute__ ((noreturn));
int main(int argc, char *argv[]);
@@ -758,7 +759,8 @@
* Display the Frame Buffer Device Information
*/
-static void DisplayFBInfo(struct fb_fix_screeninfo *fix)
+static void DisplayFBInfo(struct fb_fix_screeninfo *fix,
+ struct fb_var_screeninfo *var)
{
int i;
@@ -845,6 +847,16 @@
puts(Accelerators[i].name);
else
printf("Unknown (%d)\n", fix->accel);
+
+ printf(" Dimensions : %dx%d pixels", var->xres, var->yres);
+ if (var->width != -1 && var->height != -1)
+ printf(" (%dx%d mm)", var->width, var->height);
+ putc('\n', stdout);
+ printf(" Virtual : %dx%d pixels\n", var->xres_virtual, var->yres_virtual);
+ printf(" Offset : %dx%d pixels\n", var->xoffset, var->yoffset);
+ printf(" Bits/Pixel : %d\n", var->bits_per_pixel);
+ if (var->grayscale)
+ printf(" Graylevels : %d\n", var->grayscale);
}
@@ -1101,7 +1113,7 @@
if (Opt_verbose)
puts("Getting further frame buffer information");
GetFixScreenInfo(fh, &fix);
- DisplayFBInfo(&fix);
+ DisplayFBInfo(&fix, &var);
}
/*
--
.''`. Roger Leigh
: :' : Debian GNU/Linux http://people.debian.org/~rleigh/
`. `' Printing on GNU/Linux? http://gutenprint.sourceforge.net/
`- GPG Public Key: 0x25BFB848 Please GPG sign your mail.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* [PATCH] viafb: Automatic OLPC XO-1.5 configuration
From: Daniel Drake @ 2011-05-09 21:14 UTC (permalink / raw)
To: linux-fbdev
Detect presence of the OLPC laptop and configure default settings
accordingly. This means the kernel can now boot on XO-1.5 without
needing long, hardcoded boot options.
Signed-off-by: Daniel Drake <dsd@laptop.org>
---
drivers/video/via/global.c | 2 +-
drivers/video/via/viafbdev.c | 55 ++++++++++++++++++++++++++++++++----------
2 files changed, 43 insertions(+), 14 deletions(-)
diff --git a/drivers/video/via/global.c b/drivers/video/via/global.c
index e10d824..b994e3b 100644
--- a/drivers/video/via/global.c
+++ b/drivers/video/via/global.c
@@ -29,7 +29,7 @@ int viafb_refresh = 60;
int viafb_refresh1 = 60;
int viafb_lcd_dsp_method = LCD_EXPANDSION;
int viafb_lcd_mode = LCD_OPENLDI;
-int viafb_CRT_ON = 1;
+int viafb_CRT_ON = STATE_ON;
int viafb_DVI_ON;
int viafb_LCD_ON ;
int viafb_LCD2_ON;
diff --git a/drivers/video/via/viafbdev.c b/drivers/video/via/viafbdev.c
index 7b4390e..75b5fc7 100644
--- a/drivers/video/via/viafbdev.c
+++ b/drivers/video/via/viafbdev.c
@@ -24,6 +24,7 @@
#include <linux/slab.h>
#include <linux/stat.h>
#include <linux/via-core.h>
+#include <asm/olpc.h>
#define _MASTER_FILE
#include "global.h"
@@ -36,6 +37,8 @@ static char *viafb_mode;
static char *viafb_mode1;
static int viafb_bpp = 32;
static int viafb_bpp1 = 32;
+static int viafb_default_mode_xres = 640;
+static int viafb_default_mode_yres = 480;
static unsigned int viafb_second_xres = 640;
static unsigned int viafb_second_yres = 480;
@@ -1001,19 +1004,18 @@ static void retrieve_device_setting(struct viafb_ioctl_setting
static int __init parse_active_dev(void)
{
- viafb_CRT_ON = STATE_OFF;
viafb_DVI_ON = STATE_OFF;
- viafb_LCD_ON = STATE_OFF;
viafb_LCD2_ON = STATE_OFF;
+
+ if (!viafb_active_dev)
+ return 0;
+
/* 1. Modify the active status of devices. */
/* 2. Keep the order of devices, so we can set corresponding
IGA path to devices in SAMM case. */
/* Note: The previous of active_dev is primary device,
and the following is secondary device. */
- if (!viafb_active_dev) {
- viafb_CRT_ON = STATE_ON;
- viafb_SAMM_ON = STATE_OFF;
- } else if (!strcmp(viafb_active_dev, "CRT+DVI")) {
+ if (!strcmp(viafb_active_dev, "CRT+DVI")) {
/* CRT+DVI */
viafb_CRT_ON = STATE_ON;
viafb_DVI_ON = STATE_ON;
@@ -1035,19 +1037,23 @@ static int __init parse_active_dev(void)
viafb_primary_dev = LCD_Device;
} else if (!strcmp(viafb_active_dev, "DVI+LCD")) {
/* DVI+LCD */
+ viafb_CRT_ON = STATE_OFF;
viafb_DVI_ON = STATE_ON;
viafb_LCD_ON = STATE_ON;
viafb_primary_dev = DVI_Device;
} else if (!strcmp(viafb_active_dev, "LCD+DVI")) {
/* LCD+DVI */
+ viafb_CRT_ON = STATE_OFF;
viafb_DVI_ON = STATE_ON;
viafb_LCD_ON = STATE_ON;
viafb_primary_dev = LCD_Device;
} else if (!strcmp(viafb_active_dev, "LCD+LCD2")) {
+ viafb_CRT_ON = STATE_OFF;
viafb_LCD_ON = STATE_ON;
viafb_LCD2_ON = STATE_ON;
viafb_primary_dev = LCD_Device;
} else if (!strcmp(viafb_active_dev, "LCD2+LCD")) {
+ viafb_CRT_ON = STATE_OFF;
viafb_LCD_ON = STATE_ON;
viafb_LCD2_ON = STATE_ON;
viafb_primary_dev = LCD2_Device;
@@ -1057,10 +1063,12 @@ static int __init parse_active_dev(void)
viafb_SAMM_ON = STATE_OFF;
} else if (!strcmp(viafb_active_dev, "DVI")) {
/* DVI only */
+ viafb_CRT_ON = STATE_OFF;
viafb_DVI_ON = STATE_ON;
viafb_SAMM_ON = STATE_OFF;
} else if (!strcmp(viafb_active_dev, "LCD")) {
/* LCD only */
+ viafb_CRT_ON = STATE_OFF;
viafb_LCD_ON = STATE_ON;
viafb_SAMM_ON = STATE_OFF;
} else
@@ -1665,8 +1673,8 @@ static int parse_mode(const char *str, u32 *xres, u32 *yres)
char *ptr;
if (!str) {
- *xres = 640;
- *yres = 480;
+ *xres = viafb_default_mode_xres;
+ *yres = viafb_default_mode_yres;
return 0;
}
@@ -1922,11 +1930,16 @@ void __devexit via_fb_pci_remove(struct pci_dev *pdev)
}
#ifndef MODULE
-static int __init viafb_setup(char *options)
+static int __init viafb_setup(void)
{
char *this_opt;
+ char *options;
+
DEBUG_MSG(KERN_INFO "viafb_setup!\n");
+ if (fb_get_options("viafb", &options))
+ return -ENODEV;
+
if (!options || !*options)
return 0;
@@ -1994,17 +2007,33 @@ static int __init viafb_setup(char *options)
}
#endif
+static void __init viafb_platform_setup(void)
+{
+ if (machine_is_olpc()) {
+ /* Apply XO-1.5-specific configuration. */
+ viafb_lcd_panel_id = 23;
+ viafb_bpp = 24;
+ viafb_default_mode_xres = 1200;
+ viafb_default_mode_yres = 900;
+
+ /* LCD only */
+ viafb_CRT_ON = STATE_OFF;
+ viafb_LCD_ON = STATE_ON;
+ viafb_SAMM_ON = STATE_OFF;
+ }
+}
+
/*
* These are called out of via-core for now.
*/
int __init viafb_init(void)
{
u32 dummy_x, dummy_y;
+
+ viafb_platform_setup();
+
#ifndef MODULE
- char *option = NULL;
- if (fb_get_options("viafb", &option))
- return -ENODEV;
- viafb_setup(option);
+ viafb_setup();
#endif
if (parse_mode(viafb_mode, &dummy_x, &dummy_y)
|| !viafb_get_mode(dummy_x, dummy_y)
--
1.7.4.4
^ permalink raw reply related
* Re: [PATCH] viafb: Automatic OLPC XO-1.5 configuration
From: Florian Tobias Schandinat @ 2011-05-09 21:58 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <20110509211439.AF70A9D401C@zog.reactivated.net>
Hi Daniel,
On 05/09/2011 09:14 PM, Daniel Drake wrote:
> Detect presence of the OLPC laptop and configure default settings
> accordingly. This means the kernel can now boot on XO-1.5 without
> needing long, hardcoded boot options.
The purpose of this patch is to allow people use their own kernel configuration
and just require them to enable viafb but not to copy the built-in command line,
right?
Well I usually dislike platform specific code but given that we probably won't
be able to detect everything OLPC specific even with a working autodetection and
having already a lot of OLPC code in here I think adding a little more would be
acceptable.
>
> Signed-off-by: Daniel Drake<dsd@laptop.org>
> ---
> drivers/video/via/global.c | 2 +-
> drivers/video/via/viafbdev.c | 55 ++++++++++++++++++++++++++++++++----------
> 2 files changed, 43 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/video/via/global.c b/drivers/video/via/global.c
> index e10d824..b994e3b 100644
> --- a/drivers/video/via/global.c
> +++ b/drivers/video/via/global.c
> @@ -29,7 +29,7 @@ int viafb_refresh = 60;
> int viafb_refresh1 = 60;
> int viafb_lcd_dsp_method = LCD_EXPANDSION;
> int viafb_lcd_mode = LCD_OPENLDI;
> -int viafb_CRT_ON = 1;
> +int viafb_CRT_ON = STATE_ON;
Unnecessary but okay
> int viafb_DVI_ON;
> int viafb_LCD_ON ;
> int viafb_LCD2_ON;
> diff --git a/drivers/video/via/viafbdev.c b/drivers/video/via/viafbdev.c
> index 7b4390e..75b5fc7 100644
> --- a/drivers/video/via/viafbdev.c
> +++ b/drivers/video/via/viafbdev.c
> @@ -24,6 +24,7 @@
> #include<linux/slab.h>
> #include<linux/stat.h>
> #include<linux/via-core.h>
> +#include<asm/olpc.h>
>
> #define _MASTER_FILE
> #include "global.h"
> @@ -36,6 +37,8 @@ static char *viafb_mode;
> static char *viafb_mode1;
> static int viafb_bpp = 32;
> static int viafb_bpp1 = 32;
> +static int viafb_default_mode_xres = 640;
> +static int viafb_default_mode_yres = 480;
Nack. As OLPC will probably be ever the only platform where the default is
different and as we only use it once, we shouldn't add an extra var for it.
>
> static unsigned int viafb_second_xres = 640;
> static unsigned int viafb_second_yres = 480;
> @@ -1001,19 +1004,18 @@ static void retrieve_device_setting(struct viafb_ioctl_setting
>
> static int __init parse_active_dev(void)
> {
> - viafb_CRT_ON = STATE_OFF;
> viafb_DVI_ON = STATE_OFF;
> - viafb_LCD_ON = STATE_OFF;
> viafb_LCD2_ON = STATE_OFF;
> +
> + if (!viafb_active_dev)
> + return 0;
> +
> /* 1. Modify the active status of devices. */
> /* 2. Keep the order of devices, so we can set corresponding
> IGA path to devices in SAMM case. */
> /* Note: The previous of active_dev is primary device,
> and the following is secondary device. */
> - if (!viafb_active_dev) {
Just add here an
if (machine_is_olpc())
and handle it correct. Changing so much generic code for one platform is not a
good thing. And it certainly is a better design to start with everything off and
enable things needed and not mix it up.
> - viafb_CRT_ON = STATE_ON;
> - viafb_SAMM_ON = STATE_OFF;
> - } else if (!strcmp(viafb_active_dev, "CRT+DVI")) {
> + if (!strcmp(viafb_active_dev, "CRT+DVI")) {
> /* CRT+DVI */
> viafb_CRT_ON = STATE_ON;
> viafb_DVI_ON = STATE_ON;
> @@ -1035,19 +1037,23 @@ static int __init parse_active_dev(void)
> viafb_primary_dev = LCD_Device;
> } else if (!strcmp(viafb_active_dev, "DVI+LCD")) {
> /* DVI+LCD */
> + viafb_CRT_ON = STATE_OFF;
> viafb_DVI_ON = STATE_ON;
> viafb_LCD_ON = STATE_ON;
> viafb_primary_dev = DVI_Device;
> } else if (!strcmp(viafb_active_dev, "LCD+DVI")) {
> /* LCD+DVI */
> + viafb_CRT_ON = STATE_OFF;
> viafb_DVI_ON = STATE_ON;
> viafb_LCD_ON = STATE_ON;
> viafb_primary_dev = LCD_Device;
> } else if (!strcmp(viafb_active_dev, "LCD+LCD2")) {
> + viafb_CRT_ON = STATE_OFF;
> viafb_LCD_ON = STATE_ON;
> viafb_LCD2_ON = STATE_ON;
> viafb_primary_dev = LCD_Device;
> } else if (!strcmp(viafb_active_dev, "LCD2+LCD")) {
> + viafb_CRT_ON = STATE_OFF;
> viafb_LCD_ON = STATE_ON;
> viafb_LCD2_ON = STATE_ON;
> viafb_primary_dev = LCD2_Device;
> @@ -1057,10 +1063,12 @@ static int __init parse_active_dev(void)
> viafb_SAMM_ON = STATE_OFF;
> } else if (!strcmp(viafb_active_dev, "DVI")) {
> /* DVI only */
> + viafb_CRT_ON = STATE_OFF;
> viafb_DVI_ON = STATE_ON;
> viafb_SAMM_ON = STATE_OFF;
> } else if (!strcmp(viafb_active_dev, "LCD")) {
> /* LCD only */
> + viafb_CRT_ON = STATE_OFF;
> viafb_LCD_ON = STATE_ON;
> viafb_SAMM_ON = STATE_OFF;
> } else
> @@ -1665,8 +1673,8 @@ static int parse_mode(const char *str, u32 *xres, u32 *yres)
> char *ptr;
>
> if (!str) {
Add the OLPC mode here as in
if (machine_is_olpc()) {
*xres = 1200;
*yres = 900;
} else {
*xres = 640;
*yres = 480;
... perhaps this might change to autodetect someday as far as possible
}
> - *xres = 640;
> - *yres = 480;
> + *xres = viafb_default_mode_xres;
> + *yres = viafb_default_mode_yres;
> return 0;
> }
>
> @@ -1922,11 +1930,16 @@ void __devexit via_fb_pci_remove(struct pci_dev *pdev)
> }
>
> #ifndef MODULE
> -static int __init viafb_setup(char *options)
> +static int __init viafb_setup(void)
> {
> char *this_opt;
> + char *options;
> +
> DEBUG_MSG(KERN_INFO "viafb_setup!\n");
>
> + if (fb_get_options("viafb",&options))
> + return -ENODEV;
You move the return value here....
> +
> if (!options || !*options)
> return 0;
>
> @@ -1994,17 +2007,33 @@ static int __init viafb_setup(char *options)
> }
> #endif
>
> +static void __init viafb_platform_setup(void)
Probably after my comments it does no longer make any sense to do it in an extra
function as the only thing left is
viafb_lcd_panel_id = 23;
> +{
> + if (machine_is_olpc()) {
> + /* Apply XO-1.5-specific configuration. */
> + viafb_lcd_panel_id = 23;
> + viafb_bpp = 24;
> + viafb_default_mode_xres = 1200;
> + viafb_default_mode_yres = 900;
> +
> + /* LCD only */
> + viafb_CRT_ON = STATE_OFF;
> + viafb_LCD_ON = STATE_ON;
> + viafb_SAMM_ON = STATE_OFF;
> + }
> +}
> +
> /*
> * These are called out of via-core for now.
> */
> int __init viafb_init(void)
> {
> u32 dummy_x, dummy_y;
> +
> + viafb_platform_setup();
> +
> #ifndef MODULE
> - char *option = NULL;
> - if (fb_get_options("viafb",&option))
> - return -ENODEV;
> - viafb_setup(option);
> + viafb_setup();
...and you don't handle the return value here.
> #endif
> if (parse_mode(viafb_mode,&dummy_x,&dummy_y)
> || !viafb_get_mode(dummy_x, dummy_y)
Regards,
Florian Tobias Schandinat
^ permalink raw reply
* [PATCH] omap2/omapfb: make DBG() more resistant in if-else constructions
From: Niels de Vos @ 2011-05-10 9:20 UTC (permalink / raw)
To: linux-omap; +Cc: linux-fbdev, linux-kernel, Niels de Vos
When DBG() is used in a simple if-else, the resulting code path
currently depends on the definition of DBG(). Inserting the statement in
a "do { ... } while (0)" prevents this possible misuse.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
---
Note, I have not found any offenders, but a mistake can easily be made.
The following example shows what can go wrong if little intention is
paid to the definition of the DBG() macro.
Example:
if something_went_wrong()
DBG("oh no, something went wrong!\n");
else
printk("all went fine\n");
Old result where the else is placed inside the first if-statment:
if something_went_wrong() {
if (omapfb_debug) {
printk(KERN_DEBUG "oh no, something went wrong!\n");
} else {
printk("all went fine\n");
}
}
New result where the else is an alternative to the first if-statement:
if something_went_wrong() {
do {
if (omapfb_debug)
printk(KERN_DEBUG "oh no, something went wrong!\n");
} while (0);
} else {
printk("all went fine\n");
}
---
drivers/video/omap2/omapfb/omapfb.h | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/video/omap2/omapfb/omapfb.h b/drivers/video/omap2/omapfb/omapfb.h
index 1305fc9..a01b0bb 100644
--- a/drivers/video/omap2/omapfb/omapfb.h
+++ b/drivers/video/omap2/omapfb/omapfb.h
@@ -34,8 +34,10 @@
#ifdef DEBUG
extern unsigned int omapfb_debug;
#define DBG(format, ...) \
- if (omapfb_debug) \
- printk(KERN_DEBUG "OMAPFB: " format, ## __VA_ARGS__)
+ do { \
+ if (omapfb_debug) \
+ printk(KERN_DEBUG "OMAPFB: " format, ## __VA_ARGS__); \
+ while (0)
#else
#define DBG(format, ...)
#endif
--
1.7.4.4
^ permalink raw reply related
* Re: [PATCH] omap2/omapfb: make DBG() more resistant in if-else constructions
From: Geert Uytterhoeven @ 2011-05-10 9:42 UTC (permalink / raw)
To: Niels de Vos; +Cc: linux-omap, linux-fbdev, linux-kernel
In-Reply-To: <1305019249-9898-1-git-send-email-ndevos@redhat.com>
On Tue, May 10, 2011 at 11:20, Niels de Vos <ndevos@redhat.com> wrote:
> When DBG() is used in a simple if-else, the resulting code path
> currently depends on the definition of DBG(). Inserting the statement in
> a "do { ... } while (0)" prevents this possible misuse.
>
> Signed-off-by: Niels de Vos <ndevos@redhat.com>
> --- a/drivers/video/omap2/omapfb/omapfb.h
> +++ b/drivers/video/omap2/omapfb/omapfb.h
> @@ -34,8 +34,10 @@
> #ifdef DEBUG
> extern unsigned int omapfb_debug;
> #define DBG(format, ...) \
> - if (omapfb_debug) \
> - printk(KERN_DEBUG "OMAPFB: " format, ## __VA_ARGS__)
> + do { \
> + if (omapfb_debug) \
> + printk(KERN_DEBUG "OMAPFB: " format, ## __VA_ARGS__); \
> + while (0)
Where's the closing '}'?
> #else
> #define DBG(format, ...)
BTW, no printf()-style format checking here.
> #endif
What about using the standard pr_debug()/dev_dbg() instead?
With dynamic debug, it can be enabled at run time.
As a bonus, you get printf()-style format checking if debugging is disabled.
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: [PATCH] omap2/omapfb: make DBG() more resistant in if-else
From: Premi, Sanjeev @ 2011-05-10 9:49 UTC (permalink / raw)
To: Niels de Vos, linux-omap@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1305019249-9898-1-git-send-email-ndevos@redhat.com>
> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org
> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Niels de Vos
> Sent: Tuesday, May 10, 2011 2:51 PM
> To: linux-omap@vger.kernel.org
> Cc: linux-fbdev@vger.kernel.org;
> linux-kernel@vger.kernel.org; Niels de Vos
> Subject: [PATCH] omap2/omapfb: make DBG() more resistant in
> if-else constructions
>
> When DBG() is used in a simple if-else, the resulting code path
> currently depends on the definition of DBG(). Inserting the
> statement in
> a "do { ... } while (0)" prevents this possible misuse.
>
> Signed-off-by: Niels de Vos <ndevos@redhat.com>
>
> ---
> Note, I have not found any offenders, but a mistake can
> easily be made.
> The following example shows what can go wrong if little intention is
> paid to the definition of the DBG() macro.
>
> Example:
> if something_went_wrong()
> DBG("oh no, something went wrong!\n");
> else
> printk("all went fine\n");
>
> Old result where the else is placed inside the first if-statment:
> if something_went_wrong() {
> if (omapfb_debug) {
> printk(KERN_DEBUG "oh no, something
> went wrong!\n");
> } else {
> printk("all went fine\n");
> }
> }
>
> New result where the else is an alternative to the first if-statement:
> if something_went_wrong() {
> do {
> if (omapfb_debug)
> printk(KERN_DEBUG "oh no,
> something went wrong!\n");
> } while (0);
> } else {
> printk("all went fine\n");
> }
> ---
> drivers/video/omap2/omapfb/omapfb.h | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/omap2/omapfb/omapfb.h
> b/drivers/video/omap2/omapfb/omapfb.h
> index 1305fc9..a01b0bb 100644
> --- a/drivers/video/omap2/omapfb/omapfb.h
> +++ b/drivers/video/omap2/omapfb/omapfb.h
> @@ -34,8 +34,10 @@
> #ifdef DEBUG
> extern unsigned int omapfb_debug;
> #define DBG(format, ...) \
> - if (omapfb_debug) \
> - printk(KERN_DEBUG "OMAPFB: " format, ## __VA_ARGS__)
> + do { \
> + if (omapfb_debug) \
> + printk(KERN_DEBUG "OMAPFB: " format, ##
> __VA_ARGS__); \
> + while (0)
A real good find. Wondering if it really didn't create any problems so far...
~sanjeev
> #else
> #define DBG(format, ...)
> #endif
> --
> 1.7.4.4
>
> --
> 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
* Re: [PATCH] omap2/omapfb: make DBG() more resistant in if-else constructions
From: Niels de Vos @ 2011-05-10 10:57 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: linux-omap, linux-fbdev, linux-kernel, Premi, Sanjeev
In-Reply-To: <BANLkTinEJZ=fvJmWRkQ7kKyxbFRaJnum7g@mail.gmail.com>
On Tue, May 10, 2011 at 10:42 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Tue, May 10, 2011 at 11:20, Niels de Vos <ndevos@redhat.com> wrote:
>> When DBG() is used in a simple if-else, the resulting code path
>> currently depends on the definition of DBG(). Inserting the statement in
>> a "do { ... } while (0)" prevents this possible misuse.
>>
>> Signed-off-by: Niels de Vos <ndevos@redhat.com>
>
>> --- a/drivers/video/omap2/omapfb/omapfb.h
>> +++ b/drivers/video/omap2/omapfb/omapfb.h
>> @@ -34,8 +34,10 @@
>> #ifdef DEBUG
>> extern unsigned int omapfb_debug;
>> #define DBG(format, ...) \
>> - if (omapfb_debug) \
>> - printk(KERN_DEBUG "OMAPFB: " format, ## __VA_ARGS__)
>> + do { \
>> + if (omapfb_debug) \
>> + printk(KERN_DEBUG "OMAPFB: " format, ## __VA_ARGS__); \
>> + while (0)
>
> Where's the closing '}'?
Good catch! That's in a "fixup!" that I forgot to squash :-/
I'll post an update version in a bit.
>> #else
>> #define DBG(format, ...)
>
> BTW, no printf()-style format checking here.
>
>> #endif
>
> What about using the standard pr_debug()/dev_dbg() instead?
> With dynamic debug, it can be enabled at run time.
> As a bonus, you get printf()-style format checking if debugging is disabled.
I think removing DBG() and the omapfb_debug module-parameter is surely
a good thing. Unfortunately DBG() is used quite a bit in the code and
replacing them 'll take some more time. I don't know yet when I find
some time to do and test that.
Thanks for the pointers,
Niels
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
>
>
^ permalink raw reply
* [PATCH V2] omap2/omapfb: make DBG() more resistant in if-else constructions
From: Niels de Vos @ 2011-05-10 11:07 UTC (permalink / raw)
To: linux-omap
Cc: linux-fbdev, linux-kernel, Geert Uytterhoeven, Sanjeev Premi,
Niels de Vos
In-Reply-To: <BANLkTinEJZ=fvJmWRkQ7kKyxbFRaJnum7g@mail.gmail.com>
When DBG() is used in a simple if-else, the resulting code path
currently depends on the definition of DBG(). Inserting the statement in
a "do { ... } while (0)" prevents this possible misuse.
Signed-off-by: Niels de Vos <ndevos@redhat.com>
---
V2: add the missing closing }
Note, I have not found any offenders, but a mistake can easily be made.
The following example shows what can go wrong if little intention is
paid to the definition of the DBG() macro.
Example:
if something_went_wrong()
DBG("oh no, something went wrong!\n");
else
printk("all went fine\n");
Old result where the else is placed inside the first if-statment:
if something_went_wrong() {
if (omapfb_debug) {
printk(KERN_DEBUG "oh no, something went wrong!\n");
} else {
printk("all went fine\n");
}
}
New result where the else is an alternative to the first if-statement:
if something_went_wrong() {
do {
if (omapfb_debug)
printk(KERN_DEBUG "oh no, something went wrong!\n");
} while (0);
} else {
printk("all went fine\n");
}
---
drivers/video/omap2/omapfb/omapfb.h | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/video/omap2/omapfb/omapfb.h b/drivers/video/omap2/omapfb/omapfb.h
index 1305fc9..456c586 100644
--- a/drivers/video/omap2/omapfb/omapfb.h
+++ b/drivers/video/omap2/omapfb/omapfb.h
@@ -34,8 +34,10 @@
#ifdef DEBUG
extern unsigned int omapfb_debug;
#define DBG(format, ...) \
- if (omapfb_debug) \
- printk(KERN_DEBUG "OMAPFB: " format, ## __VA_ARGS__)
+ do { \
+ if (omapfb_debug) \
+ printk(KERN_DEBUG "OMAPFB: " format, ## __VA_ARGS__); \
+ } while (0)
#else
#define DBG(format, ...)
#endif
--
1.7.4.4
^ permalink raw reply related
* Re: [PATCH] omap2/omapfb: make DBG() more resistant in if-else
From: Tomi Valkeinen @ 2011-05-10 12:08 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Niels de Vos, linux-omap, linux-fbdev, linux-kernel
In-Reply-To: <BANLkTinEJZ=fvJmWRkQ7kKyxbFRaJnum7g@mail.gmail.com>
On Tue, 2011-05-10 at 11:42 +0200, Geert Uytterhoeven wrote:
> What about using the standard pr_debug()/dev_dbg() instead?
> With dynamic debug, it can be enabled at run time.
> As a bonus, you get printf()-style format checking if debugging is disabled.
Yes, dev_dbg & co. would be better.
However, one thing I dislike about them is the extra stuff they print.
For example, for omapfb and omapdss dev_dbg will print:
omapfb omapfb: foo
omapdss_dss omapdss_dss: foo
I originally added the debug macros to omapdss to be able to
automatically print the DSS module name, as at that point there was only
one big omapdss device. And I guess I just followed with similar macro
in omapfb also. But I believe both omapdss and omapfb should be changed
to dev_* prints sometime soon.
Tomi
^ permalink raw reply
* Re: [PATCH] omap2/omapfb: make DBG() more resistant in if-else constructions
From: Geert Uytterhoeven @ 2011-05-10 12:14 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: Niels de Vos, linux-omap, linux-fbdev, linux-kernel
In-Reply-To: <1305029285.2045.38.camel@deskari>
On Tue, May 10, 2011 at 14:08, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Tue, 2011-05-10 at 11:42 +0200, Geert Uytterhoeven wrote:
>> What about using the standard pr_debug()/dev_dbg() instead?
>> With dynamic debug, it can be enabled at run time.
>> As a bonus, you get printf()-style format checking if debugging is disabled.
>
> Yes, dev_dbg & co. would be better.
>
> However, one thing I dislike about them is the extra stuff they print.
> For example, for omapfb and omapdss dev_dbg will print:
>
> omapfb omapfb: foo
> omapdss_dss omapdss_dss: foo
>
> I originally added the debug macros to omapdss to be able to
> automatically print the DSS module name, as at that point there was only
> one big omapdss device. And I guess I just followed with similar macro
> in omapfb also. But I believe both omapdss and omapfb should be changed
> to dev_* prints sometime soon.
If you don't want the extra baggage, do
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
and use pr_debug().
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: [PATCH V2] omap2/omapfb: make DBG() more resistant in if-else
From: Tomi Valkeinen @ 2011-05-10 12:16 UTC (permalink / raw)
To: Niels de Vos
Cc: linux-omap, linux-fbdev, linux-kernel, Geert Uytterhoeven,
Sanjeev Premi
In-Reply-To: <1305025653-17138-1-git-send-email-ndevos@redhat.com>
On Tue, 2011-05-10 at 12:07 +0100, Niels de Vos wrote:
> When DBG() is used in a simple if-else, the resulting code path
> currently depends on the definition of DBG(). Inserting the statement in
> a "do { ... } while (0)" prevents this possible misuse.
>
> Signed-off-by: Niels de Vos <ndevos@redhat.com>
Thanks, I'll add this to the dss2 tree.
Tomi
^ permalink raw reply
* [PATCH V2] fbcon -- fix race between open and removal of framebuffers
From: Tim Gardner @ 2011-05-10 12:47 UTC (permalink / raw)
To: Jack Stone
Cc: linux-fbdev, lethal, linux-kernel, Andy Whitcroft,
Leann Ogasawara
In-Reply-To: <4DC30FFA.9030708@fastmail.fm>
[-- Attachment #1: Type: text/plain, Size: 1238 bytes --]
On 05/05/2011 11:00 PM, Jack Stone wrote:
> On 05/05/2011 18:41, tim.gardner@canonical.com wrote:
>> +static struct fb_info *get_framebuffer_info(int idx)
>> +__acquires(®istered_lock)
>> +__releases(®istered_lock)
>> +{
>> + struct fb_info *fb_info;
>> +
>> + spin_lock(®istered_lock);
>> + fb_info = registered_fb[idx];
>> + fb_info->ref_count++;
>> + spin_unlock(®istered_lock);
>> +
>> + return fb_info;
>> +}
>> +
>> static int
>> fb_open(struct inode *inode, struct file *file)
>> __acquires(&info->lock)
>> @@ -1363,13 +1421,17 @@ __releases(&info->lock)
>>
>> if (fbidx>= FB_MAX)
>> return -ENODEV;
>> - info = registered_fb[fbidx];
>> + info = get_framebuffer_info(fbidx);
>> if (!info)
>> request_module("fb%d", fbidx);
>> - info = registered_fb[fbidx];
>> + info = get_framebuffer_info(fbidx);
>> if (!info)
>> return -ENODEV;
>
> If the first get_framebuffer_info succeeds don't you up the ref count
> twice? Shouldn't this be:
>
> info = get_framebuffer_info(fbidx);
> if (!info) {
> request_module("fb%d", fbidx);
> info = get_framebuffer_info(fbidx);
> }
> if (!info)
> return -ENODEV;
>
> Thanks,
>
> Jack
Good catch. See attached.
rtg
--
Tim Gardner tim.gardner@canonical.com
[-- Attachment #2: 0001-fbcon-fix-race-between-open-and-removal-of-framebuff.patch --]
[-- Type: text/x-patch, Size: 9991 bytes --]
From cefd70cfbb798cca488d6e97b25f41158f222afc Mon Sep 17 00:00:00 2001
From: Andy Whitcroft <apw@canonical.com>
Date: Thu, 29 Jul 2010 16:48:20 +0100
Subject: [PATCH] fbcon -- fix race between open and removal of framebuffers
Currently there is no locking for updates to the registered_fb list.
This allows an open through /dev/fbN to pick up a registered framebuffer
pointer in parallel with it being released, as happens when a conflicting
framebuffer is ejected or on module unload. There is also no reference
counting on the framebuffer descriptor which is referenced from all open
files, leading to references to released or reused memory to persist on
these open files.
This patch adds a reference count to the framebuffer descriptor to prevent
it from being released until after all pending opens are closed. This
allows the pending opens to detect the closed status and unmap themselves.
It also adds locking to the framebuffer lookup path, locking it against
the removal path such that it is possible to atomically lookup and take a
reference to the descriptor. It also adds locking to the read and write
paths which currently could access the framebuffer descriptor after it
has been freed. Finally it moves the device to FBINFO_STATE_REMOVED to
indicate that all access should be errored for this device.
Signed-off-by: Andy Whitcroft <apw@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
Signed-off-by: Leann Ogasawara <leann.ogasawara@canonical.com>
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
drivers/video/fbmem.c | 135 ++++++++++++++++++++++++++++++++++++++-----------
include/linux/fb.h | 2 +
2 files changed, 107 insertions(+), 30 deletions(-)
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index e0c2284..ea333dd 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -42,6 +42,8 @@
#define FBPIXMAPSIZE (1024 * 8)
+/* Protects the registered framebuffer list and count. */
+static DEFINE_SPINLOCK(registered_lock);
struct fb_info *registered_fb[FB_MAX] __read_mostly;
int num_registered_fb __read_mostly;
@@ -694,9 +696,7 @@ static ssize_t
fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
{
unsigned long p = *ppos;
- struct inode *inode = file->f_path.dentry->d_inode;
- int fbidx = iminor(inode);
- struct fb_info *info = registered_fb[fbidx];
+ struct fb_info *info = file->private_data;
u8 *buffer, *dst;
u8 __iomem *src;
int c, cnt = 0, err = 0;
@@ -705,19 +705,28 @@ fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
if (!info || ! info->screen_base)
return -ENODEV;
- if (info->state != FBINFO_STATE_RUNNING)
- return -EPERM;
+ if (!lock_fb_info(info))
+ return -ENODEV;
+
+ if (info->state != FBINFO_STATE_RUNNING) {
+ err = -EPERM;
+ goto out_fb_info;
+ }
- if (info->fbops->fb_read)
- return info->fbops->fb_read(info, buf, count, ppos);
+ if (info->fbops->fb_read) {
+ err = info->fbops->fb_read(info, buf, count, ppos);
+ goto out_fb_info;
+ }
total_size = info->screen_size;
if (total_size == 0)
total_size = info->fix.smem_len;
- if (p >= total_size)
- return 0;
+ if (p >= total_size) {
+ err = 0;
+ goto out_fb_info;
+ }
if (count >= total_size)
count = total_size;
@@ -727,8 +736,10 @@ fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
buffer = kmalloc((count > PAGE_SIZE) ? PAGE_SIZE : count,
GFP_KERNEL);
- if (!buffer)
- return -ENOMEM;
+ if (!buffer) {
+ err = -ENOMEM;
+ goto out_fb_info;
+ }
src = (u8 __iomem *) (info->screen_base + p);
@@ -751,19 +762,21 @@ fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
cnt += c;
count -= c;
}
+ if (!err)
+ err = cnt;
kfree(buffer);
+out_fb_info:
+ unlock_fb_info(info);
- return (err) ? err : cnt;
+ return err;
}
static ssize_t
fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
{
unsigned long p = *ppos;
- struct inode *inode = file->f_path.dentry->d_inode;
- int fbidx = iminor(inode);
- struct fb_info *info = registered_fb[fbidx];
+ struct fb_info *info = file->private_data;
u8 *buffer, *src;
u8 __iomem *dst;
int c, cnt = 0, err = 0;
@@ -772,8 +785,13 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
if (!info || !info->screen_base)
return -ENODEV;
- if (info->state != FBINFO_STATE_RUNNING)
- return -EPERM;
+ if (!lock_fb_info(info))
+ return -ENODEV;
+
+ if (info->state != FBINFO_STATE_RUNNING) {
+ err = -EPERM;
+ goto out_fb_info;
+ }
if (info->fbops->fb_write)
return info->fbops->fb_write(info, buf, count, ppos);
@@ -783,8 +801,10 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
if (total_size == 0)
total_size = info->fix.smem_len;
- if (p > total_size)
- return -EFBIG;
+ if (p > total_size) {
+ err = -EFBIG;
+ goto out_fb_info;
+ }
if (count > total_size) {
err = -EFBIG;
@@ -800,8 +820,10 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
buffer = kmalloc((count > PAGE_SIZE) ? PAGE_SIZE : count,
GFP_KERNEL);
- if (!buffer)
- return -ENOMEM;
+ if (!buffer) {
+ err = -ENOMEM;
+ goto out_fb_info;
+ }
dst = (u8 __iomem *) (info->screen_base + p);
@@ -825,10 +847,14 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
cnt += c;
count -= c;
}
+ if (cnt)
+ err = cnt;
kfree(buffer);
+out_fb_info:
+ unlock_fb_info(info);
- return (cnt) ? cnt : err;
+ return err;
}
int
@@ -1303,8 +1329,7 @@ static long fb_compat_ioctl(struct file *file, unsigned int cmd,
static int
fb_mmap(struct file *file, struct vm_area_struct * vma)
{
- int fbidx = iminor(file->f_path.dentry->d_inode);
- struct fb_info *info = registered_fb[fbidx];
+ struct fb_info * const info = file->private_data;
struct fb_ops *fb = info->fbops;
unsigned long off;
unsigned long start;
@@ -1316,6 +1341,11 @@ fb_mmap(struct file *file, struct vm_area_struct * vma)
if (!fb)
return -ENODEV;
mutex_lock(&info->mm_lock);
+ if (info->state == FBINFO_STATE_REMOVED) {
+ mutex_unlock(&info->mm_lock);
+ return -ENODEV;
+ }
+
if (fb->fb_mmap) {
int res;
res = fb->fb_mmap(info, vma);
@@ -1352,6 +1382,34 @@ fb_mmap(struct file *file, struct vm_area_struct * vma)
return 0;
}
+static struct fb_info *get_framebuffer_info(int idx)
+__acquires(®istered_lock)
+__releases(®istered_lock)
+{
+ struct fb_info *fb_info;
+
+ spin_lock(®istered_lock);
+ fb_info = registered_fb[idx];
+ fb_info->ref_count++;
+ spin_unlock(®istered_lock);
+
+ return fb_info;
+}
+
+static void put_framebuffer_info(struct fb_info *fb_info)
+__acquires(®istered_lock)
+__releases(®istered_lock)
+{
+ int keep;
+
+ spin_lock(®istered_lock);
+ keep = --fb_info->ref_count;
+ spin_unlock(®istered_lock);
+
+ if (!keep && fb_info->fbops->fb_destroy)
+ fb_info->fbops->fb_destroy(fb_info);
+}
+
static int
fb_open(struct inode *inode, struct file *file)
__acquires(&info->lock)
@@ -1363,13 +1421,18 @@ __releases(&info->lock)
if (fbidx >= FB_MAX)
return -ENODEV;
- info = registered_fb[fbidx];
- if (!info)
+ info = get_framebuffer_info(fbidx);
+ if (!info) {
request_module("fb%d", fbidx);
- info = registered_fb[fbidx];
+ info = get_framebuffer_info(fbidx);
+ }
if (!info)
return -ENODEV;
mutex_lock(&info->lock);
+ if (info->state == FBINFO_STATE_REMOVED) {
+ res = -ENODEV;
+ goto out;
+ }
if (!try_module_get(info->fbops->owner)) {
res = -ENODEV;
goto out;
@@ -1386,6 +1449,8 @@ __releases(&info->lock)
#endif
out:
mutex_unlock(&info->lock);
+ if (res)
+ put_framebuffer_info(info);
return res;
}
@@ -1401,6 +1466,7 @@ __releases(&info->lock)
info->fbops->fb_release(info,1);
module_put(info->fbops->owner);
mutex_unlock(&info->lock);
+ put_framebuffer_info(info);
return 0;
}
@@ -1549,6 +1615,7 @@ register_framebuffer(struct fb_info *fb_info)
fb_info->node = i;
mutex_init(&fb_info->lock);
mutex_init(&fb_info->mm_lock);
+ fb_info->ref_count = 1;
fb_info->dev = device_create(fb_class, fb_info->device,
MKDEV(FB_MAJOR, i), NULL, "fb%d", i);
@@ -1592,7 +1659,6 @@ register_framebuffer(struct fb_info *fb_info)
return 0;
}
-
/**
* unregister_framebuffer - releases a frame buffer device
* @fb_info: frame buffer info structure
@@ -1627,6 +1693,16 @@ unregister_framebuffer(struct fb_info *fb_info)
return -ENODEV;
event.info = fb_info;
ret = fb_notifier_call_chain(FB_EVENT_FB_UNBIND, &event);
+ if (!ret) {
+ mutex_lock(&fb_info->mm_lock);
+ /*
+ * We must prevent any operations for this transition, we
+ * already have info->lock so grab the info->mm_lock to hold
+ * the remainder.
+ */
+ fb_info->state = FBINFO_STATE_REMOVED;
+ mutex_unlock(&fb_info->mm_lock);
+ }
unlock_fb_info(fb_info);
if (ret) {
@@ -1646,8 +1722,7 @@ unregister_framebuffer(struct fb_info *fb_info)
fb_notifier_call_chain(FB_EVENT_FB_UNREGISTERED, &event);
/* this may free fb info */
- if (fb_info->fbops->fb_destroy)
- fb_info->fbops->fb_destroy(fb_info);
+ put_framebuffer_info(fb_info);
done:
return ret;
}
diff --git a/include/linux/fb.h b/include/linux/fb.h
index df728c1..60de3fa 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -834,6 +834,7 @@ struct fb_tile_ops {
struct fb_info {
int node;
int flags;
+ int ref_count;
struct mutex lock; /* Lock for open/release/ioctl funcs */
struct mutex mm_lock; /* Lock for fb_mmap and smem_* fields */
struct fb_var_screeninfo var; /* Current var */
@@ -873,6 +874,7 @@ struct fb_info {
void *pseudo_palette; /* Fake palette of 16 colors */
#define FBINFO_STATE_RUNNING 0
#define FBINFO_STATE_SUSPENDED 1
+#define FBINFO_STATE_REMOVED 2
u32 state; /* Hardware state i.e suspend */
void *fbcon_par; /* fbcon use-only private area */
/* From here on everything is device dependent */
--
1.7.4.1
^ permalink raw reply related
* [PATCH V3] fbcon -- fix race between open and removal of framebuffers
From: Tim Gardner @ 2011-05-10 13:52 UTC (permalink / raw)
To: linux-fbdev; +Cc: lethal, linux-kernel
In-Reply-To: <1304617307-7389-1-git-send-email-tim.gardner@canonical.com>
[-- Attachment #1: Type: text/plain, Size: 487 bytes --]
There was a second patch that I missed that fixed an original
regression, so I've just squashed that one into this V3 patch along with
the reference count issue that Jack Stone pointed out.
These patches can be found in their original form in
git://kernel.ubuntu.git/ubuntu/ubuntu-oneiric.git
UBUNTU: SAUCE: fbcon -- fix race between open and removal of framebuffers
UBUNTU: SAUCE: fbcon -- fix OOPs triggered by race prevention fixes
rtg
--
Tim Gardner tim.gardner@canonical.com
[-- Attachment #2: 0001-fbcon-fix-race-between-open-and-removal-of-framebuff.patch --]
[-- Type: text/x-patch, Size: 10007 bytes --]
From ca3ef33e2235c88856a6257c0be63192ab56c678 Mon Sep 17 00:00:00 2001
From: Andy Whitcroft <apw@canonical.com>
Date: Thu, 29 Jul 2010 16:48:20 +0100
Subject: [PATCH] fbcon -- fix race between open and removal of framebuffers
Currently there is no locking for updates to the registered_fb list.
This allows an open through /dev/fbN to pick up a registered framebuffer
pointer in parallel with it being released, as happens when a conflicting
framebuffer is ejected or on module unload. There is also no reference
counting on the framebuffer descriptor which is referenced from all open
files, leading to references to released or reused memory to persist on
these open files.
This patch adds a reference count to the framebuffer descriptor to prevent
it from being released until after all pending opens are closed. This
allows the pending opens to detect the closed status and unmap themselves.
It also adds locking to the framebuffer lookup path, locking it against
the removal path such that it is possible to atomically lookup and take a
reference to the descriptor. It also adds locking to the read and write
paths which currently could access the framebuffer descriptor after it
has been freed. Finally it moves the device to FBINFO_STATE_REMOVED to
indicate that all access should be errored for this device.
Signed-off-by: Andy Whitcroft <apw@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
Signed-off-by: Leann Ogasawara <leann.ogasawara@canonical.com>
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
drivers/video/fbmem.c | 136 ++++++++++++++++++++++++++++++++++++++-----------
include/linux/fb.h | 2 +
2 files changed, 108 insertions(+), 30 deletions(-)
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index e0c2284..1afe435 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -42,6 +42,8 @@
#define FBPIXMAPSIZE (1024 * 8)
+/* Protects the registered framebuffer list and count. */
+static DEFINE_SPINLOCK(registered_lock);
struct fb_info *registered_fb[FB_MAX] __read_mostly;
int num_registered_fb __read_mostly;
@@ -694,9 +696,7 @@ static ssize_t
fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
{
unsigned long p = *ppos;
- struct inode *inode = file->f_path.dentry->d_inode;
- int fbidx = iminor(inode);
- struct fb_info *info = registered_fb[fbidx];
+ struct fb_info *info = file->private_data;
u8 *buffer, *dst;
u8 __iomem *src;
int c, cnt = 0, err = 0;
@@ -705,19 +705,28 @@ fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
if (!info || ! info->screen_base)
return -ENODEV;
- if (info->state != FBINFO_STATE_RUNNING)
- return -EPERM;
+ if (!lock_fb_info(info))
+ return -ENODEV;
+
+ if (info->state != FBINFO_STATE_RUNNING) {
+ err = -EPERM;
+ goto out_fb_info;
+ }
- if (info->fbops->fb_read)
- return info->fbops->fb_read(info, buf, count, ppos);
+ if (info->fbops->fb_read) {
+ err = info->fbops->fb_read(info, buf, count, ppos);
+ goto out_fb_info;
+ }
total_size = info->screen_size;
if (total_size == 0)
total_size = info->fix.smem_len;
- if (p >= total_size)
- return 0;
+ if (p >= total_size) {
+ err = 0;
+ goto out_fb_info;
+ }
if (count >= total_size)
count = total_size;
@@ -727,8 +736,10 @@ fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
buffer = kmalloc((count > PAGE_SIZE) ? PAGE_SIZE : count,
GFP_KERNEL);
- if (!buffer)
- return -ENOMEM;
+ if (!buffer) {
+ err = -ENOMEM;
+ goto out_fb_info;
+ }
src = (u8 __iomem *) (info->screen_base + p);
@@ -751,19 +762,21 @@ fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
cnt += c;
count -= c;
}
+ if (!err)
+ err = cnt;
kfree(buffer);
+out_fb_info:
+ unlock_fb_info(info);
- return (err) ? err : cnt;
+ return err;
}
static ssize_t
fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
{
unsigned long p = *ppos;
- struct inode *inode = file->f_path.dentry->d_inode;
- int fbidx = iminor(inode);
- struct fb_info *info = registered_fb[fbidx];
+ struct fb_info *info = file->private_data;
u8 *buffer, *src;
u8 __iomem *dst;
int c, cnt = 0, err = 0;
@@ -772,8 +785,13 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
if (!info || !info->screen_base)
return -ENODEV;
- if (info->state != FBINFO_STATE_RUNNING)
- return -EPERM;
+ if (!lock_fb_info(info))
+ return -ENODEV;
+
+ if (info->state != FBINFO_STATE_RUNNING) {
+ err = -EPERM;
+ goto out_fb_info;
+ }
if (info->fbops->fb_write)
return info->fbops->fb_write(info, buf, count, ppos);
@@ -783,8 +801,10 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
if (total_size == 0)
total_size = info->fix.smem_len;
- if (p > total_size)
- return -EFBIG;
+ if (p > total_size) {
+ err = -EFBIG;
+ goto out_fb_info;
+ }
if (count > total_size) {
err = -EFBIG;
@@ -800,8 +820,10 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
buffer = kmalloc((count > PAGE_SIZE) ? PAGE_SIZE : count,
GFP_KERNEL);
- if (!buffer)
- return -ENOMEM;
+ if (!buffer) {
+ err = -ENOMEM;
+ goto out_fb_info;
+ }
dst = (u8 __iomem *) (info->screen_base + p);
@@ -825,10 +847,14 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
cnt += c;
count -= c;
}
+ if (cnt)
+ err = cnt;
kfree(buffer);
+out_fb_info:
+ unlock_fb_info(info);
- return (cnt) ? cnt : err;
+ return err;
}
int
@@ -1303,8 +1329,7 @@ static long fb_compat_ioctl(struct file *file, unsigned int cmd,
static int
fb_mmap(struct file *file, struct vm_area_struct * vma)
{
- int fbidx = iminor(file->f_path.dentry->d_inode);
- struct fb_info *info = registered_fb[fbidx];
+ struct fb_info * const info = file->private_data;
struct fb_ops *fb = info->fbops;
unsigned long off;
unsigned long start;
@@ -1316,6 +1341,11 @@ fb_mmap(struct file *file, struct vm_area_struct * vma)
if (!fb)
return -ENODEV;
mutex_lock(&info->mm_lock);
+ if (info->state == FBINFO_STATE_REMOVED) {
+ mutex_unlock(&info->mm_lock);
+ return -ENODEV;
+ }
+
if (fb->fb_mmap) {
int res;
res = fb->fb_mmap(info, vma);
@@ -1352,6 +1382,35 @@ fb_mmap(struct file *file, struct vm_area_struct * vma)
return 0;
}
+static struct fb_info *get_framebuffer_info(int idx)
+__acquires(®istered_lock)
+__releases(®istered_lock)
+{
+ struct fb_info *fb_info;
+
+ spin_lock(®istered_lock);
+ fb_info = registered_fb[idx];
+ if (fb_info)
+ fb_info->ref_count++;
+ spin_unlock(®istered_lock);
+
+ return fb_info;
+}
+
+static void put_framebuffer_info(struct fb_info *fb_info)
+__acquires(®istered_lock)
+__releases(®istered_lock)
+{
+ int keep;
+
+ spin_lock(®istered_lock);
+ keep = --fb_info->ref_count;
+ spin_unlock(®istered_lock);
+
+ if (!keep && fb_info->fbops->fb_destroy)
+ fb_info->fbops->fb_destroy(fb_info);
+}
+
static int
fb_open(struct inode *inode, struct file *file)
__acquires(&info->lock)
@@ -1363,13 +1422,18 @@ __releases(&info->lock)
if (fbidx >= FB_MAX)
return -ENODEV;
- info = registered_fb[fbidx];
- if (!info)
+ info = get_framebuffer_info(fbidx);
+ if (!info) {
request_module("fb%d", fbidx);
- info = registered_fb[fbidx];
+ info = get_framebuffer_info(fbidx);
+ }
if (!info)
return -ENODEV;
mutex_lock(&info->lock);
+ if (info->state == FBINFO_STATE_REMOVED) {
+ res = -ENODEV;
+ goto out;
+ }
if (!try_module_get(info->fbops->owner)) {
res = -ENODEV;
goto out;
@@ -1386,6 +1450,8 @@ __releases(&info->lock)
#endif
out:
mutex_unlock(&info->lock);
+ if (res)
+ put_framebuffer_info(info);
return res;
}
@@ -1401,6 +1467,7 @@ __releases(&info->lock)
info->fbops->fb_release(info,1);
module_put(info->fbops->owner);
mutex_unlock(&info->lock);
+ put_framebuffer_info(info);
return 0;
}
@@ -1549,6 +1616,7 @@ register_framebuffer(struct fb_info *fb_info)
fb_info->node = i;
mutex_init(&fb_info->lock);
mutex_init(&fb_info->mm_lock);
+ fb_info->ref_count = 1;
fb_info->dev = device_create(fb_class, fb_info->device,
MKDEV(FB_MAJOR, i), NULL, "fb%d", i);
@@ -1592,7 +1660,6 @@ register_framebuffer(struct fb_info *fb_info)
return 0;
}
-
/**
* unregister_framebuffer - releases a frame buffer device
* @fb_info: frame buffer info structure
@@ -1627,6 +1694,16 @@ unregister_framebuffer(struct fb_info *fb_info)
return -ENODEV;
event.info = fb_info;
ret = fb_notifier_call_chain(FB_EVENT_FB_UNBIND, &event);
+ if (!ret) {
+ mutex_lock(&fb_info->mm_lock);
+ /*
+ * We must prevent any operations for this transition, we
+ * already have info->lock so grab the info->mm_lock to hold
+ * the remainder.
+ */
+ fb_info->state = FBINFO_STATE_REMOVED;
+ mutex_unlock(&fb_info->mm_lock);
+ }
unlock_fb_info(fb_info);
if (ret) {
@@ -1646,8 +1723,7 @@ unregister_framebuffer(struct fb_info *fb_info)
fb_notifier_call_chain(FB_EVENT_FB_UNREGISTERED, &event);
/* this may free fb info */
- if (fb_info->fbops->fb_destroy)
- fb_info->fbops->fb_destroy(fb_info);
+ put_framebuffer_info(fb_info);
done:
return ret;
}
diff --git a/include/linux/fb.h b/include/linux/fb.h
index df728c1..60de3fa 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -834,6 +834,7 @@ struct fb_tile_ops {
struct fb_info {
int node;
int flags;
+ int ref_count;
struct mutex lock; /* Lock for open/release/ioctl funcs */
struct mutex mm_lock; /* Lock for fb_mmap and smem_* fields */
struct fb_var_screeninfo var; /* Current var */
@@ -873,6 +874,7 @@ struct fb_info {
void *pseudo_palette; /* Fake palette of 16 colors */
#define FBINFO_STATE_RUNNING 0
#define FBINFO_STATE_SUSPENDED 1
+#define FBINFO_STATE_REMOVED 2
u32 state; /* Hardware state i.e suspend */
void *fbcon_par; /* fbcon use-only private area */
/* From here on everything is device dependent */
--
1.7.4.1
^ permalink raw reply related
* [PATCH 0/8] OMAP: DSS2: Miscellaneous patches
From: Tomi Valkeinen @ 2011-05-10 16:24 UTC (permalink / raw)
To: linux-omap, linux-fbdev; +Cc: Tomi Valkeinen
Here are some smallish fixes and cleanups I made while porting N800's display
driver to DSS2.
Tomi
Tomi Valkeinen (8):
OMAP: DSS2: Add missing dummy functions
OMAPFB: fix wrong clock aliases and device name
OMAP: DSS2: RFBI: add rfbi_bus_lock
OMAP: DSS2: RFIB: clock enable/disable changes
OMAP: DSS2: RFBI: add omap_rfbi_configure
OMAP: DSS2: RFIB: cleanup
OMAP: DSS2: OMAPFB: remove dead code
OMAP: DSS2: OMAPFB: Reduce stack usage
arch/arm/plat-omap/include/plat/display.h | 4 +
drivers/video/omap/dispc.c | 4 +-
drivers/video/omap/omapfb_main.c | 2 +-
drivers/video/omap/rfbi.c | 2 +-
drivers/video/omap2/dss/dss.h | 28 ++++-
drivers/video/omap2/dss/rfbi.c | 174 +++++------------------------
drivers/video/omap2/omapfb/omapfb-main.c | 78 +++++--------
7 files changed, 86 insertions(+), 206 deletions(-)
--
1.7.4.1
^ permalink raw reply
* [PATCH 1/8] OMAP: DSS2: Add missing dummy functions
From: Tomi Valkeinen @ 2011-05-10 16:24 UTC (permalink / raw)
To: linux-omap, linux-fbdev; +Cc: Tomi Valkeinen
In-Reply-To: <1305044656-31512-1-git-send-email-tomi.valkeinen@ti.com>
dpi.c does not compile if DSI is not compiled in. Add the missing dummy
functions so that dpi.c compiles.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
drivers/video/omap2/dss/dss.h | 21 +++++++++++++++++++++
1 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index eea5c7d..f659cfb 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -313,6 +313,27 @@ static inline unsigned long dsi_get_pll_hsdiv_dispc_rate(void)
WARN("%s: DSI not compiled in, returning rate as 0\n", __func__);
return 0;
}
+static inline int dsi_pll_set_clock_div(struct dsi_clock_info *cinfo)
+{
+ WARN("%s: DSI not compiled in\n", __func__);
+ return -ENODEV;
+}
+static inline int dsi_pll_calc_clock_div_pck(bool is_tft, unsigned long req_pck,
+ struct dsi_clock_info *cinfo,
+ struct dispc_clock_info *dispc_cinfo)
+{
+ WARN("%s: DSI not compiled in\n", __func__);
+ return -ENODEV;
+}
+static inline int dsi_pll_init(struct omap_dss_device *dssdev,
+ bool enable_hsclk, bool enable_hsdiv)
+{
+ WARN("%s: DSI not compiled in\n", __func__);
+ return -ENODEV;
+}
+static inline void dsi_pll_uninit(bool disconnect_lanes)
+{
+}
static inline void dsi_wait_pll_hsdiv_dispc_active(void)
{
}
--
1.7.4.1
^ permalink raw reply related
* [PATCH 2/8] OMAPFB: fix wrong clock aliases and device name
From: Tomi Valkeinen @ 2011-05-10 16:24 UTC (permalink / raw)
To: linux-omap, linux-fbdev; +Cc: Tomi Valkeinen
In-Reply-To: <1305044656-31512-1-git-send-email-tomi.valkeinen@ti.com>
The clock aliases and the dss platform device name have changed, and
omapfb fails to initialize. Update the names to correct ones.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
drivers/video/omap/dispc.c | 4 ++--
drivers/video/omap/omapfb_main.c | 2 +-
drivers/video/omap/rfbi.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/video/omap/dispc.c b/drivers/video/omap/dispc.c
index 5294834..0ccd7ad 100644
--- a/drivers/video/omap/dispc.c
+++ b/drivers/video/omap/dispc.c
@@ -922,14 +922,14 @@ static int get_dss_clocks(void)
return PTR_ERR(dispc.dss_ick);
}
- dispc.dss1_fck = clk_get(&dispc.fbdev->dssdev->dev, "dss1_fck");
+ dispc.dss1_fck = clk_get(&dispc.fbdev->dssdev->dev, "fck");
if (IS_ERR(dispc.dss1_fck)) {
dev_err(dispc.fbdev->dev, "can't get dss1_fck\n");
clk_put(dispc.dss_ick);
return PTR_ERR(dispc.dss1_fck);
}
- dispc.dss_54m_fck = clk_get(&dispc.fbdev->dssdev->dev, "tv_fck");
+ dispc.dss_54m_fck = clk_get(&dispc.fbdev->dssdev->dev, "tv_clk");
if (IS_ERR(dispc.dss_54m_fck)) {
dev_err(dispc.fbdev->dev, "can't get tv_fck\n");
clk_put(dispc.dss_ick);
diff --git a/drivers/video/omap/omapfb_main.c b/drivers/video/omap/omapfb_main.c
index e264efd..b3ddd74 100644
--- a/drivers/video/omap/omapfb_main.c
+++ b/drivers/video/omap/omapfb_main.c
@@ -90,7 +90,7 @@ static void omapdss_release(struct device *dev)
/* dummy device for clocks */
static struct platform_device omapdss_device = {
- .name = "omapdss",
+ .name = "omapdss_dss",
.id = -1,
.dev = {
.release = omapdss_release,
diff --git a/drivers/video/omap/rfbi.c b/drivers/video/omap/rfbi.c
index eada9f1..0c6981f 100644
--- a/drivers/video/omap/rfbi.c
+++ b/drivers/video/omap/rfbi.c
@@ -90,7 +90,7 @@ static int rfbi_get_clocks(void)
return PTR_ERR(rfbi.dss_ick);
}
- rfbi.dss1_fck = clk_get(&rfbi.fbdev->dssdev->dev, "dss1_fck");
+ rfbi.dss1_fck = clk_get(&rfbi.fbdev->dssdev->dev, "fck");
if (IS_ERR(rfbi.dss1_fck)) {
dev_err(rfbi.fbdev->dev, "can't get dss1_fck\n");
clk_put(rfbi.dss_ick);
--
1.7.4.1
^ permalink raw reply related
* [PATCH 3/8] OMAP: DSS2: RFBI: add rfbi_bus_lock
From: Tomi Valkeinen @ 2011-05-10 16:24 UTC (permalink / raw)
To: linux-omap, linux-fbdev; +Cc: Tomi Valkeinen
In-Reply-To: <1305044656-31512-1-git-send-email-tomi.valkeinen@ti.com>
Add similar bus lock to RFBI as is in DSI. The panel driver can use the
bus lock to mark that the RFBI bus is currently in use.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
arch/arm/plat-omap/include/plat/display.h | 2 ++
drivers/video/omap2/dss/rfbi.c | 16 ++++++++++++++++
2 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/arch/arm/plat-omap/include/plat/display.h b/arch/arm/plat-omap/include/plat/display.h
index bd0f08e..e6629eb 100644
--- a/arch/arm/plat-omap/include/plat/display.h
+++ b/arch/arm/plat-omap/include/plat/display.h
@@ -214,6 +214,8 @@ int omap_rfbi_enable_te(bool enable, unsigned line);
int omap_rfbi_setup_te(enum omap_rfbi_te_mode mode,
unsigned hs_pulse_time, unsigned vs_pulse_time,
int hs_pol_inv, int vs_pol_inv, int extif_div);
+void rfbi_bus_lock(void);
+void rfbi_bus_unlock(void);
/* DSI */
void dsi_bus_lock(void);
diff --git a/drivers/video/omap2/dss/rfbi.c b/drivers/video/omap2/dss/rfbi.c
index 5ea17f4..b669071 100644
--- a/drivers/video/omap2/dss/rfbi.c
+++ b/drivers/video/omap2/dss/rfbi.c
@@ -32,6 +32,7 @@
#include <linux/ktime.h>
#include <linux/hrtimer.h>
#include <linux/seq_file.h>
+#include <linux/semaphore.h>
#include <plat/display.h>
#include "dss.h"
@@ -119,6 +120,8 @@ static struct {
struct completion cmd_done;
atomic_t cmd_fifo_full;
atomic_t cmd_pending;
+
+ struct semaphore bus_lock;
} rfbi;
struct update_region {
@@ -146,6 +149,18 @@ static void rfbi_enable_clocks(bool enable)
dss_clk_disable(DSS_CLK_ICK | DSS_CLK_FCK);
}
+void rfbi_bus_lock(void)
+{
+ down(&rfbi.bus_lock);
+}
+EXPORT_SYMBOL(rfbi_bus_lock);
+
+void rfbi_bus_unlock(void)
+{
+ up(&rfbi.bus_lock);
+}
+EXPORT_SYMBOL(rfbi_bus_unlock);
+
void omap_rfbi_write_command(const void *buf, u32 len)
{
rfbi_enable_clocks(1);
@@ -1022,6 +1037,7 @@ static int omap_rfbihw_probe(struct platform_device *pdev)
rfbi.pdev = pdev;
spin_lock_init(&rfbi.cmd_lock);
+ sema_init(&rfbi.bus_lock, 1);
init_completion(&rfbi.cmd_done);
atomic_set(&rfbi.cmd_fifo_full, 0);
--
1.7.4.1
^ permalink raw reply related
* [PATCH 4/8] OMAP: DSS2: RFIB: clock enable/disable changes
From: Tomi Valkeinen @ 2011-05-10 16:24 UTC (permalink / raw)
To: linux-omap, linux-fbdev; +Cc: Tomi Valkeinen
In-Reply-To: <1305044656-31512-1-git-send-email-tomi.valkeinen@ti.com>
RFBI enables and disables clocks inside almost every function to get a
finegrained control to the clocks. However, the current understanding is
that this is not necessary power-management-wise.
Change the clocking scheme so that RFI clocks are enabled when the
omapdss_rfbi_display_enable is called, and disabled when
omapdss_rfbi_display_disable is called.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
drivers/video/omap2/dss/rfbi.c | 28 ++++------------------------
1 files changed, 4 insertions(+), 24 deletions(-)
diff --git a/drivers/video/omap2/dss/rfbi.c b/drivers/video/omap2/dss/rfbi.c
index b669071..fadfaae 100644
--- a/drivers/video/omap2/dss/rfbi.c
+++ b/drivers/video/omap2/dss/rfbi.c
@@ -163,7 +163,6 @@ EXPORT_SYMBOL(rfbi_bus_unlock);
void omap_rfbi_write_command(const void *buf, u32 len)
{
- rfbi_enable_clocks(1);
switch (rfbi.parallelmode) {
case OMAP_DSS_RFBI_PARALLELMODE_8:
{
@@ -187,13 +186,11 @@ void omap_rfbi_write_command(const void *buf, u32 len)
default:
BUG();
}
- rfbi_enable_clocks(0);
}
EXPORT_SYMBOL(omap_rfbi_write_command);
void omap_rfbi_read_data(void *buf, u32 len)
{
- rfbi_enable_clocks(1);
switch (rfbi.parallelmode) {
case OMAP_DSS_RFBI_PARALLELMODE_8:
{
@@ -221,13 +218,11 @@ void omap_rfbi_read_data(void *buf, u32 len)
default:
BUG();
}
- rfbi_enable_clocks(0);
}
EXPORT_SYMBOL(omap_rfbi_read_data);
void omap_rfbi_write_data(const void *buf, u32 len)
{
- rfbi_enable_clocks(1);
switch (rfbi.parallelmode) {
case OMAP_DSS_RFBI_PARALLELMODE_8:
{
@@ -252,7 +247,6 @@ void omap_rfbi_write_data(const void *buf, u32 len)
BUG();
}
- rfbi_enable_clocks(0);
}
EXPORT_SYMBOL(omap_rfbi_write_data);
@@ -264,8 +258,6 @@ void omap_rfbi_write_pixels(const void __iomem *buf, int scr_width,
int horiz_offset = scr_width - w;
int i;
- rfbi_enable_clocks(1);
-
if (rfbi.datatype = OMAP_DSS_RFBI_DATATYPE_16 &&
rfbi.parallelmode = OMAP_DSS_RFBI_PARALLELMODE_8) {
const u16 __iomem *pd = buf;
@@ -310,8 +302,6 @@ void omap_rfbi_write_pixels(const void __iomem *buf, int scr_width,
} else {
BUG();
}
-
- rfbi_enable_clocks(0);
}
EXPORT_SYMBOL(omap_rfbi_write_pixels);
@@ -332,8 +322,6 @@ void rfbi_transfer_area(struct omap_dss_device *dssdev, u16 width,
rfbi.framedone_callback = callback;
rfbi.framedone_callback_data = data;
- rfbi_enable_clocks(1);
-
rfbi_write_reg(RFBI_PIXEL_CNT, width * height);
l = rfbi_read_reg(RFBI_CONTROL);
@@ -352,8 +340,6 @@ static void framedone_callback(void *data, u32 mask)
REG_FLD_MOD(RFBI_CONTROL, 0, 0, 0);
- rfbi_enable_clocks(0);
-
callback = rfbi.framedone_callback;
rfbi.framedone_callback = NULL;
@@ -462,7 +448,6 @@ void rfbi_set_timings(int rfbi_module, struct rfbi_timings *t)
BUG_ON(!t->converted);
- rfbi_enable_clocks(1);
rfbi_write_reg(RFBI_ONOFF_TIME(rfbi_module), t->tim[0]);
rfbi_write_reg(RFBI_CYCLE_TIME(rfbi_module), t->tim[1]);
@@ -471,7 +456,6 @@ void rfbi_set_timings(int rfbi_module, struct rfbi_timings *t)
(t->tim[2] ? 1 : 0), 4, 4);
rfbi_print_timings();
- rfbi_enable_clocks(0);
}
static int ps_to_rfbi_ticks(int time, int div)
@@ -659,7 +643,6 @@ int omap_rfbi_setup_te(enum omap_rfbi_te_mode mode,
DSSDBG("setup_te: mode %d hs %d vs %d hs_inv %d vs_inv %d\n",
mode, hs, vs, hs_pol_inv, vs_pol_inv);
- rfbi_enable_clocks(1);
rfbi_write_reg(RFBI_HSYNC_WIDTH, hs);
rfbi_write_reg(RFBI_VSYNC_WIDTH, vs);
@@ -672,7 +655,6 @@ int omap_rfbi_setup_te(enum omap_rfbi_te_mode mode,
l &= ~(1 << 20);
else
l |= 1 << 20;
- rfbi_enable_clocks(0);
return 0;
}
@@ -687,7 +669,6 @@ int omap_rfbi_enable_te(bool enable, unsigned line)
if (line > (1 << 11) - 1)
return -EINVAL;
- rfbi_enable_clocks(1);
l = rfbi_read_reg(RFBI_CONFIG(0));
l &= ~(0x3 << 2);
if (enable) {
@@ -697,7 +678,6 @@ int omap_rfbi_enable_te(bool enable, unsigned line)
rfbi.te_enabled = 0;
rfbi_write_reg(RFBI_CONFIG(0), l);
rfbi_write_reg(RFBI_LINE_NUMBER, line);
- rfbi_enable_clocks(0);
return 0;
}
@@ -836,8 +816,6 @@ int rfbi_configure(int rfbi_module, int bpp, int lines)
break;
}
- rfbi_enable_clocks(1);
-
REG_FLD_MOD(RFBI_CONTROL, 0, 3, 2); /* clear CS */
l = 0;
@@ -871,8 +849,6 @@ int rfbi_configure(int rfbi_module, int bpp, int lines)
DSSDBG("RFBI config: bpp %d, lines %d, cycles: 0x%x 0x%x 0x%x\n",
bpp, lines, cycle1, cycle2, cycle3);
- rfbi_enable_clocks(0);
-
return 0;
}
EXPORT_SYMBOL(rfbi_configure);
@@ -975,6 +951,8 @@ int omapdss_rfbi_display_enable(struct omap_dss_device *dssdev)
{
int r;
+ rfbi_enable_clocks(1);
+
r = omap_dss_start_device(dssdev);
if (r) {
DSSERR("failed to start device\n");
@@ -1017,6 +995,8 @@ void omapdss_rfbi_display_disable(struct omap_dss_device *dssdev)
omap_dispc_unregister_isr(framedone_callback, NULL,
DISPC_IRQ_FRAMEDONE);
omap_dss_stop_device(dssdev);
+
+ rfbi_enable_clocks(0);
}
EXPORT_SYMBOL(omapdss_rfbi_display_disable);
--
1.7.4.1
^ permalink raw reply related
* [PATCH 5/8] OMAP: DSS2: RFBI: add omap_rfbi_configure
From: Tomi Valkeinen @ 2011-05-10 16:24 UTC (permalink / raw)
To: linux-omap, linux-fbdev; +Cc: Tomi Valkeinen
In-Reply-To: <1305044656-31512-1-git-send-email-tomi.valkeinen@ti.com>
Add omap_rfbi_configure() which the panel driver can use to reconfigure
the data element size and the number of data lines in the RFBI bus.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
arch/arm/plat-omap/include/plat/display.h | 2 ++
drivers/video/omap2/dss/rfbi.c | 8 +++++++-
2 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/arch/arm/plat-omap/include/plat/display.h b/arch/arm/plat-omap/include/plat/display.h
index e6629eb..7215a8d 100644
--- a/arch/arm/plat-omap/include/plat/display.h
+++ b/arch/arm/plat-omap/include/plat/display.h
@@ -614,5 +614,7 @@ int omap_rfbi_prepare_update(struct omap_dss_device *dssdev,
int omap_rfbi_update(struct omap_dss_device *dssdev,
u16 x, u16 y, u16 w, u16 h,
void (*callback)(void *), void *data);
+int omap_rfbi_configure(struct omap_dss_device *dssdev, int pixel_size,
+ int data_lines);
#endif
diff --git a/drivers/video/omap2/dss/rfbi.c b/drivers/video/omap2/dss/rfbi.c
index fadfaae..e6bbb67 100644
--- a/drivers/video/omap2/dss/rfbi.c
+++ b/drivers/video/omap2/dss/rfbi.c
@@ -851,7 +851,13 @@ int rfbi_configure(int rfbi_module, int bpp, int lines)
return 0;
}
-EXPORT_SYMBOL(rfbi_configure);
+
+int omap_rfbi_configure(struct omap_dss_device *dssdev, int pixel_size,
+ int data_lines)
+{
+ return rfbi_configure(dssdev->phy.rfbi.channel, pixel_size, data_lines);
+}
+EXPORT_SYMBOL(omap_rfbi_configure);
int omap_rfbi_prepare_update(struct omap_dss_device *dssdev,
u16 *x, u16 *y, u16 *w, u16 *h)
--
1.7.4.1
^ permalink raw reply related
* [PATCH 6/8] OMAP: DSS2: RFIB: cleanup
From: Tomi Valkeinen @ 2011-05-10 16:24 UTC (permalink / raw)
To: linux-omap, linux-fbdev; +Cc: Tomi Valkeinen
In-Reply-To: <1305044656-31512-1-git-send-email-tomi.valkeinen@ti.com>
The RFBI driver is quite messy. Remove dead and unneeded code and add
statics to functions.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
drivers/video/omap2/dss/dss.h | 7 --
drivers/video/omap2/dss/rfbi.c | 124 +---------------------------------------
2 files changed, 3 insertions(+), 128 deletions(-)
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index f659cfb..6cc56a1 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -497,13 +497,6 @@ void hdmi_panel_exit(void);
int rfbi_init_platform_driver(void);
void rfbi_uninit_platform_driver(void);
void rfbi_dump_regs(struct seq_file *s);
-
-int rfbi_configure(int rfbi_module, int bpp, int lines);
-void rfbi_enable_rfbi(bool enable);
-void rfbi_transfer_area(struct omap_dss_device *dssdev, u16 width,
- u16 height, void (callback)(void *data), void *data);
-void rfbi_set_timings(int rfbi_module, struct rfbi_timings *t);
-unsigned long rfbi_get_max_tx_rate(void);
int rfbi_init_display(struct omap_dss_device *display);
#else
static inline int rfbi_init_platform_driver(void)
diff --git a/drivers/video/omap2/dss/rfbi.c b/drivers/video/omap2/dss/rfbi.c
index e6bbb67..d09ba5f 100644
--- a/drivers/video/omap2/dss/rfbi.c
+++ b/drivers/video/omap2/dss/rfbi.c
@@ -66,9 +66,6 @@ struct rfbi_reg { u16 idx; };
#define REG_FLD_MOD(idx, val, start, end) \
rfbi_write_reg(idx, FLD_MOD(rfbi_read_reg(idx), val, start, end))
-/* To work around an RFBI transfer rate limitation */
-#define OMAP_RFBI_RATE_LIMIT 1
-
enum omap_rfbi_cycleformat {
OMAP_DSS_RFBI_CYCLEFORMAT_1_1 = 0,
OMAP_DSS_RFBI_CYCLEFORMAT_2_1 = 1,
@@ -90,11 +87,6 @@ enum omap_rfbi_parallelmode {
OMAP_DSS_RFBI_PARALLELMODE_16 = 3,
};
-enum update_cmd {
- RFBI_CMD_UPDATE = 0,
- RFBI_CMD_SYNC = 1,
-};
-
static int rfbi_convert_timings(struct rfbi_timings *t);
static void rfbi_get_clk_info(u32 *clk_period, u32 *max_clk_div);
@@ -115,22 +107,9 @@ static struct {
struct omap_dss_device *dssdev[2];
- struct kfifo cmd_fifo;
- spinlock_t cmd_lock;
- struct completion cmd_done;
- atomic_t cmd_fifo_full;
- atomic_t cmd_pending;
-
struct semaphore bus_lock;
} rfbi;
-struct update_region {
- u16 x;
- u16 y;
- u16 w;
- u16 h;
-};
-
static inline void rfbi_write_reg(const struct rfbi_reg idx, u32 val)
{
__raw_writel(val, rfbi.base + idx.idx);
@@ -305,7 +284,7 @@ void omap_rfbi_write_pixels(const void __iomem *buf, int scr_width,
}
EXPORT_SYMBOL(omap_rfbi_write_pixels);
-void rfbi_transfer_area(struct omap_dss_device *dssdev, u16 width,
+static void rfbi_transfer_area(struct omap_dss_device *dssdev, u16 width,
u16 height, void (*callback)(void *data), void *data)
{
u32 l;
@@ -345,8 +324,6 @@ static void framedone_callback(void *data, u32 mask)
if (callback != NULL)
callback(rfbi.framedone_callback_data);
-
- atomic_set(&rfbi.cmd_pending, 0);
}
#if 1 /* VERBOSE */
@@ -436,7 +413,7 @@ static int calc_extif_timings(struct rfbi_timings *t)
}
-void rfbi_set_timings(int rfbi_module, struct rfbi_timings *t)
+static void rfbi_set_timings(int rfbi_module, struct rfbi_timings *t)
{
int r;
@@ -471,59 +448,6 @@ static int ps_to_rfbi_ticks(int time, int div)
return ret;
}
-#ifdef OMAP_RFBI_RATE_LIMIT
-unsigned long rfbi_get_max_tx_rate(void)
-{
- unsigned long l4_rate, dss1_rate;
- int min_l4_ticks = 0;
- int i;
-
- /* According to TI this can't be calculated so make the
- * adjustments for a couple of known frequencies and warn for
- * others.
- */
- static const struct {
- unsigned long l4_clk; /* HZ */
- unsigned long dss1_clk; /* HZ */
- unsigned long min_l4_ticks;
- } ftab[] = {
- { 55, 132, 7, }, /* 7.86 MPix/s */
- { 110, 110, 12, }, /* 9.16 MPix/s */
- { 110, 132, 10, }, /* 11 Mpix/s */
- { 120, 120, 10, }, /* 12 Mpix/s */
- { 133, 133, 10, }, /* 13.3 Mpix/s */
- };
-
- l4_rate = rfbi.l4_khz / 1000;
- dss1_rate = dss_clk_get_rate(DSS_CLK_FCK) / 1000000;
-
- for (i = 0; i < ARRAY_SIZE(ftab); i++) {
- /* Use a window instead of an exact match, to account
- * for different DPLL multiplier / divider pairs.
- */
- if (abs(ftab[i].l4_clk - l4_rate) < 3 &&
- abs(ftab[i].dss1_clk - dss1_rate) < 3) {
- min_l4_ticks = ftab[i].min_l4_ticks;
- break;
- }
- }
- if (i = ARRAY_SIZE(ftab)) {
- /* Can't be sure, return anyway the maximum not
- * rate-limited. This might cause a problem only for the
- * tearing synchronisation.
- */
- DSSERR("can't determine maximum RFBI transfer rate\n");
- return rfbi.l4_khz * 1000;
- }
- return rfbi.l4_khz * 1000 / min_l4_ticks;
-}
-#else
-int rfbi_get_max_tx_rate(void)
-{
- return rfbi.l4_khz * 1000;
-}
-#endif
-
static void rfbi_get_clk_info(u32 *clk_period, u32 *max_clk_div)
{
*clk_period = 1000000000 / rfbi.l4_khz;
@@ -683,44 +607,7 @@ int omap_rfbi_enable_te(bool enable, unsigned line)
}
EXPORT_SYMBOL(omap_rfbi_enable_te);
-#if 0
-static void rfbi_enable_config(int enable1, int enable2)
-{
- u32 l;
- int cs = 0;
-
- if (enable1)
- cs |= 1<<0;
- if (enable2)
- cs |= 1<<1;
-
- rfbi_enable_clocks(1);
-
- l = rfbi_read_reg(RFBI_CONTROL);
-
- l = FLD_MOD(l, cs, 3, 2);
- l = FLD_MOD(l, 0, 1, 1);
-
- rfbi_write_reg(RFBI_CONTROL, l);
-
-
- l = rfbi_read_reg(RFBI_CONFIG(0));
- l = FLD_MOD(l, 0, 3, 2); /* TRIGGERMODE: ITE */
- /*l |= FLD_VAL(2, 8, 7); */ /* L4FORMAT, 2pix/L4 */
- /*l |= FLD_VAL(0, 8, 7); */ /* L4FORMAT, 1pix/L4 */
-
- l = FLD_MOD(l, 0, 16, 16); /* A0POLARITY */
- l = FLD_MOD(l, 1, 20, 20); /* TE_VSYNC_POLARITY */
- l = FLD_MOD(l, 1, 21, 21); /* HSYNCPOLARITY */
-
- l = FLD_MOD(l, OMAP_DSS_RFBI_PARALLELMODE_8, 1, 0);
- rfbi_write_reg(RFBI_CONFIG(0), l);
-
- rfbi_enable_clocks(0);
-}
-#endif
-
-int rfbi_configure(int rfbi_module, int bpp, int lines)
+static int rfbi_configure(int rfbi_module, int bpp, int lines)
{
u32 l;
int cycle1 = 0, cycle2 = 0, cycle3 = 0;
@@ -1022,13 +909,8 @@ static int omap_rfbihw_probe(struct platform_device *pdev)
rfbi.pdev = pdev;
- spin_lock_init(&rfbi.cmd_lock);
sema_init(&rfbi.bus_lock, 1);
- init_completion(&rfbi.cmd_done);
- atomic_set(&rfbi.cmd_fifo_full, 0);
- atomic_set(&rfbi.cmd_pending, 0);
-
rfbi_mem = platform_get_resource(rfbi.pdev, IORESOURCE_MEM, 0);
if (!rfbi_mem) {
DSSERR("can't get IORESOURCE_MEM RFBI\n");
--
1.7.4.1
^ permalink raw reply related
* [PATCH 7/8] OMAP: DSS2: OMAPFB: remove dead code
From: Tomi Valkeinen @ 2011-05-10 16:24 UTC (permalink / raw)
To: linux-omap, linux-fbdev; +Cc: Tomi Valkeinen
In-Reply-To: <1305044656-31512-1-git-send-email-tomi.valkeinen@ti.com>
Remove old unused code lying inside #if 0.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
drivers/video/omap2/omapfb/omapfb-main.c | 29 -----------------------------
1 files changed, 0 insertions(+), 29 deletions(-)
diff --git a/drivers/video/omap2/omapfb/omapfb-main.c b/drivers/video/omap2/omapfb/omapfb-main.c
index 1f9cb3a..30c958b 100644
--- a/drivers/video/omap2/omapfb/omapfb-main.c
+++ b/drivers/video/omap2/omapfb/omapfb-main.c
@@ -757,35 +757,6 @@ static int omapfb_open(struct fb_info *fbi, int user)
static int omapfb_release(struct fb_info *fbi, int user)
{
-#if 0
- struct omapfb_info *ofbi = FB2OFB(fbi);
- struct omapfb2_device *fbdev = ofbi->fbdev;
- struct omap_dss_device *display = fb2display(fbi);
-
- DBG("Closing fb with plane index %d\n", ofbi->id);
-
- omapfb_lock(fbdev);
-
- if (display && display->get_update_mode && display->update) {
- /* XXX this update should be removed, I think. But it's
- * good for debugging */
- if (display->get_update_mode(display) =
- OMAP_DSS_UPDATE_MANUAL) {
- u16 w, h;
-
- if (display->sync)
- display->sync(display);
-
- display->get_resolution(display, &w, &h);
- display->update(display, 0, 0, w, h);
- }
- }
-
- if (display && display->sync)
- display->sync(display);
-
- omapfb_unlock(fbdev);
-#endif
return 0;
}
--
1.7.4.1
^ permalink raw reply related
* [PATCH 8/8] OMAP: DSS2: OMAPFB: Reduce stack usage
From: Tomi Valkeinen @ 2011-05-10 16:24 UTC (permalink / raw)
To: linux-omap, linux-fbdev; +Cc: Tomi Valkeinen
In-Reply-To: <1305044656-31512-1-git-send-email-tomi.valkeinen@ti.com>
omapfb_mode_to_timings() had struct fb_info, struct fb_var and struct
fb_ops allocated from stack. This caused the stack usage grow quite
high.
Use kzalloc to allocate the structs instead.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
drivers/video/omap2/omapfb/omapfb-main.c | 49 +++++++++++++++++-------------
1 files changed, 28 insertions(+), 21 deletions(-)
diff --git a/drivers/video/omap2/omapfb/omapfb-main.c b/drivers/video/omap2/omapfb/omapfb-main.c
index 30c958b..ae3e2be 100644
--- a/drivers/video/omap2/omapfb/omapfb-main.c
+++ b/drivers/video/omap2/omapfb/omapfb-main.c
@@ -1996,9 +1996,9 @@ static int omapfb_create_framebuffers(struct omapfb2_device *fbdev)
static int omapfb_mode_to_timings(const char *mode_str,
struct omap_video_timings *timings, u8 *bpp)
{
- struct fb_info fbi;
- struct fb_var_screeninfo var;
- struct fb_ops fbops;
+ struct fb_info *fbi;
+ struct fb_var_screeninfo *var;
+ struct fb_ops *fbops;
int r;
#ifdef CONFIG_OMAP2_DSS_VENC
@@ -2016,25 +2016,25 @@ static int omapfb_mode_to_timings(const char *mode_str,
/* this is quite a hack, but I wanted to use the modedb and for
* that we need fb_info and var, so we create dummy ones */
- memset(&fbi, 0, sizeof(fbi));
- memset(&var, 0, sizeof(var));
- memset(&fbops, 0, sizeof(fbops));
- fbi.fbops = &fbops;
+ fbi = kzalloc(sizeof(*fbi), GFP_KERNEL);
+ var = kzalloc(sizeof(*var), GFP_KERNEL);
+ fbops = kzalloc(sizeof(*fbops), GFP_KERNEL);
+ fbi->fbops = fbops;
- r = fb_find_mode(&var, &fbi, mode_str, NULL, 0, NULL, 24);
+ r = fb_find_mode(var, fbi, mode_str, NULL, 0, NULL, 24);
if (r != 0) {
- timings->pixel_clock = PICOS2KHZ(var.pixclock);
- timings->hbp = var.left_margin;
- timings->hfp = var.right_margin;
- timings->vbp = var.upper_margin;
- timings->vfp = var.lower_margin;
- timings->hsw = var.hsync_len;
- timings->vsw = var.vsync_len;
- timings->x_res = var.xres;
- timings->y_res = var.yres;
-
- switch (var.bits_per_pixel) {
+ timings->pixel_clock = PICOS2KHZ(var->pixclock);
+ timings->hbp = var->left_margin;
+ timings->hfp = var->right_margin;
+ timings->vbp = var->upper_margin;
+ timings->vfp = var->lower_margin;
+ timings->hsw = var->hsync_len;
+ timings->vsw = var->vsync_len;
+ timings->x_res = var->xres;
+ timings->y_res = var->yres;
+
+ switch (var->bits_per_pixel) {
case 16:
*bpp = 16;
break;
@@ -2045,10 +2045,17 @@ static int omapfb_mode_to_timings(const char *mode_str,
break;
}
- return 0;
+ r = 0;
} else {
- return -EINVAL;
+ *bpp = 0;
+ r = -EINVAL;
}
+
+ kfree(fbi);
+ kfree(var);
+ kfree(fbops);
+
+ return r;
}
static int omapfb_set_def_mode(struct omapfb2_device *fbdev,
--
1.7.4.1
^ permalink raw reply related
* RE: [PATCH 8/8] OMAP: DSS2: OMAPFB: Reduce stack usage
From: aaro.koskinen @ 2011-05-10 19:08 UTC (permalink / raw)
To: tomi.valkeinen, linux-omap, linux-fbdev
In-Reply-To: <1305044656-31512-9-git-send-email-tomi.valkeinen@ti.com>
Hi,
Tomi Valkeinen [tomi.valkeinen@ti.com]:
> omapfb_mode_to_timings() had struct fb_info, struct fb_var and struct
> fb_ops allocated from stack. This caused the stack usage grow quite
> high.
>
> Use kzalloc to allocate the structs instead.
[...]
> + fbi = kzalloc(sizeof(*fbi), GFP_KERNEL);
> + var = kzalloc(sizeof(*var), GFP_KERNEL);
> + fbops = kzalloc(sizeof(*fbops), GFP_KERNEL);
> + fbi->fbops = fbops;
You should check and prepare for allocation failures.
A.
^ permalink raw reply
* [PATCH v2] viafb: Automatic OLPC XO-1.5 configuration
From: Daniel Drake @ 2011-05-10 19:55 UTC (permalink / raw)
To: linux-fbdev
Currently, a long set of viafb options are needed to get the XO-1.5
laptop to output video (there is only 1 configuration that works, that
can't really be autodetected).
This patch automatically detects and configures viafb for the XO-1.5
laptop, meaning all that is required for working display is that
viafb is loaded.
Signed-off-by: Daniel Drake <dsd@laptop.org>
---
drivers/video/via/viafbdev.c | 41 ++++++++++++++++++++++++++++++++---------
1 files changed, 32 insertions(+), 9 deletions(-)
v2: incorporates all feedback from Florian
diff --git a/drivers/video/via/viafbdev.c b/drivers/video/via/viafbdev.c
index 7b4390e..7a4dd0e 100644
--- a/drivers/video/via/viafbdev.c
+++ b/drivers/video/via/viafbdev.c
@@ -24,6 +24,7 @@
#include <linux/slab.h>
#include <linux/stat.h>
#include <linux/via-core.h>
+#include <asm/olpc.h>
#define _MASTER_FILE
#include "global.h"
@@ -1011,8 +1012,13 @@ static int __init parse_active_dev(void)
/* Note: The previous of active_dev is primary device,
and the following is secondary device. */
if (!viafb_active_dev) {
- viafb_CRT_ON = STATE_ON;
- viafb_SAMM_ON = STATE_OFF;
+ if (machine_is_olpc()) { /* LCD only */
+ viafb_LCD_ON = STATE_ON;
+ viafb_SAMM_ON = STATE_OFF;
+ } else {
+ viafb_CRT_ON = STATE_ON;
+ viafb_SAMM_ON = STATE_OFF;
+ }
} else if (!strcmp(viafb_active_dev, "CRT+DVI")) {
/* CRT+DVI */
viafb_CRT_ON = STATE_ON;
@@ -1665,8 +1671,13 @@ static int parse_mode(const char *str, u32 *xres, u32 *yres)
char *ptr;
if (!str) {
- *xres = 640;
- *yres = 480;
+ if (machine_is_olpc()) {
+ *xres = 1200;
+ *yres = 900;
+ } else {
+ *xres = 640;
+ *yres = 480;
+ }
return 0;
}
@@ -1922,11 +1933,16 @@ void __devexit via_fb_pci_remove(struct pci_dev *pdev)
}
#ifndef MODULE
-static int __init viafb_setup(char *options)
+static int __init viafb_setup(void)
{
char *this_opt;
+ char *options;
+
DEBUG_MSG(KERN_INFO "viafb_setup!\n");
+ if (fb_get_options("viafb", &options))
+ return -ENODEV;
+
if (!options || !*options)
return 0;
@@ -2000,11 +2016,18 @@ static int __init viafb_setup(char *options)
int __init viafb_init(void)
{
u32 dummy_x, dummy_y;
+ int r;
+
+ if (machine_is_olpc()) {
+ /* Apply XO-1.5-specific configuration. */
+ viafb_lcd_panel_id = 23;
+ viafb_bpp = 24;
+ }
+
#ifndef MODULE
- char *option = NULL;
- if (fb_get_options("viafb", &option))
- return -ENODEV;
- viafb_setup(option);
+ r = viafb_setup();
+ if (r < 0)
+ return r;
#endif
if (parse_mode(viafb_mode, &dummy_x, &dummy_y)
|| !viafb_get_mode(dummy_x, dummy_y)
--
1.7.4.4
^ permalink raw reply related
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