The DRI drawable spinlock

[Keith Whitwell]

It's a horrible hack, basically.  I used to understand it but now no 
longer want to.  I think the original DRI documentation will have some 
information on it.

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
> xorg at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/xorg
>