From mboxrd@z Thu Jan 1 00:00:00 1970 From: Imre Deak Subject: Re: [PATCH 3/6] drm/i915: change power_well->lock to be mutex Date: Wed, 16 Oct 2013 19:31:29 +0300 Message-ID: <1381941089.22425.29.camel@intelbox> References: <1381933553-19529-1-git-send-email-imre.deak@intel.com> <1381933553-19529-4-git-send-email-imre.deak@intel.com> Reply-To: imre.deak@intel.com Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0966983818==" Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 08CD1E83D3 for ; Wed, 16 Oct 2013 09:31:32 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Paulo Zanoni Cc: Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org --===============0966983818== Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-GGw14rGzvQ7zQbS3O3wj" --=-GGw14rGzvQ7zQbS3O3wj Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 2013-10-16 at 13:19 -0300, Paulo Zanoni wrote: > 2013/10/16 Imre Deak : > > There is no hard need for this to be a spin lock, as we don't take thes= e > > 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 conver= t > > the spin lock to be a mutex. >=20 > 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 >=20 > > > > Signed-off-by: Imre Deak > > --- > > 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/i91= 5_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/int= el_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 *d= ev, > > 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 *d= ev, > > 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 =3D &dev_priv->power_well; > > > > hsw_pwr->device =3D dev; > > - spin_lock_init(&hsw_pwr->lock); > > + mutex_init(&hsw_pwr->lock); > > hsw_pwr->count =3D 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_de= vice *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@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >=20 >=20 >=20 --=-GGw14rGzvQ7zQbS3O3wj Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) iQEcBAABAgAGBQJSXr9hAAoJEORIIAnNuWDFbL8H/jJ9/HBvOb/ig0GL1hqPBcBt GQN+AqTL+znwt1wQSEFpqoTqq5sOfq3ZTh1s2HKXNQVh/H5Ujs3NxgCRcFrO/5rU KLVvPt3gWGBUSxqOKiimVNhiZZPmCKiHURWOifuiXV6VEl0qi2rbegg8s0WBBvbz 2egznqoR+UNfqKOaqH2uMf83QxqsPUh9d+vAzwP3z/bhZOmikQZnX8w4D0heSpsm 3Uapd/W5NjM+G4aAMTBsGClXWVi+QAOv25sS3SG7WGyF/rlUzvcpx0Vkm/6VKtSR Mi5JjVA1bVfcnyuBY0tU+baJJXge4ER8CMWWrH45qfrHJvYn0wpGp5cBP1H3Qj0= =z+tF -----END PGP SIGNATURE----- --=-GGw14rGzvQ7zQbS3O3wj-- --===============0966983818== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============0966983818==--