[Intel-gfx] [PATCH v2 3/5] drm/i915: Make sprite updates atomic

[Daniel Vetter]

On Mon, Jan 20, 2014 at 5:56 PM, Ville Syrjälä
<ville.syrjala at linux.intel.com> 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