On Tue, 2002-04-30 at 17:47, Jose Fonseca wrote:
> On Tue, 2002-04-30 at 15:49, Michel Dänzer wrote:
> > On Tue, 2002-04-30 at 15:26, José Fonseca wrote:
> > > On 2002.04.30 14:16 Michel Dänzer wrote:
> > > > On Tue, 2002-04-30 at 10:45, José Fonseca wrote:
> > > > > .... This is still not very clear - yesterday we
> > > > > discussed this in the IRC meeting -, because the colors are ok. Looking
> > > > > careful to the picture is seems that we have to word swap and not byte
> > > > > swap. Perhaps because within a pixel, the color disposition isn't
> > > > changed
> > > > > across little/big endian architectures.
> > > > 
> > > > I was going to explain this is due to 32 bit swapping exchanging 16 bit
> > > > texels, but on second thought it should work if both the texture and
> > > > framebuffer use the same bpp.
> > > > 
> > > > Where can I see that screenshot? :)
> > > 
> > > Is in Peter's last message with attachement.
> > 
> > I don't seem to have gotten that.
> 
> It seems that the mailing list eat it! I attached it again.

Thanks, let's see if this one gets through the list, maybe not due to
its size.

It looks as I expected, however I still wonder how this comes with 16
bit textures in depth 16 (right?).


> > > I still don't know how to define the MMIO to be portable... Shouldn't we 
> > > use the read/write* macros instead of cpu_to_le32/le32_to_cpu?
> > 
> > Good idea! I've been missing those.
> > 
> > > Shouldn't be a wmb() on writes and a mb() on reads ?
> > 
> > Most definitely.
> > 
> > > Everywhere I look I read a different way of how to do... it's insane!
> > 
> > Indeed, so let me repeat my proposal to do something like this for all
> > drivers on all architectures:
> > 
> > #define ##_READ(reg)                (_##_READ(##_ADDR(reg)))
> > static inline u32 _##_READ(u32 *addr)
> > {
> >     mb();
> >     return readl(addr);
> > }
> > #define ##_WRITE(reg,val)                                   \
> > do {                                                                \
> >     wmb();                                                  \
> >     writel(val,##_ADDR(reg));                               \
> > } while (0)
> > 
> 
> This I like it more. Although it's the safe bet (and doesn't affect the
> x86 architecture), it's a little overkill to have wmb() in every write.
> It's only needed when the order of writes do matter.

Which tends to be the case for register writes, doesn't it?

> On the other hand the bulk of data is sent via DMA anyway.
> 
> > (I wonder if the wmb() shouldn't rather be after the writel() in
> > ##_WRITE() ...)
> > 
> 
> It's the same, since in a set of writes there will be always one extra
> wmb() either at the beginning or the end.

But the one at the beginning is really useless IMHO while the one at the
end assures that the last write hits the bus. This is what the DDX MMIO
macros do.


> > Looks like this could even be templated?
> 
> I don't think that the C preprocessor would allow templates in the
> macros names...

You're probably right, maybe a preprocessor wizard can think of a way?

Anyway, looking at that code I wonder why not use inline functions for
both?

static inline u32 mmio_in32(u32 *addr) {
        mb();
        return readl(addr);
}

static void mmio_out32(u32 *addr, u32 val) {
        writel(val, addr);
        wmb();
}


and then


#define ##_READ(reg)            mmio_in32(##_ADDR(reg))
#define ##_WRITE(reg,val)       mmio_out32(##_ADDR(reg),val)


That way the compiler might help us spot errors. Any drawbacks?


-- 
Earthling Michel Dänzer (MrCooper)/ Debian GNU/Linux (powerpc) developer
XFree86 and DRI project member   /  CS student, Free Software enthusiast

_______________________________________________
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel

Reply via email to