| Message ID | 20171122222510.22627-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 22/11/2017 22:25, Chris Wilson wrote: > Since commit 6060b6aec03c ("drm/i915/pmu: Add RC6 residency metrics"), > vlv_residency_raw() may be called from an irq-disabled context (via perf > event sampling on remote cpu). As such, we can no longer assume that we > are called from process context and must save/restore the irq state for > the spinlock. > > Fixes: 6060b6aec03c ("drm/i915/pmu: Add RC6 residency metrics") > Testcase: igt/perf_pmu/other-init-3 > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/intel_pm.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 7d41aad79166..03d67d8ab647 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -9396,12 +9396,13 @@ static u64 vlv_residency_raw(struct drm_i915_private *dev_priv, > const i915_reg_t reg) > { > u32 lower, upper, tmp; > + unsigned long flags; > int loop = 2; > > /* The register accessed do not need forcewake. We borrow > * uncore lock to prevent concurrent access to range reg. > */ > - spin_lock_irq(&dev_priv->uncore.lock); > + spin_lock_irqsave(&dev_priv->uncore.lock, flags); > > /* vlv and chv residency counters are 40 bits in width. > * With a control bit, we can choose between upper or lower > @@ -9432,7 +9433,7 @@ static u64 vlv_residency_raw(struct drm_i915_private *dev_priv, > * now. > */ > > - spin_unlock_irq(&dev_priv->uncore.lock); > + spin_unlock_irqrestore(&dev_priv->uncore.lock, flags); > > return lower | (u64)upper << 8; > } > @@ -9451,7 +9452,6 @@ u64 intel_rc6_residency_ns(struct drm_i915_private *dev_priv, > mul = 1000000; > div = dev_priv->czclk_freq; > time_hw = vlv_residency_raw(dev_priv, reg); > - > } else { > /* 833.33ns units on Gen9LP, 1.28us elsewhere. */ > if (IS_GEN9_LP(dev_priv)) { > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
Quoting Tvrtko Ursulin (2017-11-23 06:56:49) > > On 22/11/2017 22:25, Chris Wilson wrote: > > Since commit 6060b6aec03c ("drm/i915/pmu: Add RC6 residency metrics"), > > vlv_residency_raw() may be called from an irq-disabled context (via perf > > event sampling on remote cpu). As such, we can no longer assume that we > > are called from process context and must save/restore the irq state for > > the spinlock. > > > > Fixes: 6060b6aec03c ("drm/i915/pmu: Add RC6 residency metrics") > > Testcase: igt/perf_pmu/other-init-3 > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > drivers/gpu/drm/i915/intel_pm.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > index 7d41aad79166..03d67d8ab647 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -9396,12 +9396,13 @@ static u64 vlv_residency_raw(struct drm_i915_private *dev_priv, > > const i915_reg_t reg) > > { > > u32 lower, upper, tmp; > > + unsigned long flags; > > int loop = 2; > > > > /* The register accessed do not need forcewake. We borrow > > * uncore lock to prevent concurrent access to range reg. > > */ > > - spin_lock_irq(&dev_priv->uncore.lock); > > + spin_lock_irqsave(&dev_priv->uncore.lock, flags); > > > > /* vlv and chv residency counters are 40 bits in width. > > * With a control bit, we can choose between upper or lower > > @@ -9432,7 +9433,7 @@ static u64 vlv_residency_raw(struct drm_i915_private *dev_priv, > > * now. > > */ > > > > - spin_unlock_irq(&dev_priv->uncore.lock); > > + spin_unlock_irqrestore(&dev_priv->uncore.lock, flags); > > > > return lower | (u64)upper << 8; > > } > > @@ -9451,7 +9452,6 @@ u64 intel_rc6_residency_ns(struct drm_i915_private *dev_priv, > > mul = 1000000; > > div = dev_priv->czclk_freq; > > time_hw = vlv_residency_raw(dev_priv, reg); > > - > > } else { > > /* 833.33ns units on Gen9LP, 1.28us elsewhere. */ > > if (IS_GEN9_LP(dev_priv)) { > > > > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Ta, pushed this patch. Thanks for the review, -Chris
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 7d41aad79166..03d67d8ab647 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -9396,12 +9396,13 @@ static u64 vlv_residency_raw(struct drm_i915_private *dev_priv, const i915_reg_t reg) { u32 lower, upper, tmp; + unsigned long flags; int loop = 2; /* The register accessed do not need forcewake. We borrow * uncore lock to prevent concurrent access to range reg. */ - spin_lock_irq(&dev_priv->uncore.lock); + spin_lock_irqsave(&dev_priv->uncore.lock, flags); /* vlv and chv residency counters are 40 bits in width. * With a control bit, we can choose between upper or lower @@ -9432,7 +9433,7 @@ static u64 vlv_residency_raw(struct drm_i915_private *dev_priv, * now. */ - spin_unlock_irq(&dev_priv->uncore.lock); + spin_unlock_irqrestore(&dev_priv->uncore.lock, flags); return lower | (u64)upper << 8; } @@ -9451,7 +9452,6 @@ u64 intel_rc6_residency_ns(struct drm_i915_private *dev_priv, mul = 1000000; div = dev_priv->czclk_freq; time_hw = vlv_residency_raw(dev_priv, reg); - } else { /* 833.33ns units on Gen9LP, 1.28us elsewhere. */ if (IS_GEN9_LP(dev_priv)) {
Since commit 6060b6aec03c ("drm/i915/pmu: Add RC6 residency metrics"), vlv_residency_raw() may be called from an irq-disabled context (via perf event sampling on remote cpu). As such, we can no longer assume that we are called from process context and must save/restore the irq state for the spinlock. Fixes: 6060b6aec03c ("drm/i915/pmu: Add RC6 residency metrics") Testcase: igt/perf_pmu/other-init-3 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_pm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)