Re: [PATCH 2/6] drm/exynos: track vblank events on a per crtc basis

On Tue, Jan 27, 2015 at 12:59:15PM +0000, Daniel Stone wrote:

Hi,

On 23 January 2015 at 12:42, Gustavo Padovan gustavo@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