[PATCH] drm/i915: Apply alignment restrictions on scanout surfaces for VT-d

[PATCH] drm/i915: Apply alignment restrictions on scanout surfaces for VT-d

[Chris Wilson]

>From the w/a database:

'To prevent false VT-d type 6 error:

  The primary display plane must be 256KiB aligned, and require an extra
  128 PTEs of padding afterward;

  The sprites planes must be 128KiB aligned, and require an extra 64 PTEs
  of padding afterward;

  The cursors must be 64KiB aligned, and require an extra 2 PTEs of
  padding afterward.'

As we use the same function to pin the primary and sprite planes, we can
simply use the more strict requirements for scanouts for both.

Instead of using explicit padding PTEs following the scanout objects, we
should be able to use the scratch page that is always mapped into the
unused PTEs to avoid the VT-d error.

References: https://bugs.freedesktop.org/show_bug.cgi?id=59626
References: https://bugs.freedesktop.org/show_bug.cgi?id=59627
References: https://bugs.freedesktop.org/show_bug.cgi?id=59631
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c |   30 +++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_sprite.c  |    5 +++++
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a8966aa..139213b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1937,6 +1937,15 @@ static void intel_disable_plane(struct drm_i915_private *dev_priv,
 	intel_wait_for_vblank(dev_priv->dev, pipe);
 }
 
+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;
+}
+
 int
 intel_pin_and_fence_fb_obj(struct drm_device *dev,
 			   struct drm_i915_gem_object *obj,
@@ -1967,6 +1976,14 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
 		BUG();
 	}
 
+	/* Note that the w/a also requires 64 PTE of padding following the
+	 * bo. We currently fill all unused PTE with the shadow page and so
+	 * we should always have valid PTE following the scanout preventing
+	 * the VT-d warning.
+	 */
+	if (need_vtd_wa(dev) && alignment < 256 * 1024)
+		alignment = 256 * 1024;
+
 	dev_priv->mm.interruptible = false;
 	ret = i915_gem_object_pin_to_display_plane(obj, alignment, pipelined);
 	if (ret)
@@ -6308,13 +6325,24 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
 	/* we only need to pin inside GTT if cursor is non-phy */
 	mutex_lock(&dev->struct_mutex);
 	if (!dev_priv->info->cursor_needs_physical) {
+		unsigned alignment;
+
 		if (obj->tiling_mode) {
 			DRM_ERROR("cursor cannot be tiled\n");
 			ret = -EINVAL;
 			goto fail_locked;
 		}
 
-		ret = i915_gem_object_pin_to_display_plane(obj, 0, NULL);
+		/* Note that the w/a also requires 2 PTE of padding following
+		 * the bo. We currently fill all unused PTE with the shadow
+		 * page and so we should always have valid PTE following the
+		 * cursor preventing the VT-d warning.
+		 */
+		alignment = 0;
+		if (need_vtd_wa(dev))
+			alignment = 64*1024;
+
+		ret = i915_gem_object_pin_to_display_plane(obj, alignment, NULL);
 		if (ret) {
 			DRM_ERROR("failed to move cursor bo into the GTT\n");
 			goto fail_locked;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 1b6eb76..d28525b 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -513,6 +513,11 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 
 	mutex_lock(&dev->struct_mutex);
 
+	/* Note that this will apply the VT-d workaround for scanouts,
+	 * which is more restrictive than required for sprites. (The
+	 * primary plane requires 256KiB alignment with 64 PTE padding,
+	 * the sprite planes only require 128KiB alignment and 32 PTE padding.
+	 */
 	ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
 	if (ret)
 		goto out_unlock;
-- 
1.7.10.4

Re: [PATCH] drm/i915: Apply alignment restrictions on scanout surfaces for VT-d

[Daniel Vetter]

On Tue, Mar 05, 2013 at 02:52:39PM +0000, Chris Wilson wrote:
> From the w/a database:
> 
> 'To prevent false VT-d type 6 error:
> 
>   The primary display plane must be 256KiB aligned, and require an extra
>   128 PTEs of padding afterward;
> 
>   The sprites planes must be 128KiB aligned, and require an extra 64 PTEs
>   of padding afterward;
> 
>   The cursors must be 64KiB aligned, and require an extra 2 PTEs of
>   padding afterward.'
> 
> As we use the same function to pin the primary and sprite planes, we can
> simply use the more strict requirements for scanouts for both.
> 
> Instead of using explicit padding PTEs following the scanout objects, we
> should be able to use the scratch page that is always mapped into the
> unused PTEs to avoid the VT-d error.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=59626
> References: https://bugs.freedesktop.org/show_bug.cgi?id=59627
> References: https://bugs.freedesktop.org/show_bug.cgi?id=59631
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_display.c |   30 +++++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/intel_sprite.c  |    5 +++++
>  2 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a8966aa..139213b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1937,6 +1937,15 @@ static void intel_disable_plane(struct drm_i915_private *dev_priv,
>  	intel_wait_for_vblank(dev_priv->dev, pipe);
>  }
>  
> +static bool need_vtd_wa(struct drm_device *dev)

Bikeshed to need_vtd_scanout_wa?
-Daniel

> +{
> +#ifdef CONFIG_INTEL_IOMMU
> +	if (INTEL_INFO(dev)->gen >= 6 && intel_iommu_gfx_mapped)
> +		return true;
> +#endif
> +	return false;
> +}
> +
>  int
>  intel_pin_and_fence_fb_obj(struct drm_device *dev,
>  			   struct drm_i915_gem_object *obj,
> @@ -1967,6 +1976,14 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
>  		BUG();
>  	}
>  
> +	/* Note that the w/a also requires 64 PTE of padding following the
> +	 * bo. We currently fill all unused PTE with the shadow page and so
> +	 * we should always have valid PTE following the scanout preventing
> +	 * the VT-d warning.
> +	 */
> +	if (need_vtd_wa(dev) && alignment < 256 * 1024)
> +		alignment = 256 * 1024;
> +
>  	dev_priv->mm.interruptible = false;
>  	ret = i915_gem_object_pin_to_display_plane(obj, alignment, pipelined);
>  	if (ret)
> @@ -6308,13 +6325,24 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
>  	/* we only need to pin inside GTT if cursor is non-phy */
>  	mutex_lock(&dev->struct_mutex);
>  	if (!dev_priv->info->cursor_needs_physical) {
> +		unsigned alignment;
> +
>  		if (obj->tiling_mode) {
>  			DRM_ERROR("cursor cannot be tiled\n");
>  			ret = -EINVAL;
>  			goto fail_locked;
>  		}
>  
> -		ret = i915_gem_object_pin_to_display_plane(obj, 0, NULL);
> +		/* Note that the w/a also requires 2 PTE of padding following
> +		 * the bo. We currently fill all unused PTE with the shadow
> +		 * page and so we should always have valid PTE following the
> +		 * cursor preventing the VT-d warning.
> +		 */
> +		alignment = 0;
> +		if (need_vtd_wa(dev))
> +			alignment = 64*1024;
> +
> +		ret = i915_gem_object_pin_to_display_plane(obj, alignment, NULL);
>  		if (ret) {
>  			DRM_ERROR("failed to move cursor bo into the GTT\n");
>  			goto fail_locked;
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 1b6eb76..d28525b 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -513,6 +513,11 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  
>  	mutex_lock(&dev->struct_mutex);
>  
> +	/* Note that this will apply the VT-d workaround for scanouts,
> +	 * which is more restrictive than required for sprites. (The
> +	 * primary plane requires 256KiB alignment with 64 PTE padding,
> +	 * the sprite planes only require 128KiB alignment and 32 PTE padding.
> +	 */
>  	ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
>  	if (ret)
>  		goto out_unlock;
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: Apply alignment restrictions on scanout surfaces for VT-d

[Damien Lespiau]

On Tue, Mar 05, 2013 at 02:52:39PM +0000, Chris Wilson wrote:
> From the w/a database:
> 
> 'To prevent false VT-d type 6 error:
> 
>   The primary display plane must be 256KiB aligned, and require an extra
>   128 PTEs of padding afterward;
> 
>   The sprites planes must be 128KiB aligned, and require an extra 64 PTEs
>   of padding afterward;
> 
>   The cursors must be 64KiB aligned, and require an extra 2 PTEs of
>   padding afterward.'
> 
> As we use the same function to pin the primary and sprite planes, we can
> simply use the more strict requirements for scanouts for both.
> 
> Instead of using explicit padding PTEs following the scanout objects, we
> should be able to use the scratch page that is always mapped into the
> unused PTEs to avoid the VT-d error.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=59626
> References: https://bugs.freedesktop.org/show_bug.cgi?id=59627
> References: https://bugs.freedesktop.org/show_bug.cgi?id=59631
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

Maybe rename the mentions to "shadow page" to scratch page as it's the
name used elsewhere. I also like the even more explicit function name
from Daniel. In any case, sounds like a good patch to have.

-- 
Damien

Re: [PATCH] drm/i915: Apply alignment restrictions on scanout surfaces for VT-d

[Daniel Vetter]

On Tue, Mar 26, 2013 at 11:15:38PM +0000, Damien Lespiau wrote:
> On Tue, Mar 05, 2013 at 02:52:39PM +0000, Chris Wilson wrote:
> > From the w/a database:
> > 
> > 'To prevent false VT-d type 6 error:
> > 
> >   The primary display plane must be 256KiB aligned, and require an extra
> >   128 PTEs of padding afterward;
> > 
> >   The sprites planes must be 128KiB aligned, and require an extra 64 PTEs
> >   of padding afterward;
> > 
> >   The cursors must be 64KiB aligned, and require an extra 2 PTEs of
> >   padding afterward.'
> > 
> > As we use the same function to pin the primary and sprite planes, we can
> > simply use the more strict requirements for scanouts for both.
> > 
> > Instead of using explicit padding PTEs following the scanout objects, we
> > should be able to use the scratch page that is always mapped into the
> > unused PTEs to avoid the VT-d error.
> > 
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=59626
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=59627
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=59631
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

Merged ...

> Maybe rename the mentions to "shadow page" to scratch page as it's the
> name used elsewhere. I also like the even more explicit function name
> from Daniel. In any case, sounds like a good patch to have.

... with that little bikeshed applied.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch