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
> [email protected]
> 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
[email protected]
https://lists.sourceforge.net/lists/listinfo/dri-devel

Reply via email to