On 9/29/21 11:22, Henri Verbeet wrote:
On Wed, 29 Sept 2021 at 18:04, Zebediah Figura (she/her) zfigura@codeweavers.com wrote:
On 9/29/21 10:48, Henri Verbeet wrote:
On Tue, 28 Sept 2021 at 23:28, Zebediah Figura zfigura@codeweavers.com wrote:
@@ -2749,6 +2751,7 @@ void wined3d_device_context_emit_update_sub_resource(struct wined3d_device_conte op->box = *box; op->bo.addr.buffer_object = 0; op->bo.addr.addr = data;
- op->bo.flags = UPLOAD_BO_UPLOAD_ON_UNMAP; op->row_pitch = row_pitch; op->slice_pitch = slice_pitch;
Do we need UPLOAD_BO_UPLOAD_ON_UNMAP here? It seems a little out of place, and wined3d_cs_exec_update_sub_resource() doesn't appear to use it.
No, I suppose we don't. Thinking too far ahead...
Although if we obsolesce UPLOAD_BO_UPLOAD_ON_UNMAP entirely, which is possible, that does mean we can't use it for deferred contexts as you suggest below. (And if we don't, it's mildly weird not to pass it here.)
Is it? My understanding was that UPLOAD_ON_UNMAP was primarily for wined3d_device_context_emit_unmap(), to distinguish the case where we got a reference to the original buffer object. (As is the case with NOOVERWRITE maps on deferred contexts, but we could conceivably do that on immediate contexts as well in some cases.)
My plan had been to introduce a couple of other flags here, one such being FLUSH_ON_UNMAP, which would essentially be used for NOOVERWRITE maps of non-coherent BOs on the immediate context. That would need to get passed all the way to the CS thread. Then you end up with something like:
wined3d_cs_exec_update_sub_resource(...) { if (op->bo.flags & UPLOAD_BO_FLUSH_ON_UNMAP) ... else wined3d_buffer_copy_bo_address(...); }
wined3d_device_context_emit_unmap(...) { struct upload_bo bo;
if (context->ops->get_upload_bo(..., &bo)) { if (bo.flags & (UPLOAD_BO_FLUSH_ON_UNMAP | UPLOAD_BO_UPLOAD_ON_UNMAP)) wined3d_device_context_upload_bo(..., &bo); } }
Which is both kind of weird, and also less weird than this patch is.
But it's also not even hard to avoid introducing UPLOAD_BO_UPLOAD_ON_UNMAP until it's really used, so I might as well just restructure my patches a bit accordingly...