[U-Boot-Users] [PATCH] wrong lcd splashscreen colors (pxa)

[U-Boot-Users] [PATCH] wrong lcd splashscreen colors (pxa)

[Francesco Mandracci]

Hello world :)

it seems that the computation of colors in
common/lcd.c:lcd_display_bitmap() is wrong for green and blue
components. I checked on version 1.1.3 and on cvs.sf.net.

I'm working on a PXA270 but this seems general (as far as I understand).
The error is not evident until you use full colors (i.e. if color RGB
components/bytes are either 0x00 of 0xff).

------------------------------------------------------
                        bmp_color_table_entry_t cte = bmp->color_table[i];
                        ushort colreg =
                                ( ((cte.red)   << 8) & 0xf800) |
-                               ( ((cte.green) << 4) & 0x07e0) |
-                               ( (cte.blue) & 0x001f) ;
+                               ( ((cte.green) << 3) & 0x07e0) |
+                               ( ((cte.blue)  >> 3) & 0x001f) ;

 #ifdef CFG_INVERT_COLORS
                        *cmap = 0xffff - colreg;
 #else
                        *cmap = colreg;
 #endif
------------------------------------------------------

Hoping this helps.
Ciao
    Francesco Mandracci

PS
Thank you all for the great job: it's the first time I get to a
bootloader prompt (on a new hardware) after only about 6 hours of work.

[U-Boot-Users] [PATCH] wrong lcd splashscreen colors (pxa)

[Wolfgang Denk]

In message <43299EC6.6050206@primaelectronics.com> you wrote:
> 
> it seems that the computation of colors in
> common/lcd.c:lcd_display_bitmap() is wrong for green and blue

It's working fine on all the PowerPC systems where I am able to test.

> components. I checked on version 1.1.3 and on cvs.sf.net.
> 
> I'm working on a PXA270 but this seems general (as far as I understand).

Probably not general, but maybe byte-order dependent ?

> 
> ------------------------------------------------------
>                         bmp_color_table_entry_t cte = bmp->color_table[i];
>                         ushort colreg =
>                                 ( ((cte.red)   << 8) & 0xf800) |
> -                               ( ((cte.green) << 4) & 0x07e0) |
> -                               ( (cte.blue) & 0x001f) ;
> +                               ( ((cte.green) << 3) & 0x07e0) |
> +                               ( ((cte.blue)  >> 3) & 0x001f) ;
> 

Please see section "Submitting Patches" in the README. In  this  form
this  is  not  acceptable.  Also  make  sure  that  you  don't  break
big-endian systems with your modification.

> Thank you all for the great job: it's the first time I get to a
> bootloader prompt (on a new hardware) after only about 6 hours of work.

Congrats!

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Is the glass half empty, half full, or twice as large as it needs to

[U-Boot-Users] [PATCH] wrong lcd splashscreen colors (pxa)

[Francesco Mandracci]

Wolfgang Denk wrote:
> It's working fine on all the PowerPC systems where I am able to test.
you forgot to quote this:
  The error is not evident until you use full colors (i.e. if color RGB
  components/bytes are either 0x00 of 0xff).

In other (maybe clearer) words: "all bitmaps with palette colors whose
RGB components don't differ in the 5 high bits in the blue byte and in
the 6 high bits in the green byte" will display correctly.

That may seem not so important, but if you are testing the lcd wirings
of the brand new hardware then you really want each color to be ok down
to the last bit!
[http://www.dreamvideo.it/video/immagini/monoscopio.jpg]

>>I'm working on a PXA270 but this seems general (as far as I understand).
> Probably not general, but maybe byte-order dependent ?
No, it's not endianness dependent:
1) "bmp_color_table_entry_t" is byte order independent (4 packed __u8
fields)
2) "ushort colreg" is used coherently in each operation

The C code fills a 16bit palette entry with 3 RGB bytes in the form

  RRRR RGGG GGGB BBBB
 [....|....|....|....]  masks: (R,G,B) (0xf800, 0x07e0, 0x001f)
  fedc ba98 7654 3210

The used bits are the highest 5 bits for the red and blue bytes and the
 6 high bits for the green byte. This did not change neither in PowerPC
nor in PXA in all cvs.sf.net revisions I've found.

The computation may be either:
colreg = ((red   & 0xf8) << 8)  /* [RRRR|Rrrr|....|....]     */
       | ((green & 0xfc) << 3)  /* [....|.GGG|GGGg|g...]     */
       | ((blue  & 0xf8) >> 3); /* [....|....|...B|BBBB]bbb. */
or:
colreg = ((red   << 8) & 0xf800)
       | ((green << 3) & 0x07e0)   /* versus " << 4" */
       | ((blue  >> 3) & 0x001f);  /* versus " >> 0" */
I choose the latter form because it's the way you adopted.

> Please see section "Submitting Patches" in the README. In  this  form
> this  is  not  acceptable.
Yes Sir! :) :) :)
Actually, please forgive me if I didn't follow all your recommendations.
As an excuse: I understand your heavy efforts (not only in this mailing
list), but it was only a two-lines very-punctual not-show-stopper patch. :)

Anyway, if we'll going to use Das U-Boot in the company product then
I'll ask for MACH_TYPE_ stuff and propose some bigger patches regarding
CONFIGS_PXA27x. But today there is still nothing stable enough on my
side... with the exception of this little splashscreen issue.

Ciao
    Francesco Mandracci

[U-Boot-Users] [PATCH] wrong lcd splashscreen colors (pxa)

[Wolfgang Denk]

In message <432AA14F.8070801@primaelectronics.com> you wrote:
>
> > Please see section "Submitting Patches" in the README. In  this  form
> > this  is  not  acceptable.
> Yes Sir! :) :) :)

Thanks :-)

> As an excuse: I understand your heavy efforts (not only in this mailing
> list), but it was only a two-lines very-punctual not-show-stopper patch. :)

Nevertheless it is important to me to be able to apply this  with  as
little  effort  as  possible, and a proper patch with CHANGELOG entry
and sign-off line and everything greatly helps. So pleast resubmit as
proper patch.

> Anyway, if we'll going to use Das U-Boot in the company product then
> I'll ask for MACH_TYPE_ stuff and propose some bigger patches regarding
> CONFIGS_PXA27x. But today there is still nothing stable enough on my
> side... with the exception of this little splashscreen issue.

It's better not to wait until then, and keep  functionally  different
patcehs  separated  (actually  this  is  another  of the requirements
anyway).

Thanks.

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Ernest asks Frank how long he has been working for the company.
        "Ever since they threatened to fire me."