I think the original motiviation was for the gamma (and actually lots of subsequent drivers), you end up encoding the window position and cliprects in the command stream. And this lock somehow protected those command streams from being submitted to hardware with out-of-date window positions.
In fact if you go right back, the DRM supported multiple software command stream where commands from the X server could be prioritized ahead of commands from 3d clients, so you could get the window-move blits being submitted to hardware ahead of 3d drawing commands to the old window position.
This created all sorts of amazing problems which have subsequently been avoided by simply submitting all commands to hardware in the order they are generated.
Right now, no driver uses the spinlock directly or explicitly, but they all use it when requesting their cliprects from the X server.
So basically the sequence goes:
Client gets dri lock
Client sees timestamp changed
Client wants to talk to the X server to get new cliprects, but because client is holding the lock, the X server is unable to respond to the client. So the client needs to release the lock to let the X server reply.
--> But! (some set of circumstances might apply here), so there's a need for this funny other lock, that the client grabs before it releases the main dri lock, to stop the cliprects getting changed again in the meantime.
I don't see why you couldn't just do something like
get lock
while (timestamp mismatch) {
release lock
request new cliprects and timestamp
get lock
}Note that is the contended case only. What's the worst that could happen - somebody's wizzing windows around and our 3d client sits in this loop for the duration. Note that the loop includes X server communication so it's not going to suck up the cpu or anything drastic.
In some respects it's like the code in the radeon DDX driver which uses a software path to perform depth buffer copying on window moves - well intentioned perhaps, but not actually useful or desirable.
Keith
Thomas Hellstrom wrote:
Hi!
Does anybody have a clear understanding of the drawable spinlock?
From my reading of code in the X server and dri_utilities.c it is ment to be used to stop anyone but the context holding the lock to touch the dri drawables in a way that would change their timestamp.
The X server has a very inefficient way of checking whether a client died while holding the drawable spinlock. It waits for 10 seconds and then grabs it by force.
Also the usage is dri_util.c is beyond my understanding. Basically, to lock and validate drawable info, the following happens:
get_heavyweight_lock;
while drawable_stamps_mismatch { release_heavyweight_lock; get_drawable_spinlock; //In dri_util.c do_some_minor_processing_that_can_be_done_elsewhere; release_drawable_spinlock; call_X_to_update_drawable_info; get_drawable_spinlock; //In driver. release_drawable_spinlock; }
Basically no driver seems to be using it for anything, except possibly the gamma driver, which I figure is outdated anyway?
I have found some use for it in XvMC clients: To run the scaling engine to render to a drawable without holding the heavyweight lock for prolonged periods, but I strongly dislike the idea of freezing the X server for 10 secs if the XvMC client accidently dies.
Proposed changes:
1). Could we replace the locking value (which now seems to be 1) with the context number | _DRM_LOCK_HELD. In this way the drm can detect when the drawable lock is held by a killed client and release it.
2). Could we replace the drawable_spinlock with a futex-like lock similar to what's used in the via drm to reserve certain chip functions. The purpose would be to sched_yield() if the lock is contended, with an immediate wakeup when the lock is released. This would not be backwards binary compatible with other drivers, but it seems no up-to-date drivers is using this lock anyway.
/Thomas
_______________________________________________ xorg mailing list [EMAIL PROTECTED] http://lists.freedesktop.org/mailman/listinfo/xorg
------------------------------------------------------- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click -- _______________________________________________ Dri-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/dri-devel
