On 11/3/21 12:11 PM, Henri Verbeet wrote:
On Wed, 3 Nov 2021 at 00:37, Zebediah Figura zfigura@codeweavers.com wrote:
+static bool adapter_vk_alloc_bo(struct wined3d_device *device, struct wined3d_resource *resource,
unsigned int sub_resource_idx, struct wined3d_bo_address *addr)+{
- wined3d_not_from_cs(device->cs);
- if (resource->type == WINED3D_RTYPE_BUFFER)
- {
struct wined3d_buffer_vk *buffer_vk = wined3d_buffer_vk(buffer_from_resource(resource));struct wined3d_client_bo_vk *client_bo;if (!(client_bo = heap_alloc(sizeof(*client_bo))))return false;if (!wined3d_buffer_vk_create_buffer_object(buffer_vk, NULL, &client_bo->bo)){heap_free(client_bo);return false;}if (!client_bo->bo.b.map_ptr){struct wined3d_client_bo_vk_map_ctx ctx = {.device = device, .client_bo = client_bo};WARN_(d3d_perf)("BO %p (chunk %p, slab %p) is not persistently mapped.\n", &client_bo->bo,client_bo->bo.memory ? client_bo->bo.memory->chunk : NULL, client_bo->bo.slab);wined3d_cs_map_object(device->cs, wined3d_client_bo_vk_map_cs, &ctx);wined3d_cs_finish(device->cs, WINED3D_CS_QUEUE_MAP);}wined3d_cs_map_object() almost sounds like it would emit a WINED3D_CS_OP_MAP, somewhat like wined3d_device_context_map() back when that was still called wined3d_cs_map()...
Indeed, except that WINED3D_CS_OP_MAP takes the resource. I don't think we want to change that, either, although we could create a new CS op instead of using WINED3D_CS_OP_CALLBACK.
(Now that I think about it, *is* there any sort of a rule as to what deserves WINED3D_CS_OP_CALLBACK and what doesn't? I know that currently it's just a matter of init and destroy, but it seeems a bit arbitrary on reflection.)
Here too, Vulkan could map from application threads, it just needs to be synchronised. In the general case we'd need to make sure the GPU isn't still using the bo, but that's not an issue for newly allocated bo's.
+void wined3d_buffer_rename(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_rename_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);
+}
If we're doing this at the wined3d_buffer level, perhaps it would make sense to name this something like wined3d_buffer_set_bo(), wined3d_buffer_assign_bo(), or something along those lines.
+static void wined3d_buffer_vk_rename_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);
- 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);if (prev_bo != &buffer_vk->bo){struct wined3d_client_bo_vk *client_bo = CONTAINING_RECORD(prev_bo, struct wined3d_client_bo_vk, bo);wined3d_context_vk_destroy_bo(context_vk, &client_bo->bo);heap_free(client_bo);}- }
- else
- {
list_add_head(&bo_vk->b.users, &buffer_vk->b.bo_user.entry);- }
+}
I admit I didn't try very hard, but it's not immediately clear to me why we wouldn't always assign the new bo to "buffer_vk->bo" here, and mark the previous one for destruction.
Right, the basic reason is that the client thread might still be holding a pointer to "bo_vk" in the wined3d_client_resource structure. I'll add a comment to that effect...