From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [Intel-gfx] [PATCH v2] drm/i915: Kill check_power_well() calls Date: Thu, 18 Dec 2014 12:07:34 +0200 Message-ID: <87ppbh46ft.fsf@intel.com> References: <1418895846-13725-1-git-send-email-ville.syrjala@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <1418895846-13725-1-git-send-email-ville.syrjala@linux.intel.com> Sender: stable-owner@vger.kernel.org To: ville.syrjala@linux.intel.com, intel-gfx@lists.freedesktop.org Cc: Egbert Eich , stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org On Thu, 18 Dec 2014, ville.syrjala@linux.intel.com wrote: > From: Ville Syrj=C3=A4l=C3=A4 > > pps_{lock,unlock}() call intel_display_power_{get,put}() outside > pps_mutes to avoid deadlocks with the power_domain mutex. In theory > during aux transfers we should usually have the relevant power domain > references already held by some higher level code, so this should not > result in much overhead (exception being userspace i2c-dev access). > However thanks to the check_power_well() calls in > intel_display_power_{get/put}() we end up doing a few Punit reads for > each aux transfer. Obviously doing this for each byte transferred via > i2c-over-aux is not a good idea. > > I can't think of a good way to keep check_power_well() while eliminat= ing > the overhead, so let's just remove check_power_well() entirely. > > Fixes a driver init time regression introduced by: > commit 773538e86081d146e0020435d614f4b96996c1f9 > Author: Ville Syrj=C3=A4l=C3=A4 > Date: Thu Sep 4 14:54:56 2014 +0300 > > drm/i915: Reset power sequencer pipe tracking when disp2d is off > > Credit goes to Jani for figuring this out. > > v2: Add the regression note in the commit message. > > Cc: stable@vger.kernel.org > Cc: Egbert Eich > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=3D86201 > Tested-by: Wendy Wang > Signed-off-by: Ville Syrj=C3=A4l=C3=A4 Pushed to drm-intel-next-fixes, thanks for the patch. BR, Jani. > --- > drivers/gpu/drm/i915/intel_runtime_pm.c | 27 -----------------------= ---- > 1 file changed, 27 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/dr= m/i915/intel_runtime_pm.c > index 6aa3a81..39e1b07 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -615,29 +615,6 @@ static void chv_pipe_power_well_disable(struct d= rm_i915_private *dev_priv, > vlv_power_sequencer_reset(dev_priv); > } > =20 > -static void check_power_well_state(struct drm_i915_private *dev_priv= , > - struct i915_power_well *power_well) > -{ > - bool enabled =3D power_well->ops->is_enabled(dev_priv, power_well); > - > - if (power_well->always_on || !i915.disable_power_well) { > - if (!enabled) > - goto mismatch; > - > - return; > - } > - > - if (enabled !=3D (power_well->count > 0)) > - goto mismatch; > - > - return; > - > -mismatch: > - I915_STATE_WARN(1, "state mismatch for '%s' (always_on %d hw state = %d use-count %d disable_power_well %d\n", > - power_well->name, power_well->always_on, enabled, > - power_well->count, i915.disable_power_well); > -} > - > /** > * intel_display_power_get - grab a power domain reference > * @dev_priv: i915 device instance > @@ -669,8 +646,6 @@ void intel_display_power_get(struct drm_i915_priv= ate *dev_priv, > power_well->ops->enable(dev_priv, power_well); > power_well->hw_enabled =3D true; > } > - > - check_power_well_state(dev_priv, power_well); > } > =20 > power_domains->domain_use_count[domain]++; > @@ -709,8 +684,6 @@ void intel_display_power_put(struct drm_i915_priv= ate *dev_priv, > power_well->hw_enabled =3D false; > power_well->ops->disable(dev_priv, power_well); > } > - > - check_power_well_state(dev_priv, power_well); > } > =20 > mutex_unlock(&power_domains->lock); > --=20 > 2.0.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx --=20 Jani Nikula, Intel Open Source Technology Center