Re: [Mesa-dev] [PATCH 3/3] radeonsi: fix border color translation for integer textures

[Dieter Nützel]

For the series:

Tested-by: Dieter Nützel 

Dieter

Am 26.09.2017 16:46, schrieb Nicolai Hähnle:

From: Nicolai Hähnle 

This fixes the extremely unlikely case that an application uses
0x8000 or 0x3f80 as border color for an integer texture and
helps in the also, but perhaps slightly less, unlikely case that 1 is
used as a border color.
---
 src/gallium/drivers/radeonsi/si_descriptors.c | 37 


 src/gallium/drivers/radeonsi/si_pipe.h|  2 ++
 src/gallium/drivers/radeonsi/si_state.c   | 50 
+++

 3 files changed, 60 insertions(+), 29 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c
b/src/gallium/drivers/radeonsi/si_descriptors.c
index 27239c25389..db77b1cb982 100644
--- a/src/gallium/drivers/radeonsi/si_descriptors.c
+++ b/src/gallium/drivers/radeonsi/si_descriptors.c
@@ -372,20 +372,34 @@ void si_set_mutable_tex_desc_fields(struct
si_screen *sscreen,
unsigned pitch = base_level_info->nblk_x * block_width;
unsigned index = si_tile_mode_index(tex, base_level, 
is_stencil);

state[3] &= C_008F1C_TILING_INDEX;
state[3] |= S_008F1C_TILING_INDEX(index);
state[4] &= C_008F20_PITCH_GFX6;
state[4] |= S_008F20_PITCH_GFX6(pitch - 1);
}
 }

+static void si_set_sampler_state_desc(struct si_sampler_state *sstate,
+ struct si_sampler_view *sview,
+ struct r600_texture *tex,
+ uint32_t *desc)
+{
+   if (sview && sview->is_integer)
+   memcpy(desc, sstate->integer_val, 4*4);
+   else if (tex && tex->upgraded_depth &&
+(!sview || !sview->is_stencil_sampler))
+   memcpy(desc, sstate->upgraded_depth_val, 4*4);
+   else
+   memcpy(desc, sstate->val, 4*4);
+}
+
 static void si_set_sampler_view_desc(struct si_context *sctx,
 struct si_sampler_view *sview,
 struct si_sampler_state *sstate,
 uint32_t *desc)
 {
struct pipe_sampler_view *view = >base;
struct r600_texture *rtex = (struct r600_texture *)view->texture;
bool is_buffer = rtex->resource.b.b.target == PIPE_BUFFER;

if (unlikely(!is_buffer && sview->dcc_incompatible)) {
@@ -415,27 +429,24 @@ static void si_set_sampler_view_desc(struct
si_context *sctx,
   is_separate_stencil,
   desc);
}

if (!is_buffer && rtex->fmask.size) {
memcpy(desc + 8, sview->fmask_state, 8*4);
} else {
/* Disable FMASK and bind sampler state in [12:15]. */
memcpy(desc + 8, null_texture_descriptor, 4*4);

-   if (sstate) {
-   if (!is_buffer && rtex->upgraded_depth &&
-   !sview->is_stencil_sampler)
-   memcpy(desc + 12, sstate->upgraded_depth_val, 
4*4);
-   else
-   memcpy(desc + 12, sstate->val, 4*4);
-   }
+   if (sstate)
+   si_set_sampler_state_desc(sstate, sview,
+ is_buffer ? NULL : rtex,
+ desc + 12);
}
 }

 static void si_set_sampler_view(struct si_context *sctx,
unsigned shader,
unsigned slot, struct pipe_sampler_view *view,
bool disallow_early_out)
 {
struct si_sampler_views *views = >samplers[shader].views;
struct si_sampler_view *rview = (struct si_sampler_view*)view;
@@ -463,22 +474,22 @@ static void si_set_sampler_view(struct si_context 
*sctx,

si_sampler_view_add_buffer(sctx, view->texture,
   RADEON_USAGE_READ,
   rview->is_stencil_sampler, true);
} else {
pipe_sampler_view_reference(>views[slot], NULL);
memcpy(desc, null_texture_descriptor, 8*4);
/* Only clear the lower dwords of FMASK. */
memcpy(desc + 8, null_texture_descriptor, 4*4);
/* Re-set the sampler state if we are transitioning from FMASK. 
*/
if (views->sampler_states[slot])
-   memcpy(desc + 12,
-  views->sampler_states[slot]->val, 4*4);
+   si_set_sampler_state_desc(views->sampler_states[slot], 
NULL, NULL,
+ desc + 12);

views->enabled_mask &= ~(1u << slot);
}

 	sctx->descriptors_dirty |= 1u << 

Re: [Mesa-dev] [PATCH 3/3] radeonsi: fix border color translation for integer textures

[Marek Olšák]

For the series:

Reviewed-by: Marek Olšák 

Marek

On Tue, Sep 26, 2017 at 4:46 PM, Nicolai Hähnle  wrote:
> From: Nicolai Hähnle 
>
> This fixes the extremely unlikely case that an application uses
> 0x8000 or 0x3f80 as border color for an integer texture and
> helps in the also, but perhaps slightly less, unlikely case that 1 is
> used as a border color.
> ---
>  src/gallium/drivers/radeonsi/si_descriptors.c | 37 
>  src/gallium/drivers/radeonsi/si_pipe.h|  2 ++
>  src/gallium/drivers/radeonsi/si_state.c   | 50 
> +++
>  3 files changed, 60 insertions(+), 29 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c 
> b/src/gallium/drivers/radeonsi/si_descriptors.c
> index 27239c25389..db77b1cb982 100644
> --- a/src/gallium/drivers/radeonsi/si_descriptors.c
> +++ b/src/gallium/drivers/radeonsi/si_descriptors.c
> @@ -372,20 +372,34 @@ void si_set_mutable_tex_desc_fields(struct si_screen 
> *sscreen,
> unsigned pitch = base_level_info->nblk_x * block_width;
> unsigned index = si_tile_mode_index(tex, base_level, 
> is_stencil);
>
> state[3] &= C_008F1C_TILING_INDEX;
> state[3] |= S_008F1C_TILING_INDEX(index);
> state[4] &= C_008F20_PITCH_GFX6;
> state[4] |= S_008F20_PITCH_GFX6(pitch - 1);
> }
>  }
>
> +static void si_set_sampler_state_desc(struct si_sampler_state *sstate,
> + struct si_sampler_view *sview,
> + struct r600_texture *tex,
> + uint32_t *desc)
> +{
> +   if (sview && sview->is_integer)
> +   memcpy(desc, sstate->integer_val, 4*4);
> +   else if (tex && tex->upgraded_depth &&
> +(!sview || !sview->is_stencil_sampler))
> +   memcpy(desc, sstate->upgraded_depth_val, 4*4);
> +   else
> +   memcpy(desc, sstate->val, 4*4);
> +}
> +
>  static void si_set_sampler_view_desc(struct si_context *sctx,
>  struct si_sampler_view *sview,
>  struct si_sampler_state *sstate,
>  uint32_t *desc)
>  {
> struct pipe_sampler_view *view = >base;
> struct r600_texture *rtex = (struct r600_texture *)view->texture;
> bool is_buffer = rtex->resource.b.b.target == PIPE_BUFFER;
>
> if (unlikely(!is_buffer && sview->dcc_incompatible)) {
> @@ -415,27 +429,24 @@ static void si_set_sampler_view_desc(struct si_context 
> *sctx,
>is_separate_stencil,
>desc);
> }
>
> if (!is_buffer && rtex->fmask.size) {
> memcpy(desc + 8, sview->fmask_state, 8*4);
> } else {
> /* Disable FMASK and bind sampler state in [12:15]. */
> memcpy(desc + 8, null_texture_descriptor, 4*4);
>
> -   if (sstate) {
> -   if (!is_buffer && rtex->upgraded_depth &&
> -   !sview->is_stencil_sampler)
> -   memcpy(desc + 12, sstate->upgraded_depth_val, 
> 4*4);
> -   else
> -   memcpy(desc + 12, sstate->val, 4*4);
> -   }
> +   if (sstate)
> +   si_set_sampler_state_desc(sstate, sview,
> + is_buffer ? NULL : rtex,
> + desc + 12);
> }
>  }
>
>  static void si_set_sampler_view(struct si_context *sctx,
> unsigned shader,
> unsigned slot, struct pipe_sampler_view *view,
> bool disallow_early_out)
>  {
> struct si_sampler_views *views = >samplers[shader].views;
> struct si_sampler_view *rview = (struct si_sampler_view*)view;
> @@ -463,22 +474,22 @@ static void si_set_sampler_view(struct si_context *sctx,
> si_sampler_view_add_buffer(sctx, view->texture,
>RADEON_USAGE_READ,
>rview->is_stencil_sampler, true);
> } else {
> pipe_sampler_view_reference(>views[slot], NULL);
> memcpy(desc, null_texture_descriptor, 8*4);
> /* Only clear the lower dwords of FMASK. */
> memcpy(desc + 8, null_texture_descriptor, 4*4);
> /* Re-set the sampler state if we are transitioning from 
> FMASK. */
> if (views->sampler_states[slot])
> -   memcpy(desc + 12,
> -  views->sampler_states[slot]->val, 4*4);
> +   
> 

Re: [Mesa-dev] [PATCH 3/3] radeonsi: fix border color translation for integer textures

[Nicolai Hähnle]

On 26.09.2017 18:55, Gustaw Smolarczyk wrote:

2017-09-26 16:46 GMT+02:00 Nicolai Hähnle :

From: Nicolai Hähnle 

This fixes the extremely unlikely case that an application uses
0x8000 or 0x3f80 as border color for an integer texture and
helps in the also, but perhaps slightly less, unlikely case that 1 is
used as a border color.


Hi,

I see that it fixes the wrong optimization in si_translate_border_color.

However, I also see that for floating point textures this will change
-0.0 into 0.0 (if I understand how
V_008F3C_SQ_TEX_BORDER_COLOR_*_BLACK work). I don't know GL rules
enough to judge that (whether 0.0 and -0.0 should be
indistinguishable), but is that ok?


Good question :)

There is actually a tiny bit of evidence in the spec that GLSL care 
slightly about the sign of zero, though IMO it's a bit silly to assume 
you're getting the correct sign for zero when you're not even guaranteed 
to get denormals. Personally, I wouldn't worry about it.


Cheers,
Nicolai




Regards,
Gustaw Smolarczyk



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/3] radeonsi: fix border color translation for integer textures

[Gustaw Smolarczyk]

2017-09-26 16:46 GMT+02:00 Nicolai Hähnle :
> From: Nicolai Hähnle 
>
> This fixes the extremely unlikely case that an application uses
> 0x8000 or 0x3f80 as border color for an integer texture and
> helps in the also, but perhaps slightly less, unlikely case that 1 is
> used as a border color.

Hi,

I see that it fixes the wrong optimization in si_translate_border_color.

However, I also see that for floating point textures this will change
-0.0 into 0.0 (if I understand how
V_008F3C_SQ_TEX_BORDER_COLOR_*_BLACK work). I don't know GL rules
enough to judge that (whether 0.0 and -0.0 should be
indistinguishable), but is that ok?

Regards,
Gustaw Smolarczyk
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 3/3] radeonsi: fix border color translation for integer textures

[Nicolai Hähnle]

From: Nicolai Hähnle 

This fixes the extremely unlikely case that an application uses
0x8000 or 0x3f80 as border color for an integer texture and
helps in the also, but perhaps slightly less, unlikely case that 1 is
used as a border color.
---
 src/gallium/drivers/radeonsi/si_descriptors.c | 37 
 src/gallium/drivers/radeonsi/si_pipe.h|  2 ++
 src/gallium/drivers/radeonsi/si_state.c   | 50 +++
 3 files changed, 60 insertions(+), 29 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c 
b/src/gallium/drivers/radeonsi/si_descriptors.c
index 27239c25389..db77b1cb982 100644
--- a/src/gallium/drivers/radeonsi/si_descriptors.c
+++ b/src/gallium/drivers/radeonsi/si_descriptors.c
@@ -372,20 +372,34 @@ void si_set_mutable_tex_desc_fields(struct si_screen 
*sscreen,
unsigned pitch = base_level_info->nblk_x * block_width;
unsigned index = si_tile_mode_index(tex, base_level, 
is_stencil);
 
state[3] &= C_008F1C_TILING_INDEX;
state[3] |= S_008F1C_TILING_INDEX(index);
state[4] &= C_008F20_PITCH_GFX6;
state[4] |= S_008F20_PITCH_GFX6(pitch - 1);
}
 }
 
+static void si_set_sampler_state_desc(struct si_sampler_state *sstate,
+ struct si_sampler_view *sview,
+ struct r600_texture *tex,
+ uint32_t *desc)
+{
+   if (sview && sview->is_integer)
+   memcpy(desc, sstate->integer_val, 4*4);
+   else if (tex && tex->upgraded_depth &&
+(!sview || !sview->is_stencil_sampler))
+   memcpy(desc, sstate->upgraded_depth_val, 4*4);
+   else
+   memcpy(desc, sstate->val, 4*4);
+}
+
 static void si_set_sampler_view_desc(struct si_context *sctx,
 struct si_sampler_view *sview,
 struct si_sampler_state *sstate,
 uint32_t *desc)
 {
struct pipe_sampler_view *view = >base;
struct r600_texture *rtex = (struct r600_texture *)view->texture;
bool is_buffer = rtex->resource.b.b.target == PIPE_BUFFER;
 
if (unlikely(!is_buffer && sview->dcc_incompatible)) {
@@ -415,27 +429,24 @@ static void si_set_sampler_view_desc(struct si_context 
*sctx,
   is_separate_stencil,
   desc);
}
 
if (!is_buffer && rtex->fmask.size) {
memcpy(desc + 8, sview->fmask_state, 8*4);
} else {
/* Disable FMASK and bind sampler state in [12:15]. */
memcpy(desc + 8, null_texture_descriptor, 4*4);
 
-   if (sstate) {
-   if (!is_buffer && rtex->upgraded_depth &&
-   !sview->is_stencil_sampler)
-   memcpy(desc + 12, sstate->upgraded_depth_val, 
4*4);
-   else
-   memcpy(desc + 12, sstate->val, 4*4);
-   }
+   if (sstate)
+   si_set_sampler_state_desc(sstate, sview,
+ is_buffer ? NULL : rtex,
+ desc + 12);
}
 }
 
 static void si_set_sampler_view(struct si_context *sctx,
unsigned shader,
unsigned slot, struct pipe_sampler_view *view,
bool disallow_early_out)
 {
struct si_sampler_views *views = >samplers[shader].views;
struct si_sampler_view *rview = (struct si_sampler_view*)view;
@@ -463,22 +474,22 @@ static void si_set_sampler_view(struct si_context *sctx,
si_sampler_view_add_buffer(sctx, view->texture,
   RADEON_USAGE_READ,
   rview->is_stencil_sampler, true);
} else {
pipe_sampler_view_reference(>views[slot], NULL);
memcpy(desc, null_texture_descriptor, 8*4);
/* Only clear the lower dwords of FMASK. */
memcpy(desc + 8, null_texture_descriptor, 4*4);
/* Re-set the sampler state if we are transitioning from FMASK. 
*/
if (views->sampler_states[slot])
-   memcpy(desc + 12,
-  views->sampler_states[slot]->val, 4*4);
+   si_set_sampler_state_desc(views->sampler_states[slot], 
NULL, NULL,
+ desc + 12);
 
views->enabled_mask &= ~(1u << slot);
}
 
sctx->descriptors_dirty |= 1u << 
si_sampler_and_image_descriptors_idx(shader);
 }
 
 static bool color_needs_decompression(struct r600_texture