thr3ads.net - Nouveau - [Nouveau] [PATCH 2/2] nv30: fix clip plane uploads and enable changes [May 2015]

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

Ilia Mirkin

2015-May-24 04:58 UTC

[Nouveau] [PATCH 1/2] nv30: avoid doing extra work on clear and hitting unexpected states

Clearing can happen at a time when various state objects are incoherent
and not ready for a draw. Some of the validation functions don't handle
this well, so only flush the framebuffer state. This has the advantage
of also not doing extra work.

This works around some crashes that can happen when clearing.

Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
---
 src/gallium/drivers/nouveau/nv30/nv30_clear.c          |  2 +-
 src/gallium/drivers/nouveau/nv30/nv30_context.h        |  2 +-
 src/gallium/drivers/nouveau/nv30/nv30_draw.c           |  4 ++--
 src/gallium/drivers/nouveau/nv30/nv30_state_validate.c | 10 ++++++----
 src/gallium/drivers/nouveau/nv30/nv30_vbo.c            |  2 +-
 5 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nv30/nv30_clear.c
b/src/gallium/drivers/nouveau/nv30/nv30_clear.c
index 1ab8929..83fd1fa 100644
--- a/src/gallium/drivers/nouveau/nv30/nv30_clear.c
+++ b/src/gallium/drivers/nouveau/nv30/nv30_clear.c
@@ -58,7 +58,7 @@ nv30_clear(struct pipe_context *pipe, unsigned buffers,
    struct pipe_framebuffer_state *fb = &nv30->framebuffer;
    uint32_t colr = 0, zeta = 0, mode = 0;
 
-   if (!nv30_state_validate(nv30, TRUE))
+   if (!nv30_state_validate(nv30, NV30_NEW_FRAMEBUFFER | NV30_NEW_SCISSOR,
TRUE))
       return;
 
    if (buffers & PIPE_CLEAR_COLOR && fb->nr_cbufs) {
diff --git a/src/gallium/drivers/nouveau/nv30/nv30_context.h
b/src/gallium/drivers/nouveau/nv30/nv30_context.h
index 7b32aae..592cdbe 100644
--- a/src/gallium/drivers/nouveau/nv30/nv30_context.h
+++ b/src/gallium/drivers/nouveau/nv30/nv30_context.h
@@ -204,7 +204,7 @@ void
 nv30_render_vbo(struct pipe_context *pipe, const struct pipe_draw_info *info);
 
 boolean
-nv30_state_validate(struct nv30_context *nv30, boolean hwtnl);
+nv30_state_validate(struct nv30_context *nv30, uint32_t mask, boolean hwtnl);
 
 void
 nv30_state_release(struct nv30_context *nv30);
diff --git a/src/gallium/drivers/nouveau/nv30/nv30_draw.c
b/src/gallium/drivers/nouveau/nv30/nv30_draw.c
index 3575c3d..38c31e9 100644
--- a/src/gallium/drivers/nouveau/nv30/nv30_draw.c
+++ b/src/gallium/drivers/nouveau/nv30/nv30_draw.c
@@ -129,7 +129,7 @@ nv30_render_draw_elements(struct vbuf_render *render,
                        NOUVEAU_BO_LOW | NOUVEAU_BO_RD, 0, 0);
    }
 
-   if (!nv30_state_validate(nv30, FALSE))
+   if (!nv30_state_validate(nv30, ~0, FALSE))
       return;
 
    BEGIN_NV04(push, NV30_3D(VERTEX_BEGIN_END), 1);
@@ -174,7 +174,7 @@ nv30_render_draw_arrays(struct vbuf_render *render, unsigned
start, uint nr)
                        NOUVEAU_BO_LOW | NOUVEAU_BO_RD, 0, 0);
    }
 
-   if (!nv30_state_validate(nv30, FALSE))
+   if (!nv30_state_validate(nv30, ~0, FALSE))
       return;
 
    BEGIN_NV04(push, NV30_3D(VERTEX_BEGIN_END), 1);
diff --git a/src/gallium/drivers/nouveau/nv30/nv30_state_validate.c
b/src/gallium/drivers/nouveau/nv30/nv30_state_validate.c
index 0f9d19d..86ac4f7 100644
--- a/src/gallium/drivers/nouveau/nv30/nv30_state_validate.c
+++ b/src/gallium/drivers/nouveau/nv30/nv30_state_validate.c
@@ -456,7 +456,7 @@ nv30_state_context_switch(struct nv30_context *nv30)
 }
 
 boolean
-nv30_state_validate(struct nv30_context *nv30, boolean hwtnl)
+nv30_state_validate(struct nv30_context *nv30, uint32_t mask, boolean hwtnl)
 {
    struct nouveau_screen *screen = &nv30->screen->base;
    struct nouveau_pushbuf *push = nv30->base.pushbuf;
@@ -481,14 +481,16 @@ nv30_state_validate(struct nv30_context *nv30, boolean
hwtnl)
    else
       validate = swtnl_validate_list;
 
-   if (nv30->dirty) {
+   mask &= nv30->dirty;
+
+   if (mask) {
       while (validate->func) {
-         if (nv30->dirty & validate->mask)
+         if (mask & validate->mask)
             validate->func(nv30);
          validate++;
       }
 
-      nv30->dirty = 0;
+      nv30->dirty &= ~mask;
    }
 
    nouveau_pushbuf_bufctx(push, bctx);
diff --git a/src/gallium/drivers/nouveau/nv30/nv30_vbo.c
b/src/gallium/drivers/nouveau/nv30/nv30_vbo.c
index 67ab829..d4e384b 100644
--- a/src/gallium/drivers/nouveau/nv30/nv30_vbo.c
+++ b/src/gallium/drivers/nouveau/nv30/nv30_vbo.c
@@ -564,7 +564,7 @@ nv30_draw_vbo(struct pipe_context *pipe, const struct
pipe_draw_info *info)
    if (nv30->vbo_user && !(nv30->dirty & (NV30_NEW_VERTEX |
NV30_NEW_ARRAYS)))
       nv30_update_user_vbufs(nv30);
 
-   nv30_state_validate(nv30, TRUE);
+   nv30_state_validate(nv30, ~0, TRUE);
    if (nv30->draw_flags) {
       nv30_render_vbo(pipe, info);
       return;
-- 
2.3.6

Ilia Mirkin

2015-May-24 04:58 UTC

head link

[Nouveau] [PATCH 2/2] nv30: fix clip plane uploads and enable changes

nv30_validate_clip depends on the rasterizer state. Also we should
upload all the new clip planes on change since next time the plane data
won't have changed, but the enables might.

Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
---
 src/gallium/drivers/nouveau/nv30/nv30_state_validate.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nv30/nv30_state_validate.c
b/src/gallium/drivers/nouveau/nv30/nv30_state_validate.c
index 86ac4f7..a954dcc 100644
--- a/src/gallium/drivers/nouveau/nv30/nv30_state_validate.c
+++ b/src/gallium/drivers/nouveau/nv30/nv30_state_validate.c
@@ -272,15 +272,13 @@ nv30_validate_clip(struct nv30_context *nv30)
    uint32_t clpd_enable = 0;
 
    for (i = 0; i < 6; i++) {
-      if (nv30->rast->pipe.clip_plane_enable & (1 << i)) {
-         if (nv30->dirty & NV30_NEW_CLIP) {
-            BEGIN_NV04(push, NV30_3D(VP_UPLOAD_CONST_ID), 5);
-            PUSH_DATA (push, i);
-            PUSH_DATAp(push, nv30->clip.ucp[i], 4);
-         }
-
-         clpd_enable |= 1 << (1 + 4*i);
+      if (nv30->dirty & NV30_NEW_CLIP) {
+         BEGIN_NV04(push, NV30_3D(VP_UPLOAD_CONST_ID), 5);
+         PUSH_DATA (push, i);
+         PUSH_DATAp(push, nv30->clip.ucp[i], 4);
       }
+      if (nv30->rast->pipe.clip_plane_enable & (1 << i))
+         clpd_enable |= 2 << (4*i);
    }
 
    BEGIN_NV04(push, NV30_3D(VP_CLIP_PLANES_ENABLE), 1);
@@ -389,7 +387,7 @@ static struct state_validate hwtnl_validate_list[] = {
     { nv30_validate_stipple,       NV30_NEW_STIPPLE },
     { nv30_validate_scissor,       NV30_NEW_SCISSOR | NV30_NEW_RASTERIZER },
     { nv30_validate_viewport,      NV30_NEW_VIEWPORT },
-    { nv30_validate_clip,          NV30_NEW_CLIP },
+    { nv30_validate_clip,          NV30_NEW_CLIP | NV30_NEW_RASTERIZER },
     { nv30_fragprog_validate,      NV30_NEW_FRAGPROG | NV30_NEW_FRAGCONST },
     { nv30_vertprog_validate,      NV30_NEW_VERTPROG | NV30_NEW_VERTCONST |
                                    NV30_NEW_FRAGPROG | NV30_NEW_RASTERIZER },
-- 
2.3.6

Samuel Pitoiset

2015-May-24 08:38 UTC

head link

[Nouveau] [Mesa-dev] [PATCH 2/2] nv30: fix clip plane uploads and enable changes

On 05/24/2015 06:58 AM, Ilia Mirkin wrote:> nv30_validate_clip depends on the rasterizer state. Also we should
> upload all the new clip planes on change since next time the plane data
> won't have changed, but the enables might.
>
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
>   src/gallium/drivers/nouveau/nv30/nv30_state_validate.c | 16
+++++++---------
>   1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nv30/nv30_state_validate.c
b/src/gallium/drivers/nouveau/nv30/nv30_state_validate.c
> index 86ac4f7..a954dcc 100644
> --- a/src/gallium/drivers/nouveau/nv30/nv30_state_validate.c
> +++ b/src/gallium/drivers/nouveau/nv30/nv30_state_validate.c
> @@ -272,15 +272,13 @@ nv30_validate_clip(struct nv30_context *nv30)
>      uint32_t clpd_enable = 0;
>   
>      for (i = 0; i < 6; i++) {
> -      if (nv30->rast->pipe.clip_plane_enable & (1 << i)) {
> -         if (nv30->dirty & NV30_NEW_CLIP) {
> -            BEGIN_NV04(push, NV30_3D(VP_UPLOAD_CONST_ID), 5);
> -            PUSH_DATA (push, i);
> -            PUSH_DATAp(push, nv30->clip.ucp[i], 4);
> -         }
> -
> -         clpd_enable |= 1 << (1 + 4*i);
> +      if (nv30->dirty & NV30_NEW_CLIP) {
> +         BEGIN_NV04(push, NV30_3D(VP_UPLOAD_CONST_ID), 5);
> +         PUSH_DATA (push, i);
> +         PUSH_DATAp(push, nv30->clip.ucp[i], 4);
>         }
> +      if (nv30->rast->pipe.clip_plane_enable & (1 << i))
> +         clpd_enable |= 2 << (4*i);
Can you explain why did you change this line?
>      }
>   
>      BEGIN_NV04(push, NV30_3D(VP_CLIP_PLANES_ENABLE), 1);
> @@ -389,7 +387,7 @@ static struct state_validate hwtnl_validate_list[] = {
>       { nv30_validate_stipple,       NV30_NEW_STIPPLE },
>       { nv30_validate_scissor,       NV30_NEW_SCISSOR | NV30_NEW_RASTERIZER
},
>       { nv30_validate_viewport,      NV30_NEW_VIEWPORT },
> -    { nv30_validate_clip,          NV30_NEW_CLIP },
> +    { nv30_validate_clip,          NV30_NEW_CLIP | NV30_NEW_RASTERIZER },
>       { nv30_fragprog_validate,      NV30_NEW_FRAGPROG | NV30_NEW_FRAGCONST
},
>       { nv30_vertprog_validate,      NV30_NEW_VERTPROG | NV30_NEW_VERTCONST
|
>                                      NV30_NEW_FRAGPROG |
NV30_NEW_RASTERIZER },

Nouveau - May 2015 - [PATCH 2/2] nv30: fix clip plane uploads and enable changes

[Nouveau] [PATCH 1/2] nv30: avoid doing extra work on clear and hitting unexpected states

[Nouveau] [PATCH 2/2] nv30: fix clip plane uploads and enable changes

[Nouveau] [Mesa-dev] [PATCH 2/2] nv30: fix clip plane uploads and enable changes