[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

[Anatolij Gustschin]

Data cache flushing is required for frame buffer in RAM to fix the
distorted console text output. Currently this text distortion is
observed with cfb on beageboard and N900 when running with data
cache enabled.

Reported-by: Pali Roh?r <pali.rohar@gmail.com>
Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
 drivers/video/cfb_console.c |   35 +++++++++++++++++++++++++++++++++++
 1 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/drivers/video/cfb_console.c b/drivers/video/cfb_console.c
index 904caf7..dd7ccb7 100644
--- a/drivers/video/cfb_console.c
+++ b/drivers/video/cfb_console.c
@@ -360,6 +360,8 @@ void console_cursor(int state);
 extern void video_get_info_str(int line_number,	char *info);
 #endif
 
+DECLARE_GLOBAL_DATA_PTR;
+
 /* Locals */
 static GraphicDevice *pGD;	/* Pointer to Graphic array */
 
@@ -377,6 +379,8 @@ static int console_row;		/* cursor row */
 
 static u32 eorx, fgx, bgx;	/* color pats */
 
+static int cfb_do_flush_cache;
+
 static const int video_font_draw_table8[] = {
 	0x00000000, 0x000000ff, 0x0000ff00, 0x0000ffff,
 	0x00ff0000, 0x00ff00ff, 0x00ffff00, 0x00ffffff,
@@ -553,6 +557,8 @@ 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++;
 		}
@@ -621,6 +627,8 @@ 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);
 		}
 	}
 }
@@ -717,6 +725,8 @@ static void console_scrollup(void)
 #else
 	memsetl(CONSOLE_ROW_LAST, CONSOLE_ROW_SIZE >> 2, CONSOLE_BG_COL);
 #endif
+	if (cfb_do_flush_cache)
+		flush_cache((ulong)CONSOLE_ROW_FIRST, CONSOLE_SIZE);
 }
 
 static void console_back(void)
@@ -1651,6 +1661,29 @@ static void *video_logo(void)
 }
 #endif
 
+static int cfb_fb_is_in_dram(void)
+{
+	bd_t *bd = gd->bd;
+	ulong start, end;
+	int i;
+
+	for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) {
+#if defined(CONFIG_ARM) || defined(CONFIG_AVR32) || defined(COFNIG_NDS32) || \
+defined(CONFIG_SANDBOX) || defined(CONFIG_X86)
+		start = bd->bi_dram[i].start;
+		end = bd->bi_dram[i].start + bd->bi_dram[i].size - 1;
+#else
+		start = bd->bi_memstart;
+		end = bd->bi_memsize;
+#endif
+
+		if ((ulong)video_fb_address >= start &&
+		    (ulong)video_fb_address < end)
+			return 1;
+	}
+	return 0;
+}
+
 static int video_init(void)
 {
 	unsigned char color8;
@@ -1664,6 +1697,8 @@ static int video_init(void)
 	video_init_hw_cursor(VIDEO_FONT_WIDTH, VIDEO_FONT_HEIGHT);
 #endif
 
+	cfb_do_flush_cache = cfb_fb_is_in_dram() && dcache_status();
+
 	/* Init drawing pats */
 	switch (VIDEO_DATA_FORMAT) {
 	case GDF__8BIT_INDEX:
-- 
1.7.1

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

[Pali Rohár]

On Saturday 28 April 2012 17:04:07 you wrote:
> Data cache flushing is required for frame buffer in RAM to fix
> the distorted console text output. Currently this text
> distortion is observed with cfb on beageboard and N900 when
> running with data cache enabled.
> 
> Reported-by: Pali Roh?r <pali.rohar@gmail.com>
> Signed-off-by: Anatolij Gustschin <agust@denx.de>

Tested-by: Pali Roh?r <pali.rohar@gmail.com>

-- 
Pali Roh?r
pali.rohar at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120428/488c868f/attachment.pgp>

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

[Mike Frysinger]

On Saturday 28 April 2012 11:04:07 Anatolij Gustschin wrote:
> +static int cfb_fb_is_in_dram(void)
> +{
> +	bd_t *bd = gd->bd;
> +	ulong start, end;
> +	int i;
> +
> +	for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) {
> +#if defined(CONFIG_ARM) || defined(CONFIG_AVR32) || defined(COFNIG_NDS32)
> || \ +defined(CONFIG_SANDBOX) || defined(CONFIG_X86)
> +		start = bd->bi_dram[i].start;
> +		end = bd->bi_dram[i].start + bd->bi_dram[i].size - 1;
> +#else
> +		start = bd->bi_memstart;
> +		end = bd->bi_memsize;
> +#endif
> +
> +		if ((ulong)video_fb_address >= start &&
> +		    (ulong)video_fb_address < end)
> +			return 1;
> +	}
> +	return 0;
> +}

is this necessary ?  the cache funcs should take care of this automatically.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120428/455ca8c1/attachment.pgp>

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

[Anatolij Gustschin]

Hi Mike,

On Sat, 28 Apr 2012 14:16:39 -0400
Mike Frysinger <vapier@gentoo.org> wrote:

> On Saturday 28 April 2012 11:04:07 Anatolij Gustschin wrote:
> > +static int cfb_fb_is_in_dram(void)
> > +{
> > +	bd_t *bd = gd->bd;
> > +	ulong start, end;
> > +	int i;
> > +
> > +	for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) {
> > +#if defined(CONFIG_ARM) || defined(CONFIG_AVR32) || defined(COFNIG_NDS32)
> > || \ +defined(CONFIG_SANDBOX) || defined(CONFIG_X86)
> > +		start = bd->bi_dram[i].start;
> > +		end = bd->bi_dram[i].start + bd->bi_dram[i].size - 1;
> > +#else
> > +		start = bd->bi_memstart;
> > +		end = bd->bi_memsize;
> > +#endif
> > +
> > +		if ((ulong)video_fb_address >= start &&
> > +		    (ulong)video_fb_address < end)
> > +			return 1;
> > +	}
> > +	return 0;
> > +}
> 
> is this necessary ?  the cache funcs should take care of this automatically.

Currently they don't, or at least on some architectures. Or did you mean
that the cache instructions should take care of this?

Thanks,
Anatolij

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

[Mike Frysinger]

On Monday 30 April 2012 11:32:21 Anatolij Gustschin wrote:
> On Sat, 28 Apr 2012 14:16:39 -0400 Mike Frysinger wrote:
> > On Saturday 28 April 2012 11:04:07 Anatolij Gustschin wrote:
> > > +static int cfb_fb_is_in_dram(void)
> > > +{
> > > +	bd_t *bd = gd->bd;
> > > +	ulong start, end;
> > > +	int i;
> > > +
> > > +	for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) {
> > > +#if defined(CONFIG_ARM) || defined(CONFIG_AVR32) ||
> > > defined(COFNIG_NDS32)
> > > 
> > > || \ +defined(CONFIG_SANDBOX) || defined(CONFIG_X86)
> > > 
> > > +		start = bd->bi_dram[i].start;
> > > +		end = bd->bi_dram[i].start + bd->bi_dram[i].size - 1;
> > > +#else
> > > +		start = bd->bi_memstart;
> > > +		end = bd->bi_memsize;
> > > +#endif
> > > +
> > > +		if ((ulong)video_fb_address >= start &&
> > > +		    (ulong)video_fb_address < end)
> > > +			return 1;
> > > +	}
> > > +	return 0;
> > > +}
> > 
> > is this necessary ?  the cache funcs should take care of this
> > automatically.
> 
> Currently they don't, or at least on some architectures. Or did you mean
> that the cache instructions should take care of this?

imo, cache flush functions should be safe to call on any memory address (where 
there is data that could be dma-ed from).  this is how Linux works.

otherwise, this function ends up getting duplicated in many places and i can't 
possibly see how that's an improvement.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120501/ae23042e/attachment.pgp>

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

[Marek Vasut]

Dear Anatolij Gustschin,

> Data cache flushing is required for frame buffer in RAM to fix the
> distorted console text output. Currently this text distortion is
> observed with cfb on beageboard and N900 when running with data
> cache enabled.

beagleboard ;-)

> 
> Reported-by: Pali Roh?r <pali.rohar@gmail.com>
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> ---
>  drivers/video/cfb_console.c |   35 +++++++++++++++++++++++++++++++++++
>  1 files changed, 35 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/video/cfb_console.c b/drivers/video/cfb_console.c
> index 904caf7..dd7ccb7 100644
> --- a/drivers/video/cfb_console.c
> +++ b/drivers/video/cfb_console.c
> @@ -360,6 +360,8 @@ void console_cursor(int state);
>  extern void video_get_info_str(int line_number,	char *info);
>  #endif
> 
> +DECLARE_GLOBAL_DATA_PTR;
> +
>  /* Locals */
>  static GraphicDevice *pGD;	/* Pointer to Graphic array */
> 
> @@ -377,6 +379,8 @@ static int console_row;		/* cursor row */
> 
>  static u32 eorx, fgx, bgx;	/* color pats */
> 
> +static int cfb_do_flush_cache;
> +
>  static const int video_font_draw_table8[] = {
>  	0x00000000, 0x000000ff, 0x0000ff00, 0x0000ffff,
>  	0x00ff0000, 0x00ff00ff, 0x00ffff00, 0x00ffffff,
> @@ -553,6 +557,8 @@ 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);

flush_dcache_range() ?

>  			dest0 += VIDEO_FONT_WIDTH * VIDEO_PIXEL_SIZE;
>  			s++;
>  		}
> @@ -621,6 +627,8 @@ 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);

DTTO

>  		}
>  	}
>  }
> @@ -717,6 +725,8 @@ static void console_scrollup(void)
>  #else
>  	memsetl(CONSOLE_ROW_LAST, CONSOLE_ROW_SIZE >> 2, CONSOLE_BG_COL);
>  #endif
> +	if (cfb_do_flush_cache)
> +		flush_cache((ulong)CONSOLE_ROW_FIRST, CONSOLE_SIZE);
>  }
> 
>  static void console_back(void)
> @@ -1651,6 +1661,29 @@ static void *video_logo(void)
>  }
>  #endif
> 
> +static int cfb_fb_is_in_dram(void)
> +{
> +	bd_t *bd = gd->bd;
> +	ulong start, end;
> +	int i;
> +
> +	for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) {
> +#if defined(CONFIG_ARM) || defined(CONFIG_AVR32) || defined(COFNIG_NDS32)
> || \ +defined(CONFIG_SANDBOX) || defined(CONFIG_X86)
> +		start = bd->bi_dram[i].start;
> +		end = bd->bi_dram[i].start + bd->bi_dram[i].size - 1;
> +#else
> +		start = bd->bi_memstart;
> +		end = bd->bi_memsize;
> +#endif
> +
> +		if ((ulong)video_fb_address >= start &&
> +		    (ulong)video_fb_address < end)
> +			return 1;
> +	}
> +	return 0;
> +}

Can't you have SRAM cached too? ;-)

> +
>  static int video_init(void)
>  {
>  	unsigned char color8;
> @@ -1664,6 +1697,8 @@ static int video_init(void)
>  	video_init_hw_cursor(VIDEO_FONT_WIDTH, VIDEO_FONT_HEIGHT);
>  #endif
> 
> +	cfb_do_flush_cache = cfb_fb_is_in_dram() && dcache_status();
> +
>  	/* Init drawing pats */
>  	switch (VIDEO_DATA_FORMAT) {
>  	case GDF__8BIT_INDEX:

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

[Simon Glass]

Hi,

On Sun, Apr 29, 2012 at 7:25 PM, Marek Vasut <marex@denx.de> wrote:

> Dear Anatolij Gustschin,
>
> > Data cache flushing is required for frame buffer in RAM to fix the
> > distorted console text output. Currently this text distortion is
> > observed with cfb on beageboard and N900 when running with data
> > cache enabled.
>
> beagleboard ;-)
>

Regarding this patch, can I suggest also looking at this possibly more
generic patch that I originally did for Tegra? The series is here:

http://patchwork.ozlabs.org/user/bundle/2869/

and in particular these patches from the series:

arm: Add control over cachability of memory regions
lcd: Add CONFIG_ALIGN_LCD_TO_SECTION to align lcd for MMU
lcd: Add support for flushing LCD fb from dcache after update
tegra: Align LCD frame buffer to section boundary
tegra: Support control of cache settings for LCD
tegra: fdt: Add LCD definitions for Seaboard
lcd: Add CONSOLE_SCROLL_LINES option to speed console

Regards,
Simon


>
> >
> > Reported-by: Pali Roh?r <pali.rohar@gmail.com>
> > Signed-off-by: Anatolij Gustschin <agust@denx.de>
> > ---
> >  drivers/video/cfb_console.c |   35 +++++++++++++++++++++++++++++++++++
> >  1 files changed, 35 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/video/cfb_console.c b/drivers/video/cfb_console.c
> > index 904caf7..dd7ccb7 100644
> > --- a/drivers/video/cfb_console.c
> > +++ b/drivers/video/cfb_console.c
> > @@ -360,6 +360,8 @@ void console_cursor(int state);
> >  extern void video_get_info_str(int line_number,      char *info);
> >  #endif
> >
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> >  /* Locals */
> >  static GraphicDevice *pGD;   /* Pointer to Graphic array */
> >
> > @@ -377,6 +379,8 @@ static int console_row;           /* cursor row */
> >
> >  static u32 eorx, fgx, bgx;   /* color pats */
> >
> > +static int cfb_do_flush_cache;
> > +
> >  static const int video_font_draw_table8[] = {
> >       0x00000000, 0x000000ff, 0x0000ff00, 0x0000ffff,
> >       0x00ff0000, 0x00ff00ff, 0x00ffff00, 0x00ffffff,
> > @@ -553,6 +557,8 @@ 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);
>
> flush_dcache_range() ?
>
> >                       dest0 += VIDEO_FONT_WIDTH * VIDEO_PIXEL_SIZE;
> >                       s++;
> >               }
> > @@ -621,6 +627,8 @@ 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);
>
> DTTO
>
> >               }
> >       }
> >  }
> > @@ -717,6 +725,8 @@ static void console_scrollup(void)
> >  #else
> >       memsetl(CONSOLE_ROW_LAST, CONSOLE_ROW_SIZE >> 2, CONSOLE_BG_COL);
> >  #endif
> > +     if (cfb_do_flush_cache)
> > +             flush_cache((ulong)CONSOLE_ROW_FIRST, CONSOLE_SIZE);
> >  }
> >
> >  static void console_back(void)
> > @@ -1651,6 +1661,29 @@ static void *video_logo(void)
> >  }
> >  #endif
> >
> > +static int cfb_fb_is_in_dram(void)
> > +{
> > +     bd_t *bd = gd->bd;
> > +     ulong start, end;
> > +     int i;
> > +
> > +     for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) {
> > +#if defined(CONFIG_ARM) || defined(CONFIG_AVR32) ||
> defined(COFNIG_NDS32)
> > || \ +defined(CONFIG_SANDBOX) || defined(CONFIG_X86)
> > +             start = bd->bi_dram[i].start;
> > +             end = bd->bi_dram[i].start + bd->bi_dram[i].size - 1;
> > +#else
> > +             start = bd->bi_memstart;
> > +             end = bd->bi_memsize;
> > +#endif
> > +
> > +             if ((ulong)video_fb_address >= start &&
> > +                 (ulong)video_fb_address < end)
> > +                     return 1;
> > +     }
> > +     return 0;
> > +}
>
> Can't you have SRAM cached too? ;-)
>
> > +
> >  static int video_init(void)
> >  {
> >       unsigned char color8;
> > @@ -1664,6 +1697,8 @@ static int video_init(void)
> >       video_init_hw_cursor(VIDEO_FONT_WIDTH, VIDEO_FONT_HEIGHT);
> >  #endif
> >
> > +     cfb_do_flush_cache = cfb_fb_is_in_dram() && dcache_status();
> > +
> >       /* Init drawing pats */
> >       switch (VIDEO_DATA_FORMAT) {
> >       case GDF__8BIT_INDEX:
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

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

[Anatolij Gustschin]

Hi Simon,

On Sun, 29 Apr 2012 22:56:01 -0700
Simon Glass <sjg@chromium.org> wrote:
...
> Regarding this patch, can I suggest also looking at this possibly more
> generic patch that I originally did for Tegra? The series is here:
>
> http://patchwork.ozlabs.org/user/bundle/2869/
> 
> and in particular these patches from the series:
> 
> arm: Add control over cachability of memory regions
> lcd: Add CONFIG_ALIGN_LCD_TO_SECTION to align lcd for MMU
> lcd: Add support for flushing LCD fb from dcache after update
> tegra: Align LCD frame buffer to section boundary
> tegra: Support control of cache settings for LCD
> tegra: fdt: Add LCD definitions for Seaboard
> lcd: Add CONSOLE_SCROLL_LINES option to speed console

Thanks for the reminder, I completely forgot about these LCD patches.
I'll look at them soon.

Thanks,
Anatolij

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

[Anatolij Gustschin]

Hi,

On Mon, 30 Apr 2012 04:25:50 +0200
Marek Vasut <marex@denx.de> wrote:
...
> > observed with cfb on beageboard and N900 when running with data
> > cache enabled.
> 
> beagleboard ;-)

Thanks for catching that!!

...
> > @@ -553,6 +557,8 @@ 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);
> 
> flush_dcache_range() ?

I would have to calculate the end address, then. flush_cache() already
does it for me :-)

...
> > @@ -1651,6 +1661,29 @@ static void *video_logo(void)
> >  }
> >  #endif
> > 
> > +static int cfb_fb_is_in_dram(void)
> > +{
> > +	bd_t *bd = gd->bd;
> > +	ulong start, end;
> > +	int i;
> > +
> > +	for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) {
> > +#if defined(CONFIG_ARM) || defined(CONFIG_AVR32) || defined(COFNIG_NDS32)
> > || \ +defined(CONFIG_SANDBOX) || defined(CONFIG_X86)
> > +		start = bd->bi_dram[i].start;
> > +		end = bd->bi_dram[i].start + bd->bi_dram[i].size - 1;
> > +#else
> > +		start = bd->bi_memstart;
> > +		end = bd->bi_memsize;
> > +#endif
> > +
> > +		if ((ulong)video_fb_address >= start &&
> > +		    (ulong)video_fb_address < end)
> > +			return 1;
> > +	}
> > +	return 0;
> > +}
> 
> Can't you have SRAM cached too? ;-)

I do not know. But who will put the framebuffer into SRAM?
It is not big enough.

Thanks,
Anatolij

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

[Marek Vasut]

Dear Anatolij Gustschin,

> Hi,
> 
> On Mon, 30 Apr 2012 04:25:50 +0200
> Marek Vasut <marex@denx.de> wrote:
> ...
> 
> > > observed with cfb on beageboard and N900 when running with data
> > > cache enabled.
> > 
> > beagleboard ;-)
> 
> Thanks for catching that!!
> 
> ...
> 
> > > @@ -553,6 +557,8 @@ 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);
> > 
> > flush_dcache_range() ?
> 
> I would have to calculate the end address, then. flush_cache() already
> does it for me :-)

Well ... that's correct. Maybe someone should rename it to flush_dcache_size() 
or something ...

> 
> ...
> 
> > > @@ -1651,6 +1661,29 @@ static void *video_logo(void)
> > > 
> > >  }
> > >  #endif
> > > 
> > > +static int cfb_fb_is_in_dram(void)
> > > +{
> > > +	bd_t *bd = gd->bd;
> > > +	ulong start, end;
> > > +	int i;
> > > +
> > > +	for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) {
> > > +#if defined(CONFIG_ARM) || defined(CONFIG_AVR32) ||
> > > defined(COFNIG_NDS32)
> > > 
> > > || \ +defined(CONFIG_SANDBOX) || defined(CONFIG_X86)
> > > 
> > > +		start = bd->bi_dram[i].start;
> > > +		end = bd->bi_dram[i].start + bd->bi_dram[i].size - 1;
> > > +#else
> > > +		start = bd->bi_memstart;
> > > +		end = bd->bi_memsize;
> > > +#endif
> > > +
> > > +		if ((ulong)video_fb_address >= start &&
> > > +		    (ulong)video_fb_address < end)
> > > +			return 1;
> > > +	}
> > > +	return 0;
> > > +}
> > 
> > Can't you have SRAM cached too? ;-)
> 
> I do not know. But who will put the framebuffer into SRAM?
> It is not big enough.

Someone who has small LCD and wants to save dram bandwidth (oh, this sentence 
sounds stupid on its own). But maybe if you want to use LCD in SPL?
> 
> Thanks,
> Anatolij

Best regards,
Marek Vasut

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

[Anatolij Gustschin]

Hi,

On Mon, 30 Apr 2012 17:21:51 +0200
Marek Vasut <marex@denx.de> wrote:
...
> > > Can't you have SRAM cached too? ;-)
> > 
> > I do not know. But who will put the framebuffer into SRAM?
> > It is not big enough.
> 
> Someone who has small LCD and wants to save dram bandwidth (oh, this sentence 
> sounds stupid on its own). But maybe if you want to use LCD in SPL?

If someone really needs it, he can always add it later :-)

Thanks,
Anatolij

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

[Marek Vasut]

Dear Anatolij Gustschin,

> Hi,
> 
> On Mon, 30 Apr 2012 17:21:51 +0200
> Marek Vasut <marex@denx.de> wrote:
> ...
> 
> > > > Can't you have SRAM cached too? ;-)
> > > 
> > > I do not know. But who will put the framebuffer into SRAM?
> > > It is not big enough.
> > 
> > Someone who has small LCD and wants to save dram bandwidth (oh, this
> > sentence sounds stupid on its own). But maybe if you want to use LCD in
> > SPL?
> 
> If someone really needs it, he can always add it later :-)

I won't argue further :-)

> Thanks,
> Anatolij

Best regards,
Marek Vasut

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

[Anatolij Gustschin]

On Sat, 28 Apr 2012 17:04:07 +0200
Anatolij Gustschin <agust@denx.de> wrote:

> Data cache flushing is required for frame buffer in RAM to fix the
> distorted console text output. Currently this text distortion is
> observed with cfb on beageboard and N900 when running with data
> cache enabled.
> 
> Reported-by: Pali Roh?r <pali.rohar@gmail.com>
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> ---
>  drivers/video/cfb_console.c |   35 +++++++++++++++++++++++++++++++++++
>  1 files changed, 35 insertions(+), 0 deletions(-)

Applied to u-boot-video/master after rebasing and fixing commit log.

Anatolij

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

[Mike Frysinger]

On Tuesday 05 June 2012 03:28:40 Anatolij Gustschin wrote:
> On Sat, 28 Apr 2012 17:04:07 +0200 Anatolij Gustschin wrote:
> > Data cache flushing is required for frame buffer in RAM to fix the
> > distorted console text output. Currently this text distortion is
> > observed with cfb on beageboard and N900 when running with data
> > cache enabled.
> 
> Applied to u-boot-video/master after rebasing and fixing commit log.

this patch is still wrong for the reasons i cited earlier.  your cache 
functions are broken, not the driver.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120719/a4f51a6b/attachment.pgp>

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

[Marek Vasut]

Dear Mike Frysinger,

> On Tuesday 05 June 2012 03:28:40 Anatolij Gustschin wrote:
> > On Sat, 28 Apr 2012 17:04:07 +0200 Anatolij Gustschin wrote:
> > > Data cache flushing is required for frame buffer in RAM to fix the
> > > distorted console text output. Currently this text distortion is
> > > observed with cfb on beageboard and N900 when running with data
> > > cache enabled.
> > 
> > Applied to u-boot-video/master after rebasing and fixing commit log.
> 
> this patch is still wrong for the reasons i cited earlier.  your cache
> functions are broken, not the driver.

The cache stuff is completely borked on ARM, I have no idea why people enable 
it.

Best regards,
Marek Vasut

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

[Mike Frysinger]

On Thursday 19 July 2012 11:49:20 Marek Vasut wrote:
> Dear Mike Frysinger,
> > On Tuesday 05 June 2012 03:28:40 Anatolij Gustschin wrote:
> > > On Sat, 28 Apr 2012 17:04:07 +0200 Anatolij Gustschin wrote:
> > > > Data cache flushing is required for frame buffer in RAM to fix the
> > > > distorted console text output. Currently this text distortion is
> > > > observed with cfb on beageboard and N900 when running with data
> > > > cache enabled.
> > > 
> > > Applied to u-boot-video/master after rebasing and fixing commit log.
> > 
> > this patch is still wrong for the reasons i cited earlier.  your cache
> > functions are broken, not the driver.
> 
> The cache stuff is completely borked on ARM, I have no idea why people
> enable it.

in this case, it's making it worse for other arches, and setting bad 
precedents in the process

i think Simon's solution is a good middle ground:
	lcd: Add support for flushing LCD fb from dcache after update
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120719/4e88e505/attachment.pgp>

[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