On 10/17/2012 03:38 AM, Lukasz Majewski wrote:
Hi Simon,
Hi,
On Thu, Aug 9, 2012 at 12:43 AM, Lukasz Majewski
<l.majew...@samsung.com> wrote:
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<s...@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.
Sorry I don't think I replied to this.
Yes, this is still open issue. (not fixed I mean) I maintain the patch
locally.
Certainly we could make the flushing more fine grained. My reasons for
not (so far) are:
1. It is simpler to flush everything (no need to figure out what part
of display has changed)
2. The performance difference is likely to be small since flushing an
already-flushed range should be fast.
From the sake of "SW engineering" I would opt for fine grained
flushing. But if it turns out, that it costs too much, we can flush
everything.
Is anybody else in a position to get some numbers about how/if
performance is better when flushing at the more granular level?
Before deciding it wasn't worth the code, I implemented granular
flushing and didn't see any noticeable degradation when just
flushing at the end of all public functions as in this patch.
http://lists.denx.de/pipermail/u-boot/2012-September/134979.html
It seems that performance is the only reason for fine-grained
cache flush operations
Certainly we could enhance this code. I wonder though whether a
generic flushing mechanism may need to be added to support LCD and
also video drivers.
We can add a generic mechanism to LCD and video.
Simon, do you plan to post some code in a near future? Or we are now
just "gathering requirements"?
Regards,
Simon
--
Best regards,
Lukasz Majewski
Samsung Poland R&D Center | Linux Platform Group
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
