Re: [PATCH] drm/i915: Fill unused GGTT with scratch pages for VT-d

[David Weinehall <david.weinehall@linux.intel.com>]

On Fri, Jun 24, 2016 at 02:07:14PM +0100, Chris Wilson wrote:
> One of the numerous VT-d workarounds we require is that the display
> hardware reads past the end of the buffer triggering VT-d faults. This
> is acknowledged in the code as being safe "since we fill the unused
> portions of the GGTT with the scratch page". Alas, that is no longer
> always true and so we trigger DMAR read faults.
> 
> Skylake also requires another workaround to avoid mixing VT-d and
> unpopulated PTE, and so there we also need to ensure we fill unused
> entries with the scratch page.

LGTM.

Reviewed-by: David Weinehall <david.weinehall@linux.intel.com>

> 
> Reported-by: Mike Lothian <mike@fireburn.co.uk>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96584
> Fixes: f7770bfd9fd2 ("drm/i915: Skip clearing the GGTT on full-ppgtt systems")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: David Weinehall <david.weinehall@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  9 +++++++++
>  drivers/gpu/drm/i915/i915_gem_gtt.c  |  2 +-
>  drivers/gpu/drm/i915/intel_display.c | 11 +----------
>  3 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 442386abd516..217ca3c9b1c6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2857,6 +2857,15 @@ struct drm_i915_cmd_table {
>  
>  #include "i915_trace.h"
>  
> +static inline bool intel_scanout_needs_vtd_wa(struct drm_i915_private *dev_priv)
> +{
> +#ifdef CONFIG_INTEL_IOMMU
> +	if (INTEL_GEN(dev_priv) >= 6 && intel_iommu_gfx_mapped)
> +		return true;
> +#endif
> +	return false;
> +}
> +
>  extern int i915_suspend_switcheroo(struct drm_device *dev, pm_message_t state);
>  extern int i915_resume_switcheroo(struct drm_device *dev);
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index a8be1c2a8b9e..033fe10768e0 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2608,7 +2608,7 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
>  	ggtt->base.unbind_vma = ggtt_unbind_vma;
>  	ggtt->base.insert_page = gen8_ggtt_insert_page;
>  	ggtt->base.clear_range = nop_clear_range;
> -	if (!USES_FULL_PPGTT(dev_priv))
> +	if (!USES_FULL_PPGTT(dev_priv) || intel_scanout_needs_vtd_wa(dev_priv))
>  		ggtt->base.clear_range = gen8_ggtt_clear_range;
>  
>  	ggtt->base.insert_entries = gen8_ggtt_insert_entries;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a8b0ec13dfe9..8145b65c4262 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2071,15 +2071,6 @@ static void intel_disable_pipe(struct intel_crtc *crtc)
>  		intel_wait_for_pipe_off(crtc);
>  }
>  
> -static bool need_vtd_wa(struct drm_device *dev)
> -{
> -#ifdef CONFIG_INTEL_IOMMU
> -	if (INTEL_INFO(dev)->gen >= 6 && intel_iommu_gfx_mapped)
> -		return true;
> -#endif
> -	return false;
> -}
> -
>  static unsigned int intel_tile_size(const struct drm_i915_private *dev_priv)
>  {
>  	return IS_GEN2(dev_priv) ? 2048 : 4096;
> @@ -2262,7 +2253,7 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
>  	 * we should always have valid PTE following the scanout preventing
>  	 * the VT-d warning.
>  	 */
> -	if (need_vtd_wa(dev) && alignment < 256 * 1024)
> +	if (intel_scanout_needs_vtd_wa(dev_priv) && alignment < 256 * 1024)
>  		alignment = 256 * 1024;
>  
>  	/*
> -- 
> 2.8.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx