On 2002.05.31 02:53 Leif Delgass wrote:
> 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?
mmm.. I really don't know that much about mtrr. Documentation/mtrr.txt
says that it's indeed a write cache, but on the bus side, not
processor<->memory. So I don't see what mb(), which in case of the x86
arch is equal to rmb(), which just ensures that read operations are
completed. The idea I have is that the memmory barriers are only good to
force ordering in read/write operations to memmory, not buses..
> > - 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.
I see.
>
> > - 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?
I was referring to the amount of stuff that was on the ring structure, and
the amount of local variables in RING_LOCALS. But perhaps it's probably my
just my first impression to a unfamiliar code.
>
> > ...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.
No problem. I wish you get better soon.
Regards,
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
