From mboxrd@z Thu Jan 1 00:00:00 1970 From: Imre Deak Subject: Re: [PATCH 16/19] drm/i915: sanity check power well sw state against hw state Date: Tue, 18 Feb 2014 19:37:01 +0200 Message-ID: <1392745021.13243.66.camel@intelbox> References: <1392674540-10915-1-git-send-email-imre.deak@intel.com> <1392674540-10915-17-git-send-email-imre.deak@intel.com> <20140218165545.GX3852@intel.com> Reply-To: imre.deak@intel.com Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0294438127==" Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 03003FA593 for ; Tue, 18 Feb 2014 09:38:21 -0800 (PST) In-Reply-To: <20140218165545.GX3852@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Ville =?ISO-8859-1?Q?Syrj=E4l=E4?= Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============0294438127== Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-svOy+9l2Q7+gvN6sRyAw" --=-svOy+9l2Q7+gvN6sRyAw Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, 2014-02-18 at 18:55 +0200, Ville Syrj=C3=A4l=C3=A4 wrote: > On Tue, Feb 18, 2014 at 12:02:17AM +0200, Imre Deak wrote: > > Suggested by Daniel. > >=20 > > Signed-off-by: Imre Deak > > --- > > drivers/gpu/drm/i915/intel_pm.c | 33 ++++++++++++++++++++++++++++++--- > > 1 file changed, 30 insertions(+), 3 deletions(-) > >=20 > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/int= el_pm.c > > index e81e7de..21ccf89 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -5338,6 +5338,24 @@ static void hsw_power_well_disable(struct drm_i9= 15_private *dev_priv, > > hsw_enable_package_c8(dev_priv); > > } > > =20 > > +static void check_power_well_state(struct drm_i915_private *dev_priv, > > + struct i915_power_well *power_well) > > +{ > > + bool enabled; > > + > > + if (!power_well->ops->is_enabled) > > + return; > > + > > + enabled =3D power_well->ops->is_enabled(dev_priv, power_well); > > + > > + if (enabled !=3D (power_well->count > 0 || !i915.disable_power_well))= { >=20 > Doesn't i915.disable_power_well=3D=3Dtrue mean "leave power wells always > enabled"? So I think the '!' needs to be removed. No, i915.disable_power_well=3D=3Dtrue means disable power wells when the refcount goes to 0. Perhaps not the best name/semantics for this kind of option, the default for it should be 0 and mean normal operation, which is to disable power wells when possible. > > + DRM_ERROR("state mismatch for '%s' (hw state %d use-count %d disable= _power_well %d\n", > > + power_well->name, enabled, power_well->count, > > + i915.disable_power_well); > > + WARN_ON(1); > > + } >=20 > For an error message + backtrace, you could just use WARN(). Ok. > > +} > > + > > void intel_display_power_get(struct drm_i915_private *dev_priv, > > enum intel_display_power_domain domain) > > { > > @@ -5349,9 +5367,14 @@ void intel_display_power_get(struct drm_i915_pri= vate *dev_priv, > > =20 > > mutex_lock(&power_domains->lock); > > =20 > > - for_each_power_well(i, power_well, BIT(domain), power_domains) > > - if (!power_well->count++ && power_well->ops->enable) > > + for_each_power_well(i, power_well, BIT(domain), power_domains) { > > + if (!power_well->count++ && power_well->ops->enable) { > > + DRM_DEBUG_KMS("enabling %s\n", power_well->name); > > power_well->ops->enable(dev_priv, power_well); > > + } > > + > > + check_power_well_state(dev_priv, power_well); > > + } > > =20 > > power_domains->domain_use_count[domain]++; > > =20 > > @@ -5376,8 +5399,12 @@ void intel_display_power_put(struct drm_i915_pri= vate *dev_priv, > > WARN_ON(!power_well->count); > > =20 > > if (!--power_well->count && power_well->ops->disable && > > - i915.disable_power_well) > > + i915.disable_power_well) { > > + DRM_DEBUG_KMS("disabling %s\n", power_well->name); > > power_well->ops->disable(dev_priv, power_well); > > + } > > + > > + check_power_well_state(dev_priv, power_well); > > } > > =20 > > mutex_unlock(&power_domains->lock); > > --=20 > > 1.8.4 > >=20 > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >=20 --=-svOy+9l2Q7+gvN6sRyAw 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.14 (GNU/Linux) iQEcBAABAgAGBQJTA5o9AAoJEORIIAnNuWDF7oIH/AxptB7e4gnR6Bm7+yidxahv jVYkjdcCRuoyYidtgt/8LRe1t9GY36cNc6JhSY4elsuEyzHj+lVfmB77eda/t8WZ 69v/lPbiJEMjdJifmF+JPBT77pzye0tsZhccLNspp+jsJK+okkaJVXCGRMm4uotE IhSBotTxT2tvNNg4KmqyJw+CK1nBGBAVh0iSWHatkezK2W4caGkCKvdvgIug/JeO /OPoV1C2b/IlcpjUkAD2ZuzMU+IaRCBP6CDy4DD7GMeBqbVlu4UI7k034+0mGCwf hESkt3KRfHxDMIXjv1BlvImDWXUO5WJfakEVWtX/Ig8uhMWwJatwxyYEOEeNuEQ= =E9hQ -----END PGP SIGNATURE----- --=-svOy+9l2Q7+gvN6sRyAw-- --===============0294438127== 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 --===============0294438127==--