From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH] drm/i915: Try harder to get FBC Date: Thu, 03 Jul 2014 14:52:07 +0300 Message-ID: <871tu2ek2g.fsf@intel.com> References: <20140620165533.GC23411@bwidawsk.net> <1404150084-3629-1-git-send-email-rodrigo.vivi@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 206FA6E6D6 for ; Thu, 3 Jul 2014 04:52:19 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Rodrigo Vivi , Rodrigo Vivi Cc: intel-gfx , Art Runyan , Ben Widawsky , Ben Widawsky List-Id: intel-gfx@lists.freedesktop.org On Tue, 01 Jul 2014, Rodrigo Vivi wrote: > Jani, please ignore the 4th patch on this series and merge the 3 I've > reviewed and tested already. Patches 1-3 pushed to dinq. Thanks for the patches and review. BR, Jani. > > They are essential to allow FBC work on BDW without changing bios > configuration and allow PC7 residency. > > Thanks, > Rodrigo. > > > On Mon, Jun 30, 2014 at 10:41 AM, Rodrigo Vivi > wrote: > >> From: Ben Widawsky >> >> The GEN FBC unit provides the ability to set a low pass on frames it >> attempts to compress. If a frame is less than a certain amount >> compressibility (2:1, 4:1) it will not bother. This allows the driver to >> reduce the size it requests out of stolen memory. >> >> Unluckily, a few months ago, Ville actually began using this feature for >> framebuffers that are 16bpp (not sure why not 8bpp). In those cases, we >> are already using this mechanism for a different purpose, and so we can >> only achieve one further level of compression (2:1 -> 4:1) >> >> FBC GEN1, ie. pre-G45 is ignored. >> >> The cleverness of the patch is Art's. The bugs are mine. >> >> v2: Update message and including missing threshold case 3 (Spotted by >> Arthur). >> >> Reviewedby: Rodrigo Vivi >> Cc: Art Runyan >> Signed-off-by: Ben Widawsky >> Signed-off-by: Rodrigo Vivi >> --- >> drivers/gpu/drm/i915/i915_drv.h | 3 +- >> drivers/gpu/drm/i915/i915_gem_stolen.c | 54 >> +++++++++++++++++++++++++--------- >> drivers/gpu/drm/i915/intel_pm.c | 30 +++++++++++++++++-- >> 3 files changed, 69 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h >> b/drivers/gpu/drm/i915/i915_drv.h >> index 5b7aed2..9953ea8 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -600,6 +600,7 @@ struct intel_context { >> >> struct i915_fbc { >> unsigned long size; >> + unsigned threshold; >> unsigned int fb_id; >> enum plane plane; >> int y; >> @@ -2489,7 +2490,7 @@ static inline void i915_gem_chipset_flush(struct >> drm_device *dev) >> >> /* i915_gem_stolen.c */ >> int i915_gem_init_stolen(struct drm_device *dev); >> -int i915_gem_stolen_setup_compression(struct drm_device *dev, int size); >> +int i915_gem_stolen_setup_compression(struct drm_device *dev, int size, >> int fb_cpp); >> void i915_gem_stolen_cleanup_compression(struct drm_device *dev); >> void i915_gem_cleanup_stolen(struct drm_device *dev); >> struct drm_i915_gem_object * >> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c >> b/drivers/gpu/drm/i915/i915_gem_stolen.c >> index a86b331..b695d18 100644 >> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c >> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c >> @@ -105,35 +105,61 @@ static unsigned long i915_stolen_to_physical(struct >> drm_device *dev) >> >> static int find_compression_threshold(struct drm_device *dev, >> struct drm_mm_node *node, >> - int size) >> + int size, >> + int fb_cpp) >> { >> struct drm_i915_private *dev_priv = dev->dev_private; >> - const int compression_threshold = 1; >> + int compression_threshold = 1; >> int ret; >> >> - /* Try to over-allocate to reduce reallocations and fragmentation >> */ >> + /* HACK: This code depends on what we will do in *_enable_fbc. If >> that >> + * code changes, this code needs to change as well. >> + * >> + * The enable_fbc code will attempt to use one of our 2 compression >> + * thresholds, therefore, in that case, we only have 1 resort. >> + */ >> + >> + /* Try to over-allocate to reduce reallocations and fragmentation. >> */ >> ret = drm_mm_insert_node(&dev_priv->mm.stolen, node, >> size <<= 1, 4096, DRM_MM_SEARCH_DEFAULT); >> - if (ret) >> - ret = drm_mm_insert_node(&dev_priv->mm.stolen, node, >> - size >>= 1, 4096, >> - DRM_MM_SEARCH_DEFAULT); >> - if (ret) >> + if (ret == 0) >> + return compression_threshold; >> + >> +again: >> + /* HW's ability to limit the CFB is 1:4 */ >> + if (compression_threshold > 4 || >> + (fb_cpp == 2 && compression_threshold == 2)) >> return 0; >> - else >> + >> + ret = drm_mm_insert_node(&dev_priv->mm.stolen, node, >> + size >>= 1, 4096, >> + DRM_MM_SEARCH_DEFAULT); >> + if (ret && INTEL_INFO(dev)->gen <= 4) { >> + return 0; >> + } else if (ret) { >> + compression_threshold <<= 1; >> + goto again; >> + } else { >> return compression_threshold; >> + } >> } >> >> -static int i915_setup_compression(struct drm_device *dev, int size) >> +static int i915_setup_compression(struct drm_device *dev, int size, int >> fb_cpp) >> { >> struct drm_i915_private *dev_priv = dev->dev_private; >> struct drm_mm_node *uninitialized_var(compressed_llb); >> int ret; >> >> ret = find_compression_threshold(dev, &dev_priv->fbc.compressed_fb, >> - size); >> + size, fb_cpp); >> if (!ret) >> goto err_llb; >> + else if (ret > 1) { >> + DRM_INFO("Reducing the compressed framebuffer size. This >> may lead to less power savings than a non-reduced-size. Try to increase >> stolen memory size if available in BIOS.\n"); >> + >> + } >> + >> + dev_priv->fbc.threshold = ret; >> >> if (HAS_PCH_SPLIT(dev)) >> I915_WRITE(ILK_DPFC_CB_BASE, >> dev_priv->fbc.compressed_fb.start); >> @@ -157,7 +183,7 @@ static int i915_setup_compression(struct drm_device >> *dev, int size) >> dev_priv->mm.stolen_base + >> compressed_llb->start); >> } >> >> - dev_priv->fbc.size = size; >> + dev_priv->fbc.size = size / dev_priv->fbc.threshold; >> >> DRM_DEBUG_KMS("reserved %d bytes of contiguous stolen space for >> FBC\n", >> size); >> @@ -172,7 +198,7 @@ err_llb: >> return -ENOSPC; >> } >> >> -int i915_gem_stolen_setup_compression(struct drm_device *dev, int size) >> +int i915_gem_stolen_setup_compression(struct drm_device *dev, int size, >> int fb_cpp) >> { >> struct drm_i915_private *dev_priv = dev->dev_private; >> >> @@ -185,7 +211,7 @@ int i915_gem_stolen_setup_compression(struct >> drm_device *dev, int size) >> /* Release any current block */ >> i915_gem_stolen_cleanup_compression(dev); >> >> - return i915_setup_compression(dev, size); >> + return i915_setup_compression(dev, size, fb_cpp); >> } >> >> void i915_gem_stolen_cleanup_compression(struct drm_device *dev) >> diff --git a/drivers/gpu/drm/i915/intel_pm.c >> b/drivers/gpu/drm/i915/intel_pm.c >> index a90fdbd..4fcb2f7 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -229,9 +229,20 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc) >> >> dpfc_ctl = DPFC_CTL_PLANE(intel_crtc->plane); >> if (drm_format_plane_cpp(fb->pixel_format, 0) == 2) >> + dev_priv->fbc.threshold++; >> + >> + switch (dev_priv->fbc.threshold) { >> + case 4: >> + case 3: >> + dpfc_ctl |= DPFC_CTL_LIMIT_4X; >> + break; >> + case 2: >> dpfc_ctl |= DPFC_CTL_LIMIT_2X; >> - else >> + break; >> + case 1: >> dpfc_ctl |= DPFC_CTL_LIMIT_1X; >> + break; >> + } >> dpfc_ctl |= DPFC_CTL_FENCE_EN; >> if (IS_GEN5(dev)) >> dpfc_ctl |= obj->fence_reg; >> @@ -285,9 +296,21 @@ static void gen7_enable_fbc(struct drm_crtc *crtc) >> >> dpfc_ctl = IVB_DPFC_CTL_PLANE(intel_crtc->plane); >> if (drm_format_plane_cpp(fb->pixel_format, 0) == 2) >> + dev_priv->fbc.threshold++; >> + >> + switch (dev_priv->fbc.threshold) { >> + case 4: >> + case 3: >> + dpfc_ctl |= DPFC_CTL_LIMIT_4X; >> + break; >> + case 2: >> dpfc_ctl |= DPFC_CTL_LIMIT_2X; >> - else >> + break; >> + case 1: >> dpfc_ctl |= DPFC_CTL_LIMIT_1X; >> + break; >> + } >> + >> dpfc_ctl |= IVB_DPFC_CTL_FENCE_EN; >> >> I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN); >> @@ -566,7 +589,8 @@ void intel_update_fbc(struct drm_device *dev) >> if (in_dbg_master()) >> goto out_disable; >> >> - if (i915_gem_stolen_setup_compression(dev, >> intel_fb->obj->base.size)) { >> + if (i915_gem_stolen_setup_compression(dev, >> intel_fb->obj->base.size, >> + >> drm_format_plane_cpp(fb->pixel_format, 0))) { >> if (set_no_fbc_reason(dev_priv, FBC_STOLEN_TOO_SMALL)) >> DRM_DEBUG_KMS("framebuffer too large, disabling >> compression\n"); >> goto out_disable; >> -- >> 1.9.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >> > > > > -- > Rodrigo Vivi > Blog: http://blog.vivi.eng.br -- Jani Nikula, Intel Open Source Technology Center