Patchwork [U-Boot,1/2] video:cache:fix: Proper buffer alignment for lcd subsystem

login
register
mail settings
Submitter Łukasz Majewski
Date Jan. 7, 2013, 9:23 a.m.
Message ID <1357550590-4652-1-git-send-email-l.majewski@samsung.com>
Download mbox | patch
Permalink /patch/209884/
State Rejected
Delegated to: Anatolij Gustschin
Headers show

Comments

Łukasz Majewski - Jan. 7, 2013, 9:23 a.m.
This commit makes the video subsystem code cache aware.
Memory allocated for decompressed BMP memory is now cache line aligned.

Tested-by: Lukasz Majewski <l.majewski@samsung.com>
Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Anatolij Gustschin <agust@denx.de>
---
 common/cmd_bmp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Simon Glass - Jan. 8, 2013, 1:07 a.m.
Hi Lukasz,

On Mon, Jan 7, 2013 at 1:23 AM, Lukasz Majewski <l.majewski@samsung.com> wrote:
> This commit makes the video subsystem code cache aware.
> Memory allocated for decompressed BMP memory is now cache line aligned.
>
> Tested-by: Lukasz Majewski <l.majewski@samsung.com>
> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Anatolij Gustschin <agust@denx.de>
> ---
>  common/cmd_bmp.c |    2 +-
>  1 files changed, 1 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);
>         if (dst == NULL) {
>                 puts("Error: malloc in gunzip failed!\n");
>                 return NULL;

Sorry, I still have a question. Does this 'dst' address get used as
the actual LCD frame buffer on your board, or is it just copied to the
frame buffer?

Regards,
Simon

> --
> 1.7.2.3
>
Łukasz Majewski - Jan. 8, 2013, 8:28 a.m.
Hi Simon,

> Hi Lukasz,
> 
> On Mon, Jan 7, 2013 at 1:23 AM, Lukasz Majewski
> <l.majewski@samsung.com> wrote:
> > This commit makes the video subsystem code cache aware.
> > Memory allocated for decompressed BMP memory is now cache line
> > aligned.
> >
> > Tested-by: Lukasz Majewski <l.majewski@samsung.com>
> > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > Cc: Anatolij Gustschin <agust@denx.de>
> > ---
> >  common/cmd_bmp.c |    2 +-
> >  1 files changed, 1 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);
> >         if (dst == NULL) {
> >                 puts("Error: malloc in gunzip failed!\n");
> >                 return NULL;
> 
> Sorry, I still have a question. Does this 'dst' address get used as
> the actual LCD frame buffer on your board, or is it just copied to the
> frame buffer?
> 

I must admit, that I've misunderstood the LCD code a bit. Reply to this
post from Anatolij helped me.
The buffer (dst) is only the "internal" buffer from which we are
assigning BMP data to actual frame buffer area. Then only the frame
buffer (pointed by fb pointer) area needs cache flush (which is done at
lcd_sync()).

To sum up - this patch shall be dropped.
However the second patch - 

[PATCH 2/2] video:cache:fix:trats: Enable dcache flush for TRATS
board's LCD subsystem

is crucial to fix the BMP image display distortion.

> Regards,
> Simon
> 
> > --
> > 1.7.2.3
> >
Simon Glass - Jan. 8, 2013, 2:54 p.m.
Hi Lukasz,

On Tue, Jan 8, 2013 at 12:28 AM, Lukasz Majewski <l.majewski@samsung.com> wrote:
> Hi Simon,
>
>> Hi Lukasz,
>>
>> On Mon, Jan 7, 2013 at 1:23 AM, Lukasz Majewski
>> <l.majewski@samsung.com> wrote:
>> > This commit makes the video subsystem code cache aware.
>> > Memory allocated for decompressed BMP memory is now cache line
>> > aligned.
>> >
>> > Tested-by: Lukasz Majewski <l.majewski@samsung.com>
>> > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
>> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> > Cc: Anatolij Gustschin <agust@denx.de>
>> > ---
>> >  common/cmd_bmp.c |    2 +-
>> >  1 files changed, 1 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);
>> >         if (dst == NULL) {
>> >                 puts("Error: malloc in gunzip failed!\n");
>> >                 return NULL;
>>
>> Sorry, I still have a question. Does this 'dst' address get used as
>> the actual LCD frame buffer on your board, or is it just copied to the
>> frame buffer?
>>
>
> I must admit, that I've misunderstood the LCD code a bit. Reply to this
> post from Anatolij helped me.
> The buffer (dst) is only the "internal" buffer from which we are
> assigning BMP data to actual frame buffer area. Then only the frame
> buffer (pointed by fb pointer) area needs cache flush (which is done at
> lcd_sync()).

OK that makes sense, thanks.

>
> To sum up - this patch shall be dropped.
> However the second patch -
>
> [PATCH 2/2] video:cache:fix:trats: Enable dcache flush for TRATS
> board's LCD subsystem
>
> is crucial to fix the BMP image display distortion.

Yes I agree.

Regards,
Simon

> --
> Best regards,
>
> Lukasz Majewski
>
> Samsung R&D Poland (SRPOL) | Linux Platform Group

Patch

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);
 	if (dst == NULL) {
 		puts("Error: malloc in gunzip failed!\n");
 		return NULL;