* [RFC] drm/i915: context support unit test
@ 2010-12-25 22:53 Ben Widawsky
2010-12-28 22:36 ` Daniel Vetter
0 siblings, 1 reply; 11+ messages in thread
From: Ben Widawsky @ 2010-12-25 22:53 UTC (permalink / raw)
To: intel-gfx; +Cc: benjamin.widawsky
[-- Attachment #1: Type: text/plain, Size: 607 bytes --]
I am requesting comments on the unit test for the context support I will be
adding. Attached is the unit test. I intend to create wrappers for the create
and destroy Ioctls in libdrm, unless someone has a better solution to reuse the
existing API. For the time being, I plan to use the rsvd1 field in the exec2
structure to store the context.
In summary, you'll see two new Ioctls in this test, and one new DRM API, but
once it's cleaned up, it will probably be 3 new Ioctls, and 3 new DRM APIs.
Also I realize this test doesn't cover a lot of the bad cases, but that will
be included later.
Thanks.
Ben
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gem_ctx_basic.c --]
[-- Type: text/x-c; charset=unknown-8bit, Size: 4169 bytes --]
/*
* Copyright © 2010 Intel Corporation
*
* Permission is hereby granted, free of charge, to any person obtaining a
* copy of this software and associated documentation files (the "Software"),
* to deal in the Software without restriction, including without limitation
* the rights to use, copy, modify, merge, publish, distribute, sublicense,
* and/or sell copies of the Software, and to permit persons to whom the
* Software is furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice (including the next
* paragraph) shall be included in all copies or substantial portions of the
* Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
* THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
* IN THE SOFTWARE.
*
* Authors:
* Ben Widawsky <benjamin.widawsky@linux.intel.com>
*
*/
#include <unistd.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <assert.h>
#include <fcntl.h>
#include <inttypes.h>
#include <errno.h>
#include <sys/stat.h>
#include <sys/ioctl.h>
#include "drm.h"
#include "i915_drm.h"
#include "intel_bufmgr.h"
#include "drmtest.h"
#define MAX_CONTEXTS 100
static int ctxs[MAX_CONTEXTS];
#define MI_INSTR(opcode, flags) (((opcode) << 23) | (flags))
#define MI_NOOP(id) MI_INSTR(0, (1 << 22) | id)
#define MI_BATCH_BUFFER_END MI_INSTR(0xa, 0)
static void
create_ctxs(const int fd, const int count)
{
struct drm_i915_gem_ctx_create ctx_create;
int ret, i;
printf("Testing context creation (%d).\n", count);
for (i = 0; i < count; i++) {
memset(&ctx_create, 0, sizeof(ctx_create));
ret = ioctl(fd, DRM_IOCTL_I915_GEM_CTX_CREATE, &ctx_create);
if (ret != 0) {
printf("%d ", i);
perror("ctx_create");
ctxs[i] = -1;
} else {
ctxs[i] = ctx_create.ctx_id;
printf("Got back id #%d\n", ctx_create.ctx_id);
}
}
}
static int
create_and_submit_batches(const int fd, const int count)
{
drm_intel_bufmgr *bufmgr;
drm_intel_bo *bo;
uint32_t *data;
int ret, i = 1;
/* Create the bufmgr */
bufmgr = drm_intel_bufmgr_gem_init(fd, 16 * 4096);
drm_intel_bufmgr_set_debug(bufmgr, 1);
/* Allocate a BO */
bo = drm_intel_bo_alloc(bufmgr, "test", 4 * 1024, 4);
assert(bo != NULL);
for (i = 0; i < count; i++) {
/* Put the commands in the batch buffer */
ret = drm_intel_bo_map(bo, 1);
assert(ret == 0);
data = (uint32_t *)bo->virtual;
*data++ = MI_NOOP(i);
*data = MI_BATCH_BUFFER_END;
ret = drm_intel_bo_unmap(bo);
assert(ret == 0);
/* Execute the batch buffer */
ret = drm_intel_bo_mrb_exec_ctx(bo, 8, NULL, 0, 0, I915_EXEC_DEFAULT, ctxs[i]);
if (ret != 0) {
perror("drm_intel_bo_mrb_exec");
return i;
}
/* Wait for completion */
drm_intel_bo_wait_rendering(bo);
}
return count;
}
static void
destroy_ctxs(const int fd, const int count)
{
int ret, i;
struct drm_i915_gem_ctx_destroy destroy;
printf("Testing context destruction (%d)\n", count);
for (i = 0; i < count; i++) {
memset(&destroy, 0, sizeof(destroy));
if (ctxs[i] == -1)
continue;
destroy.ctx_id = ctxs[i];
ret = ioctl(fd, DRM_IOCTL_I915_GEM_CTX_DESTROY, &destroy);
if (ret != 0) {
printf("%d ", i);
perror("destroy");
} else {
printf("destroyed context %d\n", ctxs[i]);
}
}
}
int main(int argc, char *argv[])
{
int fd, count, ret;
if (argc != 2) {
printf("usage: %s <number of contexts>\n", argv[0]);
exit(-1);
}
fd = drm_open_any();
count = strtol(argv[1], NULL, 0);
memset(ctxs, 0, sizeof(*ctxs));
create_ctxs(fd, count);
ret = create_and_submit_batches(fd, count);
destroy_ctxs(fd, count);
assert(ret == count);
close(fd);
return 0;
}
[-- Attachment #3: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [RFC] drm/i915: context support unit test
2010-12-25 22:53 [RFC] drm/i915: context support unit test Ben Widawsky
@ 2010-12-28 22:36 ` Daniel Vetter
2010-12-29 4:03 ` Ben Widawsky
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Vetter @ 2010-12-28 22:36 UTC (permalink / raw)
To: Ben Widawsky; +Cc: benjamin.widawsky, intel-gfx
Hi Ben,
On Sat, Dec 25, 2010 at 02:53:04PM -0800, Ben Widawsky wrote:
> I am requesting comments on the unit test for the context support I will be
> adding. Attached is the unit test. I intend to create wrappers for the create
> and destroy Ioctls in libdrm, unless someone has a better solution to reuse the
> existing API. For the time being, I plan to use the rsvd1 field in the exec2
> structure to store the context.
>
> In summary, you'll see two new Ioctls in this test, and one new DRM API, but
> once it's cleaned up, it will probably be 3 new Ioctls, and 3 new DRM APIs.
> Also I realize this test doesn't cover a lot of the bad cases, but that will
> be included later.
Just a few questions on the api:
- How does this tie in with the multiple ringbuffer support? Is the kernel
supposed to lazily allocate contexts for each ring as soon as userspaces
uses it on a given ring for the first time? imho the simpler approach
than adding an explicit ring arg to the ctx_create ioctl.
- (Assuming that the context stores pointers to the indirect state
objects - public docs are unclear in that matter) How do you plan to
handle bo eviction? The simplest thing is probably to bail on execbuf
in the kernel and ask userspace to reissue a complete context. Also: How
does the kernel know that evicting/moving a given bo invalidates a
certain context? Do you intend to create that connection implicitly with
i915_gem_domain_instruction or with some new reloc flag?
> Thanks.
> Ben
Cheers, Daniel
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC] drm/i915: context support unit test
2010-12-28 22:36 ` Daniel Vetter
@ 2010-12-29 4:03 ` Ben Widawsky
2010-12-30 10:07 ` Daniel Vetter
0 siblings, 1 reply; 11+ messages in thread
From: Ben Widawsky @ 2010-12-29 4:03 UTC (permalink / raw)
To: Daniel Vetter; +Cc: benjamin.widawsky, intel-gfx
On Tue, Dec 28, 2010 at 11:36:17PM +0100, Daniel Vetter wrote:
> Hi Ben,
>
> On Sat, Dec 25, 2010 at 02:53:04PM -0800, Ben Widawsky wrote:
> > I am requesting comments on the unit test for the context support I will be
> > adding. Attached is the unit test. I intend to create wrappers for the create
> > and destroy Ioctls in libdrm, unless someone has a better solution to reuse the
> > existing API. For the time being, I plan to use the rsvd1 field in the exec2
> > structure to store the context.
> >
> > In summary, you'll see two new Ioctls in this test, and one new DRM API, but
> > once it's cleaned up, it will probably be 3 new Ioctls, and 3 new DRM APIs.
> > Also I realize this test doesn't cover a lot of the bad cases, but that will
> > be included later.
>
> Just a few questions on the api:
> - How does this tie in with the multiple ringbuffer support? Is the kernel
> supposed to lazily allocate contexts for each ring as soon as userspaces
> uses it on a given ring for the first time? imho the simpler approach
> than adding an explicit ring arg to the ctx_create ioctl.
> - (Assuming that the context stores pointers to the indirect state
> objects - public docs are unclear in that matter) How do you plan to
> handle bo eviction? The simplest thing is probably to bail on execbuf
> in the kernel and ask userspace to reissue a complete context. Also: How
> does the kernel know that evicting/moving a given bo invalidates a
> certain context? Do you intend to create that connection implicitly with
> i915_gem_domain_instruction or with some new reloc flag?
>
> > Thanks.
> > Ben
>
> Cheers, Daniel
>
> --
> Daniel Vetter
> Mail: daniel@ffwll.ch
> Mobile: +41 (0)79 365 57 48
Daniel,
There is no tie in to multiple ringbuffer support. A client may allocate
a context for all ringuffers, or one for each ringbuffer. I too must
figure out if this is relevant to anything but the graphics engine.
The immediate plan is to allocate space for the HW context at the time
the client gets (lazily or not) a new context. The memory will be pinned
at this point, because it's somewhat difficult to make sure the memory
will be there on future context switches. This is a possible area for
improvement, but to do this would require keeping tracking of the last
context to run, and then possibly paging in memory of that context
before the new context can run.
Unfortunately I'm new to this HW and SW design, so I may not have
totally followed your questions about eviction, or invalidation. The
answer is, I don't yet know. If your assumption is right, and it's
possible to set the hardware state such that it may reference memory in
a subsequent batch that's not referenced in the subsequent batch
buffers, then there is some work to be done which isn't part of the
initial implementation. It will be up to user space to pin any objects
which may be referenced in the future. After the initial implementation,
we can decide how to proceed, but your suggestion seems reasonable to
me, and I'd have to do more research.
Regarding lazy creation:
The current design allows a client to create multiple contexts, or even
possibly share contexts. I'm not sure of an easy way to meet those goals
with a lazy allocation. My original plan was to modify the gem alloc API
so that each bo would be associated with a context, but either way it
involved adding a new API (since I couldn't destroy the old alloc API),
and I figured there may be some uses for contexts which don't require a
bo in the future.
Thanks.
Ben
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC] drm/i915: context support unit test
2010-12-29 4:03 ` Ben Widawsky
@ 2010-12-30 10:07 ` Daniel Vetter
2010-12-30 11:13 ` Chris Wilson
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Vetter @ 2010-12-30 10:07 UTC (permalink / raw)
To: Ben Widawsky; +Cc: benjamin.widawsky, intel-gfx
Hi Ben,
Am Mi, 29.12.2010, 05:03, schrieb Ben Widawsky:
> There is no tie in to multiple ringbuffer support. A client may allocate
> a context for all ringuffers, or one for each ringbuffer. I too must
> figure out if this is relevant to anything but the graphics engine.
I've been simply thinking that using a hw context on multiple rings is
probably a bad idea. Hence is should be impossible to do. Maybe it's
indeed possible and makes sense/is actually required to accomplish
your goals? If not, I think your api needs some clarification for this.
> The immediate plan is to allocate space for the HW context at the time
> the client gets (lazily or not) a new context. The memory will be pinned
> at this point, because it's somewhat difficult to make sure the memory
> will be there on future context switches. This is a possible area for
> improvement, but to do this would require keeping tracking of the last
> context to run, and then possibly paging in memory of that context
> before the new context can run.
>
> Unfortunately I'm new to this HW and SW design, so I may not have
> totally followed your questions about eviction, or invalidation. The
> answer is, I don't yet know. If your assumption is right, and it's
> possible to set the hardware state such that it may reference memory in
> a subsequent batch that's not referenced in the subsequent batch
> buffers, then there is some work to be done which isn't part of the
> initial implementation. It will be up to user space to pin any objects
> which may be referenced in the future. After the initial implementation,
> we can decide how to proceed, but your suggestion seems reasonable to
> me, and I'd have to do more research.
The scenario I have in mind:
0. You create 2 hw contexts.
1. You run a batchbuffer with context 1.
2. You run a batchbuffer with another context. This may evict a few indirect
state bos (and other stuff) used by context 1.
3. You reuse context 1 in the next batchbuffer, hw reads garbaged when
loading the indirect state. I don't know whether hw actually does this
but similar problems exist for textures, render targets, ...
You mention that userspace must pin any objects that may be referenced
in the future. I absolutely don't like this plan. With kms gem has full
control over the memory layout. In theory we could even move around
scanout buffers if we exchange the backing store and execute a flip.
Forcing userspace to pin buffers (rather large ones in case of textures/
render targets) is completely against the spirit of gem. The simplest
way out is to invalidate a context a soon as a bo gets moved out of the
gtt (if it has ever been associated with that context) and tell userspace
that it needs to reissue the batch with a full context-reinit sequence.
Of course, whether that's the right thing to do depends upon the anticipated
use-case of hw contexts. Can you elaborate a bit on this? As an outside
contributor I'm slightly out of the loop in such matters ...
> Regarding lazy creation:
> The current design allows a client to create multiple contexts, or even
> possibly share contexts. I'm not sure of an easy way to meet those goals
> with a lazy allocation. My original plan was to modify the gem alloc API
> so that each bo would be associated with a context, but either way it
> involved adding a new API (since I couldn't destroy the old alloc API),
> and I figured there may be some uses for contexts which don't require a
> bo in the future.
I don't think there's any valid use-case for userspace to access the
contents of a hw context (as using bos would allow). Hence I think handling
hw contexts separately from buffer objects is a good thing. Sharing them
is probably of little value (safe for some not yet publicly disclosed
upcoming hw feature).
> Thanks.
> Ben
Cheers, Daniel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC] drm/i915: context support unit test
2010-12-30 10:07 ` Daniel Vetter
@ 2010-12-30 11:13 ` Chris Wilson
2010-12-30 12:48 ` Daniel Vetter
0 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2010-12-30 11:13 UTC (permalink / raw)
To: Daniel Vetter, Ben Widawsky; +Cc: benjamin.widawsky, intel-gfx
On Thu, 30 Dec 2010 11:07:19 +0100 (CET), "Daniel Vetter" <daniel@ffwll.ch> wrote:
> Hi Ben,
>
> Am Mi, 29.12.2010, 05:03, schrieb Ben Widawsky:
> > There is no tie in to multiple ringbuffer support. A client may allocate
> > a context for all ringuffers, or one for each ringbuffer. I too must
> > figure out if this is relevant to anything but the graphics engine.
>
> I've been simply thinking that using a hw context on multiple rings is
> probably a bad idea. Hence is should be impossible to do. Maybe it's
> indeed possible and makes sense/is actually required to accomplish
> your goals? If not, I think your api needs some clarification for this.
Per usual, Daniel is right on the mark. So let's see if I can add anything
useful...
> > The immediate plan is to allocate space for the HW context at the time
> > the client gets (lazily or not) a new context. The memory will be pinned
> > at this point, because it's somewhat difficult to make sure the memory
> > will be there on future context switches. This is a possible area for
> > improvement, but to do this would require keeping tracking of the last
> > context to run, and then possibly paging in memory of that context
> > before the new context can run.
> >
> > Unfortunately I'm new to this HW and SW design, so I may not have
> > totally followed your questions about eviction, or invalidation. The
> > answer is, I don't yet know. If your assumption is right, and it's
> > possible to set the hardware state such that it may reference memory in
> > a subsequent batch that's not referenced in the subsequent batch
> > buffers, then there is some work to be done which isn't part of the
> > initial implementation. It will be up to user space to pin any objects
> > which may be referenced in the future. After the initial implementation,
> > we can decide how to proceed, but your suggestion seems reasonable to
> > me, and I'd have to do more research.
>
> The scenario I have in mind:
> 0. You create 2 hw contexts.
> 1. You run a batchbuffer with context 1.
> 2. You run a batchbuffer with another context. This may evict a few indirect
> state bos (and other stuff) used by context 1.
> 3. You reuse context 1 in the next batchbuffer, hw reads garbaged when
> loading the indirect state. I don't know whether hw actually does this
> but similar problems exist for textures, render targets, ...
>
> You mention that userspace must pin any objects that may be referenced
> in the future. I absolutely don't like this plan. With kms gem has full
> control over the memory layout. In theory we could even move around
> scanout buffers if we exchange the backing store and execute a flip.
> Forcing userspace to pin buffers (rather large ones in case of textures/
> render targets) is completely against the spirit of gem. The simplest
> way out is to invalidate a context a soon as a bo gets moved out of the
> gtt (if it has ever been associated with that context) and tell userspace
> that it needs to reissue the batch with a full context-reinit sequence.
Exactly. Relying on userspace to pin the layout in the GTT here is just
fail. Active contexts must be tied into the batchbuffer+eviction system
and be managed accordingly.
> Of course, whether that's the right thing to do depends upon the anticipated
> use-case of hw contexts. Can you elaborate a bit on this? As an outside
> contributor I'm slightly out of the loop in such matters ...
This is just the first step towards run-lists and ppGTT. We've always
postulated that there might be some minor gain from maintaining context
between batchbuffers, but that is a secondary goal in comparison to the
incremental change towards the new dispatch mechanisms in Cantiga+,
particularly SandyBridge+.
> > Regarding lazy creation:
> > The current design allows a client to create multiple contexts, or even
> > possibly share contexts. I'm not sure of an easy way to meet those goals
> > with a lazy allocation. My original plan was to modify the gem alloc API
> > so that each bo would be associated with a context, but either way it
> > involved adding a new API (since I couldn't destroy the old alloc API),
> > and I figured there may be some uses for contexts which don't require a
> > bo in the future.
>
> I don't think there's any valid use-case for userspace to access the
> contents of a hw context (as using bos would allow). Hence I think handling
> hw contexts separately from buffer objects is a good thing. Sharing them
> is probably of little value (safe for some not yet publicly disclosed
> upcoming hw feature).
The contents of a HW context are not really privy to the driver, though
they are detailed on a per-chipset basis in the specs and one could
theoretically manipulate those contexts from userpsace. Without a valid
use case, we should restrict those to a per-file handle. Creating a global
namespace for contexts and permitting sharing between processes could be
done in exactly the same manner as named buffers, except that again I
don't believe there is a usecase for that, not even for shared GLX
contexts and the userspace synchronisation would seem to make it
nonsensical anyway.
The real issue is that once a bo has been used with a context, how can we
notify the kernel that context no longer references that bo (without
invoking a CS parser). This would seem simplest with another ioctl to
associate and disassociate bo with contexts, that would be serialised with
the execbuffer (pipelined of course!) in the same manner as tiling
changes. The active context + bo could still be evicted on mass, just so
long as we add a mechanism for restoring the gtt_offset for a particular
bo.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC] drm/i915: context support unit test
2010-12-30 11:13 ` Chris Wilson
@ 2010-12-30 12:48 ` Daniel Vetter
2010-12-30 20:21 ` Ben Widawsky
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Vetter @ 2010-12-30 12:48 UTC (permalink / raw)
To: Chris Wilson; +Cc: benjamin.widawsky, intel-gfx
Hi Chris,
Am Do, 30.12.2010, 12:13, schrieb Chris Wilson:
> The real issue is that once a bo has been used with a context, how can we
> notify the kernel that context no longer references that bo (without
> invoking a CS parser). This would seem simplest with another ioctl to
> associate and disassociate bo with contexts, that would be serialised with
> the execbuffer (pipelined of course!) in the same manner as tiling
> changes. The active context + bo could still be evicted on mass, just so
> long as we add a mechanism for restoring the gtt_offset for a particular
> bo.
An idea that just crossed my mind (probably bollocks, but who knows):
The number of buffers a context references is usually bounded (a few
indirect state bos + a bo per texture unit + render targets), so why not
specify how many persistent bos a given context uses. In execbuffer we add
a new reloc field (there are enough bits free in the domain fields) that
indicates which (if any) context bo slot this buffer should be in.
The kernel then puts that bo with a reference into the specified slot
(of the array allocated at context creation time), freeing any previous
bo in the same slot. Right before using the context (mi_batchbuffer or
mi_set_context) we can then walk this array and check whether all required
bos are still at the right place (by remembering their gtt_offset at reloc
time).
This way userspace doesn't have to track a list of context bos and the
kernel gem stays in full control. And specifying the context relation of
a bo toghether with the relocation that actually uses it is probably the
clearest solution.
And wrt restoring the gtt offset: I think to guarantee this, we need
ppGTT. In the GGTT we might get unlucky and a scanout buffer sits at
the desired spot. So at least for bring-up (until ppGTT works) we need
to be able to bail out in the kernel and politely ask userspace to fix up
the mess ;)
Cheers, Daniel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC] drm/i915: context support unit test
2010-12-30 12:48 ` Daniel Vetter
@ 2010-12-30 20:21 ` Ben Widawsky
2010-12-31 0:48 ` Ben Widawsky
0 siblings, 1 reply; 11+ messages in thread
From: Ben Widawsky @ 2010-12-30 20:21 UTC (permalink / raw)
To: Daniel Vetter; +Cc: benjamin.widawsky, intel-gfx
On Thu, Dec 30, 2010 at 01:48:36PM +0100, Daniel Vetter wrote:
> This way userspace doesn't have to track a list of context bos and the
> kernel gem stays in full control. And specifying the context relation of
> a bo toghether with the relocation that actually uses it is probably the
> clearest solution.
I'll need to research this one more.
>
> And wrt restoring the gtt offset: I think to guarantee this, we need
> ppGTT. In the GGTT we might get unlucky and a scanout buffer sits at
> the desired spot. So at least for bring-up (until ppGTT works) we need
> to be able to bail out in the kernel and politely ask userspace to fix up
> the mess ;)
I agree with this. Depending on what the HW does when it loads state, it
might also require allowing the client to have a way to prevent
restoring context on the next submission. I think it would be good to
add this to the API even if HW doesn't require it.
Either way, I think this gets messy if we allow sharing contexts amongst
clients. I hope nobody has a good use case for sharing contexts :-).
>
> Cheers, Daniel
>
Ben
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC] drm/i915: context support unit test
2010-12-30 20:21 ` Ben Widawsky
@ 2010-12-31 0:48 ` Ben Widawsky
2010-12-31 9:31 ` Chris Wilson
0 siblings, 1 reply; 11+ messages in thread
From: Ben Widawsky @ 2010-12-31 0:48 UTC (permalink / raw)
To: Chris Wilson, Daniel Vetter; +Cc: benjamin.widawsky, intel-gfx
On Thu, Dec 30, 2010 at 12:21:19PM -0800, Ben Widawsky wrote:
> On Thu, Dec 30, 2010 at 01:48:36PM +0100, Daniel Vetter wrote:
> > This way userspace doesn't have to track a list of context bos and the
> > kernel gem stays in full control. And specifying the context relation of
> > a bo toghether with the relocation that actually uses it is probably the
> > clearest solution.
> I'll need to research this one more.
It seems like you picked this slot based mechanism to get rid of needing
a disassociate IOCTL. At least if I followed you, the scheme allows you
to overwrite BOs associated with the context. The cost is userspace
would have to manage the slots. It'd be very easy to just have a flag to
associate a buffer with a context, and forget the slots.
As you point out this hinges on the assumption that not many buffers are
needed, and the size doesn't grow. I'm not the right person to judge the
accuracy of that, but it seems like an undesirable trait. I do really
like doing away with the extra IOCTLs though.
Chris, do you have an opinion? I'm leaning towards the two IOCTLs at
present.
The compromise would be to associate in the execbuffer, and disassociate
through an IOCTL.
Thanks.
Ben
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC] drm/i915: context support unit test
2010-12-31 0:48 ` Ben Widawsky
@ 2010-12-31 9:31 ` Chris Wilson
2011-01-04 20:10 ` Daniel Vetter
0 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2010-12-31 9:31 UTC (permalink / raw)
To: Ben Widawsky, Daniel Vetter; +Cc: benjamin.widawsky, intel-gfx
On Thu, 30 Dec 2010 16:48:31 -0800, Ben Widawsky <widawsky@gmail.com> wrote:
> On Thu, Dec 30, 2010 at 12:21:19PM -0800, Ben Widawsky wrote:
> > On Thu, Dec 30, 2010 at 01:48:36PM +0100, Daniel Vetter wrote:
> > > This way userspace doesn't have to track a list of context bos and the
> > > kernel gem stays in full control. And specifying the context relation of
> > > a bo toghether with the relocation that actually uses it is probably the
> > > clearest solution.
> > I'll need to research this one more.
>
> It seems like you picked this slot based mechanism to get rid of needing
> a disassociate IOCTL. At least if I followed you, the scheme allows you
> to overwrite BOs associated with the context. The cost is userspace
> would have to manage the slots. It'd be very easy to just have a flag to
> associate a buffer with a context, and forget the slots.
>
> As you point out this hinges on the assumption that not many buffers are
> needed, and the size doesn't grow. I'm not the right person to judge the
> accuracy of that, but it seems like an undesirable trait. I do really
> like doing away with the extra IOCTLs though.
>
> Chris, do you have an opinion? I'm leaning towards the two IOCTLs at
> present.
I'm still leaning towards explicit associate/disassociate ioctls as I
think that will lead to a simpler userspace API (or at least integrate
conveniently with the existing libdrm).
> The compromise would be to associate in the execbuffer, and disassociate
> through an IOCTL.
Using a flag in the execbuffer.object is one approach, but breaks the
symmetry and thereby consistency.
"Minimal, consistent and difficult to misuse."
If you've never seen them before:
http://ozlabs.org/~rusty/index.cgi/tech/2008-03-30.html with the corollary
http://ozlabs.org/~rusty/index.cgi/tech/2008-04-01.html is an enlightening
read.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC] drm/i915: context support unit test
2010-12-31 9:31 ` Chris Wilson
@ 2011-01-04 20:10 ` Daniel Vetter
2011-01-04 20:21 ` Ben Widawsky
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Vetter @ 2011-01-04 20:10 UTC (permalink / raw)
To: Chris Wilson; +Cc: benjamin.widawsky, intel-gfx
On Fri, Dec 31, 2010 at 09:31:23AM +0000, Chris Wilson wrote:
> I'm still leaning towards explicit associate/disassociate ioctls as I
> think that will lead to a simpler userspace API (or at least integrate
> conveniently with the existing libdrm).
I don't like a separate ioctl to associate/disassociate a buffer from a
context: Conceptually there's no way to change that association without an
execbuf call. Doing the book-keeping as a separate, non-atomic ioctl calls
calls for trouble imho.
> ...
> "Minimal, consistent and difficult to misuse."
i.e. violates the "difficult to misuse" paradigm ;) My idea of a special
reloc type is probably not the cleanest (hm, slight understatement here ;)
and could need some improvement. Anyway, I'll wait and see what the real
use-case looks like. My arguments feel too hand-waivy already ...
Cheers, Daniel
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC] drm/i915: context support unit test
2011-01-04 20:10 ` Daniel Vetter
@ 2011-01-04 20:21 ` Ben Widawsky
0 siblings, 0 replies; 11+ messages in thread
From: Ben Widawsky @ 2011-01-04 20:21 UTC (permalink / raw)
To: Daniel Vetter; +Cc: benjamin.widawsky, intel-gfx
On Tue, Jan 04, 2011 at 09:10:04PM +0100, Daniel Vetter wrote:
> On Fri, Dec 31, 2010 at 09:31:23AM +0000, Chris Wilson wrote:
> > I'm still leaning towards explicit associate/disassociate ioctls as I
> > think that will lead to a simpler userspace API (or at least integrate
> > conveniently with the existing libdrm).
>
> I don't like a separate ioctl to associate/disassociate a buffer from a
> context: Conceptually there's no way to change that association without an
> execbuf call. Doing the book-keeping as a separate, non-atomic ioctl calls
> calls for trouble imho.
>
I think there is a distinct possibility that we'll need a way to
associate buffers with either the GGTT or the ppGTT. I'm currently
trying to figure out if this is actually the case but I haven't yet been
able to find exactly which memory objects, if any, must be accessed
through the GGTT.
Just curious Daniel, if this is the case, does it change your feeling
on the IOCTL, or do you still prefer execbuf?
Ben
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2011-01-04 20:21 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-25 22:53 [RFC] drm/i915: context support unit test Ben Widawsky
2010-12-28 22:36 ` Daniel Vetter
2010-12-29 4:03 ` Ben Widawsky
2010-12-30 10:07 ` Daniel Vetter
2010-12-30 11:13 ` Chris Wilson
2010-12-30 12:48 ` Daniel Vetter
2010-12-30 20:21 ` Ben Widawsky
2010-12-31 0:48 ` Ben Widawsky
2010-12-31 9:31 ` Chris Wilson
2011-01-04 20:10 ` Daniel Vetter
2011-01-04 20:21 ` Ben Widawsky
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox