On Mon, Apr 18, 2016 at 10:47 AM, Oded Gabbay <oded.gab...@gmail.com> wrote: > On Thu, Apr 14, 2016 at 6:44 PM, Ilia Mirkin <imir...@alum.mit.edu> wrote: >> On Thu, Apr 14, 2016 at 11:08 AM, Oded Gabbay <oded.gab...@gmail.com> wrote: >>>> Wouldn't it make more sense to handle such issues in transfer_map? >>>> (i.e. create a staging memory area, and decode into it)? This assumes >>>> that the transfer_map() call has enough information to "do the right >>>> thing". I don't think it does today, but perhaps it could be taught? >>> It doesn't have all the info today, that's for sure. I imagine though >>> we can add parameters to it. >>> >>>> That way everything that's in a pipe_resource is in some >>>> tightly-controlled format, and we specify the LE <-> BE parameters >>>> when converting between CPU-read/written and GPU-read/written data. I >>>> believe this is a better match for what's really happening, too. What >>>> do you think? >>>> >>>> -ilia >>> >>> Unless I'm missing something, I think, at the end of the day, it will >>> be the same issues as in my solution - per code path per format is a >>> different case. That's because you will still need to "teach" >>> transfer_map, per each transfer per format what to do. So one will >>> need to go and debug every single code path there is in mesa for >>> drawing/copying/reading/textures/etc., like what I did in the last 1.5 >>> months. It's a great learning experience but it won't give anything >>> generic. >>> >>> Again, for example, in st_ReadPixels, I imagine you will need to give >>> "different orders" to transfer_map for the two different scenarios - >>> H/W blit and fallback. So what's the gain here ? >>> >>> If I'm missing something, please tell me. >> >> One of us is... let's figure out which one :) >> >> Here's my proposal: >> >> All data stored inside of resources is stored in a driver-happy >> format. The driver ensures that it's stored in proper endianness, etc. >> (Much like it does today wrt proper stride.) >> >> Blitting(/copying) between resources doesn't require any additional >> information, since you have the format(s) of the respective resources, >> and it's all inside the driver, so the driver does whatever it needs >> to do to make it all "work". >> >> *Accessing and modifying* resources (directly) from the CPU is what >> becomes tricky. The state tracker may have incorrect expectations of >> the actual backing data. There are a few different ways to resolve >> this. The one I'm proposing is that you only ever return a pointer to >> the directly underlying data if it matches the CPU's expectations >> (which will only be the case for byte-oriented array formats like >> PIPE_FORMAT_R8G8B8A8_* & co). Everything else, like e.g. >> PIPE_FORMAT_R5G6B5_UNORM and countless others, will have to go through >> a bounce buffer. >> >> At transfer map time, you convert the data from GPU-style to >> CPU-style, and copy back the relevant bits at unmap/flush time. >> >> This presents a nice clean boundary for this stuff. Instead of the >> state tracker trying to guess what the driver will do and feeding it >> endiannesses that it can't possibly guess properly, the tracking logic >> is relegated to the driver, and we extend the interfaces to allow the >> state tracker to access the data in a proper way. >> >> I believe the advantage of this scheme is that beyond adding format >> parameters to pipe_transfer_map() calls, there will not need to be any >> adjustments to the state trackers. >> >> One yet-to-be-resolved issue is what to do about glMapBuffer* - it >> maps a buffer, it's formatless (at map time), and yet the GPU will be >> required to interpret it correctly. We could decree that PIPE_BUFFER >> is just *always* an array of R8_UNORM and thus never needs any type of >> swapping. The driver needs to adjust accordingly to deal with accesses >> that don't fit that pattern (and where parameters can't be fed to the >> GPU to interpret it properly). >> >> I think something like the above will work. And I think it presents a >> cleaner barrier than your proposal, because none of the "this GPU can >> kinda-sorta understand BE, but not everywhere" details are ever >> exposed to the state tracker. >> >> Thoughts? >> >> -ilia > > Ilia, > > To make the GPU do a conversion during blitting, I need to configure > registers. This is done in a couple of functions in the r600g driver > (r600_translate_texformat, r600_colorformat_endian_swap, > r600_translate_colorformat and r600_translate_colorswap). > > The problem is that transfer_map/unmap don't call directly to those > functions. They call other functions which eventually call those 4 > functions. Among those "other" functions, there are several function > calls which are *not* in the r600g driver. i.e. we go back to generic > util functions. For example: > > #0 r600_translate_colorformat > #1 evergreen_init_color_surface > #2 evergreen_set_framebuffer_state > #3 util_blitter_custom_depth_stencil > #4 r600_blit_decompress_depth > #5 r600_texture_transfer_map > > Am I allowed to now pass information from transfer_map/unmap all the > way down to the 4 functions I mentioned through all these layers as > additional parameters ? I preferred to put it in pipe_resource as that > information goes all the way down to those functions, but if I can't > use that, then what's an acceptable alternative ?
you are allowed to put whatever you want in radeon's subclass of pipe_resource ;-) But that said, unless depth/stencil issue is what you are looking at right now, it might be easier to start w/ some color formats that don't need decompression. And for z/s formats that do need decompression, maybe just doing two blits (normal decompression and separate endianess fixup) would be easier to start with? Not sure, I haven't looked that closely at the radeon transfer_map/unmap() code.. BR, -R > This time, I would like to get an agreement *before* I implement it. > > Thanks, > > Oded > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev