pxafb: Add missing code to support palette formats
PATCH 1/1 pxafb: Add missing code to support palette formats
Subject: pxafb: Add missing code to support palette formats
Date: Fri, 23 Nov 2007 10:33:24 +0100
From: Hans J Koch <[email protected]>
A few lines of code are missing to support different palette formats in pxafb. This patch adds them, code was tested on PXA270.
--- 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/
Quoted 21 lines · Hans J. Koch
Subject: pxafb: Add missing code to support palette formats
Date: Fri, 23 Nov 2007 10:33:24 +0100
From: Hans J Koch <[email protected]>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 <[email protected]>--- 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.c2007-11-22 23:03:28.000000000 +0100 +++linux-2.6.24-rc/drivers/video/pxafb.c2007-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;
Review
Quoted 10 lines
(quoted diff) +8
-2- 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.
Review line 851
Quoted 13 lines
set_hsync_time(fbi, pcd); local_irq_restore(flags);(quoted diff) +2
-0@@ -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.
Quoted 5 lines
-------------------------------------------------------------------
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
Signature · 7 lines
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
Am Sat, 24 Nov 2007 16:28:55 +0800
schrieb "eric miao" <[email protected]>:
Quoted 27 lines · Hans J. Koch
Quoted 26 lines
Subject: pxafb: Add missing code to support palette formats
Date: Fri, 23 Nov 2007 10:33:24 +0100
From: Hans J Koch <[email protected]>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 <[email protected]>--- 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.c2007-11-22 23:03:28.000000000 +0100 +++linux-2.6.24-rc/drivers/video/pxafb.c2007-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;(quoted diff) +3
-2- 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 */
Quoted 8 lines
Review
Quoted 5 lines
(quoted diff) +5
-0+ 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.
Quoted 3 lines
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 <[email protected]>
A few lines of code are missing to support different palette formats in pxafb. This patch adds them, code was tested on PXA270.
--- drivers/video/pxafb.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
drivers/video/pxafb.c +9 -2
@@ -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/
Quoted 74 lines · Hans-Jürgen Koch
Am Sat, 24 Nov 2007 16:28:55 +0800
schrieb "eric miao" <[email protected]>:Quoted 27 lines · Hans J. Koch
Quoted 26 lines
Subject: pxafb: Add missing code to support palette formats
Date: Fri, 23 Nov 2007 10:33:24 +0100
From: Hans J Koch <[email protected]>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 <[email protected]>--- 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.c2007-11-22 23:03:28.000000000 +0100 +++linux-2.6.24-rc/drivers/video/pxafb.c2007-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;drivers/video/pxafb.c +3
-2- 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 */ Quoted 8 lines
drivers/video/pxafb.c line 754
Quoted 5 lines
drivers/video/pxafb.c +5
-0+ 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 setsfbi->reg_lccr4only.Quoted 3 lines
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 <[email protected]>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 <[email protected]>--- drivers/video/pxafb.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)Index:
linux-2.6.24-rc/drivers/video/pxafb.c
===================================================================drivers/video/pxafb.c 2007-11-26 14:46:34.000000000 +0100 renamed from drivers/video/pxafb.c 2007-11-22 23:03:28.000000000 +0100 +0
-0@@ -745,8 +745,13 @@ fbi->reg_lccr1 = new_regs.lccr1; fbi->reg_lccr2 = new_regs.lccr2;
drivers/video/pxafb.c 2007-11-26 14:46:34.000000000 +0100 line 754
Quoted 10 lines
drivers/video/pxafb.c 2007-11-26 14:46:34.000000000 +0100 renamed from drivers/video/pxafb.c 2007-11-22 23:03:28.000000000 +0100 +7
-2fbi->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
drivers/video/pxafb.c 2007-11-26 14:46:34.000000000 +0100 line 848
Quoted 10 lines
drivers/video/pxafb.c 2007-11-26 14:46:34.000000000 +0100 renamed from drivers/video/pxafb.c 2007-11-22 23:03:28.000000000 +0100 +2
-0set_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
Quoted 4 lines
drivers/video/pxafb.c 2007-11-26 14:46:34.000000000 +0100 renamed from drivers/video/pxafb.c 2007-11-22 23:03:28.000000000 +0100 +0
-0LCCR3 = 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.
Signature · 10 lines
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 [email protected] https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel