Hi Simon,
> Hi Lukasz,
>
> On Wed, Jan 2, 2013 at 8:25 AM, Lukasz Majewski
> <l.majew...@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.majew...@samsung.com>
> > Signed-off-by: Lukasz Majewski <l.majew...@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.p...@samsung.com>
> > Cc: Anatolij Gustschin <ag...@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?
Yes, it is returned to the caller, but afterwards lcd_display_bitmap
takes this pointer (and thereof probably unaligned buffer) and works on
it. (At least in the case of trats the bmp is gzipped).
>
> > 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?
Ok, I've checked. You are right, there is lcd_sync. Thanks for pointing.
I will force trats board to use it, and prepare patches.
>
> > break;
> > #endif /* CONFIG_BMP_32BPP */
> > default:
> > --
> > 1.7.2.3
> >
>
> Regards,
> Simon
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
