* [PATCH 1/2] fbdev: da8xx:: fix reporting of the display timing info
From: Anatolij Gustschin @ 2012-03-09 15:02 UTC (permalink / raw)
To: linux-fbdev
Timing info is not properly reported by the driver, e.g.:
$ fbset -i
mode "480x272-95"
# D: 21.429 MHz, H: 33.018 kHz, V: 95.429 Hz
geometry 480 272 480 544 16
timings 46666 64 64 32 32 41 10
According to the timing values defined for LK043T1DG01 display
it should be reported as:
mode "480x272-52"
# D: 7.834 MHz, H: 14.921 kHz, V: 51.810 Hz
geometry 480 272 480 544 16
timings 127655 2 2 2 2 41 10
Initialize additional fb_var_screeninfo fields so fix this problem.
Signed-off-by: Anatolij Gustschin <agust@denx.de>
Cc: Manjunathappa, Prakash <prakash.pm@ti.com>
---
drivers/video/da8xx-fb.c | 13 +++++++++++++
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 29577bf..27c2794 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -240,6 +240,14 @@ static struct da8xx_panel known_lcd_panels[] = {
},
};
+static inline unsigned long hz_to_ps(unsigned long hz_val)
+{
+ unsigned long long num = 1000000000000ULL;
+
+ do_div(num, hz_val);
+ return (unsigned long)num;
+}
+
/* Enable the Raster Engine of the LCD Controller */
static inline void lcd_enable_raster(void)
{
@@ -1209,6 +1217,11 @@ static int __devinit fb_probe(struct platform_device *device)
da8xx_fb_var.hsync_len = lcdc_info->hsw;
da8xx_fb_var.vsync_len = lcdc_info->vsw;
+ da8xx_fb_var.right_margin = lcdc_info->hfp;
+ da8xx_fb_var.left_margin = lcdc_info->hbp;
+ da8xx_fb_var.lower_margin = lcdc_info->vfp;
+ da8xx_fb_var.upper_margin = lcdc_info->vbp;
+ da8xx_fb_var.pixclock = hz_to_ps(lcdc_info->pxl_clk);
/* Initialize fbinfo */
da8xx_fb_info->flags = FBINFO_FLAG_DEFAULT;
--
1.7.7.6
^ permalink raw reply related
* [PATCH 2/2] fbdev: da8xx: add support for SP10Q010 display
From: Anatolij Gustschin @ 2012-03-09 15:02 UTC (permalink / raw)
To: linux-fbdev
Add timing data for Hitachi SP10Q010 display and allow configuration
of the 4bpp palette. For 4bpp framebuffer enable reversed order of
pixels in a byte. This requires defining FB_CFB_REV_PIXELS_IN_BYTE
and additionally setting var.nonstd to the value FB_NONSTD_REV_PIX_IN_B.
Note that it is not enough to set da8xx_fb_var.nonstd to this value
statically, since FBIOPUT_VSCREENINFO ioctl might pass var struct with
.nonstd field set to zero or another value. Therefore this setting must
be adjusted in fb_check_var() according to the requested bpp value.
Signed-off-by: Anatolij Gustschin <agust@denx.de>
Cc: Manjunathappa, Prakash <prakash.pm@ti.com>
---
drivers/video/Kconfig | 1 +
drivers/video/da8xx-fb.c | 38 +++++++++++++++++++++++++++++++++++++-
2 files changed, 38 insertions(+), 1 deletions(-)
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 6ca0c40..ce09b17 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2233,6 +2233,7 @@ config FB_DA8XX
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
+ select FB_CFB_REV_PIXELS_IN_BYTE
---help---
This is the frame buffer device driver for the TI LCD controller
found on DA8xx/OMAP-L1xx SoCs.
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 27c2794..0a382ed 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -238,6 +238,20 @@ static struct da8xx_panel known_lcd_panels[] = {
.pxl_clk = 7833600,
.invert_pxl_clk = 0,
},
+ [2] = {
+ /* Hitachi SP10Q010 */
+ .name = "SP10Q010",
+ .width = 320,
+ .height = 240,
+ .hfp = 10,
+ .hbp = 10,
+ .hsw = 10,
+ .vfp = 10,
+ .vbp = 10,
+ .vsw = 10,
+ .pxl_clk = 7833600,
+ .invert_pxl_clk = 0,
+ },
};
static inline unsigned long hz_to_ps(unsigned long hz_val)
@@ -554,7 +568,26 @@ static int fb_setcolreg(unsigned regno, unsigned red, unsigned green,
if (info->fix.visual = FB_VISUAL_DIRECTCOLOR)
return 1;
- if (info->var.bits_per_pixel = 8) {
+ if (info->var.bits_per_pixel = 4) {
+ if (regno > 15)
+ return 1;
+
+ if (info->var.grayscale) {
+ pal = regno;
+ } else {
+ red >>= 4;
+ green >>= 8;
+ blue >>= 12;
+
+ pal = (red & 0x0f00);
+ pal |= (green & 0x00f0);
+ pal |= (blue & 0x000f);
+ }
+ if (regno = 0)
+ pal |= 0x2000;
+ palette[regno] = pal;
+
+ } else if (info->var.bits_per_pixel = 8) {
red >>= 4;
green >>= 8;
blue >>= 12;
@@ -809,6 +842,7 @@ static int fb_check_var(struct fb_var_screeninfo *var,
var->blue.length = 8;
var->transp.offset = 0;
var->transp.length = 0;
+ var->nonstd = 0;
break;
case 4:
var->red.offset = 0;
@@ -819,6 +853,7 @@ static int fb_check_var(struct fb_var_screeninfo *var,
var->blue.length = 4;
var->transp.offset = 0;
var->transp.length = 0;
+ var->nonstd = FB_NONSTD_REV_PIX_IN_B;
break;
case 16: /* RGB 565 */
var->red.offset = 11;
@@ -829,6 +864,7 @@ static int fb_check_var(struct fb_var_screeninfo *var,
var->blue.length = 5;
var->transp.offset = 0;
var->transp.length = 0;
+ var->nonstd = 0;
break;
default:
err = -EINVAL;
--
1.7.7.6
^ permalink raw reply related
* [PATCH 7/7] sm501: restore big-endian operation.
From: Thomas Schwinge @ 2012-03-09 16:38 UTC (permalink / raw)
Cc: Thomas Schwinge, Paul Mundt, linux-sh, Heiko Schocher,
Samuel Ortiz, linux-fbdev, devicetree-discuss, Ben Dooks,
Vincent Sanders, Randy Dunlap
In-Reply-To: <1331311133-26937-1-git-send-email-thomas@codesourcery.com>
On SH, as of 37b7a97884ba64bf7d403351ac2a9476ab4f1bba we have to use the
endianess-agnostic I/O accessor functions.
In commit bf5f0019046d596d613caf74722ba4994e153899, Heiko fixed this for 32-bit
PowerPC; my patch now generalizes upon that.
The device is now recognized correctly for both litte-endian and big-endian
sh7785lcr, but I have not tested this any further, as the board is situated in
a remote data center.
Signed-off-by: Thomas Schwinge <thomas@codesourcery.com>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: linux-sh@vger.kernel.org
Cc: Heiko Schocher <hs@denx.de>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: linux-fbdev@vger.kernel.org
Cc: devicetree-discuss@ozlabs.org
Cc: Ben Dooks <ben@simtec.co.uk>
Cc: Vincent Sanders <vince@simtec.co.uk>
Cc: Randy Dunlap <rdunlap@xenotime.net>
---
include/linux/sm501.h | 9 ++-------
1 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/include/linux/sm501.h b/include/linux/sm501.h
index 02fde50..0312e3c 100644
--- a/include/linux/sm501.h
+++ b/include/linux/sm501.h
@@ -173,10 +173,5 @@ struct sm501_platdata {
unsigned int gpio_i2c_nr;
};
-#if defined(CONFIG_PPC32)
-#define smc501_readl(addr) ioread32be((addr))
-#define smc501_writel(val, addr) iowrite32be((val), (addr))
-#else
-#define smc501_readl(addr) readl(addr)
-#define smc501_writel(val, addr) writel(val, addr)
-#endif
+#define smc501_readl(addr) __raw_readl(addr)
+#define smc501_writel(val, addr) __raw_writel(val, addr)
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH 7/7] OMAPDSS: HDMI: hot plug detect fix
From: Tomi Valkeinen @ 2012-03-10 7:29 UTC (permalink / raw)
To: Greg KH; +Cc: stable, linux-fbdev, linux-omap, Rob Clark
In-Reply-To: <20120308152949.GB5277@kroah.com>
On Thu, 2012-03-08 at 07:29 -0800, Greg KH wrote:
> On Thu, Mar 08, 2012 at 09:35:13AM +0200, Tomi Valkeinen wrote:
> > On Wed, 2012-03-07 at 12:01 -0800, Greg KH wrote:
> > > On Thu, Mar 01, 2012 at 02:26:35PM +0200, Tomi Valkeinen wrote:
> > > > From: Rob Clark <rob@ti.com>
> > > >
> > > > The "OMAPDSS: HDMI: PHY burnout fix" commit switched the HDMI driver
> > > > over to using a GPIO for plug detect. Unfortunately the ->detect()
> > > > method was not also updated, causing HDMI to no longer work for the
> > > > omapdrm driver (because it would actually check if a connection was
> > > > detected before attempting to enable display).
> > > >
> > > > Signed-off-by: Rob Clark <rob@ti.com>
> > > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > >
> > > You forgot to tell me what the git commit id is for this patch (it's
> > > ca888a7958b3d808e4efd08ceff88913f4212c69, right?)
> >
> > Yes, that's the one. It wasn't in Linus's tree yet, only in fbdev tree,
> > so I wasn't sure what the commit id is.
>
> Then you should not have sent it to me, as if I were to take it then, I
> could not have :(
Oh, ok. I thought the "patch-must-be-in-mainline"-rule was not a totally
strict one, so I decided to include it in this case as the patch was a
rather trivial one and already in the fbdev tree (I mentioned it in the
intro mail).
I guess I got lucky and the patch got into mainline before you took the
patches.
> > > And why isn't this needed for the 3.0 kernel as well?
> >
> > The detect() function is not present in 3.0, so there was nothing to
> > break.
>
> Ok, so everything I've queued up is all that is needed, right?
Yes, looks correct. Thanks!
Tomi
^ permalink raw reply
* Re: [PATCH] Video : Amba: Use in_interrupt() in clcdfb_sleep().
From: richard -rw- weinberger @ 2012-03-11 13:22 UTC (permalink / raw)
To: santosh nayak
Cc: linux, FlorianSchandinat, linux-fbdev, linux-kernel,
kernel-janitors
In-Reply-To: <1331471665-22226-1-git-send-email-santoshprasadnayak@gmail.com>
On Sun, Mar 11, 2012 at 2:14 PM, santosh nayak
<santoshprasadnayak@gmail.com> wrote:
> From: Santosh Nayak <santoshprasadnayak@gmail.com>
>
> Instead of "in_atomic()", we can use in_interrupt() to check whether
> its an interrupt context.
What is the benefit?
Your change turns clcdfb_sleep() into a ticking bomb.
If clcdfb_sleep() is called from an atomic section (e.g. under a
spinlock) it will call msleep()
and the whole thing explodes...
--
Thanks,
//richard
^ permalink raw reply
* [PATCH] Video : Amba: Use in_interrupt() in clcdfb_sleep().
From: santosh nayak @ 2012-03-11 13:26 UTC (permalink / raw)
To: linux
Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors,
Santosh Nayak
In-Reply-To: <CAFLxGvygW=DeSQbRq8Wbcdy5E3e4vYcN666_9oWgnuwzT1WKqw@mail.gmail.com>
From: Santosh Nayak <santoshprasadnayak@gmail.com>
Instead of "in_atomic()", we can use in_interrupt() to check whether
its an interrupt context.
Signed-off-by: Santosh Nayak <santoshprasadnayak@gmail.com>
---
drivers/video/amba-clcd.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/video/amba-clcd.c b/drivers/video/amba-clcd.c
index 0a2cce7..63c25da 100644
--- a/drivers/video/amba-clcd.c
+++ b/drivers/video/amba-clcd.c
@@ -39,7 +39,7 @@ static const char *clcd_name = "CLCD FB";
*/
static inline void clcdfb_sleep(unsigned int ms)
{
- if (in_atomic()) {
+ if (in_interrupt()) {
mdelay(ms);
} else {
msleep(ms);
--
1.7.4.4
^ permalink raw reply related
* Re: [PATCH] Video : Amba: Use in_interrupt() in clcdfb_sleep().
From: Russell King - ARM Linux @ 2012-03-11 13:27 UTC (permalink / raw)
To: santosh nayak
Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors
In-Reply-To: <1331471665-22226-1-git-send-email-santoshprasadnayak@gmail.com>
On Sun, Mar 11, 2012 at 06:44:25PM +0530, santosh nayak wrote:
> From: Santosh Nayak <santoshprasadnayak@gmail.com>
>
> Instead of "in_atomic()", we can use in_interrupt() to check whether
> its an interrupt context.
What are you trying to fix?
Your description is an example of a bad commit comment. It merely
describes the change, which anyone can see by looking at the diff.
What it totally and utterly fails to do is to describe _why_ the
change is necessary or what the problem is.
So, until it does, this patch gets a definite NAK.
^ permalink raw reply
* Re: [PATCH] Video : Amba: Use in_interrupt() in clcdfb_sleep().
From: santosh prasad nayak @ 2012-03-11 14:29 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors
In-Reply-To: <20120311132746.GA13336@n2100.arm.linux.org.uk>
Not to use in_atomic() in driver code.
Following article inspired me to do the change.
http://lwn.net/Articles/274695/
"in_atomic() is for core kernel use only. Because in special
circumstances (ie: kmap_atomic()) we run inc_preempt_count() even on
non-preemptible kernels to tell the per-arch fault handler that it was
invoked by copy_*_user() inside kmap_atomic(), and it must fail.
In other words, in_atomic() works in a specific low-level situation,
but it was never meant to be used in a wider context. Its placement in
hardirq.h next to macros which can be used elsewhere was, thus, almost
certainly a mistake. As Alan Stern pointed out, the fact that Linux
Device Drivers recommends the use of in_atomic() will not have helped
the situation. Your editor recommends that the authors of that book be
immediately sacked. "
In the present case, we just check whether its an IRQ context or user
context. So for that
we can use "in_interrupt()".
Greg also mentions the same in the following mail.
http://www.spinics.net/lists/newbies/msg43402.html
Regards
Santosh
On Sun, Mar 11, 2012 at 6:57 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sun, Mar 11, 2012 at 06:44:25PM +0530, santosh nayak wrote:
>> From: Santosh Nayak <santoshprasadnayak@gmail.com>
>>
>> Instead of "in_atomic()", we can use in_interrupt() to check whether
>> its an interrupt context.
>
> What are you trying to fix?
>
> Your description is an example of a bad commit comment. It merely
> describes the change, which anyone can see by looking at the diff.
> What it totally and utterly fails to do is to describe _why_ the
> change is necessary or what the problem is.
>
> So, until it does, this patch gets a definite NAK.
^ permalink raw reply
* Re: [PATCH] Video : Amba: Use in_interrupt() in clcdfb_sleep().
From: Russell King - ARM Linux @ 2012-03-11 14:36 UTC (permalink / raw)
To: santosh prasad nayak
Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors
In-Reply-To: <CAOD=uF7kQHABCmxUL3-J-hZ1E2mXrb7dgsYq7msHhiPJ4Gm-Ew@mail.gmail.com>
On Sun, Mar 11, 2012 at 07:47:27PM +0530, santosh prasad nayak wrote:
> Not to use in_atomic() in driver code.
>
> Following article inspired me to do the change.
> http://lwn.net/Articles/274695/
>
> "in_atomic() is for core kernel use only. Because in special
> circumstances (ie: kmap_atomic()) we run inc_preempt_count() even on
> non-preemptible kernels to tell the per-arch fault handler that it was
> invoked by copy_*_user() inside kmap_atomic(), and it must fail.
> In other words, in_atomic() works in a specific low-level situation,
> but it was never meant to be used in a wider context. Its placement in
> hardirq.h next to macros which can be used elsewhere was, thus, almost
> certainly a mistake. As Alan Stern pointed out, the fact that Linux
> Device Drivers recommends the use of in_atomic() will not have helped
> the situation. Your editor recommends that the authors of that book be
> immediately sacked. "
>
> In the present case, we just check whether its an IRQ context or user
> context. So for that
> we can use "in_interrupt()".
>
> Greg also mentions the same in the following mail.
> http://www.spinics.net/lists/newbies/msg43402.html
In which case, we'll just have to do mdelay() and forget about allowing
anything else to run for the 20ms that we need to sleep. Sucky but
that's the way things are.
^ permalink raw reply
* Re: [PATCH] Video : Amba: Use in_interrupt() in clcdfb_sleep().
From: santosh prasad nayak @ 2012-03-11 14:49 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors
In-Reply-To: <20120311143615.GB13336@n2100.arm.linux.org.uk>
On Sun, Mar 11, 2012 at 8:06 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sun, Mar 11, 2012 at 07:47:27PM +0530, santosh prasad nayak wrote:
>> Not to use in_atomic() in driver code.
>>
>> Following article inspired me to do the change.
>> http://lwn.net/Articles/274695/
>>
>> "in_atomic() is for core kernel use only. Because in special
>> circumstances (ie: kmap_atomic()) we run inc_preempt_count() even on
>> non-preemptible kernels to tell the per-arch fault handler that it was
>> invoked by copy_*_user() inside kmap_atomic(), and it must fail.
>> In other words, in_atomic() works in a specific low-level situation,
>> but it was never meant to be used in a wider context. Its placement in
>> hardirq.h next to macros which can be used elsewhere was, thus, almost
>> certainly a mistake. As Alan Stern pointed out, the fact that Linux
>> Device Drivers recommends the use of in_atomic() will not have helped
>> the situation. Your editor recommends that the authors of that book be
>> immediately sacked. "
>>
>> In the present case, we just check whether its an IRQ context or user
>> context. So for that
>> we can use "in_interrupt()".
>>
>> Greg also mentions the same in the following mail.
>> http://www.spinics.net/lists/newbies/msg43402.html
>
> In which case, we'll just have to do mdelay() and forget about allowing
> anything else to run for the 20ms that we need to sleep. Sucky but
> that's the way things are.
mdelay() or msleep() are there before. I did not change that.
my point is : in_atomic() vs "in_interrupt()".
We should avoid to use "in_atomic()" in driver code.
In the present case to check IRQ context "in_interrupt()" should be preferred.
regards
Santosh
regards
Santosh
^ permalink raw reply
* Re: [PATCH] Video : Amba: Use in_interrupt() in clcdfb_sleep().
From: Russell King - ARM Linux @ 2012-03-11 15:03 UTC (permalink / raw)
To: santosh prasad nayak
Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors
In-Reply-To: <CAOD=uF5GbsyHS5iXjJ1YcqJyGLHwT8Wjhiie1daJAVqu527Umg@mail.gmail.com>
On Sun, Mar 11, 2012 at 08:18:32PM +0530, santosh prasad nayak wrote:
> On Sun, Mar 11, 2012 at 8:06 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Sun, Mar 11, 2012 at 07:47:27PM +0530, santosh prasad nayak wrote:
> >> Not to use in_atomic() in driver code.
> >>
> >> Following article inspired me to do the change.
> >> http://lwn.net/Articles/274695/
> >>
> >> "in_atomic() is for core kernel use only. Because in special
> >> circumstances (ie: kmap_atomic()) we run inc_preempt_count() even on
> >> non-preemptible kernels to tell the per-arch fault handler that it was
> >> invoked by copy_*_user() inside kmap_atomic(), and it must fail.
> >> In other words, in_atomic() works in a specific low-level situation,
> >> but it was never meant to be used in a wider context. Its placement in
> >> hardirq.h next to macros which can be used elsewhere was, thus, almost
> >> certainly a mistake. As Alan Stern pointed out, the fact that Linux
> >> Device Drivers recommends the use of in_atomic() will not have helped
> >> the situation. Your editor recommends that the authors of that book be
> >> immediately sacked. "
> >>
> >> In the present case, we just check whether its an IRQ context or user
> >> context. So for that
> >> we can use "in_interrupt()".
> >>
> >> Greg also mentions the same in the following mail.
> >> http://www.spinics.net/lists/newbies/msg43402.html
> >
> > In which case, we'll just have to do mdelay() and forget about allowing
> > anything else to run for the 20ms that we need to sleep. Sucky but
> > that's the way things are.
>
> mdelay() or msleep() are there before. I did not change that.
>
>
> my point is : in_atomic() vs "in_interrupt()".
> We should avoid to use "in_atomic()" in driver code.
>
> In the present case to check IRQ context "in_interrupt()" should be preferred.
in_interrupt() won't tell us if we're being called with spinlocks held,
which _is_ a possibility because this can be called from printk(), for
oops dumps and the like.
in_interrupt() just means that we're inside a hard or soft interrupt,
or nmi. It says nothing about whether msleep() is possible.
^ permalink raw reply
* Re: [PATCH] Video : Amba: Use in_interrupt() in clcdfb_sleep().
From: santosh prasad nayak @ 2012-03-11 15:31 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors
In-Reply-To: <20120311150311.GC13336@n2100.arm.linux.org.uk>
On Sun, Mar 11, 2012 at 8:33 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sun, Mar 11, 2012 at 08:18:32PM +0530, santosh prasad nayak wrote:
>> On Sun, Mar 11, 2012 at 8:06 PM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Sun, Mar 11, 2012 at 07:47:27PM +0530, santosh prasad nayak wrote:
>> >> Not to use in_atomic() in driver code.
>> >>
>> >> Following article inspired me to do the change.
>> >> http://lwn.net/Articles/274695/
>> >>
>> >> "in_atomic() is for core kernel use only. Because in special
>> >> circumstances (ie: kmap_atomic()) we run inc_preempt_count() even on
>> >> non-preemptible kernels to tell the per-arch fault handler that it was
>> >> invoked by copy_*_user() inside kmap_atomic(), and it must fail.
>> >> In other words, in_atomic() works in a specific low-level situation,
>> >> but it was never meant to be used in a wider context. Its placement in
>> >> hardirq.h next to macros which can be used elsewhere was, thus, almost
>> >> certainly a mistake. As Alan Stern pointed out, the fact that Linux
>> >> Device Drivers recommends the use of in_atomic() will not have helped
>> >> the situation. Your editor recommends that the authors of that book be
>> >> immediately sacked. "
>> >>
>> >> In the present case, we just check whether its an IRQ context or user
>> >> context. So for that
>> >> we can use "in_interrupt()".
>> >>
>> >> Greg also mentions the same in the following mail.
>> >> http://www.spinics.net/lists/newbies/msg43402.html
>> >
>> > In which case, we'll just have to do mdelay() and forget about allowing
>> > anything else to run for the 20ms that we need to sleep. Sucky but
>> > that's the way things are.
>>
>> mdelay() or msleep() are there before. I did not change that.
>>
>>
>> my point is : in_atomic() vs "in_interrupt()".
>> We should avoid to use "in_atomic()" in driver code.
>>
>> In the present case to check IRQ context "in_interrupt()" should be preferred.
>
> in_interrupt() won't tell us if we're being called with spinlocks held,
> which _is_ a possibility because this can be called from printk(), for
> oops dumps and the like.
>
> in_interrupt() just means that we're inside a hard or soft interrupt,
> or nmi. It says nothing about whether msleep() is possible.
in_atomic() is also not error free. I found following comment in
include/linux/hardirq.h. How do you handle it in non-preemptible
kernel ?
/*
* Are we running in atomic context? WARNING: this macro cannot
* always detect atomic context; in particular, it cannot know about
* held spinlocks in non-preemptible kernels. Thus it should not be
* used in the general case to determine whether sleeping is possible.
* Do not use in_atomic() in driver code.
*/
#define in_atomic() ((preempt_count() & ~PREEMPT_ACTIVE) != 0)
regards
Santosh
^ permalink raw reply
* [PATCH] OMAPDSS: provide default timings functions for panels
From: Grazvydas Ignotas @ 2012-03-11 15:34 UTC (permalink / raw)
To: linux-fbdev; +Cc: Tomi Valkeinen, linux-omap, Grazvydas Ignotas
With this we can eliminate some duplicate code in panel drivers.
Also lgphilips-lb035q02, nec-nl8048hl11-01b, picodlp and
tpo-td043mtea1 gain support of timings control over sysfs.
Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
---
drivers/video/omap2/displays/panel-acx565akm.c | 15 ---------------
drivers/video/omap2/displays/panel-generic-dpi.c | 22 ----------------------
drivers/video/omap2/displays/panel-n8x0.c | 8 --------
drivers/video/omap2/displays/panel-taal.c | 8 --------
drivers/video/omap2/dss/core.c | 6 ++++++
drivers/video/omap2/dss/display.c | 21 +++++++++++++++++++++
drivers/video/omap2/dss/venc.c | 7 -------
include/video/omapdss.h | 6 ++++++
8 files changed, 33 insertions(+), 60 deletions(-)
diff --git a/drivers/video/omap2/displays/panel-acx565akm.c b/drivers/video/omap2/displays/panel-acx565akm.c
index dbd59b8..be83eed 100644
--- a/drivers/video/omap2/displays/panel-acx565akm.c
+++ b/drivers/video/omap2/displays/panel-acx565akm.c
@@ -738,19 +738,6 @@ static void acx_panel_set_timings(struct omap_dss_device *dssdev,
}
}
-static void acx_panel_get_timings(struct omap_dss_device *dssdev,
- struct omap_video_timings *timings)
-{
- *timings = dssdev->panel.timings;
-}
-
-static int acx_panel_check_timings(struct omap_dss_device *dssdev,
- struct omap_video_timings *timings)
-{
- return 0;
-}
-
-
static struct omap_dss_driver acx_panel_driver = {
.probe = acx_panel_probe,
.remove = acx_panel_remove,
@@ -761,8 +748,6 @@ static struct omap_dss_driver acx_panel_driver = {
.resume = acx_panel_resume,
.set_timings = acx_panel_set_timings,
- .get_timings = acx_panel_get_timings,
- .check_timings = acx_panel_check_timings,
.get_recommended_bpp = acx_get_recommended_bpp,
diff --git a/drivers/video/omap2/displays/panel-generic-dpi.c b/drivers/video/omap2/displays/panel-generic-dpi.c
index 519c47d..45a46af 100644
--- a/drivers/video/omap2/displays/panel-generic-dpi.c
+++ b/drivers/video/omap2/displays/panel-generic-dpi.c
@@ -454,24 +454,6 @@ static int generic_dpi_panel_resume(struct omap_dss_device *dssdev)
return 0;
}
-static void generic_dpi_panel_set_timings(struct omap_dss_device *dssdev,
- struct omap_video_timings *timings)
-{
- dpi_set_timings(dssdev, timings);
-}
-
-static void generic_dpi_panel_get_timings(struct omap_dss_device *dssdev,
- struct omap_video_timings *timings)
-{
- *timings = dssdev->panel.timings;
-}
-
-static int generic_dpi_panel_check_timings(struct omap_dss_device *dssdev,
- struct omap_video_timings *timings)
-{
- return dpi_check_timings(dssdev, timings);
-}
-
static struct omap_dss_driver dpi_driver = {
.probe = generic_dpi_panel_probe,
.remove = __exit_p(generic_dpi_panel_remove),
@@ -481,10 +463,6 @@ static struct omap_dss_driver dpi_driver = {
.suspend = generic_dpi_panel_suspend,
.resume = generic_dpi_panel_resume,
- .set_timings = generic_dpi_panel_set_timings,
- .get_timings = generic_dpi_panel_get_timings,
- .check_timings = generic_dpi_panel_check_timings,
-
.driver = {
.name = "generic_dpi_panel",
.owner = THIS_MODULE,
diff --git a/drivers/video/omap2/displays/panel-n8x0.c b/drivers/video/omap2/displays/panel-n8x0.c
index 150e8ba..eba98a0 100644
--- a/drivers/video/omap2/displays/panel-n8x0.c
+++ b/drivers/video/omap2/displays/panel-n8x0.c
@@ -610,12 +610,6 @@ static int n8x0_panel_resume(struct omap_dss_device *dssdev)
return 0;
}
-static void n8x0_panel_get_timings(struct omap_dss_device *dssdev,
- struct omap_video_timings *timings)
-{
- *timings = dssdev->panel.timings;
-}
-
static void n8x0_panel_get_resolution(struct omap_dss_device *dssdev,
u16 *xres, u16 *yres)
{
@@ -678,8 +672,6 @@ static struct omap_dss_driver n8x0_panel_driver = {
.get_resolution = n8x0_panel_get_resolution,
.get_recommended_bpp = omapdss_default_get_recommended_bpp,
- .get_timings = n8x0_panel_get_timings,
-
.driver = {
.name = "n8x0_panel",
.owner = THIS_MODULE,
diff --git a/drivers/video/omap2/displays/panel-taal.c b/drivers/video/omap2/displays/panel-taal.c
index 80c3f6a..174c004 100644
--- a/drivers/video/omap2/displays/panel-taal.c
+++ b/drivers/video/omap2/displays/panel-taal.c
@@ -583,12 +583,6 @@ static const struct backlight_ops taal_bl_ops = {
.update_status = taal_bl_update_status,
};
-static void taal_get_timings(struct omap_dss_device *dssdev,
- struct omap_video_timings *timings)
-{
- *timings = dssdev->panel.timings;
-}
-
static void taal_get_resolution(struct omap_dss_device *dssdev,
u16 *xres, u16 *yres)
{
@@ -1899,8 +1893,6 @@ static struct omap_dss_driver taal_driver = {
.run_test = taal_run_test,
.memory_read = taal_memory_read,
- .get_timings = taal_get_timings,
-
.driver = {
.name = "taal",
.owner = THIS_MODULE,
diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
index 86ec12e..9dc0c10 100644
--- a/drivers/video/omap2/dss/core.c
+++ b/drivers/video/omap2/dss/core.c
@@ -434,6 +434,12 @@ int omap_dss_register_driver(struct omap_dss_driver *dssdriver)
if (dssdriver->get_recommended_bpp = NULL)
dssdriver->get_recommended_bpp omapdss_default_get_recommended_bpp;
+ if (dssdriver->set_timings = NULL)
+ dssdriver->set_timings = omapdss_default_set_timings;
+ if (dssdriver->get_timings = NULL)
+ dssdriver->get_timings = omapdss_default_get_timings;
+ if (dssdriver->check_timings = NULL)
+ dssdriver->check_timings = omapdss_default_check_timings;
return driver_register(&dssdriver->driver);
}
diff --git a/drivers/video/omap2/dss/display.c b/drivers/video/omap2/dss/display.c
index be331dc..40746d9 100644
--- a/drivers/video/omap2/dss/display.c
+++ b/drivers/video/omap2/dss/display.c
@@ -318,6 +318,27 @@ int omapdss_default_get_recommended_bpp(struct omap_dss_device *dssdev)
}
EXPORT_SYMBOL(omapdss_default_get_recommended_bpp);
+void omapdss_default_set_timings(struct omap_dss_device *dssdev,
+ struct omap_video_timings *timings)
+{
+ dpi_set_timings(dssdev, timings);
+}
+EXPORT_SYMBOL(omapdss_default_set_timings);
+
+void omapdss_default_get_timings(struct omap_dss_device *dssdev,
+ struct omap_video_timings *timings)
+{
+ *timings = dssdev->panel.timings;
+}
+EXPORT_SYMBOL(omapdss_default_get_timings);
+
+int omapdss_default_check_timings(struct omap_dss_device *dssdev,
+ struct omap_video_timings *timings)
+{
+ return dpi_check_timings(dssdev, timings);
+}
+EXPORT_SYMBOL(omapdss_default_check_timings);
+
/* Checks if replication logic should be used. Only use for active matrix,
* when overlay is in RGB12U or RGB16 mode, and LCD interface is
* 18bpp or 24bpp */
diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
index 7bb6219..dee2ec4 100644
--- a/drivers/video/omap2/dss/venc.c
+++ b/drivers/video/omap2/dss/venc.c
@@ -557,12 +557,6 @@ static int venc_panel_resume(struct omap_dss_device *dssdev)
return venc_panel_enable(dssdev);
}
-static void venc_get_timings(struct omap_dss_device *dssdev,
- struct omap_video_timings *timings)
-{
- *timings = dssdev->panel.timings;
-}
-
static void venc_set_timings(struct omap_dss_device *dssdev,
struct omap_video_timings *timings)
{
@@ -641,7 +635,6 @@ static struct omap_dss_driver venc_driver = {
.get_resolution = omapdss_default_get_resolution,
.get_recommended_bpp = omapdss_default_get_recommended_bpp,
- .get_timings = venc_get_timings,
.set_timings = venc_set_timings,
.check_timings = venc_check_timings,
diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index 378c7ed..ff1dacd 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -646,6 +646,12 @@ struct omap_overlay *omap_dss_get_overlay(int num);
void omapdss_default_get_resolution(struct omap_dss_device *dssdev,
u16 *xres, u16 *yres);
int omapdss_default_get_recommended_bpp(struct omap_dss_device *dssdev);
+void omapdss_default_set_timings(struct omap_dss_device *dssdev,
+ struct omap_video_timings *timings);
+void omapdss_default_get_timings(struct omap_dss_device *dssdev,
+ struct omap_video_timings *timings);
+int omapdss_default_check_timings(struct omap_dss_device *dssdev,
+ struct omap_video_timings *timings);
typedef void (*omap_dispc_isr_t) (void *arg, u32 mask);
int omap_dispc_register_isr(omap_dispc_isr_t isr, void *arg, u32 mask);
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] Video : Amba: Use in_interrupt() in clcdfb_sleep().
From: Russell King - ARM Linux @ 2012-03-11 15:48 UTC (permalink / raw)
To: santosh prasad nayak
Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors
In-Reply-To: <CAOD=uF5F4tOG0Y2tZdDeMgqs4-46HXYzA3RR-t8KuNuYRarJbg@mail.gmail.com>
On Sun, Mar 11, 2012 at 08:49:27PM +0530, santosh prasad nayak wrote:
> On Sun, Mar 11, 2012 at 8:33 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > in_interrupt() won't tell us if we're being called with spinlocks held,
> > which _is_ a possibility because this can be called from printk(), for
> > oops dumps and the like.
> >
> > in_interrupt() just means that we're inside a hard or soft interrupt,
> > or nmi. It says nothing about whether msleep() is possible.
>
>
> in_atomic() is also not error free. I found following comment in
> include/linux/hardirq.h. How do you handle it in non-preemptible
> kernel ?
>
> /*
> * Are we running in atomic context? WARNING: this macro cannot
> * always detect atomic context; in particular, it cannot know about
> * held spinlocks in non-preemptible kernels. Thus it should not be
> * used in the general case to determine whether sleeping is possible.
> * Do not use in_atomic() in driver code.
> */
> #define in_atomic() ((preempt_count() & ~PREEMPT_ACTIVE) != 0)
That may be, but the fact of the matter is that no one has *ever*
reported an incident where this has failed at this point - and when
it does people will end up with a might_sleep() warning from msleep().
Maybe those who are saying people should not use this should instead
be analysing why people use this, and suggest an alternative solution
to the problem instead of a basic and uninformative "you shouldn't use
this" statement.
As I've said, if we aren't going to use this, then the only solution is
to completely omit the msleep() there and just say "sod you to running
anything else for 20ms while this driver busy-spins." That's
ultimately the safe thing to do, and at the moment I see no other
alternative there.
^ permalink raw reply
* Re: [PATCH] Video : Amba: Use in_interrupt() in clcdfb_sleep().
From: Russell King - ARM Linux @ 2012-03-11 16:42 UTC (permalink / raw)
To: santosh prasad nayak
Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors
In-Reply-To: <CAOD=uF6OtpX-1eoJB3X5ZywNUeU2_-tsE+Ksu33ZFXqoxVK_aQ@mail.gmail.com>
On Sun, Mar 11, 2012 at 10:07:18PM +0530, santosh prasad nayak wrote:
> On Sun, Mar 11, 2012 at 9:18 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Sun, Mar 11, 2012 at 08:49:27PM +0530, santosh prasad nayak wrote:
> >> On Sun, Mar 11, 2012 at 8:33 PM, Russell King - ARM Linux
> >> <linux@arm.linux.org.uk> wrote:
> >> > in_interrupt() won't tell us if we're being called with spinlocks held,
> >> > which _is_ a possibility because this can be called from printk(), for
> >> > oops dumps and the like.
> >> >
> >> > in_interrupt() just means that we're inside a hard or soft interrupt,
> >> > or nmi. It says nothing about whether msleep() is possible.
> >>
> >>
> >> in_atomic() is also not error free. I found following comment in
> >> include/linux/hardirq.h. How do you handle it in non-preemptible
> >> kernel ?
> >>
> >> /*
> >> * Are we running in atomic context? WARNING: this macro cannot
> >> * always detect atomic context; in particular, it cannot know about
> >> * held spinlocks in non-preemptible kernels. Thus it should not be
> >> * used in the general case to determine whether sleeping is possible.
> >> * Do not use in_atomic() in driver code.
> >> */
> >> #define in_atomic() ((preempt_count() & ~PREEMPT_ACTIVE) != 0)
> >
> > That may be, but the fact of the matter is that no one has *ever*
> > reported an incident where this has failed at this point - and when
> > it does people will end up with a might_sleep() warning from msleep().
> >
> > Maybe those who are saying people should not use this should instead
> > be analysing why people use this, and suggest an alternative solution
> > to the problem instead of a basic and uninformative "you shouldn't use
> > this" statement.
>
> The reason is given in the article.
At this point I'm just going to restate what I said above and below, so
I'm not even going to bother doing that, and instead just say that. I'm
not arguing whether it's right or wrong. I'm just stating that the only
solution I see is to get rid of msleep() in there entirely.
> http://lwn.net/Articles/274695/
>
> "The in_atomic() macro works by checking whether preemption is
> disabled, which seems like the right thing to do. Handlers for events
> like hardware interrupts will disable preemption, but so will the
> acquisition of a spinlock. So this test appears to catch all of the
> cases where sleeping would be a bad idea. Certainly a number of people
> who have looked at this macro have come to that conclusion.
>
> But if preemption has not been configured into the kernel in the first
> place, the kernel does not raise the "preemption count" when spinlocks
> are acquired. So, in this situation (which is common - many
> distributors still do not enable preemption in their kernels),
> in_atomic() has no way to know if the calling code holds any spinlocks
> or not. So it will return zero (indicating process context) even when
> spinlocks are held. And that could lead to kernel code thinking that
> it is running in process context (and acting accordingly) when, in
> fact, it is not."
>
>
>
>
> regards
> Santosh
> >
> > As I've said, if we aren't going to use this, then the only solution is
> > to completely omit the msleep() there and just say "sod you to running
> > anything else for 20ms while this driver busy-spins." That's
> > ultimately the safe thing to do, and at the moment I see no other
> > alternative there.
^ permalink raw reply
* Re: [PATCH] Video : Amba: Use in_interrupt() in clcdfb_sleep().
From: santosh prasad nayak @ 2012-03-11 16:49 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors
In-Reply-To: <20120311154846.GD13336@n2100.arm.linux.org.uk>
On Sun, Mar 11, 2012 at 9:18 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sun, Mar 11, 2012 at 08:49:27PM +0530, santosh prasad nayak wrote:
>> On Sun, Mar 11, 2012 at 8:33 PM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > in_interrupt() won't tell us if we're being called with spinlocks held,
>> > which _is_ a possibility because this can be called from printk(), for
>> > oops dumps and the like.
>> >
>> > in_interrupt() just means that we're inside a hard or soft interrupt,
>> > or nmi. It says nothing about whether msleep() is possible.
>>
>>
>> in_atomic() is also not error free. I found following comment in
>> include/linux/hardirq.h. How do you handle it in non-preemptible
>> kernel ?
>>
>> /*
>> * Are we running in atomic context? WARNING: this macro cannot
>> * always detect atomic context; in particular, it cannot know about
>> * held spinlocks in non-preemptible kernels. Thus it should not be
>> * used in the general case to determine whether sleeping is possible.
>> * Do not use in_atomic() in driver code.
>> */
>> #define in_atomic() ((preempt_count() & ~PREEMPT_ACTIVE) != 0)
>
> That may be, but the fact of the matter is that no one has *ever*
> reported an incident where this has failed at this point - and when
> it does people will end up with a might_sleep() warning from msleep().
>
> Maybe those who are saying people should not use this should instead
> be analysing why people use this, and suggest an alternative solution
> to the problem instead of a basic and uninformative "you shouldn't use
> this" statement.
The reason is given in the article.
http://lwn.net/Articles/274695/
"The in_atomic() macro works by checking whether preemption is
disabled, which seems like the right thing to do. Handlers for events
like hardware interrupts will disable preemption, but so will the
acquisition of a spinlock. So this test appears to catch all of the
cases where sleeping would be a bad idea. Certainly a number of people
who have looked at this macro have come to that conclusion.
But if preemption has not been configured into the kernel in the first
place, the kernel does not raise the "preemption count" when spinlocks
are acquired. So, in this situation (which is common - many
distributors still do not enable preemption in their kernels),
in_atomic() has no way to know if the calling code holds any spinlocks
or not. So it will return zero (indicating process context) even when
spinlocks are held. And that could lead to kernel code thinking that
it is running in process context (and acting accordingly) when, in
fact, it is not."
regards
Santosh
>
> As I've said, if we aren't going to use this, then the only solution is
> to completely omit the msleep() there and just say "sod you to running
> anything else for 20ms while this driver busy-spins." That's
> ultimately the safe thing to do, and at the moment I see no other
> alternative there.
^ permalink raw reply
* Re: [PATCH] Video : Amba: Use in_interrupt() in clcdfb_sleep().
From: santosh prasad nayak @ 2012-03-11 16:58 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors
In-Reply-To: <20120311164243.GE13336@n2100.arm.linux.org.uk>
Russel,
Is this what you want ?
static inline void clcdfb_sleep(unsigned int ms)
{
- if (in_atomic()) {
mdelay(ms);
- } else {
- msleep(ms);
- }
}
Regards
Santosh
On Sun, Mar 11, 2012 at 10:12 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sun, Mar 11, 2012 at 10:07:18PM +0530, santosh prasad nayak wrote:
>> On Sun, Mar 11, 2012 at 9:18 PM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Sun, Mar 11, 2012 at 08:49:27PM +0530, santosh prasad nayak wrote:
>> >> On Sun, Mar 11, 2012 at 8:33 PM, Russell King - ARM Linux
>> >> <linux@arm.linux.org.uk> wrote:
>> >> > in_interrupt() won't tell us if we're being called with spinlocks held,
>> >> > which _is_ a possibility because this can be called from printk(), for
>> >> > oops dumps and the like.
>> >> >
>> >> > in_interrupt() just means that we're inside a hard or soft interrupt,
>> >> > or nmi. It says nothing about whether msleep() is possible.
>> >>
>> >>
>> >> in_atomic() is also not error free. I found following comment in
>> >> include/linux/hardirq.h. How do you handle it in non-preemptible
>> >> kernel ?
>> >>
>> >> /*
>> >> * Are we running in atomic context? WARNING: this macro cannot
>> >> * always detect atomic context; in particular, it cannot know about
>> >> * held spinlocks in non-preemptible kernels. Thus it should not be
>> >> * used in the general case to determine whether sleeping is possible.
>> >> * Do not use in_atomic() in driver code.
>> >> */
>> >> #define in_atomic() ((preempt_count() & ~PREEMPT_ACTIVE) != 0)
>> >
>> > That may be, but the fact of the matter is that no one has *ever*
>> > reported an incident where this has failed at this point - and when
>> > it does people will end up with a might_sleep() warning from msleep().
>> >
>> > Maybe those who are saying people should not use this should instead
>> > be analysing why people use this, and suggest an alternative solution
>> > to the problem instead of a basic and uninformative "you shouldn't use
>> > this" statement.
>>
>> The reason is given in the article.
>
> At this point I'm just going to restate what I said above and below, so
> I'm not even going to bother doing that, and instead just say that. I'm
> not arguing whether it's right or wrong. I'm just stating that the only
> solution I see is to get rid of msleep() in there entirely.
>
>> http://lwn.net/Articles/274695/
>>
>> "The in_atomic() macro works by checking whether preemption is
>> disabled, which seems like the right thing to do. Handlers for events
>> like hardware interrupts will disable preemption, but so will the
>> acquisition of a spinlock. So this test appears to catch all of the
>> cases where sleeping would be a bad idea. Certainly a number of people
>> who have looked at this macro have come to that conclusion.
>>
>> But if preemption has not been configured into the kernel in the first
>> place, the kernel does not raise the "preemption count" when spinlocks
>> are acquired. So, in this situation (which is common - many
>> distributors still do not enable preemption in their kernels),
>> in_atomic() has no way to know if the calling code holds any spinlocks
>> or not. So it will return zero (indicating process context) even when
>> spinlocks are held. And that could lead to kernel code thinking that
>> it is running in process context (and acting accordingly) when, in
>> fact, it is not."
>>
>>
>>
>>
>> regards
>> Santosh
>> >
>> > As I've said, if we aren't going to use this, then the only solution is
>> > to completely omit the msleep() there and just say "sod you to running
>> > anything else for 20ms while this driver busy-spins." That's
>> > ultimately the safe thing to do, and at the moment I see no other
>> > alternative there.
^ permalink raw reply
* Re: [PATCH] Video : Amba: Use in_interrupt() in clcdfb_sleep().
From: Russell King - ARM Linux @ 2012-03-11 17:05 UTC (permalink / raw)
To: santosh prasad nayak
Cc: FlorianSchandinat, linux-fbdev, linux-kernel, kernel-janitors
In-Reply-To: <CAOD=uF6sckygJf+76jY65N1-0BSD4E6VYf0KcZAQpyNmtxi_+A@mail.gmail.com>
On Sun, Mar 11, 2012 at 10:27:12PM +0530, santosh prasad nayak wrote:
> Russel,
>
> Is this what you want ?
>
> static inline void clcdfb_sleep(unsigned int ms)
> {
> - if (in_atomic()) {
> mdelay(ms);
> - } else {
> - msleep(ms);
> - }
> }
"want" is a strong word - I would not say "want" because I don't want the
system busy-spinning for 20ms at a time during normal blank and unblank
events preventing any other thread in the system from running.
Looking at do_unblank_screen() in drivers/tty/vt/vt.c:
/*
* Called by timer as well as from vt_console_driver
*/
void do_unblank_screen(int leaving_gfx)
{
struct vc_data *vc;
/* This should now always be called from a "sane" (read: can schedule)
* context for the sake of the low level drivers, except in the special
* case of oops_in_progress
*/
if (!oops_in_progress)
might_sleep();
There may be another option of changing in_atomic() to be oops_in_progress()
if that comment is to be believed. However, that comment conflicts with
the comment immediately above the function. That said, the code is more
authorative than the comment above it because if that comment were true,
we'd see might_sleep() warnings.
So, I suspect:
static inline void clcdfb_sleep(unsigned int ms)
{
- if (in_atomic()) {
+ if (oops_in_progress) {
mdelay(ms);
} else {
msleep(ms);
is about the best that can be hoped for.
However, I think framebuffer people need to comment about what contexts
a framebuffer driver's fb_blank method would be called from (including
when oops-dumping.)
^ permalink raw reply
* Re: [PATCH] Video : Amba: Use in_interrupt() in clcdfb_sleep().
From: Alan Cox @ 2012-03-11 18:24 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: santosh prasad nayak, FlorianSchandinat, linux-fbdev,
linux-kernel, kernel-janitors
In-Reply-To: <20120311154846.GD13336@n2100.arm.linux.org.uk>
> As I've said, if we aren't going to use this, then the only solution is
> to completely omit the msleep() there and just say "sod you to running
> anything else for 20ms while this driver busy-spins." That's
> ultimately the safe thing to do, and at the moment I see no other
> alternative there.
Anyone having this argument is also right now peeing into the wind. Quite
a few console drivers do this including some the big name x86 ones. We
don't seem to be getting any resulting problem reports.
Architecturally we really need a way to help console drivers separate the
civilised acceleration friendly, lock friendly output paths from printk.
That's the real fix, but a whole different matter to solve cleanly.
Alan
^ permalink raw reply
* [PATCH 1/7] drivers/video/pvr2fb.c: ensure arguments to request_irq and free_irq are compatible
From: Julia Lawall @ 2012-03-11 19:36 UTC (permalink / raw)
To: Florian Tobias Schandinat; +Cc: kernel-janitors, linux-fbdev, linux-kernel
In-Reply-To: <1331494587-12196-1-git-send-email-Julia.Lawall@lip6.fr>
From: Julia Lawall <Julia.Lawall@lip6.fr>
Convert calls to free_irq so that the second argument is the same as the
last argument of the corresponding call to request_irq. Without this
property, free_irq does nothing.
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
Not tested.
drivers/video/pvr2fb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/pvr2fb.c b/drivers/video/pvr2fb.c
index 3a3fdc6..bcd44c3 100644
--- a/drivers/video/pvr2fb.c
+++ b/drivers/video/pvr2fb.c
@@ -895,7 +895,7 @@ static int __init pvr2fb_dc_init(void)
#ifdef CONFIG_PVR2_DMA
if (request_dma(pvr2dma, "pvr2") != 0) {
- free_irq(HW_EVENT_VSYNC, 0);
+ free_irq(HW_EVENT_VSYNC, fb_info);
return -EBUSY;
}
#endif
@@ -914,7 +914,7 @@ static void __exit pvr2fb_dc_exit(void)
currentpar->mmio_base = 0;
}
- free_irq(HW_EVENT_VSYNC, 0);
+ free_irq(HW_EVENT_VSYNC, fb_info);
#ifdef CONFIG_PVR2_DMA
free_dma(pvr2dma);
#endif
^ permalink raw reply related
* Re: [PATCH] Added backlight driver for Acer Aspire 4736
From: Florian Tobias Schandinat @ 2012-03-11 19:42 UTC (permalink / raw)
To: Pradeep Subrahmanion; +Cc: rpurdie, linux-kernel, linux-fbdev
In-Reply-To: <CABNxG=CU+bOWUauLYfcS2vtFqKvXA-9axgokNoYz+KuU1Mzztw@mail.gmail.com>
Hi,
On 03/11/2012 06:21 PM, Pradeep Subrahmanion wrote:
> Hi ,
>
> Brightness control was not working on Acer Aspire 4736 using
> default ACPI interface. acer-acpi also do not support 4730 series since
> it uses new WMI interface.
> This driver adds brightness control by accessing the LBB PCI
> configuration register. This approach may also work on other laptops in
> 4730 series .But currently , it is only tested for
> Aspire 4736.
>
> From 893031c1e9bdefd9642b98825062b5df98af0d77 Mon Sep 17 00:00:00 2001
> From: Pradeep Subrahmanion <subrahmanion.pradeep@gmail.com
> <mailto:subrahmanion.pradeep@gmail.com>>
> Date: Sun, 11 Mar 2012 23:11:23 -0400
> Subject: [PATCH] Added backlight driver for Acer Aspire 4736
>
the commit message should be here, I think.
>
> Signed-off-by: Pradeep Subrahmanion <subrahmanion.pradeep@gmail.com
> <mailto:subrahmanion.pradeep@gmail.com>>
Please resend this email in the correct format: As you can see in my
email the text version of your patch is severely screwed up and nobody
wants to even try converting a HTML format to a proper patch again,
probably most people won't even receive an email that has a HTML part as
their spam filter are going to handle it as spam. git send-email will
take care of it for you if you configure it for your account.
You should also cc Andrew Morton <akpm@linux-foundation.org> as he's the
one who handles backlight patches at the moment.
Best regards,
Florian Tobias Schandinat
> ---
> drivers/video/backlight/acer4736_bl.c | 110
> +++++++++++++++++++++++++++++++++
> 1 files changed, 110 insertions(+), 0 deletions(-)
> create mode 100644 drivers/video/backlight/acer4736_bl.c
>
> diff --git a/drivers/video/backlight/acer4736_bl.c
> b/drivers/video/backlight/acer4736_bl.c
> new file mode 100644
> index 0000000..6fe2937
> --- /dev/null
> +++ b/drivers/video/backlight/acer4736_bl.c
> @@ -0,0 +1,110 @@
> +/*
> + * Backlight driver for Acer Aspire 4736
> + *
> + * Copyright (C) Pradeep Subrahmanion <subrahmanion.pradeep@gmail.com
> <mailto:subrahmanion.pradeep@gmail.com>>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This driver uses LBB PCI configuration register to change the
> + * backlight brightness.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/backlight.h>
> +#include <linux/err.h>
> +#include <linux/dmi.h>
> +#include <linux/io.h>
> +#include <linux/pci.h>
> +
> +static u8 max_brightness = 0xFF;
> +static u8 lbb_offset = 0xF4;
> +static unsigned int device_id = 0x2a42;
> +static unsigned int vendor_id = 0x8086;
> +
> +struct backlight_device *acer_backlight_device;
> +struct pci_dev *pdev;
> +
> +static int acer_dmi_match(const struct dmi_system_id *id)
> +{
> + printk(KERN_INFO "acer4736_bl: %s detected\n", id->ident);
> + return 1;
> +}
> +
> +static const struct dmi_system_id __initdata acer_device_table[] = {
> +{
> + .callback = acer_dmi_match,
> + .ident = "Aspire 4736",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 4736"),
> + },
> + },
> + {}
> +};
> +static int read_brightness(struct backlight_device *bd)
> +{
> + u8 result;
> + pci_read_config_byte(pdev, lbb_offset, &result);
> + return result;
> +}
> +
> +static int update_brightness(struct backlight_device *bd)
> +{
> + u8 intensity = bd->props.brightness;
> + if (intensity > max_brightness) {
> + printk(KERN_INFO "Acer4736_bl: Invalid parameter. Maximum value is %d"
> + , max_brightness);
> + return -1;
> + }
> + pci_write_config_byte(pdev, lbb_offset, intensity);
> + return 0;
> +}
> +static const struct backlight_ops acer_backlight_ops = {
> + .get_brightness = read_brightness,
> + .update_status = update_brightness,
> +};
> +
> +static int __init acer4736_bl_init(void)
> +{
> + struct backlight_properties props;
> + if (!dmi_check_system(acer_device_table))
> + return -ENODEV;
> +
> + pdev = pci_get_device(vendor_id, device_id, NULL);
> +
> + if (!pdev)
> + return -ENODEV;
> +
> + printk(KERN_INFO "Loading Acer 4736 backlight driver\n");
> + memset(&props, 0, sizeof(struct backlight_properties));
> + props.type = BACKLIGHT_RAW;
> + props.max_brightness = max_brightness;
> +
> + acer_backlight_device = backlight_device_register("acer_backlight",
> + NULL, NULL, &acer_backlight_ops, &props);
> + acer_backlight_device->props.max_brightness = max_brightness;
> + acer_backlight_device->props.brightness > + read_brightness(acer_backlight_device);
> + backlight_update_status(acer_backlight_device);
> +
> + return 0;
> +}
> +
> +static void __exit acer4736_bl_exit(void)
> +{
> + pci_dev_put(pdev);
> + backlight_device_unregister(acer_backlight_device);
> +}
> +
> +module_init(acer4736_bl_init);
> +module_exit(acer4736_bl_exit);
> +
> +MODULE_AUTHOR("Pradeep Subrahmanion <subrahmanion.pradeep@gmail.com
> <mailto:subrahmanion.pradeep@gmail.com>>");
> +MODULE_DESCRIPTION("Acer Aspire 4736 Backlight Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_DEVICE_TABLE(dmi, acer_device_table);
> --
> 1.7.2.5
>
> -------------
>
> Pradeep Subrahmanion
^ permalink raw reply
* RE: [PATCH 1/2] fbdev: da8xx:: fix reporting of the display timing info
From: Manjunathappa, Prakash @ 2012-03-12 5:19 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1331305336-32644-2-git-send-email-agust@denx.de>
Hi,
On Fri, Mar 09, 2012 at 20:32:15, Anatolij Gustschin wrote:
> Timing info is not properly reported by the driver, e.g.:
>
> $ fbset -i
> mode "480x272-95"
> # D: 21.429 MHz, H: 33.018 kHz, V: 95.429 Hz
> geometry 480 272 480 544 16
> timings 46666 64 64 32 32 41 10
>
> According to the timing values defined for LK043T1DG01 display
> it should be reported as:
>
> mode "480x272-52"
> # D: 7.834 MHz, H: 14.921 kHz, V: 51.810 Hz
> geometry 480 272 480 544 16
> timings 127655 2 2 2 2 41 10
>
> Initialize additional fb_var_screeninfo fields so fix this problem.
>
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> Cc: Manjunathappa, Prakash <prakash.pm@ti.com>
> ---
> drivers/video/da8xx-fb.c | 13 +++++++++++++
> 1 files changed, 13 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
> index 29577bf..27c2794 100644
> --- a/drivers/video/da8xx-fb.c
> +++ b/drivers/video/da8xx-fb.c
> @@ -240,6 +240,14 @@ static struct da8xx_panel known_lcd_panels[] = {
> },
> };
>
> +static inline unsigned long hz_to_ps(unsigned long hz_val)
> +{
> + unsigned long long num = 1000000000000ULL;
> +
> + do_div(num, hz_val);
> + return (unsigned long)num;
> +}
> +
I have patch to take care of this:
http://davinci-linux-open-source.1494791.n2.nabble.com/PATCH-v2-video-da8xx-fb-calculate-pixel-clock-period-for-the-panel-tt7268377.html#none
> /* Enable the Raster Engine of the LCD Controller */
> static inline void lcd_enable_raster(void)
> {
> @@ -1209,6 +1217,11 @@ static int __devinit fb_probe(struct platform_device *device)
>
> da8xx_fb_var.hsync_len = lcdc_info->hsw;
> da8xx_fb_var.vsync_len = lcdc_info->vsw;
> + da8xx_fb_var.right_margin = lcdc_info->hfp;
> + da8xx_fb_var.left_margin = lcdc_info->hbp;
> + da8xx_fb_var.lower_margin = lcdc_info->vfp;
> + da8xx_fb_var.upper_margin = lcdc_info->vbp;
> + da8xx_fb_var.pixclock = hz_to_ps(lcdc_info->pxl_clk);
>
Above specified patch takes care of updating "pixclock". Remaining updates seems ok.
Thanks,
Prakash
> /* Initialize fbinfo */
> da8xx_fb_info->flags = FBINFO_FLAG_DEFAULT;
> --
> 1.7.7.6
>
>
^ permalink raw reply
* Re: [PATCH] OMAPDSS: provide default timings functions for panels
From: Archit Taneja @ 2012-03-12 5:57 UTC (permalink / raw)
To: Grazvydas Ignotas; +Cc: linux-fbdev, Tomi Valkeinen, linux-omap
In-Reply-To: <1331480061-5767-1-git-send-email-notasas@gmail.com>
Hi,
On Sunday 11 March 2012 09:04 PM, Grazvydas Ignotas wrote:
> With this we can eliminate some duplicate code in panel drivers.
> Also lgphilips-lb035q02, nec-nl8048hl11-01b, picodlp and
> tpo-td043mtea1 gain support of timings control over sysfs.
>
> Signed-off-by: Grazvydas Ignotas<notasas@gmail.com>
> ---
> drivers/video/omap2/displays/panel-acx565akm.c | 15 ---------------
> drivers/video/omap2/displays/panel-generic-dpi.c | 22 ----------------------
> drivers/video/omap2/displays/panel-n8x0.c | 8 --------
> drivers/video/omap2/displays/panel-taal.c | 8 --------
> drivers/video/omap2/dss/core.c | 6 ++++++
> drivers/video/omap2/dss/display.c | 21 +++++++++++++++++++++
> drivers/video/omap2/dss/venc.c | 7 -------
> include/video/omapdss.h | 6 ++++++
> 8 files changed, 33 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/video/omap2/displays/panel-acx565akm.c b/drivers/video/omap2/displays/panel-acx565akm.c
> index dbd59b8..be83eed 100644
> --- a/drivers/video/omap2/displays/panel-acx565akm.c
> +++ b/drivers/video/omap2/displays/panel-acx565akm.c
> @@ -738,19 +738,6 @@ static void acx_panel_set_timings(struct omap_dss_device *dssdev,
> }
> }
>
> -static void acx_panel_get_timings(struct omap_dss_device *dssdev,
> - struct omap_video_timings *timings)
> -{
> - *timings = dssdev->panel.timings;
> -}
> -
> -static int acx_panel_check_timings(struct omap_dss_device *dssdev,
> - struct omap_video_timings *timings)
> -{
> - return 0;
> -}
> -
> -
> static struct omap_dss_driver acx_panel_driver = {
> .probe = acx_panel_probe,
> .remove = acx_panel_remove,
> @@ -761,8 +748,6 @@ static struct omap_dss_driver acx_panel_driver = {
> .resume = acx_panel_resume,
>
> .set_timings = acx_panel_set_timings,
> - .get_timings = acx_panel_get_timings,
> - .check_timings = acx_panel_check_timings,
>
> .get_recommended_bpp = acx_get_recommended_bpp,
>
> diff --git a/drivers/video/omap2/displays/panel-generic-dpi.c b/drivers/video/omap2/displays/panel-generic-dpi.c
> index 519c47d..45a46af 100644
> --- a/drivers/video/omap2/displays/panel-generic-dpi.c
> +++ b/drivers/video/omap2/displays/panel-generic-dpi.c
> @@ -454,24 +454,6 @@ static int generic_dpi_panel_resume(struct omap_dss_device *dssdev)
> return 0;
> }
>
> -static void generic_dpi_panel_set_timings(struct omap_dss_device *dssdev,
> - struct omap_video_timings *timings)
> -{
> - dpi_set_timings(dssdev, timings);
> -}
> -
> -static void generic_dpi_panel_get_timings(struct omap_dss_device *dssdev,
> - struct omap_video_timings *timings)
> -{
> - *timings = dssdev->panel.timings;
> -}
> -
> -static int generic_dpi_panel_check_timings(struct omap_dss_device *dssdev,
> - struct omap_video_timings *timings)
> -{
> - return dpi_check_timings(dssdev, timings);
> -}
> -
> static struct omap_dss_driver dpi_driver = {
> .probe = generic_dpi_panel_probe,
> .remove = __exit_p(generic_dpi_panel_remove),
> @@ -481,10 +463,6 @@ static struct omap_dss_driver dpi_driver = {
> .suspend = generic_dpi_panel_suspend,
> .resume = generic_dpi_panel_resume,
>
> - .set_timings = generic_dpi_panel_set_timings,
> - .get_timings = generic_dpi_panel_get_timings,
> - .check_timings = generic_dpi_panel_check_timings,
> -
> .driver = {
> .name = "generic_dpi_panel",
> .owner = THIS_MODULE,
> diff --git a/drivers/video/omap2/displays/panel-n8x0.c b/drivers/video/omap2/displays/panel-n8x0.c
> index 150e8ba..eba98a0 100644
> --- a/drivers/video/omap2/displays/panel-n8x0.c
> +++ b/drivers/video/omap2/displays/panel-n8x0.c
> @@ -610,12 +610,6 @@ static int n8x0_panel_resume(struct omap_dss_device *dssdev)
> return 0;
> }
>
> -static void n8x0_panel_get_timings(struct omap_dss_device *dssdev,
> - struct omap_video_timings *timings)
> -{
> - *timings = dssdev->panel.timings;
> -}
> -
> static void n8x0_panel_get_resolution(struct omap_dss_device *dssdev,
> u16 *xres, u16 *yres)
> {
> @@ -678,8 +672,6 @@ static struct omap_dss_driver n8x0_panel_driver = {
> .get_resolution = n8x0_panel_get_resolution,
> .get_recommended_bpp = omapdss_default_get_recommended_bpp,
>
> - .get_timings = n8x0_panel_get_timings,
> -
> .driver = {
> .name = "n8x0_panel",
> .owner = THIS_MODULE,
> diff --git a/drivers/video/omap2/displays/panel-taal.c b/drivers/video/omap2/displays/panel-taal.c
> index 80c3f6a..174c004 100644
> --- a/drivers/video/omap2/displays/panel-taal.c
> +++ b/drivers/video/omap2/displays/panel-taal.c
> @@ -583,12 +583,6 @@ static const struct backlight_ops taal_bl_ops = {
> .update_status = taal_bl_update_status,
> };
>
> -static void taal_get_timings(struct omap_dss_device *dssdev,
> - struct omap_video_timings *timings)
> -{
> - *timings = dssdev->panel.timings;
> -}
> -
> static void taal_get_resolution(struct omap_dss_device *dssdev,
> u16 *xres, u16 *yres)
> {
> @@ -1899,8 +1893,6 @@ static struct omap_dss_driver taal_driver = {
> .run_test = taal_run_test,
> .memory_read = taal_memory_read,
>
> - .get_timings = taal_get_timings,
> -
> .driver = {
> .name = "taal",
> .owner = THIS_MODULE,
> diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
> index 86ec12e..9dc0c10 100644
> --- a/drivers/video/omap2/dss/core.c
> +++ b/drivers/video/omap2/dss/core.c
> @@ -434,6 +434,12 @@ int omap_dss_register_driver(struct omap_dss_driver *dssdriver)
> if (dssdriver->get_recommended_bpp = NULL)
> dssdriver->get_recommended_bpp > omapdss_default_get_recommended_bpp;
> + if (dssdriver->set_timings = NULL)
> + dssdriver->set_timings = omapdss_default_set_timings;
> + if (dssdriver->get_timings = NULL)
> + dssdriver->get_timings = omapdss_default_get_timings;
> + if (dssdriver->check_timings = NULL)
> + dssdriver->check_timings = omapdss_default_check_timings;
This elimination makes sense with get_timings(), but having the default
check_timings() and set_timings() linked with the DPI interface doesn't
seem right.
For example, if a panel driver using DSI interface doesn't have it's own
set_timings() op populated, and if some user of DSS call's
set_timings(), it would mess up the DSS2 driver.
Probably having a check like this in check_timings()/set_timings() might
make it safer:
if (dssdev->type = OMAP_DISPAY_TYPE_DPI)
dpi_set_timings(dssdev, timings);
Regards,
Archit
>
> return driver_register(&dssdriver->driver);
> }
> diff --git a/drivers/video/omap2/dss/display.c b/drivers/video/omap2/dss/display.c
> index be331dc..40746d9 100644
> --- a/drivers/video/omap2/dss/display.c
> +++ b/drivers/video/omap2/dss/display.c
> @@ -318,6 +318,27 @@ int omapdss_default_get_recommended_bpp(struct omap_dss_device *dssdev)
> }
> EXPORT_SYMBOL(omapdss_default_get_recommended_bpp);
>
> +void omapdss_default_set_timings(struct omap_dss_device *dssdev,
> + struct omap_video_timings *timings)
> +{
> + dpi_set_timings(dssdev, timings);
> +}
> +EXPORT_SYMBOL(omapdss_default_set_timings);
> +
> +void omapdss_default_get_timings(struct omap_dss_device *dssdev,
> + struct omap_video_timings *timings)
> +{
> + *timings = dssdev->panel.timings;
> +}
> +EXPORT_SYMBOL(omapdss_default_get_timings);
> +
> +int omapdss_default_check_timings(struct omap_dss_device *dssdev,
> + struct omap_video_timings *timings)
> +{
> + return dpi_check_timings(dssdev, timings);
> +}
> +EXPORT_SYMBOL(omapdss_default_check_timings);
> +
> /* Checks if replication logic should be used. Only use for active matrix,
> * when overlay is in RGB12U or RGB16 mode, and LCD interface is
> * 18bpp or 24bpp */
> diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
> index 7bb6219..dee2ec4 100644
> --- a/drivers/video/omap2/dss/venc.c
> +++ b/drivers/video/omap2/dss/venc.c
> @@ -557,12 +557,6 @@ static int venc_panel_resume(struct omap_dss_device *dssdev)
> return venc_panel_enable(dssdev);
> }
>
> -static void venc_get_timings(struct omap_dss_device *dssdev,
> - struct omap_video_timings *timings)
> -{
> - *timings = dssdev->panel.timings;
> -}
> -
> static void venc_set_timings(struct omap_dss_device *dssdev,
> struct omap_video_timings *timings)
> {
> @@ -641,7 +635,6 @@ static struct omap_dss_driver venc_driver = {
> .get_resolution = omapdss_default_get_resolution,
> .get_recommended_bpp = omapdss_default_get_recommended_bpp,
>
> - .get_timings = venc_get_timings,
> .set_timings = venc_set_timings,
> .check_timings = venc_check_timings,
>
> diff --git a/include/video/omapdss.h b/include/video/omapdss.h
> index 378c7ed..ff1dacd 100644
> --- a/include/video/omapdss.h
> +++ b/include/video/omapdss.h
> @@ -646,6 +646,12 @@ struct omap_overlay *omap_dss_get_overlay(int num);
> void omapdss_default_get_resolution(struct omap_dss_device *dssdev,
> u16 *xres, u16 *yres);
> int omapdss_default_get_recommended_bpp(struct omap_dss_device *dssdev);
> +void omapdss_default_set_timings(struct omap_dss_device *dssdev,
> + struct omap_video_timings *timings);
> +void omapdss_default_get_timings(struct omap_dss_device *dssdev,
> + struct omap_video_timings *timings);
> +int omapdss_default_check_timings(struct omap_dss_device *dssdev,
> + struct omap_video_timings *timings);
>
> typedef void (*omap_dispc_isr_t) (void *arg, u32 mask);
> int omap_dispc_register_isr(omap_dispc_isr_t isr, void *arg, u32 mask);
^ permalink raw reply
* Re: [PATCH] OMAPDSS: provide default timings functions for panels
From: Grazvydas Ignotas @ 2012-03-12 11:26 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-fbdev, Tomi Valkeinen, linux-omap
In-Reply-To: <4F5D9002.1030505@ti.com>
On Mon, Mar 12, 2012 at 7:56 AM, Archit Taneja <a0393947@ti.com> wrote:
>> diff --git a/drivers/video/omap2/dss/core.c
>> b/drivers/video/omap2/dss/core.c
>> index 86ec12e..9dc0c10 100644
>> --- a/drivers/video/omap2/dss/core.c
>> +++ b/drivers/video/omap2/dss/core.c
>> @@ -434,6 +434,12 @@ int omap_dss_register_driver(struct omap_dss_driver
>> *dssdriver)
>> if (dssdriver->get_recommended_bpp = NULL)
>> dssdriver->get_recommended_bpp >> omapdss_default_get_recommended_bpp;
>> + if (dssdriver->set_timings = NULL)
>> + dssdriver->set_timings = omapdss_default_set_timings;
>> + if (dssdriver->get_timings = NULL)
>> + dssdriver->get_timings = omapdss_default_get_timings;
>> + if (dssdriver->check_timings = NULL)
>> + dssdriver->check_timings = omapdss_default_check_timings;
>
>
> This elimination makes sense with get_timings(), but having the default
> check_timings() and set_timings() linked with the DPI interface doesn't seem
> right.
>
> For example, if a panel driver using DSI interface doesn't have it's own
> set_timings() op populated, and if some user of DSS call's set_timings(), it
> would mess up the DSS2 driver.
>
> Probably having a check like this in check_timings()/set_timings() might
> make it safer:
>
> if (dssdev->type = OMAP_DISPAY_TYPE_DPI)
> dpi_set_timings(dssdev, timings);
Makes sense, sending v2 shortly.
--
Gražvydas
^ permalink raw reply
* [PATCH v2] OMAPDSS: provide default timings functions for panels
From: Grazvydas Ignotas @ 2012-03-12 11:27 UTC (permalink / raw)
To: linux-fbdev; +Cc: Tomi Valkeinen, linux-omap, Grazvydas Ignotas
With this we can eliminate some duplicate code in panel drivers.
Also lgphilips-lb035q02, nec-nl8048hl11-01b, picodlp and
tpo-td043mtea1 gain support of timings control over sysfs.
Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
---
drivers/video/omap2/displays/panel-acx565akm.c | 15 -------------
drivers/video/omap2/displays/panel-generic-dpi.c | 22 -------------------
drivers/video/omap2/displays/panel-n8x0.c | 8 -------
drivers/video/omap2/displays/panel-taal.c | 8 -------
drivers/video/omap2/dss/core.c | 6 +++++
drivers/video/omap2/dss/display.c | 25 ++++++++++++++++++++++
drivers/video/omap2/dss/venc.c | 7 ------
include/video/omapdss.h | 6 +++++
8 files changed, 37 insertions(+), 60 deletions(-)
diff --git a/drivers/video/omap2/displays/panel-acx565akm.c b/drivers/video/omap2/displays/panel-acx565akm.c
index dbd59b8..be83eed 100644
--- a/drivers/video/omap2/displays/panel-acx565akm.c
+++ b/drivers/video/omap2/displays/panel-acx565akm.c
@@ -738,19 +738,6 @@ static void acx_panel_set_timings(struct omap_dss_device *dssdev,
}
}
-static void acx_panel_get_timings(struct omap_dss_device *dssdev,
- struct omap_video_timings *timings)
-{
- *timings = dssdev->panel.timings;
-}
-
-static int acx_panel_check_timings(struct omap_dss_device *dssdev,
- struct omap_video_timings *timings)
-{
- return 0;
-}
-
-
static struct omap_dss_driver acx_panel_driver = {
.probe = acx_panel_probe,
.remove = acx_panel_remove,
@@ -761,8 +748,6 @@ static struct omap_dss_driver acx_panel_driver = {
.resume = acx_panel_resume,
.set_timings = acx_panel_set_timings,
- .get_timings = acx_panel_get_timings,
- .check_timings = acx_panel_check_timings,
.get_recommended_bpp = acx_get_recommended_bpp,
diff --git a/drivers/video/omap2/displays/panel-generic-dpi.c b/drivers/video/omap2/displays/panel-generic-dpi.c
index 519c47d..45a46af 100644
--- a/drivers/video/omap2/displays/panel-generic-dpi.c
+++ b/drivers/video/omap2/displays/panel-generic-dpi.c
@@ -454,24 +454,6 @@ static int generic_dpi_panel_resume(struct omap_dss_device *dssdev)
return 0;
}
-static void generic_dpi_panel_set_timings(struct omap_dss_device *dssdev,
- struct omap_video_timings *timings)
-{
- dpi_set_timings(dssdev, timings);
-}
-
-static void generic_dpi_panel_get_timings(struct omap_dss_device *dssdev,
- struct omap_video_timings *timings)
-{
- *timings = dssdev->panel.timings;
-}
-
-static int generic_dpi_panel_check_timings(struct omap_dss_device *dssdev,
- struct omap_video_timings *timings)
-{
- return dpi_check_timings(dssdev, timings);
-}
-
static struct omap_dss_driver dpi_driver = {
.probe = generic_dpi_panel_probe,
.remove = __exit_p(generic_dpi_panel_remove),
@@ -481,10 +463,6 @@ static struct omap_dss_driver dpi_driver = {
.suspend = generic_dpi_panel_suspend,
.resume = generic_dpi_panel_resume,
- .set_timings = generic_dpi_panel_set_timings,
- .get_timings = generic_dpi_panel_get_timings,
- .check_timings = generic_dpi_panel_check_timings,
-
.driver = {
.name = "generic_dpi_panel",
.owner = THIS_MODULE,
diff --git a/drivers/video/omap2/displays/panel-n8x0.c b/drivers/video/omap2/displays/panel-n8x0.c
index 150e8ba..eba98a0 100644
--- a/drivers/video/omap2/displays/panel-n8x0.c
+++ b/drivers/video/omap2/displays/panel-n8x0.c
@@ -610,12 +610,6 @@ static int n8x0_panel_resume(struct omap_dss_device *dssdev)
return 0;
}
-static void n8x0_panel_get_timings(struct omap_dss_device *dssdev,
- struct omap_video_timings *timings)
-{
- *timings = dssdev->panel.timings;
-}
-
static void n8x0_panel_get_resolution(struct omap_dss_device *dssdev,
u16 *xres, u16 *yres)
{
@@ -678,8 +672,6 @@ static struct omap_dss_driver n8x0_panel_driver = {
.get_resolution = n8x0_panel_get_resolution,
.get_recommended_bpp = omapdss_default_get_recommended_bpp,
- .get_timings = n8x0_panel_get_timings,
-
.driver = {
.name = "n8x0_panel",
.owner = THIS_MODULE,
diff --git a/drivers/video/omap2/displays/panel-taal.c b/drivers/video/omap2/displays/panel-taal.c
index 80c3f6a..174c004 100644
--- a/drivers/video/omap2/displays/panel-taal.c
+++ b/drivers/video/omap2/displays/panel-taal.c
@@ -583,12 +583,6 @@ static const struct backlight_ops taal_bl_ops = {
.update_status = taal_bl_update_status,
};
-static void taal_get_timings(struct omap_dss_device *dssdev,
- struct omap_video_timings *timings)
-{
- *timings = dssdev->panel.timings;
-}
-
static void taal_get_resolution(struct omap_dss_device *dssdev,
u16 *xres, u16 *yres)
{
@@ -1899,8 +1893,6 @@ static struct omap_dss_driver taal_driver = {
.run_test = taal_run_test,
.memory_read = taal_memory_read,
- .get_timings = taal_get_timings,
-
.driver = {
.name = "taal",
.owner = THIS_MODULE,
diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
index 86ec12e..9dc0c10 100644
--- a/drivers/video/omap2/dss/core.c
+++ b/drivers/video/omap2/dss/core.c
@@ -434,6 +434,12 @@ int omap_dss_register_driver(struct omap_dss_driver *dssdriver)
if (dssdriver->get_recommended_bpp = NULL)
dssdriver->get_recommended_bpp omapdss_default_get_recommended_bpp;
+ if (dssdriver->set_timings = NULL)
+ dssdriver->set_timings = omapdss_default_set_timings;
+ if (dssdriver->get_timings = NULL)
+ dssdriver->get_timings = omapdss_default_get_timings;
+ if (dssdriver->check_timings = NULL)
+ dssdriver->check_timings = omapdss_default_check_timings;
return driver_register(&dssdriver->driver);
}
diff --git a/drivers/video/omap2/dss/display.c b/drivers/video/omap2/dss/display.c
index be331dc..0d724a4 100644
--- a/drivers/video/omap2/dss/display.c
+++ b/drivers/video/omap2/dss/display.c
@@ -318,6 +318,31 @@ int omapdss_default_get_recommended_bpp(struct omap_dss_device *dssdev)
}
EXPORT_SYMBOL(omapdss_default_get_recommended_bpp);
+void omapdss_default_set_timings(struct omap_dss_device *dssdev,
+ struct omap_video_timings *timings)
+{
+ if (dssdev->type = OMAP_DISPLAY_TYPE_DPI)
+ dpi_set_timings(dssdev, timings);
+}
+EXPORT_SYMBOL(omapdss_default_set_timings);
+
+void omapdss_default_get_timings(struct omap_dss_device *dssdev,
+ struct omap_video_timings *timings)
+{
+ *timings = dssdev->panel.timings;
+}
+EXPORT_SYMBOL(omapdss_default_get_timings);
+
+int omapdss_default_check_timings(struct omap_dss_device *dssdev,
+ struct omap_video_timings *timings)
+{
+ if (dssdev->type = OMAP_DISPLAY_TYPE_DPI)
+ return dpi_check_timings(dssdev, timings);
+
+ return 0;
+}
+EXPORT_SYMBOL(omapdss_default_check_timings);
+
/* Checks if replication logic should be used. Only use for active matrix,
* when overlay is in RGB12U or RGB16 mode, and LCD interface is
* 18bpp or 24bpp */
diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
index 7bb6219..dee2ec4 100644
--- a/drivers/video/omap2/dss/venc.c
+++ b/drivers/video/omap2/dss/venc.c
@@ -557,12 +557,6 @@ static int venc_panel_resume(struct omap_dss_device *dssdev)
return venc_panel_enable(dssdev);
}
-static void venc_get_timings(struct omap_dss_device *dssdev,
- struct omap_video_timings *timings)
-{
- *timings = dssdev->panel.timings;
-}
-
static void venc_set_timings(struct omap_dss_device *dssdev,
struct omap_video_timings *timings)
{
@@ -641,7 +635,6 @@ static struct omap_dss_driver venc_driver = {
.get_resolution = omapdss_default_get_resolution,
.get_recommended_bpp = omapdss_default_get_recommended_bpp,
- .get_timings = venc_get_timings,
.set_timings = venc_set_timings,
.check_timings = venc_check_timings,
diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index 378c7ed..ff1dacd 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -646,6 +646,12 @@ struct omap_overlay *omap_dss_get_overlay(int num);
void omapdss_default_get_resolution(struct omap_dss_device *dssdev,
u16 *xres, u16 *yres);
int omapdss_default_get_recommended_bpp(struct omap_dss_device *dssdev);
+void omapdss_default_set_timings(struct omap_dss_device *dssdev,
+ struct omap_video_timings *timings);
+void omapdss_default_get_timings(struct omap_dss_device *dssdev,
+ struct omap_video_timings *timings);
+int omapdss_default_check_timings(struct omap_dss_device *dssdev,
+ struct omap_video_timings *timings);
typedef void (*omap_dispc_isr_t) (void *arg, u32 mask);
int omap_dispc_register_isr(omap_dispc_isr_t isr, void *arg, u32 mask);
--
1.7.0.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