[PATCH 8/8] wined3d: Try to allocate new Vulkan BOs from the client thread for DISCARD maps.

[Matteo Bruni]

On Thu, Nov 4, 2021 at 4:45 PM Zebediah Figura <zfigura at codeweavers.com> wrote:
>
> On 11/4/21 8:17 AM, Henri Verbeet wrote:
> > On Wed, 3 Nov 2021 at 21:47, Zebediah Figura <zfigura at codeweavers.com> wrote:
> >> On 11/3/21 12:11 PM, Henri Verbeet wrote:
> >>> On Wed, 3 Nov 2021 at 00:37, Zebediah Figura <zfigura at 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...