Re: GEM merging to master

[Eric A. <er...@an...>]

On Thu, 2008-06-12 at 17:44 +0200, Thomas Hellström wrote:
> Eric Anholt wrote:
> > We're getting close to ready to mark GEM on Intel as done.  We've got
> > one failing testcase that we isolated this week with interrupt handling,
> > and we've got a fix in testing that appears to be doing the job.
> >
> > Tomorrow I'm planning on merging the GEM code to master of all 3
> > repositories.  At that point, I'll cut a branch called drm-ttm in drm
> > with the existing interface and support.  After that I'm planning on the
> > following changes:
> >
> >   
> I'm not sure whether you're interested in a quick code review, but here 
> are some quick questions / comments.
> 
> 1) I'm not completely sure how you manage scanout buffers with GEM, but 
> am I correct in assuming that the X server maps the scanout buffers 
> through the GTT aperture, and that there are both physical and virtual 
> page aliases with different caching policy to these buffers?

Yes.

> 2) Delayed buffer delete. I might have missed something, but I see no 
> such functionality in the i915 gem implementation. (That means, the 
> thread deleting the last reference to a buffer object does must not 
> block waiting for the GPU to finish with it). If this functionality is 
> missing, the ring will drain while the client is waiting for destruction 
> of batch-buffers or texture images.

Closing your handle to a buffer buffer doesn't block, as if the buffer
is active (on the active or flushing lists), its refcount is still one.
Once the hardware's done with it, the move to the inactive queue
triggered from somebody waiting on a request to finish or the delayed
work proc will decrement the count to 0 and free it.

> 3) The i915_wait_request function is waiting in interruptible mode for 
> the GPU, and if it's hit by a signal (silken mouse movement) it will 
> return an -ERESTARTSYS to user-space but there doesn't seem to be any 
> code to take care of this and restart the system call?

Did you see some specific bugs with this?  The restarted system call
should work correctly.

-- 
Eric Anholt                             anholt@FreeBSD.org
er...@an...                         eri...@in...