Re: [Nouveau] Fix unaligned accesses for SPARC

2013-03-13 Thread Marcin Slusarz
On Tue, Mar 12, 2013 at 10:18:41PM -0500, Patrick Baggett wrote:
The nouveau driver makes a number of unaligned accesses via the
ROM16(), ROM32() and ROM64() macros which fault on SPARC (but would be
transparently handled by x86 hardware). Making use of get_unaligned()
macro fixes the problem for me.
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_bios.h
b/drivers/gpu/drm/nouveau/nouveau_bios.h
index 7ccd28f..92031f6 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bios.h
+++ b/drivers/gpu/drm/nouveau/nouveau_bios.h
@@ -26,6 +26,8 @@
 #include nvreg.h
+#include asm/unaligned.h
+
 #define DCB_MAX_NUM_ENTRIES 16
 #define DCB_MAX_NUM_I2C_ENTRIES 16
 #define DCB_MAX_NUM_GPIO_ENTRIES 32
@@ -33,10 +35,10 @@
 #define DCB_LOC_ON_CHIP 0
-#define ROM16(x) le16_to_cpu(*(u16 *)(x))
-#define ROM32(x) le32_to_cpu(*(u32 *)(x))
+#define ROM16(x) le16_to_cpu(get_unaligned((u16 *)(x)))
+#define ROM32(x) le32_to_cpu(get_unaligned((u32 *)(x)))
 #define ROM48(x) ({ u8 *p = (x); (u64)ROM16(p[4])  32 |
ROM32(p[0]); })
-#define ROM64(x) le64_to_cpu(*(u64 *)(x))
+#define ROM64(x) le64_to_cpu(get_unaligned((u64 *)(x)))
 #define ROMPTR(d,x) ({\
struct nouveau_drm *drm = nouveau_drm((d)); \
ROM16(x) ? drm-vbios.data[ROM16(x)] : NULL; \

Please take a look at Documentation/SubmittingPatches and resubmit after
adapting.

IMHO, I think you should submit this patch only once you figure out how to
correctly boot and use at least basic accelerated operations, because I'm not
100% sure this is correct - 1. you have other severe issues with nouveau which
may or may not be connected to this code, 2. sparc defines get_unaligned to
__get_unaligned_be, so maybe you need to use _le variant? or maybe use
get_unaligned_le* without le*_to_cpu?.

Marcin
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau


Re: [Nouveau] Fix unaligned accesses for SPARC

2013-03-13 Thread Patrick Baggett
Marcin,

I had the same messages on NV17, which worked extremely well (OpenGL games
worked fine). The kernel will properly emulate the unaligned access, it
just causes a trap which is slow. If the unaligned access caused an error,
the NV17 card would have been unable to function given the number of
erroneous accesses.

Using __get_unaligned_le would probably remove one byteswap though.

Patrick

On Wed, Mar 13, 2013 at 4:29 PM, Marcin Slusarz marcin.slus...@gmail.comwrote:

 On Tue, Mar 12, 2013 at 10:18:41PM -0500, Patrick Baggett wrote:
 The nouveau driver makes a number of unaligned accesses via the
 ROM16(), ROM32() and ROM64() macros which fault on SPARC (but would be
 transparently handled by x86 hardware). Making use of get_unaligned()
 macro fixes the problem for me.
 
 diff --git a/drivers/gpu/drm/nouveau/nouveau_bios.h
 b/drivers/gpu/drm/nouveau/nouveau_bios.h
 index 7ccd28f..92031f6 100644
 --- a/drivers/gpu/drm/nouveau/nouveau_bios.h
 +++ b/drivers/gpu/drm/nouveau/nouveau_bios.h
 @@ -26,6 +26,8 @@
  #include nvreg.h
 +#include asm/unaligned.h
 +
  #define DCB_MAX_NUM_ENTRIES 16
  #define DCB_MAX_NUM_I2C_ENTRIES 16
  #define DCB_MAX_NUM_GPIO_ENTRIES 32
 @@ -33,10 +35,10 @@
  #define DCB_LOC_ON_CHIP 0
 -#define ROM16(x) le16_to_cpu(*(u16 *)(x))
 -#define ROM32(x) le32_to_cpu(*(u32 *)(x))
 +#define ROM16(x) le16_to_cpu(get_unaligned((u16 *)(x)))
 +#define ROM32(x) le32_to_cpu(get_unaligned((u32 *)(x)))
  #define ROM48(x) ({ u8 *p = (x); (u64)ROM16(p[4])  32 |
 ROM32(p[0]); })
 -#define ROM64(x) le64_to_cpu(*(u64 *)(x))
 +#define ROM64(x) le64_to_cpu(get_unaligned((u64 *)(x)))
  #define ROMPTR(d,x) ({\
 struct nouveau_drm *drm = nouveau_drm((d)); \
 ROM16(x) ? drm-vbios.data[ROM16(x)] : NULL; \

 Please take a look at Documentation/SubmittingPatches and resubmit after
 adapting.

 IMHO, I think you should submit this patch only once you figure out how to
 correctly boot and use at least basic accelerated operations, because I'm
 not
 100% sure this is correct - 1. you have other severe issues with nouveau
 which
 may or may not be connected to this code, 2. sparc defines get_unaligned to
 __get_unaligned_be, so maybe you need to use _le variant? or maybe use
 get_unaligned_le* without le*_to_cpu?.

 Marcin

___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau


[Nouveau] Fix unaligned accesses for SPARC

2013-03-12 Thread Patrick Baggett
The nouveau driver makes a number of unaligned accesses via the ROM16(),
ROM32() and ROM64() macros which fault on SPARC (but would be transparently
handled by x86 hardware). Making use of get_unaligned() macro fixes the
problem for me.

diff --git a/drivers/gpu/drm/nouveau/nouveau_bios.h
b/drivers/gpu/drm/nouveau/nouveau_bios.h
index 7ccd28f..92031f6 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bios.h
+++ b/drivers/gpu/drm/nouveau/nouveau_bios.h
@@ -26,6 +26,8 @@

 #include nvreg.h

+#include asm/unaligned.h
+
 #define DCB_MAX_NUM_ENTRIES 16
 #define DCB_MAX_NUM_I2C_ENTRIES 16
 #define DCB_MAX_NUM_GPIO_ENTRIES 32
@@ -33,10 +35,10 @@

 #define DCB_LOC_ON_CHIP 0

-#define ROM16(x) le16_to_cpu(*(u16 *)(x))
-#define ROM32(x) le32_to_cpu(*(u32 *)(x))
+#define ROM16(x) le16_to_cpu(get_unaligned((u16 *)(x)))
+#define ROM32(x) le32_to_cpu(get_unaligned((u32 *)(x)))
 #define ROM48(x) ({ u8 *p = (x); (u64)ROM16(p[4])  32 | ROM32(p[0]); })
-#define ROM64(x) le64_to_cpu(*(u64 *)(x))
+#define ROM64(x) le64_to_cpu(get_unaligned((u64 *)(x)))
 #define ROMPTR(d,x) ({\
struct nouveau_drm *drm = nouveau_drm((d)); \
ROM16(x) ? drm-vbios.data[ROM16(x)] : NULL; \
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau