diff mbox

[U-Boot,7/7] common lcd: simplify lcd_display_bitmap

Message ID 1337859764-16086-8-git-send-email-grinberg@compulab.co.il
State Changes Requested
Delegated to: Anatolij Gustschin
Headers show

Commit Message

Igor Grinberg May 24, 2012, 11:42 a.m. UTC 
From: Nikita Kiryanov <nikita@compulab.co.il>

Move highly platform dependant code into its own functions to reduce the
number of #ifdefs in lcd_display_bitmap

Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
---
 common/lcd.c |   44 +++++++++++++++++++++++++++-----------------
 1 files changed, 27 insertions(+), 17 deletions(-)

Comments

Anatolij Gustschin June 8, 2012, 1:38 p.m. UTC | #1 
Hi,

On Thu, 24 May 2012 14:42:44 +0300
Igor Grinberg <grinberg@compulab.co.il> wrote:

> From: Nikita Kiryanov <nikita@compulab.co.il>
> 
> Move highly platform dependant code into its own functions to reduce the
> number of #ifdefs in lcd_display_bitmap
> 
> Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
> ---
>  common/lcd.c |   44 +++++++++++++++++++++++++++-----------------
>  1 files changed, 27 insertions(+), 17 deletions(-)
> 
> diff --git a/common/lcd.c b/common/lcd.c
> index 199a8c2..a55ee58 100644
> --- a/common/lcd.c
> +++ b/common/lcd.c
> @@ -638,6 +638,29 @@ static void splash_align_axis(int *axis, unsigned long panel_size,
...
> +#if defined(CONFIG_BMP_16BPP)
> +#if defined(CONFIG_ATMEL_LCD_BGR555)
> +static inline void configuration_fb_puts(uchar *fb, uchar *from)
> +{
> +	*(fb++) = ((from[0] & 0x1f) << 2) | (from[1] & 0x03);
> +	*(fb++) = (from[0] & 0xe0) | ((from[1] & 0x7c) >> 2);
> +	from += 2;
> +}
> +#else
> +static inline void configuration_fb_puts(uchar *fb, uchar *from)
> +{
> +	*(fb++) = *(from++);
> +	*(fb++) = *(from++);
> +}
> +#endif
> +#endif /* CONFIG_BMP_16BPP */

This won't work. The original code increments 'fb' and 'bmap' pointers
in the inner for loop. Using this function in the inner loop won't
increment the pointers as needed, as these will only be incremented in
the function itself (as local variables).

Also please use a different name for the macro, CONFIGURATION_FB_PUTB
isn't a descriptive name. FB_PUT_PIXEL or similar perhaps?

Thanks,

Anatolij
diff mbox

Patch

diff --git a/common/lcd.c b/common/lcd.c
index 199a8c2..a55ee58 100644
--- a/common/lcd.c
+++ b/common/lcd.c
@@ -638,6 +638,29 @@  static void splash_align_axis(int *axis, unsigned long panel_size,
 }
 #endif
 
+#if defined CONFIG_CPU_PXA || defined(CONFIG_ATMEL_LCD)
+#define CONFIGURATION_FB_PUTB(fb, from) *(fb)++ = *(from)++
+#elif defined(CONFIG_MPC823) || defined(CONFIG_MCC200)
+#define CONFIGURATION_FB_PUTB(fb, from) *(fb)++ = (255 - *(from)++)
+#endif
+
+#if defined(CONFIG_BMP_16BPP)
+#if defined(CONFIG_ATMEL_LCD_BGR555)
+static inline void configuration_fb_puts(uchar *fb, uchar *from)
+{
+	*(fb++) = ((from[0] & 0x1f) << 2) | (from[1] & 0x03);
+	*(fb++) = (from[0] & 0xe0) | ((from[1] & 0x7c) >> 2);
+	from += 2;
+}
+#else
+static inline void configuration_fb_puts(uchar *fb, uchar *from)
+{
+	*(fb++) = *(from++);
+	*(fb++) = *(from++);
+}
+#endif
+#endif /* CONFIG_BMP_16BPP */
+
 int lcd_display_bitmap(ulong bmp_image, int x, int y)
 {
 #if !defined(CONFIG_MCC200)
@@ -761,11 +784,7 @@  int lcd_display_bitmap(ulong bmp_image, int x, int y)
 			WATCHDOG_RESET();
 			for (j = 0; j < width; j++) {
 				if (bpix != 16) {
-#if defined(CONFIG_CPU_PXA) || defined(CONFIG_ATMEL_LCD)
-					*(fb++) = *(bmap++);
-#elif defined(CONFIG_MPC823) || defined(CONFIG_MCC200)
-					*(fb++) = 255 - *(bmap++);
-#endif
+					CONFIGURATION_FB_PUTB(fb, bmap);
 				} else {
 					*(uint16_t *)fb = cmap_base[*(bmap++)];
 					fb += sizeof(uint16_t) / sizeof(*fb);
@@ -780,18 +799,9 @@  int lcd_display_bitmap(ulong bmp_image, int x, int y)
 	case 16:
 		for (i = 0; i < height; ++i) {
 			WATCHDOG_RESET();
-			for (j = 0; j < width; j++) {
-#if defined(CONFIG_ATMEL_LCD_BGR555)
-				*(fb++) = ((bmap[0] & 0x1f) << 2) |
-					(bmap[1] & 0x03);
-				*(fb++) = (bmap[0] & 0xe0) |
-					((bmap[1] & 0x7c) >> 2);
-				bmap += 2;
-#else
-				*(fb++) = *(bmap++);
-				*(fb++) = *(bmap++);
-#endif
-			}
+			for (j = 0; j < width; j++)
+				configuration_fb_puts(fb, bmap);
+
 			bmap += (padded_line - width) * 2;
 			fb   -= (width * 2 + lcd_line_length);
 		}