All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] nv50: make sure to clear _all_ layers of all attachments
@ 2014-02-13 10:13 Ilia Mirkin
  2014-02-13 16:38 ` Roland Scheidegger
  2014-02-14  3:27 ` [PATCH v2] " Ilia Mirkin
  0 siblings, 2 replies; 3+ messages in thread
From: Ilia Mirkin @ 2014-02-13 10:13 UTC (permalink / raw)
  To: Christoph Bumiller, mesa-dev, nouveau

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@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

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] nv50: make sure to clear _all_ layers of all attachments
  2014-02-13 10:13 [PATCH] nv50: make sure to clear _all_ layers of all attachments Ilia Mirkin
@ 2014-02-13 16:38 ` Roland Scheidegger
  2014-02-14  3:27 ` [PATCH v2] " Ilia Mirkin
  1 sibling, 0 replies; 3+ messages in thread
From: Roland Scheidegger @ 2014-02-13 16:38 UTC (permalink / raw)
  To: Ilia Mirkin, Christoph Bumiller, mesa-dev, nouveau

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@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);
>  }
>  
>  
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread
* [PATCH v2] nv50: make sure to clear _all_ layers of all attachments
  2014-02-13 10:13 [PATCH] nv50: make sure to clear _all_ layers of all attachments Ilia Mirkin
  2014-02-13 16:38 ` Roland Scheidegger
@ 2014-02-14  3:27 ` Ilia Mirkin
  1 sibling, 0 replies; 3+ messages in thread
From: Ilia Mirkin @ 2014-02-14  3:27 UTC (permalink / raw)
  To: Christoph Bumiller, mesa-dev, nouveau

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@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

^ permalink raw reply related	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-02-14  3:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-13 10:13 [PATCH] nv50: make sure to clear _all_ layers of all attachments Ilia Mirkin
2014-02-13 16:38 ` Roland Scheidegger
2014-02-14  3:27 ` [PATCH v2] " Ilia Mirkin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.