* [U-Boot] [PATCH] drivers:lcd: fix unaligned access on lcd
@ 2013-05-24 7:46 Piotr Wilczek
2013-05-24 18:33 ` Wolfgang Denk
` (2 more replies)
0 siblings, 3 replies; 20+ messages in thread
From: Piotr Wilczek @ 2013-05-24 7:46 UTC (permalink / raw)
To: u-boot
This patch replace 'le32_to_cpu' function with 'get_unaligend_le32' to
avoid unaligned access exception on some ARM platforms (ex Trats2).
Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
CC: Minkyu Kang <mk7.kang@samsung.com>
CC: Anatolij Gustschin <agust@denx.de>
---
common/lcd.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/common/lcd.c b/common/lcd.c
index edae835..577a452 100644
--- a/common/lcd.c
+++ b/common/lcd.c
@@ -42,6 +42,7 @@
#endif
#include <lcd.h>
#include <watchdog.h>
+#include <asm/unaligned.h>
#if defined(CONFIG_CPU_PXA25X) || defined(CONFIG_CPU_PXA27X) || \
defined(CONFIG_CPU_MONAHANS)
@@ -907,9 +908,9 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
return 1;
}
- width = le32_to_cpu(bmp->header.width);
- height = le32_to_cpu(bmp->header.height);
- bmp_bpix = le16_to_cpu(bmp->header.bit_count);
+ width = get_unaligned_le32(&bmp->header.width);
+ height = get_unaligned_le32(&bmp->header.height);
+ bmp_bpix = get_unaligned_le32(&bmp->header.bit_count);
colors = 1 << bmp_bpix;
bpix = NBITS(panel_info.vl_bpix);
@@ -994,7 +995,7 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
if ((y + height) > panel_info.vl_row)
height = panel_info.vl_row - y;
- bmap = (uchar *) bmp + le32_to_cpu(bmp->header.data_offset);
+ bmap = (uchar *)bmp + get_unaligned_le32(&bmp->header.data_offset);
fb = (uchar *) (lcd_base +
(y + height - 1) * lcd_line_length + x * bpix / 8);
@@ -1002,7 +1003,8 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
case 1: /* pass through */
case 8:
#ifdef CONFIG_LCD_BMP_RLE8
- if (le32_to_cpu(bmp->header.compression) == BMP_BI_RLE8) {
+ if (get_unaligned_le32(&bmp->header.compression) ==
+ BMP_BI_RLE8) {
if (bpix != 16) {
/* TODO implement render code for bpix != 16 */
printf("Error: only support 16 bpix");
--
1.7.9.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH] drivers:lcd: fix unaligned access on lcd
2013-05-24 7:46 [U-Boot] [PATCH] drivers:lcd: fix unaligned access on lcd Piotr Wilczek
@ 2013-05-24 18:33 ` Wolfgang Denk
2013-05-27 11:35 ` Piotr Wilczek
2013-05-24 18:41 ` Jeroen Hofstee
2013-05-31 11:26 ` [U-Boot] [PATCH] lcd: align bmp header when uncopmressing image Piotr Wilczek
2 siblings, 1 reply; 20+ messages in thread
From: Wolfgang Denk @ 2013-05-24 18:33 UTC (permalink / raw)
To: u-boot
Dear Piotr Wilczek,
In message <1369381565-13946-1-git-send-email-p.wilczek@samsung.com> you wrote:
> This patch replace 'le32_to_cpu' function with 'get_unaligend_le32' to
> avoid unaligned access exception on some ARM platforms (ex Trats2).
I think we've been discussng this before, and then decided to rather
make sure that there will be no unaligned accesses because the header
will be kept properly aligned.
What is your new use case that requires this?
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
"Spock, did you see the looks on their faces?"
"Yes, Captain, a sort of vacant contentment."
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH] drivers:lcd: fix unaligned access on lcd
2013-05-24 7:46 [U-Boot] [PATCH] drivers:lcd: fix unaligned access on lcd Piotr Wilczek
2013-05-24 18:33 ` Wolfgang Denk
@ 2013-05-24 18:41 ` Jeroen Hofstee
2013-05-31 11:26 ` [U-Boot] [PATCH] lcd: align bmp header when uncopmressing image Piotr Wilczek
2 siblings, 0 replies; 20+ messages in thread
From: Jeroen Hofstee @ 2013-05-24 18:41 UTC (permalink / raw)
To: u-boot
Hello Piotr,
On 05/24/2013 09:46 AM, Piotr Wilczek wrote:
> This patch replace 'le32_to_cpu' function with 'get_unaligend_le32' to
> avoid unaligned access exception on some ARM platforms (ex Trats2).
>
> Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> CC: Minkyu Kang <mk7.kang@samsung.com>
> CC: Anatolij Gustschin <agust@denx.de>
> ---
> common/lcd.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> ntf("Error: only support 16 bpix");
If you use an aligned address + 2 as the BMP location, the aligned
accesses should work. If you have user input (which I doubt) you
can set CONFIG_SPLASHIMAGE_GUARD (see README).
The cm_t35 LCD thread has more info if you are interested.
Regards,
Jeroen
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH] drivers:lcd: fix unaligned access on lcd
2013-05-24 18:33 ` Wolfgang Denk
@ 2013-05-27 11:35 ` Piotr Wilczek
2013-05-28 6:10 ` Nikita Kiryanov
0 siblings, 1 reply; 20+ messages in thread
From: Piotr Wilczek @ 2013-05-27 11:35 UTC (permalink / raw)
To: u-boot
Dear Wolfgang Denk,
> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de]
> Sent: Friday, May 24, 2013 8:33 PM
> To: Piotr Wilczek
> Cc: u-boot at lists.denx.de; Kyungmin Park
> Subject: Re: [U-Boot] [PATCH] drivers:lcd: fix unaligned access on lcd
>
> Dear Piotr Wilczek,
>
> In message <1369381565-13946-1-git-send-email-p.wilczek@samsung.com>
> you wrote:
> > This patch replace 'le32_to_cpu' function with 'get_unaligend_le32'
> to
> > avoid unaligned access exception on some ARM platforms (ex Trats2).
>
> I think we've been discussng this before, and then decided to rather
> make sure that there will be no unaligned accesses because the header
> will be kept properly aligned.
>
> What is your new use case that requires this?
My case is a bmp unzipped to a 4-byte aligned address. The two signature bytes of the bmp header make the other fields 4-byte unaligned. We could force unzip the bmp to an aligned address + 2.
Is this a better solution?
>
> 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
> "Spock, did you see the looks on their faces?"
> "Yes, Captain, a sort of vacant contentment."
Best regards,
Piotr Wilczek
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH] drivers:lcd: fix unaligned access on lcd
2013-05-27 11:35 ` Piotr Wilczek
@ 2013-05-28 6:10 ` Nikita Kiryanov
0 siblings, 0 replies; 20+ messages in thread
From: Nikita Kiryanov @ 2013-05-28 6:10 UTC (permalink / raw)
To: u-boot
Hi Piotr,
On 05/27/2013 02:35 PM, Piotr Wilczek wrote:
> Dear Wolfgang Denk,
>
>> -----Original Message-----
>> From: Wolfgang Denk [mailto:wd at denx.de]
>> Sent: Friday, May 24, 2013 8:33 PM
>> To: Piotr Wilczek
>> Cc: u-boot at lists.denx.de; Kyungmin Park
>> Subject: Re: [U-Boot] [PATCH] drivers:lcd: fix unaligned access on lcd
>>
[...]
>
> My case is a bmp unzipped to a 4-byte aligned address. The two signature bytes of the bmp header make the other fields 4-byte unaligned. We could force unzip the bmp to an aligned address + 2.
>
> Is this a better solution?
That is the solution we settled on the last time this was discussed.
See CONFIG_SPLASHIMAGE_GUARD in the README file if your use case
involves user input, or just pick an aligned address + 2 in your code.
--
Regards,
Nikita.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH] lcd: align bmp header when uncopmressing image
2013-05-24 7:46 [U-Boot] [PATCH] drivers:lcd: fix unaligned access on lcd Piotr Wilczek
2013-05-24 18:33 ` Wolfgang Denk
2013-05-24 18:41 ` Jeroen Hofstee
@ 2013-05-31 11:26 ` Piotr Wilczek
2013-05-31 14:22 ` Wolfgang Denk
` (2 more replies)
2 siblings, 3 replies; 20+ messages in thread
From: Piotr Wilczek @ 2013-05-31 11:26 UTC (permalink / raw)
To: u-boot
When compressed image is loaded, it must be decompressed
to an aligned address + 2 to avoid unaligned access exception
on some ARM platforms.
Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
CC: Anatolij Gustschin <agust@denx.de>
CC: Wolfgang Denk: <wd@denx.de>
---
common/cmd_bmp.c | 40 +++++++++++++++++++++++++++-------------
include/lcd.h | 3 ++-
2 files changed, 29 insertions(+), 14 deletions(-)
diff --git a/common/cmd_bmp.c b/common/cmd_bmp.c
index 5a52edd..fea4708 100644
--- a/common/cmd_bmp.c
+++ b/common/cmd_bmp.c
@@ -38,14 +38,19 @@ static int bmp_info (ulong addr);
/*
* Allocate and decompress a BMP image using gunzip().
*
- * Returns a pointer to the decompressed image data. Must be freed by
- * the caller after use.
+ * Returns a pointer to the decompressed image data. This pointer is
+ * is aligned to 32-bit-aligned-address + 2.
+ * See doc/README.displaying-bmps for explanation.
+ *
+ * The allocation address is passed to 'alloc_addr' and must be freed
+ * by the caller after use.
*
* Returns NULL if decompression failed, or if the decompressed data
* didn't contain a valid BMP signature.
*/
#ifdef CONFIG_VIDEO_BMP_GZIP
-bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp)
+bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp,
+ void **alloc_addr)
{
void *dst;
unsigned long len;
@@ -60,7 +65,14 @@ bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp)
puts("Error: malloc in gunzip failed!\n");
return NULL;
}
- if (gunzip(dst, CONFIG_SYS_VIDEO_LOGO_MAX_SIZE, (uchar *)addr, &len) != 0) {
+
+ bmp = dst;
+
+ /* align to 32-bit-aligned-address + 2 */
+ if ((unsigned int)bmp % 0x04 != 0x02)
+ bmp = (bmp_image_t *)(((unsigned int)dst + 0x02) & ~0x01);
+
+ if (gunzip(bmp, CONFIG_SYS_VIDEO_LOGO_MAX_SIZE, (uchar *)addr, &len) != 0) {
free(dst);
return NULL;
}
@@ -68,8 +80,6 @@ bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp)
puts("Image could be truncated"
" (increase CONFIG_SYS_VIDEO_LOGO_MAX_SIZE)!\n");
- bmp = dst;
-
/*
* Check for bmp mark 'BM'
*/
@@ -81,10 +91,12 @@ bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp)
debug("Gzipped BMP image detected!\n");
+ *alloc_addr = dst;
return bmp;
}
#else
-bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp)
+bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp,
+ void **alloc_addr)
{
return NULL;
}
@@ -189,11 +201,12 @@ U_BOOT_CMD(
static int bmp_info(ulong addr)
{
bmp_image_t *bmp=(bmp_image_t *)addr;
+ void *bmp_alloc_addr = NULL;
unsigned long len;
if (!((bmp->header.signature[0]=='B') &&
(bmp->header.signature[1]=='M')))
- bmp = gunzip_bmp(addr, &len);
+ bmp = gunzip_bmp(addr, &len, &bmp_alloc_addr);
if (bmp == NULL) {
printf("There is no valid bmp file at the given address\n");
@@ -205,8 +218,8 @@ static int bmp_info(ulong addr)
printf("Bits per pixel: %d\n", le16_to_cpu(bmp->header.bit_count));
printf("Compression : %d\n", le32_to_cpu(bmp->header.compression));
- if ((unsigned long)bmp != addr)
- free(bmp);
+ if (bmp_alloc_addr)
+ free(bmp_alloc_addr);
return(0);
}
@@ -225,11 +238,12 @@ int bmp_display(ulong addr, int x, int y)
{
int ret;
bmp_image_t *bmp = (bmp_image_t *)addr;
+ void *bmp_alloc_addr = NULL;
unsigned long len;
if (!((bmp->header.signature[0]=='B') &&
(bmp->header.signature[1]=='M')))
- bmp = gunzip_bmp(addr, &len);
+ bmp = gunzip_bmp(addr, &len, &bmp_alloc_addr);
if (!bmp) {
printf("There is no valid bmp file at the given address\n");
@@ -244,8 +258,8 @@ int bmp_display(ulong addr, int x, int y)
# error bmp_display() requires CONFIG_LCD or CONFIG_VIDEO
#endif
- if ((unsigned long)bmp != addr)
- free(bmp);
+ if (bmp_alloc_addr)
+ free(bmp_alloc_addr);
return ret;
}
diff --git a/include/lcd.h b/include/lcd.h
index c6e7fc5..482e606 100644
--- a/include/lcd.h
+++ b/include/lcd.h
@@ -46,7 +46,8 @@ void lcd_initcolregs(void);
int lcd_getfgcolor(void);
/* gunzip_bmp used if CONFIG_VIDEO_BMP_GZIP */
-struct bmp_image *gunzip_bmp(unsigned long addr, unsigned long *lenp);
+struct bmp_image *gunzip_bmp(unsigned long addr, unsigned long *lenp,
+ void **alloc_addr);
int bmp_display(ulong addr, int x, int y);
/**
--
1.7.9.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH] lcd: align bmp header when uncopmressing image
2013-05-31 11:26 ` [U-Boot] [PATCH] lcd: align bmp header when uncopmressing image Piotr Wilczek
@ 2013-05-31 14:22 ` Wolfgang Denk
2013-06-03 6:17 ` Piotr Wilczek
2013-06-02 11:05 ` Nikita Kiryanov
2013-06-03 13:59 ` [U-Boot] [PATCH V2] " Piotr Wilczek
2 siblings, 1 reply; 20+ messages in thread
From: Wolfgang Denk @ 2013-05-31 14:22 UTC (permalink / raw)
To: u-boot
Dear Piotr Wilczek,
In message <1369999573-15449-1-git-send-email-p.wilczek@samsung.com> you wrote:
> When compressed image is loaded, it must be decompressed
> to an aligned address + 2 to avoid unaligned access exception
> on some ARM platforms.
If you do this, you must also account for the up to 2 additional
bytes needed in the allocated buffer.
Otherwise you might write over the end of the buffer...
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
Die ganzen Zahlen hat der liebe Gott geschaffen, alles andere ist
Menschenwerk... Leopold Kronecker
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH] lcd: align bmp header when uncopmressing image
2013-05-31 11:26 ` [U-Boot] [PATCH] lcd: align bmp header when uncopmressing image Piotr Wilczek
2013-05-31 14:22 ` Wolfgang Denk
@ 2013-06-02 11:05 ` Nikita Kiryanov
2013-06-03 6:39 ` Piotr Wilczek
2013-06-03 13:59 ` [U-Boot] [PATCH V2] " Piotr Wilczek
2 siblings, 1 reply; 20+ messages in thread
From: Nikita Kiryanov @ 2013-06-02 11:05 UTC (permalink / raw)
To: u-boot
Hello Piotr,
On 05/31/2013 02:26 PM, Piotr Wilczek wrote:
> -bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp)
> +bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp,
> + void **alloc_addr)
> {
> void *dst;
> unsigned long len;
> @@ -60,7 +65,14 @@ bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp)
> puts("Error: malloc in gunzip failed!\n");
> return NULL;
> }
> - if (gunzip(dst, CONFIG_SYS_VIDEO_LOGO_MAX_SIZE, (uchar *)addr, &len) != 0) {
> +
> + bmp = dst;
> +
> + /* align to 32-bit-aligned-address + 2 */
> + if ((unsigned int)bmp % 0x04 != 0x02)
> + bmp = (bmp_image_t *)(((unsigned int)dst + 0x02) & ~0x01);
This is wrong. Suppose that bmp % 4 == 3, then (dst + 2) % 4 == 1,
and thus ((dst + 2) & ~1) % 4 == 0, which is not an aligned+2 address.
> +
> + if (gunzip(bmp, CONFIG_SYS_VIDEO_LOGO_MAX_SIZE, (uchar *)addr, &len) != 0) {
> free(dst);
> return NULL;
> }
> @@ -68,8 +80,6 @@ bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp)
> puts("Image could be truncated"
> " (increase CONFIG_SYS_VIDEO_LOGO_MAX_SIZE)!\n");
>
--
Regards,
Nikita.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH] lcd: align bmp header when uncopmressing image
2013-05-31 14:22 ` Wolfgang Denk
@ 2013-06-03 6:17 ` Piotr Wilczek
2013-06-03 11:15 ` Wolfgang Denk
0 siblings, 1 reply; 20+ messages in thread
From: Piotr Wilczek @ 2013-06-03 6:17 UTC (permalink / raw)
To: u-boot
Dear Wolfgang Denk,
> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de]
> Sent: Friday, May 31, 2013 4:23 PM
> To: Piotr Wilczek
> Cc: u-boot at lists.denx.de; Minkyu Kang; Kyungmin Park; Lukasz Majewski;
> Anatolij Gustschin
> Subject: Re: [PATCH] lcd: align bmp header when uncopmressing image
>
> Dear Piotr Wilczek,
>
> In message <1369999573-15449-1-git-send-email-p.wilczek@samsung.com>
> you wrote:
> > When compressed image is loaded, it must be decompressed to an
> aligned
> > address + 2 to avoid unaligned access exception on some ARM
> platforms.
>
> If you do this, you must also account for the up to 2 additional bytes
> needed in the allocated buffer.
>
> Otherwise you might write over the end of the buffer...
>
Because 8-byte alignment is guaranteed by malloc I don't think might over write anything when moving by only 2 bytes.
But you are right that in principle extra bytes should be allocated.
> 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
> Die ganzen Zahlen hat der liebe Gott geschaffen, alles andere ist
> Menschenwerk... Leopold Kronecker
Best regards,
Piotr Wilczek
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH] lcd: align bmp header when uncopmressing image
2013-06-02 11:05 ` Nikita Kiryanov
@ 2013-06-03 6:39 ` Piotr Wilczek
2013-06-03 11:18 ` Wolfgang Denk
0 siblings, 1 reply; 20+ messages in thread
From: Piotr Wilczek @ 2013-06-03 6:39 UTC (permalink / raw)
To: u-boot
Hello Nikita,
> -----Original Message-----
> From: Nikita Kiryanov [mailto:nikita at compulab.co.il]
> Sent: Sunday, June 02, 2013 1:06 PM
> To: Piotr Wilczek
> Cc: u-boot at lists.denx.de; Kyungmin Park
> Subject: Re: [U-Boot] [PATCH] lcd: align bmp header when uncopmressing
> image
>
> Hello Piotr,
>
> On 05/31/2013 02:26 PM, Piotr Wilczek wrote:
> > -bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp)
> > +bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp,
> > + void **alloc_addr)
> > {
> > void *dst;
> > unsigned long len;
> > @@ -60,7 +65,14 @@ bmp_image_t *gunzip_bmp(unsigned long addr,
> unsigned long *lenp)
> > puts("Error: malloc in gunzip failed!\n");
> > return NULL;
> > }
> > - if (gunzip(dst, CONFIG_SYS_VIDEO_LOGO_MAX_SIZE, (uchar *)addr,
> &len) != 0) {
> > +
> > + bmp = dst;
> > +
> > + /* align to 32-bit-aligned-address + 2 */
> > + if ((unsigned int)bmp % 0x04 != 0x02)
> > + bmp = (bmp_image_t *)(((unsigned int)dst + 0x02) & ~0x01);
>
> This is wrong. Suppose that bmp % 4 == 3, then (dst + 2) % 4 == 1, and
> thus ((dst + 2) & ~1) % 4 == 0, which is not an aligned+2 address.
>
You are right but here I'm aligning a pointer returned by malloc which is
guaranteed to be aligned to 8 bytes. In fact it is sufficient only to add
two bytes.
Anyway, to make the code universal I provide a better alignment method.
> > +
> > + if (gunzip(bmp, CONFIG_SYS_VIDEO_LOGO_MAX_SIZE, (uchar *)addr,
> &len)
> > +!= 0) {
> > free(dst);
> > return NULL;
> > }
> > @@ -68,8 +80,6 @@ bmp_image_t *gunzip_bmp(unsigned long addr,
> unsigned long *lenp)
> > puts("Image could be truncated"
> > " (increase
> CONFIG_SYS_VIDEO_LOGO_MAX_SIZE)!\n");
> >
>
> --
> Regards,
> Nikita.
Best regards,
Piotr
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH] lcd: align bmp header when uncopmressing image
2013-06-03 6:17 ` Piotr Wilczek
@ 2013-06-03 11:15 ` Wolfgang Denk
2013-06-03 14:14 ` Piotr Wilczek
0 siblings, 1 reply; 20+ messages in thread
From: Wolfgang Denk @ 2013-06-03 11:15 UTC (permalink / raw)
To: u-boot
Dear Piotr Wilczek,
In message <000601ce6021$fcb8f490$f62addb0$%wilczek@samsung.com> you wrote:
>
> > If you do this, you must also account for the up to 2 additional bytes
> > needed in the allocated buffer.
> >
> > Otherwise you might write over the end of the buffer...
> >
> Because 8-byte alignment is guaranteed by malloc I don't think might
> over write anything when moving by only 2 bytes.
Oops??? Initial alignment has NOTHING to do with writing over the
allocated end of memory!
> But you are right that in principle extra bytes should be allocated.
This is not a questions of "pricniple", but plainly of invoking
undefined behaviour.
Please fix.
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
A direct quote from the Boss: "We passed over a lot of good people to
get the ones we hired."
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH] lcd: align bmp header when uncopmressing image
2013-06-03 6:39 ` Piotr Wilczek
@ 2013-06-03 11:18 ` Wolfgang Denk
0 siblings, 0 replies; 20+ messages in thread
From: Wolfgang Denk @ 2013-06-03 11:18 UTC (permalink / raw)
To: u-boot
Dear Piotr Wilczek,
In message <000701ce6025$173886c0$45a99440$%wilczek@samsung.com> you wrote:
>
> > > + /* align to 32-bit-aligned-address + 2 */
> > > + if ((unsigned int)bmp % 0x04 != 0x02)
> > > + bmp = (bmp_image_t *)(((unsigned int)dst + 0x02) & ~0x01);
> >
> > This is wrong. Suppose that bmp % 4 == 3, then (dst + 2) % 4 == 1, and
> > thus ((dst + 2) & ~1) % 4 == 0, which is not an aligned+2 address.
> >
> You are right but here I'm aligning a pointer returned by malloc which is
> guaranteed to be aligned to 8 bytes. In fact it is sufficient only to add
> two bytes.
Such assumptions are black magic at best.
Please always consider the mentalk welfare of your follow programmers
who for example might change this into a character array (say, a
buffer on the stack) and suddenly experience mysetrious crashes.
Please always write code such that it is NOT based on non-obvious
requirements.
> Anyway, to make the code universal I provide a better alignment method.
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
"God is a comedian playing to an audience too afraid to laugh."
- Voltaire
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH V2] lcd: align bmp header when uncopmressing image
2013-05-31 11:26 ` [U-Boot] [PATCH] lcd: align bmp header when uncopmressing image Piotr Wilczek
2013-05-31 14:22 ` Wolfgang Denk
2013-06-02 11:05 ` Nikita Kiryanov
@ 2013-06-03 13:59 ` Piotr Wilczek
2013-06-03 18:44 ` Wolfgang Denk
2013-06-04 11:00 ` [U-Boot] [PATCH V3] " Piotr Wilczek
2 siblings, 2 replies; 20+ messages in thread
From: Piotr Wilczek @ 2013-06-03 13:59 UTC (permalink / raw)
To: u-boot
When compressed image is loaded, it must be decompressed
to an aligned address + 2 to avoid unaligned access exception
on some ARM platforms.
Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
CC: Anatolij Gustschin <agust@denx.de>
CC: Wolfgang Denk <wd@denx.de>
---
Changes for V2:
- add additional space for uncompressed bmp header due to alignment requirements
- fix to 32-bit-aligned-address + 2 alignent
common/cmd_bmp.c | 42 ++++++++++++++++++++++++++++--------------
include/lcd.h | 3 ++-
2 files changed, 30 insertions(+), 15 deletions(-)
diff --git a/common/cmd_bmp.c b/common/cmd_bmp.c
index 5a52edd..06634b2 100644
--- a/common/cmd_bmp.c
+++ b/common/cmd_bmp.c
@@ -38,14 +38,19 @@ static int bmp_info (ulong addr);
/*
* Allocate and decompress a BMP image using gunzip().
*
- * Returns a pointer to the decompressed image data. Must be freed by
- * the caller after use.
+ * Returns a pointer to the decompressed image data. This pointer is
+ * is aligned to 32-bit-aligned-address + 2.
+ * See doc/README.displaying-bmps for explanation.
+ *
+ * The allocation address is passed to 'alloc_addr' and must be freed
+ * by the caller after use.
*
* Returns NULL if decompression failed, or if the decompressed data
* didn't contain a valid BMP signature.
*/
#ifdef CONFIG_VIDEO_BMP_GZIP
-bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp)
+bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp,
+ void **alloc_addr)
{
void *dst;
unsigned long len;
@@ -55,12 +60,19 @@ 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 = malloc(CONFIG_SYS_VIDEO_LOGO_MAX_SIZE + 0x04);
if (dst == NULL) {
puts("Error: malloc in gunzip failed!\n");
return NULL;
}
- if (gunzip(dst, CONFIG_SYS_VIDEO_LOGO_MAX_SIZE, (uchar *)addr, &len) != 0) {
+
+ bmp = dst;
+
+ /* align to 32-bit-aligned-address + 2 */
+ if ((unsigned int)bmp % 0x04 != 0x02)
+ bmp = (bmp_image_t *)((((unsigned int)dst + 0x2) & ~0x1) | 0x2);
+
+ if (gunzip(bmp, CONFIG_SYS_VIDEO_LOGO_MAX_SIZE, (uchar *)addr, &len) != 0) {
free(dst);
return NULL;
}
@@ -68,8 +80,6 @@ bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp)
puts("Image could be truncated"
" (increase CONFIG_SYS_VIDEO_LOGO_MAX_SIZE)!\n");
- bmp = dst;
-
/*
* Check for bmp mark 'BM'
*/
@@ -81,10 +91,12 @@ bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp)
debug("Gzipped BMP image detected!\n");
+ *alloc_addr = dst;
return bmp;
}
#else
-bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp)
+bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp,
+ void **alloc_addr)
{
return NULL;
}
@@ -189,11 +201,12 @@ U_BOOT_CMD(
static int bmp_info(ulong addr)
{
bmp_image_t *bmp=(bmp_image_t *)addr;
+ void *bmp_alloc_addr = NULL;
unsigned long len;
if (!((bmp->header.signature[0]=='B') &&
(bmp->header.signature[1]=='M')))
- bmp = gunzip_bmp(addr, &len);
+ bmp = gunzip_bmp(addr, &len, &bmp_alloc_addr);
if (bmp == NULL) {
printf("There is no valid bmp file at the given address\n");
@@ -205,8 +218,8 @@ static int bmp_info(ulong addr)
printf("Bits per pixel: %d\n", le16_to_cpu(bmp->header.bit_count));
printf("Compression : %d\n", le32_to_cpu(bmp->header.compression));
- if ((unsigned long)bmp != addr)
- free(bmp);
+ if (bmp_alloc_addr)
+ free(bmp_alloc_addr);
return(0);
}
@@ -225,11 +238,12 @@ int bmp_display(ulong addr, int x, int y)
{
int ret;
bmp_image_t *bmp = (bmp_image_t *)addr;
+ void *bmp_alloc_addr = NULL;
unsigned long len;
if (!((bmp->header.signature[0]=='B') &&
(bmp->header.signature[1]=='M')))
- bmp = gunzip_bmp(addr, &len);
+ bmp = gunzip_bmp(addr, &len, &bmp_alloc_addr);
if (!bmp) {
printf("There is no valid bmp file at the given address\n");
@@ -244,8 +258,8 @@ int bmp_display(ulong addr, int x, int y)
# error bmp_display() requires CONFIG_LCD or CONFIG_VIDEO
#endif
- if ((unsigned long)bmp != addr)
- free(bmp);
+ if (bmp_alloc_addr)
+ free(bmp_alloc_addr);
return ret;
}
diff --git a/include/lcd.h b/include/lcd.h
index c6e7fc5..482e606 100644
--- a/include/lcd.h
+++ b/include/lcd.h
@@ -46,7 +46,8 @@ void lcd_initcolregs(void);
int lcd_getfgcolor(void);
/* gunzip_bmp used if CONFIG_VIDEO_BMP_GZIP */
-struct bmp_image *gunzip_bmp(unsigned long addr, unsigned long *lenp);
+struct bmp_image *gunzip_bmp(unsigned long addr, unsigned long *lenp,
+ void **alloc_addr);
int bmp_display(ulong addr, int x, int y);
/**
--
1.7.9.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH] lcd: align bmp header when uncopmressing image
2013-06-03 11:15 ` Wolfgang Denk
@ 2013-06-03 14:14 ` Piotr Wilczek
2013-06-03 18:45 ` Wolfgang Denk
0 siblings, 1 reply; 20+ messages in thread
From: Piotr Wilczek @ 2013-06-03 14:14 UTC (permalink / raw)
To: u-boot
Dear Wolfgang Denk,
> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de]
> Sent: Monday, June 03, 2013 1:16 PM
> To: Piotr Wilczek
> Cc: u-boot at lists.denx.de; 'Minkyu Kang'; 'Kyungmin Park'; Lukasz
> Majewski; 'Anatolij Gustschin'
> Subject: Re: [PATCH] lcd: align bmp header when uncopmressing image
>
> Dear Piotr Wilczek,
>
> In message <000601ce6021$fcb8f490$f62addb0$%wilczek@samsung.com> you
> wrote:
> >
> > > If you do this, you must also account for the up to 2 additional
> > > bytes needed in the allocated buffer.
> > >
> > > Otherwise you might write over the end of the buffer...
> > >
> > Because 8-byte alignment is guaranteed by malloc I don't think might
> > over write anything when moving by only 2 bytes.
>
> Oops??? Initial alignment has NOTHING to do with writing over the
> allocated end of memory!
>
No, I meant only that malloc allocates memory in at least 4-byte resolution.
I surely should have allocated extra memory for the aligned header in the first patch.
Please see my fixed patch.
Best regards,
Piotr Wilczek
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH V2] lcd: align bmp header when uncopmressing image
2013-06-03 13:59 ` [U-Boot] [PATCH V2] " Piotr Wilczek
@ 2013-06-03 18:44 ` Wolfgang Denk
2013-06-04 11:00 ` [U-Boot] [PATCH V3] " Piotr Wilczek
1 sibling, 0 replies; 20+ messages in thread
From: Wolfgang Denk @ 2013-06-03 18:44 UTC (permalink / raw)
To: u-boot
Dear Piotr Wilczek,
In message <1370267979-17800-1-git-send-email-p.wilczek@samsung.com> you wrote:
> When compressed image is loaded, it must be decompressed
> to an aligned address + 2 to avoid unaligned access exception
> on some ARM platforms.
...
> - dst = malloc(CONFIG_SYS_VIDEO_LOGO_MAX_SIZE);
> + dst = malloc(CONFIG_SYS_VIDEO_LOGO_MAX_SIZE + 0x04);
Why + 4? This needs a comment. And please write 4 as 4, and not as
0x4 when there is no good reason for it.
> + /* align to 32-bit-aligned-address + 2 */
> + if ((unsigned int)bmp % 0x04 != 0x02)
> + bmp = (bmp_image_t *)((((unsigned int)dst + 0x2) & ~0x1) | 0x2);
This is difficult to read and inefficient.
If you want to do it manually, just write it as
bmp = (bmp_image_t *)((((unsigned int)dst + 1) & ~3) + 2);
[Actually adding 3 to the "malloc()" size above would be sufficient
with this].
A more readable way but slightly less efficient way might be:
u32_t n = (u32_t)dst;
bmp = (bmp_image_t *)(ALIGN(n, 4) + 2);
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
I've got to get something inside me. Some coffee or something. And
then the world will somehow be better.
- Terry Pratchett, _Men at Arms_
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH] lcd: align bmp header when uncopmressing image
2013-06-03 14:14 ` Piotr Wilczek
@ 2013-06-03 18:45 ` Wolfgang Denk
0 siblings, 0 replies; 20+ messages in thread
From: Wolfgang Denk @ 2013-06-03 18:45 UTC (permalink / raw)
To: u-boot
Dear Piotr Wilczek,
In message <000b01ce6064$b52a7de0$1f7f79a0$%wilczek@samsung.com> you wrote:
>
> > Oops??? Initial alignment has NOTHING to do with writing over the
> > allocated end of memory!
>
> No, I meant only that malloc allocates memory in at least 4-byte resolution.
Does it? I see no such guarantee in the specification.
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
No problem is insoluble.
-- Dr. Janet Wallace, "The Deadly Years", stardate 3479.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH V3] lcd: align bmp header when uncopmressing image
2013-06-03 13:59 ` [U-Boot] [PATCH V2] " Piotr Wilczek
2013-06-03 18:44 ` Wolfgang Denk
@ 2013-06-04 11:00 ` Piotr Wilczek
2013-06-04 15:05 ` Wolfgang Denk
2013-06-05 6:14 ` [U-Boot] [PATCH V4] " Piotr Wilczek
1 sibling, 2 replies; 20+ messages in thread
From: Piotr Wilczek @ 2013-06-04 11:00 UTC (permalink / raw)
To: u-boot
When compressed image is loaded, it must be decompressed
to an aligned address + 2 to avoid unaligned access exception
on some ARM platforms.
Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
CC: Anatolij Gustschin <agust@denx.de>
CC: Wolfgang Denk <wd@denx.de>
---
Changes for V3:
- add comment on why extra space is allocated for for uncompressed bmp header
- change the + 2 aligment method as Wolfgang Denk suggested
Changes for V2:
- add additional space for uncompressed bmp header due to alignment requirements
- fix to 32-bit-aligned-address + 2 alignent
common/cmd_bmp.c | 43 +++++++++++++++++++++++++++++--------------
include/lcd.h | 3 ++-
2 files changed, 31 insertions(+), 15 deletions(-)
diff --git a/common/cmd_bmp.c b/common/cmd_bmp.c
index 5a52edd..1c1e2a9 100644
--- a/common/cmd_bmp.c
+++ b/common/cmd_bmp.c
@@ -38,14 +38,19 @@ static int bmp_info (ulong addr);
/*
* Allocate and decompress a BMP image using gunzip().
*
- * Returns a pointer to the decompressed image data. Must be freed by
- * the caller after use.
+ * Returns a pointer to the decompressed image data. This pointer is
+ * is aligned to 32-bit-aligned-address + 2.
+ * See doc/README.displaying-bmps for explanation.
+ *
+ * The allocation address is passed to 'alloc_addr' and must be freed
+ * by the caller after use.
*
* Returns NULL if decompression failed, or if the decompressed data
* didn't contain a valid BMP signature.
*/
#ifdef CONFIG_VIDEO_BMP_GZIP
-bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp)
+bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp,
+ void **alloc_addr)
{
void *dst;
unsigned long len;
@@ -55,12 +60,20 @@ 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);
+ /* allocate extra 3 bytes for 32-bit-aligned-address + 2 alignment */
+ dst = malloc(CONFIG_SYS_VIDEO_LOGO_MAX_SIZE + 3);
if (dst == NULL) {
puts("Error: malloc in gunzip failed!\n");
return NULL;
}
- if (gunzip(dst, CONFIG_SYS_VIDEO_LOGO_MAX_SIZE, (uchar *)addr, &len) != 0) {
+
+ bmp = dst;
+
+ /* align to 32-bit-aligned-address + 2 */
+ if ((unsigned int)bmp % 4 != 2)
+ bmp = (bmp_image_t *)((((unsigned int)dst + 1) & ~3) + 2);
+
+ if (gunzip(bmp, CONFIG_SYS_VIDEO_LOGO_MAX_SIZE, (uchar *)addr, &len) != 0) {
free(dst);
return NULL;
}
@@ -68,8 +81,6 @@ bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp)
puts("Image could be truncated"
" (increase CONFIG_SYS_VIDEO_LOGO_MAX_SIZE)!\n");
- bmp = dst;
-
/*
* Check for bmp mark 'BM'
*/
@@ -81,10 +92,12 @@ bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp)
debug("Gzipped BMP image detected!\n");
+ *alloc_addr = dst;
return bmp;
}
#else
-bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp)
+bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp,
+ void **alloc_addr)
{
return NULL;
}
@@ -189,11 +202,12 @@ U_BOOT_CMD(
static int bmp_info(ulong addr)
{
bmp_image_t *bmp=(bmp_image_t *)addr;
+ void *bmp_alloc_addr = NULL;
unsigned long len;
if (!((bmp->header.signature[0]=='B') &&
(bmp->header.signature[1]=='M')))
- bmp = gunzip_bmp(addr, &len);
+ bmp = gunzip_bmp(addr, &len, &bmp_alloc_addr);
if (bmp == NULL) {
printf("There is no valid bmp file at the given address\n");
@@ -205,8 +219,8 @@ static int bmp_info(ulong addr)
printf("Bits per pixel: %d\n", le16_to_cpu(bmp->header.bit_count));
printf("Compression : %d\n", le32_to_cpu(bmp->header.compression));
- if ((unsigned long)bmp != addr)
- free(bmp);
+ if (bmp_alloc_addr)
+ free(bmp_alloc_addr);
return(0);
}
@@ -225,11 +239,12 @@ int bmp_display(ulong addr, int x, int y)
{
int ret;
bmp_image_t *bmp = (bmp_image_t *)addr;
+ void *bmp_alloc_addr = NULL;
unsigned long len;
if (!((bmp->header.signature[0]=='B') &&
(bmp->header.signature[1]=='M')))
- bmp = gunzip_bmp(addr, &len);
+ bmp = gunzip_bmp(addr, &len, &bmp_alloc_addr);
if (!bmp) {
printf("There is no valid bmp file at the given address\n");
@@ -244,8 +259,8 @@ int bmp_display(ulong addr, int x, int y)
# error bmp_display() requires CONFIG_LCD or CONFIG_VIDEO
#endif
- if ((unsigned long)bmp != addr)
- free(bmp);
+ if (bmp_alloc_addr)
+ free(bmp_alloc_addr);
return ret;
}
diff --git a/include/lcd.h b/include/lcd.h
index c6e7fc5..482e606 100644
--- a/include/lcd.h
+++ b/include/lcd.h
@@ -46,7 +46,8 @@ void lcd_initcolregs(void);
int lcd_getfgcolor(void);
/* gunzip_bmp used if CONFIG_VIDEO_BMP_GZIP */
-struct bmp_image *gunzip_bmp(unsigned long addr, unsigned long *lenp);
+struct bmp_image *gunzip_bmp(unsigned long addr, unsigned long *lenp,
+ void **alloc_addr);
int bmp_display(ulong addr, int x, int y);
/**
--
1.7.9.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH V3] lcd: align bmp header when uncopmressing image
2013-06-04 11:00 ` [U-Boot] [PATCH V3] " Piotr Wilczek
@ 2013-06-04 15:05 ` Wolfgang Denk
2013-06-05 6:14 ` [U-Boot] [PATCH V4] " Piotr Wilczek
1 sibling, 0 replies; 20+ messages in thread
From: Wolfgang Denk @ 2013-06-04 15:05 UTC (permalink / raw)
To: u-boot
Dear Piotr Wilczek,
In message <1370343625-16596-1-git-send-email-p.wilczek@samsung.com> you wrote:
> When compressed image is loaded, it must be decompressed
> to an aligned address + 2 to avoid unaligned access exception
> on some ARM platforms.
...
> + /* align to 32-bit-aligned-address + 2 */
> + if ((unsigned int)bmp % 4 != 2)
> + bmp = (bmp_image_t *)((((unsigned int)dst + 1) & ~3) + 2);
Please drop the "if()" - it is not necessary and just adds to the code
size.
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
Never underestimate the bandwidth of a station wagon full of tapes.
-- Dr. Warren Jackson, Director, UTCS
^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH V4] lcd: align bmp header when uncopmressing image
2013-06-04 11:00 ` [U-Boot] [PATCH V3] " Piotr Wilczek
2013-06-04 15:05 ` Wolfgang Denk
@ 2013-06-05 6:14 ` Piotr Wilczek
2013-07-01 18:51 ` Anatolij Gustschin
1 sibling, 1 reply; 20+ messages in thread
From: Piotr Wilczek @ 2013-06-05 6:14 UTC (permalink / raw)
To: u-boot
When compressed image is loaded, it must be decompressed
to an aligned address + 2 to avoid unaligned access exception
on some ARM platforms.
Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
CC: Anatolij Gustschin <agust@denx.de>
CC: Wolfgang Denk <wd@denx.de>
---
Changes for V4:
- dropped the if condition
Changes for V3:
- add comment on why extra space is allocated for uncompressed bmp header
- change the + 2 aligment method as Wolfgang Denk suggested
Changes for V2:
- add additional space for uncompressed bmp header due to alignment requirements
- fix to 32-bit-aligned-address + 2 alignent
common/cmd_bmp.c | 42 ++++++++++++++++++++++++++++--------------
include/lcd.h | 3 ++-
2 files changed, 30 insertions(+), 15 deletions(-)
diff --git a/common/cmd_bmp.c b/common/cmd_bmp.c
index 5a52edd..675f47a 100644
--- a/common/cmd_bmp.c
+++ b/common/cmd_bmp.c
@@ -38,14 +38,19 @@ static int bmp_info (ulong addr);
/*
* Allocate and decompress a BMP image using gunzip().
*
- * Returns a pointer to the decompressed image data. Must be freed by
- * the caller after use.
+ * Returns a pointer to the decompressed image data. This pointer is
+ * is aligned to 32-bit-aligned-address + 2.
+ * See doc/README.displaying-bmps for explanation.
+ *
+ * The allocation address is passed to 'alloc_addr' and must be freed
+ * by the caller after use.
*
* Returns NULL if decompression failed, or if the decompressed data
* didn't contain a valid BMP signature.
*/
#ifdef CONFIG_VIDEO_BMP_GZIP
-bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp)
+bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp,
+ void **alloc_addr)
{
void *dst;
unsigned long len;
@@ -55,12 +60,19 @@ 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);
+ /* allocate extra 3 bytes for 32-bit-aligned-address + 2 alignment */
+ dst = malloc(CONFIG_SYS_VIDEO_LOGO_MAX_SIZE + 3);
if (dst == NULL) {
puts("Error: malloc in gunzip failed!\n");
return NULL;
}
- if (gunzip(dst, CONFIG_SYS_VIDEO_LOGO_MAX_SIZE, (uchar *)addr, &len) != 0) {
+
+ bmp = dst;
+
+ /* align to 32-bit-aligned-address + 2 */
+ bmp = (bmp_image_t *)((((unsigned int)dst + 1) & ~3) + 2);
+
+ if (gunzip(bmp, CONFIG_SYS_VIDEO_LOGO_MAX_SIZE, (uchar *)addr, &len) != 0) {
free(dst);
return NULL;
}
@@ -68,8 +80,6 @@ bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp)
puts("Image could be truncated"
" (increase CONFIG_SYS_VIDEO_LOGO_MAX_SIZE)!\n");
- bmp = dst;
-
/*
* Check for bmp mark 'BM'
*/
@@ -81,10 +91,12 @@ bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp)
debug("Gzipped BMP image detected!\n");
+ *alloc_addr = dst;
return bmp;
}
#else
-bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp)
+bmp_image_t *gunzip_bmp(unsigned long addr, unsigned long *lenp,
+ void **alloc_addr)
{
return NULL;
}
@@ -189,11 +201,12 @@ U_BOOT_CMD(
static int bmp_info(ulong addr)
{
bmp_image_t *bmp=(bmp_image_t *)addr;
+ void *bmp_alloc_addr = NULL;
unsigned long len;
if (!((bmp->header.signature[0]=='B') &&
(bmp->header.signature[1]=='M')))
- bmp = gunzip_bmp(addr, &len);
+ bmp = gunzip_bmp(addr, &len, &bmp_alloc_addr);
if (bmp == NULL) {
printf("There is no valid bmp file at the given address\n");
@@ -205,8 +218,8 @@ static int bmp_info(ulong addr)
printf("Bits per pixel: %d\n", le16_to_cpu(bmp->header.bit_count));
printf("Compression : %d\n", le32_to_cpu(bmp->header.compression));
- if ((unsigned long)bmp != addr)
- free(bmp);
+ if (bmp_alloc_addr)
+ free(bmp_alloc_addr);
return(0);
}
@@ -225,11 +238,12 @@ int bmp_display(ulong addr, int x, int y)
{
int ret;
bmp_image_t *bmp = (bmp_image_t *)addr;
+ void *bmp_alloc_addr = NULL;
unsigned long len;
if (!((bmp->header.signature[0]=='B') &&
(bmp->header.signature[1]=='M')))
- bmp = gunzip_bmp(addr, &len);
+ bmp = gunzip_bmp(addr, &len, &bmp_alloc_addr);
if (!bmp) {
printf("There is no valid bmp file at the given address\n");
@@ -244,8 +258,8 @@ int bmp_display(ulong addr, int x, int y)
# error bmp_display() requires CONFIG_LCD or CONFIG_VIDEO
#endif
- if ((unsigned long)bmp != addr)
- free(bmp);
+ if (bmp_alloc_addr)
+ free(bmp_alloc_addr);
return ret;
}
diff --git a/include/lcd.h b/include/lcd.h
index c6e7fc5..482e606 100644
--- a/include/lcd.h
+++ b/include/lcd.h
@@ -46,7 +46,8 @@ void lcd_initcolregs(void);
int lcd_getfgcolor(void);
/* gunzip_bmp used if CONFIG_VIDEO_BMP_GZIP */
-struct bmp_image *gunzip_bmp(unsigned long addr, unsigned long *lenp);
+struct bmp_image *gunzip_bmp(unsigned long addr, unsigned long *lenp,
+ void **alloc_addr);
int bmp_display(ulong addr, int x, int y);
/**
--
1.7.9.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH V4] lcd: align bmp header when uncopmressing image
2013-06-05 6:14 ` [U-Boot] [PATCH V4] " Piotr Wilczek
@ 2013-07-01 18:51 ` Anatolij Gustschin
0 siblings, 0 replies; 20+ messages in thread
From: Anatolij Gustschin @ 2013-07-01 18:51 UTC (permalink / raw)
To: u-boot
On Wed, 05 Jun 2013 08:14:30 +0200
Piotr Wilczek <p.wilczek@samsung.com> wrote:
> When compressed image is loaded, it must be decompressed
> to an aligned address + 2 to avoid unaligned access exception
> on some ARM platforms.
>
> Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> CC: Anatolij Gustschin <agust@denx.de>
> CC: Wolfgang Denk <wd@denx.de>
> ---
> Changes for V4:
> - dropped the if condition
>
> Changes for V3:
> - add comment on why extra space is allocated for uncompressed bmp header
> - change the + 2 aligment method as Wolfgang Denk suggested
>
> Changes for V2:
> - add additional space for uncompressed bmp header due to alignment requirements
> - fix to 32-bit-aligned-address + 2 alignent
>
> common/cmd_bmp.c | 42 ++++++++++++++++++++++++++++--------------
> include/lcd.h | 3 ++-
> 2 files changed, 30 insertions(+), 15 deletions(-)
Applied to u-boot-video/master (slightly corrected the comment
for gunzip_bmp()). Thanks!
Anatolij
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2013-07-01 18:51 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-24 7:46 [U-Boot] [PATCH] drivers:lcd: fix unaligned access on lcd Piotr Wilczek
2013-05-24 18:33 ` Wolfgang Denk
2013-05-27 11:35 ` Piotr Wilczek
2013-05-28 6:10 ` Nikita Kiryanov
2013-05-24 18:41 ` Jeroen Hofstee
2013-05-31 11:26 ` [U-Boot] [PATCH] lcd: align bmp header when uncopmressing image Piotr Wilczek
2013-05-31 14:22 ` Wolfgang Denk
2013-06-03 6:17 ` Piotr Wilczek
2013-06-03 11:15 ` Wolfgang Denk
2013-06-03 14:14 ` Piotr Wilczek
2013-06-03 18:45 ` Wolfgang Denk
2013-06-02 11:05 ` Nikita Kiryanov
2013-06-03 6:39 ` Piotr Wilczek
2013-06-03 11:18 ` Wolfgang Denk
2013-06-03 13:59 ` [U-Boot] [PATCH V2] " Piotr Wilczek
2013-06-03 18:44 ` Wolfgang Denk
2013-06-04 11:00 ` [U-Boot] [PATCH V3] " Piotr Wilczek
2013-06-04 15:05 ` Wolfgang Denk
2013-06-05 6:14 ` [U-Boot] [PATCH V4] " Piotr Wilczek
2013-07-01 18:51 ` Anatolij Gustschin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox