Subject: Re: [PATCH] au1100fb.c ported from 2.4 to 2.6

From: Ulrich Eckhardt <[email-masked]>
Date: Wed, 2 Feb 2005 10:05:18 +0100

Christian wrote:
> works in 16bpp, anyone with a system that works in 8bbp can give me a
> hand to test this mode?

I have a system that has a 4bpp monochrome display and the driver doesn't 
work. After initializing, it turns on the display[1] but I fail to get any 
content onto it. I extended the array with the display characteristics 
according to an existing driver, but to no avail.

I'm pretty lost there, since I have zero prior experience with framebuffers or 
video drivers, so I'd appreciate any hint that might get me towards debugging 
this.

Uli

[1] p_lcd_reg->lcd_control |= LCD_CONTROL_GO;

---

From: Christian <[email-masked]>
Date: Fri, 04 Feb 2005 10:56:43 +0100

> video drivers, so I'd appreciate any hint that might get me towards debugging 

Hi,

you actually have to do the following modification modifications. 

First you have to modify the function au1100fb_setcolreg, add the switch
case for 4bpp. Look in the Alchemy manual for the format of the palette
in the 4 bpp case. Then look for my comment "TODO: 8bbp", put a switch
and for pseudocolor like visual ... I think this is not even necessary.
More important is that you change the monitor type. If you want I can
send you privately a patch this weekend, but I just cannot assure it
works, you have to test it and eventually contribute.

-- 
Christian <[email-masked]>

---

From: Ulrich Eckhardt <[email-masked]>
Date: Fri, 4 Feb 2005 14:18:47 +0100

> and for pseudocolor like visual ... I think this is not even necessary.

Thanks, will look.

> More important is that you change the monitor type. 

Yep, finding working parameters without real knowledge of what the registers 
mean was the hardest part, but I have it working now.

> If you want I can 

I'd appreciate that, thank you.

Currently I'm facing a different problem though, look at this code and the 
bitmap it generates (4bpp):

   unsigned long* fb = my_fb_info->fb_virt_start;
   fb[0]   = 0x0000000f; // *.......
   fb[40]  = 0x000000f0; // .*......
   fb[80]  = 0x000000ff; // **......
   fb[120] = 0xf0000f00; // ..*....*
   fb[160] = 0xff000f0f; // *.*...**
   fb[200] = 0xfff00ff0; // .**..***
   fb[240] = 0xffff0fff; // ***.****

To me, expecting the LSB to be part of the leftmost pixel is not really 
surprising, but functions like cfb_fillrect() (or rather bitfill32() etc) 
expect the order to be the other way around. I'm afraid that other 
framebuffer functions also expect that, but I haven't gotten far enough to 
test those. I only tested cfb_fillrect() and, seeing it fail, started looking 
for answers... any idea?

I stumbled across another .. err .. 'feature': take a look at the appended 
patch, in particular the handling of the mask for the first long, am I 
dreaming or is this in fact handled wrongly in the current FB code? I'm a bit 
confused, because the same mistake is made in pretty much every FB driver I 
looked at.

Thanks

Uli

---

Index: cfbcopyarea.c
===================================================================
RCS file: /home/cvs/linux/drivers/video/cfbcopyarea.c,v
retrieving revision 1.10
diff -u -w -r1.10 cfbcopyarea.c
--- cfbcopyarea.c 12 Oct 2004 01:45:47 -0000 1.10
+++ cfbcopyarea.c 4 Feb 2005 13:11:16 -0000
@@ -70,7 +70,7 @@
   } else {
    // Multiple destination words
    // Leading bits
-   if (first) {
+   if (first != ~0UL) {
     
     FB_WRITEL((FB_READL(src) & first) | 
        (FB_READL(dst) & ~first), dst);
@@ -223,7 +223,7 @@
   } else {
    // Multiple destination words
    // Leading bits
-   if (first) {
+   if (first != ~0UL) {
     FB_WRITEL((FB_READL(src) & first) | (FB_READL(dst) & ~first), dst); 
     dst--;
     src--;
Index: cfbfillrect.c
===================================================================
RCS file: /home/cvs/linux/drivers/video/cfbfillrect.c,v
retrieving revision 1.8
diff -u -w -r1.8 cfbfillrect.c
--- cfbfillrect.c 12 Oct 2004 01:45:47 -0000 1.8
+++ cfbfillrect.c 4 Feb 2005 13:11:16 -0000
@@ -142,7 +142,7 @@
  } else {
   // Multiple destination words
   // Leading bits
-  if (first) {
+  if (first != ~0UL) {
    FB_WRITEL(comp(val, FB_READL(dst), first), dst);
    dst++;
    n -= BITS_PER_LONG-dst_idx;
@@ -166,7 +166,7 @@
   
   // Trailing bits
   if (last)
-   FB_WRITEL(comp(val, FB_READL(dst), first), dst);
+   FB_WRITEL(comp(val, FB_READL(dst), last), dst);
  }
 }
 
@@ -197,7 +197,7 @@
  } else {
   // Multiple destination words
   // Leading bits
-  if (first) {
+  if (first != ~0UL) {
    FB_WRITEL(comp(pat, FB_READL(dst), first), dst);
    dst++;
    pat = pat << left | pat >> right;
@@ -224,7 +224,7 @@
   
   // Trailing bits
   if (last)
-   FB_WRITEL(comp(pat, FB_READL(dst), first), dst);
+   FB_WRITEL(comp(pat, FB_READL(dst), last), dst);
  }
 }
 
@@ -252,7 +252,7 @@
  } else {
   // Multiple destination words
   // Leading bits
-  if (first) {
+  if (first != ~0UL) {
    dat = FB_READL(dst);
    FB_WRITEL(comp(dat ^ val, dat, first), dst);
    dst++;
@@ -287,7 +287,7 @@
   // Trailing bits
   if (last) {
    dat = FB_READL(dst);
-   FB_WRITEL(comp(dat ^ val, dat, first), dst);
+   FB_WRITEL(comp(dat ^ val, dat, last), dst);
   }
  }
 }
@@ -320,7 +320,7 @@
  } else {
   // Multiple destination words
   // Leading bits
-  if (first) {
+  if (first != ~0UL) {
    dat = FB_READL(dst);
    FB_WRITEL(comp(dat ^ pat, dat, first), dst);
    dst++;
@@ -354,7 +354,7 @@
   // Trailing bits
   if (last) {
    dat = FB_READL(dst);
-   FB_WRITEL(comp(dat ^ pat, dat, first), dst);
+   FB_WRITEL(comp(dat ^ pat, dat, last), dst);
   }
  }
 }

---

From: Ulrich Eckhardt <[email-masked]>
Date: Tue, 8 Feb 2005 15:14:07 +0100

Another two suggestions:

- you hard-coded the used display in au1100fb.{h,c} with #ifdef CONFIG_WWPC. 
There is another option already used in arch/mips/au1000/common/setup.c, and 
that is to append the necessary lines to the kernel commandline if no 
conflicting arguments are present.

- I have removed the comments that say to which field a constant belongs and 
instead converted au1100fb.h to use new-style initialisers which is just 
easier to not get wrong. I also added parameters for my display and replaced 
the #define for uint32. The new file is attached as whole, I didn't know 
against what to diff it...

Could you merge the file and resend the patch? I hope it will get committed 
then.

Uli