Discussion:
[PATCH] radeonsi/gfx9: use the VI codepath for clamping Z
Add Reply
Marek Olšák
2017-08-10 20:33:42 UTC
Reply
Permalink
Raw Message
From: Marek Olšák <***@amd.com>

This fixes corrupted shadows in Unigine Valley.
The corruption disappeared when I stopped setting IMG_DATA_FORMAT_24_8
for depth.

Cc: 17.2 <mesa-***@lists.freedesktop.org>
---
src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c | 2 +-
src/gallium/drivers/radeonsi/si_state.c | 12 +-----------
2 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c b/src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c
index 42f977d..f8c99ff 100644
--- a/src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c
+++ b/src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c
@@ -1385,21 +1385,21 @@ static void tex_fetch_args(
z = coords[ref_pos];
}

/* TC-compatible HTILE promotes Z16 and Z24 to Z32_FLOAT,
* so the depth comparison value isn't clamped for Z16 and
* Z24 anymore. Do it manually here.
*
* It's unnecessary if the original texture format was
* Z32_FLOAT, but we don't know that here.
*/
- if (ctx->screen->b.chip_class == VI)
+ if (ctx->screen->b.chip_class >= VI)
z = ac_build_clamp(&ctx->ac, z);

address[count++] = z;
}

/* Pack user derivatives */
if (opcode == TGSI_OPCODE_TXD) {
int param, num_src_deriv_channels, num_dst_deriv_channels;

switch (target) {
diff --git a/src/gallium/drivers/radeonsi/si_state.c b/src/gallium/drivers/radeonsi/si_state.c
index 11dee49..2c413a4 100644
--- a/src/gallium/drivers/radeonsi/si_state.c
+++ b/src/gallium/drivers/radeonsi/si_state.c
@@ -3170,28 +3170,27 @@ si_make_texture_descriptor(struct si_screen *screen,
enum pipe_texture_target target,
enum pipe_format pipe_format,
const unsigned char state_swizzle[4],
unsigned first_level, unsigned last_level,
unsigned first_layer, unsigned last_layer,
unsigned width, unsigned height, unsigned depth,
uint32_t *state,
uint32_t *fmask_state)
{
struct pipe_resource *res = &tex->resource.b.b;
- const struct util_format_description *base_desc, *desc;
+ const struct util_format_description *desc;
unsigned char swizzle[4];
int first_non_void;
unsigned num_format, data_format, type;
uint64_t va;

desc = util_format_description(pipe_format);
- base_desc = util_format_description(res->format);

if (desc->colorspace == UTIL_FORMAT_COLORSPACE_ZS) {
const unsigned char swizzle_xxxx[4] = {0, 0, 0, 0};
const unsigned char swizzle_yyyy[4] = {1, 1, 1, 1};
const unsigned char swizzle_wwww[4] = {3, 3, 3, 3};

switch (pipe_format) {
case PIPE_FORMAT_S8_UINT_Z24_UNORM:
case PIPE_FORMAT_X32_S8X24_UINT:
case PIPE_FORMAT_X8Z24_UNORM:
@@ -3278,29 +3277,20 @@ si_make_texture_descriptor(struct si_screen *screen,
num_format = V_008F14_IMG_NUM_FORMAT_USCALED;
}
}
}

data_format = si_translate_texformat(&screen->b.b, pipe_format, desc, first_non_void);
if (data_format == ~0) {
data_format = 0;
}

- /* Enable clamping for UNORM depth formats promoted to Z32F. */
- if (screen->b.chip_class >= GFX9 &&
- util_format_has_depth(desc) &&
- num_format == V_008F14_IMG_NUM_FORMAT_FLOAT &&
- util_get_depth_format_type(base_desc) != UTIL_FORMAT_TYPE_FLOAT) {
- /* NUM_FORMAT=FLOAT and DATA_FORMAT=24_8 means "clamp to [0,1]". */
- data_format = V_008F14_IMG_DATA_FORMAT_24_8;
- }
-
/* S8 with Z32 HTILE needs a special format. */
if (screen->b.chip_class >= GFX9 &&
pipe_format == PIPE_FORMAT_S8_UINT &&
tex->tc_compatible_htile)
data_format = V_008F14_IMG_DATA_FORMAT_S8_32;

if (!sampler &&
(res->target == PIPE_TEXTURE_CUBE ||
res->target == PIPE_TEXTURE_CUBE_ARRAY ||
(screen->b.chip_class <= VI &&
--
2.7.4
Nicolai Hähnle
2017-08-11 15:59:22 UTC
Reply
Permalink
Raw Message
Post by Marek Olšák
This fixes corrupted shadows in Unigine Valley.
The corruption disappeared when I stopped setting IMG_DATA_FORMAT_24_8
for depth.
---
src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c | 2 +-
src/gallium/drivers/radeonsi/si_state.c | 12 +-----------
2 files changed, 2 insertions(+), 12 deletions(-)
diff --git a/src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c b/src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c
index 42f977d..f8c99ff 100644
--- a/src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c
+++ b/src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c
@@ -1385,21 +1385,21 @@ static void tex_fetch_args(
z = coords[ref_pos];
}
/* TC-compatible HTILE promotes Z16 and Z24 to Z32_FLOAT,
* so the depth comparison value isn't clamped for Z16 and
* Z24 anymore. Do it manually here.
*
* It's unnecessary if the original texture format was
* Z32_FLOAT, but we don't know that here.
*/
- if (ctx->screen->b.chip_class == VI)
+ if (ctx->screen->b.chip_class >= VI)
z = ac_build_clamp(&ctx->ac, z);
address[count++] = z;
}
/* Pack user derivatives */
if (opcode == TGSI_OPCODE_TXD) {
int param, num_src_deriv_channels, num_dst_deriv_channels;
switch (target) {
diff --git a/src/gallium/drivers/radeonsi/si_state.c b/src/gallium/drivers/radeonsi/si_state.c
index 11dee49..2c413a4 100644
--- a/src/gallium/drivers/radeonsi/si_state.c
+++ b/src/gallium/drivers/radeonsi/si_state.c
@@ -3170,28 +3170,27 @@ si_make_texture_descriptor(struct si_screen *screen,
enum pipe_texture_target target,
enum pipe_format pipe_format,
const unsigned char state_swizzle[4],
unsigned first_level, unsigned last_level,
unsigned first_layer, unsigned last_layer,
unsigned width, unsigned height, unsigned depth,
uint32_t *state,
uint32_t *fmask_state)
{
struct pipe_resource *res = &tex->resource.b.b;
- const struct util_format_description *base_desc, *desc;
+ const struct util_format_description *desc;
unsigned char swizzle[4];
int first_non_void;
unsigned num_format, data_format, type;
uint64_t va;
desc = util_format_description(pipe_format);
- base_desc = util_format_description(res->format);
if (desc->colorspace == UTIL_FORMAT_COLORSPACE_ZS) {
const unsigned char swizzle_xxxx[4] = {0, 0, 0, 0};
const unsigned char swizzle_yyyy[4] = {1, 1, 1, 1};
const unsigned char swizzle_wwww[4] = {3, 3, 3, 3};
switch (pipe_format) {
@@ -3278,29 +3277,20 @@ si_make_texture_descriptor(struct si_screen *screen,
num_format = V_008F14_IMG_NUM_FORMAT_USCALED;
}
}
}
data_format = si_translate_texformat(&screen->b.b, pipe_format, desc, first_non_void);
if (data_format == ~0) {
data_format = 0;
}
- /* Enable clamping for UNORM depth formats promoted to Z32F. */
- if (screen->b.chip_class >= GFX9 &&
- util_format_has_depth(desc) &&
- num_format == V_008F14_IMG_NUM_FORMAT_FLOAT &&
- util_get_depth_format_type(base_desc) != UTIL_FORMAT_TYPE_FLOAT) {
- /* NUM_FORMAT=FLOAT and DATA_FORMAT=24_8 means "clamp to [0,1]". */
- data_format = V_008F14_IMG_DATA_FORMAT_24_8;
- }
-
/* S8 with Z32 HTILE needs a special format. */
if (screen->b.chip_class >= GFX9 &&
pipe_format == PIPE_FORMAT_S8_UINT &&
tex->tc_compatible_htile)
data_format = V_008F14_IMG_DATA_FORMAT_S8_32;
if (!sampler &&
(res->target == PIPE_TEXTURE_CUBE ||
res->target == PIPE_TEXTURE_CUBE_ARRAY ||
(screen->b.chip_class <= VI &&
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
Andres Gomez
2017-08-18 09:01:42 UTC
Reply
Permalink
Raw Message
Hi Marek,

This patch landed tagged for 17.2 stable and has been collected for
17.2.0-rc4.

However, it seems like it could be also interesting for 17.1.x (?)

WDYT?

Br.
Post by Marek Olšák
This fixes corrupted shadows in Unigine Valley.
The corruption disappeared when I stopped setting IMG_DATA_FORMAT_24_8
for depth.
---
src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c | 2 +-
src/gallium/drivers/radeonsi/si_state.c | 12 +-----------
2 files changed, 2 insertions(+), 12 deletions(-)
diff --git a/src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c b/src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c
index 42f977d..f8c99ff 100644
--- a/src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c
+++ b/src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c
@@ -1385,21 +1385,21 @@ static void tex_fetch_args(
z = coords[ref_pos];
}
/* TC-compatible HTILE promotes Z16 and Z24 to Z32_FLOAT,
* so the depth comparison value isn't clamped for Z16 and
* Z24 anymore. Do it manually here.
*
* It's unnecessary if the original texture format was
* Z32_FLOAT, but we don't know that here.
*/
- if (ctx->screen->b.chip_class == VI)
+ if (ctx->screen->b.chip_class >= VI)
z = ac_build_clamp(&ctx->ac, z);
address[count++] = z;
}
/* Pack user derivatives */
if (opcode == TGSI_OPCODE_TXD) {
int param, num_src_deriv_channels, num_dst_deriv_channels;
switch (target) {
diff --git a/src/gallium/drivers/radeonsi/si_state.c b/src/gallium/drivers/radeonsi/si_state.c
index 11dee49..2c413a4 100644
--- a/src/gallium/drivers/radeonsi/si_state.c
+++ b/src/gallium/drivers/radeonsi/si_state.c
@@ -3170,28 +3170,27 @@ si_make_texture_descriptor(struct si_screen *screen,
enum pipe_texture_target target,
enum pipe_format pipe_format,
const unsigned char state_swizzle[4],
unsigned first_level, unsigned last_level,
unsigned first_layer, unsigned last_layer,
unsigned width, unsigned height, unsigned depth,
uint32_t *state,
uint32_t *fmask_state)
{
struct pipe_resource *res = &tex->resource.b.b;
- const struct util_format_description *base_desc, *desc;
+ const struct util_format_description *desc;
unsigned char swizzle[4];
int first_non_void;
unsigned num_format, data_format, type;
uint64_t va;
desc = util_format_description(pipe_format);
- base_desc = util_format_description(res->format);
if (desc->colorspace == UTIL_FORMAT_COLORSPACE_ZS) {
const unsigned char swizzle_xxxx[4] = {0, 0, 0, 0};
const unsigned char swizzle_yyyy[4] = {1, 1, 1, 1};
const unsigned char swizzle_wwww[4] = {3, 3, 3, 3};
switch (pipe_format) {
@@ -3278,29 +3277,20 @@ si_make_texture_descriptor(struct si_screen *screen,
num_format = V_008F14_IMG_NUM_FORMAT_USCALED;
}
}
}
data_format = si_translate_texformat(&screen->b.b, pipe_format, desc, first_non_void);
if (data_format == ~0) {
data_format = 0;
}
- /* Enable clamping for UNORM depth formats promoted to Z32F. */
- if (screen->b.chip_class >= GFX9 &&
- util_format_has_depth(desc) &&
- num_format == V_008F14_IMG_NUM_FORMAT_FLOAT &&
- util_get_depth_format_type(base_desc) != UTIL_FORMAT_TYPE_FLOAT) {
- /* NUM_FORMAT=FLOAT and DATA_FORMAT=24_8 means "clamp to [0,1]". */
- data_format = V_008F14_IMG_DATA_FORMAT_24_8;
- }
-
/* S8 with Z32 HTILE needs a special format. */
if (screen->b.chip_class >= GFX9 &&
pipe_format == PIPE_FORMAT_S8_UINT &&
tex->tc_compatible_htile)
data_format = V_008F14_IMG_DATA_FORMAT_S8_32;
if (!sampler &&
(res->target == PIPE_TEXTURE_CUBE ||
res->target == PIPE_TEXTURE_CUBE_ARRAY ||
(screen->b.chip_class <= VI &&
--
Br,

Andres
Marek Olšák
2017-08-18 14:00:17 UTC
Reply
Permalink
Raw Message
Post by Andres Gomez
Hi Marek,
This patch landed tagged for 17.2 stable and has been collected for
17.2.0-rc4.
However, it seems like it could be also interesting for 17.1.x (?)
WDYT?
It was only nominated for 17.2. Let's keep it that way.

Thanks,
Marek

Loading...