diff mbox

[U-Boot,RFC] common/lcd: use lcd_setcolreg() in bitmap_plot

Message ID 1351250594-5397-1-git-send-email-andreas.devel@googlemail.com
State RFC
Delegated to: Anatolij Gustschin
Headers show

Commit Message

Andreas Bießmann Oct. 26, 2012, 11:23 a.m. UTC
The lcd_setcolreg() is a API provided by the lcd driver and used to setup the
color lookup table. However the lcd driver setup the CLUT by using a lot
of #ifdiffery and accessing the CLUT arrays which are cleanly hidden by the API
call. Remove that and use the API.

Signed-off-by: Andreas Bießmann <andreas.devel@googlemail.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Bo Shen <voice.shen@atmel.com>
Cc: Tom Rini <trini@ti.com>
---
This is a RFC currently cause it touches a lot of boards but is only runtime
tested on at91sam9263ek. Unfortunately a 'MAKEALL -a arm' does break cause the
atmel_hlcd driver do not provide the required API (lcd_setcolreg()) -> Bo, can
you please fix this, I do not have an atmel_hlcdc enabled device.

On the other hand this change also touches other architectures I can not test.
This patch is actually only compile tested for arm and avr32 arches.

And last I have to say that there is another position in this file where the
CLUT is modified directly by the code. This should also be changed to use the
lcd_setcolreg API.

This patch shall superseed http://patchwork.ozlabs.org/patch/193466/

 common/lcd.c |   47 +++++++++--------------------------------------
 1 file changed, 9 insertions(+), 38 deletions(-)

Comments

Nikita Kiryanov Oct. 28, 2012, 7:10 a.m. UTC | #1
Hi Andreas,

I think this change can also be applied to lcd_display_bitmap(), and
then configuration_get_cmap() can be eliminated.
Andreas Bießmann Oct. 29, 2012, 9:16 a.m. UTC | #2
Hi Nikita,

On 28.10.2012 08:10, Nikita Kiryanov wrote:
> Hi Andreas,
> 
> I think this change can also be applied to lcd_display_bitmap(), and
> then configuration_get_cmap() can be eliminated.

hopefully yes, but first of all I would like to hear that the change can
work on all supported devices, so please test and send results to the list.

Best regards

Andreas Bießmann
Bo Shen Oct. 29, 2012, 9:17 a.m. UTC | #3
On 10/26/2012 19:23, Andreas Bießmann wrote:
> The lcd_setcolreg() is a API provided by the lcd driver and used to setup the
> color lookup table. However the lcd driver setup the CLUT by using a lot
> of #ifdiffery and accessing the CLUT arrays which are cleanly hidden by the API
> call. Remove that and use the API.
>
> Signed-off-by: Andreas Bießmann <andreas.devel@googlemail.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Bo Shen <voice.shen@atmel.com>
> Cc: Tom Rini <trini@ti.com>
> ---
> This is a RFC currently cause it touches a lot of boards but is only runtime
> tested on at91sam9263ek. Unfortunately a 'MAKEALL -a arm' does break cause the
> atmel_hlcd driver do not provide the required API (lcd_setcolreg()) -> Bo, can
> you please fix this, I do not have an atmel_hlcdc enabled device.
>
> On the other hand this change also touches other architectures I can not test.
> This patch is actually only compile tested for arm and avr32 arches.
>
> And last I have to say that there is another position in this file where the
> CLUT is modified directly by the code. This should also be changed to use the
> lcd_setcolreg API.
>
> This patch shall superseed http://patchwork.ozlabs.org/patch/193466/
>
>   common/lcd.c |   47 +++++++++--------------------------------------
>   1 file changed, 9 insertions(+), 38 deletions(-)

Tested on sam9x5ek board. It works well.

Tested-by: Bo Shen <voice.shen@atmel.com>

> diff --git a/common/lcd.c b/common/lcd.c
> index b6be800..4938381 100644
> --- a/common/lcd.c
> +++ b/common/lcd.c
> @@ -523,11 +523,6 @@ static inline ushort *configuration_get_cmap(void)
>   #ifdef CONFIG_LCD_LOGO
>   void bitmap_plot(int x, int y)
>   {
> -#ifdef CONFIG_ATMEL_LCD
> -	uint *cmap = (uint *)bmp_logo_palette;
> -#else
> -	ushort *cmap = (ushort *)bmp_logo_palette;
> -#endif
>   	ushort i, j;
>   	uchar *bmap;
>   	uchar *fb;
> @@ -545,44 +540,20 @@ 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
> -		 * default case: generic system with no cmap (most likely 16bpp)
> -		 * cmap was set to the source palette, so no change is done.
> -		 * This avoids even more ifdefs in the next stanza
> -		 */
> -#if defined(CONFIG_MPC823)
> -		cmap = (ushort *) &(cp->lcd_cmap[BMP_LOGO_OFFSET * sizeof(ushort)]);
> -#elif defined(CONFIG_ATMEL_LCD)
> -		cmap = (uint *)configuration_get_cmap();
> -#else
> -		cmap = configuration_get_cmap();
> -#endif
>
>   		WATCHDOG_RESET();
>
>   		/* Set color map */
>   		for (i = 0; i < ARRAY_SIZE(bmp_logo_palette); ++i) {
> -			ushort colreg = bmp_logo_palette[i];
> -#ifdef CONFIG_ATMEL_LCD
> -			uint lut_entry;
> -#ifdef CONFIG_ATMEL_LCD_BGR555
> -			lut_entry = ((colreg & 0x000F) << 11) |
> -					((colreg & 0x00F0) <<  2) |
> -					((colreg & 0x0F00) >>  7);
> -#else /* CONFIG_ATMEL_LCD_RGB565 */
> -			lut_entry = ((colreg & 0x000F) << 1) |
> -					((colreg & 0x00F0) << 3) |
> -					((colreg & 0x0F00) << 4);
> -#endif
> -			*(cmap + BMP_LOGO_OFFSET) = lut_entry;
> -			cmap++;
> -#else /* !CONFIG_ATMEL_LCD */
> -#ifdef  CONFIG_SYS_INVERT_COLORS
> -			*cmap++ = 0xffff - colreg;
> -#else
> -			*cmap++ = colreg;
> -#endif
> -#endif /* CONFIG_ATMEL_LCD */
> +			/* use the most significant bits here */
> +			uint8_t red   = ((bmp_logo_palette[i] >> 4) & 0xF0);
> +			uint8_t green = ((bmp_logo_palette[i] >> 0) & 0xF0);
> +			uint8_t blue  = ((bmp_logo_palette[i] << 4) & 0xF0);
> +			debug("LCD: setup colreg %u with "
> +			      "R: 0x%02x G: 0x%02x B: 0x%02x (0x%04x)\n",
> +			      i+BMP_LOGO_OFFSET, red, green, blue, bmp_logo_palette[i]);
> +			/* leave room for the default colormap here */
> +			lcd_setcolreg(i+BMP_LOGO_OFFSET, red, green, blue);
>   		}
>
>   		WATCHDOG_RESET();
>
diff mbox

Patch

diff --git a/common/lcd.c b/common/lcd.c
index b6be800..4938381 100644
--- a/common/lcd.c
+++ b/common/lcd.c
@@ -523,11 +523,6 @@  static inline ushort *configuration_get_cmap(void)
 #ifdef CONFIG_LCD_LOGO
 void bitmap_plot(int x, int y)
 {
-#ifdef CONFIG_ATMEL_LCD
-	uint *cmap = (uint *)bmp_logo_palette;
-#else
-	ushort *cmap = (ushort *)bmp_logo_palette;
-#endif
 	ushort i, j;
 	uchar *bmap;
 	uchar *fb;
@@ -545,44 +540,20 @@  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
-		 * default case: generic system with no cmap (most likely 16bpp)
-		 * cmap was set to the source palette, so no change is done.
-		 * This avoids even more ifdefs in the next stanza
-		 */
-#if defined(CONFIG_MPC823)
-		cmap = (ushort *) &(cp->lcd_cmap[BMP_LOGO_OFFSET * sizeof(ushort)]);
-#elif defined(CONFIG_ATMEL_LCD)
-		cmap = (uint *)configuration_get_cmap();
-#else
-		cmap = configuration_get_cmap();
-#endif
 
 		WATCHDOG_RESET();
 
 		/* Set color map */
 		for (i = 0; i < ARRAY_SIZE(bmp_logo_palette); ++i) {
-			ushort colreg = bmp_logo_palette[i];
-#ifdef CONFIG_ATMEL_LCD
-			uint lut_entry;
-#ifdef CONFIG_ATMEL_LCD_BGR555
-			lut_entry = ((colreg & 0x000F) << 11) |
-					((colreg & 0x00F0) <<  2) |
-					((colreg & 0x0F00) >>  7);
-#else /* CONFIG_ATMEL_LCD_RGB565 */
-			lut_entry = ((colreg & 0x000F) << 1) |
-					((colreg & 0x00F0) << 3) |
-					((colreg & 0x0F00) << 4);
-#endif
-			*(cmap + BMP_LOGO_OFFSET) = lut_entry;
-			cmap++;
-#else /* !CONFIG_ATMEL_LCD */
-#ifdef  CONFIG_SYS_INVERT_COLORS
-			*cmap++ = 0xffff - colreg;
-#else
-			*cmap++ = colreg;
-#endif
-#endif /* CONFIG_ATMEL_LCD */
+			/* use the most significant bits here */
+			uint8_t red   = ((bmp_logo_palette[i] >> 4) & 0xF0);
+			uint8_t green = ((bmp_logo_palette[i] >> 0) & 0xF0);
+			uint8_t blue  = ((bmp_logo_palette[i] << 4) & 0xF0);
+			debug("LCD: setup colreg %u with "
+			      "R: 0x%02x G: 0x%02x B: 0x%02x (0x%04x)\n",
+			      i+BMP_LOGO_OFFSET, red, green, blue, bmp_logo_palette[i]);
+			/* leave room for the default colormap here */
+			lcd_setcolreg(i+BMP_LOGO_OFFSET, red, green, blue);
 		}
 
 		WATCHDOG_RESET();