during the review of the fix for locks problems in drm_wait_vblank, a couple of false concerns were raised about how the drm_vblank_get and drm_vblank_put are used in this function; it turned out that the code is correct and that it cannot be simplified add a few comments to explain non-obvious flows in the code, to prevent "false alarms" in the future Signed-off-by: Ilija Hadzic <ihadzic@xxxxxxxxxxxxxxxxxxxxxx> --- drivers/gpu/drm/drm_irq.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index c8b4da8..e9dd19d 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -1065,6 +1065,10 @@ out: return ret; } +/* must acquire vblank reference count (call drm_vblank_get) */ +/* before calling this function; the matching drm_vblank_put */ +/* will either be issued here or in drm_handle_vblank_events */ +/* after the vblank is signaled */ static int drm_queue_vblank_event(struct drm_device *dev, int pipe, union drm_wait_vblank *vblwait, struct drm_file *file_priv) @@ -1124,6 +1128,9 @@ static int drm_queue_vblank_event(struct drm_device *dev, int pipe, trace_drm_vblank_event_delivered(current->pid, pipe, vblwait->request.sequence); } else { + /* can't call drm_vblank_put here because interrupt */ + /* must remain enabled until the event occurs */ + /* drm_handle_vblank_events will do this for us */ list_add_tail(&e->base.link, &dev->vblank_event_list); vblwait->reply.sequence = vblwait->request.sequence; } @@ -1215,6 +1222,7 @@ int drm_wait_vblank(struct drm_device *dev, void *data, if (flags & _DRM_VBLANK_EVENT) { spin_unlock_irqrestore(&dev->vbl_lock, irqflags); + /* drm_queue_vblank_event() will call drm_vblank_put() */ return drm_queue_vblank_event(dev, crtc, vblwait, file_priv); } -- 1.7.7 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel