[lkml]   [2000]   [Jan]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [DRIVER UPDATE] aty128fb
    >Well then I suggest fixing it instead ;-)

    Yep ;) I'm discussing with Paul about this issue. My current
    understanding is that the few cases where sync is actually needed are
    arch-specific anyway (TLB management, spinlock implementation, ...) and
    so doesn't require this generic macro, but I want to be sure first.

    >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.

    Using lwbrx/stwbrx allow to have separate registers for the base address
    and the offset of the register. That means that a smart compiler can put
    the base address in a register once and keep it for all the subsequent
    accesses. But that's probably not relevant in real-life performances when
    doing a register access in a driver. (It's more interesting when using
    those functions to byte-swap a bunch of stuffs in memory, but that's a
    different matter).

    >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.

    This problem is true, regardless of the position of eieio. It's even a
    non-platform specific problem since PCI write posting will cause this to
    happen, regardless of the CPU ordering or eieio's.

    If you need to have a known state and make sure the register was actually
    written to the card, you need to do a read after the write from the same
    address if possible (or from an other address on the same bus path).
    Since this read will have it's eieio, things will then be completely
    safe. This typically happens when dealing with on-board interrupt
    masking/unmasking registers, when you want to make sure the interrupt is
    actually masked before proceeding. (In this specific case of interrupt
    management, you may also need an isync, but that's yet-another problem. I
    beleive we lack a cross-platform macro for instruction-barrier, but I
    still need to think more about this).

    >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.

    Yep. Or you can eventually use normal memory accesses like it's done
    usually when accessing the fb. datas for the cursor pixels. In any case,
    you need wmb() macro between filling the cursor datas and enabling it,
    and this is not ppc-specific.

    >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.

    I need to think more about this (and discuss with some people having more
    knowledge thatn I do) before giving you a definitive answer on this. I
    just noticed that they were defined as sync.

    >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.

    In this case, the compiler can't help since the readl/writel macro don't
    use the index argument of the lwbrx/stwbrx instructions at all. But you
    are probably right about the irrelevance of such an optimisation in the
    driver. It may help keeing the code a bit smaller, which is always a good
    thing IMHO, but I agree we could stick with readl/writel for now.

    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    Please read the FAQ at

     \ /
      Last update: 2005-03-22 13:56    [W:0.033 / U:9.440 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site