Leif,
On 2002.05.30 02:02 José Fonseca wrote:
> ... Tomorrow I'll take a look more carefully to see if I find anything
> suspicious. ...
I've been analyzing the diff very carefully and what I've found so far are
just some details which can be taken care later (i.e., no bugs sorry) :
- This code in the OUT_RING macro
+ if (idx > 3) idx = 0; \
+ if (idx == DMA_COMMAND) { \
+ last_cmd = (x) & ~DMA_EOL; \
+ last_cmd_ofs = write; \
+ } \
+ ring[write++] = cpu_to_le32( x ); \
+ idx++; \
+ write &= tail_mask; \
seems unnecessary and inefficient. We dont need to bookkeep idx,
last_cmd ans last_cmd_ofs when writing to the ring. We just need to make a
copy of the tail in the begining, and can get everything info we need from
that in COMMIT_RING.
- I think mach64_flush_write_combine is excessive and creates an excessive
penalty as it calls mb(). We just need to make sure that everything we
write goes to the physical memory, i.e., call wb() - which in the x86
platform is a null macro since there is no write cache. And this should be
only necessary in COMMIT_RING, as no-one else will read what we are
writing until toggle the continuation bit of the last descriptor.
- the RING* macros shouldn't work in number of DWORDS, but in number of
entries, and the OUT_RING macro should take 3 (or four?) arguments.
- ADVANCE_RING and COMMIT_RING should be one.
- Why do you call mach64_do_dma_flush( dev_priv ) in COMMIT_RING?
- In general I have the idea that we are bookkeeping to much redundant
variables related with the ring, and that's prone to bugs.
...but I did found something that annoyed the most:
- I want to play with the nice stuff you've been making! Can you commit,
plz! Pretty please! ;-)
José Fonseca
_______________________________________________________________
Don't miss the 2002 Sprint PCS Application Developer's Conference
August 25-28 in Las Vegas -- http://devcon.sprintpcs.com/adp/index.cfm
_______________________________________________
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel
