Sorry for the really long delay in replying! This patch is still needed in order to fix a number of Piglit tests so it would be good to get it landed.
Ben Widawsky <b...@bwidawsk.net> writes: > Sorry for the delay, but I put this off initially because I wasn't > sure which part of the docs this was addressing. I see that the > Surface Horizontal Alignment field is now used for compressed formats. > Assuming that's what this is referring to (if not, please correct > me)... Yes, that's right. > The only thing that appears to be missing is the handling of the MCS > case (must always be 16) which may or may not be relevant. AFAICT, > it's a possible scenario. The MCS buffer is only used when rendering right? I don't think it is possible or would make sense to render to a compressed buffer. There is already a section at the bottom of the function to handle the alignment for uncompressed buffers when there is an MCS buffer. If it were possible to use a compressed buffer with an MCS buffer then it would be broken anyway even without this patch because it would just return the block size which wouldn't be 16. > Also, doesn't this make FXT1 have an alignment of 32? Yes, but bear in mind that a row of 32 pixels only takes up 4 blocks so that is only 64 bytes. I guess that is still quite a lot but if that's what the hardware requires then that's what we have to do. >> if (_mesa_is_format_compressed(format)) >> - return 4; >> + /* See comment above for the horizontal alignment */ >> + return brw->gen >= 9 ? 16 : 4; > > This seems okay since the max height we support is 4. What do you mean by the max height? I think previously this was just hardcoding the block height to 4 so I am just bumping the alignment to 16 to represent 4 blocks. > These hunks look okay to me. Any particular reason not to update > brw_miptree_layout_texture_array as well? I am pretty certain we don't > use ALL_SLICES_AT_EACH_LOD in Gen9 today, but presumably we could, > right? /me shrugs We actually can't use ALL_SLICES_AT_EACH_LOD on Gen9 (see the earlier patches) but yes, brw_miptree_layout_texture_array needs to work for 2D array textures. However I don't think anything needs changing there. The only changes I made to brw_miptree_layout_2d were because it was assuming that mt->align_w/h is always equal to the compressed block size but with this patch that is no longer the case. I only changed the places that wanted the block size so that they would use a separate variable instead of conflating it with the alignment. I don't think anything in brw_miptree_layout_texture_array needs to refer to the block size so it should be ok. - Neil _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
