On Wed, 2008-02-27 at 21:55 +0100, Thomas Hellström wrote: > Eric Anholt wrote: > > On Wed, 2008-02-27 at 20:41 +0100, Thomas Hellström wrote: > > > >> Eric Anholt wrote: > >> > >>> On Wed, 2008-02-27 at 10:49 -0800, Thomas Hellstrom wrote: > >>> > >>> > >>>> shared-core/i915_dma.c | 13 +++++++++---- > >>>> 1 file changed, 9 insertions(+), 4 deletions(-) > >>>> > >>>> New commits: > >>>> commit 72983ff30183745cd96760aa07b857c44daebde7 > >>>> Author: Thomas Hellstrom <thomas-at-tungstengraphics-dot-com> > >>>> Date: Wed Feb 27 19:46:28 2008 +0100 > >>>> > >>>> Don't wait for buffer idle before applying relocations. > >>>> > >>>> diff --git a/shared-core/i915_dma.c b/shared-core/i915_dma.c > >>>> index b916441..2d26fcc 100644 > >>>> --- a/shared-core/i915_dma.c > >>>> +++ b/shared-core/i915_dma.c > >>>> @@ -860,6 +860,15 @@ int i915_apply_reloc(struct drm_file *file_priv, > >>>> int num_buffers, > >>>> drm_bo_kunmap(&relocatee->kmap); > >>>> relocatee->data_page = NULL; > >>>> relocatee->offset = new_cmd_offset; > >>>> + > >>>> + /* > >>>> + * Note on buffer idle: > >>>> + * Since we're applying relocations, this part of the > >>>> + * buffer is obviously not used by the GPU and we don't > >>>> + * need to wait for buffer idle. This is an important > >>>> + * consideration for user-space buffer pools. > >>>> + */ > >>>> + > >>>> > >>>> > >>> I don't understand this comment. What would have ensured that this part > >>> of the buffer is not used by the GPU if you've removed the syncing that > >>> the DRM does when applying relocations? > >>> > >>> > >> Hmm, > >> I'd say user-space would've idled the buffers when they were written to > >> and the relocations were created. > >> But I guess you can use relocations to just patch state buffers up > >> without mapping them from user-space. Are you doing that for i965? In > >> that case I'll immediately revert to the original code. > >> > > > > User space *can't* idle the buffers, because it doesn't know if the > > kernel will need to write an updated relocation for buffer location > > moving or not. Only the kernel has that knowledge, unless you want to > > just fail out to userland when any buffer has moved. That sounds like > > minimal fun. > > > OK, I was assuming that anyone submitting a relocation would actually > have mapped the buffer when doing that, causing an implicit idle. I > guess this breaks i965.
Nope. The usage model for state buffers is that we create a state
buffer for a unique set of state (hardware state plus buffer objects to
be referenced by that hardware state), then we reuse that over and over
without ever mapping it again from userland[1]. The trick is that the
referenced buffers may get moved by the kernel, so we associate these
relocations with them when handing them to the kernel, and the kernel
does the fixups as necessary (which is rather rare, once we did the
presumed offset change).
Note that the state hash key data is pointers to referenced buffer
objects, not offsets of referenced buffer objects (wouldn't work as you
can imagine some new buffer of the same class but different contents
getting migrated in at the same offset as a different old one).
We thought about trying to achieve truly static state buffers, but it
sounded bad. If we did use buffer pointers plus buffer offsets in the
keys so we never had to rewrite relocations again, it would mean that
when the kernel did have to move something, we'd have to exit all the
way out from the kernel, create a new buffer referencing the new offset,
and recalculate our state into it. That sounded worse than the current
plan. In particular, the case of creating a new set of state several
levels deep results in:
exec with relocation A -> B, B -> C
kernel migrates C in
exit from kernel
userland creates new B' pointing at C
exec with relocation A -> B' -> C
kernel migrates B' in
exit from kernel
userland creates A' pointing at B'
exec with relocation A' -> B' -> C
kernel migrates A' in and you finally win. Winning somehow feels like
losing.
It would mean that for any chance at performance you'd have to
pre-validate C when creating B, and B when creating A. And when a
buffer like C gets migrated, presumably you would want to throw out B,
and the A that points at B, since you're going to have to redo them
anyway and otherwise they'll pollute your state cache and waste memory.
Also, experiments with trying to do prevalidation with drmBOSetStatus to
avoid kernel relocations on new buffers were generally unpleasant. So
I'm pretty sure we've made the right design decision with having our
buffers be static except in the case of migration of relocation targets,
with that relocation update handled by the kernel.
[1] OK, there's an exception to this that we do to avoid kernel
relocations in one case. Here's a set of different relocation paths
that may occur, to highlight the weird one:
1) app startup, new buffers
create BO A, B
exec with relocation BO A -> BO B
Result: migrate BO B and A, kernel writes BO A relocation to BO B
2) normal path
exec with relocation BO A -> BO B
Result: kernel doesn't move BO B, no relocations occur because userland
told the kernel that BO A already had the old location for BO B.
Common case.
3) An new buffer pointing at the same state
create BO C
exec with relocation BO C -> BO B
Result: Userland wrote BO B's old location in creating BO C, and marked
its presumed offset. If BO B doesn't move, kernel skips
relocation.
Common case.
Suppose kernel migrates BO B this time, though.
Kernel gives BO C relocation to BO B's new offset
4) Go back to #2 rendering path again
exec with relocation BO A -> BO B
Result: userland decides to map and relocate BO A instead of the kernel,
because it knows that even if its current guess at where BO B
happens to be (from #3 rendering) is wrong, the old guess at
where BO B is (which is still in BO A from #2 rendering) is
surely wrong.
Suppose kernel doesn't migrate BO B this time. The kernel
skips relocation because BO A had the right offset already,
thanks to userland. High fives all around.
Instead, in #4, we could just let the kernel handle that case like in #3
by unmarking presumed offset on BO B when submitting, but that affects
every relocation pointing at the given buffer in that submission.
Performance tradeoff either way is unknown, but probably not significant
since it's not the common case.
--
Eric Anholt [EMAIL PROTECTED]
[EMAIL PROTECTED] [EMAIL PROTECTED]
signature.asc
Description: This is a digitally signed message part
------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
-- _______________________________________________ Dri-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/dri-devel
