[Mesa-dev] [PATCH v2] st/nine: Implement Managed vertex/index buffers

Patrick Rudolph siro at das-labor.org
Thu Feb 11 10:03:52 UTC 2016


On 2016-02-06 03:12 PM, Axel Davy wrote:
> We were implementing those the same way than
> the default pool, which is sub-optimal.
> 
> The buffer is supposed to return pointer to
> a ram copy when user locks, and automatically
> update the vram copy when needed.
> 
> v2: Rename NineBuffer9_Validate to NineBuffer9_Upload
> Rename validate_buffers to update_managed_buffers
> Initialize NineBuffer9 managed fields after the resource
> is allocated. In case of allocation failure, when the dtor
> is executed, This->base.pool is then rightfully set.
> 
> Signed-off-by: Axel Davy <axel.davy at ens.fr>
> ---
>  src/gallium/state_trackers/nine/buffer9.c    | 77 ++++++++++++++++++++++++----
>  src/gallium/state_trackers/nine/buffer9.h    | 28 ++++++++++
>  src/gallium/state_trackers/nine/device9.c    | 11 ++++
>  src/gallium/state_trackers/nine/device9.h    |  2 +
>  src/gallium/state_trackers/nine/nine_state.c | 12 +++++
>  5 files changed, 121 insertions(+), 9 deletions(-)
> 
> diff --git a/src/gallium/state_trackers/nine/buffer9.c
> b/src/gallium/state_trackers/nine/buffer9.c
> index b4b91ec..322cab7 100644
> --- a/src/gallium/state_trackers/nine/buffer9.c
> +++ b/src/gallium/state_trackers/nine/buffer9.c
> @@ -93,7 +93,26 @@ NineBuffer9_ctor( struct NineBuffer9 *This,
>  
>      hr = NineResource9_ctor(&This->base, pParams, NULL, TRUE,
>                              Type, Pool, Usage);
> -    return hr;
> +
> +    if (FAILED(hr))
> +        return hr;
> +
> +    if (Pool == D3DPOOL_MANAGED) {
> +        This->managed.data = align_malloc(
> +            nine_format_get_level_alloc_size(This->base.info.format,
> +                                             Size, 1, 0), 32);

You want to use Size here, which gives the buffer size in Bytes.
nine_format_get_level_alloc_size with format PIPE_FORMAT_R8_UNORM will
return the same value.
So this call is pointless.

> +        if (!This->managed.data)
> +            return E_OUTOFMEMORY;
> +        memset(This->managed.data, 0, Size);
> +        This->managed.dirty = TRUE;
> +        u_box_1d(0, Size, &This->managed.dirty_box);
> +        list_inithead(&This->managed.list);
> +        list_inithead(&This->managed.list2);
> +        list_add(&This->managed.list, &pParams->device->update_buffers);
> +        list_add(&This->managed.list2, &pParams->device->managed_buffers);
> +    }
> +
> +    return D3D_OK;
>  }
>  
>  void
> @@ -106,6 +125,15 @@ NineBuffer9_dtor( struct NineBuffer9 *This )
>          FREE(This->maps);
>      }
>  
> +    if (This->base.pool == D3DPOOL_MANAGED) {
> +        if (This->managed.data)
> +            align_free(This->managed.data);
> +        if (This->managed.list.prev != NULL && This->managed.list.next != NULL)
> +            list_del(&This->managed.list);
> +        if (This->managed.list2.prev != NULL &&
> This->managed.list2.next != NULL)
> +            list_del(&This->managed.list2);
> +    }
> +
>      NineResource9_dtor(&This->base);
>  }
>  
> @@ -138,6 +166,28 @@ NineBuffer9_Lock( struct NineBuffer9 *This,
>                              D3DLOCK_READONLY |
>                              D3DLOCK_NOOVERWRITE)), D3DERR_INVALIDCALL);
>  
> +    if (SizeToLock == 0) {
> +        SizeToLock = This->size - OffsetToLock;
> +        user_warn(OffsetToLock != 0);
> +    }
> +
> +    u_box_1d(OffsetToLock, SizeToLock, &box);
> +
> +    if (This->base.pool == D3DPOOL_MANAGED) {
> +        if (!This->managed.dirty) {
> +            assert(LIST_IS_EMPTY(&This->managed.list));
> +            list_add(&This->managed.list,
> &This->base.base.device->update_buffers);
> +            This->managed.dirty = TRUE;
> +            This->managed.dirty_box = box;
> +        } else {
> +            u_box_union_2d(&This->managed.dirty_box,
> &This->managed.dirty_box, &box);
> +        }
> +        *ppbData = (char *)This->managed.data + OffsetToLock;
> +        DBG("returning pointer %p\n", *ppbData);
> +        This->nmaps++;
> +        return D3D_OK;
> +    }
> +
>      if (This->nmaps == This->maxmaps) {
>          struct pipe_transfer **newmaps =
>              REALLOC(This->maps, sizeof(struct pipe_transfer *)*This->maxmaps,
> @@ -149,13 +199,6 @@ NineBuffer9_Lock( struct NineBuffer9 *This,
>          This->maps = newmaps;
>      }
>  
> -    if (SizeToLock == 0) {
> -        SizeToLock = This->size - OffsetToLock;
> -        user_warn(OffsetToLock != 0);
> -    }
> -
> -    u_box_1d(OffsetToLock, SizeToLock, &box);
> -
>      data = This->pipe->transfer_map(This->pipe, This->base.resource, 0,
>                                      usage, &box, &This->maps[This->nmaps]);
>  
> @@ -184,6 +227,22 @@ NineBuffer9_Unlock( struct NineBuffer9 *This )
>      DBG("This=%p\n", This);
>  
>      user_assert(This->nmaps > 0, D3DERR_INVALIDCALL);
> -    This->pipe->transfer_unmap(This->pipe, This->maps[--(This->nmaps)]);
> +    if (This->base.pool != D3DPOOL_MANAGED)
> +        This->pipe->transfer_unmap(This->pipe, This->maps[--(This->nmaps)]);
> +    else
> +        This->nmaps--;
>      return D3D_OK;
>  }
> +
> +void
> +NineBuffer9_SetDirty( struct NineBuffer9 *This )
> +{
> +    assert(This->base.pool == D3DPOOL_MANAGED);
> +
> +    if (!This->managed.dirty) {
> +        assert(LIST_IS_EMPTY(&This->managed.list));
> +        list_add(&This->managed.list, &This->base.base.device->update_buffers);
> +        This->managed.dirty = TRUE;
> +    }
> +    u_box_1d(0, This->size, &This->managed.dirty_box);
> +}
> diff --git a/src/gallium/state_trackers/nine/buffer9.h
> b/src/gallium/state_trackers/nine/buffer9.h
> index 1afd9a9..d9237f8 100644
> --- a/src/gallium/state_trackers/nine/buffer9.h
> +++ b/src/gallium/state_trackers/nine/buffer9.h
> @@ -25,6 +25,9 @@
>  #define _NINE_BUFFER9_H_
>  
>  #include "resource9.h"
> +#include "pipe/p_context.h"
> +#include "pipe/p_state.h"
> +#include "util/list.h"
>  
>  struct pipe_screen;
>  struct pipe_context;
> @@ -39,6 +42,15 @@ struct NineBuffer9
>      struct pipe_transfer **maps;
>      int nmaps, maxmaps;
>      UINT size;
> +
> +    /* Specific to managed buffers */
> +    struct {
> +        void *data;
> +        boolean dirty;
> +        struct pipe_box dirty_box;
> +        struct list_head list; /* for update_buffers */
> +        struct list_head list2; /* for managed_buffers */
> +    } managed;
>  };
>  static inline struct NineBuffer9 *
>  NineBuffer9( void *data )
> @@ -70,4 +82,20 @@ NineBuffer9_Lock( struct NineBuffer9 *This,
>  HRESULT WINAPI
>  NineBuffer9_Unlock( struct NineBuffer9 *This );
>  
> +static inline void
> +NineBuffer9_Upload( struct NineBuffer9 *This )
> +{
> +    struct pipe_context *pipe = This->pipe;
> +
> +    assert(This->base.pool == D3DPOOL_MANAGED && This->managed.dirty);
> +    pipe->transfer_inline_write(pipe, This->base.resource, 0, 0,
> +                                &This->managed.dirty_box,
> +                                (char *)This->managed.data +
> This->managed.dirty_box.x,
> +                                This->size, This->size);
> +    This->managed.dirty = FALSE;
> +}
> +
> +void
> +NineBuffer9_SetDirty( struct NineBuffer9 *This );
> +
>  #endif /* _NINE_BUFFER9_H_ */
> diff --git a/src/gallium/state_trackers/nine/device9.c
> b/src/gallium/state_trackers/nine/device9.c
> index 1bca166..1a767e5 100644
> --- a/src/gallium/state_trackers/nine/device9.c
> +++ b/src/gallium/state_trackers/nine/device9.c
> @@ -147,7 +147,9 @@ NineDevice9_ctor( struct NineDevice9 *This,
>  
>      if (FAILED(hr)) { return hr; }
>  
> +    list_inithead(&This->update_buffers);
>      list_inithead(&This->update_textures);
> +    list_inithead(&This->managed_buffers);
>      list_inithead(&This->managed_textures);
>  
>      This->screen = pScreen;
> @@ -563,11 +565,20 @@ HRESULT WINAPI
>  NineDevice9_EvictManagedResources( struct NineDevice9 *This )
>  {
>      struct NineBaseTexture9 *tex;
> +    struct NineBuffer9 *buf;
>  
>      DBG("This=%p\n", This);
>      LIST_FOR_EACH_ENTRY(tex, &This->managed_textures, list2) {
>          NineBaseTexture9_UnLoad(tex);
>      }
> +    /* Vertex/index buffers don't take a lot of space and aren't accounted
> +     * for d3d memory usage. Instead of actually freeing from memory,
> +     * just mark the buffer dirty to trigger a re-upload later. We
> +     * could just ignore, but some bad behaving apps could rely on it (if
> +     * they write outside the locked regions typically). */
> +    LIST_FOR_EACH_ENTRY(buf, &This->managed_buffers, managed.list2) {
> +        NineBuffer9_SetDirty(buf);
> +    }
>  
>      return D3D_OK;
>  }
> diff --git a/src/gallium/state_trackers/nine/device9.h
> b/src/gallium/state_trackers/nine/device9.h
> index 34edf0c..9fbc212 100644
> --- a/src/gallium/state_trackers/nine/device9.h
> +++ b/src/gallium/state_trackers/nine/device9.h
> @@ -68,7 +68,9 @@ struct NineDevice9
>      struct nine_state *update; /* state to update (&state / &record->state) */
>      struct nine_state state;   /* device state */
>  
> +    struct list_head update_buffers;
>      struct list_head update_textures;
> +    struct list_head managed_buffers;
>      struct list_head managed_textures;
>  
>      boolean is_recording;
> diff --git a/src/gallium/state_trackers/nine/nine_state.c
> b/src/gallium/state_trackers/nine/nine_state.c
> index ee9c6c9..9f726ea 100644
> --- a/src/gallium/state_trackers/nine/nine_state.c
> +++ b/src/gallium/state_trackers/nine/nine_state.c
> @@ -23,6 +23,7 @@
>  
>  #include "device9.h"
>  #include "basetexture9.h"
> +#include "buffer9.h"
>  #include "indexbuffer9.h"
>  #include "surface9.h"
>  #include "vertexdeclaration9.h"
> @@ -935,6 +936,16 @@ validate_textures(struct NineDevice9 *device)
>      }
>  }
>  
> +static void
> +update_managed_buffers(struct NineDevice9 *device)
> +{
> +    struct NineBuffer9 *buf, *ptr;
> +    LIST_FOR_EACH_ENTRY_SAFE(buf, ptr, &device->update_buffers, managed.list) {
> +        list_delinit(&buf->managed.list);
> +        NineBuffer9_Upload(buf);
> +    }
> +}
> +
>  void
>  nine_update_state_framebuffer_clear(struct NineDevice9 *device)
>  {
> @@ -962,6 +973,7 @@ nine_update_state(struct NineDevice9 *device)
>       * may be dirty anyway, even if no texture bindings changed.
>       */
>      validate_textures(device); /* may clobber state */
> +    update_managed_buffers(device);
>  
>      /* ff_update may change VS/PS dirty bits */
>      if (unlikely(!state->programmable_vs || !state->ps))


More information about the mesa-dev mailing list