On Thu, Nov 4, 2021 at 5:31 PM Zebediah Figura zfigura@codeweavers.com wrote:
On 11/4/21 11:19 AM, Matteo Bruni wrote:
On Thu, Nov 4, 2021 at 4:45 PM Zebediah Figura zfigura@codeweavers.com wrote:
On 11/4/21 8:17 AM, Henri Verbeet wrote:
On Wed, 3 Nov 2021 at 21:47, Zebediah Figura zfigura@codeweavers.com wrote:
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.
Yeah, I'm not suggesting we change WINED3D_CS_OP_MAP. It's somewhat moot for the Vulkan backend now, but I would argue that if we would have to go through the CS to map the bo anyway, we might as well either give up and just map the resource through the CS, or allocate the staging memory on the CPU.
I think the motivation for not just doing that was to make sure that we actually did allocate new memory, instead of repeatedly going through the slow path to use the same mapped memory. I don't remember if this happened in practice, but there's a comment to similar effect on one of Matteo's patches...
That was more or less the idea, although the whole mechanism was a bit different (and with GL posing more constraints). Specifically, we can't / don't want to make GL calls from non-CS threads, but at the same time we want to be able to create new BOs for DISCARD maps (either because we use a separate BO for each wined3d buffer DISCARD map or because we're suballocating and there is no free space but we want to trigger the allocation of a new BO to suballocate from - basically the same as the non-slab case of wined3d_context_vk_create_bo()). So yeah, I don't think you have to care about that case here in the VK callback and it's probably nicer to do what Henri suggested i.e. go explicitly through the CS for a "slow" alloc, since that way the fallback is in generic code.
Assuming I understood the whole thing correctly, I'm not up to speed with this as much as I'd like...
For Vulkan I think it doesn't matter, since we can just map from the client thread. For GL we have to to map from the CS thread, but if we just map the old resource via wined3d_resource_map(), we'll use &wined3d_buffer_gl.bo instead of allocating new memory, which means that the client will continue to have no accessible memory to return for a discard map. Repeat ad infinitum.
Right, I guess the problem is that our "CS api", so to speak, works with wined3d buffers, which match d3d resources. That's really not what we would want in an ideal world.