Re: [Mesa-dev] [PATCH 04/10] radeonsi: move code for setting one shader image into separate function

[Marek Olšák]

v1->v2 diff fixing a crash. Found by Christoph Haag:

diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c
b/src/gallium/drivers/radeonsi/si_descriptors.c
index d264ae7..13a3413 100644
--- a/src/gallium/drivers/radeonsi/si_descriptors.c
+++ b/src/gallium/drivers/radeonsi/si_descriptors.c
@@ -590,8 +590,12 @@ si_set_shader_images(struct pipe_context *pipe,
unsigned shader,

assert(start_slot + count <= SI_NUM_IMAGES);

-   for (i = 0, slot = start_slot; i < count; ++i, ++slot)
-   si_set_shader_image(ctx, images, slot, [i]);
+   for (i = 0, slot = start_slot; i < count; ++i, ++slot) {
+   if (views)
+   si_set_shader_image(ctx, images, slot, [i]);
+   else
+   si_set_shader_image(ctx, images, slot, NULL);
+   }
 }

 static void

Marek


On Thu, May 19, 2016 at 12:59 PM, Marek Olšák  wrote:
> From: Marek Olšák 
>
> ---
>  src/gallium/drivers/radeonsi/si_descriptors.c | 144 
> ++
>  1 file changed, 75 insertions(+), 69 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c 
> b/src/gallium/drivers/radeonsi/si_descriptors.c
> index 48b1e14..d264ae7 100644
> --- a/src/gallium/drivers/radeonsi/si_descriptors.c
> +++ b/src/gallium/drivers/radeonsi/si_descriptors.c
> @@ -501,91 +501,97 @@ si_disable_shader_image(struct si_images_info *images, 
> unsigned slot)
> }
>  }
>
> -static void
> -si_set_shader_images(struct pipe_context *pipe, unsigned shader,
> -unsigned start_slot, unsigned count,
> -struct pipe_image_view *views)
> +static void si_set_shader_image(struct si_context *ctx,
> +   struct si_images_info *images,
> +   unsigned slot, struct pipe_image_view *view)
>  {
> -   struct si_context *ctx = (struct si_context *)pipe;
> struct si_screen *screen = ctx->screen;
> -   struct si_images_info *images = >images[shader];
> -   unsigned i, slot;
> -
> -   assert(shader < SI_NUM_SHADERS);
> +   struct r600_resource *res;
>
> -   if (!count)
> +   if (!view || !view->resource) {
> +   si_disable_shader_image(images, slot);
> return;
> +   }
>
> -   assert(start_slot + count <= SI_NUM_IMAGES);
> +   res = (struct r600_resource *)view->resource;
> +   util_copy_image_view(>views[slot], view);
>
> -   for (i = 0, slot = start_slot; i < count; ++i, ++slot) {
> -   struct r600_resource *res;
> +   si_sampler_view_add_buffer(ctx, >b.b,
> +  RADEON_USAGE_READWRITE);
>
> -   if (!views || !views[i].resource) {
> -   si_disable_shader_image(images, slot);
> -   continue;
> -   }
> +   if (res->b.b.target == PIPE_BUFFER) {
> +   si_make_buffer_descriptor(screen, res,
> + view->format,
> + view->u.buf.first_element,
> + view->u.buf.last_element,
> + images->desc.list + slot * 8);
> +   images->compressed_colortex_mask &= ~(1 << slot);
> +   } else {
> +   static const unsigned char swizzle[4] = { 0, 1, 2, 3 };
> +   struct r600_texture *tex = (struct r600_texture *)res;
> +   unsigned level;
> +   unsigned width, height, depth;
> +   uint32_t *desc = images->desc.list + slot * 8;
>
> -   res = (struct r600_resource *)views[i].resource;
> -   util_copy_image_view(>views[slot], [i]);
> +   assert(!tex->is_depth);
> +   assert(tex->fmask.size == 0);
>
> -   si_sampler_view_add_buffer(ctx, >b.b,
> -  RADEON_USAGE_READWRITE);
> +   if (tex->dcc_offset &&
> +   view->access & PIPE_IMAGE_ACCESS_WRITE)
> +   r600_texture_disable_dcc(>b, tex);
>
> -   if (res->b.b.target == PIPE_BUFFER) {
> -   si_make_buffer_descriptor(screen, res,
> - views[i].format,
> - 
> views[i].u.buf.first_element,
> - views[i].u.buf.last_element,
> - images->desc.list + slot * 
> 8);
> -   images->compressed_colortex_mask &= ~(1 << slot);
> +   if (is_compressed_colortex(tex)) {
> +   images->compressed_colortex_mask |= 1 << slot;
> } else {
> -   static const unsigned char swizzle[4] = { 0, 1, 2, 3 
> };
> -   struct r600_texture *tex = (struct r600_texture 

[Mesa-dev] [PATCH 04/10] radeonsi: move code for setting one shader image into separate function

[Marek Olšák]

From: Marek Olšák 

---
 src/gallium/drivers/radeonsi/si_descriptors.c | 144 ++
 1 file changed, 75 insertions(+), 69 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c 
b/src/gallium/drivers/radeonsi/si_descriptors.c
index 48b1e14..d264ae7 100644
--- a/src/gallium/drivers/radeonsi/si_descriptors.c
+++ b/src/gallium/drivers/radeonsi/si_descriptors.c
@@ -501,91 +501,97 @@ si_disable_shader_image(struct si_images_info *images, 
unsigned slot)
}
 }
 
-static void
-si_set_shader_images(struct pipe_context *pipe, unsigned shader,
-unsigned start_slot, unsigned count,
-struct pipe_image_view *views)
+static void si_set_shader_image(struct si_context *ctx,
+   struct si_images_info *images,
+   unsigned slot, struct pipe_image_view *view)
 {
-   struct si_context *ctx = (struct si_context *)pipe;
struct si_screen *screen = ctx->screen;
-   struct si_images_info *images = >images[shader];
-   unsigned i, slot;
-
-   assert(shader < SI_NUM_SHADERS);
+   struct r600_resource *res;
 
-   if (!count)
+   if (!view || !view->resource) {
+   si_disable_shader_image(images, slot);
return;
+   }
 
-   assert(start_slot + count <= SI_NUM_IMAGES);
+   res = (struct r600_resource *)view->resource;
+   util_copy_image_view(>views[slot], view);
 
-   for (i = 0, slot = start_slot; i < count; ++i, ++slot) {
-   struct r600_resource *res;
+   si_sampler_view_add_buffer(ctx, >b.b,
+  RADEON_USAGE_READWRITE);
 
-   if (!views || !views[i].resource) {
-   si_disable_shader_image(images, slot);
-   continue;
-   }
+   if (res->b.b.target == PIPE_BUFFER) {
+   si_make_buffer_descriptor(screen, res,
+ view->format,
+ view->u.buf.first_element,
+ view->u.buf.last_element,
+ images->desc.list + slot * 8);
+   images->compressed_colortex_mask &= ~(1 << slot);
+   } else {
+   static const unsigned char swizzle[4] = { 0, 1, 2, 3 };
+   struct r600_texture *tex = (struct r600_texture *)res;
+   unsigned level;
+   unsigned width, height, depth;
+   uint32_t *desc = images->desc.list + slot * 8;
 
-   res = (struct r600_resource *)views[i].resource;
-   util_copy_image_view(>views[slot], [i]);
+   assert(!tex->is_depth);
+   assert(tex->fmask.size == 0);
 
-   si_sampler_view_add_buffer(ctx, >b.b,
-  RADEON_USAGE_READWRITE);
+   if (tex->dcc_offset &&
+   view->access & PIPE_IMAGE_ACCESS_WRITE)
+   r600_texture_disable_dcc(>b, tex);
 
-   if (res->b.b.target == PIPE_BUFFER) {
-   si_make_buffer_descriptor(screen, res,
- views[i].format,
- views[i].u.buf.first_element,
- views[i].u.buf.last_element,
- images->desc.list + slot * 8);
-   images->compressed_colortex_mask &= ~(1 << slot);
+   if (is_compressed_colortex(tex)) {
+   images->compressed_colortex_mask |= 1 << slot;
} else {
-   static const unsigned char swizzle[4] = { 0, 1, 2, 3 };
-   struct r600_texture *tex = (struct r600_texture *)res;
-   unsigned level;
-   unsigned width, height, depth;
-   uint32_t *desc = images->desc.list + slot * 8;
+   images->compressed_colortex_mask &= ~(1 << slot);
+   }
+
+   /* Always force the base level to the selected level.
+*
+* This is required for 3D textures, where otherwise
+* selecting a single slice for non-layered bindings
+* fails. It doesn't hurt the other targets.
+*/
+   level = view->u.tex.level;
+   width = u_minify(res->b.b.width0, level);
+   height = u_minify(res->b.b.height0, level);
+   depth = u_minify(res->b.b.depth0, level);
+
+   si_make_texture_descriptor(screen, tex,
+  false, res->b.b.target,
+  view->format, swizzle,
+  0, 0,
+