[Intel-gfx] [PATCH] drm/i915: Limit Valleyview and earlier to only using mappable scanout

[Chris Wilson]

On Fri, Nov 04, 2016 at 01:29:04PM +0200, Jani Nikula wrote:
> On Fri, 04 Nov 2016, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> > Valleyview and Cherryview are definitely limited to only scanning out
> > from the first 256MiB and 512MiB of the Global GTT respectively. Lets
> > presume that this behaviour was inherited from the display block copied
> > from g4x (not Ironlake) and all earlier generations are similarly
> > affected. For simplicity, impose that these platforms must scanout from
> > the mappable region.
> >
> > Reported-by: Luis Botello <luis.botello.ortega at 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 at chris-wilson.co.uk>
> > Cc: Akash Goel <akash.goel at intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > Cc: <drm-intel-fixes at lists.freedesktop.org> # v4.9-rc1+
> > ---
> > This leaves Ironlake -> Haswell with a bit of uncertainity. It is also
> > not clear if the scanout accessible region is similarly limited on all
> > gen8+, and so whether we need to similarly curtain the upper range for
> > their scanouts.
> > ---
> >  drivers/gpu/drm/i915/i915_gem.c | 18 ++++++++++++++++--
> >  1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 269e2487c104..408875fbec66 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -3661,8 +3661,22 @@ 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 and Cherryview are definitely limited to scanning
> > +		 * out the first 256MiB and 512MiB respectively. Lets presume
> > +		 * this behaviour was inherited from their g4x display engine
> > +		 * and that all earlier gen are similarly limited.
> > +		 */
> > +		flags = 0;
> > +		if (INTEL_GEN(i915) < 5 ||
> > +		    IS_VALLEYVIEW(i915) ||
> > +		    IS_CHERRYVIEW(i915))
> 
> Since it's related to the display engine, HAS_GMCH_DISPLAY()?

Ah, that's synonym I was thinking off. That describes the split I used
here much better. We may need to refine this as more information
becomes available (if ever!)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre