Re: i915: wait for buffer idle before writing relocations

2007-12-10 Thread Keith Whitwell 
Keith Packard wrote:
 On Fri, 2007-12-07 at 11:15 +, Keith Whitwell wrote:
 Keith,

 Thomas has just left for two weeks of (well deserved!) holiday, so he 
 may be slow to respond.
 
 Thanks for taking the time to have a look while he's away; we're
 finishing up the 965 TTM work, and it is posing some challenges with the
 existing kernel interface.
 
 In the meantime, have you considered how this will interact with 
 userspace buffer pools?
 
 No, I hadn't considered that as we're not considering a two-level
 allocation strategy at this point.
 
 However, if you consider the blocking patch in conjunction with the
 presumed_offset optimization, I think you'll find that userspace buffer
 pools will not actually be affected negatively by this change.
 
 The presumed_offset optimization allows the application to compute all
 relocations itself for target buffers which have been mapped to the
 hardware. The kernel relocations are purely a back-up, for cases where
 buffers move between EXECBUFFER invocations.
 
   I know you guys aren't using them at this 
 point, but I'm of the opinion that they are an important facility which 
 needs to be preserved.  At worst it may be that some additional flag is 
 needed to control this behaviour.
 
 We could do this, but I believe this would actually require more
 blocking by the client -- it doesn't know when objects are moving in the
 kernel, so it doesn't know when relocation data will need to be
 rewritten.
 
 Secondly I wonder whether this isn't already caught by other aspects of 
 the buffer manager behaviour?
 
 ie, if the buffer to which the relocation points to is being moved, 
 doesn't that imply all hardware activity related to that buffer must 
 have concluded?  IE, if the buffer itself is free to move, surely all 
 commands containing relocations (or chains of relocations) which point 
 to the buffer must themselves have completed??
 
 Yes, if the target buffer is moving, then the operation related to the
 relocatee will have been completed and waited for. But, re-writing
 relocations doesn't require that the buffers have moved. 
 
 Consider the case of the binding table on 965 which points at surface
 state structures. Executing a command that uses the binding table will
 require that relocations be evaluated for the entries in the table; even
 if nothing moves (ignoring my presumed_offset optimization), those
 relocations will need to be evaluated and the surface state pointers
 stored to the binding table.
 
 For the application to guarantee that the binding table relocations can
 be written without the kernel needing to wait for the binding table
 buffer to be idle, the application would have to wait every time, not
 just when the buffer actually moves.

OK, it sounds like you're talking about situations where the driver is 
modifying state in buffers *only* through changes to the relocations?

It's probably not surprising the fence is not implemented as I'd 
normally think that those relocation changes would be associated with 
some changes to the other data, and that would imply mapping the buffer 
(and hence the wait).  I do understand the examples though and can see 
where you're trying to take this.

Anyway, I'm hopeful that this won't break other usages...

Keith

-
SF.Net email is sponsored by: 
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Re: i915: wait for buffer idle before writing relocations

2007-12-10 Thread Keith Packard 

On Mon, 2007-12-10 at 13:27 +, Keith Whitwell wrote:

 OK, it sounds like you're talking about situations where the driver is 
 modifying state in buffers *only* through changes to the relocations?

Yes, although I also don't expect this to be common.

 It's probably not surprising the fence is not implemented as I'd 
 normally think that those relocation changes would be associated with 
 some changes to the other data, and that would imply mapping the buffer 
 (and hence the wait).

If the buffer was mapped (and waited for) by the client, then the kernel
'wait' will be free.

   I do understand the examples though and can see 
 where you're trying to take this.

Ok, thanks for thinking it through.

 Anyway, I'm hopeful that this won't break other usages...

I think the interesting usage that you point out is where the
application knows that a wait isn't necessary as the previously
referenced data will not be re-used, and only new portions of the buffer
need relocations. 

Given the choice between avoiding waits for cases we have today vs
avoiding waits for cases we may try in the future, it seems reasonable
to solve what we're using now.

-- 
[EMAIL PROTECTED]


signature.asc
Description: This is a digitally signed message part
-
SF.Net email is sponsored by: 
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Re: i915: wait for buffer idle before writing relocations

2007-12-10 Thread Keith Whitwell 
Yeah, I'm pretty interested to come up with an 'append' type of semantic for 
buffer usage, particularly for things like the state pools you guys are 
probably playing with at the moment.  It's not something that's ever going to 
be a *requirement* for a driver, and may not necessarily be a big win or even 
particularly difficult, but at this point nobody's really dug into it enough to 
know one way or another.

Ignoring relocation issues, an 'append' mapping semantic, as opposed to the 
existing read/write maps, is probably an interesting concept also as it could 
allow mapping a state pool buffer to add new states as required by the 
application, but not require a fence as the old ones won't be interfered with.  

Keith



- Original Message 
From: Keith Packard [EMAIL PROTECTED]
To: Keith Whitwell [EMAIL PROTECTED]
Cc: [EMAIL PROTECTED]; dri-devel dri-devel@lists.sourceforge.net
Sent: Monday, December 10, 2007 4:44:44 PM
Subject: Re: i915: wait for buffer idle before writing relocations

[...]

I think the interesting usage that you point out is where the
application knows that a wait isn't necessary as the previously
referenced data will not be re-used, and only new portions of the
 buffer
need relocations. 

Given the choice between avoiding waits for cases we have today vs
avoiding waits for cases we may try in the future, it seems reasonable
to solve what we're using now.

-- 
[EMAIL PROTECTED]




-
SF.Net email is sponsored by: 
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Re: i915: wait for buffer idle before writing relocations

2007-12-08 Thread Eric Anholt 
On Fri, 2007-12-07 at 11:15 +, Keith Whitwell wrote:
 Keith,
 
 Thomas has just left for two weeks of (well deserved!) holiday, so he 
 may be slow to respond.
 
 In the meantime, have you considered how this will interact with 
 userspace buffer pools?  I know you guys aren't using them at this 
 point, but I'm of the opinion that they are an important facility which 
 needs to be preserved.  At worst it may be that some additional flag is 
 needed to control this behaviour.
 
 Secondly I wonder whether this isn't already caught by other aspects of 
 the buffer manager behaviour?
 
 ie, if the buffer to which the relocation points to is being moved, 
 doesn't that imply all hardware activity related to that buffer must 
 have concluded?  IE, if the buffer itself is free to move, surely all 
 commands containing relocations (or chains of relocations) which point 
 to the buffer must themselves have completed??

No, I may be emitting a relocation in a state buffer pointing at a new
(not moved) state buffer, without changing any other state in my buffer.
Think of the SF viewport, which can change without anything in SF state
changing other than the SF VP offset.

You can also imagine once the kernel's entirely in control of ring
operations, it doing accelerated moves of buffers for memory management
purposes, which means that you don't have to sync to move buffers.
(though this may not be a win, as estimates I did for EXA based on
simulation a long time ago didn't look promising).  It would be nice if
we could use MI_WRITE_DATA_IMM (sp?) for relocations in that case to
avoid syncing.  However, you don't get any guarantees about the flushing
of it, so it's unreliable, along with not being an obvious performance
win in tests done in the 2d driver as I recall.

-- 
Eric Anholt [EMAIL PROTECTED]
[EMAIL PROTECTED] [EMAIL PROTECTED]



signature.asc
Description: This is a digitally signed message part
-
SF.Net email is sponsored by: 
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Re: i915: wait for buffer idle before writing relocations

2007-12-07 Thread Keith Whitwell 
Keith,

Thomas has just left for two weeks of (well deserved!) holiday, so he 
may be slow to respond.

In the meantime, have you considered how this will interact with 
userspace buffer pools?  I know you guys aren't using them at this 
point, but I'm of the opinion that they are an important facility which 
needs to be preserved.  At worst it may be that some additional flag is 
needed to control this behaviour.

Secondly I wonder whether this isn't already caught by other aspects of 
the buffer manager behaviour?

ie, if the buffer to which the relocation points to is being moved, 
doesn't that imply all hardware activity related to that buffer must 
have concluded?  IE, if the buffer itself is free to move, surely all 
commands containing relocations (or chains of relocations) which point 
to the buffer must themselves have completed??

Keith



Keith Packard wrote:
 Here's a patch I believe is necessary for the i915 DRM kernel driver;
 right now, the i915 mesa driver never re-uses batch buffers, so there
 can never be an outstanding fence for a buffer with relocations. On 965,
 buffers other than the batch buffer will contain relocations, and may be
 reused (we'll avoid this because of the performance costs).
 
 In any case, this is a correctness fix, as the kernel must not presume
 that user space isn't reusing buffers with relocations.
 
 commit 6f5816b45d62c5b29eb6997885f103c21c92bed1
 Author: Keith Packard [EMAIL PROTECTED]
 Date:   Thu Dec 6 15:12:21 2007 -0800
 
 i915: wait for buffer idle before writing relocations
 
 When writing a relocation entry, make sure the target buffer is idle,
 otherwise the GPU may see inconsistent data.
 
 diff --git a/shared-core/i915_dma.c b/shared-core/i915_dma.c
 index 8791af6..42a2216 100644
 --- a/shared-core/i915_dma.c
 +++ b/shared-core/i915_dma.c
 @@ -756,6 +756,13 @@ int i915_apply_reloc(struct drm_file *file_priv, int 
 num_buffers,
 !drm_bo_same_page(relocatee-offset, new_cmd_offset)) {
 drm_bo_kunmap(relocatee-kmap);
 relocatee-offset = new_cmd_offset;
 +   mutex_lock (relocatee-buf-mutex);
 +   ret = drm_bo_wait (relocatee-buf, 0, 0, FALSE);
 +   mutex_unlock (relocatee-buf-mutex);
 +   if (ret) {
 +   DRM_ERROR(Could not wait for buffer to apply 
 relocs\n %08lx, new_cmd_offset);
 +   return ret;
 +   }
 ret = drm_bo_kmap(relocatee-buf, new_cmd_offset  
 PAGE_SHIFT,
   1, relocatee-kmap);
 if (ret) {
 
 
 
 
 
 -
 SF.Net email is sponsored by: 
 Check out the new SourceForge.net Marketplace.
 It's the best place to buy or sell services for
 just about anything Open Source.
 http://sourceforge.net/services/buy/index.php
 
 
 
 
 --
 ___
 Dri-devel mailing list
 Dri-devel@lists.sourceforge.net
 https://lists.sourceforge.net/lists/listinfo/dri-devel


-
SF.Net email is sponsored by: 
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Re: i915: wait for buffer idle before writing relocations

2007-12-07 Thread Keith Packard 

On Fri, 2007-12-07 at 11:15 +, Keith Whitwell wrote:
 Keith,
 
 Thomas has just left for two weeks of (well deserved!) holiday, so he 
 may be slow to respond.

Thanks for taking the time to have a look while he's away; we're
finishing up the 965 TTM work, and it is posing some challenges with the
existing kernel interface.

 In the meantime, have you considered how this will interact with 
 userspace buffer pools?

No, I hadn't considered that as we're not considering a two-level
allocation strategy at this point.

However, if you consider the blocking patch in conjunction with the
presumed_offset optimization, I think you'll find that userspace buffer
pools will not actually be affected negatively by this change.

The presumed_offset optimization allows the application to compute all
relocations itself for target buffers which have been mapped to the
hardware. The kernel relocations are purely a back-up, for cases where
buffers move between EXECBUFFER invocations.

   I know you guys aren't using them at this 
 point, but I'm of the opinion that they are an important facility which 
 needs to be preserved.  At worst it may be that some additional flag is 
 needed to control this behaviour.

We could do this, but I believe this would actually require more
blocking by the client -- it doesn't know when objects are moving in the
kernel, so it doesn't know when relocation data will need to be
rewritten.

 Secondly I wonder whether this isn't already caught by other aspects of 
 the buffer manager behaviour?

 ie, if the buffer to which the relocation points to is being moved, 
 doesn't that imply all hardware activity related to that buffer must 
 have concluded?  IE, if the buffer itself is free to move, surely all 
 commands containing relocations (or chains of relocations) which point 
 to the buffer must themselves have completed??

Yes, if the target buffer is moving, then the operation related to the
relocatee will have been completed and waited for. But, re-writing
relocations doesn't require that the buffers have moved. 

Consider the case of the binding table on 965 which points at surface
state structures. Executing a command that uses the binding table will
require that relocations be evaluated for the entries in the table; even
if nothing moves (ignoring my presumed_offset optimization), those
relocations will need to be evaluated and the surface state pointers
stored to the binding table.

For the application to guarantee that the binding table relocations can
be written without the kernel needing to wait for the binding table
buffer to be idle, the application would have to wait every time, not
just when the buffer actually moves.

-- 
[EMAIL PROTECTED]


signature.asc
Description: This is a digitally signed message part
-
SF.Net email is sponsored by: 
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

i915: wait for buffer idle before writing relocations

2007-12-06 Thread Keith Packard 
Here's a patch I believe is necessary for the i915 DRM kernel driver;
right now, the i915 mesa driver never re-uses batch buffers, so there
can never be an outstanding fence for a buffer with relocations. On 965,
buffers other than the batch buffer will contain relocations, and may be
reused (we'll avoid this because of the performance costs).

In any case, this is a correctness fix, as the kernel must not presume
that user space isn't reusing buffers with relocations.

commit 6f5816b45d62c5b29eb6997885f103c21c92bed1
Author: Keith Packard [EMAIL PROTECTED]
Date:   Thu Dec 6 15:12:21 2007 -0800

i915: wait for buffer idle before writing relocations

When writing a relocation entry, make sure the target buffer is idle,
otherwise the GPU may see inconsistent data.

diff --git a/shared-core/i915_dma.c b/shared-core/i915_dma.c
index 8791af6..42a2216 100644
--- a/shared-core/i915_dma.c
+++ b/shared-core/i915_dma.c
@@ -756,6 +756,13 @@ int i915_apply_reloc(struct drm_file *file_priv, int 
num_buffers,
!drm_bo_same_page(relocatee-offset, new_cmd_offset)) {
drm_bo_kunmap(relocatee-kmap);
relocatee-offset = new_cmd_offset;
+   mutex_lock (relocatee-buf-mutex);
+   ret = drm_bo_wait (relocatee-buf, 0, 0, FALSE);
+   mutex_unlock (relocatee-buf-mutex);
+   if (ret) {
+   DRM_ERROR(Could not wait for buffer to apply relocs\n 
%08lx, new_cmd_offset);
+   return ret;
+   }
ret = drm_bo_kmap(relocatee-buf, new_cmd_offset  PAGE_SHIFT,
  1, relocatee-kmap);
if (ret) {

-- 
[EMAIL PROTECTED]


signature.asc
Description: This is a digitally signed message part
-
SF.Net email is sponsored by: 
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel