Nouveau - Dec 2010 - [PATCH 1/3] nvfx: fixes after array textures merge

- pipe_surface->context not initialized in nvfx_miptree_surface_new

Should util_surfaces_get handle this initialization and take a pipe_context
rather than a pipe_screen ?

- ns was not properly initialized in nvfx_miptree_surface_new

ns->offset == ~0 test was dead, the initialization of offset to ~0 was done
in util_surfaces_* but was killed together with offset (removed from
pipe_surface)

util_surfaces_get now return a boolean value indicating if pipe surface is
new, so that driver specific fields can be initialized in that case.

- fix bogus assert in nvfx_surface_copy_temp
---
 src/gallium/auxiliary/util/u_surfaces.c |   16 +++++++++++-----
 src/gallium/auxiliary/util/u_surfaces.h |   19 ++++++++++++++-----
 src/gallium/drivers/nvfx/nvfx_miptree.c |   17 +++++++++--------
 src/gallium/drivers/nvfx/nvfx_surface.c |    2 +-
 4 files changed, 35 insertions(+), 19 deletions(-)

diff --git a/src/gallium/auxiliary/util/u_surfaces.c
b/src/gallium/auxiliary/util/u_surfaces.c
index 45aa15e..fd55bd1 100644
--- a/src/gallium/auxiliary/util/u_surfaces.c
+++ b/src/gallium/auxiliary/util/u_surfaces.c
@@ -29,10 +29,11 @@
 #include "util/u_inlines.h"
 #include "util/u_memory.h"
 
-struct pipe_surface *
+boolean
 util_surfaces_do_get(struct util_surfaces *us, unsigned surface_struct_size,
                      struct pipe_screen *pscreen, struct pipe_resource *pt,
-                     unsigned level, unsigned layer, unsigned flags)
+                     unsigned level, unsigned layer, unsigned flags,
+                     struct pipe_surface **res)
 {
    struct pipe_surface *ps;
 
@@ -53,12 +54,16 @@ util_surfaces_do_get(struct util_surfaces *us, unsigned
surface_struct_size,
    if(ps)
    {
       p_atomic_inc(&ps->reference.count);
-      return ps;
+      *res = ps;
+      return FALSE;
    }
 
    ps = (struct pipe_surface *)CALLOC(1, surface_struct_size);
    if(!ps)
-      return NULL;
+   {
+      *res = NULL;
+      return FALSE;
+   }
 
    pipe_surface_init(ps, pt, level, layer, flags);
 
@@ -67,7 +72,8 @@ util_surfaces_do_get(struct util_surfaces *us, unsigned
surface_struct_size,
    else
       us->u.array[level] = ps;
 
-   return ps;
+   *res = ps;
+   return TRUE;
 }
 
 void
diff --git a/src/gallium/auxiliary/util/u_surfaces.h
b/src/gallium/auxiliary/util/u_surfaces.h
index 86a1c2f..da4fbbf 100644
--- a/src/gallium/auxiliary/util/u_surfaces.h
+++ b/src/gallium/auxiliary/util/u_surfaces.h
@@ -42,11 +42,19 @@ struct util_surfaces
    } u;
 };
 
-struct pipe_surface *util_surfaces_do_get(struct util_surfaces *us, unsigned
surface_struct_size, struct pipe_screen *pscreen, struct pipe_resource *pt,
unsigned level, unsigned layer, unsigned flags);
+/* Return value indicates if the pipe surface result is new */
+boolean
+util_surfaces_do_get(struct util_surfaces *us, unsigned surface_struct_size,
+                     struct pipe_screen *pscreen, struct pipe_resource *pt,
+                     unsigned level, unsigned layer, unsigned flags,
+                     struct pipe_surface **res);
 
 /* fast inline path for the very common case */
-static INLINE struct pipe_surface *
-util_surfaces_get(struct util_surfaces *us, unsigned surface_struct_size,
struct pipe_screen *pscreen, struct pipe_resource *pt, unsigned level, unsigned
layer, unsigned flags)
+static INLINE boolean
+util_surfaces_get(struct util_surfaces *us, unsigned surface_struct_size,
+                  struct pipe_screen *pscreen, struct pipe_resource *pt,
+                  unsigned level, unsigned layer, unsigned flags,
+                  struct pipe_surface **res)
 {
    if(likely((pt->target == PIPE_TEXTURE_2D || pt->target ==
PIPE_TEXTURE_RECT) && us->u.array))
    {
@@ -54,11 +62,12 @@ util_surfaces_get(struct util_surfaces *us, unsigned
surface_struct_size, struct
       if(ps)
       {
 	 p_atomic_inc(&ps->reference.count);
-	 return ps;
+	 *res = ps;
+	 return FALSE;
       }
    }
 
-   return util_surfaces_do_get(us, surface_struct_size, pscreen, pt, level,
layer, flags);
+   return util_surfaces_do_get(us, surface_struct_size, pscreen, pt, level,
layer, flags, res);
 }
 
 static INLINE struct pipe_surface *
diff --git a/src/gallium/drivers/nvfx/nvfx_miptree.c
b/src/gallium/drivers/nvfx/nvfx_miptree.c
index db48025..2f9d553 100644
--- a/src/gallium/drivers/nvfx/nvfx_miptree.c
+++ b/src/gallium/drivers/nvfx/nvfx_miptree.c
@@ -193,18 +193,19 @@ struct pipe_surface *
 nvfx_miptree_surface_new(struct pipe_context *pipe, struct pipe_resource *pt,
 			 const struct pipe_surface *surf_tmpl)
 {
-	struct nvfx_miptree* mt = (struct nvfx_miptree*)pt;
-	struct nvfx_surface *ns;
+	struct nvfx_miptree *mt = (struct nvfx_miptree *)pt;
 	unsigned level = surf_tmpl->u.tex.level;
+	struct nvfx_surface *ns = NULL;
 
 	assert(surf_tmpl->u.tex.first_layer == surf_tmpl->u.tex.last_layer);
-	ns = (struct nvfx_surface*)util_surfaces_get(&mt->surfaces,
sizeof(struct nvfx_surface), NULL, pt,
-						     level, surf_tmpl->u.tex.first_layer, surf_tmpl->usage);
-	if(ns->offset == ~0) {
-		util_dirty_surface_init(&ns->base);
-		ns->pitch = nvfx_subresource_pitch(pt, level);
-		ns->offset = nvfx_subresource_offset(pt, surf_tmpl->u.tex.first_layer,
level, surf_tmpl->u.tex.first_layer);
+	if(util_surfaces_get(&mt->surfaces, sizeof(struct nvfx_surface), NULL,
+                             pt, level, surf_tmpl->u.tex.first_layer,
+                             surf_tmpl->usage, (struct pipe_surface
**)&ns)) {
+                util_dirty_surface_init(&ns->base);
+                ns->pitch = nvfx_subresource_pitch(pt, level);
+                ns->offset = nvfx_subresource_offset(pt,
surf_tmpl->u.tex.first_layer, level, surf_tmpl->u.tex.first_layer);
 	}
+        ns->base.base.context = pipe;
 
 	return &ns->base.base;
 }
diff --git a/src/gallium/drivers/nvfx/nvfx_surface.c
b/src/gallium/drivers/nvfx/nvfx_surface.c
index 7f315e9..6fd6c47 100644
--- a/src/gallium/drivers/nvfx/nvfx_surface.c
+++ b/src/gallium/drivers/nvfx/nvfx_surface.c
@@ -389,7 +389,7 @@ nvfx_surface_copy_temp(struct pipe_context* pipe, struct
pipe_surface* surf, int
 	base_vertex = nvfx->base_vertex;
 
 	box.x = box.y = 0;
-	assert(surf->u.tex.first_layer = surf->u.tex.last_layer);
+	assert(surf->u.tex.first_layer == surf->u.tex.last_layer);
 	box.width = surf->width;
 	box.height = surf->height;
 	box.depth = 1;
-- 
1.7.3.2

Pass pipe_context rather than pipe_screen to util_surfaces_get and
util_surfaces_do_get so that they can check whether the saved pipe_surface
have the current context, and init properly a new pipe_surface with the
current context.

pipe_context is also passed to pipe_surface_reset and pipe_surface_init so
that context initialization is done in one central place.
---
 src/gallium/auxiliary/util/u_inlines.h  |   13 ++++++++-----
 src/gallium/auxiliary/util/u_surfaces.c |    6 +++---
 src/gallium/auxiliary/util/u_surfaces.h |    8 ++++----
 src/gallium/drivers/nvfx/nvfx_miptree.c |    3 +--
 4 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/src/gallium/auxiliary/util/u_inlines.h
b/src/gallium/auxiliary/util/u_inlines.h
index d5bc114..e55aafe 100644
--- a/src/gallium/auxiliary/util/u_inlines.h
+++ b/src/gallium/auxiliary/util/u_inlines.h
@@ -136,8 +136,9 @@ pipe_sampler_view_reference(struct pipe_sampler_view **ptr,
struct pipe_sampler_
 }
 
 static INLINE void
-pipe_surface_reset(struct pipe_surface* ps, struct pipe_resource *pt,
-                   unsigned level, unsigned layer, unsigned flags)
+pipe_surface_reset(struct pipe_context *ctx, struct pipe_surface* ps,
+                   struct pipe_resource *pt, unsigned level, unsigned layer,
+                   unsigned flags)
 {
    pipe_resource_reference(&ps->texture, pt);
    ps->format = pt->format;
@@ -146,15 +147,17 @@ pipe_surface_reset(struct pipe_surface* ps, struct
pipe_resource *pt,
    ps->usage = flags;
    ps->u.tex.level = level;
    ps->u.tex.first_layer = ps->u.tex.last_layer = layer;
+   ps->context = ctx;
 }
 
 static INLINE void
-pipe_surface_init(struct pipe_surface* ps, struct pipe_resource *pt,
-                  unsigned level, unsigned layer, unsigned flags)
+pipe_surface_init(struct pipe_context *ctx, struct pipe_surface* ps,
+                  struct pipe_resource *pt, unsigned level, unsigned layer,
+                  unsigned flags)
 {
    ps->texture = 0;
    pipe_reference_init(&ps->reference, 1);
-   pipe_surface_reset(ps, pt, level, layer, flags);
+   pipe_surface_reset(ctx, ps, pt, level, layer, flags);
 }
 
 /*
diff --git a/src/gallium/auxiliary/util/u_surfaces.c
b/src/gallium/auxiliary/util/u_surfaces.c
index fd55bd1..b0cfec2 100644
--- a/src/gallium/auxiliary/util/u_surfaces.c
+++ b/src/gallium/auxiliary/util/u_surfaces.c
@@ -31,7 +31,7 @@
 
 boolean
 util_surfaces_do_get(struct util_surfaces *us, unsigned surface_struct_size,
-                     struct pipe_screen *pscreen, struct pipe_resource *pt,
+                     struct pipe_context *ctx, struct pipe_resource *pt,
                      unsigned level, unsigned layer, unsigned flags,
                      struct pipe_surface **res)
 {
@@ -51,7 +51,7 @@ util_surfaces_do_get(struct util_surfaces *us, unsigned
surface_struct_size,
       ps = us->u.array[level];
    }
 
-   if(ps)
+   if(ps && ps->context == ctx)
    {
       p_atomic_inc(&ps->reference.count);
       *res = ps;
@@ -65,7 +65,7 @@ util_surfaces_do_get(struct util_surfaces *us, unsigned
surface_struct_size,
       return FALSE;
    }
 
-   pipe_surface_init(ps, pt, level, layer, flags);
+   pipe_surface_init(ctx, ps, pt, level, layer, flags);
 
    if(pt->target == PIPE_TEXTURE_3D || pt->target == PIPE_TEXTURE_CUBE)
       cso_hash_insert(us->u.hash, (layer << 8) | level, ps);
diff --git a/src/gallium/auxiliary/util/u_surfaces.h
b/src/gallium/auxiliary/util/u_surfaces.h
index da4fbbf..9581fed 100644
--- a/src/gallium/auxiliary/util/u_surfaces.h
+++ b/src/gallium/auxiliary/util/u_surfaces.h
@@ -45,21 +45,21 @@ struct util_surfaces
 /* Return value indicates if the pipe surface result is new */
 boolean
 util_surfaces_do_get(struct util_surfaces *us, unsigned surface_struct_size,
-                     struct pipe_screen *pscreen, struct pipe_resource *pt,
+                     struct pipe_context *ctx, struct pipe_resource *pt,
                      unsigned level, unsigned layer, unsigned flags,
                      struct pipe_surface **res);
 
 /* fast inline path for the very common case */
 static INLINE boolean
 util_surfaces_get(struct util_surfaces *us, unsigned surface_struct_size,
-                  struct pipe_screen *pscreen, struct pipe_resource *pt,
+                  struct pipe_context *ctx, struct pipe_resource *pt,
                   unsigned level, unsigned layer, unsigned flags,
                   struct pipe_surface **res)
 {
    if(likely((pt->target == PIPE_TEXTURE_2D || pt->target ==
PIPE_TEXTURE_RECT) && us->u.array))
    {
       struct pipe_surface *ps = us->u.array[level];
-      if(ps)
+      if(ps && ps->context == ctx)
       {
 	 p_atomic_inc(&ps->reference.count);
 	 *res = ps;
@@ -67,7 +67,7 @@ util_surfaces_get(struct util_surfaces *us, unsigned
surface_struct_size,
       }
    }
 
-   return util_surfaces_do_get(us, surface_struct_size, pscreen, pt, level,
layer, flags, res);
+   return util_surfaces_do_get(us, surface_struct_size, ctx, pt, level, layer,
flags, res);
 }
 
 static INLINE struct pipe_surface *
diff --git a/src/gallium/drivers/nvfx/nvfx_miptree.c
b/src/gallium/drivers/nvfx/nvfx_miptree.c
index 2f9d553..8c043b8 100644
--- a/src/gallium/drivers/nvfx/nvfx_miptree.c
+++ b/src/gallium/drivers/nvfx/nvfx_miptree.c
@@ -198,14 +198,13 @@ nvfx_miptree_surface_new(struct pipe_context *pipe, struct
pipe_resource *pt,
 	struct nvfx_surface *ns = NULL;
 
 	assert(surf_tmpl->u.tex.first_layer == surf_tmpl->u.tex.last_layer);
-	if(util_surfaces_get(&mt->surfaces, sizeof(struct nvfx_surface), NULL,
+	if(util_surfaces_get(&mt->surfaces, sizeof(struct nvfx_surface), pipe,
                              pt, level, surf_tmpl->u.tex.first_layer,
                              surf_tmpl->usage, (struct pipe_surface
**)&ns)) {
                 util_dirty_surface_init(&ns->base);
                 ns->pitch = nvfx_subresource_pitch(pt, level);
                 ns->offset = nvfx_subresource_offset(pt,
surf_tmpl->u.tex.first_layer, level, surf_tmpl->u.tex.first_layer);
 	}
-        ns->base.base.context = pipe;
 
 	return &ns->base.base;
 }
-- 
1.7.3.2

cso_cache_delete checks if pipe has bind_vertex_sampler_states and
set_vertex_sampler_views so do the same in tr_context.

This avoids a segfault when tracing nvfx driver on piglit
glx-destroycontext-2
---
 src/gallium/drivers/trace/tr_context.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/src/gallium/drivers/trace/tr_context.c
b/src/gallium/drivers/trace/tr_context.c
index 2fdb6c9..eaabae8 100644
--- a/src/gallium/drivers/trace/tr_context.c
+++ b/src/gallium/drivers/trace/tr_context.c
@@ -314,6 +314,9 @@ trace_context_bind_vertex_sampler_states(struct pipe_context
*_pipe,
    struct trace_context *tr_ctx = trace_context(_pipe);
    struct pipe_context *pipe = tr_ctx->pipe;
 
+   if (!pipe->bind_vertex_sampler_states)
+      return;
+
    trace_dump_call_begin("pipe_context",
"bind_vertex_sampler_states");
 
    trace_dump_arg(ptr, pipe);
@@ -980,6 +983,9 @@ trace_context_set_vertex_sampler_views(struct pipe_context
*_pipe,
    struct pipe_sampler_view *unwrapped_views[PIPE_MAX_VERTEX_SAMPLERS];
    unsigned i;
 
+   if (!pipe->set_vertex_sampler_views)
+      return;
+
    for(i = 0; i < num; ++i) {
       tr_view = trace_sampler_view(views[i]);
       unwrapped_views[i] = tr_view ? tr_view->sampler_view : NULL;
-- 
1.7.3.2

On Sun, Dec 5, 2010 at 1:16 AM, Xavier Chantry <chantry.xavier at
gmail.com> wrote:
> Pass pipe_context rather than pipe_screen to util_surfaces_get and
> util_surfaces_do_get so that they can check whether the saved pipe_surface
> have the current context, and init properly a new pipe_surface with the
> current context.
>
> pipe_context is also passed to pipe_surface_reset and pipe_surface_init so
> that context initialization is done in one central place.
Reviewed-by: Jakob Bornecrantz <wallbraker at gmail.com>

[SNIP]

On Sun, Dec 5, 2010 at 1:16 AM, Xavier Chantry <chantry.xavier at
gmail.com> wrote:
> cso_cache_delete checks if pipe has bind_vertex_sampler_states and
> set_vertex_sampler_views so do the same in tr_context.
>
> This avoids a segfault when tracing nvfx driver on piglit
> glx-destroycontext-2
Reviewed-by: Jakob Bornecrantz <wallbraker at gmail.com>

[SNIP]