linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] pxafb: Add missing code to support palette formats
@ 2007-11-23 11:26 Hans J. Koch
  2007-11-24  8:28 ` eric miao
  0 siblings, 1 reply; 4+ messages in thread
From: Hans J. Koch @ 2007-11-23 11:26 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: linux-fbdev-devel

Subject: pxafb: Add missing code to support palette formats
Date: Fri, 23 Nov 2007 10:33:24 +0100
From: Hans J Koch <hjk@linutronix.de>

A few lines of code are missing to support different palette formats in
pxafb. This patch adds them, code was tested on PXA270.

Signed-off-by: Hans J Koch <hjk@linutronix.de>

---
 drivers/video/pxafb.c |   12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Index: linux-2.6.24-rc/drivers/video/pxafb.c
===================================================================
--- linux-2.6.24-rc.orig/drivers/video/pxafb.c	2007-11-22
23:03:28.000000000 +0100 +++
linux-2.6.24-rc/drivers/video/pxafb.c	2007-11-22
23:43:07.000000000 +0100 @@ -745,8 +745,14 @@ fbi->reg_lccr1 =
new_regs.lccr1; fbi->reg_lccr2 = new_regs.lccr2;
 	fbi->reg_lccr3 = new_regs.lccr3;
-	fbi->reg_lccr4 = LCCR4 & (~LCCR4_PAL_FOR_MASK);
-	fbi->reg_lccr4 |= (fbi->lccr4 & LCCR4_PAL_FOR_MASK);
+
+	if (((read_cpuid(CPUID_ID) >> 4) & 0x1f) == 0x11) {
+		/* PXA 27x only */
+		fbi->reg_lccr4 = LCCR4 & (~LCCR4_PAL_FOR_MASK);
+		fbi->reg_lccr4 |= (fbi->lccr4 & LCCR4_PAL_FOR_MASK);
+	} else
+		fbi->reg_lccr4 = 0;
+
 	set_hsync_time(fbi, pcd);
 	local_irq_restore(flags);
 
@@ -838,6 +844,8 @@
 	clk_enable(fbi->clk);
 
 	/* Sequence from 11.7.10 */
+	if (((read_cpuid(CPUID_ID) >> 4) & 0x1f) == 0x11)
+		LCCR4 = fbi->reg_lccr4; /* PXA 27x only */
 	LCCR3 = fbi->reg_lccr3;
 	LCCR2 = fbi->reg_lccr2;
 	LCCR1 = fbi->reg_lccr1;

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] pxafb: Add missing code to support palette formats
  2007-11-23 11:26 [PATCH] pxafb: Add missing code to support palette formats Hans J. Koch
@ 2007-11-24  8:28 ` eric miao
  2007-11-26 13:58   ` Hans-Jürgen Koch
  0 siblings, 1 reply; 4+ messages in thread
From: eric miao @ 2007-11-24  8:28 UTC (permalink / raw)
  To: Hans J. Koch; +Cc: linux-fbdev-devel, linux-arm-kernel

On Nov 23, 2007 7:26 PM, Hans J. Koch <hjk@linutronix.de> wrote:
> Subject: pxafb: Add missing code to support palette formats
> Date: Fri, 23 Nov 2007 10:33:24 +0100
> From: Hans J Koch <hjk@linutronix.de>
>
> A few lines of code are missing to support different palette formats in
> pxafb. This patch adds them, code was tested on PXA270.
>
> Signed-off-by: Hans J Koch <hjk@linutronix.de>
>
> ---
>  drivers/video/pxafb.c |   12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> Index: linux-2.6.24-rc/drivers/video/pxafb.c
> ===================================================================
> --- linux-2.6.24-rc.orig/drivers/video/pxafb.c  2007-11-22
> 23:03:28.000000000 +0100 +++
> linux-2.6.24-rc/drivers/video/pxafb.c   2007-11-22
> 23:43:07.000000000 +0100 @@ -745,8 +745,14 @@ fbi->reg_lccr1 =
> new_regs.lccr1; fbi->reg_lccr2 = new_regs.lccr2;
>         fbi->reg_lccr3 = new_regs.lccr3;
> -       fbi->reg_lccr4 = LCCR4 & (~LCCR4_PAL_FOR_MASK);
> -       fbi->reg_lccr4 |= (fbi->lccr4 & LCCR4_PAL_FOR_MASK);
> +
> +       if (((read_cpuid(CPUID_ID) >> 4) & 0x1f) == 0x11) {
> +               /* PXA 27x only */
> +               fbi->reg_lccr4 = LCCR4 & (~LCCR4_PAL_FOR_MASK);
> +               fbi->reg_lccr4 |= (fbi->lccr4 & LCCR4_PAL_FOR_MASK);
> +       } else
> +               fbi->reg_lccr4 = 0;
> +

I don't see what's wrong with the original code. Can you explain this
a bit more?

And there are already cpu_is_pxa27x() and friends in the kernel, so
use those instead of your own please.

>         set_hsync_time(fbi, pcd);
>         local_irq_restore(flags);
>
> @@ -838,6 +844,8 @@
>         clk_enable(fbi->clk);
>
>         /* Sequence from 11.7.10 */
> +       if (((read_cpuid(CPUID_ID) >> 4) & 0x1f) == 0x11)
> +               LCCR4 = fbi->reg_lccr4; /* PXA 27x only */
>         LCCR3 = fbi->reg_lccr3;
>         LCCR2 = fbi->reg_lccr2;
>         LCCR1 = fbi->reg_lccr1;
>

Ditto.

> -------------------------------------------------------------------
> List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
> FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
> Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php
>

-- 
Cheers
- eric

-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] pxafb: Add missing code to support palette formats
  2007-11-24  8:28 ` eric miao
@ 2007-11-26 13:58   ` Hans-Jürgen Koch
  2007-11-27  2:48     ` eric miao
  0 siblings, 1 reply; 4+ messages in thread
From: Hans-Jürgen Koch @ 2007-11-26 13:58 UTC (permalink / raw)
  To: eric miao; +Cc: linux-fbdev-devel, linux-arm-kernel

Am Sat, 24 Nov 2007 16:28:55 +0800
schrieb "eric miao" <eric.y.miao@gmail.com>:

> On Nov 23, 2007 7:26 PM, Hans J. Koch <hjk@linutronix.de> wrote:
> > Subject: pxafb: Add missing code to support palette formats
> > Date: Fri, 23 Nov 2007 10:33:24 +0100
> > From: Hans J Koch <hjk@linutronix.de>
> >
> > A few lines of code are missing to support different palette
> > formats in pxafb. This patch adds them, code was tested on PXA270.
> >
> > Signed-off-by: Hans J Koch <hjk@linutronix.de>
> >
> > ---
> >  drivers/video/pxafb.c |   12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > Index: linux-2.6.24-rc/drivers/video/pxafb.c
> > ===================================================================
> > --- linux-2.6.24-rc.orig/drivers/video/pxafb.c  2007-11-22
> > 23:03:28.000000000 +0100 +++
> > linux-2.6.24-rc/drivers/video/pxafb.c   2007-11-22
> > 23:43:07.000000000 +0100 @@ -745,8 +745,14 @@ fbi->reg_lccr1 =
> > new_regs.lccr1; fbi->reg_lccr2 = new_regs.lccr2;
> >         fbi->reg_lccr3 = new_regs.lccr3;
> > -       fbi->reg_lccr4 = LCCR4 & (~LCCR4_PAL_FOR_MASK);
> > -       fbi->reg_lccr4 |= (fbi->lccr4 & LCCR4_PAL_FOR_MASK);
> > +
> > +       if (((read_cpuid(CPUID_ID) >> 4) & 0x1f) == 0x11) {
> > +               /* PXA 27x only */
> > +               fbi->reg_lccr4 = LCCR4 & (~LCCR4_PAL_FOR_MASK);
> > +               fbi->reg_lccr4 |= (fbi->lccr4 & LCCR4_PAL_FOR_MASK);
> > +       } else
> > +               fbi->reg_lccr4 = 0;
> > +
> 
> I don't see what's wrong with the original code. Can you explain this
> a bit more?

The patch fixes two problems:

1. Register LCCR4 exists on PXA27x only.
2. The original code never writes to LCCR4. It sets fbi->reg_lccr4 only.


> 
> And there are already cpu_is_pxa27x() and friends in the kernel, so
> use those instead of your own please.

Right.

Thanks for your hints, Eric! Here's an updated version:

Subject: pxafb: Add missing code to support palette formats
Date: Fri, 23 Nov 2007 10:33:24 +0100
From: Hans J Koch <hjk@linutronix.de>

A few lines of code are missing to support different palette formats in pxafb.
This patch adds them, code was tested on PXA270.

Signed-off-by: Hans J Koch <hjk@linutronix.de>

---
 drivers/video/pxafb.c |   11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Index: linux-2.6.24-rc/drivers/video/pxafb.c
===================================================================
--- linux-2.6.24-rc.orig/drivers/video/pxafb.c	2007-11-22 23:03:28.000000000 +0100
+++ linux-2.6.24-rc/drivers/video/pxafb.c	2007-11-26 14:46:34.000000000 +0100
@@ -745,8 +745,13 @@
 	fbi->reg_lccr1 = new_regs.lccr1;
 	fbi->reg_lccr2 = new_regs.lccr2;
 	fbi->reg_lccr3 = new_regs.lccr3;
-	fbi->reg_lccr4 = LCCR4 & (~LCCR4_PAL_FOR_MASK);
-	fbi->reg_lccr4 |= (fbi->lccr4 & LCCR4_PAL_FOR_MASK);
+
+	if (cpu_is_pxa27x()) {
+		fbi->reg_lccr4 = LCCR4 & (~LCCR4_PAL_FOR_MASK);
+		fbi->reg_lccr4 |= (fbi->lccr4 & LCCR4_PAL_FOR_MASK);
+	} else
+		fbi->reg_lccr4 = 0;
+
 	set_hsync_time(fbi, pcd);
 	local_irq_restore(flags);
 
@@ -838,6 +843,8 @@
 	clk_enable(fbi->clk);
 
 	/* Sequence from 11.7.10 */
+	if (cpu_is_pxa27x())
+		LCCR4 = fbi->reg_lccr4;
 	LCCR3 = fbi->reg_lccr3;
 	LCCR2 = fbi->reg_lccr2;
 	LCCR1 = fbi->reg_lccr1;

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] pxafb: Add missing code to support palette formats
  2007-11-26 13:58   ` Hans-Jürgen Koch
@ 2007-11-27  2:48     ` eric miao
  0 siblings, 0 replies; 4+ messages in thread
From: eric miao @ 2007-11-27  2:48 UTC (permalink / raw)
  To: Hans-Jürgen Koch; +Cc: linux-fbdev-devel, linux-arm-kernel

On Nov 26, 2007 9:58 PM, Hans-Jürgen Koch <hjk@linutronix.de> wrote:
> Am Sat, 24 Nov 2007 16:28:55 +0800
> schrieb "eric miao" <eric.y.miao@gmail.com>:
>
>
> > On Nov 23, 2007 7:26 PM, Hans J. Koch <hjk@linutronix.de> wrote:
> > > Subject: pxafb: Add missing code to support palette formats
> > > Date: Fri, 23 Nov 2007 10:33:24 +0100
> > > From: Hans J Koch <hjk@linutronix.de>
> > >
> > > A few lines of code are missing to support different palette
> > > formats in pxafb. This patch adds them, code was tested on PXA270.
> > >
> > > Signed-off-by: Hans J Koch <hjk@linutronix.de>
> > >
> > > ---
> > >  drivers/video/pxafb.c |   12 ++++++++++--
> > >  1 file changed, 10 insertions(+), 2 deletions(-)
> > >
> > > Index: linux-2.6.24-rc/drivers/video/pxafb.c
> > > ===================================================================
> > > --- linux-2.6.24-rc.orig/drivers/video/pxafb.c  2007-11-22
> > > 23:03:28.000000000 +0100 +++
> > > linux-2.6.24-rc/drivers/video/pxafb.c   2007-11-22
> > > 23:43:07.000000000 +0100 @@ -745,8 +745,14 @@ fbi->reg_lccr1 =
> > > new_regs.lccr1; fbi->reg_lccr2 = new_regs.lccr2;
> > >         fbi->reg_lccr3 = new_regs.lccr3;
> > > -       fbi->reg_lccr4 = LCCR4 & (~LCCR4_PAL_FOR_MASK);
> > > -       fbi->reg_lccr4 |= (fbi->lccr4 & LCCR4_PAL_FOR_MASK);
> > > +
> > > +       if (((read_cpuid(CPUID_ID) >> 4) & 0x1f) == 0x11) {
> > > +               /* PXA 27x only */
> > > +               fbi->reg_lccr4 = LCCR4 & (~LCCR4_PAL_FOR_MASK);
> > > +               fbi->reg_lccr4 |= (fbi->lccr4 & LCCR4_PAL_FOR_MASK);
> > > +       } else
> > > +               fbi->reg_lccr4 = 0;
> > > +
> >
> > I don't see what's wrong with the original code. Can you explain this
> > a bit more?
>
> The patch fixes two problems:
>
> 1. Register LCCR4 exists on PXA27x only.
> 2. The original code never writes to LCCR4. It sets fbi->reg_lccr4 only.
>
>
> >
> > And there are already cpu_is_pxa27x() and friends in the kernel, so
> > use those instead of your own please.
>
> Right.
>
> Thanks for your hints, Eric! Here's an updated version:
>
> Subject: pxafb: Add missing code to support palette formats
> Date: Fri, 23 Nov 2007 10:33:24 +0100
> From: Hans J Koch <hjk@linutronix.de>
>
> A few lines of code are missing to support different palette formats in pxafb.
> This patch adds them, code was tested on PXA270.
>
> Signed-off-by: Hans J Koch <hjk@linutronix.de>
>
> ---
>  drivers/video/pxafb.c |   11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> Index: linux-2.6.24-rc/drivers/video/pxafb.c
> ===================================================================
> --- linux-2.6.24-rc.orig/drivers/video/pxafb.c  2007-11-22 23:03:28.000000000 +0100
> +++ linux-2.6.24-rc/drivers/video/pxafb.c       2007-11-26 14:46:34.000000000 +0100
> @@ -745,8 +745,13 @@
>         fbi->reg_lccr1 = new_regs.lccr1;
>         fbi->reg_lccr2 = new_regs.lccr2;
>         fbi->reg_lccr3 = new_regs.lccr3;
> -       fbi->reg_lccr4 = LCCR4 & (~LCCR4_PAL_FOR_MASK);
> -       fbi->reg_lccr4 |= (fbi->lccr4 & LCCR4_PAL_FOR_MASK);
> +
> +       if (cpu_is_pxa27x()) {
> +               fbi->reg_lccr4 = LCCR4 & (~LCCR4_PAL_FOR_MASK);
> +               fbi->reg_lccr4 |= (fbi->lccr4 & LCCR4_PAL_FOR_MASK);
> +       } else
> +               fbi->reg_lccr4 = 0;
> +

it would be more appropriate to initialize fbi->lccr4 to '0' in
pxafb_init_fbinfo()
if it is pxa25x/pxa21x since fbi->lccr4 will be used to calculate the size and
format of the palette buffer while pxa25x/pxa21x support only 16-bit
palette _only_.

it is also possible no change made here to fbi->reg_lccr4 since the value
will not be guarded to be actually written if pxa25x/pxa21x

>         set_hsync_time(fbi, pcd);
>         local_irq_restore(flags);
>
> @@ -838,6 +843,8 @@
>         clk_enable(fbi->clk);
>
>         /* Sequence from 11.7.10 */
> +       if (cpu_is_pxa27x())
> +               LCCR4 = fbi->reg_lccr4;
>

this breaks the pxa3xx, if (cpu_is_pxa27x() || cpu_is_pxa3xx()) would be better

>         LCCR3 = fbi->reg_lccr3;
>         LCCR2 = fbi->reg_lccr2;
>         LCCR1 = fbi->reg_lccr1;
>

Anyway, I do think the issue is within the introduce of lccr4 in
previous patches.
There should be much clean patch to this issue, though.

-- 
Cheers
- eric
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
Linux-fbdev-devel mailing list
Linux-fbdev-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2007-11-27  2:48 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-11-23 11:26 [PATCH] pxafb: Add missing code to support palette formats Hans J. Koch
2007-11-24  8:28 ` eric miao
2007-11-26 13:58   ` Hans-Jürgen Koch
2007-11-27  2:48     ` eric miao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).