[PATCH 3/3] drm: do not sleep on vblank while holding a mutex

Ilija Hadzic ihadzic at research.bell-labs.com
Thu Oct 27 21:36:46 PDT 2011



On Thu, 27 Oct 2011, Ilija Hadzic wrote:

>
>> While locking at the code I also noticed that wait_vblank calls
>> drm_vblank_get, but not always drm_vblank_put. The code is correct, the
>> missing vblank_put is hidden in drm_queue_vblank_event. Can you also write
>> the patch to move that around to not trip up reviewers the next time
>> around?
>> 
>
> Actually, there is something fishy here. Look at the 'else' branch under the 
> 'if ((seq - vblwait->request.sequence) <= (1 << 23))' at the end of the 
> drm_queue_vblank_event. It doesn't have the 'drm_vblank_put'. It looks wrong 
> to me, but I need to first convince myself that there is not some other 
> obscure place where drm_vblank_put is accounted for. If that branch is really 
> missing a drm_vblank_put, then it will be easy pull the drm_vblank_put out. 
> Otherwise, it will be another knot to untangle.
>
>

OK, I checked this. Although the code is a little hard to read, it is done 
the way it is with the reason. Whenever the drm_queue_vblank_event and 
that 'else' branch is caught, the drm_vblank_put should *not* be called.
The reason is that the relevant vblank has not come in yet, so the 
reference must remain up so that the vblank interrupts are not disabled 
until the event occurs.

The seemingly missing drm_vblank_put will be accounted for in 
drm_handle_vblank_events.

So I should not move anything around. If anything, this should be 
annotated with comments to explain what's going on.

-- Ilija



More information about the dri-devel mailing list