Re: [U-Boot] [PATCH] video: cfb_console: flush dcache for frame buffer in DRAM
Hi Anatolij,
I hate to come in late to the discussion, but while testing [1] with cache enabled, I started down the path of fixing up code paths within drivers/video/cfb_console.c that are followed by SABRE Lite as you did in [2].
[1] [PATCH V2 0/2] i.MX6: mx6qsabrelite: Add splash screen support http://lists.denx.de/pipermail/u-boot/2012-September/134241.html [2] [PATCH] video: cfb_console: flush dcache for frame buffer in DRAM http://lists.denx.de/pipermail/u-boot/2012-April/123333.html
That code essentially looks like this:
void video_drawchars(...) { ... switch (VIDEO_DATA_FORMAT) { case GDF__8BIT_INDEX: case GDF__8BIT_332RGB: case GDF_15BIT_555RGB: case GDF_16BIT_565RGB: ... cache not flushed in these cases
case GDF_32BIT_X888RGB: ... cache is flushed here }
SABRE Lite (ipu3) code happens to use the RGB565 frame buffer format, so it needs additional fixup.
Continuing this pattern seems to be a mistake though. There are a lot of cases, with all sorts of different alignments (most of which won't sync with cache line boundaries) and it seems that there's little to no benefit in the fine granularity. It seems that by the time we fix up all of the individual spots where we're writing to a line in the frame buffer, the cache can never get dirty in the first place.
It seems much more sensible to simply use flush_dcache_range() on the entire frame buffer in each of the public calls and let the MMU figure out what needs flushing.
Is there something I'm missing?
My other immediate thought was that we should really place the frame buffer in un-cacheable memory, but then we really will be doing more memory accesses.
Please advise,
Eric
Removed internal cache_flush operations and placed a flush of the entire frame-buffer at the end of each public function.
Changed logo_plot() to static because it isn't called externally or declared in a header.
Signed-off-by: Eric Nelson eric.nelson@boundarydevices.com
--- drivers/video/cfb_console.c | 16 +++++++++------- 1 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/video/cfb_console.c b/drivers/video/cfb_console.c index 19d061f..1372353 100644 --- a/drivers/video/cfb_console.c +++ b/drivers/video/cfb_console.c @@ -557,8 +557,6 @@ static void video_drawchars(int xx, int yy, unsigned char *s, int count) SWAP32((video_font_draw_table32 [bits & 15][3] & eorx) ^ bgx); } - if (cfb_do_flush_cache) - flush_cache((ulong)dest0, 32); dest0 += VIDEO_FONT_WIDTH * VIDEO_PIXEL_SIZE; s++; } @@ -627,8 +625,6 @@ static void video_invertchar(int xx, int yy) for (x = firstx; x < lastx; x++) { u8 *dest = (u8 *)(video_fb_address) + x + y; *dest = ~*dest; - if (cfb_do_flush_cache) - flush_cache((ulong)dest, 4); } } } @@ -672,6 +668,8 @@ void console_cursor(int state) } cursor_state = state; } + if (cfb_do_flush_cache) + flush_cache(VIDEO_FB_ADRS, VIDEO_SIZE); } #endif
@@ -724,8 +722,6 @@ static void console_clear_line(int line, int begin, int end) memsetl(offset + i * VIDEO_LINE_LEN, size, bgx); } #endif - if (cfb_do_flush_cache) - flush_cache((ulong)CONSOLE_ROW_FIRST, CONSOLE_SIZE); }
static void console_scrollup(void) @@ -828,6 +824,8 @@ void video_putc(const char c) } } CURSOR_SET; + if (cfb_do_flush_cache) + flush_cache(VIDEO_FB_ADRS, VIDEO_SIZE); }
void video_puts(const char *s) @@ -1474,13 +1472,15 @@ int video_display_bitmap(ulong bmp_image, int x, int y) } #endif
+ if (cfb_do_flush_cache) + flush_cache(VIDEO_FB_ADRS, VIDEO_SIZE); return (0); } #endif
#ifdef CONFIG_VIDEO_LOGO -void logo_plot(void *screen, int width, int x, int y) +static void logo_plot(void *screen, int width, int x, int y) {
int xcount, i; @@ -1862,6 +1862,8 @@ int drv_video_init(void) if (stdio_register(&console_dev) != 0) return 0;
+ if (cfb_do_flush_cache) + flush_cache(VIDEO_FB_ADRS, VIDEO_SIZE); /* Return success */ return 1; }
On Sat, 22 Sep 2012 16:29:38 -0700 Eric Nelson eric.nelson@boundarydevices.com wrote:
Removed internal cache_flush operations and placed a flush of the entire frame-buffer at the end of each public function.
Changed logo_plot() to static because it isn't called externally or declared in a header.
Signed-off-by: Eric Nelson eric.nelson@boundarydevices.com
drivers/video/cfb_console.c | 16 +++++++++------- 1 files changed, 9 insertions(+), 7 deletions(-)
Applied to u-boot-video/master after rebasing and moving the cache flush from drv_video_init() to video_init().
Thanks,
Anatolij
On 05/06/2013 06:02 AM, Anatolij Gustschin wrote:
On Sat, 22 Sep 2012 16:29:38 -0700 Eric Nelson eric.nelson@boundarydevices.com wrote:
Removed internal cache_flush operations and placed a flush of the entire frame-buffer at the end of each public function.
Changed logo_plot() to static because it isn't called externally or declared in a header.
Signed-off-by: Eric Nelson eric.nelson@boundarydevices.com
drivers/video/cfb_console.c | 16 +++++++++------- 1 files changed, 9 insertions(+), 7 deletions(-)
Applied to u-boot-video/master after rebasing and moving the cache flush from drv_video_init() to video_init().
Thanks Anatolij.
participants (2)
-
Anatolij Gustschin -
Eric Nelson