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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
