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

[Daniel Vetter <daniel@ffwll.ch>]

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
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch