* [U-Boot] [PATCH 1/2] video:cache:fix: Buffer alignment and dcache flush for lcd subsystem
@ 2012-08-08 15:10 Lukasz Majewski
2012-08-08 15:10 ` [U-Boot] [PATCH 2/2] video:trats:logo: Make tizen_hd_logo cache line aligned Lukasz Majewski
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Lukasz Majewski @ 2012-08-08 15:10 UTC (permalink / raw)
To: u-boot
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.
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 +-
common/lcd.c | 3 +++
2 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/common/cmd_bmp.c b/common/cmd_bmp.c
index b8809e3..7d3f45a 100644
--- a/common/cmd_bmp.c
+++ b/common/cmd_bmp.c
@@ -54,7 +54,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;
diff --git a/common/lcd.c b/common/lcd.c
index 506a138..b092a11 100644
--- a/common/lcd.c
+++ b/common/lcd.c
@@ -802,6 +802,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));
break;
#endif /* CONFIG_BMP_32BPP */
default:
--
1.7.2.3
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH 2/2] video:trats:logo: Make tizen_hd_logo cache line aligned.
2012-08-08 15:10 [U-Boot] [PATCH 1/2] video:cache:fix: Buffer alignment and dcache flush for lcd subsystem Lukasz Majewski
@ 2012-08-08 15:10 ` Lukasz Majewski
2012-08-08 15:24 ` [U-Boot] [PATCH 1/2] video:cache:fix: Buffer alignment and dcache flush for lcd subsystem Mike Frysinger
2013-01-02 16:25 ` [U-Boot] [PATCH RESEND] " Lukasz Majewski
2 siblings, 0 replies; 13+ messages in thread
From: Lukasz Majewski @ 2012-08-08 15:10 UTC (permalink / raw)
To: u-boot
The tizen_hd_logo is now aligned to cache line size.
Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Anatolij Gustschin <agust@denx.de>
---
lib/tizen/tizen_hd_logo.h | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/lib/tizen/tizen_hd_logo.h b/lib/tizen/tizen_hd_logo.h
index 2258fd5..51ed844 100644
--- a/lib/tizen/tizen_hd_logo.h
+++ b/lib/tizen/tizen_hd_logo.h
@@ -22,7 +22,9 @@
#ifndef _TIZEN_HD_LOGO_H_
#define _TIZEN_HD_LOGO_H_
-unsigned char tizen_hd_logo[]={
+#include <linux/compiler.h>
+
+unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE) tizen_hd_logo[] = {
0x1f,0x8b,0x08,0x08,0xe0,0x6f,0x8f,0x4f,0x00,0x03,0x74,0x72,0x61,0x74,0x73,0x2e,
0x62,0x6d,0x70,0x00,0xbc,0x5c,0xe9,0x53,0x1c,0x57,0x92,0xaf,0x6a,0xa0,0x81,0xa6,
0xb9,0x1b,0x04,0x48,0x02,0x04,0x7d,0xd0,0xdd,0x1c,0x12,0xba,0xd1,0x61,0x59,0x48,
--
1.7.2.3
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH 1/2] video:cache:fix: Buffer alignment and dcache flush for lcd subsystem
2012-08-08 15:10 [U-Boot] [PATCH 1/2] video:cache:fix: Buffer alignment and dcache flush for lcd subsystem Lukasz Majewski
2012-08-08 15:10 ` [U-Boot] [PATCH 2/2] video:trats:logo: Make tizen_hd_logo cache line aligned Lukasz Majewski
@ 2012-08-08 15:24 ` Mike Frysinger
2012-08-09 7:14 ` Lukasz Majewski
2013-01-02 16:25 ` [U-Boot] [PATCH RESEND] " Lukasz Majewski
2 siblings, 1 reply; 13+ messages in thread
From: Mike Frysinger @ 2012-08-08 15:24 UTC (permalink / raw)
To: u-boot
On Wednesday 08 August 2012 11:10:34 Lukasz Majewski 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.
i think this is a more comprehensive fix:
http://patchwork.ozlabs.org/patch/164722/
although your memalign() call might be useful to integrate into Simon's change
-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/20120808/92bd6d36/attachment.pgp>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH 1/2] video:cache:fix: Buffer alignment and dcache flush for lcd subsystem
2012-08-08 15:24 ` [U-Boot] [PATCH 1/2] video:cache:fix: Buffer alignment and dcache flush for lcd subsystem Mike Frysinger
@ 2012-08-09 7:14 ` Lukasz Majewski
0 siblings, 0 replies; 13+ messages in thread
From: Lukasz Majewski @ 2012-08-09 7:14 UTC (permalink / raw)
To: u-boot
Hi Mike,
> On Wednesday 08 August 2012 11:10:34 Lukasz Majewski 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.
>
> i think this is a more comprehensive fix:
> http://patchwork.ozlabs.org/patch/164722/
Yes, indeed. Thanks for pointing out.
>
> although your memalign() call might be useful to integrate into
> Simon's change -mike
I will comment the v3 patch verison.
--
Best regards,
Lukasz Majewski
Samsung Poland R&D Center | Linux Platform Group
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH RESEND] video:cache:fix: Buffer alignment and dcache flush for lcd subsystem
2012-08-08 15:10 [U-Boot] [PATCH 1/2] video:cache:fix: Buffer alignment and dcache flush for lcd subsystem Lukasz Majewski
2012-08-08 15:10 ` [U-Boot] [PATCH 2/2] video:trats:logo: Make tizen_hd_logo cache line aligned Lukasz Majewski
2012-08-08 15:24 ` [U-Boot] [PATCH 1/2] video:cache:fix: Buffer alignment and dcache flush for lcd subsystem Mike Frysinger
@ 2013-01-02 16:25 ` Lukasz Majewski
2013-01-05 1:25 ` Simon Glass
2 siblings, 1 reply; 13+ messages in thread
From: Lukasz Majewski @ 2013-01-02 16:25 UTC (permalink / raw)
To: u-boot
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)).
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 +-
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);
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));
break;
#endif /* CONFIG_BMP_32BPP */
default:
--
1.7.2.3
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH RESEND] video:cache:fix: Buffer alignment and dcache flush for lcd subsystem
2013-01-02 16:25 ` [U-Boot] [PATCH RESEND] " Lukasz Majewski
@ 2013-01-05 1:25 ` Simon Glass
2013-01-06 8:03 ` Lukasz Majewski
0 siblings, 1 reply; 13+ messages in thread
From: Simon Glass @ 2013-01-05 1:25 UTC (permalink / raw)
To: u-boot
Hi Lukasz,
On Wed, Jan 2, 2013 at 8:25 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.
>
> 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@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 +-
> 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH RESEND] video:cache:fix: Buffer alignment and dcache flush for lcd subsystem
2013-01-05 1:25 ` Simon Glass
@ 2013-01-06 8:03 ` Lukasz Majewski
2013-01-06 15:47 ` Simon Glass
0 siblings, 1 reply; 13+ messages in thread
From: Lukasz Majewski @ 2013-01-06 8:03 UTC (permalink / raw)
To: u-boot
Hi Simon,
> Hi Lukasz,
>
> On Wed, Jan 2, 2013 at 8:25 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.
> >
> > 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@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 +-
> > 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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH RESEND] video:cache:fix: Buffer alignment and dcache flush for lcd subsystem
2013-01-06 8:03 ` Lukasz Majewski
@ 2013-01-06 15:47 ` Simon Glass
2013-01-06 20:21 ` Wolfgang Denk
2013-01-06 22:54 ` Anatolij Gustschin
0 siblings, 2 replies; 13+ messages in thread
From: Simon Glass @ 2013-01-06 15:47 UTC (permalink / raw)
To: u-boot
Hi Lukasz,
On Sun, Jan 6, 2013 at 12:03 AM, Lukasz Majewski <l.majewski@majess.pl> wrote:
> Hi Simon,
>
>> Hi Lukasz,
>>
>> On Wed, Jan 2, 2013 at 8:25 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.
>> >
>> > 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@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 +-
>> > 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).
OK, so it is directly used as a frame buffer? In that case it looks
right to me. I doubt you want to be able to control the cache features
for this area, since you only write it once. But if you did, then the
memory would currently need to be section-aligned.
>
>>
>> > 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.
OK.
>
>>
>> > break;
>> > #endif /* CONFIG_BMP_32BPP */
>> > default:
>> > --
>> > 1.7.2.3
>> >
>>
>> Regards,
>> Simon
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
>
Regards,
Simon
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH RESEND] video:cache:fix: Buffer alignment and dcache flush for lcd subsystem
2013-01-06 15:47 ` Simon Glass
@ 2013-01-06 20:21 ` Wolfgang Denk
2013-01-06 21:38 ` Simon Glass
2013-01-06 23:09 ` Anatolij Gustschin
2013-01-06 22:54 ` Anatolij Gustschin
1 sibling, 2 replies; 13+ messages in thread
From: Wolfgang Denk @ 2013-01-06 20:21 UTC (permalink / raw)
To: u-boot
Dear Lukasz & Simon,
In message
<CAPnjgZ3tfvRJO-16ncwE43hPptdMEtOmPEK7pfeLkrMn-rVrgw@mail.gmail.com>
Simon Glass wrote:
>
> >> > - 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).
>
> OK, so it is directly used as a frame buffer? In that case it looks
> right to me. I doubt you want to be able to control the cache features
> for this area, since you only write it once. But if you did, then the
> memory would currently need to be section-aligned.
I don't think this is as it should be. Any frame buffer that actually
gets used as such should be located in the memory area specifically
allocated for this purpose using lcd_setmem(). This is especially
important when you load a splash screen image and intend to display it
flicker-free when booting an OS. If you use malloc() or memalign(),
it will be memory in the malloc arena, which gets overwritten when an
OS boots, resulting in a corrupted display.
Note: I added Anatolij, our video custodian to the Cc: list.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If you think the problem is bad now, just wait until we've solved it.
Epstein's Law
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH RESEND] video:cache:fix: Buffer alignment and dcache flush for lcd subsystem
2013-01-06 20:21 ` Wolfgang Denk
@ 2013-01-06 21:38 ` Simon Glass
2013-01-06 23:40 ` Wolfgang Denk
2013-01-06 23:09 ` Anatolij Gustschin
1 sibling, 1 reply; 13+ messages in thread
From: Simon Glass @ 2013-01-06 21:38 UTC (permalink / raw)
To: u-boot
Hi Wolfgang,
On Sun, Jan 6, 2013 at 12:21 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Lukasz & Simon,
>
> In message
> <CAPnjgZ3tfvRJO-16ncwE43hPptdMEtOmPEK7pfeLkrMn-rVrgw@mail.gmail.com>
> Simon Glass wrote:
>>
>> >> > - 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).
>>
>> OK, so it is directly used as a frame buffer? In that case it looks
>> right to me. I doubt you want to be able to control the cache features
>> for this area, since you only write it once. But if you did, then the
>> memory would currently need to be section-aligned.
>
> I don't think this is as it should be. Any frame buffer that actually
> gets used as such should be located in the memory area specifically
> allocated for this purpose using lcd_setmem(). This is especially
> important when you load a splash screen image and intend to display it
> flicker-free when booting an OS. If you use malloc() or memalign(),
> it will be memory in the malloc arena, which gets overwritten when an
> OS boots, resulting in a corrupted display.
>
> Note: I added Anatolij, our video custodian to the Cc: list.
OK, well in that case we should deprecate this business of using a
separate memory buffer as a frame buffer, and make people uncompress
the BMP to the normal frame buffer. It would simplify a few things and
I doubt there would be a speed penalty (or at least it could be
corrected by decompressing directly to the frame buffer).
Regards,
Simon
>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> If you think the problem is bad now, just wait until we've solved it.
> Epstein's Law
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH RESEND] video:cache:fix: Buffer alignment and dcache flush for lcd subsystem
2013-01-06 15:47 ` Simon Glass
2013-01-06 20:21 ` Wolfgang Denk
@ 2013-01-06 22:54 ` Anatolij Gustschin
1 sibling, 0 replies; 13+ messages in thread
From: Anatolij Gustschin @ 2013-01-06 22:54 UTC (permalink / raw)
To: u-boot
Hi Simon, Lukasz,
On Sun, 6 Jan 2013 07:47:58 -0800
Simon Glass <sjg@chromium.org> wrote:
...
> >> > 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).
>
> OK, so it is directly used as a frame buffer?
the allocated area isn't directly used as a frame buffer, it is
a buffer for bmp image which will be processed by lcd_display_bitmap().
The latter looks at the image header/palette and fills the
frame buffer (lcd_base) with the image data. This image buffer can
be unaligned I think.
Thanks,
Anatolij
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH RESEND] video:cache:fix: Buffer alignment and dcache flush for lcd subsystem
2013-01-06 20:21 ` Wolfgang Denk
2013-01-06 21:38 ` Simon Glass
@ 2013-01-06 23:09 ` Anatolij Gustschin
1 sibling, 0 replies; 13+ messages in thread
From: Anatolij Gustschin @ 2013-01-06 23:09 UTC (permalink / raw)
To: u-boot
Hello Wolfgang,
On Sun, 06 Jan 2013 21:21:00 +0100
Wolfgang Denk <wd@denx.de> wrote:
...
> > OK, so it is directly used as a frame buffer? In that case it looks
> > right to me. I doubt you want to be able to control the cache features
> > for this area, since you only write it once. But if you did, then the
> > memory would currently need to be section-aligned.
>
> I don't think this is as it should be. Any frame buffer that actually
> gets used as such should be located in the memory area specifically
> allocated for this purpose using lcd_setmem(). This is especially
> important when you load a splash screen image and intend to display it
> flicker-free when booting an OS. If you use malloc() or memalign(),
> it will be memory in the malloc arena, which gets overwritten when an
> OS boots, resulting in a corrupted display.
yes, for lcd.c driver the frame buffer is allocated by lcd_setmem().
malloc() is used only to buffer uncompressed BMP data here and it will
be freed right after extracting the pixel data to the actual frame buffer.
Thanks,
Anatolij
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH RESEND] video:cache:fix: Buffer alignment and dcache flush for lcd subsystem
2013-01-06 21:38 ` Simon Glass
@ 2013-01-06 23:40 ` Wolfgang Denk
0 siblings, 0 replies; 13+ messages in thread
From: Wolfgang Denk @ 2013-01-06 23:40 UTC (permalink / raw)
To: u-boot
Dear Simon,
In message <CAPnjgZ1X+anT6x4C0D6ZCoB3VvVVm1aBudkrcGkxg122F8XGmQ@mail.gmail.com> you wrote:
>
> > I don't think this is as it should be. Any frame buffer that actually
> > gets used as such should be located in the memory area specifically
> > allocated for this purpose using lcd_setmem(). This is especially
> > important when you load a splash screen image and intend to display it
> > flicker-free when booting an OS. If you use malloc() or memalign(),
> > it will be memory in the malloc arena, which gets overwritten when an
> > OS boots, resulting in a corrupted display.
> >
> > Note: I added Anatolij, our video custodian to the Cc: list.
>
> OK, well in that case we should deprecate this business of using a
> separate memory buffer as a frame buffer, and make people uncompress
> the BMP to the normal frame buffer. It would simplify a few things and
> I doubt there would be a speed penalty (or at least it could be
> corrected by decompressing directly to the frame buffer).
Agreed - thanks!
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
All he had was nothing, but that was something, and now it had been
taken away. - Terry Pratchett, _Sourcery_
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2013-01-06 23:40 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-08 15:10 [U-Boot] [PATCH 1/2] video:cache:fix: Buffer alignment and dcache flush for lcd subsystem Lukasz Majewski
2012-08-08 15:10 ` [U-Boot] [PATCH 2/2] video:trats:logo: Make tizen_hd_logo cache line aligned Lukasz Majewski
2012-08-08 15:24 ` [U-Boot] [PATCH 1/2] video:cache:fix: Buffer alignment and dcache flush for lcd subsystem Mike Frysinger
2012-08-09 7:14 ` Lukasz Majewski
2013-01-02 16:25 ` [U-Boot] [PATCH RESEND] " Lukasz Majewski
2013-01-05 1:25 ` Simon Glass
2013-01-06 8:03 ` Lukasz Majewski
2013-01-06 15:47 ` Simon Glass
2013-01-06 20:21 ` Wolfgang Denk
2013-01-06 21:38 ` Simon Glass
2013-01-06 23:40 ` Wolfgang Denk
2013-01-06 23:09 ` Anatolij Gustschin
2013-01-06 22:54 ` Anatolij Gustschin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox