Re: [PATCH] drm/i915: Try harder to get FBC

[Jani Nikula <jani.nikula@intel.com>]

On Tue, 01 Jul 2014, Rodrigo Vivi <rodrigo.vivi@gmail.com> 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 <rodrigo.vivi@intel.com>
> wrote:
>
>> From: Ben Widawsky <benjamin.widawsky@intel.com>
>>
>> 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 <rodrigo.vivi@intel.com>
>> Cc: Art Runyan <arthur.j.runyan@intel.com>
>> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> ---
>>  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