Re: Do cpu-endian MMIO accessors exist?

[Pekka Paalanen <pq@iki.fi>]

On Wed, 22 Jul 2009 00:01:59 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> On Tuesday 21 July 2009, Christoph Hellwig wrote:
> > Why would you want to do that?  That just means a useless byteswap.
> > We really should have a generic native-endian MMIO-access API as there
> > is quite a bit of hardware with features like that, and currently we
> > have a miriad of hacks using __raw_* and manual barriers, the ppc
> > specific accessors and god knows what.

My motivation is to clean up nouveau code, and I was first thinking the
same as Christoph.

Arnd is right, there is an easy way to switch the card's endianess,
in theory. I hear there are few cases where it doesn't work yet.

> The byte swap on powerpc I/O instructions is practically free
> on all the interesting CPUs, and on the others it is still
> swamped by the overhead of the synchronization. If you care
> about the latency of MMIO instructions, going to explicit
> synchronization would help much more, saving hundreds of
> cycles per I/O rather than one cycle for a saved byte swap.

I figured something like that while reading the ppc implementation.
Therefore having the card always LE is not much different from setting
it to cpu endianness. I'll let the Nouveau developers decide which
way to do it, and keep it as is for now.

> The powerpc in_le32 style functions are a completely different
> story, they are basically defined to operate only on on-chip
> components, while ioread32 and readl do work on PCI devices.

So what should I use on BE arches when the card is in BE mode?
Not out_be32() but iowrite32be()?
I never even noticed that io*be() functions exist, thanks for the hint.

On LE arches I'll stick to {read,write}[bwl], which in my
understanding always handle the hardware as LE. Right?

> No portable code should ever use the __raw_* functions and
> architecture specific barriers.

Yeah, it felt like that, hence I asked here.

> It would of course be easy to just define an API extension
> to ioread along the lines of
> 
> #ifdef __BIG_ENDIAN
> #define ioread16_native ioread16be
> #define ioread32_native ioread32be
> #define iowrite16_native iowrite16be
> #define iowrite32_native iowrite32be
> #else
> #define ioread16_native ioread16
> #define ioread32_native ioread32
> #define iowrite16_native iowrite16
> #define iowrite32_native iowrite32
> #endif
> 
> but I'm not yet convinced that there is a potential user that
> should not just be fixed in a different way.

This is how it currently is.


Thanks.

-- 
Pekka Paalanen
http://www.iki.fi/pq/