From: Henri Verbeet Subject: Re: [PATCH 1/2] wined3d: Implement wined3d_deferred_context_update_sub_resource(). Message-Id: Date: Fri, 11 Jun 2021 19:36:54 +0200 In-Reply-To: References: <20210528052127.1589918-1-z.figura12@gmail.com> <7d0b188b-803a-0fc9-ea73-7803011fb9d5@codeweavers.com> On Fri, 11 Jun 2021 at 05:39, Zebediah Figura (she/her) wrote: > On 5/31/21 6:43 PM, Henri Verbeet wrote: > > Conceptually, sure, you could split this into allocate/map/unmap. In > > practice though, you'll always want the upload buffer to be mapped, in > > order to copy data into it from system memory. And in fact, if we're > > going to return a GPU buffer here, it should generally already be > > mapped. (For various reasons; for 64-bit applications, we use > > persistent maps if we can; for OpenGL, we want to avoid making GL > > calls from application threads; if we have a dedicated upload buffer > > belonging to the device context, it would have been mapped during > > creation.) When needed, unmap can happen when the upload buffer is > > consumed. > > Thanks for explaining the rationale there, that does help a lot. > > It still feels awkward to me to pass the same information maybe twice, > but I guess the wined3d_bo_address part should basically be considered > opaque above the CS thread. > In a sense, it's opaque outside the adapter specific code. I.e., resolving the "buffer_object" part of the address is specific to a particular adapter implementation. You could think of those as address spaces, if you like. Of course "buffer_object" 0 is special because it's CPU memory, but there isn't necessarily any relation between the "addr" part and what would get returned in "map_ptr". > > Non-discard maps still exist, and > > wined3d_device_context_get_upload_bo() wouldn't be used for those. > > > > So the idea goes like this: > > (1) Pass a whole wined3d_bo_address as part of struct > wined3d_cs_update_sub_resource. > > (2) Introduce wined3d_device_context_get_upload_bo() to allocate memory. > At first, only use it for deferred contexts. > > (3) Use wined3d_device_context_get_upload_bo() for immediate contexts as > well, for both DISCARD and NOOVERWRITE maps and for update_sub_resource. > This implies always allocating new CPU/GPU memory, going through the > DEFAULT queue, and not waiting for the upload to complete. > As Matteo said, more "available" than "new", but essentially yes. > The idea that map/unmap/update_sub_resource can go away seems to imply > (3) to me, but I'm more than a little curious as to whether it's > actually desirable in all cases. Even with persistent mapping you still > need to allocate new memory, which isn't exactly cheap, especially with > large resources. > Well, we can make wined3d_device_context_get_upload_bo() fail when desired, and the caller would then need to handle that, probably by using the existing path. > And then there's the aforementioned non-discard maps—don't we still need > an explicit WINED3D_CS_OP_UNMAP for those? Or any case (are there any?) > where we don't want to persistently map a resource? Yes, we're much more conservative about creating persistent mappings on 32-bit, for example.