All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Limit Valleyview and earlier to only using mappable scanout
@ 2016-11-04 11:08 Chris Wilson
  2016-11-04 11:29 ` Jani Nikula
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Chris Wilson @ 2016-11-04 11:08 UTC (permalink / raw)
  To: intel-gfx; +Cc: Akash Goel, # v4 . 9-rc1+

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@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: <drm-intel-fixes@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))
+			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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915: Limit Valleyview and earlier to only using mappable scanout
  2016-11-04 11:08 [PATCH] drm/i915: Limit Valleyview and earlier to only using mappable scanout Chris Wilson
@ 2016-11-04 11:29 ` Jani Nikula
  2016-11-04 11:41   ` Chris Wilson
  2016-11-04 11:45 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Jani Nikula @ 2016-11-04 11:29 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: Akash Goel, # v4 . 9-rc1+

On Fri, 04 Nov 2016, Chris Wilson <chris@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@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: <drm-intel-fixes@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()?

BR,
Jani.

> +			flags = PIN_MAPPABLE;
> +		vma = i915_gem_object_ggtt_pin(obj, view, 0, alignment, flags);
> +	}
>  	if (IS_ERR(vma))
>  		goto err_unpin_display;

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915: Limit Valleyview and earlier to only using mappable scanout
  2016-11-04 11:29 ` Jani Nikula
@ 2016-11-04 11:41   ` Chris Wilson
  0 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2016-11-04 11:41 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, # v4 . 9-rc1+, Akash Goel

On Fri, Nov 04, 2016 at 01:29:04PM +0200, Jani Nikula wrote:
> On Fri, 04 Nov 2016, Chris Wilson <chris@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@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: <drm-intel-fixes@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Limit Valleyview and earlier to only using mappable scanout
  2016-11-04 11:08 [PATCH] drm/i915: Limit Valleyview and earlier to only using mappable scanout Chris Wilson
  2016-11-04 11:29 ` Jani Nikula
@ 2016-11-04 11:45 ` Patchwork
  2016-11-04 12:59 ` [PATCH] " Tvrtko Ursulin
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2016-11-04 11:45 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Limit Valleyview and earlier to only using mappable scanout
URL   : https://patchwork.freedesktop.org/series/14835/
State : success

== Summary ==

Series 14835v1 drm/i915: Limit Valleyview and earlier to only using mappable scanout
https://patchwork.freedesktop.org/api/1.0/series/14835/revisions/1/mbox/

Test kms_force_connector_basic:
        Subgroup force-load-detect:
                dmesg-warn -> PASS       (fi-snb-2520m)

fi-bdw-5557u     total:241  pass:226  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:241  pass:201  dwarn:0   dfail:0   fail:0   skip:40 
fi-bxt-t5700     total:241  pass:213  dwarn:0   dfail:0   fail:0   skip:28 
fi-byt-j1900     total:241  pass:213  dwarn:0   dfail:0   fail:0   skip:28 
fi-byt-n2820     total:241  pass:209  dwarn:0   dfail:0   fail:0   skip:32 
fi-hsw-4770      total:241  pass:221  dwarn:0   dfail:0   fail:0   skip:20 
fi-hsw-4770r     total:241  pass:221  dwarn:0   dfail:0   fail:0   skip:20 
fi-ilk-650       total:241  pass:188  dwarn:0   dfail:0   fail:0   skip:53 
fi-ivb-3520m     total:241  pass:219  dwarn:0   dfail:0   fail:0   skip:22 
fi-ivb-3770      total:241  pass:219  dwarn:0   dfail:0   fail:0   skip:22 
fi-kbl-7200u     total:241  pass:219  dwarn:0   dfail:0   fail:0   skip:22 
fi-skl-6260u     total:241  pass:227  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:241  pass:220  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6700k     total:241  pass:219  dwarn:1   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:241  pass:227  dwarn:0   dfail:0   fail:0   skip:14 
fi-snb-2520m     total:241  pass:209  dwarn:0   dfail:0   fail:0   skip:32 
fi-snb-2600      total:241  pass:208  dwarn:0   dfail:0   fail:0   skip:33 

21f242e536b5077c046df785a8c4c28374941c15 drm-intel-nightly: 2016y-11m-03d-21h-01m-03s UTC integration manifest
b9cc4a5 drm/i915: Limit Valleyview and earlier to only using mappable scanout

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_2903/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915: Limit Valleyview and earlier to only using mappable scanout
  2016-11-04 11:08 [PATCH] drm/i915: Limit Valleyview and earlier to only using mappable scanout Chris Wilson
  2016-11-04 11:29 ` Jani Nikula
  2016-11-04 11:45 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2016-11-04 12:59 ` Tvrtko Ursulin
  2016-11-04 13:06   ` Chris Wilson
  2016-11-07 10:27 ` Ville Syrjälä
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Tvrtko Ursulin @ 2016-11-04 12:59 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: Akash Goel, # v4 . 9-rc1+


On 04/11/2016 11:08, Chris Wilson 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@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: <drm-intel-fixes@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);

dev_priv ?

What do we do with i915_params being a global i915?

Regards,

Tvrtko

> +		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))
> +			flags = PIN_MAPPABLE;
> +		vma = i915_gem_object_ggtt_pin(obj, view, 0, alignment, flags);
> +	}
>  	if (IS_ERR(vma))
>  		goto err_unpin_display;
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915: Limit Valleyview and earlier to only using mappable scanout
  2016-11-04 12:59 ` [PATCH] " Tvrtko Ursulin
@ 2016-11-04 13:06   ` Chris Wilson
  2016-11-04 14:17     ` Jani Nikula
  0 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2016-11-04 13:06 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx, # v4 . 9-rc1+, Akash Goel

On Fri, Nov 04, 2016 at 12:59:08PM +0000, Tvrtko Ursulin wrote:
> 
> On 04/11/2016 11:08, Chris Wilson 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@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: <drm-intel-fixes@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);
> 
> dev_priv ?
> 
> What do we do with i915_params being a global i915?

Sssh, I'm gradually waging war against dev_priv.
Eventually Jani won't be able to complain about i915 being the minority.

The global modparams is an easy rename.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915: Limit Valleyview and earlier to only using mappable scanout
  2016-11-04 13:06   ` Chris Wilson
@ 2016-11-04 14:17     ` Jani Nikula
  0 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2016-11-04 14:17 UTC (permalink / raw)
  To: Chris Wilson, Tvrtko Ursulin; +Cc: intel-gfx, # v4 . 9-rc1+, Akash Goel

On Fri, 04 Nov 2016, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Fri, Nov 04, 2016 at 12:59:08PM +0000, Tvrtko Ursulin wrote:
>> 
>> On 04/11/2016 11:08, Chris Wilson 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@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: <drm-intel-fixes@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);
>> 
>> dev_priv ?
>> 
>> What do we do with i915_params being a global i915?
>
> Sssh, I'm gradually waging war against dev_priv.
> Eventually Jani won't be able to complain about i915 being the minority.
>
> The global modparams is an easy rename.

I just liked that i915.foo was the same on both the kernel command line
and in code. I kinda still do, but like Chris I'm not too fond of
dev_priv either, and i915 seems like a good replacement.

Seeing how module parameters multiply like rabbits, with all sorts of
sanitization, how the parameters are changed in kernel, and
/sys/module/i915/parameters/ not reflecting what the user did, maybe you
could come up with something nice for that while at it...

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] drm/i915: Limit Valleyview and earlier to only using mappable scanout
  2016-11-04 11:08 [PATCH] drm/i915: Limit Valleyview and earlier to only using mappable scanout Chris Wilson
                   ` (2 preceding siblings ...)
  2016-11-04 12:59 ` [PATCH] " Tvrtko Ursulin
@ 2016-11-07 10:27 ` Ville Syrjälä
  2016-11-07 11:00 ` [PATCH v3] " Chris Wilson
  2016-11-07 11:01 ` [PATCH v4] " Chris Wilson
  5 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2016-11-07 10:27 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx, # v4 . 9-rc1+, Akash Goel

On Fri, Nov 04, 2016 at 11:08:38AM +0000, Chris Wilson wrote:
> Valleyview and Cherryview are definitely limited to only scanning out
> from the first 256MiB and 512MiB of the Global GTT respectively.

Actually no. My VLVs (B3 and C0 steppings should that matter) would
appear to be good up to 512MiB. The access just wrap every 512MiB,
so I presume it just ignores the 3 msbs.

My CHV seemed good all the way up to 4 GiB.

When I tried the same test on an ELK I got some strange results. On
the first run it seemed to handle things up to 512MiB or maybe even past
it, but then once my test went back to lower addresses something fishy
happened and I started to see failures. I need to figure out if my test
is just buggy on ELK or if the hw gets semi-permanently confused when
you feed it large addresses.

> 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@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: <drm-intel-fixes@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))
> +			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
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3] drm/i915: Limit Valleyview and earlier to only using mappable scanout
  2016-11-04 11:08 [PATCH] drm/i915: Limit Valleyview and earlier to only using mappable scanout Chris Wilson
                   ` (3 preceding siblings ...)
  2016-11-07 10:27 ` Ville Syrjälä
@ 2016-11-07 11:00 ` Chris Wilson
  2016-11-07 11:01 ` [PATCH v4] " Chris Wilson
  5 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2016-11-07 11:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: Akash Goel, # v4 . 9-rc1+

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ä

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 | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6a2005683f6d..b50420b277fe 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3673,8 +3673,20 @@ 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 (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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v4] drm/i915: Limit Valleyview and earlier to only using mappable scanout
  2016-11-04 11:08 [PATCH] drm/i915: Limit Valleyview and earlier to only using mappable scanout Chris Wilson
                   ` (4 preceding siblings ...)
  2016-11-07 11:00 ` [PATCH v3] " Chris Wilson
@ 2016-11-07 11:01 ` Chris Wilson
  2016-11-07 11:36   ` Ville Syrjälä
  5 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2016-11-07 11:01 UTC (permalink / raw)
  To: intel-gfx; +Cc: Akash Goel, # v4 . 9-rc1+

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
+		 * 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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] drm/i915: Limit Valleyview and earlier to only using mappable scanout
  2016-11-07 11:01 ` [PATCH v4] " Chris Wilson
@ 2016-11-07 11:36   ` Ville Syrjälä
  2016-11-07 12:28     ` Chris Wilson
  0 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2016-11-07 11:36 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx, # v4 . 9-rc1+, Akash Goel

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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] drm/i915: Limit Valleyview and earlier to only using mappable scanout
  2016-11-07 11:36   ` Ville Syrjälä
@ 2016-11-07 12:28     ` Chris Wilson
  0 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2016-11-07 12:28 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, # v4 . 9-rc1+, Akash Goel

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

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-11-07 12:28 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-04 11:08 [PATCH] drm/i915: Limit Valleyview and earlier to only using mappable scanout Chris Wilson
2016-11-04 11:29 ` Jani Nikula
2016-11-04 11:41   ` Chris Wilson
2016-11-04 11:45 ` ✓ Fi.CI.BAT: success for " Patchwork
2016-11-04 12:59 ` [PATCH] " Tvrtko Ursulin
2016-11-04 13:06   ` Chris Wilson
2016-11-04 14:17     ` Jani Nikula
2016-11-07 10:27 ` Ville Syrjälä
2016-11-07 11:00 ` [PATCH v3] " Chris Wilson
2016-11-07 11:01 ` [PATCH v4] " Chris Wilson
2016-11-07 11:36   ` Ville Syrjälä
2016-11-07 12:28     ` Chris Wilson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.