[PATCH] drm/i915: Fallback to using unmappable memory for scanout

[PATCH] drm/i915: Fallback to using unmappable memory for scanout

[Chris Wilson]

The existing ABI says that scanouts are pinned into the mappable region
so that legacy clients (e.g. old Xorg or plymouthd) can write directly
into the scanout through a GTT mapping. However if the surface does not
fit into the mappable region, we are better off just trying to fit it
anywhere and hoping for the best. (Any userspace that is cappable of
using ginormous scanouts is also likely not to rely on pure GTT
updates.) In the future, there may even be a kernel mediated method for
the legacy clients.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Satyanantha, Rama Gopal M <rama.gopal.m.satyanantha@intel.com>
Cc: Deepak S <deepak.s@linux.intel.com>
Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_gem.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9e498e0bbf22..9a1de848e450 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4034,10 +4034,15 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
 
 	/* As the user may map the buffer once pinned in the display plane
 	 * (e.g. libkms for the bootup splash), we have to ensure that we
-	 * always use map_and_fenceable for all scanout buffers.
+	 * always use map_and_fenceable for all scanout buffers. However,
+	 * it may simply be too big to fit into mappable, in which case
+	 * put it anyway and hope that userspace can cope (but always first
+	 * try to preserve the existing ABI).
 	 */
 	ret = i915_gem_obj_ggtt_pin(obj, alignment, PIN_MAPPABLE);
 	if (ret)
+		ret = i915_gem_obj_ggtt_pin(obj, alignment, 0);
+	if (ret)
 		goto err_unpin_display;
 
 	i915_gem_object_flush_cpu_write_domain(obj);
-- 
2.1.4

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

Re: [PATCH] drm/i915: Fallback to using unmappable memory for scanout

[Deepak S]

On Thursday 19 March 2015 03:38 AM, Chris Wilson wrote:
> The existing ABI says that scanouts are pinned into the mappable region
> so that legacy clients (e.g. old Xorg or plymouthd) can write directly
> into the scanout through a GTT mapping. However if the surface does not
> fit into the mappable region, we are better off just trying to fit it
> anywhere and hoping for the best. (Any userspace that is cappable of
> using ginormous scanouts is also likely not to rely on pure GTT
> updates.) In the future, there may even be a kernel mediated method for
> the legacy clients.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Satyanantha, Rama Gopal M <rama.gopal.m.satyanantha@intel.com>
> Cc: Deepak S <deepak.s@linux.intel.com>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>   drivers/gpu/drm/i915/i915_gem.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 9e498e0bbf22..9a1de848e450 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4034,10 +4034,15 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
>   
>   	/* As the user may map the buffer once pinned in the display plane
>   	 * (e.g. libkms for the bootup splash), we have to ensure that we
> -	 * always use map_and_fenceable for all scanout buffers.
> +	 * always use map_and_fenceable for all scanout buffers. However,
> +	 * it may simply be too big to fit into mappable, in which case
> +	 * put it anyway and hope that userspace can cope (but always first
> +	 * try to preserve the existing ABI).
>   	 */
>   	ret = i915_gem_obj_ggtt_pin(obj, alignment, PIN_MAPPABLE);
>   	if (ret)
> +		ret = i915_gem_obj_ggtt_pin(obj, alignment, 0);
> +	if (ret)
>   		goto err_unpin_display;
>   

Hi Chris,

if we map the object into unmappable region. I think we should skip fence create ?

Thanks
Deepak

>   	i915_gem_object_flush_cpu_write_domain(obj);

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

Re: [PATCH] drm/i915: Fallback to using unmappable memory for scanout

[Chris Wilson]

On Thu, Mar 19, 2015 at 04:23:32PM +0530, Deepak S wrote:
> Hi Chris,
> 
> if we map the object into unmappable region. I think we should skip fence create ?

We should just ignore the failure. Whether or not we want the fence
pinned is a matter for the FBC code, which is a different level.

But true, it should be fixed in this patch as well.
-Chris

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

[PATCH v2] drm/i915: Fallback to using unmappable memory for scanout

[Chris Wilson]

The existing ABI says that scanouts are pinned into the mappable region
so that legacy clients (e.g. old Xorg or plymouthd) can write directly
into the scanout through a GTT mapping. However if the surface does not
fit into the mappable region, we are better off just trying to fit it
anywhere and hoping for the best. (Any userspace that is cappable of
using ginormous scanouts is also likely not to rely on pure GTT
updates.) In the future, there may even be a kernel mediated method for
the legacy clients.

v2: Skip fence pinning when not mappable.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Satyanantha, Rama Gopal M <rama.gopal.m.satyanantha@intel.com>
Cc: Deepak S <deepak.s@linux.intel.com>
Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_gem.c      |  7 ++++++-
 drivers/gpu/drm/i915/intel_display.c | 23 +++++++++++++----------
 2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9e498e0bbf22..9a1de848e450 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4034,10 +4034,15 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
 
 	/* As the user may map the buffer once pinned in the display plane
 	 * (e.g. libkms for the bootup splash), we have to ensure that we
-	 * always use map_and_fenceable for all scanout buffers.
+	 * always use map_and_fenceable for all scanout buffers. However,
+	 * it may simply be too big to fit into mappable, in which case
+	 * put it anyway and hope that userspace can cope (but always first
+	 * try to preserve the existing ABI).
 	 */
 	ret = i915_gem_obj_ggtt_pin(obj, alignment, PIN_MAPPABLE);
 	if (ret)
+		ret = i915_gem_obj_ggtt_pin(obj, alignment, 0);
+	if (ret)
 		goto err_unpin_display;
 
 	i915_gem_object_flush_cpu_write_domain(obj);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d621ebecd33e..628aace63b43 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2308,16 +2308,18 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
 	if (ret)
 		goto err_interruptible;
 
-	/* Install a fence for tiled scan-out. Pre-i965 always needs a
-	 * fence, whereas 965+ only requires a fence if using
-	 * framebuffer compression.  For simplicity, we always install
-	 * a fence as the cost is not that onerous.
-	 */
-	ret = i915_gem_object_get_fence(obj);
-	if (ret)
-		goto err_unpin;
+	if (obj->map_and_fenceable) {
+		/* Install a fence for tiled scan-out. Pre-i965 always needs a
+		 * fence, whereas 965+ only requires a fence if using
+		 * framebuffer compression.  For simplicity, we always, when
+		 * possible, install a fence as the cost is not that onerous.
+		 */
+		ret = i915_gem_object_get_fence(obj);
+		if (ret)
+			goto err_unpin;
 
-	i915_gem_object_pin_fence(obj);
+		i915_gem_object_pin_fence(obj);
+	}
 
 	dev_priv->mm.interruptible = true;
 	intel_runtime_pm_put(dev_priv);
@@ -2335,7 +2337,8 @@ static void intel_unpin_fb_obj(struct drm_i915_gem_object *obj)
 {
 	WARN_ON(!mutex_is_locked(&obj->base.dev->struct_mutex));
 
-	i915_gem_object_unpin_fence(obj);
+	if (obj->map_and_fenceable)
+		i915_gem_object_unpin_fence(obj);
 	i915_gem_object_unpin_from_display_plane(obj);
 }
 
-- 
2.1.4

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

Re: [PATCH v2] drm/i915: Fallback to using unmappable memory for scanout

[Deepak S]

On Thursday 19 March 2015 04:59 PM, Chris Wilson wrote:
> The existing ABI says that scanouts are pinned into the mappable region
> so that legacy clients (e.g. old Xorg or plymouthd) can write directly
> into the scanout through a GTT mapping. However if the surface does not
> fit into the mappable region, we are better off just trying to fit it
> anywhere and hoping for the best. (Any userspace that is cappable of
> using ginormous scanouts is also likely not to rely on pure GTT
> updates.) In the future, there may even be a kernel mediated method for
> the legacy clients.
>
> v2: Skip fence pinning when not mappable.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Satyanantha, Rama Gopal M <rama.gopal.m.satyanantha@intel.com>
> Cc: Deepak S <deepak.s@linux.intel.com>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>   drivers/gpu/drm/i915/i915_gem.c      |  7 ++++++-
>   drivers/gpu/drm/i915/intel_display.c | 23 +++++++++++++----------
>   2 files changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 9e498e0bbf22..9a1de848e450 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4034,10 +4034,15 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
>   
>   	/* As the user may map the buffer once pinned in the display plane
>   	 * (e.g. libkms for the bootup splash), we have to ensure that we
> -	 * always use map_and_fenceable for all scanout buffers.
> +	 * always use map_and_fenceable for all scanout buffers. However,
> +	 * it may simply be too big to fit into mappable, in which case
> +	 * put it anyway and hope that userspace can cope (but always first
> +	 * try to preserve the existing ABI).
>   	 */
>   	ret = i915_gem_obj_ggtt_pin(obj, alignment, PIN_MAPPABLE);
>   	if (ret)
> +		ret = i915_gem_obj_ggtt_pin(obj, alignment, 0);
> +	if (ret)
>   		goto err_unpin_display;
>   
>   	i915_gem_object_flush_cpu_write_domain(obj);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d621ebecd33e..628aace63b43 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2308,16 +2308,18 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
>   	if (ret)
>   		goto err_interruptible;
>   
> -	/* Install a fence for tiled scan-out. Pre-i965 always needs a
> -	 * fence, whereas 965+ only requires a fence if using
> -	 * framebuffer compression.  For simplicity, we always install
> -	 * a fence as the cost is not that onerous.
> -	 */
> -	ret = i915_gem_object_get_fence(obj);
> -	if (ret)
> -		goto err_unpin;
> +	if (obj->map_and_fenceable) {
> +		/* Install a fence for tiled scan-out. Pre-i965 always needs a
> +		 * fence, whereas 965+ only requires a fence if using
> +		 * framebuffer compression.  For simplicity, we always, when
> +		 * possible, install a fence as the cost is not that onerous.
> +		 */
> +		ret = i915_gem_object_get_fence(obj);
> +		if (ret)
> +			goto err_unpin;
>   
> -	i915_gem_object_pin_fence(obj);
> +		i915_gem_object_pin_fence(obj);
> +	}
>   
>   	dev_priv->mm.interruptible = true;
>   	intel_runtime_pm_put(dev_priv);
> @@ -2335,7 +2337,8 @@ static void intel_unpin_fb_obj(struct drm_i915_gem_object *obj)
>   {
>   	WARN_ON(!mutex_is_locked(&obj->base.dev->struct_mutex));
>   
> -	i915_gem_object_unpin_fence(obj);
> +	if (obj->map_and_fenceable)
> +		i915_gem_object_unpin_fence(obj);
>   	i915_gem_object_unpin_from_display_plane(obj);
>   }
>   

should we skip put_fence in overlay_do_put_image ?


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

Re: [PATCH v2] drm/i915: Fallback to using unmappable memory for scanout

[Chris Wilson]

On Thu, Mar 19, 2015 at 06:31:04PM +0530, Deepak S wrote:
> should we skip put_fence in overlay_do_put_image ?

Ah interesting point you raise there. That is buggy code fullstop.
We should not be call put_fence if pin_to_display_plane pins the fence.
Techinically the overlay could use a fence, the restriction is more to
do with an artificial limitation on the overlay API, and so the actual
call to i915_gem_object_put_fence() can be removed without any ill
side-effects. Something for the next time someone considers gen2-4 code.
-Chris

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

Re: [PATCH v2] drm/i915: Fallback to using unmappable memory for scanout

[Deepak S]

On Thursday 19 March 2015 06:40 PM, Chris Wilson wrote:
> On Thu, Mar 19, 2015 at 06:31:04PM +0530, Deepak S wrote:
>> should we skip put_fence in overlay_do_put_image ?
> Ah interesting point you raise there. That is buggy code fullstop.
> We should not be call put_fence if pin_to_display_plane pins the fence.
> Techinically the overlay could use a fence, the restriction is more to
> do with an artificial limitation on the overlay API, and so the actual
> call to i915_gem_object_put_fence() can be removed without any ill
> side-effects. Something for the next time someone considers gen2-4 code.
> -Chris
>
:) Ok got it
Reviewed-by: Deepak S<deepak.s@linux.intel.com>

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

Re: [PATCH] drm/i915: Fallback to using unmappable memory for scanout

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6000
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -1              272/272              271/272
ILK                                  301/301              301/301
SNB                                  303/303              303/303
IVB                 -1              342/342              341/342
BYT                                  287/287              287/287
HSW                                  362/362              362/362
BDW                                  308/308              308/308
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*PNV  igt_gem_userptr_blits_minor-sync-interruptible      PASS(2)      DMESG_WARN(1)PASS(1)
*IVB  igt_gem_storedw_batches_loop_secure-dispatch      PASS(2)      DMESG_WARN(1)PASS(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915: Fallback to using unmappable memory for scanout

[Daniel Vetter]

On Thu, Mar 19, 2015 at 01:10:13PM +0000, Chris Wilson wrote:
> On Thu, Mar 19, 2015 at 06:31:04PM +0530, Deepak S wrote:
> > should we skip put_fence in overlay_do_put_image ?
> 
> Ah interesting point you raise there. That is buggy code fullstop.
> We should not be call put_fence if pin_to_display_plane pins the fence.
> Techinically the overlay could use a fence, the restriction is more to
> do with an artificial limitation on the overlay API, and so the actual
> call to i915_gem_object_put_fence() can be removed without any ill
> side-effects. Something for the next time someone considers gen2-4 code.

The put_fence in there is iirc to synchronize with the lazy tiling, just
in case. Or at least that's the story I remember.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915: Fallback to using unmappable memory for scanout

[Daniel Vetter]

On Thu, Mar 19, 2015 at 11:29:40AM +0000, Chris Wilson wrote:
> The existing ABI says that scanouts are pinned into the mappable region
> so that legacy clients (e.g. old Xorg or plymouthd) can write directly
> into the scanout through a GTT mapping. However if the surface does not
> fit into the mappable region, we are better off just trying to fit it
> anywhere and hoping for the best. (Any userspace that is cappable of
> using ginormous scanouts is also likely not to rely on pure GTT
> updates.) In the future, there may even be a kernel mediated method for
> the legacy clients.
> 
> v2: Skip fence pinning when not mappable.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Satyanantha, Rama Gopal M <rama.gopal.m.satyanantha@intel.com>
> Cc: Deepak S <deepak.s@linux.intel.com>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_gem.c      |  7 ++++++-
>  drivers/gpu/drm/i915/intel_display.c | 23 +++++++++++++----------
>  2 files changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 9e498e0bbf22..9a1de848e450 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4034,10 +4034,15 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
>  
>  	/* As the user may map the buffer once pinned in the display plane
>  	 * (e.g. libkms for the bootup splash), we have to ensure that we
> -	 * always use map_and_fenceable for all scanout buffers.
> +	 * always use map_and_fenceable for all scanout buffers. However,
> +	 * it may simply be too big to fit into mappable, in which case
> +	 * put it anyway and hope that userspace can cope (but always first
> +	 * try to preserve the existing ABI).
>  	 */
>  	ret = i915_gem_obj_ggtt_pin(obj, alignment, PIN_MAPPABLE);
>  	if (ret)
> +		ret = i915_gem_obj_ggtt_pin(obj, alignment, 0);
> +	if (ret)
>  		goto err_unpin_display;
>  
>  	i915_gem_object_flush_cpu_write_domain(obj);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d621ebecd33e..628aace63b43 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2308,16 +2308,18 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
>  	if (ret)
>  		goto err_interruptible;
>  
> -	/* Install a fence for tiled scan-out. Pre-i965 always needs a
> -	 * fence, whereas 965+ only requires a fence if using
> -	 * framebuffer compression.  For simplicity, we always install
> -	 * a fence as the cost is not that onerous.
> -	 */
> -	ret = i915_gem_object_get_fence(obj);
> -	if (ret)
> -		goto err_unpin;
> +	if (obj->map_and_fenceable) {
> +		/* Install a fence for tiled scan-out. Pre-i965 always needs a
> +		 * fence, whereas 965+ only requires a fence if using
> +		 * framebuffer compression.  For simplicity, we always, when
> +		 * possible, install a fence as the cost is not that onerous.
> +		 */
> +		ret = i915_gem_object_get_fence(obj);
> +		if (ret)
> +			goto err_unpin;

FBC still assumes that a fence is there (and with Paulo's recent rework
that's made even more explicit). I think we need a change in the fbc
frontbuffer tracking integration to not filter out GTT invalidates if the
buffer isn't mappable. Paulo?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915: Fallback to using unmappable memory for scanout

[Damien Lespiau]

On Thu, Mar 19, 2015 at 11:29:40AM +0000, Chris Wilson wrote:
> The existing ABI says that scanouts are pinned into the mappable region
> so that legacy clients (e.g. old Xorg or plymouthd) can write directly
> into the scanout through a GTT mapping. However if the surface does not
> fit into the mappable region, we are better off just trying to fit it
> anywhere and hoping for the best. (Any userspace that is cappable of
> using ginormous scanouts is also likely not to rely on pure GTT
> updates.) In the future, there may even be a kernel mediated method for
> the legacy clients.
> 
> v2: Skip fence pinning when not mappable.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Satyanantha, Rama Gopal M <rama.gopal.m.satyanantha@intel.com>
> Cc: Deepak S <deepak.s@linux.intel.com>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---

Hum, isn't that still overlooking that we can't put the framebuffers at
the end of the GGTT?

-- 
Damien

>  drivers/gpu/drm/i915/i915_gem.c      |  7 ++++++-
>  drivers/gpu/drm/i915/intel_display.c | 23 +++++++++++++----------
>  2 files changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 9e498e0bbf22..9a1de848e450 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4034,10 +4034,15 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
>  
>  	/* As the user may map the buffer once pinned in the display plane
>  	 * (e.g. libkms for the bootup splash), we have to ensure that we
> -	 * always use map_and_fenceable for all scanout buffers.
> +	 * always use map_and_fenceable for all scanout buffers. However,
> +	 * it may simply be too big to fit into mappable, in which case
> +	 * put it anyway and hope that userspace can cope (but always first
> +	 * try to preserve the existing ABI).
>  	 */
>  	ret = i915_gem_obj_ggtt_pin(obj, alignment, PIN_MAPPABLE);
>  	if (ret)
> +		ret = i915_gem_obj_ggtt_pin(obj, alignment, 0);
> +	if (ret)
>  		goto err_unpin_display;
>  
>  	i915_gem_object_flush_cpu_write_domain(obj);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d621ebecd33e..628aace63b43 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2308,16 +2308,18 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
>  	if (ret)
>  		goto err_interruptible;
>  
> -	/* Install a fence for tiled scan-out. Pre-i965 always needs a
> -	 * fence, whereas 965+ only requires a fence if using
> -	 * framebuffer compression.  For simplicity, we always install
> -	 * a fence as the cost is not that onerous.
> -	 */
> -	ret = i915_gem_object_get_fence(obj);
> -	if (ret)
> -		goto err_unpin;
> +	if (obj->map_and_fenceable) {
> +		/* Install a fence for tiled scan-out. Pre-i965 always needs a
> +		 * fence, whereas 965+ only requires a fence if using
> +		 * framebuffer compression.  For simplicity, we always, when
> +		 * possible, install a fence as the cost is not that onerous.
> +		 */
> +		ret = i915_gem_object_get_fence(obj);
> +		if (ret)
> +			goto err_unpin;
>  
> -	i915_gem_object_pin_fence(obj);
> +		i915_gem_object_pin_fence(obj);
> +	}
>  
>  	dev_priv->mm.interruptible = true;
>  	intel_runtime_pm_put(dev_priv);
> @@ -2335,7 +2337,8 @@ static void intel_unpin_fb_obj(struct drm_i915_gem_object *obj)
>  {
>  	WARN_ON(!mutex_is_locked(&obj->base.dev->struct_mutex));
>  
> -	i915_gem_object_unpin_fence(obj);
> +	if (obj->map_and_fenceable)
> +		i915_gem_object_unpin_fence(obj);
>  	i915_gem_object_unpin_from_display_plane(obj);
>  }
>  
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915: Fallback to using unmappable memory for scanout

[Chris Wilson]

On Thu, Mar 19, 2015 at 05:35:17PM +0100, Daniel Vetter wrote:
> On Thu, Mar 19, 2015 at 11:29:40AM +0000, Chris Wilson wrote:
> > The existing ABI says that scanouts are pinned into the mappable region
> > so that legacy clients (e.g. old Xorg or plymouthd) can write directly
> > into the scanout through a GTT mapping. However if the surface does not
> > fit into the mappable region, we are better off just trying to fit it
> > anywhere and hoping for the best. (Any userspace that is cappable of
> > using ginormous scanouts is also likely not to rely on pure GTT
> > updates.) In the future, there may even be a kernel mediated method for
> > the legacy clients.
> > 
> > v2: Skip fence pinning when not mappable.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Satyanantha, Rama Gopal M <rama.gopal.m.satyanantha@intel.com>
> > Cc: Deepak S <deepak.s@linux.intel.com>
> > Cc: Damien Lespiau <damien.lespiau@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/i915/i915_gem.c      |  7 ++++++-
> >  drivers/gpu/drm/i915/intel_display.c | 23 +++++++++++++----------
> >  2 files changed, 19 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 9e498e0bbf22..9a1de848e450 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -4034,10 +4034,15 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
> >  
> >  	/* As the user may map the buffer once pinned in the display plane
> >  	 * (e.g. libkms for the bootup splash), we have to ensure that we
> > -	 * always use map_and_fenceable for all scanout buffers.
> > +	 * always use map_and_fenceable for all scanout buffers. However,
> > +	 * it may simply be too big to fit into mappable, in which case
> > +	 * put it anyway and hope that userspace can cope (but always first
> > +	 * try to preserve the existing ABI).
> >  	 */
> >  	ret = i915_gem_obj_ggtt_pin(obj, alignment, PIN_MAPPABLE);
> >  	if (ret)
> > +		ret = i915_gem_obj_ggtt_pin(obj, alignment, 0);
> > +	if (ret)
> >  		goto err_unpin_display;
> >  
> >  	i915_gem_object_flush_cpu_write_domain(obj);
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index d621ebecd33e..628aace63b43 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2308,16 +2308,18 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
> >  	if (ret)
> >  		goto err_interruptible;
> >  
> > -	/* Install a fence for tiled scan-out. Pre-i965 always needs a
> > -	 * fence, whereas 965+ only requires a fence if using
> > -	 * framebuffer compression.  For simplicity, we always install
> > -	 * a fence as the cost is not that onerous.
> > -	 */
> > -	ret = i915_gem_object_get_fence(obj);
> > -	if (ret)
> > -		goto err_unpin;
> > +	if (obj->map_and_fenceable) {
> > +		/* Install a fence for tiled scan-out. Pre-i965 always needs a
> > +		 * fence, whereas 965+ only requires a fence if using
> > +		 * framebuffer compression.  For simplicity, we always, when
> > +		 * possible, install a fence as the cost is not that onerous.
> > +		 */
> > +		ret = i915_gem_object_get_fence(obj);
> > +		if (ret)
> > +			goto err_unpin;
> 
> FBC still assumes that a fence is there (and with Paulo's recent rework
> that's made even more explicit). I think we need a change in the fbc
> frontbuffer tracking integration to not filter out GTT invalidates if the
> buffer isn't mappable. Paulo?

I checked that we have such a check in the fbc enable code. I think if
we have a framebuffer that won't fit in the GTT, we are reasonably sure
it won't be FBC compatible. On the other hand, if we have 4
framebuffers...

if (obj->tiling_mode != I915_TILING_X ||
    obj->fence_reg == I915_FENCE_REG_NONE) {
	if (set_no_fbc_reason(dev_priv, FBC_NOT_TILED))
		DRM_DEBUG_KMS("framebuffer not tiled or fenced, disabling compression\n");

I think it is preferrable that the system continues to run in a degraded
mode in such circumstances than fail entirely.
-Chris

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

Re: [PATCH v2] drm/i915: Fallback to using unmappable memory for scanout

[Chris Wilson]

On Thu, Mar 19, 2015 at 05:34:09PM +0100, Daniel Vetter wrote:
> On Thu, Mar 19, 2015 at 01:10:13PM +0000, Chris Wilson wrote:
> > On Thu, Mar 19, 2015 at 06:31:04PM +0530, Deepak S wrote:
> > > should we skip put_fence in overlay_do_put_image ?
> > 
> > Ah interesting point you raise there. That is buggy code fullstop.
> > We should not be call put_fence if pin_to_display_plane pins the fence.
> > Techinically the overlay could use a fence, the restriction is more to
> > do with an artificial limitation on the overlay API, and so the actual
> > call to i915_gem_object_put_fence() can be removed without any ill
> > side-effects. Something for the next time someone considers gen2-4 code.
> 
> The put_fence in there is iirc to synchronize with the lazy tiling, just
> in case. Or at least that's the story I remember.

Right, that's before we then did the pin_fence. We should have caught
the conflict then. Fortunately, it should just come out in the wash now.
-Chris

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

Re: [PATCH v2] drm/i915: Fallback to using unmappable memory for scanout

[Chris Wilson]

On Thu, Mar 19, 2015 at 04:39:06PM +0000, Damien Lespiau wrote:
> On Thu, Mar 19, 2015 at 11:29:40AM +0000, Chris Wilson wrote:
> > The existing ABI says that scanouts are pinned into the mappable region
> > so that legacy clients (e.g. old Xorg or plymouthd) can write directly
> > into the scanout through a GTT mapping. However if the surface does not
> > fit into the mappable region, we are better off just trying to fit it
> > anywhere and hoping for the best. (Any userspace that is cappable of
> > using ginormous scanouts is also likely not to rely on pure GTT
> > updates.) In the future, there may even be a kernel mediated method for
> > the legacy clients.
> > 
> > v2: Skip fence pinning when not mappable.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Satyanantha, Rama Gopal M <rama.gopal.m.satyanantha@intel.com>
> > Cc: Deepak S <deepak.s@linux.intel.com>
> > Cc: Damien Lespiau <damien.lespiau@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> 
> Hum, isn't that still overlooking that we can't put the framebuffers at
> the end of the GGTT?

Yes. We don't have the interface yet. When we do we can apply the vtd
requirements as well..
-Chris

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

Re: [PATCH v2] drm/i915: Fallback to using unmappable memory for scanout

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6006
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -1              272/272              271/272
ILK                                  301/301              301/301
SNB                                  303/303              303/303
IVB                 -1              342/342              341/342
BYT                                  287/287              287/287
HSW                                  362/362              362/362
BDW                                  308/308              308/308
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*PNV  igt_gem_userptr_blits_minor-unsync-interruptible      PASS(3)      DMESG_WARN(1)PASS(1)
*IVB  igt_gem_storedw_batches_loop_normal      PASS(2)      DMESG_WARN(1)PASS(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915: Fallback to using unmappable memory for scanout

[Daniel Vetter]

On Thu, Mar 19, 2015 at 04:50:22PM +0000, Chris Wilson wrote:
> On Thu, Mar 19, 2015 at 05:35:17PM +0100, Daniel Vetter wrote:
> > On Thu, Mar 19, 2015 at 11:29:40AM +0000, Chris Wilson wrote:
> > > The existing ABI says that scanouts are pinned into the mappable region
> > > so that legacy clients (e.g. old Xorg or plymouthd) can write directly
> > > into the scanout through a GTT mapping. However if the surface does not
> > > fit into the mappable region, we are better off just trying to fit it
> > > anywhere and hoping for the best. (Any userspace that is cappable of
> > > using ginormous scanouts is also likely not to rely on pure GTT
> > > updates.) In the future, there may even be a kernel mediated method for
> > > the legacy clients.
> > > 
> > > v2: Skip fence pinning when not mappable.
> > > 
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Satyanantha, Rama Gopal M <rama.gopal.m.satyanantha@intel.com>
> > > Cc: Deepak S <deepak.s@linux.intel.com>
> > > Cc: Damien Lespiau <damien.lespiau@intel.com>
> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > ---
> > >  drivers/gpu/drm/i915/i915_gem.c      |  7 ++++++-
> > >  drivers/gpu/drm/i915/intel_display.c | 23 +++++++++++++----------
> > >  2 files changed, 19 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > > index 9e498e0bbf22..9a1de848e450 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > @@ -4034,10 +4034,15 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
> > >  
> > >  	/* As the user may map the buffer once pinned in the display plane
> > >  	 * (e.g. libkms for the bootup splash), we have to ensure that we
> > > -	 * always use map_and_fenceable for all scanout buffers.
> > > +	 * always use map_and_fenceable for all scanout buffers. However,
> > > +	 * it may simply be too big to fit into mappable, in which case
> > > +	 * put it anyway and hope that userspace can cope (but always first
> > > +	 * try to preserve the existing ABI).
> > >  	 */
> > >  	ret = i915_gem_obj_ggtt_pin(obj, alignment, PIN_MAPPABLE);
> > >  	if (ret)
> > > +		ret = i915_gem_obj_ggtt_pin(obj, alignment, 0);
> > > +	if (ret)
> > >  		goto err_unpin_display;
> > >  
> > >  	i915_gem_object_flush_cpu_write_domain(obj);
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index d621ebecd33e..628aace63b43 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -2308,16 +2308,18 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
> > >  	if (ret)
> > >  		goto err_interruptible;
> > >  
> > > -	/* Install a fence for tiled scan-out. Pre-i965 always needs a
> > > -	 * fence, whereas 965+ only requires a fence if using
> > > -	 * framebuffer compression.  For simplicity, we always install
> > > -	 * a fence as the cost is not that onerous.
> > > -	 */
> > > -	ret = i915_gem_object_get_fence(obj);
> > > -	if (ret)
> > > -		goto err_unpin;
> > > +	if (obj->map_and_fenceable) {
> > > +		/* Install a fence for tiled scan-out. Pre-i965 always needs a
> > > +		 * fence, whereas 965+ only requires a fence if using
> > > +		 * framebuffer compression.  For simplicity, we always, when
> > > +		 * possible, install a fence as the cost is not that onerous.
> > > +		 */
> > > +		ret = i915_gem_object_get_fence(obj);
> > > +		if (ret)
> > > +			goto err_unpin;
> > 
> > FBC still assumes that a fence is there (and with Paulo's recent rework
> > that's made even more explicit). I think we need a change in the fbc
> > frontbuffer tracking integration to not filter out GTT invalidates if the
> > buffer isn't mappable. Paulo?
> 
> I checked that we have such a check in the fbc enable code. I think if
> we have a framebuffer that won't fit in the GTT, we are reasonably sure
> it won't be FBC compatible. On the other hand, if we have 4
> framebuffers...
> 
> if (obj->tiling_mode != I915_TILING_X ||
>     obj->fence_reg == I915_FENCE_REG_NONE) {
> 	if (set_no_fbc_reason(dev_priv, FBC_NOT_TILED))
> 		DRM_DEBUG_KMS("framebuffer not tiled or fenced, disabling compression\n");
> 
> I think it is preferrable that the system continues to run in a degraded
> mode in such circumstances than fail entirely.

Oh right I've forgotten that fbc hw only works with X tiled and that we
use the fence_reg as a proxy. Adding a comment would be useful though.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915: Fallback to using unmappable memory for scanout

[Chris Wilson]

On Fri, Mar 20, 2015 at 11:29:25AM +0100, Daniel Vetter wrote:
> On Thu, Mar 19, 2015 at 04:50:22PM +0000, Chris Wilson wrote:
> > On Thu, Mar 19, 2015 at 05:35:17PM +0100, Daniel Vetter wrote:
> > > On Thu, Mar 19, 2015 at 11:29:40AM +0000, Chris Wilson wrote:
> > > > +	if (obj->map_and_fenceable) {
> > > > +		/* Install a fence for tiled scan-out. Pre-i965 always needs a
> > > > +		 * fence, whereas 965+ only requires a fence if using
> > > > +		 * framebuffer compression.  For simplicity, we always, when
> > > > +		 * possible, install a fence as the cost is not that onerous.

> Oh right I've forgotten that fbc hw only works with X tiled and that we
> use the fence_reg as a proxy. Adding a comment would be useful though.

* If we fail to fence the tiled scanout, then either the modeset will
* reject the change (which is highly unlikely as the affected systems,
* all but one, do not have unmappable space) or we will not be able to
* enable full powersaving techniques (also likely not to apply due to
* various limits FBC and the like impose on the size of the buffer,
* which presumably we violated anyway with this unmappable buffer).
* Anyway, it is presumably better to stumble onwards with something and
* try to run the system in a "less than optimal" mode that matches the
* user configuration.

> > > > +		 */
> > > > +		ret = i915_gem_object_get_fence(obj);
> > > > +		if (ret)
> > > > +			goto err_unpin;

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

Re: [PATCH v2] drm/i915: Fallback to using unmappable memory for scanout

[Daniel Vetter]

On Fri, Mar 20, 2015 at 10:49:19AM +0000, Chris Wilson wrote:
> On Fri, Mar 20, 2015 at 11:29:25AM +0100, Daniel Vetter wrote:
> > On Thu, Mar 19, 2015 at 04:50:22PM +0000, Chris Wilson wrote:
> > > On Thu, Mar 19, 2015 at 05:35:17PM +0100, Daniel Vetter wrote:
> > > > On Thu, Mar 19, 2015 at 11:29:40AM +0000, Chris Wilson wrote:
> > > > > +	if (obj->map_and_fenceable) {
> > > > > +		/* Install a fence for tiled scan-out. Pre-i965 always needs a
> > > > > +		 * fence, whereas 965+ only requires a fence if using
> > > > > +		 * framebuffer compression.  For simplicity, we always, when
> > > > > +		 * possible, install a fence as the cost is not that onerous.
> 
> > Oh right I've forgotten that fbc hw only works with X tiled and that we
> > use the fence_reg as a proxy. Adding a comment would be useful though.
> 
> * If we fail to fence the tiled scanout, then either the modeset will
> * reject the change (which is highly unlikely as the affected systems,
> * all but one, do not have unmappable space) or we will not be able to
> * enable full powersaving techniques (also likely not to apply due to
> * various limits FBC and the like impose on the size of the buffer,
> * which presumably we violated anyway with this unmappable buffer).
> * Anyway, it is presumably better to stumble onwards with something and
> * try to run the system in a "less than optimal" mode that matches the
> * user configuration.

I actually thought of a comment in the obj->fence_reg check in the fbc
code that we now can have frontbuffers lacking fences. And that the check
isn't just a proxy check for x-tiled anymore.

Just to avoid someone replacing the obj->fence_reg check with a
obj->tiling_mode == X_TILING check somewhen in the future.

Not fencing here is imo clear, since iirc on gen3 fences in the unmappable
part are not allowed.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915: Fallback to using unmappable memory for scanout

[Ville Syrjälä]

On Fri, Mar 20, 2015 at 11:29:25AM +0100, Daniel Vetter wrote:
> On Thu, Mar 19, 2015 at 04:50:22PM +0000, Chris Wilson wrote:
> > On Thu, Mar 19, 2015 at 05:35:17PM +0100, Daniel Vetter wrote:
> > > On Thu, Mar 19, 2015 at 11:29:40AM +0000, Chris Wilson wrote:
> > > > The existing ABI says that scanouts are pinned into the mappable region
> > > > so that legacy clients (e.g. old Xorg or plymouthd) can write directly
> > > > into the scanout through a GTT mapping. However if the surface does not
> > > > fit into the mappable region, we are better off just trying to fit it
> > > > anywhere and hoping for the best. (Any userspace that is cappable of
> > > > using ginormous scanouts is also likely not to rely on pure GTT
> > > > updates.) In the future, there may even be a kernel mediated method for
> > > > the legacy clients.
> > > > 
> > > > v2: Skip fence pinning when not mappable.
> > > > 
> > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Cc: Satyanantha, Rama Gopal M <rama.gopal.m.satyanantha@intel.com>
> > > > Cc: Deepak S <deepak.s@linux.intel.com>
> > > > Cc: Damien Lespiau <damien.lespiau@intel.com>
> > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_gem.c      |  7 ++++++-
> > > >  drivers/gpu/drm/i915/intel_display.c | 23 +++++++++++++----------
> > > >  2 files changed, 19 insertions(+), 11 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > > > index 9e498e0bbf22..9a1de848e450 100644
> > > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > > @@ -4034,10 +4034,15 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
> > > >  
> > > >  	/* As the user may map the buffer once pinned in the display plane
> > > >  	 * (e.g. libkms for the bootup splash), we have to ensure that we
> > > > -	 * always use map_and_fenceable for all scanout buffers.
> > > > +	 * always use map_and_fenceable for all scanout buffers. However,
> > > > +	 * it may simply be too big to fit into mappable, in which case
> > > > +	 * put it anyway and hope that userspace can cope (but always first
> > > > +	 * try to preserve the existing ABI).
> > > >  	 */
> > > >  	ret = i915_gem_obj_ggtt_pin(obj, alignment, PIN_MAPPABLE);
> > > >  	if (ret)
> > > > +		ret = i915_gem_obj_ggtt_pin(obj, alignment, 0);
> > > > +	if (ret)
> > > >  		goto err_unpin_display;
> > > >  
> > > >  	i915_gem_object_flush_cpu_write_domain(obj);
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > > index d621ebecd33e..628aace63b43 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -2308,16 +2308,18 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
> > > >  	if (ret)
> > > >  		goto err_interruptible;
> > > >  
> > > > -	/* Install a fence for tiled scan-out. Pre-i965 always needs a
> > > > -	 * fence, whereas 965+ only requires a fence if using
> > > > -	 * framebuffer compression.  For simplicity, we always install
> > > > -	 * a fence as the cost is not that onerous.
> > > > -	 */
> > > > -	ret = i915_gem_object_get_fence(obj);
> > > > -	if (ret)
> > > > -		goto err_unpin;
> > > > +	if (obj->map_and_fenceable) {
> > > > +		/* Install a fence for tiled scan-out. Pre-i965 always needs a
> > > > +		 * fence, whereas 965+ only requires a fence if using
> > > > +		 * framebuffer compression.  For simplicity, we always, when
> > > > +		 * possible, install a fence as the cost is not that onerous.
> > > > +		 */
> > > > +		ret = i915_gem_object_get_fence(obj);
> > > > +		if (ret)
> > > > +			goto err_unpin;
> > > 
> > > FBC still assumes that a fence is there (and with Paulo's recent rework
> > > that's made even more explicit). I think we need a change in the fbc
> > > frontbuffer tracking integration to not filter out GTT invalidates if the
> > > buffer isn't mappable. Paulo?
> > 
> > I checked that we have such a check in the fbc enable code. I think if
> > we have a framebuffer that won't fit in the GTT, we are reasonably sure
> > it won't be FBC compatible. On the other hand, if we have 4
> > framebuffers...
> > 
> > if (obj->tiling_mode != I915_TILING_X ||
> >     obj->fence_reg == I915_FENCE_REG_NONE) {
> > 	if (set_no_fbc_reason(dev_priv, FBC_NOT_TILED))
> > 		DRM_DEBUG_KMS("framebuffer not tiled or fenced, disabling compression\n");
> > 
> > I think it is preferrable that the system continues to run in a degraded
> > mode in such circumstances than fail entirely.
> 
> Oh right I've forgotten that fbc hw only works with X tiled and that we
> use the fence_reg as a proxy. Adding a comment would be useful though.

SKL supports FBC with Y tiled as well.

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

Re: [PATCH v2] drm/i915: Fallback to using unmappable memory for scanout

[Jani Nikula]

On Thu, 19 Mar 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> The existing ABI says that scanouts are pinned into the mappable region
> so that legacy clients (e.g. old Xorg or plymouthd) can write directly
> into the scanout through a GTT mapping. However if the surface does not
> fit into the mappable region, we are better off just trying to fit it
> anywhere and hoping for the best. (Any userspace that is cappable of
> using ginormous scanouts is also likely not to rely on pure GTT
> updates.) In the future, there may even be a kernel mediated method for
> the legacy clients.
>
> v2: Skip fence pinning when not mappable.

Chris, this patch conflicts with current nightly in a way that I'm not
comfortable resolving.

BR,
Jani.


>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Satyanantha, Rama Gopal M <rama.gopal.m.satyanantha@intel.com>
> Cc: Deepak S <deepak.s@linux.intel.com>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_gem.c      |  7 ++++++-
>  drivers/gpu/drm/i915/intel_display.c | 23 +++++++++++++----------
>  2 files changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 9e498e0bbf22..9a1de848e450 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4034,10 +4034,15 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
>  
>  	/* As the user may map the buffer once pinned in the display plane
>  	 * (e.g. libkms for the bootup splash), we have to ensure that we
> -	 * always use map_and_fenceable for all scanout buffers.
> +	 * always use map_and_fenceable for all scanout buffers. However,
> +	 * it may simply be too big to fit into mappable, in which case
> +	 * put it anyway and hope that userspace can cope (but always first
> +	 * try to preserve the existing ABI).
>  	 */
>  	ret = i915_gem_obj_ggtt_pin(obj, alignment, PIN_MAPPABLE);
>  	if (ret)
> +		ret = i915_gem_obj_ggtt_pin(obj, alignment, 0);
> +	if (ret)
>  		goto err_unpin_display;
>  
>  	i915_gem_object_flush_cpu_write_domain(obj);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d621ebecd33e..628aace63b43 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2308,16 +2308,18 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
>  	if (ret)
>  		goto err_interruptible;
>  
> -	/* Install a fence for tiled scan-out. Pre-i965 always needs a
> -	 * fence, whereas 965+ only requires a fence if using
> -	 * framebuffer compression.  For simplicity, we always install
> -	 * a fence as the cost is not that onerous.
> -	 */
> -	ret = i915_gem_object_get_fence(obj);
> -	if (ret)
> -		goto err_unpin;
> +	if (obj->map_and_fenceable) {
> +		/* Install a fence for tiled scan-out. Pre-i965 always needs a
> +		 * fence, whereas 965+ only requires a fence if using
> +		 * framebuffer compression.  For simplicity, we always, when
> +		 * possible, install a fence as the cost is not that onerous.
> +		 */
> +		ret = i915_gem_object_get_fence(obj);
> +		if (ret)
> +			goto err_unpin;
>  
> -	i915_gem_object_pin_fence(obj);
> +		i915_gem_object_pin_fence(obj);
> +	}
>  
>  	dev_priv->mm.interruptible = true;
>  	intel_runtime_pm_put(dev_priv);
> @@ -2335,7 +2337,8 @@ static void intel_unpin_fb_obj(struct drm_i915_gem_object *obj)
>  {
>  	WARN_ON(!mutex_is_locked(&obj->base.dev->struct_mutex));
>  
> -	i915_gem_object_unpin_fence(obj);
> +	if (obj->map_and_fenceable)
> +		i915_gem_object_unpin_fence(obj);
>  	i915_gem_object_unpin_from_display_plane(obj);
>  }
>  
> -- 
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH v3] drm/i915: Fallback to using unmappable memory for scanout

[Chris Wilson]

The existing ABI says that scanouts are pinned into the mappable region
so that legacy clients (e.g. old Xorg or plymouthd) can write directly
into the scanout through a GTT mapping. However if the surface does not
fit into the mappable region, we are better off just trying to fit it
anywhere and hoping for the best. (Any userspace that is cappable of
using ginormous scanouts is also likely not to rely on pure GTT
updates.) In the future, there may even be a kernel mediated method for
the legacy clients.

v2: Skip fence pinning when not mappable.
v3: Add a comment to explain the possible rammifactions of not being
    able to use fences for unmappable scanouts.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Satyanantha, Rama Gopal M <rama.gopal.m.satyanantha@intel.com>
Cc: Deepak S <deepak.s@linux.intel.com>
Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c      |  7 ++++++-
 drivers/gpu/drm/i915/intel_display.c | 34 ++++++++++++++++++++++++----------
 drivers/gpu/drm/i915/intel_fbc.c     |  4 ++++
 3 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index cdba1597c905..3383aa0aab5e 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4042,12 +4042,17 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
 
 	/* As the user may map the buffer once pinned in the display plane
 	 * (e.g. libkms for the bootup splash), we have to ensure that we
-	 * always use map_and_fenceable for all scanout buffers.
+	 * always use map_and_fenceable for all scanout buffers. However,
+	 * it may simply be too big to fit into mappable, in which case
+	 * put it anyway and hope that userspace can cope (but always first
+	 * try to preserve the existing ABI).
 	 */
 	ret = i915_gem_object_ggtt_pin(obj, view, 0, alignment,
 				       view->type == I915_GGTT_VIEW_NORMAL ?
 				       PIN_MAPPABLE : 0);
 	if (ret)
+		ret = i915_gem_obj_ggtt_pin(obj, alignment, 0);
+	if (ret)
 		goto err_unpin_display;
 
 	i915_gem_object_flush_cpu_write_domain(obj);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dcb2b6ed3a65..2a566aef58a6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2394,16 +2394,29 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
 	if (ret)
 		goto err_interruptible;
 
-	/* Install a fence for tiled scan-out. Pre-i965 always needs a
-	 * fence, whereas 965+ only requires a fence if using
-	 * framebuffer compression.  For simplicity, we always install
-	 * a fence as the cost is not that onerous.
-	 */
-	ret = i915_gem_object_get_fence(obj);
-	if (ret)
-		goto err_unpin;
+	if (obj->map_and_fenceable) {
+		/* Install a fence for tiled scan-out. Pre-i965 always needs a
+		 * fence, whereas 965+ only requires a fence if using
+		 * framebuffer compression.  For simplicity, we always, when
+		 * possible, install a fence as the cost is not that onerous.
+		 *
+		 * If we fail to fence the tiled scanout, then either the
+		 * modeset will reject the change (which is highly unlikely as
+		 * the affected systems, all but one, do not have unmappable
+		 * space) or we will not be able to enable full powersaving
+		 * techniques (also likely not to apply due to various limits
+		 * FBC and the like impose on the size of the buffer, which
+		 * presumably we violated anyway with this unmappable buffer).
+		 * Anyway, it is presumably better to stumble onwards with
+		 * something and try to run the system in a "less than optimal"
+		 * mode that matches the user configuration.
+		 */
+		ret = i915_gem_object_get_fence(obj);
+		if (ret)
+			goto err_unpin;
 
-	i915_gem_object_pin_fence(obj);
+		i915_gem_object_pin_fence(obj);
+	}
 
 	dev_priv->mm.interruptible = true;
 	intel_runtime_pm_put(dev_priv);
@@ -2429,7 +2442,8 @@ static void intel_unpin_fb_obj(struct drm_framebuffer *fb,
 	ret = intel_fill_fb_ggtt_view(&view, fb, plane_state);
 	WARN_ONCE(ret, "Couldn't get view from plane state!");
 
-	i915_gem_object_unpin_fence(obj);
+	if (obj->map_and_fenceable)
+		i915_gem_object_unpin_fence(obj);
 	i915_gem_object_unpin_from_display_plane(obj, &view);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 9fcf446e95f5..7ee1baade095 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -578,6 +578,10 @@ void intel_fbc_update(struct drm_device *dev)
 
 	/* The use of a CPU fence is mandatory in order to detect writes
 	 * by the CPU to the scanout and trigger updates to the FBC.
+	 *
+	 * Note that is possible for a tiled surface to be unmappable (and
+	 * so have no fence associated with it) due to aperture constaints
+	 * at the time of pinning.
 	 */
 	if (obj->tiling_mode != I915_TILING_X ||
 	    obj->fence_reg == I915_FENCE_REG_NONE) {
-- 
2.1.4

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

[PATCH v4] drm/i915: Fallback to using unmappable memory for scanout

[Chris Wilson]

The existing ABI says that scanouts are pinned into the mappable region
so that legacy clients (e.g. old Xorg or plymouthd) can write directly
into the scanout through a GTT mapping. However if the surface does not
fit into the mappable region, we are better off just trying to fit it
anywhere and hoping for the best. (Any userspace that is cappable of
using ginormous scanouts is also likely not to rely on pure GTT
updates.) In the future, there may even be a kernel mediated method for
the legacy clients.

v2: Skip fence pinning when not mappable.
v3: Add a comment to explain the possible rammifactions of not being
    able to use fences for unmappable scanouts.
v4: Rebase to skip over some local patches

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Satyanantha, Rama Gopal M <rama.gopal.m.satyanantha@intel.com>
Cc: Deepak S <deepak.s@linux.intel.com>
Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c      |  7 ++++++-
 drivers/gpu/drm/i915/intel_display.c | 34 ++++++++++++++++++++++++----------
 drivers/gpu/drm/i915/intel_fbc.c     |  4 ++++
 3 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3300963ca716..dd3b42dafae2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3910,12 +3910,17 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
 
 	/* As the user may map the buffer once pinned in the display plane
 	 * (e.g. libkms for the bootup splash), we have to ensure that we
-	 * always use map_and_fenceable for all scanout buffers.
+	 * always use map_and_fenceable for all scanout buffers. However,
+	 * it may simply be too big to fit into mappable, in which case
+	 * put it anyway and hope that userspace can cope (but always first
+	 * try to preserve the existing ABI).
 	 */
 	ret = i915_gem_object_ggtt_pin(obj, view, alignment,
 				       view->type == I915_GGTT_VIEW_NORMAL ?
 				       PIN_MAPPABLE : 0);
 	if (ret)
+		ret = i915_gem_obj_ggtt_pin(obj, alignment, 0);
+	if (ret)
 		goto err_unpin_display;
 
 	i915_gem_object_flush_cpu_write_domain(obj);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f9dc5babcf27..cf348e954ff1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2394,16 +2394,29 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
 	if (ret)
 		goto err_interruptible;
 
-	/* Install a fence for tiled scan-out. Pre-i965 always needs a
-	 * fence, whereas 965+ only requires a fence if using
-	 * framebuffer compression.  For simplicity, we always install
-	 * a fence as the cost is not that onerous.
-	 */
-	ret = i915_gem_object_get_fence(obj);
-	if (ret)
-		goto err_unpin;
+	if (obj->map_and_fenceable) {
+		/* Install a fence for tiled scan-out. Pre-i965 always needs a
+		 * fence, whereas 965+ only requires a fence if using
+		 * framebuffer compression.  For simplicity, we always, when
+		 * possible, install a fence as the cost is not that onerous.
+		 *
+		 * If we fail to fence the tiled scanout, then either the
+		 * modeset will reject the change (which is highly unlikely as
+		 * the affected systems, all but one, do not have unmappable
+		 * space) or we will not be able to enable full powersaving
+		 * techniques (also likely not to apply due to various limits
+		 * FBC and the like impose on the size of the buffer, which
+		 * presumably we violated anyway with this unmappable buffer).
+		 * Anyway, it is presumably better to stumble onwards with
+		 * something and try to run the system in a "less than optimal"
+		 * mode that matches the user configuration.
+		 */
+		ret = i915_gem_object_get_fence(obj);
+		if (ret)
+			goto err_unpin;
 
-	i915_gem_object_pin_fence(obj);
+		i915_gem_object_pin_fence(obj);
+	}
 
 	dev_priv->mm.interruptible = true;
 	intel_runtime_pm_put(dev_priv);
@@ -2429,7 +2442,8 @@ static void intel_unpin_fb_obj(struct drm_framebuffer *fb,
 	ret = intel_fill_fb_ggtt_view(&view, fb, plane_state);
 	WARN_ONCE(ret, "Couldn't get view from plane state!");
 
-	i915_gem_object_unpin_fence(obj);
+	if (obj->map_and_fenceable)
+		i915_gem_object_unpin_fence(obj);
 	i915_gem_object_unpin_from_display_plane(obj, &view);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 9fcf446e95f5..7ee1baade095 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -578,6 +578,10 @@ void intel_fbc_update(struct drm_device *dev)
 
 	/* The use of a CPU fence is mandatory in order to detect writes
 	 * by the CPU to the scanout and trigger updates to the FBC.
+	 *
+	 * Note that is possible for a tiled surface to be unmappable (and
+	 * so have no fence associated with it) due to aperture constaints
+	 * at the time of pinning.
 	 */
 	if (obj->tiling_mode != I915_TILING_X ||
 	    obj->fence_reg == I915_FENCE_REG_NONE) {
-- 
2.1.4

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

Re: [PATCH v4] drm/i915: Fallback to using unmappable memory for scanout

[Daniel Vetter]

On Wed, Mar 25, 2015 at 12:44:47PM +0000, Chris Wilson wrote:
> The existing ABI says that scanouts are pinned into the mappable region
> so that legacy clients (e.g. old Xorg or plymouthd) can write directly
> into the scanout through a GTT mapping. However if the surface does not
> fit into the mappable region, we are better off just trying to fit it
> anywhere and hoping for the best. (Any userspace that is cappable of
> using ginormous scanouts is also likely not to rely on pure GTT
> updates.) In the future, there may even be a kernel mediated method for
> the legacy clients.
> 
> v2: Skip fence pinning when not mappable.
> v3: Add a comment to explain the possible rammifactions of not being
>     able to use fences for unmappable scanouts.
> v4: Rebase to skip over some local patches
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Satyanantha, Rama Gopal M <rama.gopal.m.satyanantha@intel.com>
> Cc: Deepak S <deepak.s@linux.intel.com>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>

I'm a bit uncomfortable merging this before Joonas has the partial mmap
ready. I fear that if we end up with a framebuffer stuck in unmappable
that we'll then have a cascade of crashing compositors because they can't
cope. Yes SNA can, but last time I've checked that's the only one.

Earmarked for merging as soon as the partial view stuff is in.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem.c      |  7 ++++++-
>  drivers/gpu/drm/i915/intel_display.c | 34 ++++++++++++++++++++++++----------
>  drivers/gpu/drm/i915/intel_fbc.c     |  4 ++++
>  3 files changed, 34 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 3300963ca716..dd3b42dafae2 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3910,12 +3910,17 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
>  
>  	/* As the user may map the buffer once pinned in the display plane
>  	 * (e.g. libkms for the bootup splash), we have to ensure that we
> -	 * always use map_and_fenceable for all scanout buffers.
> +	 * always use map_and_fenceable for all scanout buffers. However,
> +	 * it may simply be too big to fit into mappable, in which case
> +	 * put it anyway and hope that userspace can cope (but always first
> +	 * try to preserve the existing ABI).
>  	 */
>  	ret = i915_gem_object_ggtt_pin(obj, view, alignment,
>  				       view->type == I915_GGTT_VIEW_NORMAL ?
>  				       PIN_MAPPABLE : 0);
>  	if (ret)
> +		ret = i915_gem_obj_ggtt_pin(obj, alignment, 0);
> +	if (ret)
>  		goto err_unpin_display;
>  
>  	i915_gem_object_flush_cpu_write_domain(obj);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f9dc5babcf27..cf348e954ff1 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2394,16 +2394,29 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
>  	if (ret)
>  		goto err_interruptible;
>  
> -	/* Install a fence for tiled scan-out. Pre-i965 always needs a
> -	 * fence, whereas 965+ only requires a fence if using
> -	 * framebuffer compression.  For simplicity, we always install
> -	 * a fence as the cost is not that onerous.
> -	 */
> -	ret = i915_gem_object_get_fence(obj);
> -	if (ret)
> -		goto err_unpin;
> +	if (obj->map_and_fenceable) {
> +		/* Install a fence for tiled scan-out. Pre-i965 always needs a
> +		 * fence, whereas 965+ only requires a fence if using
> +		 * framebuffer compression.  For simplicity, we always, when
> +		 * possible, install a fence as the cost is not that onerous.
> +		 *
> +		 * If we fail to fence the tiled scanout, then either the
> +		 * modeset will reject the change (which is highly unlikely as
> +		 * the affected systems, all but one, do not have unmappable
> +		 * space) or we will not be able to enable full powersaving
> +		 * techniques (also likely not to apply due to various limits
> +		 * FBC and the like impose on the size of the buffer, which
> +		 * presumably we violated anyway with this unmappable buffer).
> +		 * Anyway, it is presumably better to stumble onwards with
> +		 * something and try to run the system in a "less than optimal"
> +		 * mode that matches the user configuration.
> +		 */
> +		ret = i915_gem_object_get_fence(obj);
> +		if (ret)
> +			goto err_unpin;
>  
> -	i915_gem_object_pin_fence(obj);
> +		i915_gem_object_pin_fence(obj);
> +	}
>  
>  	dev_priv->mm.interruptible = true;
>  	intel_runtime_pm_put(dev_priv);
> @@ -2429,7 +2442,8 @@ static void intel_unpin_fb_obj(struct drm_framebuffer *fb,
>  	ret = intel_fill_fb_ggtt_view(&view, fb, plane_state);
>  	WARN_ONCE(ret, "Couldn't get view from plane state!");
>  
> -	i915_gem_object_unpin_fence(obj);
> +	if (obj->map_and_fenceable)
> +		i915_gem_object_unpin_fence(obj);
>  	i915_gem_object_unpin_from_display_plane(obj, &view);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 9fcf446e95f5..7ee1baade095 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -578,6 +578,10 @@ void intel_fbc_update(struct drm_device *dev)
>  
>  	/* The use of a CPU fence is mandatory in order to detect writes
>  	 * by the CPU to the scanout and trigger updates to the FBC.
> +	 *
> +	 * Note that is possible for a tiled surface to be unmappable (and
> +	 * so have no fence associated with it) due to aperture constaints
> +	 * at the time of pinning.
>  	 */
>  	if (obj->tiling_mode != I915_TILING_X ||
>  	    obj->fence_reg == I915_FENCE_REG_NONE) {
> -- 
> 2.1.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3] drm/i915: Fallback to using unmappable memory for scanout

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6048
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -2              269/269              267/269
ILK                 -2              303/303              301/303
SNB                                  304/304              304/304
IVB                                  338/338              338/338
BYT                                  287/287              287/287
HSW                                  362/362              362/362
BDW                                  310/310              310/310
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 PNV  igt@gem_userptr_blits@coherency-sync      CRASH(2)PASS(3)      CRASH(2)
*PNV  igt@gem_userptr_blits@minor-normal-sync      PASS(3)      DMESG_WARN(1)PASS(1)
WARNING:at_drivers/gpu/drm/i915/i915_gem_evict.c:#i915_gem_evict_vm[i915]()@WARNING:.* at .* i915_gem_evict_vm+0x
*ILK  igt@kms_pipe_crc_basic@bad-pipe      PASS(2)      DMESG_WARN(1)PASS(1)
*ILK  igt@gem_seqno_wrap      PASS(4)      DMESG_WARN(1)PASS(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4] drm/i915: Fallback to using unmappable memory for scanout

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6049
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -6              269/269              263/269
ILK                                  303/303              303/303
SNB                                  304/304              304/304
IVB                                  338/338              338/338
BYT                                  287/287              287/287
HSW                                  362/362              362/362
BDW                                  310/310              310/310
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 PNV  igt@gem_userptr_blits@coherency-sync      CRASH(3)PASS(3)      CRASH(2)
*PNV  igt@gem_fence_thrash@bo-write-verify-none      PASS(2)      FAIL(1)PASS(1)
 PNV  igt@gem_userptr_blits@minor-unsync-interruptible      DMESG_WARN(1)PASS(3)      DMESG_WARN(1)PASS(1)
WARNING:at_drivers/gpu/drm/i915/i915_gem_evict.c:#i915_gem_evict_vm[i915]()@WARNING:.* at .* i915_gem_evict_vm+0x
 PNV  igt@gem_userptr_blits@minor-unsync-normal      DMESG_WARN(1)PASS(2)      DMESG_WARN(1)PASS(1)
WARNING:at_drivers/gpu/drm/i915/i915_gem_evict.c:#i915_gem_evict_vm[i915]()@WARNING:.* at .* i915_gem_evict_vm+0x
*PNV  igt@gem_fence_thrash@bo-write-verify-threaded-none      PASS(2)      CRASH(1)PASS(1)
*PNV  igt@gem_unfence_active_buffers      PASS(2)      NO_RESULT(1)PASS(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx