From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 16/19] drm/i915: sanity check power well sw state against hw state Date: Wed, 5 Mar 2014 11:32:37 +0100 Message-ID: <20140305103237.GV17001@phenom.ffwll.local> 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> <1392745021.13243.66.camel@intelbox> <20140218175905.GY3852@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-ee0-f47.google.com (mail-ee0-f47.google.com [74.125.83.47]) by gabe.freedesktop.org (Postfix) with ESMTP id 5D152FA4B0 for ; Wed, 5 Mar 2014 02:32:42 -0800 (PST) Received: by mail-ee0-f47.google.com with SMTP id b15so342039eek.6 for ; Wed, 05 Mar 2014 02:32:41 -0800 (PST) Content-Disposition: inline In-Reply-To: <20140218175905.GY3852@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 On Tue, Feb 18, 2014 at 07:59:05PM +0200, Ville Syrj=E4l=E4 wrote: > On Tue, Feb 18, 2014 at 07:37:01PM +0200, Imre Deak wrote: > > On Tue, 2014-02-18 at 18:55 +0200, Ville Syrj=E4l=E4 wrote: > > > On Tue, Feb 18, 2014 at 12:02:17AM +0200, Imre Deak wrote: > > > > Suggested by Daniel. > > > > = > > > > Signed-off-by: Imre Deak > > > > --- > > > > drivers/gpu/drm/i915/intel_pm.c | 33 +++++++++++++++++++++++++++++= +--- > > > > 1 file changed, 30 insertions(+), 3 deletions(-) > > > > = > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915= /intel_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 dr= m_i915_private *dev_priv, > > > > hsw_enable_package_c8(dev_priv); > > > > } > > > > = > > > > +static void check_power_well_state(struct drm_i915_private *dev_pr= iv, > > > > + 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_we= ll)) { > > > = > > > Doesn't i915.disable_power_well=3D=3Dtrue mean "leave power wells alw= ays > > > 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. > = > Oh I had the impression it was the other way around, but you're right. > Seems I keep getting confused by this thing. It has happened before and > I'm guessing it will happen again after I've forgotten the details > again. I concur that a module option rename pass is in order. Maybe once this has all settled a bit ;-) -Daniel -- = Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch