[PATCH] pxafb: correct the vertical back porch macro

[PATCH] pxafb: correct the vertical back porch macro

[Matt Reimer]

Fix an off-by-one bug in the LCCR2_BegFrmDel() macro. The vertical back porch
knob LCCR2[BFW] specifies the number of clock periods minus one, but the
macro wasn't doing the subtraction.
---
 arch/arm/mach-pxa/include/mach/regs-lcd.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-pxa/include/mach/regs-lcd.h b/arch/arm/mach-pxa/include/mach/regs-lcd.h
index f82dcea..5074d18 100644
--- a/arch/arm/mach-pxa/include/mach/regs-lcd.h
+++ b/arch/arm/mach-pxa/include/mach/regs-lcd.h
@@ -107,7 +107,7 @@
 #define LCCR2_EndFrmDel(Tln)   ((Tln) << FShft (LCCR2_EFW))
 
 #define LCCR2_BFW      Fld (8, 24)     /* Beginning-of-Frame line clock */
-#define LCCR2_BegFrmDel(Tln)   ((Tln) << FShft (LCCR2_BFW))
+#define LCCR2_BegFrmDel(Tln)   (((Tln) - 1) << FShft (LCCR2_BFW))
 
 #define LCCR3_API      (0xf << 16)     /* AC Bias pin trasitions per interrupt */
 #define LCCR3_API_S    16
-- 
1.7.0.4

[PATCH] pxafb: correct the vertical back porch macro

[Eric Miao]

On Sat, Oct 23, 2010 at 5:26 AM, Matt Reimer <mreimer@sdgsystems.com> wrote:
> Fix an off-by-one bug in the LCCR2_BegFrmDel() macro. The vertical back porch
> knob LCCR2[BFW] specifies the number of clock periods minus one, but the
> macro wasn't doing the subtraction.
> ---
> ?arch/arm/mach-pxa/include/mach/regs-lcd.h | ? ?2 +-
> ?1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-pxa/include/mach/regs-lcd.h b/arch/arm/mach-pxa/include/mach/regs-lcd.h
> index f82dcea..5074d18 100644
> --- a/arch/arm/mach-pxa/include/mach/regs-lcd.h
> +++ b/arch/arm/mach-pxa/include/mach/regs-lcd.h
> @@ -107,7 +107,7 @@
> ?#define LCCR2_EndFrmDel(Tln) ? ((Tln) << FShft (LCCR2_EFW))
>
> ?#define LCCR2_BFW ? ? ?Fld (8, 24) ? ? /* Beginning-of-Frame line clock */
> -#define LCCR2_BegFrmDel(Tln) ? ((Tln) << FShft (LCCR2_BFW))
> +#define LCCR2_BegFrmDel(Tln) ? (((Tln) - 1) << FShft (LCCR2_BFW))

Nothing in the spec suggests that this minus 1 is necessary. Could you please
double check?

>
> ?#define LCCR3_API ? ? ?(0xf << 16) ? ? /* AC Bias pin trasitions per interrupt */
> ?#define LCCR3_API_S ? ?16
> --
> 1.7.0.4
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

[PATCH] pxafb: correct the vertical back porch macro

[Marek Vasut]

Dne So 23. ??jna 2010 03:07:34 Eric Miao napsal(a):
> On Sat, Oct 23, 2010 at 5:26 AM, Matt Reimer <mreimer@sdgsystems.com> wrote:
> > Fix an off-by-one bug in the LCCR2_BegFrmDel() macro. The vertical back
> > porch knob LCCR2[BFW] specifies the number of clock periods minus one,
> > but the macro wasn't doing the subtraction.
> > ---
> >  arch/arm/mach-pxa/include/mach/regs-lcd.h |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/arm/mach-pxa/include/mach/regs-lcd.h
> > b/arch/arm/mach-pxa/include/mach/regs-lcd.h index f82dcea..5074d18
> > 100644
> > --- a/arch/arm/mach-pxa/include/mach/regs-lcd.h
> > +++ b/arch/arm/mach-pxa/include/mach/regs-lcd.h
> > @@ -107,7 +107,7 @@
> >  #define LCCR2_EndFrmDel(Tln)   ((Tln) << FShft (LCCR2_EFW))
> > 
> >  #define LCCR2_BFW      Fld (8, 24)     /* Beginning-of-Frame line clock
> > */ -#define LCCR2_BegFrmDel(Tln)   ((Tln) << FShft (LCCR2_BFW))
> > +#define LCCR2_BegFrmDel(Tln)   (((Tln) - 1) << FShft (LCCR2_BFW))
> 
> Nothing in the spec suggests that this minus 1 is necessary. Could you
> please double check?

It doesn't fit the settings in U-Boot or something ... it's a long known thing. I 
remember in U-Boot, one had to actually subtract one in the LCD configuration 
data structure so it'd be programmer correctly.

> 
> >  #define LCCR3_API      (0xf << 16)     /* AC Bias pin trasitions per
> > interrupt */ #define LCCR3_API_S    16
> > --
> > 1.7.0.4
> > 
> > 
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH] pxafb: correct the vertical back porch macro

[Lothar Waßmann]

Hi,

Matt Reimer writes:
> Fix an off-by-one bug in the LCCR2_BegFrmDel() macro. The vertical back porch
> knob LCCR2[BFW] specifies the number of clock periods minus one, but the
> macro wasn't doing the subtraction.
>
Nope! This is true only for the ELW and BLW settings in LCCR1, but not
for the EFW and BFW settings in LCCR2! The vertical front and back
porch can actually be programmed to a length of zero lines.

Furthermore, if you introduce such a change, you should also convert
all users of the macro to use an appropriately adjusted value so that
the result will be the same as without the patch!


Lothar Wa?mann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________

[PATCH] pxafb: correct the vertical back porch macro

[Matt Reimer]

On Fri, Oct 22, 2010 at 9:07 PM, Eric Miao <eric.y.miao@gmail.com> wrote:
> On Sat, Oct 23, 2010 at 5:26 AM, Matt Reimer <mreimer@sdgsystems.com> wrote:
>> Fix an off-by-one bug in the LCCR2_BegFrmDel() macro. The vertical back porch
>> knob LCCR2[BFW] specifies the number of clock periods minus one, but the
>> macro wasn't doing the subtraction.
>> ---
>> ?arch/arm/mach-pxa/include/mach/regs-lcd.h | ? ?2 +-
>> ?1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-pxa/include/mach/regs-lcd.h b/arch/arm/mach-pxa/include/mach/regs-lcd.h
>> index f82dcea..5074d18 100644
>> --- a/arch/arm/mach-pxa/include/mach/regs-lcd.h
>> +++ b/arch/arm/mach-pxa/include/mach/regs-lcd.h
>> @@ -107,7 +107,7 @@
>> ?#define LCCR2_EndFrmDel(Tln) ? ((Tln) << FShft (LCCR2_EFW))
>>
>> ?#define LCCR2_BFW ? ? ?Fld (8, 24) ? ? /* Beginning-of-Frame line clock */
>> -#define LCCR2_BegFrmDel(Tln) ? ((Tln) << FShft (LCCR2_BFW))
>> +#define LCCR2_BegFrmDel(Tln) ? (((Tln) - 1) << FShft (LCCR2_BFW))
>
> Nothing in the spec suggests that this minus 1 is necessary. Could you please
> double check?

The docs I have describing this bit (vol 3 rev 1.0) say, "If LCCR6[29]
is set for LCD panels with an internal frame buffer, BFW specifies the
pulse width of the read signal (L_FCLK_RD), which is equal to
(BFW+1)*LCD_CLK_PERIOD."

I saw the "+1" in the docs and that the macro didn't match. But I
hadn't read closely enough to see that BFW gets different meanings
depending on what mode the controller is in. Because of this, I agree
it should stay as it is, so I withdraw this patch.

Matt