On Tue, Jan 27, 2015 at 12:59:15PM +0000, Daniel Stone wrote: > Hi, > > On 23 January 2015 at 12:42, Gustavo Padovan <gust...@padovan.org> wrote: > > void exynos_drm_crtc_finish_pageflip(struct drm_device *dev, int pipe) > > { > > struct exynos_drm_private *dev_priv = dev->dev_private; > > - struct drm_pending_vblank_event *e, *t; > > struct drm_crtc *drm_crtc = dev_priv->crtc[pipe]; > > struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(drm_crtc); > > - unsigned long flags; > > > > - spin_lock_irqsave(&dev->event_lock, flags); > > + if (exynos_crtc->event) { > > > > - list_for_each_entry_safe(e, t, &dev_priv->pageflip_event_list, > > - base.link) { > > - /* if event's pipe isn't same as crtc then ignore it. */ > > - if (pipe != e->pipe) > > - continue; > > - > > - list_del(&e->base.link); > > - drm_send_vblank_event(dev, -1, e); > > + spin_lock_irq(&dev->event_lock); > > + drm_send_vblank_event(dev, -1, exynos_crtc->event); > > drm_vblank_put(dev, pipe); > > - atomic_set(&exynos_crtc->pending_flip, 0); > > wake_up(&exynos_crtc->pending_flip_queue); > > - } > > + spin_unlock_irq(&dev->event_lock); > > > > - spin_unlock_irqrestore(&dev->event_lock, flags); > > + exynos_crtc->event = NULL; > > + } > > } > > Doesn't this need to hold the CRTC lock to not race with, e.g, the > page_flip caller? This gets called from IRQ context though, so might > need conversion to soft-IRQ to do so, or another per-CRTC spinlock > just to protect the event.
dev->even_lock should be enough, but I suspect that lock also protects exynos_crtc->event (at least that's the case in i915.ko). So would need to be moved out of the if block again. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html