[Intel-gfx] [PATCH] drm/i915: Fix pte updates in ggtt clear range

[Ben Widawsky]

On Mon, 26 Nov 2012 21:52:54 -0800
Ben Widawsky <ben at bwidawsk.net> wrote:

> This bug was introduced by me:
> commit e76e9aebcdbfebae8f4cd147e3c0f800d36e97f3
> Author: Ben Widawsky <ben at bwidawsk.net>
> Date:   Sun Nov 4 09:21:27 2012 -0800
> 
>     drm/i915: Stop using AGP layer for GEN6+
> 
> The existing code uses memset_io which follows memset semantics in only
> guaranteeing a write of individual bytes. Since a PTE entry is 4 bytes,
> this can only be correct if the scratch page address is 0.
> 
> This caused unsightly errors when we clear the range at load time,
> though I'm not really sure what the heck is referencing that memory
> anyway. I caught this is because I believe we have some other bug where
> the display is doing reads of memory we feel should be cleared (or we
> are relying on scratch pages to be a specific value).
> 
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 51f79bb..f7ac61e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -367,8 +367,9 @@ static void i915_ggtt_clear_range(struct drm_device *dev,
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	gtt_pte_t scratch_pte;
> -	volatile void __iomem *gtt_base = dev_priv->mm.gtt->gtt + first_entry;
> +	gtt_pte_t __iomem *gtt_base = dev_priv->mm.gtt->gtt + first_entry;
>  	const int max_entries = dev_priv->mm.gtt->gtt_total_entries - first_entry;
> +	int i;
>  
>  	if (INTEL_INFO(dev)->gen < 6) {
>  		intel_gtt_clear_range(first_entry, num_entries);
> @@ -381,7 +382,8 @@ static void i915_ggtt_clear_range(struct drm_device *dev,
>  		num_entries = max_entries;
>  
>  	scratch_pte = pte_encode(dev, dev_priv->mm.gtt->scratch_page_dma, I915_CACHE_LLC);
> -	memset_io(gtt_base, scratch_pte, num_entries * sizeof(scratch_pte));
> +	for (i = 0; i < num_entries; i++)
> +		iowrite32(scratch_pte, &gtt_base[i]);
>  	readl(gtt_base);
>  }
>  

"This has been slurped into -queued. Thanks for the patch" - or
something like that.

-- 
Ben Widawsky, Intel Open Source Technology Center