All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Petr Vandrovec <vandrove@vc.cvut.cz>
Cc: Linus Torvalds <torvalds@osdl.org>,
	Roland Dreier <roland@topspin.com>,
	Alan Cox <alan@lxorguk.ukuu.org.uk>,
	Andrew Morton <akpm@osdl.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ppc64: Fix __raw_* IO accessors
Date: Thu, 23 Sep 2004 10:49:00 +1000	[thread overview]
Message-ID: <1095900539.6359.46.camel@gaston> (raw)
In-Reply-To: <20040922185851.GA11017@vana.vc.cvut.cz>

On Thu, 2004-09-23 at 04:58, Petr Vandrovec wrote:

> > There still is that issue with __raw_* doing both barrier-less and
> > endianswap-less accesses though. I think there is a fundamental problem
> > here with drivers like matroxfb using them to get endian-less access and
> > losing barriers at the same time.
> 
> Before I put __raw_* there, code was using direct *(u_int32_t*)(mmio +
> reg) = value, and nobody complained... (and it worked on my PReP box).
> It seems that PPC does not reorder concurrent writes targetting one
> device.

It may re-order write vs. reads, nobody complained because on most old
machines, the CPU would be too dumb to do really heavy re-ordering but
that is no longer the case. This is definitely a bug.

> > I'd rather have matroxfb use writel with an explicit swap, or better, the
> > driver could maybe disable big endian register access and switch the card
> > to little endian, provided it can do that while keeping the frame buffer
> > itself set to BE (which is necessary most of the time).
>
> It is due to compatibility with XFree (or at least I was told) - they want 
> both framebuffer and accelerator in big-endian mode, so there is really no
> choice (other than not supporting ppc...).

Hrm... having a quick look at mga driver in current Xorg tree, it uses
the MMIO_IN/OUT macros directly, those are not byteswapping ?

It also does this at one point (ugh !) :

#if X_BYTE_ORDER == X_BIG_ENDIAN
	/* Disable byte-swapping for big-endian architectures - the XFree
	   driver seems to like a little-endian framebuffer -ReneR */
	/* pReg->Option |= 0x80000000; */
	pReg->Option &= ~0x80000000;
#endif

Weird... I think the X driver just lacks any "knowledge" of what's going
on with endianness...

> But of course, I can use writel(swab(...)) to get big-endian PCI
> accesses if __raw_* does not work on your hardware...

It's not that "__raw_*" does not work for my hardware ... it's that __raw_*
is always wrong to use on MMIO register accesses (unless you know _exactly_ 
what you are doing, for example it may be acceptable for filling a fifo in
some cases provided the first & last writes are not __raw)

Ben.


  reply	other threads:[~2004-09-23  0:52 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-09-21  9:23 [PATCH] ppc64: Fix __raw_* IO accessors Benjamin Herrenschmidt
2004-09-21 10:05 ` Alan Cox
2004-09-21 11:41   ` Benjamin Herrenschmidt
2004-09-21 19:30     ` Roland Dreier
2004-09-21 19:41       ` Linus Torvalds
2004-09-21 20:55         ` Geert Uytterhoeven
2004-09-21 22:05         ` Roland Dreier
2004-09-21 22:16           ` Linus Torvalds
2004-09-22  1:34             ` Benjamin Herrenschmidt
2004-09-22 18:58               ` Petr Vandrovec
2004-09-23  0:49                 ` Benjamin Herrenschmidt [this message]
2004-09-23 15:25                   ` Petr Vandrovec
2004-09-23 20:26                     ` [PATCH] matroxfb big-endian update (was Re: [PATCH] ppc64: Fix __raw_* IO accessors) Petr Vandrovec
2004-09-24  6:25                       ` Benjamin Herrenschmidt
2004-09-24  9:53                         ` Petr Vandrovec
2004-09-24 16:16                           ` Kostas Georgiou
2004-09-25  1:40                           ` Benjamin Herrenschmidt
2004-09-23 22:23                     ` [PATCH] ppc64: Fix __raw_* IO accessors Benjamin Herrenschmidt
2004-09-22  2:15             ` Paul Mackerras
2004-09-22  7:36               ` Roland Dreier
2004-09-22  1:31           ` Benjamin Herrenschmidt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1095900539.6359.46.camel@gaston \
    --to=benh@kernel.crashing.org \
    --cc=akpm@osdl.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=roland@topspin.com \
    --cc=torvalds@osdl.org \
    --cc=vandrove@vc.cvut.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.