>>>>> "Benjamin" == Benjamin Herrenschmidt <bh40@calva.net> writes:
Benjamin> On Thu, Jan 27, 2000, Jes Sorensen <Jes.Sorensen@cern.ch>
Benjamin> wrote:
>> First of all you ought to use wmb() and rmb() which are the
>> portable names for those instructions rather than the PPC specific
>> eieio() macro.
Benjamin> rmb() is broken on PPC. Not really broken, but it does a
Benjamin> sync, which is a _huge_ overkill and which should be avoided
Benjamin> except when interrupt synchro or SMP synchro must be
Benjamin> acheived.
Well then I suggest fixing it instead ;-)
>> Second, you are not adding any io barrier to the 32 bit store macro
>> (aty_st_le32()) but you do for the 8 bit version, why?
Benjamin> Hum. It should be there. Did I miss this when you asked my
Benjamin> about your patch, Brad ? I Need some more sleep ...
Benjamin> More simply, I beleive you can directly use readl/writel
Benjamin> instead of using your own asm accessors. (Yours allow to use
Benjamin> the index regiser. So they may actually be good for perfs).
I don't think understand this one, sorry.
>> You also add the io barriers before the operation you are about to
>> perform instead of after. That means that if you are storing a
>> value to register, you have no idea when it is actually being
>> propagated out to the hardware. The safer way to do this would be
>> to put a wmb() after each register write and an rmb() after each
>> register read(), instead of before
Benjamin> I don't completely agree. Putting them before or after is a
Benjamin> matter of style. The rare cases where this actually matters
Benjamin> must usually be hand-tuned. (For example, filling some HW
Benjamin> cursor datas in the framebuffer, so without using eieio, and
Benjamin> then writing a register that will cause the chip to use
Benjamin> those datas. One eieio is needed before the write in this
Benjamin> case. I tend personally to prefer having them before.
The problem of putting them in front is that you may set a register
and you have no idea when the operation will take place, thus the card
may run with stale information in it's registers for a while. With
regard to filling the cursor then that particular example is not very
good since opdating the cursor data is rarely a performance critical
item ;-) However even for the performance critical cases (like
printing data on the screen), you'd rather use the
__raw_{read,write}l() macros and explicitly add the [wm]mb() macros.
Benjamin> Again, rmb() is evil on PPC since it does a sync, which is,
Benjamin> I think, very expensive on recent PPC CPUs.
Why is it not fixed then? Or is there a reason for this, ie. does it
need a "sync" on rmb for the SMP case of writing pointers to real
memory. In that case we may need tp add pci_[rw]mb() instead.
>> Last, this is more of a question, why does the PPC version of the
>> driver use it's own inline asm version for the 32 access instead of
>> using the readl/writel macros? readl/writel are defined to access
>> the PCI bus shared memory in the bus native byte order, ie. little
>> endian. Is this because it is running the chip in big endian mode?
>> If you want to access it in the hos CPU's native byte order you
>> should use the __raw_{write,read}l() macros instead and then add
>> the appropriate [rw]mb() macro calls youself.
Benjamin> Optimisation ? Doing so this way allow to use the 2
Benjamin> parameters of the stwbrx/lwbrx instrutions for the base and
Benjamin> the index. That means that the compiler can store the
Benjamin> register base address in one register, and keep it there all
Benjamin> the time, changing only the offset in a second register.
I never got far enough in the PPC User's Manual to comment on how the
specific instructions work. However if {read,write}l() is done as
inline macros you still let the compiler optimize away. Though, even
if they are not, then the calculations you perform to get to the
proper address are quickly lost in the noise of the time it actually
takes to access the PCI shared memory.
Jes
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Mon Jan 31 2000 - 21:00:20 EST