On 06/08/2012 03:52 PM, Anatolij Gustschin wrote:
> Hi,
>
> On Thu, 24 May 2012 14:42:39 +0300
> Igor Grinberg<[email protected]> wrote:
>
>> From: Nikita Kiryanov<[email protected]>
>>
>> Simplify #ifdefs by slightly changing the order of operations
>>
>> Signed-off-by: Nikita Kiryanov<[email protected]>
>> Signed-off-by: Igor Grinberg<[email protected]>
>> ---
>> common/lcd.c | 20 ++++++++------------
>> 1 files changed, 8 insertions(+), 12 deletions(-)
>>
>> diff --git a/common/lcd.c b/common/lcd.c
>> index 506a138..3b2f25f 100644
>> --- a/common/lcd.c
>> +++ b/common/lcd.c
>> @@ -525,20 +525,18 @@ void bitmap_plot(int x, int y)
>> fb = (uchar *)(lcd_base + y * lcd_line_length + x);
>>
>> if (NBITS(panel_info.vl_bpix)< 12) {
>> - /* Leave room for default color map */
>> + /* Leave room for default color map
>> + * default case: generic system with no cmap (most likely
16bpp)
>> + * We set cmap to the source palette, so no change is done.
>> + * This avoids even more ifdefs in the next stanza
>> + */
>> + cmap = bmp_logo_palette;
>> #if defined(CONFIG_CPU_PXA)
>> cmap = (ushort *) fbi->palette;
>> #elif defined(CONFIG_MPC823)
>> cmap = (ushort *)&(cp->lcd_cmap[BMP_LOGO_OFFSET *
sizeof(ushort)]);
>> #elif defined(CONFIG_ATMEL_LCD)
>> cmap = (uint *) (panel_info.mmio + ATMEL_LCDC_LUT(0));
>> -#else
>> - /*
>> - * default case: generic system with no cmap (most likely
16bpp)
>> - * We set cmap to the source palette, so no change is done.
>> - * This avoids even more ifdef in the next stanza
>> - */
>> - cmap = bmp_logo_palette;
>> #endif
>>
>> WATCHDOG_RESET();
>> @@ -680,14 +678,12 @@ int lcd_display_bitmap(ulong bmp_image, int x,
int y)
>> for (i = 0; i< colors; ++i) {
>> bmp_color_table_entry_t cte = bmp->color_table[i];
>> #if !defined(CONFIG_ATMEL_LCD)
>> - ushort colreg =
>> + *cmap =
>> ( ((cte.red)<< 8)& 0xf800) |
>> ( ((cte.green)<< 3)& 0x07e0) |
>> ( ((cte.blue)>> 3)& 0x001f) ;
>> #ifdef CONFIG_SYS_INVERT_COLORS
>> - *cmap = 0xffff - colreg;
>> -#else
>> - *cmap = colreg;
>> + *cmap = 0xffff - *cmap;
>> #endif
>> #if defined(CONFIG_MPC823)
>> cmap--;
>
> Sorry, but I do not like this change. We should not try to simplify it
> this way. Better would be to just reduce all this color map setting code
> to something like:
>
> for (i = 0; i< ARRAY_SIZE(bmp_logo_palette); ++i) {
> bmp_color_table_entry_t cte = bmp->color_table[i];
> lcd_setcolreg(i, cte.red, cte.green, cte.blue);
> }
>
> and fix lcd_setcolreg() functions of the drivers if/where needed.
>
> Thanks,
>
> Anatolij
This does sound like the way to go, but having looked at the different
versions of lcd_setcolreg I am unsure of how to consolidate the code in
lcd_display_bitmap (and bitmap_plot in patch 3) with the
implementations I see in the drivers.
For one, they shift the color values in a different way than the code
in common/lcd, and I'm not sure who I should listen to, what's in the
drivers or common/lcd...
Also, I see that lcd_setcolreg is used in lcd_clear, where it sets
console colors. This raises the question can/should the treatment of
bmp color map values be similar to that of console color values?
I'd appreciate some guidance on these issues.
Thanks,
Nikita
_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/mailman/listinfo/u-boot
