Patchwork [U-Boot] drivers:lcd: fix unaligned access on lcd

login
register
mail settings
Submitter Piotr Wilczek
Date May 24, 2013, 7:46 a.m.
Message ID <1369381565-13946-1-git-send-email-p.wilczek@samsung.com>
Download mbox | patch
Permalink /patch/246101/
State Rejected
Delegated to: Anatolij Gustschin
Headers show

Comments

Piotr Wilczek - May 24, 2013, 7:46 a.m.
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(-)
Wolfgang Denk - May 24, 2013, 6:33 p.m.
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
Jeroen Hofstee - May 24, 2013, 6:41 p.m.
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
Piotr Wilczek - May 27, 2013, 11:35 a.m.
Dear Wolfgang Denk,

> -----Original Message-----
> From: Wolfgang Denk [mailto:wd@denx.de]
> Sent: Friday, May 24, 2013 8:33 PM
> To: Piotr Wilczek
> Cc: u-boot@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@denx.de
> "Spock, did you see the looks on their faces?"
> "Yes, Captain, a sort of vacant contentment."

Best regards,
Piotr Wilczek
Nikita Kiryanov - May 28, 2013, 6:10 a.m.
Hi Piotr,

On 05/27/2013 02:35 PM, Piotr Wilczek wrote:
> Dear Wolfgang Denk,
>
>> -----Original Message-----
>> From: Wolfgang Denk [mailto:wd@denx.de]
>> Sent: Friday, May 24, 2013 8:33 PM
>> To: Piotr Wilczek
>> Cc: u-boot@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.

Patch

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");