[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#572311: No display output from Radeon RV610 on Alpha



On 13/03/10 06:57, Alex Deucher wrote:
On Fri, Mar 12, 2010 at 4:57 AM, Michael Cree<mcree@orcon.net.nz>  wrote:
On 10/03/10 08:44, Alex Deucher wrote:

On Sun, Mar 7, 2010 at 3:47 AM, Michael Cree<mcree@orcon.net.nz>
  wrote:

Thanks, that hint was helpful.  I have drummed up a patch (attached)
that
replaces some use of the UINT16LE_TO_CPU(), etc., macros with generic
interfaces from the Xserver's compiler.h header file.  Now works
correctly
on RV610 video card on an Alpha XP1000.  Have also verified that the
driver
still works on an RV710 card on AMD64 architecture.

Can you add the alignment stuff to the ATOM_BSWAP16/32 functions in
radeon_atombios.c?
e.g.,
return ldw_u(bswap_16(x));

That's a good idea, however I think the ldw_u() must be inside the byte swap
as the (mis)alignment issues must be dealt with at the point of loading the
datum, whereas endianess can be fixed later.

Attached is a new patch that uses the ldw_u() macros and also leaves the
UINT16LE_TO_CPU, etc., macros in place.  Verified working on Alpha and AMD64
architectures, but I don't have a suitable big-endian machine to test this.

Wouldn't it be cleaner to just put the ldw_u() in ATOM_BSWAP16()?

--- a/src/radeon_atombios.c
+++ b/src/radeon_atombios.c
@@ -28,6 +28,7 @@
  #endif
  #include "xf86.h"
  #include "xf86_OSproc.h"
+#include "compiler.h"

  #include "radeon.h"
  #include "radeon_reg.h"
@@ -2981,7 +2982,7 @@
atombios_get_command_table_version(atomBiosHandlePtr atomBIOS, int
index, int *m

  UINT16 ATOM_BSWAP16(UINT16 x)
  {
-    return bswap_16(x);
+    return bswap_16(ldw_u(x));
  }

No, that won't work for two reasons:

1) It's only in the big endian code path. There are little endian architectures that need the ldw_u().

2) ldw_u()'s argument is a pointer (i.e. the address from which the word is to be loaded) not a value. This is also the reason that I didn't include the ldw_u()s in the UINT16LE_TO_CPU, etc., macro definitions in Decoder.h. Those macros sometimes wrap an access via a pointer and at other times they wrap an actual value. Use of ldw_u() is only appropriate at the actual access of a datum from the AtomBios, i.e., those cases that perform an access through a pointer.

Cheers
Michael.



Reply to: