[U-Boot] [PATCH RESEND] video:cache:fix: Buffer alignment and dcache flush for lcd subsystem

[Simon Glass]

Hi Lukasz,

On Wed, Jan 2, 2013 at 8:25 AM, Lukasz Majewski <l.majewski at samsung.com> wrote:
> This commit makes the video subsystem code cache aware.
> Memory allocated for decompressed BMP memory is now cache line aligned.
>
> Flushing of the dcache is also performed after copying BMP data to fb
> address (it is done for 32 BPP bitmap used on Trats board (Exynos4210)).
>

Sorry if I have this wrong, just have a few comments.

>
> Tested-by: Lukasz Majewski <l.majewski at samsung.com>
> Signed-off-by: Lukasz Majewski <l.majewski at samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park at samsung.com>
> Cc: Anatolij Gustschin <agust at denx.de>
> ---
>  common/cmd_bmp.c |    2 +-
>  common/lcd.c     |    3 +++
>  2 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/common/cmd_bmp.c b/common/cmd_bmp.c
> index 5a52edd..57f3eb5 100644
> --- a/common/cmd_bmp.c
> +++ b/common/cmd_bmp.c
> @@ -55,7 +55,7 @@ bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp)
>          * Decompress bmp image
>          */
>         len = CONFIG_SYS_VIDEO_LOGO_MAX_SIZE;
> -       dst = malloc(CONFIG_SYS_VIDEO_LOGO_MAX_SIZE);
> +       dst = memalign(CONFIG_SYS_CACHELINE_SIZE, len);

Why do you need to align this one? It is just returned to the caller, isn't it?

>         if (dst == NULL) {
>                 puts("Error: malloc in gunzip failed!\n");
>                 return NULL;
> diff --git a/common/lcd.c b/common/lcd.c
> index 4778655..784d1fb 100644
> --- a/common/lcd.c
> +++ b/common/lcd.c
> @@ -1023,6 +1023,9 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
>                         }
>                         fb  -= (lcd_line_length + width * (bpix / 8));
>                 }
> +               flush_dcache_range((unsigned long) fb,
> +                                  (unsigned long) fb +
> +                                  (lcd_line_length * height));

I'm not sure this is needed - there is a call to lcd_sync() at the
bottom of this function now which should do the same thing,

Please can you take a look at currently mainline and see if you need
to do anything more?

>                 break;
>  #endif /* CONFIG_BMP_32BPP */
>         default:
> --
> 1.7.2.3
>

Regards,
Simon