* [PATCH] gem_pin: Be explicit about GGTT needs @ 2013-11-07 4:05 Ben Widawsky 2013-11-07 8:30 ` Chris Wilson 2013-11-07 8:33 ` Daniel Vetter 0 siblings, 2 replies; 9+ messages in thread From: Ben Widawsky @ 2013-11-07 4:05 UTC (permalink / raw) To: Intel GFX; +Cc: Ben Widawsky, Ben Widawsky This test assumes that using gem_pin will pin in the same address space used by execbuf. Since pin is for the global GTT only, use the existing flag to notify i915.ko that the exec object is special. Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- tests/gem_pin.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/gem_pin.c b/tests/gem_pin.c index 273315f..ca48a04 100644 --- a/tests/gem_pin.c +++ b/tests/gem_pin.c @@ -45,6 +45,8 @@ #define BLT_WRITE_ALPHA (1<<21) #define BLT_WRITE_RGB (1<<20) +#define LOCAL__EXEC_OBJ_NEEDS_GTT (1<<1) + static void exec(int fd, uint32_t handle, uint32_t offset) { struct drm_i915_gem_execbuffer2 execbuf; @@ -63,7 +65,7 @@ static void exec(int fd, uint32_t handle, uint32_t offset) gem_exec[0].relocs_ptr = (uintptr_t) gem_reloc; gem_exec[0].alignment = 0; gem_exec[0].offset = 0; - gem_exec[0].flags = 0; + gem_exec[0].flags = LOCAL__EXEC_OBJ_NEEDS_GTT; gem_exec[0].rsvd1 = 0; gem_exec[0].rsvd2 = 0; -- 1.8.4.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] gem_pin: Be explicit about GGTT needs 2013-11-07 4:05 [PATCH] gem_pin: Be explicit about GGTT needs Ben Widawsky @ 2013-11-07 8:30 ` Chris Wilson 2013-11-07 8:33 ` Daniel Vetter 1 sibling, 0 replies; 9+ messages in thread From: Chris Wilson @ 2013-11-07 8:30 UTC (permalink / raw) To: Ben Widawsky; +Cc: Intel GFX, Ben Widawsky On Wed, Nov 06, 2013 at 08:05:02PM -0800, Ben Widawsky wrote: > This test assumes that using gem_pin will pin in the same address space > used by execbuf. Since pin is for the global GTT only, use the existing > flag to notify i915.ko that the exec object is special. Interesting. Yes, I think you are right, the flag is required for the test to be correct with full-ppgtt. > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] gem_pin: Be explicit about GGTT needs 2013-11-07 4:05 [PATCH] gem_pin: Be explicit about GGTT needs Ben Widawsky 2013-11-07 8:30 ` Chris Wilson @ 2013-11-07 8:33 ` Daniel Vetter 2013-11-07 18:11 ` Ben Widawsky 1 sibling, 1 reply; 9+ messages in thread From: Daniel Vetter @ 2013-11-07 8:33 UTC (permalink / raw) To: Ben Widawsky; +Cc: Intel GFX, Ben Widawsky On Wed, Nov 06, 2013 at 08:05:02PM -0800, Ben Widawsky wrote: > This test assumes that using gem_pin will pin in the same address space > used by execbuf. Since pin is for the global GTT only, use the existing > flag to notify i915.ko that the exec object is special. > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > --- > tests/gem_pin.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/tests/gem_pin.c b/tests/gem_pin.c > index 273315f..ca48a04 100644 > --- a/tests/gem_pin.c > +++ b/tests/gem_pin.c > @@ -45,6 +45,8 @@ > #define BLT_WRITE_ALPHA (1<<21) > #define BLT_WRITE_RGB (1<<20) > > +#define LOCAL__EXEC_OBJ_NEEDS_GTT (1<<1) > + > static void exec(int fd, uint32_t handle, uint32_t offset) > { > struct drm_i915_gem_execbuffer2 execbuf; > @@ -63,7 +65,7 @@ static void exec(int fd, uint32_t handle, uint32_t offset) > gem_exec[0].relocs_ptr = (uintptr_t) gem_reloc; > gem_exec[0].alignment = 0; > gem_exec[0].offset = 0; > - gem_exec[0].flags = 0; > + gem_exec[0].flags = LOCAL__EXEC_OBJ_NEEDS_GTT; This only really works with the aliasing ppgtt stuff on gen6, I'd just skip the test -it's not really useful with real ppgtt. -Daniel > gem_exec[0].rsvd1 = 0; > gem_exec[0].rsvd2 = 0; > > -- > 1.8.4.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] gem_pin: Be explicit about GGTT needs 2013-11-07 8:33 ` Daniel Vetter @ 2013-11-07 18:11 ` Ben Widawsky 2013-11-07 18:36 ` Daniel Vetter 0 siblings, 1 reply; 9+ messages in thread From: Ben Widawsky @ 2013-11-07 18:11 UTC (permalink / raw) To: Daniel Vetter; +Cc: Intel GFX, Ben Widawsky On Thu, Nov 07, 2013 at 09:33:06AM +0100, Daniel Vetter wrote: > On Wed, Nov 06, 2013 at 08:05:02PM -0800, Ben Widawsky wrote: > > This test assumes that using gem_pin will pin in the same address space > > used by execbuf. Since pin is for the global GTT only, use the existing > > flag to notify i915.ko that the exec object is special. > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net> > > --- > > tests/gem_pin.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/tests/gem_pin.c b/tests/gem_pin.c > > index 273315f..ca48a04 100644 > > --- a/tests/gem_pin.c > > +++ b/tests/gem_pin.c > > @@ -45,6 +45,8 @@ > > #define BLT_WRITE_ALPHA (1<<21) > > #define BLT_WRITE_RGB (1<<20) > > > > +#define LOCAL__EXEC_OBJ_NEEDS_GTT (1<<1) > > + > > static void exec(int fd, uint32_t handle, uint32_t offset) > > { > > struct drm_i915_gem_execbuffer2 execbuf; > > @@ -63,7 +65,7 @@ static void exec(int fd, uint32_t handle, uint32_t offset) > > gem_exec[0].relocs_ptr = (uintptr_t) gem_reloc; > > gem_exec[0].alignment = 0; > > gem_exec[0].offset = 0; > > - gem_exec[0].flags = 0; > > + gem_exec[0].flags = LOCAL__EXEC_OBJ_NEEDS_GTT; > > This only really works with the aliasing ppgtt stuff on gen6, I'd just > skip the test -it's not really useful with real ppgtt. > -Daniel It is really useful with real ppgtt. Please rethink your assertion. > > > gem_exec[0].rsvd1 = 0; > > gem_exec[0].rsvd2 = 0; > > > > -- > > 1.8.4.2 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Ben Widawsky, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] gem_pin: Be explicit about GGTT needs 2013-11-07 18:11 ` Ben Widawsky @ 2013-11-07 18:36 ` Daniel Vetter 2013-11-07 18:46 ` Ben Widawsky 0 siblings, 1 reply; 9+ messages in thread From: Daniel Vetter @ 2013-11-07 18:36 UTC (permalink / raw) To: Ben Widawsky; +Cc: Intel GFX, Ben Widawsky On Thu, Nov 7, 2013 at 7:11 PM, Ben Widawsky <ben@bwidawsk.net> wrote: >> > @@ -63,7 +65,7 @@ static void exec(int fd, uint32_t handle, uint32_t offset) >> > gem_exec[0].relocs_ptr = (uintptr_t) gem_reloc; >> > gem_exec[0].alignment = 0; >> > gem_exec[0].offset = 0; >> > - gem_exec[0].flags = 0; >> > + gem_exec[0].flags = LOCAL__EXEC_OBJ_NEEDS_GTT; >> >> This only really works with the aliasing ppgtt stuff on gen6, I'd just >> skip the test -it's not really useful with real ppgtt. >> -Daniel > > It is really useful with real ppgtt. Please rethink your assertion. The test links up the global gtt used by the current pin ioctl with the ppgtt. That's not useful, except when they alias. If we want soft-pinning, then we need a new ioctl mode to return the right address from the right address space. Which means a new (sub)test. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] gem_pin: Be explicit about GGTT needs 2013-11-07 18:36 ` Daniel Vetter @ 2013-11-07 18:46 ` Ben Widawsky 2013-11-07 18:54 ` Daniel Vetter 0 siblings, 1 reply; 9+ messages in thread From: Ben Widawsky @ 2013-11-07 18:46 UTC (permalink / raw) To: Daniel Vetter; +Cc: Intel GFX, Ben Widawsky On Thu, Nov 07, 2013 at 07:36:50PM +0100, Daniel Vetter wrote: > On Thu, Nov 7, 2013 at 7:11 PM, Ben Widawsky <ben@bwidawsk.net> wrote: > >> > @@ -63,7 +65,7 @@ static void exec(int fd, uint32_t handle, uint32_t offset) > >> > gem_exec[0].relocs_ptr = (uintptr_t) gem_reloc; > >> > gem_exec[0].alignment = 0; > >> > gem_exec[0].offset = 0; > >> > - gem_exec[0].flags = 0; > >> > + gem_exec[0].flags = LOCAL__EXEC_OBJ_NEEDS_GTT; > >> > >> This only really works with the aliasing ppgtt stuff on gen6, I'd just > >> skip the test -it's not really useful with real ppgtt. > >> -Daniel > > > > It is really useful with real ppgtt. Please rethink your assertion. > > The test links up the global gtt used by the current pin ioctl with > the ppgtt. That's not useful, except when they alias. If we want > soft-pinning, then we need a new ioctl mode to return the right > address from the right address space. Which means a new (sub)test. > -Daniel This is only indirectly related to soft pinning. No context will ever have the global GTT address space. If you want to support the pin IOCTL, (which Chris has before said he requires - the original patch series disabled it) you must do this. We already have a flag that does what we want, and, demonstrably, a test which exercises it. I do not think a new ioctl, nor a new subtest is the solution for this. -- Ben Widawsky, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] gem_pin: Be explicit about GGTT needs 2013-11-07 18:46 ` Ben Widawsky @ 2013-11-07 18:54 ` Daniel Vetter 2013-11-07 18:58 ` Ben Widawsky 0 siblings, 1 reply; 9+ messages in thread From: Daniel Vetter @ 2013-11-07 18:54 UTC (permalink / raw) To: Ben Widawsky; +Cc: Intel GFX, Ben Widawsky On Thu, Nov 7, 2013 at 7:46 PM, Ben Widawsky <ben@bwidawsk.net> wrote: > On Thu, Nov 07, 2013 at 07:36:50PM +0100, Daniel Vetter wrote: >> On Thu, Nov 7, 2013 at 7:11 PM, Ben Widawsky <ben@bwidawsk.net> wrote: >> >> > @@ -63,7 +65,7 @@ static void exec(int fd, uint32_t handle, uint32_t offset) >> >> > gem_exec[0].relocs_ptr = (uintptr_t) gem_reloc; >> >> > gem_exec[0].alignment = 0; >> >> > gem_exec[0].offset = 0; >> >> > - gem_exec[0].flags = 0; >> >> > + gem_exec[0].flags = LOCAL__EXEC_OBJ_NEEDS_GTT; >> >> >> >> This only really works with the aliasing ppgtt stuff on gen6, I'd just >> >> skip the test -it's not really useful with real ppgtt. >> >> -Daniel >> > >> > It is really useful with real ppgtt. Please rethink your assertion. >> >> The test links up the global gtt used by the current pin ioctl with >> the ppgtt. That's not useful, except when they alias. If we want >> soft-pinning, then we need a new ioctl mode to return the right >> address from the right address space. Which means a new (sub)test. >> -Daniel > > This is only indirectly related to soft pinning. No context will ever have the > global GTT address space. If you want to support the pin IOCTL, (which > Chris has before said he requires - the original patch series disabled > it) you must do this. We already have a flag that does what we want, > and, demonstrably, a test which exercises it. > > I do not think a new ioctl, nor a new subtest is the solution for this. Ok, I'll wait until I see the code that implements this on top of ppgtt. But until we have that I don't think we need to change the test at all. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] gem_pin: Be explicit about GGTT needs 2013-11-07 18:54 ` Daniel Vetter @ 2013-11-07 18:58 ` Ben Widawsky 2013-11-07 20:59 ` Chris Wilson 0 siblings, 1 reply; 9+ messages in thread From: Ben Widawsky @ 2013-11-07 18:58 UTC (permalink / raw) To: Daniel Vetter; +Cc: Intel GFX, Ben Widawsky On Thu, Nov 07, 2013 at 07:54:59PM +0100, Daniel Vetter wrote: > On Thu, Nov 7, 2013 at 7:46 PM, Ben Widawsky <ben@bwidawsk.net> wrote: > > On Thu, Nov 07, 2013 at 07:36:50PM +0100, Daniel Vetter wrote: > >> On Thu, Nov 7, 2013 at 7:11 PM, Ben Widawsky <ben@bwidawsk.net> wrote: > >> >> > @@ -63,7 +65,7 @@ static void exec(int fd, uint32_t handle, uint32_t offset) > >> >> > gem_exec[0].relocs_ptr = (uintptr_t) gem_reloc; > >> >> > gem_exec[0].alignment = 0; > >> >> > gem_exec[0].offset = 0; > >> >> > - gem_exec[0].flags = 0; > >> >> > + gem_exec[0].flags = LOCAL__EXEC_OBJ_NEEDS_GTT; > >> >> > >> >> This only really works with the aliasing ppgtt stuff on gen6, I'd just > >> >> skip the test -it's not really useful with real ppgtt. > >> >> -Daniel > >> > > >> > It is really useful with real ppgtt. Please rethink your assertion. > >> > >> The test links up the global gtt used by the current pin ioctl with > >> the ppgtt. That's not useful, except when they alias. If we want > >> soft-pinning, then we need a new ioctl mode to return the right > >> address from the right address space. Which means a new (sub)test. > >> -Daniel > > > > This is only indirectly related to soft pinning. No context will ever have the > > global GTT address space. If you want to support the pin IOCTL, (which > > Chris has before said he requires - the original patch series disabled > > it) you must do this. We already have a flag that does what we want, > > and, demonstrably, a test which exercises it. > > > > I do not think a new ioctl, nor a new subtest is the solution for this. > > Ok, I'll wait until I see the code that implements this on top of > ppgtt. But until we have that I don't think we need to change the test > at all. > -Daniel This is a question for Chris. If DDX only depends on the behavior with aliasing PPGTT, then I agree - no test change needed. -- Ben Widawsky, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] gem_pin: Be explicit about GGTT needs 2013-11-07 18:58 ` Ben Widawsky @ 2013-11-07 20:59 ` Chris Wilson 0 siblings, 0 replies; 9+ messages in thread From: Chris Wilson @ 2013-11-07 20:59 UTC (permalink / raw) To: Ben Widawsky; +Cc: Intel GFX, Ben Widawsky On Thu, Nov 07, 2013 at 10:58:24AM -0800, Ben Widawsky wrote: > On Thu, Nov 07, 2013 at 07:54:59PM +0100, Daniel Vetter wrote: > > On Thu, Nov 7, 2013 at 7:46 PM, Ben Widawsky <ben@bwidawsk.net> wrote: > > > On Thu, Nov 07, 2013 at 07:36:50PM +0100, Daniel Vetter wrote: > > >> On Thu, Nov 7, 2013 at 7:11 PM, Ben Widawsky <ben@bwidawsk.net> wrote: > > >> >> > @@ -63,7 +65,7 @@ static void exec(int fd, uint32_t handle, uint32_t offset) > > >> >> > gem_exec[0].relocs_ptr = (uintptr_t) gem_reloc; > > >> >> > gem_exec[0].alignment = 0; > > >> >> > gem_exec[0].offset = 0; > > >> >> > - gem_exec[0].flags = 0; > > >> >> > + gem_exec[0].flags = LOCAL__EXEC_OBJ_NEEDS_GTT; > > >> >> > > >> >> This only really works with the aliasing ppgtt stuff on gen6, I'd just > > >> >> skip the test -it's not really useful with real ppgtt. > > >> >> -Daniel > > >> > > > >> > It is really useful with real ppgtt. Please rethink your assertion. > > >> > > >> The test links up the global gtt used by the current pin ioctl with > > >> the ppgtt. That's not useful, except when they alias. If we want > > >> soft-pinning, then we need a new ioctl mode to return the right > > >> address from the right address space. Which means a new (sub)test. > > >> -Daniel > > > > > > This is only indirectly related to soft pinning. No context will ever have the > > > global GTT address space. If you want to support the pin IOCTL, (which > > > Chris has before said he requires - the original patch series disabled > > > it) you must do this. We already have a flag that does what we want, > > > and, demonstrably, a test which exercises it. > > > > > > I do not think a new ioctl, nor a new subtest is the solution for this. > > > > Ok, I'll wait until I see the code that implements this on top of > > ppgtt. But until we have that I don't think we need to change the test > > at all. > > -Daniel > > This is a question for Chris. If DDX only depends on the behavior with > aliasing PPGTT, then I agree - no test change needed. Where pin has come in handy has been to w/a hw bugs, so predicting when and why we might need it again is difficult. As such I'd rather we maintained its existing semantics of pinning an object in the ggtt into the full-ppgtt future. As such it is should be trivial to implement the ioctl with very little impact. -chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-11-07 20:59 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-11-07 4:05 [PATCH] gem_pin: Be explicit about GGTT needs Ben Widawsky 2013-11-07 8:30 ` Chris Wilson 2013-11-07 8:33 ` Daniel Vetter 2013-11-07 18:11 ` Ben Widawsky 2013-11-07 18:36 ` Daniel Vetter 2013-11-07 18:46 ` Ben Widawsky 2013-11-07 18:54 ` Daniel Vetter 2013-11-07 18:58 ` Ben Widawsky 2013-11-07 20:59 ` Chris Wilson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.