On Wed, May 21, 2014 at 08:54:04AM +0100, Chris Wilson wrote:
> On Wed, May 21, 2014 at 01:09:58PM +0530, Arun R Murthy wrote:
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c 
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index 56edff3..f97f0fe 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1513,8 +1513,11 @@ static irqreturn_t valleyview_irq_handler(int irq, 
> > void *arg)
> >             spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> >  
> >             for_each_pipe(pipe) {
> > -                   if (pipe_stats[pipe] & 
> > PIPE_START_VBLANK_INTERRUPT_STATUS)
> > +                   if (pipe_stats[pipe] &
> > +                                   PIPE_START_VBLANK_INTERRUPT_STATUS) {
> >                             drm_handle_vblank(dev, pipe);
> > +                           wake_up_interruptible(&dev_priv->wait_vblank);
> 
> We now have intel_handle_vblank() so these chunks can be simplified.
> 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 4d4a0d9..e1eb564 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -757,12 +757,14 @@ enum transcoder intel_pipe_to_cpu_transcoder(struct 
> > drm_i915_private *dev_priv,
> >  static void g4x_wait_for_vblank(struct drm_device *dev, int pipe)
> >  {
> >     struct drm_i915_private *dev_priv = dev->dev_private;
> > -   u32 frame, frame_reg = PIPE_FRMCOUNT_GM45(pipe);
> > +   u32 vblank_cnt;
> >  
> > -   frame = I915_READ(frame_reg);
> > +   vblank_cnt = drm_vblank_count(dev, pipe);
> >  
> > -   if (wait_for(I915_READ_NOTRACE(frame_reg) != frame, 50))
> > -           DRM_DEBUG_KMS("vblank wait timed out\n");
> > +   /* TODO: get the vblank time dynamically or from platform data */
> > +   wait_event_interruptible_timeout(dev_priv->wait_vblank,
> > +                   (vblank_cnt != drm_vblank_count(dev, pipe)),
> > +                   msecs_to_jiffies(16));
> 
> Keep the ultimate timeout at 50 until you have evidence you can reduce
> it. And then it should be 2x vrefresh interval to be safe. However, you
> are likely still hitting the timeout as the vblank irq is not guaranteed
> to be enabled here. How safe calling drm_vblank_get() is during modeset
> I defer to Ville since he has just taken a pass over the whole mess.

The plan is to make drm_vblank_get() work until drm_vblank_off() has
been called. And when enabling, drm_vblank_get() will succeed only after
drm_vblank_on() has been called. The place where those should end up is
at the start/end of intel_crtc_{disable,enable}_planes(). So you have
access to vblank irqs while planes are getting enabled/disabled, but no
further since we can't guarantee their function once we start shutting
off pipes/ports/etc.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to