On Mon, Jan 20, 2014 at 5:56 PM, Ville Syrjälä
<ville.syrjala@xxxxxxxxxxxxxxx> wrote:
>> > +{
>> > + struct drm_device *dev = crtc->dev;
>> > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> > + const struct drm_display_mode *mode = &intel_crtc->config.adjusted_mode;
>> > + enum pipe pipe = intel_crtc->pipe;
>> > + /* FIXME needs to be calibrated sensibly */
>> > + unsigned int min = mode->crtc_vblank_start - usecs_to_scanlines(mode, 100);
>> > + unsigned int max = mode->crtc_vblank_start - 1;
>> > + long timeout = msecs_to_jiffies_timeout(1);
>> > + unsigned int scanline;
>> > +
>> > + if (WARN_ON(drm_vblank_get(dev, pipe)))
>> > + return;
>> > +
>> > + local_irq_disable();
>> > +
>> > + intel_crtc->vbl_received = false;
>
> Now that you got me thinking about barriers again, I wonder if I should
> add an explicit compiler barrier here. The intel_get_crtc_scanline() call
> should act as a compiler barrier though, so it shouldn't be needed. So
> maybe I should add a comment here too?
This piece of code here was the actual reason I've asked for barrier
comments ;-) Ofc document the wake_up/wait_even barriers for the irq
write -> read here is also good, but this write here is imo the
crucial piece.
Also I think we should have a check here that the caller is holding
the crtc lock, to make sure that only one thread is using this
facility. Oh, and one more while I ponder this: We enable interrupt
processing before crtcs are fully set up, so chasing the pipe->crtc
mapping from the irq handling either needs to be done carefully (i.e.
a small analysis of why we won't ever get an vblank interrupt before
the crtc is set up) or needs to use something statically allocated in
dev_priv.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
