[PATCH v2 7/7] wined3d: Try to allocate new Vulkan BOs from the client thread for DISCARD maps.
Zebediah Figura
zfigura at codeweavers.com
Thu Nov 4 10:31:14 CDT 2021
On 11/4/21 7:31 AM, Henri Verbeet wrote:
>> +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. */
>> +
> Would it make sense then to either make "buffer_vk->bo" a pointer, or
> get rid of it entirely? Right now, we're handling the initial bo
> different from subsequent ones, and I'm not sure there's a good reason
> to justify that.
It seems reasonable. I guess in the ideal case we're not going to make
much use of buffer_vk->bo anyway.
In that case I guess struct wined3d_client_bo_vk doesn't really have
much reason to exist. I don't believe I had plans to put anything else
in it; I just wanted a way to communicate that the BO belonged to a
separate allocation.
I'll try to order that patch before this one; it seems feasible.
>
> Incidentally, now that we have an actual wined3d_bo structure, it
> would probably make sense to change the type of "buffer_object" in
> e.g. struct wined3d_buffer, struct wined3d_bo_address, etc. from
> uintptr_t (and variants) to a wined3d_bo pointer. That's not a
> prerequisite for this series, but would probably be a nice cleanup.
That one occurred to me as well. (I wasn't sure if there was a reason it
wasn't done like that in the first place. Not that wined3d_bo_vk and
wined3d_bo_gl used to have anything in common, but at least "void *"
would have made more sense...)
>
>> @@ -3121,12 +3124,20 @@ static bool wined3d_cs_map_upload_bo(struct wined3d_device_context *context, str
>> {
>> /* Limit NOOVERWRITE maps to buffers for now; there are too many ways that
>> * a texture can be invalidated to even count. */
>> - if (wined3d_map_persistent() && resource->type == WINED3D_RTYPE_BUFFER && (flags & WINED3D_MAP_NOOVERWRITE))
>> + if (wined3d_map_persistent() && resource->type == WINED3D_RTYPE_BUFFER
>> + && (flags & (WINED3D_MAP_DISCARD | WINED3D_MAP_NOOVERWRITE)))
>> {
>> struct wined3d_client_resource *client = &resource->client;
>> + struct wined3d_device *device = context->device;
>> const struct wined3d_bo *bo;
>> uint8_t *map_ptr;
>>
>> + if (flags & WINED3D_MAP_DISCARD)
>> + {
>> + if (!device->adapter->adapter_ops->adapter_alloc_bo(device, resource, sub_resource_idx, &client->addr))
>> + return NULL;
>> + }
>> +
> Returning NULL here works, but "false" would be more appropriate.
>
Oops, holdover from an old version of this patch...
More information about the wine-devel
mailing list