On Sat, Aug 11, 2012 at 03:41:17PM +0100, Chris Wilson wrote:
> As we may wish to wrap regions preallocated by the BIOS, we need to do
> that before carving out contiguous chunks of stolen space for FBC.
> 
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>

Some comments inline below.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h        |    1 +
>  drivers/gpu/drm/i915/i915_gem_stolen.c |  114 
> +++++++++++++++++---------------
>  drivers/gpu/drm/i915/intel_display.c   |    3 +
>  drivers/gpu/drm/i915/intel_pm.c        |   13 ++--
>  4 files changed, 70 insertions(+), 61 deletions(-)
> 

[snip]

> +int i915_gem_stolen_setup_compression(struct drm_device *dev)
> +{
> +     struct drm_i915_private *dev_priv = dev->dev_private;
> +     struct drm_mm_node *entry;
> +     unsigned long size;
> +
> +     if (dev_priv->mm.stolen_base == 0)
> +             return 0;
> +
> +     if (dev_priv->cfb_size)
> +             return dev_priv->cfb_size;
> +
> +     /* Try to set up FBC with a reasonable compressed buffer size */
> +     size = 0;
> +     list_for_each_entry(entry, &dev_priv->mm.stolen.hole_stack, hole_stack) 
> {
> +             unsigned long hole_start = entry->start + entry->size;
> +             unsigned long hole_end = list_entry(entry->node_list.next,
> +                                                 struct drm_mm_node,
> +                                                 node_list)->start;
> +             unsigned long hole_size = hole_end - hole_start;

This feels a bit to much munging around in drm_mm internals. What about a
drm_mm_for_each_hole(entry, mm, hole_start, hole_end)
#define helper?

> +             if (hole_size > size)
> +                     size = hole_size;
> +     }
> +
> +     /* Try to get a 32M buffer... */
> +     if (size > (36*1024*1024))
> +             size = 32*1024*1024;
> +     else /* fall back to 3/4 of the stolen space */
> +             size = size * 3 / 4;
> +
> +     return i915_setup_compression(dev, size);
>  }
>  
>  void intel_modeset_gem_init(struct drm_device *dev)

[snap]

> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 3021c18..6f0f498 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -438,12 +438,6 @@ void intel_update_fbc(struct drm_device *dev)
>               dev_priv->no_fbc_reason = FBC_MODULE_PARAM;
>               goto out_disable;
>       }
> -     if (intel_fb->obj->base.size > dev_priv->cfb_size) {
> -             DRM_DEBUG_KMS("framebuffer too large, disabling "
> -                           "compression\n");
> -             dev_priv->no_fbc_reason = FBC_STOLEN_TOO_SMALL;
> -             goto out_disable;
> -     }
>       if ((crtc->mode.flags & DRM_MODE_FLAG_INTERLACE) ||
>           (crtc->mode.flags & DRM_MODE_FLAG_DBLSCAN)) {
>               DRM_DEBUG_KMS("mode incompatible with compression, "
> @@ -477,6 +471,13 @@ void intel_update_fbc(struct drm_device *dev)
>       if (in_dbg_master())
>               goto out_disable;
>  
> +     if (intel_fb->obj->base.size > i915_gem_stolen_setup_compression(dev)) {
> +             DRM_DEBUG_KMS("framebuffer too large, disabling "
> +                           "compression\n");
> +             dev_priv->no_fbc_reason = FBC_STOLEN_TOO_SMALL;
> +             goto out_disable;
> +     }

I couldn't figure out why this block here had to move ... please enlighten
me.

> +
>       /* If the scanout has not changed, don't modify the FBC settings.
>        * Note that we make the fundamental assumption that the fb->obj
>        * cannot be unpinned (and have its GTT offset and fence revoked)
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to