On Thu, 30 May 2002, José Fonseca wrote:
> 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) :
I've fixed one bug already that was related to the ring tail being left
pre-incremented (the table_end I had before wasn't). Some of my problems
were related to not flushing on swaps (flickering on the quake menu), but
this may not be a new problem. The bug that remains is that there are
some rendering problems, seemingly random garbage polygons here and there
in quake3.
Another thing I realized (not related to these changes) was a problem with
moving a window leaving frames behind. We need to make sure DMA is
quiescent in the XAA sync (flush before waiting for idle) so that there
aren't frames left in the queue with old clipping coords. Also the
Init/MoveBuffer functions in atidri.c aren't implemented yet (this will
require getting XAA working).
> - 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 above code and the mb()'s are just quick hacking and testing. I
always assumed that flush_write_combine referred to the mtrr for the AGP
aperture on x86, which is a kind of write cache, right?
> - 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.
I was thinking about that, but it seemed cleaner to have a single argument
for OUT_RING. The ADD_BUF_TO_RING is more similar to what you're
suggesting: we could have a parameter or a register and host-data version.
The ring macros come from the r128 driver and it was a quick way to get
started, we can always tweak this stuff more to fit our needs.
> - ADVANCE_RING and COMMIT_RING should be one.
The reason for separating those was mainly because the dispatch function
needs to advance the ring, but not "commit" it and mess with the end of
list flags. Also, we may need to have multiple BEGIN/OUT/ADVANCE blocks
before a COMMIT, like when emmitting state and then emmiting a vertex
buffer. I think we should only have one commit per ioctl, it's a waste to
move the end of list flag more that once if we don't have to.
> - Why do you call mach64_do_dma_flush( dev_priv ) in COMMIT_RING?
Again, that was just from my hacking/testing.
> - In general I have the idea that we are bookkeeping to much redundant
> variables related with the ring, and that's prone to bugs.
>
Are you just referring to the end of list flag stuff here, or were there
other redundancies that you saw?
> ...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! ;-)
OK, I'll try to do this soon. I was not feeling well today so I didn't
get much done.
--
Leif Delgass
http://www.retinalburn.net
_______________________________________________________________
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
