[U-Boot] [PATCH v3 13/18] lcd: Add support for flushing LCD fb from dcache after update

[Lukasz Majewski <l.majewski@samsung.com>]

Hi Simon,

> This provides an option for the LCD to flush the dcache after each
> update (puts, scroll or clear).
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> Changes in v2:
> - Put the LCD cache flush logic into lcd_putc() instead of lcd_puts()
> 
> Changes in v3:
> - Put the LCD cache flush logic back into lcd_puts()
> 
>  common/lcd.c  |   46 +++++++++++++++++++++++++++++++++++++++-------
>  include/lcd.h |    8 ++++++++
>  2 files changed, 47 insertions(+), 7 deletions(-)
> 
> diff --git a/common/lcd.c b/common/lcd.c
> index 18525a7..f7514a4 100644
> --- a/common/lcd.c
> +++ b/common/lcd.c
> @@ -97,6 +97,9 @@ static void lcd_setbgcolor (int color);
>  
>  char lcd_is_enabled = 0;
>  
> +static char lcd_flush_dcache;	/* 1 to flush dcache after each
> lcd update */ +
> +
>  #ifdef	NOT_USED_SO_FAR
>  static void lcd_getcolreg (ushort regno,
>  				ushort *red, ushort *green, ushort
> *blue); @@ -105,6 +108,28 @@ static int lcd_getfgcolor (void);
>  
>  /************************************************************************/
>  
> +/* Flush LCD activity to the caches */
> +void lcd_sync(void)
> +{
> +	/*
> +	 * flush_dcache_range() is declared in common.h but it seems
> that some
> +	 * architectures do not actually implement it. Is there a
> way to find
> +	 * out whether it exists? For now, ARM is safe.
> +	 */
> +#ifdef CONFIG_ARM
> +	int line_length;
> +
> +	if (lcd_flush_dcache)
> +		flush_dcache_range((u32)lcd_base,
> +			(u32)(lcd_base +
> lcd_get_size(&line_length))); +#endif

I'm struggling with a similar problem - but not in console putc, but
at lcd_display_bitmap().

The solution (in mine case) is:
flush_dcache_range((unsigned long) fb,
		   (unsigned long) fb +
		   (lcd_line_length * height));
which takes the "real" image range (as it is defined by fb).

Flushing the lcd_base based range is a bit overkill for me.

> +}
> +
> +void lcd_set_flush_dcache(int flush)
> +{
> +	lcd_flush_dcache = (flush != 0);
> +}
> +

I'm wondering if this flush_dcache_range cannot be added directly to
relevant places in the code?

flush_dcache_* calls are either defined (for a relevant - cache aware
archs) or are dummy.

>  /*----------------------------------------------------------------------*/
>  
>  static void console_scrollup (void)
> @@ -114,6 +139,7 @@ static void console_scrollup (void)
>  
>  	/* Clear the last one */
>  	memset (CONSOLE_ROW_LAST, COLOR_MASK(lcd_color_bg),
> CONSOLE_ROW_SIZE);
> +	lcd_sync();
>  }
>  
>  /*----------------------------------------------------------------------*/
> @@ -144,6 +170,8 @@ static inline void console_newline (void)
>  		/* Scroll everything up */
>  		console_scrollup () ;
>  		--console_row;
> +	} else {
> +		lcd_sync();
>  	}
>  }
>  
> @@ -198,6 +226,7 @@ void lcd_puts (const char *s)
>  	while (*s) {
>  		lcd_putc (*s++);
>  	}
> +	lcd_sync();
>  }
>  
>  /*----------------------------------------------------------------------*/
> @@ -365,13 +394,6 @@ int drv_lcd_init (void)
>  }
>  
>  /*----------------------------------------------------------------------*/
> -static
> -int do_lcd_clear(cmd_tbl_t *cmdtp, int flag, int argc, char *const
> argv[]) -{
> -	lcd_clear();
> -	return 0;
> -}
> -
>  void lcd_clear(void)
>  {
>  #if LCD_BPP == LCD_MONOCHROME
> @@ -413,6 +435,14 @@ void lcd_clear(void)
>  
>  	console_col = 0;
>  	console_row = 0;
> +	lcd_sync();
> +}
> +
> +static int do_lcd_clear(cmd_tbl_t *cmdtp, int flag, int argc,
> +			char *const argv[])
> +{
> +	lcd_clear();
> +	return 0;
>  }
>  
>  U_BOOT_CMD(
> @@ -607,6 +637,7 @@ void bitmap_plot (int x, int y)
>  	}
>  
>  	WATCHDOG_RESET();
> +	lcd_sync();
>  }
>  #else
>  static inline void bitmap_plot(int x, int y) {}
> @@ -820,6 +851,7 @@ int lcd_display_bitmap(ulong bmp_image, int x,
> int y) break;
>  	};
>  
> +	lcd_sync();
>  	return (0);
>  }
>  #endif
> diff --git a/include/lcd.h b/include/lcd.h
> index 26f6d83..4363131 100644
> --- a/include/lcd.h
> +++ b/include/lcd.h
> @@ -57,6 +57,14 @@ extern void lcd_initcolregs (void);
>  extern struct bmp_image *gunzip_bmp(unsigned long addr, unsigned
> long *lenp); extern int bmp_display(ulong addr, int x, int y);
>  
> +/**
> + * Set whether we need to flush the dcache when changing the LCD
> image. This
> + * defaults to off.
> + *
> + * @param flush		non-zero to flush cache after update,
> 0 to skip
> + */
> +void lcd_set_flush_dcache(int flush);
> +
>  #if defined CONFIG_MPC823
>  /*
>   * LCD controller stucture for MPC823 CPU

Anyway, I'm looking forward for v4 version of this patch.

-- 
Best regards,

Lukasz Majewski

Samsung Poland R&D Center | Linux Platform Group