From patchwork Tue Oct 20 13:49:48 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Zanoni, Paulo R" X-Patchwork-Id: 7447521 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.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 08BF3BEEA4 for ; Tue, 20 Oct 2015 13:50:48 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B600C2076E for ; Tue, 20 Oct 2015 13:50:46 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id C0EF620785 for ; Tue, 20 Oct 2015 13:50:42 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 0534B6E865; Tue, 20 Oct 2015 06:50:42 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 7157D6E865 for ; Tue, 20 Oct 2015 06:50:40 -0700 (PDT) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga101.jf.intel.com with ESMTP; 20 Oct 2015 06:50:19 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.17,707,1437462000"; d="scan'208";a="798059407" Received: from quanshi-mobl1.amr.corp.intel.com (HELO panetone.amr.corp.intel.com) ([10.254.77.241]) by orsmga001.jf.intel.com with ESMTP; 20 Oct 2015 06:50:19 -0700 From: Paulo Zanoni To: intel-gfx@lists.freedesktop.org Date: Tue, 20 Oct 2015 11:49:48 -0200 Message-Id: <1445349004-16409-3-git-send-email-paulo.r.zanoni@intel.com> X-Mailer: git-send-email 2.6.1 In-Reply-To: <1445349004-16409-1-git-send-email-paulo.r.zanoni@intel.com> References: <1445349004-16409-1-git-send-email-paulo.r.zanoni@intel.com> Subject: [Intel-gfx] [PATCH 02/18] drm/i915: don't stop+start FBC at every flip X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 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.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, 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 The hardware already takes care of disabling and recompressing FBC when we do a page flip, so all we need to do is to update the fence registers and move on. One of the important things to notice is that on the pre-gen6 platforms the fence is programmed on the FBC control register and the documentation says we can't update the control register while FBC is enabled. This would basically mean we'd have to to disable+enable FBC at every flip in order to make sure the hardware tracking still works, which doesn't seem to make too much sense. So I sent an email to the hardware team requesting some clarification. The information I got was this: "I don't think any hardware is latching on to 0x100100, 0x100104, or the old fence number in FBC_CTL. Instructions against changing on the fly would be to simplify testing and ensure you don't miss any invalidation that happened right at that time." So I guess we're fine for flips. But I can't really say I tested FBC on these ancient platforms - nor that I'll ever propose enabling FBC by default on them exactly because of problems like these. Testcase: igt/kms_frontbuffer_tracking/fbc*-fliptrack Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_reg.h | 3 + drivers/gpu/drm/i915/intel_display.c | 1 - drivers/gpu/drm/i915/intel_drv.h | 2 + drivers/gpu/drm/i915/intel_fbc.c | 98 +++++++++++++++++++++++++++++++- drivers/gpu/drm/i915/intel_frontbuffer.c | 1 + 6 files changed, 103 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index c334525..f04f56f 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -930,6 +930,7 @@ struct i915_fbc { bool (*fbc_enabled)(struct drm_i915_private *dev_priv); void (*enable_fbc)(struct intel_crtc *crtc); void (*disable_fbc)(struct drm_i915_private *dev_priv); + void (*flip_prepare)(struct drm_i915_private *dev_priv); }; /** diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 9ebf032..3620b59 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2028,6 +2028,7 @@ enum skl_disp_power_wells { #define FBC_CTL_C3_IDLE (1<<13) #define FBC_CTL_STRIDE_SHIFT (5) #define FBC_CTL_FENCENO_SHIFT (0) +#define FBC_CTL_FENCENO_MASK 0xF #define FBC_COMMAND 0x0320c #define FBC_CMD_COMPRESS (1<<0) #define FBC_STATUS 0x03210 @@ -2064,6 +2065,7 @@ enum skl_disp_power_wells { #define DPFC_CTL_LIMIT_1X (0<<6) #define DPFC_CTL_LIMIT_2X (1<<6) #define DPFC_CTL_LIMIT_4X (2<<6) +#define DPFC_CTL_FENCE_MASK 0xF #define DPFC_RECOMP_CTL 0x320c #define DPFC_RECOMP_STALL_EN (1<<27) #define DPFC_RECOMP_STALL_WM_SHIFT (16) @@ -2086,6 +2088,7 @@ enum skl_disp_power_wells { #define FBC_CTL_FALSE_COLOR (1<<10) /* The bit 28-8 is reserved */ #define DPFC_RESERVED (0x1FFFFF00) +#define ILK_DPFC_FENCE_MASK 0xF #define ILK_DPFC_RECOMP_CTL 0x4320c #define ILK_DPFC_STATUS 0x43210 #define ILK_DPFC_FENCE_YOFF 0x43218 diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 1fc1d24..e83a428 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11470,7 +11470,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, to_intel_plane(primary)->frontbuffer_bit); mutex_unlock(&dev->struct_mutex); - intel_fbc_disable_crtc(intel_crtc); intel_frontbuffer_flip_prepare(dev, to_intel_plane(primary)->frontbuffer_bit); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index dc55e4c..1e08c8a 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1287,6 +1287,8 @@ void intel_fbc_invalidate(struct drm_i915_private *dev_priv, enum fb_op_origin origin); void intel_fbc_flush(struct drm_i915_private *dev_priv, unsigned int frontbuffer_bits, enum fb_op_origin origin); +void intel_fbc_flip_prepare(struct drm_i915_private *dev_priv, + unsigned int frontbuffer_bits); void intel_fbc_cleanup_cfb(struct drm_i915_private *dev_priv); /* intel_hdmi.c */ diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index d9d7e54..9477379 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -82,6 +82,22 @@ static void i8xx_fbc_disable(struct drm_i915_private *dev_priv) DRM_DEBUG_KMS("disabled FBC\n"); } +static void i8xx_fbc_flip_prepare(struct drm_i915_private *dev_priv) +{ + struct intel_crtc *crtc = dev_priv->fbc.crtc; + struct drm_framebuffer *fb = crtc->base.primary->fb; + struct drm_i915_gem_object *obj = intel_fb_obj(fb); + uint32_t val; + + /* Although the documentation suggests we can't change DPFC_CONTROL + * while compression is enabled, the hardware guys said that updating + * the fence register bits during a flip is fine. */ + val = I915_READ(FBC_CONTROL); + val &= ~FBC_CTL_FENCENO_MASK; + val |= obj->fence_reg; + I915_WRITE(FBC_CONTROL, val); +} + static void i8xx_fbc_enable(struct intel_crtc *crtc) { struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; @@ -161,6 +177,22 @@ static void g4x_fbc_enable(struct intel_crtc *crtc) DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(crtc->plane)); } +static void g4x_fbc_flip_prepare(struct drm_i915_private *dev_priv) +{ + struct intel_crtc *crtc = dev_priv->fbc.crtc; + struct drm_framebuffer *fb = crtc->base.primary->fb; + struct drm_i915_gem_object *obj = intel_fb_obj(fb); + uint32_t val; + + /* Although the documentation suggests we can't change DPFC_CONTROL + * while compression is enabled, the hardware guys said that updating + * the fence register bits during a flip is fine. */ + val = I915_READ(DPFC_CONTROL); + val &= ~DPFC_CTL_FENCE_MASK; + val |= obj->fence_reg; + I915_WRITE(DPFC_CONTROL, val); +} + static void g4x_fbc_disable(struct drm_i915_private *dev_priv) { u32 dpfc_ctl; @@ -236,6 +268,31 @@ static void ilk_fbc_enable(struct intel_crtc *crtc) DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(crtc->plane)); } +static void ilk_fbc_flip_prepare(struct drm_i915_private *dev_priv) +{ + struct intel_crtc *crtc = dev_priv->fbc.crtc; + struct drm_framebuffer *fb = crtc->base.primary->fb; + struct drm_i915_gem_object *obj = intel_fb_obj(fb); + uint32_t val; + + /* Although the documentation suggests we can't change DPFC_CONTROL + * while compression is enabled, the hardware guys said that updating + * the fence register bits during a flip is fine. */ + val = I915_READ(ILK_DPFC_CONTROL); + val &= ~ILK_DPFC_FENCE_MASK; + val |= obj->fence_reg; + I915_WRITE(ILK_DPFC_CONTROL, val); +} + +static void snb_fbc_flip_prepare(struct drm_i915_private *dev_priv) +{ + struct intel_crtc *crtc = dev_priv->fbc.crtc; + struct drm_framebuffer *fb = crtc->base.primary->fb; + struct drm_i915_gem_object *obj = intel_fb_obj(fb); + + I915_WRITE(SNB_DPFC_CTL_SA, SNB_CPU_FENCE_ENABLE | obj->fence_reg); +} + static void ilk_fbc_disable(struct drm_i915_private *dev_priv) { u32 dpfc_ctl; @@ -1020,14 +1077,44 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv, if (origin == ORIGIN_GTT) return; + if (origin == ORIGIN_FLIP && dev_priv->fbc.enabled) + return; mutex_lock(&dev_priv->fbc.lock); dev_priv->fbc.busy_bits &= ~frontbuffer_bits; if (!dev_priv->fbc.busy_bits) { - __intel_fbc_disable(dev_priv); - __intel_fbc_update(dev_priv); + if (origin == ORIGIN_FLIP) { + __intel_fbc_update(dev_priv); + } else { + __intel_fbc_disable(dev_priv); + __intel_fbc_update(dev_priv); + } + } + + mutex_unlock(&dev_priv->fbc.lock); +} + +void intel_fbc_flip_prepare(struct drm_i915_private *dev_priv, + unsigned int frontbuffer_bits) +{ + unsigned int fbc_bits; + + if (!fbc_supported(dev_priv)) + return; + + mutex_lock(&dev_priv->fbc.lock); + + if (dev_priv->fbc.enabled) { + fbc_bits = INTEL_FRONTBUFFER_PRIMARY(dev_priv->fbc.crtc->pipe); + if (fbc_bits & frontbuffer_bits) + dev_priv->fbc.flip_prepare(dev_priv); + } else if (dev_priv->fbc.fbc_work) { + fbc_bits = INTEL_FRONTBUFFER_PRIMARY( + dev_priv->fbc.fbc_work->crtc->pipe); + if (fbc_bits & frontbuffer_bits) + __intel_fbc_disable(dev_priv); } mutex_unlock(&dev_priv->fbc.lock); @@ -1063,18 +1150,25 @@ void intel_fbc_init(struct drm_i915_private *dev_priv) dev_priv->fbc.fbc_enabled = ilk_fbc_enabled; dev_priv->fbc.enable_fbc = gen7_fbc_enable; dev_priv->fbc.disable_fbc = ilk_fbc_disable; + dev_priv->fbc.flip_prepare = snb_fbc_flip_prepare; } else if (INTEL_INFO(dev_priv)->gen >= 5) { dev_priv->fbc.fbc_enabled = ilk_fbc_enabled; dev_priv->fbc.enable_fbc = ilk_fbc_enable; dev_priv->fbc.disable_fbc = ilk_fbc_disable; + if (INTEL_INFO(dev_priv)->gen == 5) + dev_priv->fbc.flip_prepare = ilk_fbc_flip_prepare; + else + dev_priv->fbc.flip_prepare = snb_fbc_flip_prepare; } else if (IS_GM45(dev_priv)) { dev_priv->fbc.fbc_enabled = g4x_fbc_enabled; dev_priv->fbc.enable_fbc = g4x_fbc_enable; dev_priv->fbc.disable_fbc = g4x_fbc_disable; + dev_priv->fbc.flip_prepare = g4x_fbc_flip_prepare; } else { dev_priv->fbc.fbc_enabled = i8xx_fbc_enabled; dev_priv->fbc.enable_fbc = i8xx_fbc_enable; dev_priv->fbc.disable_fbc = i8xx_fbc_disable; + dev_priv->fbc.flip_prepare = i8xx_fbc_flip_prepare; /* This value was pulled out of someone's hat */ I915_WRITE(FBC_CONTROL, 500 << FBC_CTL_INTERVAL_SHIFT); diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c index ac85357..31a1ad3 100644 --- a/drivers/gpu/drm/i915/intel_frontbuffer.c +++ b/drivers/gpu/drm/i915/intel_frontbuffer.c @@ -192,6 +192,7 @@ void intel_frontbuffer_flip_prepare(struct drm_device *dev, mutex_unlock(&dev_priv->fb_tracking.lock); intel_psr_single_frame_update(dev, frontbuffer_bits); + intel_fbc_flip_prepare(dev_priv, frontbuffer_bits); } /**