From patchwork Fri Sep 19 19:04:55 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paulo Zanoni X-Patchwork-Id: 4939731 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 8E4E9BEEA5 for ; Fri, 19 Sep 2014 19:05:30 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 9FE802008F for ; Fri, 19 Sep 2014 19:05:29 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 9A6E7201EF for ; Fri, 19 Sep 2014 19:05:28 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 241A86E2D1; Fri, 19 Sep 2014 12:05:28 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-qg0-f45.google.com (mail-qg0-f45.google.com [209.85.192.45]) by gabe.freedesktop.org (Postfix) with ESMTP id 09A096E2D1 for ; Fri, 19 Sep 2014 12:05:26 -0700 (PDT) Received: by mail-qg0-f45.google.com with SMTP id q108so367000qgd.4 for ; Fri, 19 Sep 2014 12:05:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=DQI8g9I8Qha8iR7r6XQc3p1gx7wr5gHWfYsnM4RmHXE=; b=rXrjUo7n2n7+PSluxB5Owi9I1jBK3h1LPPx7HQaLwi3SXzA16pCf7zzEVcLpLI61Ay BEw271HQl6KetXYp0NxcbIWs2fEHgmqZMEp96fRPsZNSa318qt+CXyUDBR8W/ezxy8GQ RkACuPTiuCGgPc4/xWVpO/G7StRX2Lz9WbkMc9Wgohr010E3COOru4vfMI7M/9kjHxPw tnc2Ph6V9OUUaj60k4gWSERPv0c42SdAjOeLbBdR/gauojcLFk7AdadQIFIuIb/HVoYx rWuSm5x/30+u2b+t990Ary89Nz8upnVdpkNpIv7k7t7dCftnGiT94auKu3WwDjtL20T2 7sXA== X-Received: by 10.224.130.138 with SMTP id t10mr3646373qas.2.1411153526419; Fri, 19 Sep 2014 12:05:26 -0700 (PDT) Received: from localhost.localdomain ([177.132.71.69]) by mx.google.com with ESMTPSA id p18sm2048921qay.22.2014.09.19.12.05.24 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 19 Sep 2014 12:05:25 -0700 (PDT) From: Paulo Zanoni To: intel-gfx@lists.freedesktop.org Date: Fri, 19 Sep 2014 16:04:55 -0300 Message-Id: <1411153495-2658-2-git-send-email-przanoni@gmail.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1411153495-2658-1-git-send-email-przanoni@gmail.com> References: <1411153495-2658-1-git-send-email-przanoni@gmail.com> Cc: Paulo Zanoni , Rodrigo Vivi Subject: [Intel-gfx] [PATCH 2/2] drm/i915: add SW tracking to FBC enabling X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-4.7 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Paulo Zanoni Currently, calling intel_fbc_enabled() will trigger a register read. And we call it a lot of times, even when FBC is disabled, so saving a few cycles would be a good thing. Another reason for this patch is because we currently call intel_fbc_enabled() while the HW is runtime suspended, so the read makes no sense and triggers a WARN. This happens even if FBC is disabled by default. Of course one could argue that we just shouldn't be calling intel_fbc_enabled() while the driver is runtime suspended, and I agree that's a good argument, but I still think that the reason explained in the first paragraph already justifies the patch. This problem can easily be reproduced with many subtests of igt/pm_rpm, and it is a regression introduced by: commit c5ad011d7d256ecbe173324029e992817194d2b0 Author: Rodrigo Vivi Date: Mon Aug 4 03:51:38 2014 -0700 drm/i915: FBC flush nuke for BDW Testcase: igt/pm_rpm/cursor (and others) Cc: Rodrigo Vivi Signed-off-by: Paulo Zanoni Reviewed-by: Rodrigo Vivi --- drivers/gpu/drm/i915/i915_drv.h | 4 ++++ drivers/gpu/drm/i915/intel_pm.c | 31 ++++++++++++++++++++----------- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 5fce16c..999bd57 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -662,6 +662,10 @@ struct i915_fbc { bool false_color; + /* Tracks whether the HW is actually enabled, not whether the feature is + * possible. */ + bool enabled; + struct intel_fbc_work { struct delayed_work work; struct drm_crtc *crtc; diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 2ca9fdb..6b41620 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -71,6 +71,8 @@ static void i8xx_disable_fbc(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; u32 fbc_ctl; + dev_priv->fbc.enabled = false; + /* Disable compression */ fbc_ctl = I915_READ(FBC_CONTROL); if ((fbc_ctl & FBC_CTL_EN) == 0) @@ -99,6 +101,8 @@ static void i8xx_enable_fbc(struct drm_crtc *crtc) int i; u32 fbc_ctl; + dev_priv->fbc.enabled = true; + cfb_pitch = dev_priv->fbc.size / FBC_LL_SIZE; if (fb->pitches[0] < cfb_pitch) cfb_pitch = fb->pitches[0]; @@ -153,6 +157,8 @@ static void g4x_enable_fbc(struct drm_crtc *crtc) struct intel_crtc *intel_crtc = to_intel_crtc(crtc); u32 dpfc_ctl; + dev_priv->fbc.enabled = true; + dpfc_ctl = DPFC_CTL_PLANE(intel_crtc->plane) | DPFC_SR_EN; if (drm_format_plane_cpp(fb->pixel_format, 0) == 2) dpfc_ctl |= DPFC_CTL_LIMIT_2X; @@ -173,6 +179,8 @@ static void g4x_disable_fbc(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; u32 dpfc_ctl; + dev_priv->fbc.enabled = false; + /* Disable compression */ dpfc_ctl = I915_READ(DPFC_CONTROL); if (dpfc_ctl & DPFC_CTL_EN) { @@ -224,6 +232,8 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc) struct intel_crtc *intel_crtc = to_intel_crtc(crtc); u32 dpfc_ctl; + dev_priv->fbc.enabled = true; + dpfc_ctl = DPFC_CTL_PLANE(intel_crtc->plane); if (drm_format_plane_cpp(fb->pixel_format, 0) == 2) dev_priv->fbc.threshold++; @@ -264,6 +274,8 @@ static void ironlake_disable_fbc(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; u32 dpfc_ctl; + dev_priv->fbc.enabled = false; + /* Disable compression */ dpfc_ctl = I915_READ(ILK_DPFC_CONTROL); if (dpfc_ctl & DPFC_CTL_EN) { @@ -290,6 +302,8 @@ static void gen7_enable_fbc(struct drm_crtc *crtc) struct intel_crtc *intel_crtc = to_intel_crtc(crtc); u32 dpfc_ctl; + dev_priv->fbc.enabled = true; + dpfc_ctl = IVB_DPFC_CTL_PLANE(intel_crtc->plane); if (drm_format_plane_cpp(fb->pixel_format, 0) == 2) dev_priv->fbc.threshold++; @@ -339,16 +353,7 @@ bool intel_fbc_enabled(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; - /* If it wasn't never enabled by kernel parameter or platform default - * we can avoid reading registers so many times in vain - */ - if (!i915.enable_fbc) - return false; - - if (!dev_priv->display.fbc_enabled) - return false; - - return dev_priv->display.fbc_enabled(dev); + return dev_priv->fbc.enabled; } void gen8_fbc_sw_flush(struct drm_device *dev, u32 value) @@ -7360,8 +7365,10 @@ void intel_fini_runtime_pm(struct drm_i915_private *dev_priv) static void intel_init_fbc(struct drm_i915_private *dev_priv) { - if (!HAS_FBC(dev_priv)) + if (!HAS_FBC(dev_priv)) { + dev_priv->fbc.enabled = false; return; + } if (INTEL_INFO(dev_priv)->gen >= 7) { dev_priv->display.fbc_enabled = ironlake_fbc_enabled; @@ -7383,6 +7390,8 @@ static void intel_init_fbc(struct drm_i915_private *dev_priv) /* This value was pulled out of someone's hat */ I915_WRITE(FBC_CONTROL, 500 << FBC_CTL_INTERVAL_SHIFT); } + + dev_priv->fbc.enabled = dev_priv->display.fbc_enabled(dev_priv->dev); } /* Set up chip specific power management-related functions */