Re: [PATCH] pxafb: Add missing code to support palette formats
From: eric miao <hidden>
Date: 2007-11-27 02:48:54
On Nov 26, 2007 9:58 PM, Hans-Jürgen Koch [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Am Sat, 24 Nov 2007 16:28:55 +0800 schrieb "eric miao" [off-list ref]:quoted
On Nov 23, 2007 7:26 PM, Hans J. Koch [off-list ref] wrote:quoted
Subject: pxafb: Add missing code to support palette formats Date: Fri, 23 Nov 2007 10:33:24 +0100 From: Hans J Koch <redacted> 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 <redacted> --- 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-2223: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.quoted
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 <redacted> 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 <redacted> --- 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
quoted hunk ↗ jump to hunk
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