[Intel-gfx] [PATCH 2/2] drm/i915: fully apply WaSkipStolenMemoryFirstPage
Daniel Vetter
daniel at ffwll.ch
Thu Dec 15 07:54:02 UTC 2016
On Wed, Dec 14, 2016 at 05:55:39PM -0200, Paulo Zanoni wrote:
> Don't even tell the mm allocator to handle the first page of stolen on
> the affected platforms. This means that we won't inherit the FB in
> case the BIOS decides to put it at the start of stolen. But the BIOS
> should not be putting it at the start of stolen since it's going to
> get corrupted. I suppose the bug here is that some pixels at the very
> top of the screen will be corrupted, so it's not exactly easy to
> notice.
>
> We have confirmation that the first page of stolen does actually get
> corrupted, so I really think we should do this in order to avoid any
> possible future headaches, even if that means losing BIOS framebuffer
> inheritance. Let's not use the HW in a way it's not supposed to be
> used.
>
> v2: don't even put the first page on the mm (Chris).
> v3: drm_mm_init() takes size instead of end as argument (Ville).
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.h | 1 +
> drivers/gpu/drm/i915/i915_gem_stolen.c | 34 +++++++++++++---------------------
> drivers/gpu/drm/i915/intel_fbc.c | 6 +++---
> 3 files changed, 17 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 8965bbb..aefc968 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -316,6 +316,7 @@ struct i915_ggtt {
> struct io_mapping mappable; /* Mapping to our CPU mappable region */
>
> size_t stolen_size; /* Total size of stolen memory */
> + size_t stolen_usable_start; /* First page may be unusable. */
> size_t stolen_usable_size; /* Total size minus BIOS reserved */
> size_t stolen_reserved_base;
> size_t stolen_reserved_size;
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index b1c8897..56d7e0b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -54,12 +54,6 @@ int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv,
> if (!drm_mm_initialized(&dev_priv->mm.stolen))
> return -ENODEV;
>
> - /* See the comment at the drm_mm_init() call for more about this check.
> - * WaSkipStolenMemoryFirstPage:bdw+ (incomplete)
> - */
> - if (start < 4096 && INTEL_GEN(dev_priv) >= 8)
> - start = 4096;
> -
> mutex_lock(&dev_priv->mm.stolen_lock);
> ret = drm_mm_insert_node_in_range(&dev_priv->mm.stolen, node, size,
> alignment, start, end,
> @@ -74,10 +68,11 @@ int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
> unsigned alignment)
> {
> struct i915_ggtt *ggtt = &dev_priv->ggtt;
> + u64 start = ggtt->stolen_usable_start;
> + u64 end = start + ggtt->stolen_usable_size;
>
> return i915_gem_stolen_insert_node_in_range(dev_priv, node, size,
> - alignment, 0,
> - ggtt->stolen_usable_size);
> + alignment, start, end);
> }
>
> void i915_gem_stolen_remove_node(struct drm_i915_private *dev_priv,
> @@ -489,20 +484,17 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
> ggtt->stolen_size >> 10,
> (ggtt->stolen_size - reserved_total) >> 10);
>
> - ggtt->stolen_usable_size = ggtt->stolen_size - reserved_total;
> + ggtt->stolen_usable_start = 0;
> + /* WaSkipStolenMemoryFirstPage:bdw+ */
> + if (INTEL_GEN(dev_priv) >= 8)
> + ggtt->stolen_usable_start = 4096;
>
> - /*
> - * Basic memrange allocator for stolen space.
> - *
> - * TODO: Notice that some platforms require us to not use the first page
> - * of the stolen memory but their BIOSes may still put the framebuffer
> - * on the first page. So we don't reserve this page for now because of
> - * that. Our current solution is to just prevent new nodes from being
> - * inserted on the first page - see the check we have at
> - * i915_gem_stolen_insert_node_in_range(). We may want to fix the fbcon
> - * problem later.
> - */
> - drm_mm_init(&dev_priv->mm.stolen, 0, ggtt->stolen_usable_size);
> + ggtt->stolen_usable_size = ggtt->stolen_size - reserved_total -
> + ggtt->stolen_usable_start;
> +
> + /* Basic memrange allocator for stolen space. */
> + drm_mm_init(&dev_priv->mm.stolen, ggtt->stolen_usable_start,
> + ggtt->stolen_usable_size);
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 62f215b..534fcb3c 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -528,7 +528,7 @@ static int find_compression_threshold(struct drm_i915_private *dev_priv,
> struct i915_ggtt *ggtt = &dev_priv->ggtt;
> int compression_threshold = 1;
> int ret;
> - u64 end;
> + u64 start = ggtt->stolen_usable_start, end;
>
> /* The FBC hardware for BDW/SKL doesn't have access to the stolen
> * reserved range size, so it always assumes the maximum (8mb) is used.
> @@ -538,7 +538,7 @@ static int find_compression_threshold(struct drm_i915_private *dev_priv,
> IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
> end = ggtt->stolen_size - 8 * 1024 * 1024;
> else
> - end = ggtt->stolen_usable_size;
> + end = start + ggtt->stolen_usable_size;
Funny mix&match of size end end we have :( We might want to switch
drm_mm_init to start/end for consistency with all the _in_range functions.
Let's see whether I'll regret this too ...
Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
>
> /* HACK: This code depends on what we will do in *_enable_fbc. If that
> * code changes, this code needs to change as well.
> @@ -549,7 +549,7 @@ static int find_compression_threshold(struct drm_i915_private *dev_priv,
>
> /* Try to over-allocate to reduce reallocations and fragmentation. */
> ret = i915_gem_stolen_insert_node_in_range(dev_priv, node, size <<= 1,
> - 4096, 0, end);
> + 4096, start, end);
> if (ret == 0)
> return compression_threshold;
>
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the Intel-gfx
mailing list