| Message ID | 1385152524-3130-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 22 Nov 2013 20:35:24 +0000 Chris Wilson <chris@chris-wilson.co.uk> wrote: > I believe, and an evening of i-g-t, that our original workaround for the > missed interrupts on Sandybridge, that of holding forcewake whilst we > wait for an interrupts, is no longer required. This leaves us dependent > on the second workaround of forcing an UC read of the ACTHD before > reading back the seqno from the snooped HWS. Dropping the forcewake > should allow us to conserve a little power, not much as the GPU is meant > to be busy whilst we wait for it! > > Cc: Jesse Barnes <jbarnes@virtuousgeek.org> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/intel_ringbuffer.c | 7 ------- > 1 file changed, 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 80893af4062b..6b121ba9d3f7 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -1030,11 +1030,6 @@ gen6_ring_get_irq(struct intel_ring_buffer *ring) > if (!dev->irq_enabled) > return false; > > - /* It looks like we need to prevent the gt from suspending while waiting > - * for an notifiy irq, otherwise irqs seem to get lost on at least the > - * blt/bsd rings on ivb. */ > - gen6_gt_force_wake_get(dev_priv); > - > spin_lock_irqsave(&dev_priv->irq_lock, flags); > if (ring->irq_refcount++ == 0) { > if (HAS_L3_DPF(dev) && ring->id == RCS) > @@ -1066,8 +1061,6 @@ gen6_ring_put_irq(struct intel_ring_buffer *ring) > ilk_disable_gt_irq(dev_priv, ring->irq_enable_mask); > } > spin_unlock_irqrestore(&dev_priv->irq_lock, flags); > - > - gen6_gt_force_wake_put(dev_priv); > } > > static bool Yay! This is just what I was testing on BYT today, and it works there too (and gives me tons more RC6 residency). Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
On Fri, Nov 22, 2013 at 12:46:40PM -0800, Jesse Barnes wrote: > On Fri, 22 Nov 2013 20:35:24 +0000 > Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > I believe, and an evening of i-g-t, that our original workaround for the > > missed interrupts on Sandybridge, that of holding forcewake whilst we > > wait for an interrupts, is no longer required. This leaves us dependent > > on the second workaround of forcing an UC read of the ACTHD before > > reading back the seqno from the snooped HWS. Dropping the forcewake > > should allow us to conserve a little power, not much as the GPU is meant > > to be busy whilst we wait for it! > > > > Cc: Jesse Barnes <jbarnes@virtuousgeek.org> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > drivers/gpu/drm/i915/intel_ringbuffer.c | 7 ------- > > 1 file changed, 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > > index 80893af4062b..6b121ba9d3f7 100644 > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > > @@ -1030,11 +1030,6 @@ gen6_ring_get_irq(struct intel_ring_buffer *ring) > > if (!dev->irq_enabled) > > return false; > > > > - /* It looks like we need to prevent the gt from suspending while waiting > > - * for an notifiy irq, otherwise irqs seem to get lost on at least the > > - * blt/bsd rings on ivb. */ > > - gen6_gt_force_wake_get(dev_priv); > > - > > spin_lock_irqsave(&dev_priv->irq_lock, flags); > > if (ring->irq_refcount++ == 0) { > > if (HAS_L3_DPF(dev) && ring->id == RCS) > > @@ -1066,8 +1061,6 @@ gen6_ring_put_irq(struct intel_ring_buffer *ring) > > ilk_disable_gt_irq(dev_priv, ring->irq_enable_mask); > > } > > spin_unlock_irqrestore(&dev_priv->irq_lock, flags); > > - > > - gen6_gt_force_wake_put(dev_priv); > > } > > > > static bool > > Yay! This is just what I was testing on BYT today, and it works there > too (and gives me tons more RC6 residency). Risky imo, since I see missed interrupts sporadically on all my gen6+ machines, and we don't have any solid testcase in igt that readily hits them. Expect a swift strike with the revert hammer if this causes trouble ;-) > Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> Queued for -next, thanks for the patch. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 80893af4062b..6b121ba9d3f7 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1030,11 +1030,6 @@ gen6_ring_get_irq(struct intel_ring_buffer *ring) if (!dev->irq_enabled) return false; - /* It looks like we need to prevent the gt from suspending while waiting - * for an notifiy irq, otherwise irqs seem to get lost on at least the - * blt/bsd rings on ivb. */ - gen6_gt_force_wake_get(dev_priv); - spin_lock_irqsave(&dev_priv->irq_lock, flags); if (ring->irq_refcount++ == 0) { if (HAS_L3_DPF(dev) && ring->id == RCS) @@ -1066,8 +1061,6 @@ gen6_ring_put_irq(struct intel_ring_buffer *ring) ilk_disable_gt_irq(dev_priv, ring->irq_enable_mask); } spin_unlock_irqrestore(&dev_priv->irq_lock, flags); - - gen6_gt_force_wake_put(dev_priv); } static bool
I believe, and an evening of i-g-t, that our original workaround for the missed interrupts on Sandybridge, that of holding forcewake whilst we wait for an interrupts, is no longer required. This leaves us dependent on the second workaround of forcing an UC read of the ACTHD before reading back the seqno from the snooped HWS. Dropping the forcewake should allow us to conserve a little power, not much as the GPU is meant to be busy whilst we wait for it! Cc: Jesse Barnes <jbarnes@virtuousgeek.org> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_ringbuffer.c | 7 ------- 1 file changed, 7 deletions(-)