* [PATCH] drm/i915: Stop discarding GTT cache-domain on unbind vma
@ 2015-04-23 11:54 Chris Wilson
2015-05-04 14:14 ` Daniel Vetter
0 siblings, 1 reply; 8+ messages in thread
From: Chris Wilson @ 2015-04-23 11:54 UTC (permalink / raw)
To: intel-gfx; +Cc: Akash Goel
Since
commit 43566dedde54f9729113f5f9fde77d53e75e61e9
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date: Fri Jan 2 16:29:29 2015 +0530
drm/i915: Broaden application of set-domain(GTT)
we allowed objects to be in the GTT domain, but unbound. Therefore
removing the GTT cache domain when removing the GGTT vma is no longer
semantically correct.
An unfortunate side-effect is we lose the wondrously named
i915_gem_object_finish_gtt(), not to be confused with
i915_gem_gtt_finish_object()!
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: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 22 +++-------------------
1 file changed, 3 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6ffc4a619849..1ea94aec41d1 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3171,23 +3171,6 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj,
return 0;
}
-static void i915_gem_object_finish_gtt(struct drm_i915_gem_object *obj)
-{
- /* Force a pagefault for domain tracking on next user access */
- i915_gem_release_mmap(obj);
-
- if ((obj->base.read_domains & I915_GEM_DOMAIN_GTT) == 0)
- return;
-
- /* Wait for any direct GTT access to complete */
- mb();
-
- obj->base.read_domains &= ~I915_GEM_DOMAIN_GTT;
- obj->base.write_domain &= ~I915_GEM_DOMAIN_GTT;
-
- trace_i915_gem_object_change_domain(obj);
-}
-
int i915_vma_unbind(struct i915_vma *vma)
{
struct drm_i915_gem_object *obj = vma->obj;
@@ -3219,12 +3202,13 @@ int i915_vma_unbind(struct i915_vma *vma)
*/
if (vma->is_ggtt && vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) {
- i915_gem_object_finish_gtt(obj);
-
/* release the fence reg _after_ flushing */
ret = i915_gem_object_put_fence(obj);
if (ret)
return ret;
+
+ /* Force a pagefault for domain tracking on next user access */
+ i915_gem_release_mmap(obj);
}
trace_i915_vma_unbind(vma);
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Stop discarding GTT cache-domain on unbind vma
2015-04-23 11:54 [PATCH] drm/i915: Stop discarding GTT cache-domain on unbind vma Chris Wilson
@ 2015-05-04 14:14 ` Daniel Vetter
2015-05-04 14:21 ` Daniel Vetter
0 siblings, 1 reply; 8+ messages in thread
From: Daniel Vetter @ 2015-05-04 14:14 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx, Akash Goel
On Thu, Apr 23, 2015 at 12:54:37PM +0100, Chris Wilson wrote:
> Since
>
> commit 43566dedde54f9729113f5f9fde77d53e75e61e9
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date: Fri Jan 2 16:29:29 2015 +0530
>
> drm/i915: Broaden application of set-domain(GTT)
>
> we allowed objects to be in the GTT domain, but unbound. Therefore
> removing the GTT cache domain when removing the GGTT vma is no longer
> semantically correct.
>
> An unfortunate side-effect is we lose the wondrously named
> i915_gem_object_finish_gtt(), not to be confused with
> i915_gem_gtt_finish_object()!
What are we going to do without that bit of hilarity ...
>
> 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: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Queued for -next (with the rebase conflict resolved because I don't have
your tree), thanks for the patch.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_gem.c | 22 +++-------------------
> 1 file changed, 3 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 6ffc4a619849..1ea94aec41d1 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3171,23 +3171,6 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj,
> return 0;
> }
>
> -static void i915_gem_object_finish_gtt(struct drm_i915_gem_object *obj)
> -{
> - /* Force a pagefault for domain tracking on next user access */
> - i915_gem_release_mmap(obj);
> -
> - if ((obj->base.read_domains & I915_GEM_DOMAIN_GTT) == 0)
> - return;
> -
> - /* Wait for any direct GTT access to complete */
> - mb();
> -
> - obj->base.read_domains &= ~I915_GEM_DOMAIN_GTT;
> - obj->base.write_domain &= ~I915_GEM_DOMAIN_GTT;
> -
> - trace_i915_gem_object_change_domain(obj);
> -}
> -
> int i915_vma_unbind(struct i915_vma *vma)
> {
> struct drm_i915_gem_object *obj = vma->obj;
> @@ -3219,12 +3202,13 @@ int i915_vma_unbind(struct i915_vma *vma)
> */
>
> if (vma->is_ggtt && vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) {
> - i915_gem_object_finish_gtt(obj);
> -
> /* release the fence reg _after_ flushing */
> ret = i915_gem_object_put_fence(obj);
> if (ret)
> return ret;
> +
> + /* Force a pagefault for domain tracking on next user access */
> + i915_gem_release_mmap(obj);
> }
>
> trace_i915_vma_unbind(vma);
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Stop discarding GTT cache-domain on unbind vma
2015-05-04 14:14 ` Daniel Vetter
@ 2015-05-04 14:21 ` Daniel Vetter
2015-05-04 14:26 ` Chris Wilson
0 siblings, 1 reply; 8+ messages in thread
From: Daniel Vetter @ 2015-05-04 14:21 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx, Akash Goel
On Mon, May 04, 2015 at 04:14:41PM +0200, Daniel Vetter wrote:
> On Thu, Apr 23, 2015 at 12:54:37PM +0100, Chris Wilson wrote:
> > Since
> >
> > commit 43566dedde54f9729113f5f9fde77d53e75e61e9
> > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > Date: Fri Jan 2 16:29:29 2015 +0530
> >
> > drm/i915: Broaden application of set-domain(GTT)
> >
> > we allowed objects to be in the GTT domain, but unbound. Therefore
> > removing the GTT cache domain when removing the GGTT vma is no longer
> > semantically correct.
> >
> > An unfortunate side-effect is we lose the wondrously named
> > i915_gem_object_finish_gtt(), not to be confused with
> > i915_gem_gtt_finish_object()!
>
> What are we going to do without that bit of hilarity ...
> >
> > 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: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Queued for -next (with the rebase conflict resolved because I don't have
> your tree), thanks for the patch.
Well not since there's another finish_gtt in my tree in set_cache_level.
And tbh I'm not sure what to do with that one, but it seems rather
redundnant. Care to respin/rebase?
-Daniel
> -Daniel
>
> > ---
> > drivers/gpu/drm/i915/i915_gem.c | 22 +++-------------------
> > 1 file changed, 3 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 6ffc4a619849..1ea94aec41d1 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -3171,23 +3171,6 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj,
> > return 0;
> > }
> >
> > -static void i915_gem_object_finish_gtt(struct drm_i915_gem_object *obj)
> > -{
> > - /* Force a pagefault for domain tracking on next user access */
> > - i915_gem_release_mmap(obj);
> > -
> > - if ((obj->base.read_domains & I915_GEM_DOMAIN_GTT) == 0)
> > - return;
> > -
> > - /* Wait for any direct GTT access to complete */
> > - mb();
> > -
> > - obj->base.read_domains &= ~I915_GEM_DOMAIN_GTT;
> > - obj->base.write_domain &= ~I915_GEM_DOMAIN_GTT;
> > -
> > - trace_i915_gem_object_change_domain(obj);
> > -}
> > -
> > int i915_vma_unbind(struct i915_vma *vma)
> > {
> > struct drm_i915_gem_object *obj = vma->obj;
> > @@ -3219,12 +3202,13 @@ int i915_vma_unbind(struct i915_vma *vma)
> > */
> >
> > if (vma->is_ggtt && vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) {
> > - i915_gem_object_finish_gtt(obj);
> > -
> > /* release the fence reg _after_ flushing */
> > ret = i915_gem_object_put_fence(obj);
> > if (ret)
> > return ret;
> > +
> > + /* Force a pagefault for domain tracking on next user access */
> > + i915_gem_release_mmap(obj);
> > }
> >
> > trace_i915_vma_unbind(vma);
> > --
> > 2.1.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Stop discarding GTT cache-domain on unbind vma
2015-05-04 14:21 ` Daniel Vetter
@ 2015-05-04 14:26 ` Chris Wilson
0 siblings, 0 replies; 8+ messages in thread
From: Chris Wilson @ 2015-05-04 14:26 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, Akash Goel
On Mon, May 04, 2015 at 04:21:20PM +0200, Daniel Vetter wrote:
> On Mon, May 04, 2015 at 04:14:41PM +0200, Daniel Vetter wrote:
> > On Thu, Apr 23, 2015 at 12:54:37PM +0100, Chris Wilson wrote:
> > > Since
> > >
> > > commit 43566dedde54f9729113f5f9fde77d53e75e61e9
> > > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > > Date: Fri Jan 2 16:29:29 2015 +0530
> > >
> > > drm/i915: Broaden application of set-domain(GTT)
> > >
> > > we allowed objects to be in the GTT domain, but unbound. Therefore
> > > removing the GTT cache domain when removing the GGTT vma is no longer
> > > semantically correct.
> > >
> > > An unfortunate side-effect is we lose the wondrously named
> > > i915_gem_object_finish_gtt(), not to be confused with
> > > i915_gem_gtt_finish_object()!
> >
> > What are we going to do without that bit of hilarity ...
> > >
> > > 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: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >
> > Queued for -next (with the rebase conflict resolved because I don't have
> > your tree), thanks for the patch.
>
> Well not since there's another finish_gtt in my tree in set_cache_level.
> And tbh I'm not sure what to do with that one, but it seems rather
> redundnant. Care to respin/rebase?
There's a patch on list to remove it, I think I am about 90% of the way
through convincing Tvrtko that that patch is golden.
Between Akash any myself we have a had a little debate over whether we
want to keep the mb() here or not. The consensus, at least how I have the
patch in my tree, is to use:
if (vma->map_and_fenceable) {
ret = i915_gem_object_put_fence(obj);
if (ret)
return ret;
/* Force a pagefault for domain tracking on next user access */
i915_gem_release_mmap(obj);
/* Wait for any direct GTT access to complete */
if (obj->base.read_domains & I915_GEM_DOMAIN_GTT)
mb();
}
as the replacement. The worry is that we don't have sufficient barriers
between the user doing their GTT writes through one BAR and updating the
PTE through another.
-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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] drm/i915: Stop discarding GTT cache-domain on unbind vma
2015-10-06 11:41 [PATCH 2/2] drm/i915: Drop i915_gem_obj_is_pinned() from set-cache-level Chris Wilson
@ 2015-10-06 12:02 ` Chris Wilson
2015-10-06 12:40 ` Daniel Vetter
0 siblings, 1 reply; 8+ messages in thread
From: Chris Wilson @ 2015-10-06 12:02 UTC (permalink / raw)
To: intel-gfx; +Cc: Akash Goel
Since
commit 43566dedde54f9729113f5f9fde77d53e75e61e9
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date: Fri Jan 2 16:29:29 2015 +0530
drm/i915: Broaden application of set-domain(GTT)
we allowed objects to be in the GTT domain, but unbound. Therefore
removing the GTT cache domain when removing the GGTT vma is no longer
semantically correct.
An unfortunate side-effect is we lose the wondrously named
i915_gem_object_finish_gtt(), not to be confused with
i915_gem_gtt_finish_object()!
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: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 20 +++-----------------
1 file changed, 3 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 8f498d4d874d..682af2ae3681 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3183,20 +3183,6 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj,
return 0;
}
-static void i915_gem_object_finish_gtt(struct drm_i915_gem_object *obj)
-{
- /* Force a pagefault for domain tracking on next user access */
- i915_gem_release_mmap(obj);
-
- if ((obj->base.read_domains & I915_GEM_DOMAIN_GTT) == 0)
- return;
-
- obj->base.read_domains &= ~I915_GEM_DOMAIN_GTT;
- obj->base.write_domain &= ~I915_GEM_DOMAIN_GTT;
-
- trace_i915_gem_object_change_domain(obj);
-}
-
int i915_vma_unbind(struct i915_vma *vma)
{
struct drm_i915_gem_object *obj = vma->obj;
@@ -3228,12 +3214,12 @@ int i915_vma_unbind(struct i915_vma *vma)
*/
if (vma->is_ggtt && vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) {
- i915_gem_object_finish_gtt(obj);
-
- /* release the fence reg _after_ flushing */
ret = i915_gem_object_put_fence(obj);
if (ret)
return ret;
+
+ /* Force a pagefault for domain tracking on next user access */
+ i915_gem_release_mmap(obj);
}
if (!vma->vm->closed) {
--
2.6.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Stop discarding GTT cache-domain on unbind vma
2015-10-06 12:02 ` [PATCH] drm/i915: Stop discarding GTT cache-domain on unbind vma Chris Wilson
@ 2015-10-06 12:40 ` Daniel Vetter
2015-10-06 12:46 ` Chris Wilson
0 siblings, 1 reply; 8+ messages in thread
From: Daniel Vetter @ 2015-10-06 12:40 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx, Akash Goel
On Tue, Oct 06, 2015 at 01:02:22PM +0100, Chris Wilson wrote:
> Since
>
> commit 43566dedde54f9729113f5f9fde77d53e75e61e9
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date: Fri Jan 2 16:29:29 2015 +0530
>
> drm/i915: Broaden application of set-domain(GTT)
>
> we allowed objects to be in the GTT domain, but unbound. Therefore
> removing the GTT cache domain when removing the GGTT vma is no longer
> semantically correct.
>
> An unfortunate side-effect is we lose the wondrously named
> i915_gem_object_finish_gtt(), not to be confused with
> i915_gem_gtt_finish_object()!
>
> 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: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem.c | 20 +++-----------------
> 1 file changed, 3 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 8f498d4d874d..682af2ae3681 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3183,20 +3183,6 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj,
> return 0;
> }
>
> -static void i915_gem_object_finish_gtt(struct drm_i915_gem_object *obj)
> -{
> - /* Force a pagefault for domain tracking on next user access */
> - i915_gem_release_mmap(obj);
> -
> - if ((obj->base.read_domains & I915_GEM_DOMAIN_GTT) == 0)
> - return;
> -
> - obj->base.read_domains &= ~I915_GEM_DOMAIN_GTT;
> - obj->base.write_domain &= ~I915_GEM_DOMAIN_GTT;
> -
> - trace_i915_gem_object_change_domain(obj);
> -}
> -
> int i915_vma_unbind(struct i915_vma *vma)
> {
> struct drm_i915_gem_object *obj = vma->obj;
> @@ -3228,12 +3214,12 @@ int i915_vma_unbind(struct i915_vma *vma)
> */
>
> if (vma->is_ggtt && vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) {
> - i915_gem_object_finish_gtt(obj);
> -
> - /* release the fence reg _after_ flushing */
> ret = i915_gem_object_put_fence(obj);
> if (ret)
> return ret;
> +
> + /* Force a pagefault for domain tracking on next user access */
> + i915_gem_release_mmap(obj);
Can't put_fence before release_mmap ... I guess we should have a testcase
for this somewhere? Hard to provoke probably ...
-Daniel
> }
>
> if (!vma->vm->closed) {
> --
> 2.6.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Stop discarding GTT cache-domain on unbind vma
2015-10-06 12:40 ` Daniel Vetter
@ 2015-10-06 12:46 ` Chris Wilson
2015-10-06 13:05 ` Daniel Vetter
0 siblings, 1 reply; 8+ messages in thread
From: Chris Wilson @ 2015-10-06 12:46 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, Akash Goel
On Tue, Oct 06, 2015 at 02:40:26PM +0200, Daniel Vetter wrote:
> On Tue, Oct 06, 2015 at 01:02:22PM +0100, Chris Wilson wrote:
> > Since
> >
> > commit 43566dedde54f9729113f5f9fde77d53e75e61e9
> > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > Date: Fri Jan 2 16:29:29 2015 +0530
> >
> > drm/i915: Broaden application of set-domain(GTT)
> >
> > we allowed objects to be in the GTT domain, but unbound. Therefore
> > removing the GTT cache domain when removing the GGTT vma is no longer
> > semantically correct.
> >
> > An unfortunate side-effect is we lose the wondrously named
> > i915_gem_object_finish_gtt(), not to be confused with
> > i915_gem_gtt_finish_object()!
> >
> > 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: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_gem.c | 20 +++-----------------
> > 1 file changed, 3 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 8f498d4d874d..682af2ae3681 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -3183,20 +3183,6 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj,
> > return 0;
> > }
> >
> > -static void i915_gem_object_finish_gtt(struct drm_i915_gem_object *obj)
> > -{
> > - /* Force a pagefault for domain tracking on next user access */
> > - i915_gem_release_mmap(obj);
> > -
> > - if ((obj->base.read_domains & I915_GEM_DOMAIN_GTT) == 0)
> > - return;
> > -
> > - obj->base.read_domains &= ~I915_GEM_DOMAIN_GTT;
> > - obj->base.write_domain &= ~I915_GEM_DOMAIN_GTT;
> > -
> > - trace_i915_gem_object_change_domain(obj);
> > -}
> > -
> > int i915_vma_unbind(struct i915_vma *vma)
> > {
> > struct drm_i915_gem_object *obj = vma->obj;
> > @@ -3228,12 +3214,12 @@ int i915_vma_unbind(struct i915_vma *vma)
> > */
> >
> > if (vma->is_ggtt && vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) {
> > - i915_gem_object_finish_gtt(obj);
> > -
> > - /* release the fence reg _after_ flushing */
> > ret = i915_gem_object_put_fence(obj);
> > if (ret)
> > return ret;
> > +
> > + /* Force a pagefault for domain tracking on next user access */
> > + i915_gem_release_mmap(obj);
>
> Can't put_fence before release_mmap ... I guess we should have a testcase
> for this somewhere? Hard to provoke probably ...
Why not? i915_gem_object_put_fence() has to release the mmaps itself if
it has a fence register assigned for the object.
-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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Stop discarding GTT cache-domain on unbind vma
2015-10-06 12:46 ` Chris Wilson
@ 2015-10-06 13:05 ` Daniel Vetter
0 siblings, 0 replies; 8+ messages in thread
From: Daniel Vetter @ 2015-10-06 13:05 UTC (permalink / raw)
To: Chris Wilson, Daniel Vetter, intel-gfx, Akash Goel
On Tue, Oct 06, 2015 at 01:46:25PM +0100, Chris Wilson wrote:
> On Tue, Oct 06, 2015 at 02:40:26PM +0200, Daniel Vetter wrote:
> > On Tue, Oct 06, 2015 at 01:02:22PM +0100, Chris Wilson wrote:
> > > Since
> > >
> > > commit 43566dedde54f9729113f5f9fde77d53e75e61e9
> > > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > > Date: Fri Jan 2 16:29:29 2015 +0530
> > >
> > > drm/i915: Broaden application of set-domain(GTT)
> > >
> > > we allowed objects to be in the GTT domain, but unbound. Therefore
> > > removing the GTT cache domain when removing the GGTT vma is no longer
> > > semantically correct.
> > >
> > > An unfortunate side-effect is we lose the wondrously named
> > > i915_gem_object_finish_gtt(), not to be confused with
> > > i915_gem_gtt_finish_object()!
> > >
> > > 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: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/i915_gem.c | 20 +++-----------------
> > > 1 file changed, 3 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > > index 8f498d4d874d..682af2ae3681 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > @@ -3183,20 +3183,6 @@ i915_gem_object_sync(struct drm_i915_gem_object *obj,
> > > return 0;
> > > }
> > >
> > > -static void i915_gem_object_finish_gtt(struct drm_i915_gem_object *obj)
> > > -{
> > > - /* Force a pagefault for domain tracking on next user access */
> > > - i915_gem_release_mmap(obj);
> > > -
> > > - if ((obj->base.read_domains & I915_GEM_DOMAIN_GTT) == 0)
> > > - return;
> > > -
> > > - obj->base.read_domains &= ~I915_GEM_DOMAIN_GTT;
> > > - obj->base.write_domain &= ~I915_GEM_DOMAIN_GTT;
> > > -
> > > - trace_i915_gem_object_change_domain(obj);
> > > -}
> > > -
> > > int i915_vma_unbind(struct i915_vma *vma)
> > > {
> > > struct drm_i915_gem_object *obj = vma->obj;
> > > @@ -3228,12 +3214,12 @@ int i915_vma_unbind(struct i915_vma *vma)
> > > */
> > >
> > > if (vma->is_ggtt && vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) {
> > > - i915_gem_object_finish_gtt(obj);
> > > -
> > > - /* release the fence reg _after_ flushing */
> > > ret = i915_gem_object_put_fence(obj);
> > > if (ret)
> > > return ret;
> > > +
> > > + /* Force a pagefault for domain tracking on next user access */
> > > + i915_gem_release_mmap(obj);
> >
> > Can't put_fence before release_mmap ... I guess we should have a testcase
> > for this somewhere? Hard to provoke probably ...
>
> Why not? i915_gem_object_put_fence() has to release the mmaps itself if
> it has a fence register assigned for the object.
Oh right, forgotten that put_fence is robust. Looking at this simply
brought up bad memories since I fixed this kind of bug 5 years ago once
;-)
Cheers, 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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-10-06 13:03 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-04-23 11:54 [PATCH] drm/i915: Stop discarding GTT cache-domain on unbind vma Chris Wilson
2015-05-04 14:14 ` Daniel Vetter
2015-05-04 14:21 ` Daniel Vetter
2015-05-04 14:26 ` Chris Wilson
-- strict thread matches above, loose matches on Subject: below --
2015-10-06 11:41 [PATCH 2/2] drm/i915: Drop i915_gem_obj_is_pinned() from set-cache-level Chris Wilson
2015-10-06 12:02 ` [PATCH] drm/i915: Stop discarding GTT cache-domain on unbind vma Chris Wilson
2015-10-06 12:40 ` Daniel Vetter
2015-10-06 12:46 ` Chris Wilson
2015-10-06 13:05 ` Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox