Re: [Dri-devel] Texture management (Was: Client context uploads...)

[Leif D. <lde...@re...>]

On Mon, 13 May 2002, Ian Romanick wrote:

> On Sun, May 12, 2002 at 06:18:58PM -0400, Leif Delgass wrote:
> 
> > In working on AGP texturing for mach64, I'm starting from the Rage128
> > code, which seems to have some problems (though the texture aging problem
> > could affect other drivers).  My understanding is that textures in the
> > global LRU are marked as "used" and aged so that placeholders can be
> > inserted in a context's local LRU when another context steals its texture
> > memory.  The problem is that nowhere are these texture regions released by
> > the context using them.  The global LRU is only reset when the heap is
> > full.  So the heap has to fill up before placeholders begin to get swapped
> > out.  I've seen this when running multiple contexts at once, or repeatedly
> > starting, stopping, and restarting a single app.  This isn't a huge
> > problem with a single heap, but with an AGP heap it means that card memory
> > is effectively leaked.  Once the card memory global LRU is nearly filled
> > in the sarea with regions marked as "used", newly started apps will start
> > out only using AGP mem (with the r128 algorithm).  Only if the app uses
> > enough mem. to fill AGP will it start to swap out the placeholders from
> > the local LRU and use card memory.
> 
> If this is true, then I believe it is a bug in the r128 driver.  IIRC, all
> of the space in the global texture heaps is freed when the context is
> destroyed.  This is the way that the Radeon and MGA drivers work.  The r128
> driver follows the same model so it is /intended/ to work the same.

It looks to me like this is a bug in all the drivers.  Try grepping for
'in_use' in the Mesa drivers.  I don't see anywhere in _any_ of the
drivers where a context sets in_use to zero in a texture region in the
global heap.  The local heap is destroyed when the context is destroyed,
but it doesn't touch the global heap.  There are only two places where the
global LRU is modified.  One is in UpdateTexLRU where the region is marked
as in_use, aged, and moved to the head of the list.  The second is in
ResetGlobalLRU, where the list is (re)built and ages are reset.  The Reset
function is only called from AgeTextures when the heap is full (which it
can detect because the Reset function leaves out one region when
rebuilding the list), which also forces TexturesGone to swap out
everything in the heap, including all of the placeholders.  To see this
happening, you can enable the Print[Global,Local]LRU functions in
UpdateTexLRU, restart X, then repeatedly start, stop and restart the
texenv Mesa demo.  Each time you restart, you'll see a new placeholder in
the local LRU if the problem is there.  If I'm right, this should be
happening in all the drivers.  Can someone test this on the Radeon driver?

> > One possible solution I'm playing with would be to use a context
> > identifier on texture regions in the global LRU rather than a boolean
> > "in_use" (similar to the ctxOwner identifier used for marking the last
> > owner of the sarea's state information).  Then when a context swaps out or
> > destroys textures, it can free regions that it owns from the global LRU
> > and age them so that other contexts will swap out their corresponding
> > placeholders.  The downside is an increased penalty for swapping textures.  
> > Another problem is how to reclaim "leaked" regions when an app doesn't
> > exit normally?
> 
> This is not needed.  When texture space is freed in the global heap by one
> context, the other contexts will see that the state of those blocks in the
> global heap has changed from owned to free.

This is assuming that the space is actually freed.  Maybe it wouldn't be 
necessary if you only swap or destroy textures when holding the lock.  Is 
DestroyContext called while holding the lock?
 
> > I've also found what looks to me like a bug in the Rage128 driver in
> > UploadTexImages.  The beginning of the function does this:
> > 
> >    /* Choose the heap appropriately */
> >    heap = t->heap = R128_CARD_HEAP;
> >    if ( !rmesa->r128Screen->IsPCI &&
> >         t->totalSize > rmesa->r128Screen->texSize[heap] ) {
> >       heap = t->heap = R128_AGP_HEAP;
> >    }
> > 
> >    /* Do we need to eject LRU texture objects? */
> >    if ( !t->memBlock ) {
> > 
> > 	Find a memBlock, swapping and/or changing heaps if necessary...
> > 
> >    }
> > 
> >    Update LRU and upload dirty images...
> > 
> > The problem I see here is that setting t->heap before checking for an
> > existing memBlock could potentially lead to a situation where t->heap !=
> > t->memBlock->heap.  So in my code I've deferred changing t->heap = heap to
> > inside the 'if' block where we know there is no memBlock.  Again this
> > situation can only occur if there is an AGP heap.  Is there a reason for
> > this behavior in the Rage128 code that I'm missing, or is this a bug?
> 
> The MGA driver does something similar.  The idea is sound, but there may be
> implementation bugs.  Basically, the driver will start with heap 0 (i.e., the
> on-card memory) and all the remaining heaps.  The if-block in the above
> code, basically, tells the drive to not bother checking on-card memory and
> start with the AGP heap.  
> 
> Here it looks like it's telling the driver to not try to allocate from
> on-card memory if the texture is larger than will fit on the card if all of
> the memory is available.  This is a good heuristic.  Without it, trying to
> allocate a very large texture would cause ALL of the other textures to get
> kicked out of on-card memory AND still have the allocation FAIL. :)

I understand the algorithm, that's not the problem -- it's the _second_ if
block.  My point is that if, at the beginning of the function, the texture
is already allocated on the AGP heap (t->heap == t->memBlock->heap == AGP)
and is small enough to fit card memory, t->heap is changed to the CARD
heap unconditionally by the first line.  However, since the texture
already has a memBlock in AGP, the second if block is not entered and
memBlock remains on the AGP heap, so t->heap == CARD, but
t->memBlock->heap == AGP.  I'm saying that t->heap should only be changed
to the CARD heap if it doesn't already have a memBlock (i.e. it's new or
has been swapped out).

-- 
Leif Delgass 
http://www.retinalburn.net