[PATCH v3 4/4] wined3d: Try to allocate new Vulkan BOs from the client thread for DISCARD maps.
Henri Verbeet
hverbeet at gmail.com
Fri Nov 5 12:39:52 CDT 2021
On Fri, 5 Nov 2021 at 04:52, Zebediah Figura <zfigura at codeweavers.com> wrote:
> +struct wined3d_client_bo_vk_map_ctx
> +{
> + struct wined3d_device *device;
> + struct wined3d_client_bo_vk *client_bo;
> +};
> +
We don't need this anymore.
> +static bool adapter_vk_alloc_bo(struct wined3d_device *device, struct wined3d_resource *resource,
> + unsigned int sub_resource_idx, struct wined3d_bo_address *addr)
> +{
> + struct wined3d_device_vk *device_vk = wined3d_device_vk(device);
> + struct wined3d_context_vk *context_vk = &device_vk->context_vk;
> +
> + wined3d_not_from_cs(device->cs);
> + assert(device->context_count);
> +
> + if (resource->type == WINED3D_RTYPE_BUFFER)
> + {
> + struct wined3d_bo_vk *bo_vk;
> +
> + if (!(bo_vk = heap_alloc(sizeof(*bo_vk))))
> + return false;
> +
> + if (!(wined3d_context_vk_create_bo(context_vk, resource->size,
> + vk_buffer_usage_from_bind_flags(resource->bind_flags),
> + vk_memory_type_from_access_flags(resource->access, resource->usage), bo_vk)))
> + {
> + WARN("Failed to create Vulkan buffer.\n");
> + return false;
> + }
We leak "bo_vk" here.
> + if (!bo_vk->b.map_ptr)
> + {
> + WARN_(d3d_perf)("BO %p (chunk %p, slab %p) is not persistently mapped.\n",
> + bo_vk, bo_vk->memory ? bo_vk->memory->chunk : NULL, bo_vk->slab);
> +
> + if (!wined3d_bo_vk_map(bo_vk, context_vk))
> + ERR("Failed to map bo.\n");
> + }
> +
I suppose we could clean up and return "false" if mapping the bo fails
here. On the other hand, I suppose it shouldn't fail in the first
place.
> +void wined3d_buffer_set_bo(struct wined3d_buffer *buffer, struct wined3d_context *context, struct wined3d_bo *bo)
> +{
> + TRACE("buffer %p, context %p, bo %p.\n", buffer, context, bo);
> +
> + buffer->buffer_ops->buffer_set_bo(buffer, context, bo);
> + buffer->buffer_object = (uintptr_t)bo;
> + wined3d_buffer_validate_location(buffer, WINED3D_LOCATION_BUFFER);
> + wined3d_buffer_invalidate_location(buffer, ~WINED3D_LOCATION_BUFFER);
> +}
> +
Is wined3d_buffer_set_bo() the proper place for the
validate/invalidate? Arguably it would be more appropriate to do that
in wined3d_cs_exec_update_sub_resource(). I suppose it mirrors
wined3d_buffer_copy_bo_address().
> +static void wined3d_buffer_vk_set_bo(struct wined3d_buffer *buffer,
> + struct wined3d_context *context, struct wined3d_bo *bo)
> +{
> + struct wined3d_bo_vk *prev_bo = (struct wined3d_bo_vk *)buffer->buffer_object;
> + struct wined3d_context_vk *context_vk = wined3d_context_vk(context);
> + struct wined3d_buffer_vk *buffer_vk = wined3d_buffer_vk(buffer);
> + struct wined3d_bo_vk *bo_vk = wined3d_bo_vk(bo);
> +
> + /* We can't just copy the contents of bo_vk into buffer_vk->bo, because the
> + * new BO might still be in use by the client thread. We could allow both to
> + * be valid, although knowing when to destroy a BO then becomes tricky, and
> + * ensuring it's not mapped more than once also becomes tricky. */
> +
There's probably less of a need for the comment now.
> + if (prev_bo)
> + {
> + struct wined3d_bo_user *bo_user;
> +
> + LIST_FOR_EACH_ENTRY(bo_user, &prev_bo->b.users, struct wined3d_bo_user, entry)
> + bo_user->valid = false;
> + assert(list_empty(&bo_vk->b.users));
> + list_move_head(&bo_vk->b.users, &prev_bo->b.users);
> +
> + wined3d_context_vk_destroy_bo(context_vk, prev_bo);
> + heap_free(prev_bo);
> + }
> + else
> + {
> + list_add_head(&bo_vk->b.users, &buffer_vk->b.bo_user.entry);
> + }
> +}
> +
Note that aside from the wined3d_context_vk_destroy_bo() call, this is
all just generic code now.
More information about the wine-devel
mailing list