thr3ads.net - Nouveau - [Nouveau] [PATCH] nv50: make sure to clear _all

If this information is useful, please help other people find it:
Share via:

Ilia Mirkin

2014-Feb-13 10:13 UTC

[Nouveau] [PATCH] nv50: make sure to clear _all_ layers of all attachments

Unfortunately there's only one RT_ARRAY_MODE setting for all
attachments, so clears were previously truncated to the minimum number
of layers any attachment had. Instead set the RT_ARRAY_MODE to 512 (the
max number of layers) before doing the clear. This fixes
gl-3.2-layered-rendering-clear-color-mismatched-layer-count.

Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
---

Haven't had a chance to do a full piglit run on this yet, but it does fix
the
failing test. Have a look. I'm not sure if zeta can have layers, it seems
like
a couple of things assumed it couldn't. I've changed that assumption
around.

 src/gallium/drivers/nouveau/nv50/nv50_context.h       |  2 ++
 .../drivers/nouveau/nv50/nv50_state_validate.c        |  1 +
 src/gallium/drivers/nouveau/nv50/nv50_surface.c       | 19 +++++++++++++++++--
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_context.h
b/src/gallium/drivers/nouveau/nv50/nv50_context.h
index 57a3090..84ff46e 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_context.h
+++ b/src/gallium/drivers/nouveau/nv50/nv50_context.h
@@ -173,6 +173,8 @@ struct nv50_context {
 
    boolean vbo_push_hint;
 
+   uint32_t rt_array_mode;
+
    struct pipe_query *cond_query;
    boolean cond_cond;
    uint cond_mode;
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
b/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
index f953422..100d02d 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
@@ -65,6 +65,7 @@ nv50_validate_fb(struct nv50_context *nv50)
          PUSH_DATA (push, sf->height);
          BEGIN_NV04(push, NV50_3D(RT_ARRAY_MODE), 1);
          PUSH_DATA (push, array_mode | array_size);
+         nv50->rt_array_mode = array_mode | array_size;
       } else {
          PUSH_DATA (push, 0);
          PUSH_DATA (push, 0);
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_surface.c
b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
index a22436b..68924c9 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_surface.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
@@ -303,7 +303,7 @@ nv50_clear_render_target(struct pipe_context *pipe,
       PUSH_DATA(push, NV50_3D_RT_HORIZ_LINEAR | mt->level[0].pitch);
    PUSH_DATA (push, sf->height);
    BEGIN_NV04(push, NV50_3D(RT_ARRAY_MODE), 1);
-   PUSH_DATA (push, 1);
+   PUSH_DATA (push, 512);
 
    if (!nouveau_bo_memtype(bo)) {
       BEGIN_NV04(push, NV50_3D(ZETA_ENABLE), 1);
@@ -366,7 +366,7 @@ nv50_clear_depth_stencil(struct pipe_context *pipe,
    PUSH_DATA (push, bo->offset + sf->offset);
    PUSH_DATA (push, nv50_format_table[dst->format].rt);
    PUSH_DATA (push, mt->level[sf->base.u.tex.level].tile_mode);
-   PUSH_DATA (push, 0);
+   PUSH_DATA (push, mt->layer_stride >> 2);
    BEGIN_NV04(push, NV50_3D(ZETA_ENABLE), 1);
    PUSH_DATA (push, 1);
    BEGIN_NV04(push, NV50_3D(ZETA_HORIZ), 3);
@@ -374,6 +374,9 @@ nv50_clear_depth_stencil(struct pipe_context *pipe,
    PUSH_DATA (push, sf->height);
    PUSH_DATA (push, (1 << 16) | 1);
 
+   BEGIN_NV04(push, NV50_3D(RT_ARRAY_MODE), 1);
+   PUSH_DATA (push, 512);
+
    BEGIN_NV04(push, NV50_3D(VIEWPORT_HORIZ(0)), 2);
    PUSH_DATA (push, (width << 16) | dstx);
    PUSH_DATA (push, (height << 16) | dsty);
@@ -402,6 +405,14 @@ nv50_clear(struct pipe_context *pipe, unsigned buffers,
    if (!nv50_state_validate(nv50, NV50_NEW_FRAMEBUFFER, 9 + (fb->nr_cbufs *
2)))
       return;
 
+   /* We have to clear ALL of the layers, not up to the min number of layers
+    * of any attachment. Don't touch 3d textures, they can't be arrays.
The
+    * above nv50_state_validate call will have written to rt_array_mode. */
+   if (!(nv50->rt_array_mode & NV50_3D_RT_ARRAY_MODE_MODE_3D)) {
+      BEGIN_NV04(push, NV50_3D(RT_ARRAY_MODE), 1);
+      PUSH_DATA (push, 512);
+   }
+
    if (buffers & PIPE_CLEAR_COLOR && fb->nr_cbufs) {
       BEGIN_NV04(push, NV50_3D(CLEAR_COLOR(0)), 4);
       PUSH_DATAf(push, color->f[0]);
@@ -459,6 +470,10 @@ nv50_clear(struct pipe_context *pipe, unsigned buffers,
                     (j << NV50_3D_CLEAR_BUFFERS_LAYER__SHIFT));
       }
    }
+
+   /* restore the array mode */
+   BEGIN_NV04(push, NV50_3D(RT_ARRAY_MODE), 1);
+   PUSH_DATA (push, nv50->rt_array_mode);
 }
 
 
-- 
1.8.3.2

Roland Scheidegger

2014-Feb-13 16:38 UTC

head link

[Nouveau] [Mesa-dev] [PATCH] nv50: make sure to clear _all_ layers of all attachments

Hmm that reminds me when I implemented layered rendering in llvmpipe I
figured we'd only need to store the min amount of layers, because
rendering is undefined if the index is higher anyway. But of course I
didn't think about clears so we can't do that now correctly. Ah well if
anyone would really care it is fixable...
In any case the idea behind the patch looks good to me.

Roland


Am 13.02.2014 11:13, schrieb Ilia Mirkin:> Unfortunately there's only one RT_ARRAY_MODE setting for all
> attachments, so clears were previously truncated to the minimum number
> of layers any attachment had. Instead set the RT_ARRAY_MODE to 512 (the
> max number of layers) before doing the clear. This fixes
> gl-3.2-layered-rendering-clear-color-mismatched-layer-count.
> 
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
> 
> Haven't had a chance to do a full piglit run on this yet, but it does
fix the
> failing test. Have a look. I'm not sure if zeta can have layers, it
seems like
> a couple of things assumed it couldn't. I've changed that
assumption around.
> 
>  src/gallium/drivers/nouveau/nv50/nv50_context.h       |  2 ++
>  .../drivers/nouveau/nv50/nv50_state_validate.c        |  1 +
>  src/gallium/drivers/nouveau/nv50/nv50_surface.c       | 19
+++++++++++++++++--
>  3 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_context.h
b/src/gallium/drivers/nouveau/nv50/nv50_context.h
> index 57a3090..84ff46e 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_context.h
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_context.h
> @@ -173,6 +173,8 @@ struct nv50_context {
>  
>     boolean vbo_push_hint;
>  
> +   uint32_t rt_array_mode;
> +
>     struct pipe_query *cond_query;
>     boolean cond_cond;
>     uint cond_mode;
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
b/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
> index f953422..100d02d 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
> @@ -65,6 +65,7 @@ nv50_validate_fb(struct nv50_context *nv50)
>           PUSH_DATA (push, sf->height);
>           BEGIN_NV04(push, NV50_3D(RT_ARRAY_MODE), 1);
>           PUSH_DATA (push, array_mode | array_size);
> +         nv50->rt_array_mode = array_mode | array_size;
>        } else {
>           PUSH_DATA (push, 0);
>           PUSH_DATA (push, 0);
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_surface.c
b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
> index a22436b..68924c9 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_surface.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
> @@ -303,7 +303,7 @@ nv50_clear_render_target(struct pipe_context *pipe,
>        PUSH_DATA(push, NV50_3D_RT_HORIZ_LINEAR | mt->level[0].pitch);
>     PUSH_DATA (push, sf->height);
>     BEGIN_NV04(push, NV50_3D(RT_ARRAY_MODE), 1);
> -   PUSH_DATA (push, 1);
> +   PUSH_DATA (push, 512);
>  
>     if (!nouveau_bo_memtype(bo)) {
>        BEGIN_NV04(push, NV50_3D(ZETA_ENABLE), 1);
> @@ -366,7 +366,7 @@ nv50_clear_depth_stencil(struct pipe_context *pipe,
>     PUSH_DATA (push, bo->offset + sf->offset);
>     PUSH_DATA (push, nv50_format_table[dst->format].rt);
>     PUSH_DATA (push, mt->level[sf->base.u.tex.level].tile_mode);
> -   PUSH_DATA (push, 0);
> +   PUSH_DATA (push, mt->layer_stride >> 2);
>     BEGIN_NV04(push, NV50_3D(ZETA_ENABLE), 1);
>     PUSH_DATA (push, 1);
>     BEGIN_NV04(push, NV50_3D(ZETA_HORIZ), 3);
> @@ -374,6 +374,9 @@ nv50_clear_depth_stencil(struct pipe_context *pipe,
>     PUSH_DATA (push, sf->height);
>     PUSH_DATA (push, (1 << 16) | 1);
>  
> +   BEGIN_NV04(push, NV50_3D(RT_ARRAY_MODE), 1);
> +   PUSH_DATA (push, 512);
> +
>     BEGIN_NV04(push, NV50_3D(VIEWPORT_HORIZ(0)), 2);
>     PUSH_DATA (push, (width << 16) | dstx);
>     PUSH_DATA (push, (height << 16) | dsty);
> @@ -402,6 +405,14 @@ nv50_clear(struct pipe_context *pipe, unsigned
buffers,
>     if (!nv50_state_validate(nv50, NV50_NEW_FRAMEBUFFER, 9 +
(fb->nr_cbufs * 2)))
>        return;
>  
> +   /* We have to clear ALL of the layers, not up to the min number of
layers
> +    * of any attachment. Don't touch 3d textures, they can't be
arrays. The
> +    * above nv50_state_validate call will have written to rt_array_mode.
*/
> +   if (!(nv50->rt_array_mode & NV50_3D_RT_ARRAY_MODE_MODE_3D)) {
> +      BEGIN_NV04(push, NV50_3D(RT_ARRAY_MODE), 1);
> +      PUSH_DATA (push, 512);
> +   }
> +
>     if (buffers & PIPE_CLEAR_COLOR && fb->nr_cbufs) {
>        BEGIN_NV04(push, NV50_3D(CLEAR_COLOR(0)), 4);
>        PUSH_DATAf(push, color->f[0]);
> @@ -459,6 +470,10 @@ nv50_clear(struct pipe_context *pipe, unsigned
buffers,
>                      (j << NV50_3D_CLEAR_BUFFERS_LAYER__SHIFT));
>        }
>     }
> +
> +   /* restore the array mode */
> +   BEGIN_NV04(push, NV50_3D(RT_ARRAY_MODE), 1);
> +   PUSH_DATA (push, nv50->rt_array_mode);
>  }
>  
>  
>

Ilia Mirkin

2014-Feb-14 03:27 UTC

head link

[Nouveau] [PATCH v2] nv50: make sure to clear _all_ layers of all attachments

Unfortunately there's only one RT_ARRAY_MODE setting for all
attachments, so clears were previously truncated to the minimum number
of layers any attachment had. Instead set the RT_ARRAY_MODE to 512 (the
max number of layers) before doing the clear. This fixes
gl-3.2-layered-rendering-clear-color-mismatched-layer-count.

Also fix clears of layered depth/stencils, in case it ever happens.

Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
---

v1 -> v2:
  - turns out that 3d textures are actually cleared 1 layer at a time, and
    their depths might also not match, so handle that case as well.
  - handle the situation where nv50_clear_render_target receives a 3d texture

 src/gallium/drivers/nouveau/nv50/nv50_context.h       |  2 ++
 .../drivers/nouveau/nv50/nv50_state_validate.c        |  1 +
 src/gallium/drivers/nouveau/nv50/nv50_surface.c       | 19 +++++++++++++++++--
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_context.h
b/src/gallium/drivers/nouveau/nv50/nv50_context.h
index 57a3090..84ff46e 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_context.h
+++ b/src/gallium/drivers/nouveau/nv50/nv50_context.h
@@ -173,6 +173,8 @@ struct nv50_context {
 
    boolean vbo_push_hint;
 
+   uint32_t rt_array_mode;
+
    struct pipe_query *cond_query;
    boolean cond_cond;
    uint cond_mode;
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
b/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
index f953422..100d02d 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
@@ -65,6 +65,7 @@ nv50_validate_fb(struct nv50_context *nv50)
          PUSH_DATA (push, sf->height);
          BEGIN_NV04(push, NV50_3D(RT_ARRAY_MODE), 1);
          PUSH_DATA (push, array_mode | array_size);
+         nv50->rt_array_mode = array_mode | array_size;
       } else {
          PUSH_DATA (push, 0);
          PUSH_DATA (push, 0);
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_surface.c
b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
index a22436b..af8d6c2 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_surface.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
@@ -303,7 +303,10 @@ nv50_clear_render_target(struct pipe_context *pipe,
       PUSH_DATA(push, NV50_3D_RT_HORIZ_LINEAR | mt->level[0].pitch);
    PUSH_DATA (push, sf->height);
    BEGIN_NV04(push, NV50_3D(RT_ARRAY_MODE), 1);
-   PUSH_DATA (push, 1);
+   if (mt->layout_3d)
+      PUSH_DATA(push, NV50_3D_RT_ARRAY_MODE_MODE_3D | 512);
+   else
+      PUSH_DATA(push, 512);
 
    if (!nouveau_bo_memtype(bo)) {
       BEGIN_NV04(push, NV50_3D(ZETA_ENABLE), 1);
@@ -366,7 +369,7 @@ nv50_clear_depth_stencil(struct pipe_context *pipe,
    PUSH_DATA (push, bo->offset + sf->offset);
    PUSH_DATA (push, nv50_format_table[dst->format].rt);
    PUSH_DATA (push, mt->level[sf->base.u.tex.level].tile_mode);
-   PUSH_DATA (push, 0);
+   PUSH_DATA (push, mt->layer_stride >> 2);
    BEGIN_NV04(push, NV50_3D(ZETA_ENABLE), 1);
    PUSH_DATA (push, 1);
    BEGIN_NV04(push, NV50_3D(ZETA_HORIZ), 3);
@@ -374,6 +377,9 @@ nv50_clear_depth_stencil(struct pipe_context *pipe,
    PUSH_DATA (push, sf->height);
    PUSH_DATA (push, (1 << 16) | 1);
 
+   BEGIN_NV04(push, NV50_3D(RT_ARRAY_MODE), 1);
+   PUSH_DATA (push, 512);
+
    BEGIN_NV04(push, NV50_3D(VIEWPORT_HORIZ(0)), 2);
    PUSH_DATA (push, (width << 16) | dstx);
    PUSH_DATA (push, (height << 16) | dsty);
@@ -402,6 +408,11 @@ nv50_clear(struct pipe_context *pipe, unsigned buffers,
    if (!nv50_state_validate(nv50, NV50_NEW_FRAMEBUFFER, 9 + (fb->nr_cbufs *
2)))
       return;
 
+   /* We have to clear ALL of the layers, not up to the min number of layers
+    * of any attachment. */
+   BEGIN_NV04(push, NV50_3D(RT_ARRAY_MODE), 1);
+   PUSH_DATA (push, (nv50->rt_array_mode &
NV50_3D_RT_ARRAY_MODE_MODE_3D) | 512);
+
    if (buffers & PIPE_CLEAR_COLOR && fb->nr_cbufs) {
       BEGIN_NV04(push, NV50_3D(CLEAR_COLOR(0)), 4);
       PUSH_DATAf(push, color->f[0]);
@@ -459,6 +470,10 @@ nv50_clear(struct pipe_context *pipe, unsigned buffers,
                     (j << NV50_3D_CLEAR_BUFFERS_LAYER__SHIFT));
       }
    }
+
+   /* restore the array mode */
+   BEGIN_NV04(push, NV50_3D(RT_ARRAY_MODE), 1);
+   PUSH_DATA (push, nv50->rt_array_mode);
 }
 
 
-- 
1.8.3.2

Maybe Matching Threads

Search for more apparently analagous threads

Nouveau - Feb 2014 - [PATCH] nv50: make sure to clear _all_ layers of all attachments

[Nouveau] [PATCH] nv50: make sure to clear _all_ layers of all attachments

[Nouveau] [Mesa-dev] [PATCH] nv50: make sure to clear _all_ layers of all attachments

[Nouveau] [PATCH v2] nv50: make sure to clear _all_ layers of all attachments

Maybe Matching Threads