From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 03/19] drm/i915: get a PC8 reference when enabling the power well Date: Tue, 10 Dec 2013 22:29:47 +0100 Message-ID: <20131210212947.GM9804@phenom.ffwll.local> References: <1385048853-1579-4-git-send-email-przanoni@gmail.com> <1385582362-2307-1-git-send-email-przanoni@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ee0-f46.google.com (mail-ee0-f46.google.com [74.125.83.46]) by gabe.freedesktop.org (Postfix) with ESMTP id 80501FA89D for ; Tue, 10 Dec 2013 13:28:59 -0800 (PST) Received: by mail-ee0-f46.google.com with SMTP id d49so2494167eek.19 for ; Tue, 10 Dec 2013 13:28:58 -0800 (PST) Content-Disposition: inline In-Reply-To: <1385582362-2307-1-git-send-email-przanoni@gmail.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: Paulo Zanoni Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org On Wed, Nov 27, 2013 at 05:59:22PM -0200, Paulo Zanoni wrote: > From: Paulo Zanoni > > In the current code, at haswell_modeset_global_resources, first we > decide if we want to enable/disable the power well, then we decide if > we want to enable/disable PC8. On the case where we're enabling PC8 > this works fine, but on the case where we disable PC8 due to a non-eDP > monitor being enabled, we first enable the power well and then disable > PC8. Although wrong, this doesn't seem to be causing any problems now, > and we don't even see anything in dmesg. But the patches for runtime > D3 turn this problem into a real bug, so we need to fix it. > > This fixes the "modeset-non-lpsp" subtest from the "pm_pc8" test from > intel-gpu-tools. > > v2: - Rebase (i915_disable_power_well). > v3: - More reabase. > > Signed-off-by: Paulo Zanoni > --- > drivers/gpu/drm/i915/intel_pm.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 432ec13..1cb02b1 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -5677,6 +5677,8 @@ static void hsw_set_power_well(struct drm_device *dev, > unsigned long irqflags; > uint32_t tmp; > > + WARN_ON(dev_priv->pc8.enabled); > + > tmp = I915_READ(HSW_PWR_WELL_DRIVER); > is_enabled = tmp & HSW_PWR_WELL_STATE_ENABLED; > enable_requested = tmp & HSW_PWR_WELL_ENABLE_REQUEST; > @@ -5736,17 +5738,26 @@ static void hsw_set_power_well(struct drm_device *dev, > static void __intel_power_well_get(struct drm_device *dev, > struct i915_power_well *power_well) > { > - if (!power_well->count++ && power_well->set) > + struct drm_i915_private *dev_priv = dev->dev_private; > + > + if (!power_well->count++ && power_well->set) { > + hsw_disable_package_c8(dev_priv); > power_well->set(dev, power_well, true); Just to reiterate what we've discussed: Once this stuff has settled we need to push the hsw_*_pc8 out of this common code into the power_well->set/get functions. Merged the patch for now. -Daniel > + } > } > > static void __intel_power_well_put(struct drm_device *dev, > struct i915_power_well *power_well) > { > + struct drm_i915_private *dev_priv = dev->dev_private; > + > WARN_ON(!power_well->count); > > - if (!--power_well->count && power_well->set && i915_disable_power_well) > + if (!--power_well->count && power_well->set && > + i915_disable_power_well) { > power_well->set(dev, power_well, false); > + hsw_enable_package_c8(dev_priv); > + } > } > > void intel_display_power_get(struct drm_device *dev, > -- > 1.8.3.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch