diff mbox

[v4] drm/i915: Limit Valleyview and earlier to only using mappable scanout

Message ID 20161107110128.28762-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson Nov. 7, 2016, 11:01 a.m. UTC
Valleyview appears tobe limited to only scanning out from the first 512MiB
of the Global GTT. Lets presume that this behaviour was inherited from the
display block copied from g4x (not Ironlake) and all earlier generations
are similarly affected, thought testing suggests different symptoms. For
simplicity, impose that these platforms must scanout from the mappable
region. (For extra simplicity, use HAS_GMCH_DISPLAY even though this
catches Cherryview which does not appear to be limited to the low
aperture for its scanout.)

v2: Use HAS_GMCH_DISPLAY() to more clearly convey my intent about
limiting this workaround to the old style of display engine.

v3: Update change log to reflect testing by Ville Syrjälä
v4: Include the changes to the comments as well

Reported-by: Luis Botello <luis.botello.ortega@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98036
Fixes: 2efb813d5388 ("drm/i915: Fallback to using unmappable memory for scanout")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Akash Goel <akash.goel@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.9-rc1+
---
 drivers/gpu/drm/i915/i915_gem.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

Comments

Ville Syrjala Nov. 7, 2016, 11:36 a.m. UTC | #1
On Mon, Nov 07, 2016 at 11:01:28AM +0000, Chris Wilson wrote:
> Valleyview appears tobe limited to only scanning out from the first 512MiB
> of the Global GTT. Lets presume that this behaviour was inherited from the
> display block copied from g4x (not Ironlake) and all earlier generations
> are similarly affected, thought testing suggests different symptoms. For
> simplicity, impose that these platforms must scanout from the mappable
> region. (For extra simplicity, use HAS_GMCH_DISPLAY even though this
> catches Cherryview which does not appear to be limited to the low
> aperture for its scanout.)
> 
> v2: Use HAS_GMCH_DISPLAY() to more clearly convey my intent about
> limiting this workaround to the old style of display engine.
> 
> v3: Update change log to reflect testing by Ville Syrjälä
> v4: Include the changes to the comments as well
> 
> Reported-by: Luis Botello <luis.botello.ortega@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98036
> Fixes: 2efb813d5388 ("drm/i915: Fallback to using unmappable memory for scanout")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Akash Goel <akash.goel@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: <drm-intel-fixes@lists.freedesktop.org> # v4.9-rc1+
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 6a2005683f6d..4116809f3ed2 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3673,8 +3673,23 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
>  	if (view->type == I915_GGTT_VIEW_NORMAL)
>  		vma = i915_gem_object_ggtt_pin(obj, view, 0, alignment,
>  					       PIN_MAPPABLE | PIN_NONBLOCK);
> -	if (IS_ERR(vma))
> -		vma = i915_gem_object_ggtt_pin(obj, view, 0, alignment, 0);
> +	if (IS_ERR(vma)) {
> +		struct drm_i915_private *i915 = to_i915(obj->base.dev);
> +		unsigned int flags;
> +
> +		/* Valleyview is definitely limited to scanning out the first
> +		 * 512MiB respectively. Lets presume this behaviour was
                          ^^^^^^^^^^^^

With that word removed this is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +		 * inherited from the g4x display engine and that all earlier
> +		 * gen are similarly limited. Testing suggests that it is a
> +		 * little more complicated than this. For example, Cherryview
> +		 * appears quite happy to scanout from anywhere within its
> +		 * global aperture.
> +		 */
> +		flags = 0;
> +		if (HAS_GMCH_DISPLAY(i915))
> +			flags = PIN_MAPPABLE;
> +		vma = i915_gem_object_ggtt_pin(obj, view, 0, alignment, flags);
> +	}
>  	if (IS_ERR(vma))
>  		goto err_unpin_display;
>  
> -- 
> 2.10.2
Chris Wilson Nov. 7, 2016, 12:28 p.m. UTC | #2
On Mon, Nov 07, 2016 at 01:36:03PM +0200, Ville Syrjälä wrote:
> On Mon, Nov 07, 2016 at 11:01:28AM +0000, Chris Wilson wrote:
> > +		/* Valleyview is definitely limited to scanning out the first
> > +		 * 512MiB respectively. Lets presume this behaviour was
>                           ^^^^^^^^^^^^
> 
> With that word removed this is
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Fixed the grammatical errors and pushed. Thanks,
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6a2005683f6d..4116809f3ed2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3673,8 +3673,23 @@  i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
 	if (view->type == I915_GGTT_VIEW_NORMAL)
 		vma = i915_gem_object_ggtt_pin(obj, view, 0, alignment,
 					       PIN_MAPPABLE | PIN_NONBLOCK);
-	if (IS_ERR(vma))
-		vma = i915_gem_object_ggtt_pin(obj, view, 0, alignment, 0);
+	if (IS_ERR(vma)) {
+		struct drm_i915_private *i915 = to_i915(obj->base.dev);
+		unsigned int flags;
+
+		/* Valleyview is definitely limited to scanning out the first
+		 * 512MiB respectively. Lets presume this behaviour was
+		 * inherited from the g4x display engine and that all earlier
+		 * gen are similarly limited. Testing suggests that it is a
+		 * little more complicated than this. For example, Cherryview
+		 * appears quite happy to scanout from anywhere within its
+		 * global aperture.
+		 */
+		flags = 0;
+		if (HAS_GMCH_DISPLAY(i915))
+			flags = PIN_MAPPABLE;
+		vma = i915_gem_object_ggtt_pin(obj, view, 0, alignment, flags);
+	}
 	if (IS_ERR(vma))
 		goto err_unpin_display;