On Mon, Jan 20, 2014 at 06:43:42PM +0100, Daniel Vetter wrote:
> 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.
We have the same problem already w/ underrun interrupts at least, no?
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
