On Tue, Apr 19, 2016 at 9:10 AM, Michel Dänzer <[email protected]> wrote: > On 19.04.2016 00:03, Ilia Mirkin wrote: >> On Mon, Apr 18, 2016 at 10:47 AM, Oded Gabbay <[email protected]> wrote: >>> On Thu, Apr 14, 2016 at 6:44 PM, Ilia Mirkin <[email protected]> wrote: >>> >>> 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 ? >>> >>> This time, I would like to get an agreement *before* I implement it. >> >> Probably a good idea. And as issues are investigated, people's >> opinions on the "correct" way might shift. Let's think about this... >> >> So clearly *a* correct way to handle this would be to stop all the >> lying. What's the lie? The lie is the PIPE_FORMAT. It talks about e.g. >> R5G6B5 but makes no mention of the byte layout in memory for those 16 >> bits. Really what we have right now is a format and an *implicit* >> endian ordering, which is the CPU's. But what happens when the CPU and >> GPU don't agree? >> >> There's a path we could take which would be to add an endianness >> alongside each format (be it by doubling formats, or an explicit >> second field). This would be a very far-reaching change though, and I >> doubt you'll want to do it. What we're left with is having a format >> and an *implicit* endianness. Which means that the consumers of the >> format need to be able to work out the implicit endianness involved. >> And the endianness will be GPU endian for regular resources, and CPU >> endian for "staging" resources. So it's definitely tempting to stick >> the endian thing into a private field of the resource, like Rob is >> suggesting - when creating a staging texture in >> transfer_map/unmap/flush, set the endianness the cpu endian. Otherwise >> set it to gpu endian. And I think this is somewhat similar to your >> former approach. > > A fundamental issue of the former approach was that it tried to attach > endianness to 8888 array formats, which makes no sense.
I think this was mostly a semantic mistake. I didn't want to attach endianess to 8888 array formats. What I did wanted to do, is to mark them such that the r600g driver won't configure the registers to do any byte swaps, or component swaps. Thus, I marked them as LE, and in the configuration functions, if the mark is LE, I don't configure any swap. Perhaps a better way was to define natural endianess, or alternatively, instead of marking them, just check in r600_translate_colorswap() if the format is an array format, and if it is, don't configure any component swaps. I actually have this covered already in r600_colorformat_endian_swap() for the byte swaps part. Oded The order of > colour components in memory either matches that described by such a > format or it doesn't (in which case there's a bug that needs to be > fixed, not hacked around), endianness doesn't come into play for that. > > Also, what you're describing above isn't really necessary (from a > make-it-work POV, as opposed to a > make-the-format-model-complete-and-consistent POV) for formats such as > R5G6B5, because GPUs supported by the r600 driver can be programmed to > just always access such formats using the CPU byte order. > > > -- > Earthling Michel Dänzer | http://www.amd.com > Libre software enthusiast | Mesa and X developer _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
