From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752846Ab3GNR4w (ORCPT ); Sun, 14 Jul 2013 13:56:52 -0400 Received: from mail-lb0-f177.google.com ([209.85.217.177]:65049 "EHLO mail-lb0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751267Ab3GNR4v (ORCPT ); Sun, 14 Jul 2013 13:56:51 -0400 Message-ID: <51E2E65D.5050803@openvz.org> Date: Sun, 14 Jul 2013 21:56:45 +0400 From: Konstantin Khlebnikov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0.12) Gecko/20130119 Firefox/10.0.11esrpre Iceape/2.7.12 MIME-Version: 1.0 To: Daniel Vetter CC: David Airlie , intel-gfx , Linux Kernel Mailing List , dri-devel , Chris Wilson Subject: Re: [PATCH] drm/i915: fix long-standing SNB regression in power consumption after resume References: <20130714163009.22374.22100.stgit@zurg> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Daniel Vetter wrote: > On Sun, Jul 14, 2013 at 6:30 PM, Konstantin Khlebnikov > wrote: >> This patch fixes regression in power consumtion of sandy bridge gpu, which >> exists since v3.6 Sometimes after resuming from s2ram gpu starts thinking that >> it's extremely busy. After that it never reaches rc6 state. >> >> Bug was introduce by commit b4ae3f22d238617ca11610b29fde16cf8c0bc6e0 >> ("drm/i915: load boot context at driver init time"). Without documentation >> it's not clear what is happening here, probably this breaks internal state of >> hardware ring buffers and confuses RPS engine. Fortunately keeping forcewake >> during whole initialization sequence in gen6_init_clock_gating() fixes this bug. >> >> References: https://bugs.freedesktop.org/show_bug.cgi?id=54089 >> Signed-off-by: Konstantin Khlebnikov > > We already hold an forcewake reference while setting up the rps stuff, > should we maybe hold the forcewake for the entire duration, i.e. grab > it here in clock_gating and release it only in gen6/vlv_enable_rps? > Can you please test that version, too? This will be racy because rps stuff is done in separate work which might be canceled if intel_disable_gt_powersave() happens before its completion. > > In any case the forcewake grabbing here in the clock gating function > needs a big comment that otherwise setting the MCTL register might > break rc6 entry. > -Daniel > >> --- >> drivers/gpu/drm/i915/intel_pm.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >> index aa01128..839a43f 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -3640,6 +3640,8 @@ static void gen6_init_clock_gating(struct drm_device *dev) >> int pipe; >> uint32_t dspclk_gate = ILK_VRHUNIT_CLOCK_GATE_DISABLE; >> >> + gen6_gt_force_wake_get(dev_priv); >> + >> I915_WRITE(ILK_DSPCLK_GATE_D, dspclk_gate); >> >> I915_WRITE(ILK_DISPLAY_CHICKEN2, >> @@ -3728,6 +3730,8 @@ static void gen6_init_clock_gating(struct drm_device *dev) >> cpt_init_clock_gating(dev); >> >> gen6_check_mch_setup(dev); >> + >> + gen6_gt_force_wake_put(dev_priv); >> } >> >> static void gen7_setup_fixed_func_scheduler(struct drm_i915_private *dev_priv) >> > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konstantin Khlebnikov Subject: Re: [PATCH] drm/i915: fix long-standing SNB regression in power consumption after resume Date: Sun, 14 Jul 2013 21:56:45 +0400 Message-ID: <51E2E65D.5050803@openvz.org> References: <20130714163009.22374.22100.stgit@zurg> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Daniel Vetter Cc: intel-gfx , Linux Kernel Mailing List , dri-devel List-Id: dri-devel@lists.freedesktop.org Daniel Vetter wrote: > On Sun, Jul 14, 2013 at 6:30 PM, Konstantin Khlebnikov > wrote: >> This patch fixes regression in power consumtion of sandy bridge gpu, which >> exists since v3.6 Sometimes after resuming from s2ram gpu starts thinking that >> it's extremely busy. After that it never reaches rc6 state. >> >> Bug was introduce by commit b4ae3f22d238617ca11610b29fde16cf8c0bc6e0 >> ("drm/i915: load boot context at driver init time"). Without documentation >> it's not clear what is happening here, probably this breaks internal state of >> hardware ring buffers and confuses RPS engine. Fortunately keeping forcewake >> during whole initialization sequence in gen6_init_clock_gating() fixes this bug. >> >> References: https://bugs.freedesktop.org/show_bug.cgi?id=54089 >> Signed-off-by: Konstantin Khlebnikov > > We already hold an forcewake reference while setting up the rps stuff, > should we maybe hold the forcewake for the entire duration, i.e. grab > it here in clock_gating and release it only in gen6/vlv_enable_rps? > Can you please test that version, too? This will be racy because rps stuff is done in separate work which might be canceled if intel_disable_gt_powersave() happens before its completion. > > In any case the forcewake grabbing here in the clock gating function > needs a big comment that otherwise setting the MCTL register might > break rc6 entry. > -Daniel > >> --- >> drivers/gpu/drm/i915/intel_pm.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >> index aa01128..839a43f 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -3640,6 +3640,8 @@ static void gen6_init_clock_gating(struct drm_device *dev) >> int pipe; >> uint32_t dspclk_gate = ILK_VRHUNIT_CLOCK_GATE_DISABLE; >> >> + gen6_gt_force_wake_get(dev_priv); >> + >> I915_WRITE(ILK_DSPCLK_GATE_D, dspclk_gate); >> >> I915_WRITE(ILK_DISPLAY_CHICKEN2, >> @@ -3728,6 +3730,8 @@ static void gen6_init_clock_gating(struct drm_device *dev) >> cpt_init_clock_gating(dev); >> >> gen6_check_mch_setup(dev); >> + >> + gen6_gt_force_wake_put(dev_priv); >> } >> >> static void gen7_setup_fixed_func_scheduler(struct drm_i915_private *dev_priv) >> > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch