From patchwork Thu Jul 26 09:16:14 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 1240921 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork1.kernel.org (Postfix) with ESMTP id 0AD2D3FC5A for ; Thu, 26 Jul 2012 09:16:53 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id A9F379EEA4 for ; Thu, 26 Jul 2012 02:16:52 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-bk0-f49.google.com (mail-bk0-f49.google.com [209.85.214.49]) by gabe.freedesktop.org (Postfix) with ESMTP id 901B09E824 for ; Thu, 26 Jul 2012 02:16:21 -0700 (PDT) Received: by bkcji2 with SMTP id ji2so1130772bkc.36 for ; Thu, 26 Jul 2012 02:16:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=from:to:cc:subject:date:message-id:x-mailer:in-reply-to:references; bh=r3keyJBa6/pgX7E8Hv0p5eqVayRSX8qYYJpVNkJ98hs=; b=IUkG79Sa9Y4oqpvnnXFbWyupVF2HNzSSb/wiYfQPwqxjK+WSSMDJHdjqxSkbyytp0K hu/qEiuEoPD4CXAXB4gLBGJbMmjrOGiwbmqC64gaIhUnU/QjTy50dyin4jkkbUUWFVcV hmIT7pJVnYYDDjJZfBiLU1Qq9a94VLTbtrNkQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=from:to:cc:subject:date:message-id:x-mailer:in-reply-to:references :x-gm-message-state; bh=r3keyJBa6/pgX7E8Hv0p5eqVayRSX8qYYJpVNkJ98hs=; b=SPZSjcZBuyZpft8Rfz7UvMeuaYWpjYr1uGopIUafjErQsFmvG/7+2JMnah98zRvCUn eGNVqUnqCvrR5cwTjmBY5O8oT8H5wPaAu95ZDjHzvS/3XtvwLd6yG4tqDnAF39S4anBz 7TSyIXQBE9w3i5MN0qsIqIAIIHnEQeZ2ZBn3QZNFzecJVIT7l46NgMxu3kzOE+nLdOO2 wVdbhAsrCKekPLLMcD/njE7ct1LXaSeqKC7nhP6Q/92G7tc/l6w8zLXJC6kcBQ3crM0s Ebu0zeHWTiLGgg2kChsCTwtjDXAAsTa4tAGsXdsDzuSev+uZFT+xrInYxc9YhKdD6FGO yujQ== Received: by 10.204.8.65 with SMTP id g1mr7427234bkg.50.1343294180430; Thu, 26 Jul 2012 02:16:20 -0700 (PDT) Received: from bremse.ffwll.local (178-83-130-250.dynamic.hispeed.ch. [178.83.130.250]) by mx.google.com with ESMTPS id gq2sm14074415bkc.13.2012.07.26.02.16.18 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 26 Jul 2012 02:16:19 -0700 (PDT) From: Daniel Vetter To: Intel Graphics Development Date: Thu, 26 Jul 2012 11:16:14 +0200 Message-Id: <1343294174-21811-1-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 1.7.10.4 In-Reply-To: <1343254184_355@CP5-2952> References: <1343254184_355@CP5-2952> X-Gm-Message-State: ALoCoQnaB8hmsHE+ykylxClI4kf8c8bhieqLcIuBInmwrT81jTYiABsAJKbSD/X1zELIPEsS1P1i Cc: Daniel Vetter Subject: [Intel-gfx] [PATCH] [CFT] drm/i915: Only set the down rps limit when at the loweset frequency X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org The power docs say that when the gt leaves rc6, it is in the lowest frequency and only about 25 usec later will switch to the frequency selected in GEN6_RPNSWREQ. If the downclock limit expires in that window and the down limit is set to the lowest possible frequency, the hw will not send the down interrupt. Which leads to a too high gpu clock and wasted power. Chris Wilson already worked on this with commit 7b9e0ae6da0a7eaf2680a1a788f08df123724f3b Author: Chris Wilson Date: Sat Apr 28 08:56:39 2012 +0100 drm/i915: Always update RPS interrupts thresholds along with frequency but got the logic inverted: The current code set the down limit as long as we haven't reached it. Instead of only once with reached the lowest frequency. Note that we can't always set the downclock limit to 0, because otherwise the hw will keep on bugging us with downclock request irqs once the lowest level is reached. For similar reasons also always set the upclock limit, otherwise the hw might poke us again with interrupts. v2: Chris Wilson noticed that the limit reg is also computed in sanitize_pm. To avoid duplication, extract the code into a common function. Signed-Off-by: Daniel Vetter Reviewed-by: Chris Wilson --- drivers/gpu/drm/i915/intel_pm.c | 43 +++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index d0ce2a5..a537768 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2267,21 +2267,33 @@ void ironlake_disable_drps(struct drm_device *dev) } -void gen6_set_rps(struct drm_device *dev, u8 val) +static u32 gen6_rps_limits(struct drm_i915_private *dev_priv, u8 val) { - struct drm_i915_private *dev_priv = dev->dev_private; u32 limits; limits = 0; if (val >= dev_priv->max_delay) val = dev_priv->max_delay; - else - limits |= dev_priv->max_delay << 24; - - if (val <= dev_priv->min_delay) + limits |= dev_priv->max_delay << 24; + + /* Only set the down limit when we've reached the lowest level to avoid + * getting more interrupts, otherwise leave this clear. This prevents a + * race in the hw when coming out of rc6: There's a tiny window where + * the hw runs at the minimal clock before selecting the desired + * frequency, if the down threshold expires in that window we will not + * receive a down interrupt. */ + if (val <= dev_priv->min_delay) { val = dev_priv->min_delay; - else limits |= dev_priv->min_delay << 16; + } + + return limits; +} + +void gen6_set_rps(struct drm_device *dev, u8 val) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + u32 limits = gen6_rps_limits(dev_priv, val); if (val == dev_priv->cur_delay) return; @@ -3587,25 +3599,20 @@ void intel_init_clock_gating(struct drm_device *dev) static void gen6_sanitize_pm(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; - u32 limits, delay, old; + u32 limits, current_limits; gen6_gt_force_wake_get(dev_priv); - old = limits = I915_READ(GEN6_RP_INTERRUPT_LIMITS); + current_limits = I915_READ(GEN6_RP_INTERRUPT_LIMITS); /* Make sure we continue to get interrupts * until we hit the minimum or maximum frequencies. */ - limits &= ~(0x3f << 16 | 0x3f << 24); - delay = dev_priv->cur_delay; - if (delay < dev_priv->max_delay) - limits |= (dev_priv->max_delay & 0x3f) << 24; - if (delay > dev_priv->min_delay) - limits |= (dev_priv->min_delay & 0x3f) << 16; - - if (old != limits) { + limits = gen6_rps_limits(dev_priv, dev_priv->cur_delay); + + if (current_limits != limits) { /* Note that the known failure case is to read back 0. */ DRM_DEBUG_DRIVER("Power management discrepancy: GEN6_RP_INTERRUPT_LIMITS " - "expected %08x, was %08x\n", limits, old); + "expected %08x, was %08x\n", limits, current_limits); I915_WRITE(GEN6_RP_INTERRUPT_LIMITS, limits); }