On Thu, 2017-04-27 at 20:30 -0700, Jason Ekstrand wrote:
> On Wed, Apr 26, 2017 at 9:04 AM, Juan A. Suarez Romero <[email protected]>
> wrote:
> > On Wed, 2017-04-26 at 07:35 -0700, Jason Ekstrand wrote:
> > > ---
> > > src/intel/vulkan/anv_allocator.c | 71
> > >+++++++++++++++++------------------
> > > src/intel/vulkan/anv_cmd_buffer.c | 8 ++--
> > > src/intel/vulkan/anv_descriptor_set.c | 4 +-
> > > src/intel/vulkan/anv_private.h | 21 ++++++-----
> > > 4 files changed, 52 insertions(+), 52 deletions(-)
> > >
> > > diff --git a/src/intel/vulkan/anv_allocator.c
> > > b/src/intel/vulkan/anv_allocator.c
> > > index d93d4c9..1c94d1b 100644
> > > --- a/src/intel/vulkan/anv_allocator.c
> > > +++ b/src/intel/vulkan/anv_allocator.c
> > > @@ -736,14 +736,12 @@ anv_state_pool_free(struct anv_state_pool *pool,
> > > struct anv_state state)
> > > anv_state_pool_free_no_vg(pool, state);
> > > }
> > >
> > > -#define NULL_BLOCK 1
> > > struct anv_state_stream_block {
> > > + struct anv_state block;
> > > +
> > > /* The next block */
> > > struct anv_state_stream_block *next;
> > >
> > > - /* The offset into the block pool at which this block starts */
> > > - uint32_t offset;
> > > -
> > > #ifdef HAVE_VALGRIND
> > > /* A pointer to the first user-allocated thing in this block. This is
> > > * what valgrind sees as the start of the block.
> > > @@ -757,16 +755,18 @@ struct anv_state_stream_block {
> > > */
> > > void
> > > anv_state_stream_init(struct anv_state_stream *stream,
> > > - struct anv_block_pool *block_pool)
> > > + struct anv_state_pool *state_pool,
> > > + uint32_t block_size)
> > > {
> > > - stream->block_pool = block_pool;
> > > - stream->block = NULL;
> > > + stream->state_pool = state_pool;
> > > + stream->block_size = block_size;
> >
>
> There's a bug here where I don't initialize stream->block. It's fixed
> locally.
>
> > > - /* Ensure that next + whatever > end. This way the first call to
> > > + stream->block_list = NULL;
> > > +
> > > + /* Ensure that next + whatever > block_size. This way the first call
> > > to
> > > * state_stream_alloc fetches a new block.
> > > */
> > > - stream->next = 1;
> > > - stream->end = 0;
> > > + stream->next = block_size;
> > >
> > > VG(VALGRIND_CREATE_MEMPOOL(stream, 0, false));
> > > }
> > > @@ -774,14 +774,12 @@ anv_state_stream_init(struct anv_state_stream
> > > *stream,
> > > void
> > > anv_state_stream_finish(struct anv_state_stream *stream)
> > > {
> > > - VG(const uint32_t block_size = stream->block_pool->block_size);
> > > -
> > > - struct anv_state_stream_block *next = stream->block;
> > > + struct anv_state_stream_block *next = stream->block_list;
> > > while (next != NULL) {
> > > struct anv_state_stream_block sb = VG_NOACCESS_READ(next);
> > > VG(VALGRIND_MEMPOOL_FREE(stream, sb._vg_ptr));
> > > - VG(VALGRIND_MAKE_MEM_UNDEFINED(next, block_size));
> > > - anv_block_pool_free(stream->block_pool, sb.offset);
> > > + VG(VALGRIND_MAKE_MEM_UNDEFINED(next, stream->block_size));
> > > + anv_state_pool_free_no_vg(stream->state_pool, sb.block);
> > > next = sb.next;
> > > }
> > >
> > > @@ -795,35 +793,36 @@ anv_state_stream_alloc(struct anv_state_stream
> > > *stream,
> > > if (size == 0)
> > > return ANV_STATE_NULL;
> > >
> > > - struct anv_state_stream_block *sb = stream->block;
> > > + uint32_t offset = align_u32(stream->next, alignment);
> > > + if (offset + size > stream->block_size) {
> > > + stream->block = anv_state_pool_alloc_no_vg(stream->state_pool,
> > > + stream->block_size,
> > > + stream->block_size);
> >
> >
> > Why are we calling anv_state_pool_alloc_no_vg() with block_size twice?
> > Shouldn't we use alignment as the last parameter?
>
> Actually, what we really want here is page alignment. All alignments passed
> into the allocator will be no more than the page size and we can't guarantee
> an alignment larger than that anyway. I've locally changed this to pass
> PAGE_SIZE to state_pool_alloc_no_vg. How's that sound?
>
Sounds good. Thanks.
> --Jason
>
> > >
> > > - struct anv_state state;
> > > -
> > > - state.offset = align_u32(stream->next, alignment);
> > > - if (state.offset + size > stream->end) {
> > > - uint32_t block = anv_block_pool_alloc(stream->block_pool);
> > > - sb = stream->block_pool->map + block;
> > > + struct anv_state_stream_block *sb = stream->block.map;
> > > + VG_NOACCESS_WRITE(&sb->block, stream->block);
> > > + VG_NOACCESS_WRITE(&sb->next, stream->block_list);
> > > + stream->block_list = sb;
> > > + VG_NOACCESS_WRITE(&sb->_vg_ptr, NULL);
> > >
> > > - VG(VALGRIND_MAKE_MEM_UNDEFINED(sb, sizeof(*sb)));
> > > - sb->next = stream->block;
> > > - sb->offset = block;
> > > - VG(sb->_vg_ptr = NULL);
> > > - VG(VALGRIND_MAKE_MEM_NOACCESS(sb, stream->block_pool->block_size));
> > > + VG(VALGRIND_MAKE_MEM_NOACCESS(stream->block.map,
> > > stream->block_size));
> > >
> > > - stream->block = sb;
> > > - stream->start = block;
> > > - stream->next = block + sizeof(*sb);
> > > - stream->end = block + stream->block_pool->block_size;
> > > + /* Reset back to the start plus space for the header */
> > > + stream->next = sizeof(*sb);
> > >
> > > - state.offset = align_u32(stream->next, alignment);
> > > - assert(state.offset + size <= stream->end);
> > > + offset = align_u32(stream->next, alignment);
> > > + assert(offset + size <= stream->block_size);
> > > }
> > >
> > > - assert(state.offset > stream->start);
> > > - state.map = (void *)sb + (state.offset - stream->start);
> > > + struct anv_state state = stream->block;
> > > + state.offset += offset;
> > > state.alloc_size = size;
> > > + state.map += offset;
> > > +
> > > + stream->next = offset + size;
> > >
> > > #ifdef HAVE_VALGRIND
> > > + struct anv_state_stream_block *sb = stream->block_list;
> > > void *vg_ptr = VG_NOACCESS_READ(&sb->_vg_ptr);
> > > if (vg_ptr == NULL) {
> > > vg_ptr = state.map;
> > > @@ -839,8 +838,6 @@ anv_state_stream_alloc(struct anv_state_stream
> > > *stream,
> > > }
> > > #endif
> > >
> > > - stream->next = state.offset + size;
> > > -
> > > return state;
> > > }
> > >
> > > diff --git a/src/intel/vulkan/anv_cmd_buffer.c
> > > b/src/intel/vulkan/anv_cmd_buffer.c
> > > index c65eba2..120b864 100644
> > > --- a/src/intel/vulkan/anv_cmd_buffer.c
> > > +++ b/src/intel/vulkan/anv_cmd_buffer.c
> > > @@ -212,9 +212,9 @@ static VkResult anv_create_cmd_buffer(
> > > goto fail;
> > >
> > > anv_state_stream_init(&cmd_buffer->surface_state_stream,
> > > - &device->surface_state_block_pool);
> > > + &device->surface_state_pool, 4096);
> > > anv_state_stream_init(&cmd_buffer->dynamic_state_stream,
> > > - &device->dynamic_state_block_pool);
> > > + &device->dynamic_state_pool, 16384);
> > >
> > > memset(&cmd_buffer->state.push_descriptor, 0,
> > > sizeof(cmd_buffer->state.push_descriptor));
> > > @@ -306,11 +306,11 @@ anv_cmd_buffer_reset(struct anv_cmd_buffer
> > > *cmd_buffer)
> > >
> > > anv_state_stream_finish(&cmd_buffer->surface_state_stream);
> > > anv_state_stream_init(&cmd_buffer->surface_state_stream,
> > > - &cmd_buffer->device->surface_state_block_pool);
> > > + &cmd_buffer->device->surface_state_pool, 4096);
> > >
> > > anv_state_stream_finish(&cmd_buffer->dynamic_state_stream);
> > > anv_state_stream_init(&cmd_buffer->dynamic_state_stream,
> > > - &cmd_buffer->device->dynamic_state_block_pool);
> > > + &cmd_buffer->device->dynamic_state_pool, 16384);
> > > return VK_SUCCESS;
> > > }
> > >
> > > diff --git a/src/intel/vulkan/anv_descriptor_set.c
> > > b/src/intel/vulkan/anv_descriptor_set.c
> > > index 4797c1e..4b58b0b 100644
> > > --- a/src/intel/vulkan/anv_descriptor_set.c
> > > +++ b/src/intel/vulkan/anv_descriptor_set.c
> > > @@ -345,7 +345,7 @@ VkResult anv_CreateDescriptorPool(
> > > pool->free_list = EMPTY;
> > >
> > > anv_state_stream_init(&pool->surface_state_stream,
> > > - &device->surface_state_block_pool);
> > > + &device->surface_state_pool, 4096);
> > > pool->surface_state_free_list = NULL;
> > >
> > > *pDescriptorPool = anv_descriptor_pool_to_handle(pool);
> > > @@ -380,7 +380,7 @@ VkResult anv_ResetDescriptorPool(
> > > pool->free_list = EMPTY;
> > > anv_state_stream_finish(&pool->surface_state_stream);
> > > anv_state_stream_init(&pool->surface_state_stream,
> > > - &device->surface_state_block_pool);
> > > + &device->surface_state_pool, 4096);
> > > pool->surface_state_free_list = NULL;
> > >
> > > return VK_SUCCESS;
> > > diff --git a/src/intel/vulkan/anv_private.h
> > > b/src/intel/vulkan/anv_private.h
> > > index 6ee8f54..216c6eb 100644
> > > --- a/src/intel/vulkan/anv_private.h
> > > +++ b/src/intel/vulkan/anv_private.h
> > > @@ -511,17 +511,19 @@ struct anv_state_pool {
> > > struct anv_state_stream_block;
> > >
> > > struct anv_state_stream {
> > > - struct anv_block_pool *block_pool;
> > > + struct anv_state_pool *state_pool;
> > > +
> > > + /* The size of blocks to allocate from the state pool */
> > > + uint32_t block_size;
> > >
> > > - /* The current working block */
> > > - struct anv_state_stream_block *block;
> > > + /* Current block we're allocating from */
> > > + struct anv_state block;
> > >
> > > - /* Offset at which the current block starts */
> > > - uint32_t start;
> > > - /* Offset at which to allocate the next state */
> > > + /* Offset into the current block at which to allocate the next state
> > > */
> > > uint32_t next;
> > > - /* Offset at which the current block ends */
> > > - uint32_t end;
> > > +
> > > + /* List of all blocks allocated from this pool */
> > > + struct anv_state_stream_block *block_list;
> > > };
> > >
> > > #define CACHELINE_SIZE 64
> > > @@ -566,7 +568,8 @@ struct anv_state anv_state_pool_alloc(struct
> > > anv_state_pool *pool,
> > > size_t state_size, size_t
> > >alignment);
> > > void anv_state_pool_free(struct anv_state_pool *pool, struct anv_state
> > >state);
> > > void anv_state_stream_init(struct anv_state_stream *stream,
> > > - struct anv_block_pool *block_pool);
> > > + struct anv_state_pool *state_pool,
> > > + uint32_t block_size);
> > > void anv_state_stream_finish(struct anv_state_stream *stream);
> > > struct anv_state anv_state_stream_alloc(struct anv_state_stream *stream,
> > > uint32_t size, uint32_t
> > >alignment);
> >
>
>
_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
