[Mesa-dev] [PATCH] r600g: Adjust pipe format when decompressing depth in BE
Oded Gabbay
oded.gabbay at gmail.com
Fri Mar 4 15:56:24 UTC 2016
On Fri, Mar 4, 2016 at 2:19 PM, Marek Olšák <maraeo at gmail.com> wrote:
> Note that the DB only supports tiling and separate depth and stencil, so
> it's unmappable. Before transfers and sometimes even texturing, the buffer
> must be copied via the DB->CB path, because CB supports both interleaved and
> linear layouts. The result is the flushed texture. The goal here is to
> ensure the flushed texture uses the correct format.
>
> Marek
>
Marek,
Thanks for the info, makes the code more clear :)
I can do what you asked, but frankly, I don't think it looks better:
@@ -2657,9 +2657,15 @@ uint32_t r600_translate_colorformat(enum
chip_class chip, enum pipe_format forma
return V_0280A0_COLOR_32_32;
}
} else if (HAS_SIZE(8,24,0,0)) {
- return V_0280A0_COLOR_24_8;
+ if (R600_BIG_ENDIAN)
+ return V_0280A0_COLOR_8_24;
+ else
+ return V_0280A0_COLOR_24_8;
} else if (HAS_SIZE(24,8,0,0)) {
- return V_0280A0_COLOR_8_24;
+ if (R600_BIG_ENDIAN)
+ return V_0280A0_COLOR_24_8;
+ else
+ return V_0280A0_COLOR_8_24;
}
break;
case 3:
@@ -1296,7 +1296,11 @@ unsigned r600_translate_colorswap(enum
pipe_format format)
(HAS_SWIZZLE(0,Y) && HAS_SWIZZLE(1,NONE)) ||
(HAS_SWIZZLE(0,NONE) && HAS_SWIZZLE(1,X))) {
entry = 2;
+#ifdef PIPE_ARCH_LITTLE_ENDIAN
ret = V_0280A0_SWAP_STD_REV; /* YX__ */
+#else
+ ret = V_0280A0_SWAP_STD; /* YX__ */
+#endif
} else if (HAS_SWIZZLE(0,X) && HAS_SWIZZLE(3,Y)) {
entry = 3;
ret = V_0280A0_SWAP_ALT; /* X__Y */
Actually I think it looks worse as we need to intervene in two places
to get the same result I got with just switching the pipe format of
the CB. And I still didn't check what happens with textures.
In any case, this is basically a workaround, because gallium still
thinks it uses the PIPE_FORMAT_Z24_UNORM_S8_UINT format, while we
program the GPU with the PIPE_FORMAT_S8_UINT_Z24_UNORM parameters.
I think that if we use a workaround, better use a cleaner/simpler one.
Oded
> On Mar 4, 2016 12:20 PM, "Oded Gabbay" <oded.gabbay at gmail.com> wrote:
>>
>> On Fri, Mar 4, 2016 at 12:16 PM, Marek Olšák <maraeo at gmail.com> wrote:
>> > I think this is not the right place to do this. It looks like
>> > r600_translate_colorformat and/or r600_colorformat_endian_swap and
>> > r600_translate_texformat should be adjusted instead.
>> >
>> > Marek
>>
>> Adjusted how ?
>> You want me to check specifically for this format there ?
>> I'll take a look if it can be done and I'll get back to you.
>>
>> Oded
>>
>> >
>> > On Thu, Mar 3, 2016 at 4:47 PM, Oded Gabbay <oded.gabbay at gmail.com>
>> > wrote:
>> >> The following is what happens when trying to do glReadPixels() with
>> >> GL_DEPTH_COMPONENT:
>> >>
>> >> 1. When decompressing depth in BE, the GPU performs a swap of the depth
>> >> value when it is written to the memory-mapped buffer.
>> >>
>> >> 2. When the pipe format is PIPE_FORMAT_Z24_UNORM_S8_UINT, the values
>> >> are unpakeded using unpack_uint_z_Z24_UNORM_X8_UINT() into the
>> >> staging
>> >> buffer that is passed back to glReadPixels().
>> >>
>> >> 3. unpack_uint_z_Z24_UNORM_X8_UINT() expects to unpack values that are
>> >> packed according to Z24_UNORM_X8_UINT format.
>> >>
>> >> The combination of the above makes the values that are returned to the
>> >> user erroneous, because the values inside the memory-mapped buffer are,
>> >> in reality, in the packed form of X8_UINT_Z24_UNORM.
>> >>
>> >> llvmpipe/softpipe don't have this problem in big-endian because they
>> >> call util_pack64_z_stencil() when doing the CLEAR operation, which is
>> >> not
>> >> a valid solution when a real GPU is doing that operation.
>> >>
>> >> This patch fix this problem by checking for these specific conditions
>> >> when doing the map memory (r600_texture_transfer_map). In case the
>> >> conditions match, the code will adjust the pipe format of the cbsurf
>> >> before creating the surface, and later, when the unpack function will
>> >> be
>> >> called to copy the values, unpack_uint_z_X8_UINT_Z24_UNORM() will be
>> >> called instead of unpack_uint_z_Z24_UNORM_X8_UINT.
>> >>
>> >> This patch fix gl-1.0-readpixsanity (check_depth and check_stencil).
>> >>
>> >> Signed-off-by: Oded Gabbay <oded.gabbay at gmail.com>
>> >> Cc: "11.1 11.2" <mesa-stable at lists.freedesktop.org>
>> >> ---
>> >> src/gallium/drivers/r600/r600_blit.c | 7 +++++++
>> >> 1 file changed, 7 insertions(+)
>> >>
>> >> diff --git a/src/gallium/drivers/r600/r600_blit.c
>> >> b/src/gallium/drivers/r600/r600_blit.c
>> >> index c52d5a9..5cbc24f 100644
>> >> --- a/src/gallium/drivers/r600/r600_blit.c
>> >> +++ b/src/gallium/drivers/r600/r600_blit.c
>> >> @@ -173,6 +173,13 @@ static void r600_blit_decompress_depth(struct
>> >> pipe_context *ctx,
>> >> zsurf = ctx->create_surface(ctx,
>> >> &texture->resource.b.b, &surf_tmpl);
>> >>
>> >> surf_tmpl.format =
>> >> flushed_depth_texture->resource.b.b.format;
>> >> + /* In BE machine, the GPU returns a
>> >> swapped result of the depth
>> >> + * decompression, that doesn't comply
>> >> with gallium packed formats,
>> >> + * so we need to adjust the format of
>> >> cbsurf */
>> >> + if (R600_BIG_ENDIAN &&
>> >> +
>> >> surf_tmpl.format == PIPE_FORMAT_Z24_UNORM_S8_UINT)
>> >> + surf_tmpl.format =
>> >> PIPE_FORMAT_S8_UINT_Z24_UNORM;
>> >> +
>> >> cbsurf = ctx->create_surface(ctx,
>> >>
>> >> &flushed_depth_texture->resource.b.b, &surf_tmpl);
>> >>
>> >> --
>> >> 2.5.0
>> >>
>> >> _______________________________________________
>> >> mesa-dev mailing list
>> >> mesa-dev at lists.freedesktop.org
>> >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list