[U-Boot] [PATCH] video: cfb_console: flush dcache for frame buffer in DRAM

[U-Boot] [PATCH] video: cfb_console: flush dcache for frame buffer in DRAM

[Eric Nelson]

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

[U-Boot] [PATCH] cfb_console: flush FB cache at end of public functions

[Eric Nelson]

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;
 }
-- 
1.7.9

[U-Boot] [PATCH] cfb_console: flush FB cache at end of public functions

[Anatolij Gustschin]

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

[U-Boot] [PATCH] cfb_console: flush FB cache at end of public functions

[Eric Nelson]

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.