On Mon, Jun 16, 2014 at 04:19:30PM +0300, Imre Deak wrote:
> On Mon, 2014-06-16 at 12:30 +0100, [email protected] wrote:
> > From: Oscar Mateo <[email protected]>
> > 
> > Otherwise, we might receive a new interrupt before we have time to
> > ack the first one, eventually missing it.
> > 
> > Notice that, before clearing a port-sourced interrupt in the IIR, the
> > corresponding interrupt source status in the PORT_HOTPLUG_STAT must be
> > cleared.
> > 
> > Spotted by Bob Beckett <[email protected]>.
> > 
> > Signed-off-by: Oscar Mateo <[email protected]>
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 61 
> > +++++++++++++++++++++++------------------
> >  1 file changed, 35 insertions(+), 26 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c 
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index 4439e2d..9d381cc 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1813,26 +1813,28 @@ static void i9xx_hpd_irq_handler(struct drm_device 
> > *dev)
> >     struct drm_i915_private *dev_priv = dev->dev_private;
> >     u32 hotplug_status = I915_READ(PORT_HOTPLUG_STAT);
> >  
> > -   if (IS_G4X(dev)) {
> > -           u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_G4X;
> > +   if (hotplug_status) {
> > +           I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
> > +           /*
> > +            * Make sure hotplug status is cleared before we clear IIR, or 
> > else we
> > +            * may miss hotplug events.
> > +            */
> > +           POSTING_READ(PORT_HOTPLUG_STAT);
> >  
> > -           intel_hpd_irq_handler(dev, hotplug_trigger, hpd_status_g4x);
> > -   } else {
> > -           u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_I915;
> > +           if (IS_G4X(dev)) {
> > +                   u32 hotplug_trigger = hotplug_status & 
> > HOTPLUG_INT_STATUS_G4X;
> >  
> > -           intel_hpd_irq_handler(dev, hotplug_trigger, hpd_status_i915);
> > -   }
> > +                   intel_hpd_irq_handler(dev, hotplug_trigger, 
> > hpd_status_g4x);
> > +           } else {
> > +                   u32 hotplug_trigger = hotplug_status & 
> > HOTPLUG_INT_STATUS_I915;
> >  
> > -   if ((IS_G4X(dev) || IS_VALLEYVIEW(dev)) &&
> > -       hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X)
> > -           dp_aux_irq_handler(dev);
> > +                   intel_hpd_irq_handler(dev, hotplug_trigger, 
> > hpd_status_i915);
> > +           }
> >  
> > -   I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
> > -   /*
> > -    * Make sure hotplug status is cleared before we clear IIR, or else we
> > -    * may miss hotplug events.
> > -    */
> > -   POSTING_READ(PORT_HOTPLUG_STAT);
> > +           if ((IS_G4X(dev) || IS_VALLEYVIEW(dev)) &&
> > +               hotplug_status & DP_AUX_CHANNEL_MASK_INT_STATUS_G4X)
> > +                   dp_aux_irq_handler(dev);
> > +   }
> >  }
> >  
> >  static irqreturn_t valleyview_irq_handler(int irq, void *arg)
> > @@ -1843,29 +1845,36 @@ static irqreturn_t valleyview_irq_handler(int irq, 
> > void *arg)
> >     irqreturn_t ret = IRQ_NONE;
> >  
> >     while (true) {
> > -           iir = I915_READ(VLV_IIR);
> >             gt_iir = I915_READ(GTIIR);
> >             pm_iir = I915_READ(GEN6_PMIIR);
> > +           iir = I915_READ(VLV_IIR);
> >  
> >             if (gt_iir == 0 && pm_iir == 0 && iir == 0)
> >                     goto out;
> >  
> > -           ret = IRQ_HANDLED;
> > +           if (gt_iir)
> > +                   I915_WRITE(GTIIR, gt_iir);
> >  
> > -           snb_gt_irq_handler(dev, dev_priv, gt_iir);
> > +           if (pm_iir)
> > +                   I915_WRITE(GEN6_PMIIR, pm_iir);
> >  
> > -           valleyview_pipestat_irq_handler(dev, iir);
> > +           if (iir) {
> > +                   /* Consume port. Then clear IIR or we'll miss events */
> > +                   if (iir & I915_DISPLAY_PORT_INTERRUPT)
> > +                           i9xx_hpd_irq_handler(dev);
> > +                   I915_WRITE(VLV_IIR, iir);
> > +           }
> >  
> > -           /* Consume port.  Then clear IIR or we'll miss events */
> > -           if (iir & I915_DISPLAY_PORT_INTERRUPT)
> > -                   i9xx_hpd_irq_handler(dev);
> > +           ret = IRQ_HANDLED;
> > +
> > +           if (gt_iir)
> > +                   snb_gt_irq_handler(dev, dev_priv, gt_iir);
> >  
> >             if (pm_iir)
> >                     gen6_rps_irq_handler(dev_priv, pm_iir);
> >  
> > -           I915_WRITE(GTIIR, gt_iir);
> > -           I915_WRITE(GEN6_PMIIR, pm_iir);
> > -           I915_WRITE(VLV_IIR, iir);
> > +           if (iir)
> > +                   valleyview_pipestat_irq_handler(dev, iir);
> 
> Afaik the pipe underrun flag handled in
> valleyview_pipestat_irq_handler() is not signaled in IIR, although bspec
> is rather unclear about this.

Pipe underrun isn't signalling the top-level interrupt and we also can't
mask it in any way. Hence the funny logic.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to