On Wed, 2013-10-16 at 13:19 -0300, Paulo Zanoni wrote:
> 2013/10/16 Imre Deak <imre.deak@xxxxxxxxx>:
> > There is no hard need for this to be a spin lock, as we don't take these
> > locks in irq context from anywhere. An upcoming patch will add calls to
> > punit read/write functions from within regions protected by this lock
> > and those functions need a mutex in turn. As a solution for that convert
> > the spin lock to be a mutex.
>
> Not even from snd_hda_intel? Did we check this?
Yea, at least I tried to check this at all call sites and haven't found
any place where they request the power well in atomic context. In any
case I think it's a design problem if they do ..
--Imre
>
> >
> > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 2 +-
> > drivers/gpu/drm/i915/intel_pm.c | 26 +++++++++++++-------------
> > 2 files changed, 14 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index ca05f3a..e4354dd 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -910,7 +910,7 @@ struct intel_ilk_power_mgmt {
> > /* Power well structure for haswell */
> > struct i915_power_well {
> > struct drm_device *device;
> > - spinlock_t lock;
> > + struct mutex lock;
> > /* power well enable/disable usage count */
> > int count;
> > int i915_request;
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 57d08a2..f7363a8 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -5476,9 +5476,9 @@ void intel_display_power_get(struct drm_device *dev,
> > if (is_always_on_power_domain(dev, domain))
> > return;
> >
> > - spin_lock_irq(&power_well->lock);
> > + mutex_lock(&power_well->lock);
> > __intel_power_well_get(power_well);
> > - spin_unlock_irq(&power_well->lock);
> > + mutex_unlock(&power_well->lock);
> > }
> >
> > void intel_display_power_put(struct drm_device *dev,
> > @@ -5493,9 +5493,9 @@ void intel_display_power_put(struct drm_device *dev,
> > if (is_always_on_power_domain(dev, domain))
> > return;
> >
> > - spin_lock_irq(&power_well->lock);
> > + mutex_lock(&power_well->lock);
> > __intel_power_well_put(power_well);
> > - spin_unlock_irq(&power_well->lock);
> > + mutex_unlock(&power_well->lock);
> > }
> >
> > static struct i915_power_well *hsw_pwr;
> > @@ -5506,9 +5506,9 @@ void i915_request_power_well(void)
> > if (WARN_ON(!hsw_pwr))
> > return;
> >
> > - spin_lock_irq(&hsw_pwr->lock);
> > + mutex_lock(&hsw_pwr->lock);
> > __intel_power_well_get(hsw_pwr);
> > - spin_unlock_irq(&hsw_pwr->lock);
> > + mutex_unlock(&hsw_pwr->lock);
> > }
> > EXPORT_SYMBOL_GPL(i915_request_power_well);
> >
> > @@ -5518,9 +5518,9 @@ void i915_release_power_well(void)
> > if (WARN_ON(!hsw_pwr))
> > return;
> >
> > - spin_lock_irq(&hsw_pwr->lock);
> > + mutex_lock(&hsw_pwr->lock);
> > __intel_power_well_put(hsw_pwr);
> > - spin_unlock_irq(&hsw_pwr->lock);
> > + mutex_unlock(&hsw_pwr->lock);
> > }
> > EXPORT_SYMBOL_GPL(i915_release_power_well);
> >
> > @@ -5531,7 +5531,7 @@ int i915_init_power_well(struct drm_device *dev)
> > hsw_pwr = &dev_priv->power_well;
> >
> > hsw_pwr->device = dev;
> > - spin_lock_init(&hsw_pwr->lock);
> > + mutex_init(&hsw_pwr->lock);
> > hsw_pwr->count = 0;
> >
> > return 0;
> > @@ -5553,7 +5553,7 @@ void intel_set_power_well(struct drm_device *dev, bool enable)
> > if (!i915_disable_power_well && !enable)
> > return;
> >
> > - spin_lock_irq(&power_well->lock);
> > + mutex_lock(&power_well->lock);
> >
> > /*
> > * This function will only ever contribute one
> > @@ -5572,7 +5572,7 @@ void intel_set_power_well(struct drm_device *dev, bool enable)
> > __intel_power_well_put(power_well);
> >
> > out:
> > - spin_unlock_irq(&power_well->lock);
> > + mutex_unlock(&power_well->lock);
> > }
> >
> > static void intel_resume_power_well(struct drm_device *dev)
> > @@ -5583,9 +5583,9 @@ static void intel_resume_power_well(struct drm_device *dev)
> > if (!HAS_POWER_WELL(dev))
> > return;
> >
> > - spin_lock_irq(&power_well->lock);
> > + mutex_lock(&power_well->lock);
> > __intel_set_power_well(dev, power_well->count > 0);
> > - spin_unlock_irq(&power_well->lock);
> > + mutex_unlock(&power_well->lock);
> > }
> >
> > /*
> > --
> > 1.8.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
Attachment:
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
