[Intel-gfx] [RFC 2/2] drm/i915: fully apply WaSkipStolenMemoryFirstPage

[Chris Wilson]

On Tue, Dec 13, 2016 at 05:57:26PM -0200, Paulo Zanoni wrote:
> Reserve the first page of stolen memory right after initializing the
> mm allocator. This means that we won't inherit the FB in case the BIOS
> decides to put it at the start of stolen. IMHO, avoiding constant
> screen flickering is more important than BIOS framebuffer inheritance.
> 
> TODO: the goal is to ask everybody with screen flickering problems to
> test this patch. Hopefully we'll be able to add a bunch of Bugzilla
> tags here before merging it.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94605
> Cc: stable at vger.kernel.org
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h        |  3 +++
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 30 +++++++++++++-----------------
>  2 files changed, 16 insertions(+), 17 deletions(-)
> 
> 
> Time to spam bugzilla again with links to patchwork. The tester on
> fd.o bug 94605 suggests this may be the fix we've been looking for.
> 
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 0e11ed7..0868a88 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1531,6 +1531,9 @@ struct i915_gem_mm {
>  	/** LRU list of objects with fence regs on them. */
>  	struct list_head fence_list;
>  
> +	/** First page of stolen, for platforms where it's reserved. */
> +	struct drm_mm_node first_page;


> +
>  	/**
>  	 * Are we in a non-interruptible section of code like
>  	 * modesetting?
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index b1c8897..9b3ae58 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,
> @@ -286,6 +280,9 @@ void i915_gem_cleanup_stolen(struct drm_device *dev)
>  	if (!drm_mm_initialized(&dev_priv->mm.stolen))
>  		return;
>  
> +	if (drm_mm_node_allocated(&dev_priv->mm.first_page))
> +		drm_mm_remove_node(&dev_priv->mm.first_page);
> +
>  	drm_mm_takedown(&dev_priv->mm.stolen);
>  }
>  
> @@ -491,19 +488,18 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
>  
>  	ggtt->stolen_usable_size = ggtt->stolen_size - reserved_total;
>  
> -	/*
> -	 * 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.
> -	 */
> +	/* Basic memrange allocator for stolen space. */
>  	drm_mm_init(&dev_priv->mm.stolen, 0, ggtt->stolen_usable_size);
>  
> +	/* WaSkipStolenMemoryFirstPage:bdw+ */
> +	if (INTEL_GEN(dev_priv) >= 8) {
> +		dev_priv->mm.first_page.start = 0;
> +		dev_priv->mm.first_page.size = 4096;
> +		if (drm_mm_reserve_node(&dev_priv->mm.stolen,
> +					&dev_priv->mm.first_page))
> +			DRM_DEBUG_KMS("Failed to reserve the first page of stolen memory.\n");
> +	}

If you are saying that the first page can never be used, then just don't
put into the range allocator.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre