Re: [Mesa-dev] [PATCH] configure.ac: Fix typos.

2017-04-20 Thread Alejandro Piñeiro
Reviewed-by: Alejandro Piñeiro 

On 20/04/17 23:50, Vinson Lee wrote:
> Signed-off-by: Vinson Lee 
> ---
>  configure.ac | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 957d15df8caa..e42fcfff7757 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -724,7 +724,7 @@ dnl Arch/platform-specific settings
>  dnl
>  AC_ARG_ENABLE([asm],
>  [AS_HELP_STRING([--disable-asm],
> -[disable assembly usage @<:@default=enabled on supported 
> plaforms@:>@])],
> +[disable assembly usage @<:@default=enabled on supported 
> platforms@:>@])],
>  [enable_asm="$enableval"],
>  [enable_asm=yes]
>  )
> @@ -2146,7 +2146,7 @@ dnl DEPRECATED: EGL Platforms configuration
>  dnl
>  AC_ARG_WITH([egl-platforms],
>  [AS_HELP_STRING([--with-egl-platforms@<:@=DIRS...@:>@],
> -[DEPRECATED: use --with-plaforms instead@<:@default=auto@:>@])],
> +[DEPRECATED: use --with-platforms instead@<:@default=auto@:>@])],
>  [with_egl_platforms="$withval"],
>  [with_egl_platforms=auto])
>  
> @@ -2161,7 +2161,7 @@ if test "x$with_egl_platforms" = xauto; then
>  with_egl_platforms=""
>  fi
>  else
> -AC_MSG_WARN([--with-egl-platforms is deprecated. Use --with-plaforms 
> instead.])
> +AC_MSG_WARN([--with-egl-platforms is deprecated. Use --with-platforms 
> instead.])
>  fi
>  
>  dnl

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 3/4] mesa: Remove deleteFlag pattern.

2017-04-20 Thread Timothy Arceri
From: Matt Turner 

---
 src/mesa/main/arrayobj.c| 4 +---
 src/mesa/main/pipelineobj.c | 4 +---
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/src/mesa/main/arrayobj.c b/src/mesa/main/arrayobj.c
index 9f4477e..24555d9 100644
--- a/src/mesa/main/arrayobj.c
+++ b/src/mesa/main/arrayobj.c
@@ -181,28 +181,26 @@ _mesa_delete_vao(struct gl_context *ctx, struct 
gl_vertex_array_object *obj)
  */
 void
 _mesa_reference_vao_(struct gl_context *ctx,
  struct gl_vertex_array_object **ptr,
  struct gl_vertex_array_object *vao)
 {
assert(*ptr != vao);
 
if (*ptr) {
   /* Unreference the old array object */
-  GLboolean deleteFlag = GL_FALSE;
   struct gl_vertex_array_object *oldObj = *ptr;
 
   assert(oldObj->RefCount > 0);
   oldObj->RefCount--;
-  deleteFlag = (oldObj->RefCount == 0);
 
-  if (deleteFlag)
+  if (oldObj->RefCount == 0)
  _mesa_delete_vao(ctx, oldObj);
 
   *ptr = NULL;
}
assert(!*ptr);
 
if (vao) {
   /* reference new array object */
   assert(vao->RefCount > 0);
 
diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c
index a0fa55a..9a852be 100644
--- a/src/mesa/main/pipelineobj.c
+++ b/src/mesa/main/pipelineobj.c
@@ -177,28 +177,26 @@ remove_pipeline_object(struct gl_context *ctx, struct 
gl_pipeline_object *obj)
  */
 void
 _mesa_reference_pipeline_object_(struct gl_context *ctx,
  struct gl_pipeline_object **ptr,
  struct gl_pipeline_object *obj)
 {
assert(*ptr != obj);
 
if (*ptr) {
   /* Unreference the old pipeline object */
-  GLboolean deleteFlag = GL_FALSE;
   struct gl_pipeline_object *oldObj = *ptr;
 
   assert(oldObj->RefCount > 0);
   oldObj->RefCount--;
-  deleteFlag = (oldObj->RefCount == 0);
 
-  if (deleteFlag) {
+  if (oldObj->RefCount == 0) {
  _mesa_delete_pipeline_object(ctx, oldObj);
   }
 
   *ptr = NULL;
}
assert(!*ptr);
 
if (obj) {
   /* reference new pipeline object */
   assert(obj->RefCount > 0);
-- 
2.9.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 2/4] mesa: Remove unnecessary locking from container objects.

2017-04-20 Thread Timothy Arceri
From: Matt Turner 

From Chapter 5 'Shared Objects and Multiple Contexts' of
the OpenGL 4.5 spec:

   "Objects which contain references to other objects include
   framebuffer, program pipeline, query, transform feedback,
   and vertex array objects.   Such objects are called container
   objects and are not shared"

For we leave locking in place for framebuffer objects because
the EXT fbo extension allowed sharing.

V2: (Timothy Arceri)
 - rebased and dropped changes to framebuffer objects
---
 src/mesa/main/arrayobj.c| 6 --
 src/mesa/main/mtypes.h  | 4 
 src/mesa/main/pipelineobj.c | 6 --
 src/mesa/main/shaderapi.c   | 3 ---
 4 files changed, 19 deletions(-)

diff --git a/src/mesa/main/arrayobj.c b/src/mesa/main/arrayobj.c
index fdb3caa..9f4477e 100644
--- a/src/mesa/main/arrayobj.c
+++ b/src/mesa/main/arrayobj.c
@@ -162,21 +162,20 @@ _mesa_new_vao(struct gl_context *ctx, GLuint name)
 
 
 /**
  * Delete an array object.
  */
 void
 _mesa_delete_vao(struct gl_context *ctx, struct gl_vertex_array_object *obj)
 {
unbind_array_object_vbos(ctx, obj);
_mesa_reference_buffer_object(ctx, >IndexBufferObj, NULL);
-   mtx_destroy(>Mutex);
free(obj->Label);
free(obj);
 }
 
 
 /**
  * Set ptr to vao w/ reference counting.
  * Note: this should only be called from the _mesa_reference_vao()
  * inline function.
  */
@@ -185,41 +184,37 @@ _mesa_reference_vao_(struct gl_context *ctx,
  struct gl_vertex_array_object **ptr,
  struct gl_vertex_array_object *vao)
 {
assert(*ptr != vao);
 
if (*ptr) {
   /* Unreference the old array object */
   GLboolean deleteFlag = GL_FALSE;
   struct gl_vertex_array_object *oldObj = *ptr;
 
-  mtx_lock(>Mutex);
   assert(oldObj->RefCount > 0);
   oldObj->RefCount--;
   deleteFlag = (oldObj->RefCount == 0);
-  mtx_unlock(>Mutex);
 
   if (deleteFlag)
  _mesa_delete_vao(ctx, oldObj);
 
   *ptr = NULL;
}
assert(!*ptr);
 
if (vao) {
   /* reference new array object */
-  mtx_lock(>Mutex);
   assert(vao->RefCount > 0);
 
   vao->RefCount++;
   *ptr = vao;
-  mtx_unlock(>Mutex);
}
 }
 
 
 /**
  * Initialize attribtes of a vertex array within a vertex array object.
  * \param vao  the container vertex array object
  * \param index  which array in the VAO to initialize
  * \param size  number of components (1, 2, 3 or 4) per attribute
  * \param type  datatype of the attribute (GL_FLOAT, GL_INT, etc).
@@ -261,21 +256,20 @@ init_array(struct gl_context *ctx,
  */
 void
 _mesa_initialize_vao(struct gl_context *ctx,
  struct gl_vertex_array_object *vao,
  GLuint name)
 {
GLuint i;
 
vao->Name = name;
 
-   mtx_init(>Mutex, mtx_plain);
vao->RefCount = 1;
 
/* Init the individual arrays */
for (i = 0; i < ARRAY_SIZE(vao->VertexAttrib); i++) {
   switch (i) {
   case VERT_ATTRIB_WEIGHT:
  init_array(ctx, vao, VERT_ATTRIB_WEIGHT, 1, GL_FLOAT);
  break;
   case VERT_ATTRIB_NORMAL:
  init_array(ctx, vao, VERT_ATTRIB_NORMAL, 3, GL_FLOAT);
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index c4fab9d..b3711ba 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -1502,22 +1502,20 @@ struct gl_vertex_buffer_binding
  */
 struct gl_vertex_array_object
 {
/** Name of the VAO as received from glGenVertexArray. */
GLuint Name;
 
GLint RefCount;
 
GLchar *Label;   /**< GL_KHR_debug */
 
-   mtx_t Mutex;
-
/**
 * Does the VAO use ARB semantics or Apple semantics?
 *
 * There are several ways in which ARB_vertex_array_object and
 * APPLE_vertex_array_object VAOs have differing semantics.  At the very
 * least,
 *
 * - ARB VAOs require that all array data be sourced from vertex buffer
 *   objects, but Apple VAOs do not.
 *
@@ -2994,22 +2992,20 @@ struct gl_shader_program
  */
 struct gl_pipeline_object
 {
/** Name of the pipeline object as received from glGenProgramPipelines.
 * It would be 0 for shaders without separate shader objects.
 */
GLuint Name;
 
GLint RefCount;
 
-   mtx_t Mutex;
-
GLchar *Label;   /**< GL_KHR_debug */
 
/**
 * Programs used for rendering
 *
 * There is a separate program set for each shader stage.
 */
struct gl_program *CurrentProgram[MESA_SHADER_STAGES];
 
struct gl_shader_program *ReferencedPrograms[MESA_SHADER_STAGES];
diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c
index 2988c97..a0fa55a 100644
--- a/src/mesa/main/pipelineobj.c
+++ b/src/mesa/main/pipelineobj.c
@@ -59,35 +59,33 @@ _mesa_delete_pipeline_object(struct gl_context *ctx,
unsigned i;
 
_mesa_reference_program(ctx, >_CurrentFragmentProgram, NULL);
 
for (i = 0; i < MESA_SHADER_STAGES; i++) {
   _mesa_reference_program(ctx, >CurrentProgram[i], NULL);
   

[Mesa-dev] [PATCH 4/4] mesa: don't lock hashtables that are not shared across contexts

2017-04-20 Thread Timothy Arceri
From Chapter 5 'Shared Objects and Multiple Contexts' of
the OpenGL 4.5 spec:

   "Objects which contain references to other objects include
   framebuffer, program pipeline, query, transform feedback,
   and vertex array objects.   Such objects are called container
   objects and are not shared"

For we leave locking in place for framebuffer objects because
the EXT fbo extension allowed sharing.

We could maybe just replace the hash with an ordinary hash table
but for now this should remove most of the unnecessary locking.
---
 src/mesa/main/arrayobj.c  | 8 
 src/mesa/main/pipelineobj.c   | 6 +++---
 src/mesa/main/queryobj.c  | 8 
 src/mesa/main/queryobj.h  | 2 +-
 src/mesa/main/transformfeedback.c | 7 ---
 5 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/src/mesa/main/arrayobj.c b/src/mesa/main/arrayobj.c
index 24555d9..b901a89 100644
--- a/src/mesa/main/arrayobj.c
+++ b/src/mesa/main/arrayobj.c
@@ -64,21 +64,21 @@
  * non-existent.
  */
 
 struct gl_vertex_array_object *
 _mesa_lookup_vao(struct gl_context *ctx, GLuint id)
 {
if (id == 0)
   return NULL;
else
   return (struct gl_vertex_array_object *)
- _mesa_HashLookup(ctx->Array.Objects, id);
+ _mesa_HashLookupLocked(ctx->Array.Objects, id);
 }
 
 
 /**
  * Looks up the array object for the given ID.
  *
  * Unlike _mesa_lookup_vao, this function generates a GL_INVALID_OPERATION
  * error if the array object does not exist. It also returns the default
  * array object when ctx is a compatibility profile context and id is zero.
  */
@@ -101,21 +101,21 @@ _mesa_lookup_vao_err(struct gl_context *ctx, GLuint id, 
const char *caller)
 
   return ctx->Array.DefaultVAO;
} else {
   struct gl_vertex_array_object *vao;
 
   if (ctx->Array.LastLookedUpVAO &&
   ctx->Array.LastLookedUpVAO->Name == id) {
  vao = ctx->Array.LastLookedUpVAO;
   } else {
  vao = (struct gl_vertex_array_object *)
-_mesa_HashLookup(ctx->Array.Objects, id);
+_mesa_HashLookupLocked(ctx->Array.Objects, id);
 
  /* The ARB_direct_state_access specification says:
   *
   *"An INVALID_OPERATION error is generated if  is not
   * [compatibility profile: zero or] the name of an existing
   * vertex array object."
   */
  if (!vao || !vao->EverBound) {
 _mesa_error(ctx, GL_INVALID_OPERATION,
 "%s(non-existent vaobj=%u)", caller, id);
@@ -299,35 +299,35 @@ _mesa_initialize_vao(struct gl_context *ctx,
 
 
 /**
  * Add the given array object to the array object pool.
  */
 static void
 save_array_object(struct gl_context *ctx, struct gl_vertex_array_object *vao)
 {
if (vao->Name > 0) {
   /* insert into hash table */
-  _mesa_HashInsert(ctx->Array.Objects, vao->Name, vao);
+  _mesa_HashInsertLocked(ctx->Array.Objects, vao->Name, vao);
}
 }
 
 
 /**
  * Remove the given array object from the array object pool.
  * Do not deallocate the array object though.
  */
 static void
 remove_array_object(struct gl_context *ctx, struct gl_vertex_array_object *vao)
 {
if (vao->Name > 0) {
   /* remove from hash table */
-  _mesa_HashRemove(ctx->Array.Objects, vao->Name);
+  _mesa_HashRemoveLocked(ctx->Array.Objects, vao->Name);
}
 }
 
 
 /**
  * Updates the derived gl_vertex_arrays when a gl_vertex_attrib_array
  * or a gl_vertex_buffer_binding has changed.
  */
 void
 _mesa_update_vao_client_arrays(struct gl_context *ctx,
diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c
index 9a852be..dbca3c3 100644
--- a/src/mesa/main/pipelineobj.c
+++ b/src/mesa/main/pipelineobj.c
@@ -137,43 +137,43 @@ _mesa_free_pipeline_data(struct gl_context *ctx)
  * a non-existent ID.  The spec defines ID 0 as being technically
  * non-existent.
  */
 struct gl_pipeline_object *
 _mesa_lookup_pipeline_object(struct gl_context *ctx, GLuint id)
 {
if (id == 0)
   return NULL;
else
   return (struct gl_pipeline_object *)
- _mesa_HashLookup(ctx->Pipeline.Objects, id);
+ _mesa_HashLookupLocked(ctx->Pipeline.Objects, id);
 }
 
 /**
  * Add the given pipeline object to the pipeline object pool.
  */
 static void
 save_pipeline_object(struct gl_context *ctx, struct gl_pipeline_object *obj)
 {
if (obj->Name > 0) {
-  _mesa_HashInsert(ctx->Pipeline.Objects, obj->Name, obj);
+  _mesa_HashInsertLocked(ctx->Pipeline.Objects, obj->Name, obj);
}
 }
 
 /**
  * Remove the given pipeline object from the pipeline object pool.
  * Do not deallocate the pipeline object though.
  */
 static void
 remove_pipeline_object(struct gl_context *ctx, struct gl_pipeline_object *obj)
 {
if (obj->Name > 0) {
-  _mesa_HashRemove(ctx->Pipeline.Objects, obj->Name);
+  _mesa_HashRemoveLocked(ctx->Pipeline.Objects, obj->Name);
}
 }
 
 /**
  * Set ptr to obj w/ reference counting.
  * 

[Mesa-dev] [PATCH 1/4] mesa: remove fallback RefCount == 0 pattern

2017-04-20 Thread Timothy Arceri
We should never get here if this is 0 unless there is a
bug. Replace the check with an assert.
---
 src/mesa/main/arrayobj.c  | 14 --
 src/mesa/main/bufferobj.c | 14 --
 src/mesa/main/pipelineobj.c   | 14 --
 src/mesa/main/samplerobj.c| 14 --
 src/mesa/main/texobj.c| 14 --
 src/mesa/main/transformfeedback.c | 14 +-
 6 files changed, 25 insertions(+), 59 deletions(-)

diff --git a/src/mesa/main/arrayobj.c b/src/mesa/main/arrayobj.c
index ab1b834..fdb3caa 100644
--- a/src/mesa/main/arrayobj.c
+++ b/src/mesa/main/arrayobj.c
@@ -201,30 +201,24 @@ _mesa_reference_vao_(struct gl_context *ctx,
   if (deleteFlag)
  _mesa_delete_vao(ctx, oldObj);
 
   *ptr = NULL;
}
assert(!*ptr);
 
if (vao) {
   /* reference new array object */
   mtx_lock(>Mutex);
-  if (vao->RefCount == 0) {
- /* this array's being deleted (look just above) */
- /* Not sure this can every really happen.  Warn if it does. */
- _mesa_problem(NULL, "referencing deleted array object");
- *ptr = NULL;
-  }
-  else {
- vao->RefCount++;
- *ptr = vao;
-  }
+  assert(vao->RefCount > 0);
+
+  vao->RefCount++;
+  *ptr = vao;
   mtx_unlock(>Mutex);
}
 }
 
 
 /**
  * Initialize attribtes of a vertex array within a vertex array object.
  * \param vao  the container vertex array object
  * \param index  which array in the VAO to initialize
  * \param size  number of components (1, 2, 3 or 4) per attribute
diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
index 922c7d8..961871c 100644
--- a/src/mesa/main/bufferobj.c
+++ b/src/mesa/main/bufferobj.c
@@ -504,30 +504,24 @@ _mesa_reference_buffer_object_(struct gl_context *ctx,
  ctx->Driver.DeleteBuffer(ctx, oldObj);
   }
 
   *ptr = NULL;
}
assert(!*ptr);
 
if (bufObj) {
   /* reference new buffer */
   mtx_lock(>Mutex);
-  if (bufObj->RefCount == 0) {
- /* this buffer's being deleted (look just above) */
- /* Not sure this can every really happen.  Warn if it does. */
- _mesa_problem(NULL, "referencing deleted buffer object");
- *ptr = NULL;
-  }
-  else {
- bufObj->RefCount++;
- *ptr = bufObj;
-  }
+  assert(bufObj->RefCount > 0);
+
+  bufObj->RefCount++;
+  *ptr = bufObj;
   mtx_unlock(>Mutex);
}
 }
 
 
 /**
  * Get the value of MESA_NO_MINMAX_CACHE.
  */
 static bool
 get_no_minmax_cache()
diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c
index c1dd8d7..2988c97 100644
--- a/src/mesa/main/pipelineobj.c
+++ b/src/mesa/main/pipelineobj.c
@@ -199,30 +199,24 @@ _mesa_reference_pipeline_object_(struct gl_context *ctx,
  _mesa_delete_pipeline_object(ctx, oldObj);
   }
 
   *ptr = NULL;
}
assert(!*ptr);
 
if (obj) {
   /* reference new pipeline object */
   mtx_lock(>Mutex);
-  if (obj->RefCount == 0) {
- /* this pipeline's being deleted (look just above) */
- /* Not sure this can ever really happen.  Warn if it does. */
- _mesa_problem(NULL, "referencing deleted pipeline object");
- *ptr = NULL;
-  }
-  else {
- obj->RefCount++;
- *ptr = obj;
-  }
+  assert(obj->RefCount > 0);
+
+  obj->RefCount++;
+  *ptr = obj;
   mtx_unlock(>Mutex);
}
 }
 
 static void
 use_program_stage(struct gl_context *ctx, GLenum type,
   struct gl_shader_program *shProg,
   struct gl_pipeline_object *pipe) {
gl_shader_stage stage = _mesa_shader_enum_to_shader_stage(type);
struct gl_program *prog = NULL;
diff --git a/src/mesa/main/samplerobj.c b/src/mesa/main/samplerobj.c
index 183f1d2..63beaf1 100644
--- a/src/mesa/main/samplerobj.c
+++ b/src/mesa/main/samplerobj.c
@@ -90,30 +90,24 @@ _mesa_reference_sampler_object_(struct gl_context *ctx,
   if (deleteFlag)
  delete_sampler_object(ctx, oldSamp);
 
   *ptr = NULL;
}
assert(!*ptr);
 
if (samp) {
   /* reference new sampler */
   mtx_lock(>Mutex);
-  if (samp->RefCount == 0) {
- /* this sampler's being deleted (look just above) */
- /* Not sure this can every really happen.  Warn if it does. */
- _mesa_problem(NULL, "referencing deleted sampler object");
- *ptr = NULL;
-  }
-  else {
- samp->RefCount++;
- *ptr = samp;
-  }
+  assert(samp->RefCount > 0);
+
+  samp->RefCount++;
+  *ptr = samp;
   mtx_unlock(>Mutex);
}
 }
 
 
 /**
  * Initialize the fields of the given sampler object.
  */
 static void
 _mesa_init_sampler_object(struct gl_sampler_object *sampObj, GLuint name)
diff --git a/src/mesa/main/texobj.c b/src/mesa/main/texobj.c
index 00feb97..af9baa9 100644
--- a/src/mesa/main/texobj.c
+++ b/src/mesa/main/texobj.c
@@ -559,30 

Re: [Mesa-dev] [PATCH 1/2] i965/vec4: set swizzle when loading an uniform

2017-04-20 Thread Samuel Iglesias Gonsálvez
On Thu, 2017-04-20 at 10:26 -0700, Francisco Jerez wrote:
> Samuel Iglesias Gonsálvez  writes:
> 
> > It was setting XYWZ swizzle to all uniforms, no matter if they were
> > a vector or not.
> > 
> > Signed-off-by: Samuel Iglesias Gonsálvez 
> > Cc: curroje...@riseup.net
> 
> Don't you need to CC mesa-stable here and in the next patch?
> 

I considered it but I has doubts about which tag use "17.1.0-rc1" or
just "17.1.0" or whatever. So my plan is to notify Emil once they are
merged (and add Cc to stable in the commit log before pushing it to
master).

If you are more comfortable with Cc mesa-stable, I will do it next time
(or if I need to send v2 of this series).

> > ---
> >  src/intel/compiler/brw_vec4_nir.cpp | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/src/intel/compiler/brw_vec4_nir.cpp
> > b/src/intel/compiler/brw_vec4_nir.cpp
> > index a82d52088a8..5f4488c7e86 100644
> > --- a/src/intel/compiler/brw_vec4_nir.cpp
> > +++ b/src/intel/compiler/brw_vec4_nir.cpp
> > @@ -863,6 +863,7 @@
> > vec4_visitor::nir_emit_intrinsic(nir_intrinsic_instr *instr)
> >   unsigned offset = const_offset->u32[0] + shift * 4;
> >   src.offset = ROUND_DOWN_TO(offset, 16);
> >   shift = (offset % 16) / 4;
> > + src.swizzle = brw_swizzle_for_size(instr-
> > >num_components);
> 
> What about the indirect case a few lines below?  Isn't the swizzle
> passed
> to the mov indirect instruction still bogus?
> 

This is different. It is expecting to have a swizzle of XYZW because
MOV_INDIRECT will copy all the contents. See assert in
move_uniform_array_access_to_pull_constants() and the comment in
pack_uniform_registers():

/* We just mark every register touched by a MOV_INDIRECT as being
 * fully used.  This ensures that it doesn't broken up piecewise by
 * the next part of our packing algorithm.
 */

Sam

> >   src.swizzle += BRW_SWIZZLE4(shift, shift, shift, shift);
> >  
> >   emit(MOV(dest, src));
> > -- 
> > 2.11.0

signature.asc
Description: This is a digitally signed message part
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 3/3] egl/android: Dequeue buffers inside EGL calls (v2)

2017-04-20 Thread Tapani Pälli



On 04/20/2017 07:51 PM, Mauro Rossi wrote:



2017-04-20 4:18 GMT+02:00 Tomasz Figa >:


On Wed, Apr 19, 2017 at 11:51 PM, Emil Velikov
> wrote:
 > Hi Tomasz,
 >
 > On 19 April 2017 at 08:00, Tomasz Figa > wrote:
 >> Android buffer queues can be abandoned, which results in failing to
 >> dequeue next buffer. Currently this would fail somewhere deep within
 >> the DRI stack calling loader's getBuffers*(), without any error
 >> reporting to the client app. However Android framework code
relies on
 >> proper signaling of this event, so we move buffer dequeue to
 >> createWindowSurface() and swapBuffers() call, which can generate
proper
 >> EGL errors. To keep the performance benefits of delayed buffer
handling,
 >> if any, fence wait and DRI image creation is kept delayed until
 >> getBuffers*() is called by the DRI driver.
 >>
 >> v2:
 >>  - add missing fence wait in DRI loader case,
 >>  - split simple code moving to a separate patch (Emil),
 >>  - fix previous rebase error.
 >>
 >> Signed-off-by: Tomasz Figa >
 >> ---
 >>  src/egl/drivers/dri2/egl_dri2.h |  1 +
 >>  src/egl/drivers/dri2/platform_android.c | 94
+++--
 >>  2 files changed, 54 insertions(+), 41 deletions(-)
 >>
 >> diff --git a/src/egl/drivers/dri2/egl_dri2.h
b/src/egl/drivers/dri2/egl_dri2.h
 >> index f16663712d..92b234d622 100644
 >> --- a/src/egl/drivers/dri2/egl_dri2.h
 >> +++ b/src/egl/drivers/dri2/egl_dri2.h
 >> @@ -288,6 +288,7 @@ struct dri2_egl_surface
 >>  #ifdef HAVE_ANDROID_PLATFORM
 >> struct ANativeWindow *window;
 >> struct ANativeWindowBuffer *buffer;
 >> +   int acquire_fence_fd;
 >> __DRIimage *dri_image_back;
 >> __DRIimage *dri_image_front;
 >>
 >> diff --git a/src/egl/drivers/dri2/platform_android.c
b/src/egl/drivers/dri2/platform_android.c
 >> index 9a84a4c43d..0a75d790c5 100644
 >> --- a/src/egl/drivers/dri2/platform_android.c
 >> +++ b/src/egl/drivers/dri2/platform_android.c
 >> @@ -189,15 +189,9 @@ droid_free_local_buffers(struct
dri2_egl_surface *dri2_surf)
 >> }
 >>  }
 >>
 >> -static EGLBoolean
 >> -droid_window_dequeue_buffer(struct dri2_egl_surface *dri2_surf)
 >> +static void
 >> +wait_and_close_acquire_fence(struct dri2_egl_surface *dri2_surf)
 >>  {
 >> -   int fence_fd;
 >> -
 >> -   if (dri2_surf->window->dequeueBuffer(dri2_surf->window,
_surf->buffer,
 >> -_fd))
 >> -  return EGL_FALSE;
 >> -
 >> /* If access to the buffer is controlled by a sync fence,
then block on the
 >>  * fence.
 >>  *
 >> @@ -215,15 +209,25 @@ droid_window_dequeue_buffer(struct
dri2_egl_surface *dri2_surf)
 >>  *any value except -1) then the caller is responsible
for closing the
 >>  *file descriptor.
 >>  */
 >> -if (fence_fd >= 0) {
 >> +if (dri2_surf->acquire_fence_fd >= 0) {
 >> /* From the SYNC_IOC_WAIT documentation in :
 >>  *
 >>  *Waits indefinitely if timeout < 0.
 >>  */
 >>  int timeout = -1;
 >> -sync_wait(fence_fd, timeout);
 >> -close(fence_fd);
 >> +sync_wait(dri2_surf->acquire_fence_fd, timeout);
 >> +close(dri2_surf->acquire_fence_fd);
 >> +dri2_surf->acquire_fence_fd = -1;
 >> }
 >> +}
 >> +
 >> +static EGLBoolean
 >> +droid_window_dequeue_buffer(_EGLDisplay *disp,
 >> +struct dri2_egl_surface *dri2_surf)
 >> +{
 >> +   if (dri2_surf->window->dequeueBuffer(dri2_surf->window,
_surf->buffer,
 >> +   
_surf->acquire_fence_fd))

 >> +  return EGL_FALSE;
 >>
 >> dri2_surf->buffer->common.incRef(_surf->buffer->common);
 >>
 >> @@ -254,6 +258,14 @@ droid_window_dequeue_buffer(struct
dri2_egl_surface *dri2_surf)
 >>dri2_surf->back = _surf->color_buffers[0];
 >> }
 >>
 >> +   /* free outdated buffers and update the surface size */
 >> +   if (dri2_surf->base.Width != dri2_surf->buffer->width ||
 >> +   dri2_surf->base.Height != dri2_surf->buffer->height) {
 >> +  droid_free_local_buffers(dri2_surf);
 >> +  dri2_surf->base.Width = dri2_surf->buffer->width;
 >> +  dri2_surf->base.Height = dri2_surf->buffer->height;
 >> +   }
 >> +
 >> return EGL_TRUE;
 >>  }
 >>
 >> @@ -263,6 +275,9 @@ droid_window_enqueue_buffer(_EGLDisplay 

[Mesa-dev] [PATCH 2/3] radv/ac: overhaul vs output/ps input routing

2017-04-20 Thread Dave Airlie
From: Dave Airlie 

In order to cleanly eliminate exports rewrite the
code first to mirror how radeonsi works for now.

Signed-off-by: Dave Airlie 
---
 src/amd/common/ac_nir_to_llvm.c | 10 +++---
 src/amd/common/ac_nir_to_llvm.h | 15 +++--
 src/amd/vulkan/radv_pipeline.c  | 67 ++---
 3 files changed, 55 insertions(+), 37 deletions(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 514c9e9..ab929bc 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -5133,8 +5133,9 @@ handle_vs_outputs_post(struct nir_to_llvm_context *ctx,
LLVMValueRef psize_value = NULL, layer_value = NULL, 
viewport_index_value = NULL;
int i;
 
-   outinfo->prim_id_output = 0x;
-   outinfo->layer_output = 0x;
+   memset(outinfo->vs_output_param_offset, EXP_PARAM_UNDEFINED,
+  sizeof(outinfo->vs_output_param_offset));
+
if (ctx->output_mask & (1ull << VARYING_SLOT_CLIP_DIST0)) {
LLVMValueRef slots[8];
unsigned j;
@@ -5184,20 +5185,21 @@ handle_vs_outputs_post(struct nir_to_llvm_context *ctx,
} else if (i == VARYING_SLOT_LAYER) {
outinfo->writes_layer = true;
layer_value = values[0];
-   outinfo->layer_output = param_count;
target = V_008DFC_SQ_EXP_PARAM + param_count;
+   outinfo->vs_output_param_offset[VARYING_SLOT_LAYER] = 
param_count;
param_count++;
} else if (i == VARYING_SLOT_VIEWPORT) {
outinfo->writes_viewport_index = true;
viewport_index_value = values[0];
continue;
} else if (i == VARYING_SLOT_PRIMITIVE_ID) {
-   outinfo->prim_id_output = param_count;
target = V_008DFC_SQ_EXP_PARAM + param_count;
+   
outinfo->vs_output_param_offset[VARYING_SLOT_PRIMITIVE_ID] = param_count;
param_count++;
} else if (i >= VARYING_SLOT_VAR0) {
outinfo->export_mask |= 1u << (i - VARYING_SLOT_VAR0);
target = V_008DFC_SQ_EXP_PARAM + param_count;
+   outinfo->vs_output_param_offset[i] = param_count;
param_count++;
}
 
diff --git a/src/amd/common/ac_nir_to_llvm.h b/src/amd/common/ac_nir_to_llvm.h
index 401d284..f77a9b8 100644
--- a/src/amd/common/ac_nir_to_llvm.h
+++ b/src/amd/common/ac_nir_to_llvm.h
@@ -120,14 +120,25 @@ struct ac_userdata_locations {
struct ac_userdata_info shader_data[AC_UD_MAX_UD];
 };
 
+enum {
+   /* SPI_PS_INPUT_CNTL_i.OFFSET[0:4] */
+   EXP_PARAM_OFFSET_0 = 0,
+   EXP_PARAM_OFFSET_31 = 31,
+   /* SPI_PS_INPUT_CNTL_i.DEFAULT_VAL[0:1] */
+   EXP_PARAM_DEFAULT_VAL_ = 64,
+   EXP_PARAM_DEFAULT_VAL_0001,
+   EXP_PARAM_DEFAULT_VAL_1110,
+   EXP_PARAM_DEFAULT_VAL_,
+   EXP_PARAM_UNDEFINED = 255,
+};
+
 struct ac_vs_output_info {
+   uint8_t vs_output_param_offset[VARYING_SLOT_MAX];
uint8_t clip_dist_mask;
uint8_t cull_dist_mask;
bool writes_pointsize;
bool writes_layer;
bool writes_viewport_index;
-   uint32_t prim_id_output;
-   uint32_t layer_output;
uint32_t export_mask;
unsigned param_exports;
unsigned pos_exports;
diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
index 4cd832a..457fa91 100644
--- a/src/amd/vulkan/radv_pipeline.c
+++ b/src/amd/vulkan/radv_pipeline.c
@@ -1869,6 +1869,25 @@ static void calculate_pa_cl_vs_out_cntl(struct 
radv_pipeline *pipeline)
clip_dist_mask;
 
 }
+
+static uint32_t offset_to_ps_input(uint32_t offset, bool flat_shade)
+{
+   uint32_t ps_input_cntl;
+   if (offset <= EXP_PARAM_OFFSET_31)
+   ps_input_cntl = S_028644_OFFSET(offset);
+   else {
+   /* The input is a DEFAULT_VAL constant. */
+   assert(offset >= EXP_PARAM_DEFAULT_VAL_ &&
+  offset <= EXP_PARAM_DEFAULT_VAL_);
+   offset -= EXP_PARAM_DEFAULT_VAL_;
+   ps_input_cntl = S_028644_OFFSET(0x20) |
+   S_028644_DEFAULT_VAL(offset);
+   }
+   if (flat_shade)
+   ps_input_cntl |= S_028644_FLAT_SHADE(1);
+   return ps_input_cntl;
+}
+
 static void calculate_ps_inputs(struct radv_pipeline *pipeline)
 {
struct radv_shader_variant *ps, *vs;
@@ -1881,24 +1900,20 @@ static void calculate_ps_inputs(struct radv_pipeline 
*pipeline)
 
unsigned ps_offset = 0;
 
-   if (ps->info.fs.prim_id_input && (outinfo->prim_id_output != 
0x)) {
-   unsigned vs_offset, flat_shade;
-   unsigned val;
-   

[Mesa-dev] [PATCH 3/3] radv/ac: eliminate unused vertex shader outputs.

2017-04-20 Thread Dave Airlie
From: Dave Airlie 

This is ported from radeonsi, and I can see at least one
Talos shader drops an export due to this, and saves some
VGPR usage.

Signed-off-by: Dave Airlie 
---
 src/amd/common/ac_nir_to_llvm.c | 178 
 1 file changed, 178 insertions(+)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index ab929bc..38d5359 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -5753,6 +5753,182 @@ static void ac_llvm_finalize_module(struct 
nir_to_llvm_context * ctx)
LLVMDisposePassManager(passmgr);
 }
 
+#define EXP_TARGET (HAVE_LLVM >= 0x0500 ? 0 : 3)
+#define EXP_OUT0 (HAVE_LLVM >= 0x0500 ? 2 : 5)
+
+/* Return true if the PARAM export has been eliminated. */
+static bool
+ac_eliminate_const_output(struct nir_to_llvm_context *ctx,
+ struct ac_vs_output_info *outinfo,
+ LLVMValueRef inst, unsigned offset)
+{
+   unsigned i, default_val; /* SPI_PS_INPUT_CNTL_i.DEFAULT_VAL */
+   bool is_zero[4] = {}, is_one[4] = {};
+
+   for (i = 0; i < 4; i++) {
+   LLVMBool loses_info;
+   LLVMValueRef p = LLVMGetOperand(inst, EXP_OUT0 + i);
+
+   /* It's a constant expression. Undef outputs are eliminated 
too. */
+   if (LLVMIsUndef(p)) {
+   is_zero[i] = true;
+   is_one[i] = true;
+   } else if (LLVMIsAConstantFP(p)) {
+   double a = LLVMConstRealGetDouble(p, _info);
+
+   if (a == 0)
+   is_zero[i] = true;
+   else if (a == 1)
+   is_one[i] = true;
+   else
+
+   return false; /* other constant */
+   } else
+   return false;
+   }
+
+   /* Only certain combinations of 0 and 1 can be eliminated. */
+   if (is_zero[0] && is_zero[1] && is_zero[2])
+   default_val = is_zero[3] ? 0 : 1;
+   else if (is_one[0] && is_one[1] && is_one[2])
+   default_val = is_zero[3] ? 2 : 3;
+   else
+   return false;
+
+   /* The PARAM export can be represented as DEFAULT_VAL. Kill it. */
+   LLVMInstructionEraseFromParent(inst);
+
+   /* Change OFFSET to DEFAULT_VAL. */
+   for (i = 0; i < VARYING_SLOT_MAX; i++) {
+   if (outinfo->vs_output_param_offset[i] == offset) {
+   outinfo->vs_output_param_offset[i] =
+   EXP_PARAM_DEFAULT_VAL_ + default_val;
+   break;
+   }
+   }
+   return true;
+}
+
+struct si_vs_exports {
+   unsigned num;
+   unsigned offset[VARYING_SLOT_MAX];
+   LLVMValueRef inst[VARYING_SLOT_MAX];
+};
+
+static bool
+ac_is_function(LLVMValueRef v)
+{
+   return LLVMGetValueKind(v) == LLVMFunctionValueKind;
+}
+
+static void
+ac_eliminate_const_vs_outputs(struct nir_to_llvm_context *ctx)
+{
+   LLVMBasicBlockRef bb;
+   struct si_vs_exports exports;
+   bool removed_any = false;
+   struct ac_vs_output_info *outinfo;
+   exports.num = 0;
+
+   if (ctx->stage == MESA_SHADER_FRAGMENT ||
+   ctx->stage == MESA_SHADER_COMPUTE ||
+   ctx->stage == MESA_SHADER_TESS_CTRL ||
+   ctx->stage == MESA_SHADER_GEOMETRY)
+   return;
+
+   if (ctx->stage == MESA_SHADER_VERTEX) {
+   if (ctx->options->key.vs.as_ls ||
+   ctx->options->key.vs.as_es)
+   return;
+   outinfo = >shader_info->vs.outinfo;
+   }
+
+   if (ctx->stage == MESA_SHADER_TESS_EVAL) {
+   if (ctx->options->key.vs.as_es)
+   return;
+   outinfo = >shader_info->tes.outinfo;
+   }
+
+   /* Process all LLVM instructions. */
+   bb = LLVMGetFirstBasicBlock(ctx->main_function);
+   while (bb) {
+   LLVMValueRef inst = LLVMGetFirstInstruction(bb);
+
+   while (inst) {
+   LLVMValueRef cur = inst;
+   inst = LLVMGetNextInstruction(inst);
+
+   if (LLVMGetInstructionOpcode(cur) != LLVMCall)
+   continue;
+
+   LLVMValueRef callee = LLVMGetCalledValue(cur);
+
+   if (!ac_is_function(callee))
+   continue;
+
+   const char *name = LLVMGetValueName(callee);
+   unsigned num_args = LLVMCountParams(callee);
+
+   /* Check if this is an export instruction. */
+   if ((num_args != 9 && num_args != 8) ||
+   (strcmp(name, "llvm.SI.export") &&
+strcmp(name, "llvm.amdgcn.exp.f32")))
+   

[Mesa-dev] [PATCH 1/3] radv/ac: move point coord after layer/viewport.

2017-04-20 Thread Dave Airlie
From: Dave Airlie 

These need to be ordered as per shader enum ordering, I'll
rewrite this soon, but this is a bug fix.

Signed-off-by: Dave Airlie 
---
 src/amd/vulkan/radv_pipeline.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
index 8e71d59..4cd832a 100644
--- a/src/amd/vulkan/radv_pipeline.c
+++ b/src/amd/vulkan/radv_pipeline.c
@@ -1880,12 +1880,6 @@ static void calculate_ps_inputs(struct radv_pipeline 
*pipeline)
outinfo = >info.vs.outinfo;
 
unsigned ps_offset = 0;
-   if (ps->info.fs.has_pcoord) {
-   unsigned val;
-   val = S_028644_PT_SPRITE_TEX(1) | S_028644_OFFSET(0x20);
-   pipeline->graphics.ps_input_cntl[ps_offset] = val;
-   ps_offset++;
-   }
 
if (ps->info.fs.prim_id_input && (outinfo->prim_id_output != 
0x)) {
unsigned vs_offset, flat_shade;
@@ -1907,6 +1901,13 @@ static void calculate_ps_inputs(struct radv_pipeline 
*pipeline)
++ps_offset;
}
 
+   if (ps->info.fs.has_pcoord) {
+   unsigned val;
+   val = S_028644_PT_SPRITE_TEX(1) | S_028644_OFFSET(0x20);
+   pipeline->graphics.ps_input_cntl[ps_offset] = val;
+   ps_offset++;
+   }
+
for (unsigned i = 0; i < 32 && (1u << i) <= ps->info.fs.input_mask; 
++i) {
unsigned vs_offset, flat_shade;
unsigned val;
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v5 0/3] asynchronous pbo transfer with glthread

2017-04-20 Thread Michel Dänzer
On 21/04/17 09:01 AM, Marek Olšák wrote:
> On Thu, Apr 20, 2017 at 9:44 PM, gregory hainaut
>  wrote:
>> On Thu, 20 Apr 2017 20:01:00 +0200
>> Marek Olšák  wrote:
>>
>>> On Thu, Apr 20, 2017 at 6:53 PM, gregory hainaut
>>>  wrote:
 On Thu, 20 Apr 2017 11:57:08 +0200
 Marek Olšák  wrote:

> On Thu, Apr 20, 2017 at 10:28 AM, gregory hainaut
>  wrote:
>> On Thu, 20 Apr 2017 12:29:11 +0900
>> Michel Dänzer  wrote:
>>
>>> On 20/04/17 01:54 AM, Gregory Hainaut wrote:
 Hello,

 Please find the latest version that include a small fix for hash 
 deletion. I
 think the series is good now. Please review/ack it.
>>>
>>> I'm afraid I have to NACK it. As discussed in the v4 cover letter
>>> thread, Mesa's glthread cannot make any libX11 API calls.
>>>
>>>
>>
>> Hello Michel,
>>
>> Just to be sure we are on the same line, let's me do a summary.
>>
>> PCSX2 does the following bad pattern
>> 1/ no call to XInitThread
>> 2/ XGetGeometry to get the size of the surface
>> 3/ glDrawArray into the default framebuffer 0 (the window, I'm not sure 
>> how to call it)
>>=> which seem to call DRI2GetBuffersWithFormat under the hood. I 
>> guess to get the
>>   associated buffer of the window.
>>
>>
>> So far it was (kind of) working fine because PCSX2 does tons of PBO 
>> transfer. So glthread
>> was mostly synchronous.
>>
>> This series removes the (useless) PBO transfer synchronization. So now 
>> glthread is really
>> asynchronous and the above bad pattern crash as expected.
>>
>> I didn't add any libX11 API call on the patches. And I don't think we 
>> can remvove the DRI stuff.
>>
>> Hum, I don't know what will be the impact on the perf but potentially we 
>> can force a synchronization
>> when there is a draw to framebuffer 0.
>
> Can you send us the backtrace when DRI2GetBuffersWithFormat is called
> by glDrawArrays?
>
> Marek

 Hello Marek, Michel,

 Here the full backtrace.

 #0  DRI2GetBuffersWithFormat (dpy=0xc6307e48, drawable=104857784, 
 width=0xdef348c0, height=0xdef348c4, attachments=0xdcc15c88, count=1, 
 outCount=0xdcc15c74) at dri2.c:464
 #1  0xf05bac45 in dri2GetBuffersWithFormat (driDrawable=0xdef348a8, 
 width=0xdef348c0, height=0xdef348c4, attachments=0xdcc15c88, count=1, 
 out_count=0xdcc15c74, loaderPrivate=0xdefc6ef0) at dri2_glx.c:894
 #2  0xe3ec3111 in dri2_drawable_get_buffers (count=, 
 atts=0xdef252f8, drawable=0xdefc7b08) at dri2.c:285
 #3  dri2_allocate_textures (ctx=0xdef15928, drawable=0xdefc7b08, 
 statts=0xdef252f8, statts_count=2) at dri2.c:480
 #4  0xe3ebcbb0 in dri_st_framebuffer_validate (stctx=0xdef4ab10, 
 stfbi=0xdefc7b08, statts=0xdef252f8, count=2, out=0xdcc15d78) at 
 dri_drawable.c:83
 #5  0xe3d37afc in st_framebuffer_validate (stfb=stfb@entry=0xdef24f58, 
 st=st@entry=0xdef4ab10) at state_tracker/st_manager.c:189
 Note "print stfb->Base->Name" give me 0
 #6  0xe3d38649 in st_manager_validate_framebuffers (st=0xdef4ab10) at 
 state_tracker/st_manager.c:869
 #7  0xe3cd0580 in st_validate_state (st=0xdef4ab10, 
 pipeline=ST_PIPELINE_RENDER) at state_tracker/st_atom.c:174
 #8  0xe3cf935d in st_draw_vbo (ctx=0xdef7f8f8, prims=0xdcc15f40, 
 nr_prims=1, ib=0x0, index_bounds_valid=1 '\001', min_index=39911, 
 max_index=39914, tfb_vertcount=0x0, stream=0, indirect=0x0) at 
 state_tracker/st_draw.c:191
 #9  0xe3caf35e in vbo_draw_arrays (baseInstance=0, numInstances=1, 
 count=4, start=39911, mode=5, ctx=0xdef7f8f8) at vbo/vbo_exec_array.c:427
 #10 vbo_exec_DrawArrays (mode=5, start=39911, count=4) at 
 vbo/vbo_exec_array.c:575
 #11 0xe3be4f07 in _mesa_unmarshal_DrawArrays (ctx=0xdef7f8f8, 
 cmd=0xc41574b0) at main/marshal_generated.c:26644
 #12 _mesa_unmarshal_dispatch_cmd (ctx=0xdef7f8f8, cmd=0xc41574b0) at 
 main/marshal_generated.c:42457
 #13 0xe3ba8af0 in glthread_unmarshal_batch (release_batch=true, 
 batch=0xc4157410, ctx=0xdef7f8f8) at main/glthread.c:64
 #14 glthread_worker (data=0xdef7f8f8) at main/glthread.c:110


> If this DRI2GetBuffersWithFormat call results in libX11 API calls on the
> glthread, that's a bug which needs to be fixed, either by moving the
> DRI2GetBuffersWithFormat call to the main thread or (if possible) by
> changing DRI2GetBuffersWithFormat to use XCB directly.
>>>
>>> First, we need to understand why it's happening. Does the app use the
>>> front buffer? Does it ever call glDrawBuffer(GL_FRONT) or
>>> glReadBuffer(GL_FRONT)?
>>
>> Hello Marek,
>>
>> No, I don't use the front buffer. I don't call 

Re: [Mesa-dev] [PATCH v5 0/3] asynchronous pbo transfer with glthread

2017-04-20 Thread Michel Dänzer
On 20/04/17 05:28 PM, gregory hainaut wrote:
> On Thu, 20 Apr 2017 12:29:11 +0900
> Michel Dänzer  wrote:
> 
>> On 20/04/17 01:54 AM, Gregory Hainaut wrote:
>>> Hello,
>>>
>>> Please find the latest version that include a small fix for hash deletion. I
>>> think the series is good now. Please review/ack it.
>>
>> I'm afraid I have to NACK it. As discussed in the v4 cover letter
>> thread, Mesa's glthread cannot make any libX11 API calls.
>>
>>
> 
> Hello Michel,
> 
> Just to be sure we are on the same line, let's me do a summary.
> 
> PCSX2 does the following bad pattern
> 1/ no call to XInitThread
> 2/ XGetGeometry to get the size of the surface
> 3/ glDrawArray into the default framebuffer 0 (the window, I'm not sure how 
> to call it)
>=> which seem to call DRI2GetBuffersWithFormat under the hood. I guess to 
> get the
>   associated buffer of the window.

If this DRI2GetBuffersWithFormat call results in libX11 API calls on the
glthread, that's a bug which needs to be fixed, either by moving the
DRI2GetBuffersWithFormat call to the main thread or (if possible) by
changing DRI2GetBuffersWithFormat to use XCB directly.


> So far it was (kind of) working fine because PCSX2 does tons of PBO transfer. 
> So glthread
> was mostly synchronous.
> 
> This series removes the (useless) PBO transfer synchronization. So now 
> glthread is really
> asynchronous and the above bad pattern crash as expected. 
> 
> I didn't add any libX11 API call on the patches.

It sounds like the glthread bug above exists independently from this
patch series, which might just make it more likely to result in a crash.
I think it would still be better to fix the bug before landing this series.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] RFC: mesa: remove MESA_VERBOSE env variable

2017-04-20 Thread Timothy Arceri

On 21/04/17 11:37, Jordan Justen wrote:

On 2017-04-20 12:33:45, Samuel Pitoiset wrote:

I have used it sometimes, but since VERBOSE_API is missing in a bunch of
places, that's quite useless. :)



I also use MESA_VERBOSE=api every few months or so. I've found it
pretty useful, but frustratingly I've nearly always ended up having to
add new calls for it become useful.

Something that automates wrapping the API probably is perhaps more
maintainable. 


As Eric mentioned it would probably be best to add something to dispatch 
generation if we want this to be reliable. It shouldn't be too difficult 
to do.



A lightweight apitrace mode? (Or, maybe this already
exists??) Perhaps something built into a 'debug' mode for libglvnd or
libepoxy? The downside of the library shim option is that it can be
sometimes be annoying to get your app to run with the shim.

-Jordan


On 04/20/2017 04:12 PM, Emil Velikov wrote:

From: Emil Velikov 

The env variable was useful in the early days of mesa development, when
tools such at Apitrace were not available.

Nowadays, out of the 12 options only a third are used. With nearly all
instances printing the API state. And even then not all entry points
are annotated, thus one cannot rely upon it.

The current patch removes the variable all together alongside a few
instances (listed below) which developers may see value in keeping.

FLUSH_VERTICES/FLUSH_CURRENT macros in src/mesa/main/context.h
_mesa_GetGraphicsResetStatusARB in src/mesa/main/robustness.c
_mesa_print_state() in src/mesa/main/state.c
_mesa_notifySwapBuffers() in src/mesa/main/context.c

Do we want to keep any of the MESA_VERBOSE instances? If so documenting
the variable seems reasonable.

Cc: Brian Paul 
Signed-off-by: Emil Velikov 
---
Brian, I assume that you're the author for most of these. How do you
feel on the topic?
---
   src/mesa/main/attrib.c  |   8 ---
   src/mesa/main/blend.c   |  40 
   src/mesa/main/blit.c|  17 -
   src/mesa/main/bufferobj.c   |  38 ---
   src/mesa/main/buffers.c |   7 ---
   src/mesa/main/clear.c   |   3 -
   src/mesa/main/compute.c |  13 
   src/mesa/main/context.c |  19 +-
   src/mesa/main/context.h |   4 --
   src/mesa/main/copyimage.c   |  10 ---
   src/mesa/main/debug.c   |  42 -
   src/mesa/main/depth.c   |  12 
   src/mesa/main/dlist.c   |  17 -
   src/mesa/main/drawpix.c |  21 ---
   src/mesa/main/enable.c  |   6 --
   src/mesa/main/fbobject.c|  32 --
   src/mesa/main/feedback.c|   3 -
   src/mesa/main/getstring.c   |   6 --
   src/mesa/main/hint.c|   5 --
   src/mesa/main/light.c   |  14 -
   src/mesa/main/lines.c   |   6 --
   src/mesa/main/matrix.c  |  29 -
   src/mesa/main/mtypes.h  |  22 ---
   src/mesa/main/performance_monitor.c |   6 --
   src/mesa/main/pipelineobj.c |  33 --
   src/mesa/main/polygon.c |  23 ---
   src/mesa/main/program_resource.c|  33 --
   src/mesa/main/queryobj.c|  31 -
   src/mesa/main/readpix.c |   7 ---
   src/mesa/main/robustness.c  |  14 +
   src/mesa/main/samplerobj.c  |   3 -
   src/mesa/main/scissor.c |  11 
   src/mesa/main/shaderapi.c   |  16 -
   src/mesa/main/state.c   |   3 -
   src/mesa/main/stencil.c |  27 
   src/mesa/main/texenv.c  |   7 ---
   src/mesa/main/texgen.c  |   7 ---
   src/mesa/main/texgetimage.c |  17 -
   src/mesa/main/teximage.c|  52 ---
   src/mesa/main/texobj.c  |  29 -
   src/mesa/main/texstate.c|   8 ---
   src/mesa/main/texstorage.c  |  13 
   src/mesa/main/textureview.c |   6 --
   src/mesa/main/varray.c  |   6 --
   src/mesa/main/viewport.c|  28 -
   src/mesa/tnl/t_vb_render.c  |   5 --
   src/mesa/vbo/vbo_exec_array.c   | 122 

   47 files changed, 2 insertions(+), 879 deletions(-)

diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
index 8e738c91c8f..bb230404d29 100644
--- a/src/mesa/main/attrib.c
+++ b/src/mesa/main/attrib.c
@@ -245,9 +245,6 @@ _mesa_PushAttrib(GLbitfield mask)
   
  GET_CURRENT_CONTEXT(ctx);
   
-   if (MESA_VERBOSE & VERBOSE_API)

-  _mesa_debug(ctx, "glPushAttrib %x\n", (int) mask);
-
  if (ctx->AttribStackDepth >= MAX_ATTRIB_STACK_DEPTH) {
 _mesa_error( ctx, GL_STACK_OVERFLOW, "glPushAttrib" );
 return;
@@ -935,11 +932,6 @@ _mesa_PopAttrib(void)
   
  while (attr) {
   
-  if 

Re: [Mesa-dev] [PATCH] RFC: mesa: remove MESA_VERBOSE env variable

2017-04-20 Thread Jordan Justen
On 2017-04-20 12:33:45, Samuel Pitoiset wrote:
> I have used it sometimes, but since VERBOSE_API is missing in a bunch of
> places, that's quite useless. :)
>

I also use MESA_VERBOSE=api every few months or so. I've found it
pretty useful, but frustratingly I've nearly always ended up having to
add new calls for it become useful.

Something that automates wrapping the API probably is perhaps more
maintainable. A lightweight apitrace mode? (Or, maybe this already
exists??) Perhaps something built into a 'debug' mode for libglvnd or
libepoxy? The downside of the library shim option is that it can be
sometimes be annoying to get your app to run with the shim.

-Jordan

> On 04/20/2017 04:12 PM, Emil Velikov wrote:
> > From: Emil Velikov 
> > 
> > The env variable was useful in the early days of mesa development, when
> > tools such at Apitrace were not available.
> > 
> > Nowadays, out of the 12 options only a third are used. With nearly all
> > instances printing the API state. And even then not all entry points
> > are annotated, thus one cannot rely upon it.
> > 
> > The current patch removes the variable all together alongside a few
> > instances (listed below) which developers may see value in keeping.
> > 
> > FLUSH_VERTICES/FLUSH_CURRENT macros in src/mesa/main/context.h
> > _mesa_GetGraphicsResetStatusARB in src/mesa/main/robustness.c
> > _mesa_print_state() in src/mesa/main/state.c
> > _mesa_notifySwapBuffers() in src/mesa/main/context.c
> > 
> > Do we want to keep any of the MESA_VERBOSE instances? If so documenting
> > the variable seems reasonable.
> > 
> > Cc: Brian Paul 
> > Signed-off-by: Emil Velikov 
> > ---
> > Brian, I assume that you're the author for most of these. How do you
> > feel on the topic?
> > ---
> >   src/mesa/main/attrib.c  |   8 ---
> >   src/mesa/main/blend.c   |  40 
> >   src/mesa/main/blit.c|  17 -
> >   src/mesa/main/bufferobj.c   |  38 ---
> >   src/mesa/main/buffers.c |   7 ---
> >   src/mesa/main/clear.c   |   3 -
> >   src/mesa/main/compute.c |  13 
> >   src/mesa/main/context.c |  19 +-
> >   src/mesa/main/context.h |   4 --
> >   src/mesa/main/copyimage.c   |  10 ---
> >   src/mesa/main/debug.c   |  42 -
> >   src/mesa/main/depth.c   |  12 
> >   src/mesa/main/dlist.c   |  17 -
> >   src/mesa/main/drawpix.c |  21 ---
> >   src/mesa/main/enable.c  |   6 --
> >   src/mesa/main/fbobject.c|  32 --
> >   src/mesa/main/feedback.c|   3 -
> >   src/mesa/main/getstring.c   |   6 --
> >   src/mesa/main/hint.c|   5 --
> >   src/mesa/main/light.c   |  14 -
> >   src/mesa/main/lines.c   |   6 --
> >   src/mesa/main/matrix.c  |  29 -
> >   src/mesa/main/mtypes.h  |  22 ---
> >   src/mesa/main/performance_monitor.c |   6 --
> >   src/mesa/main/pipelineobj.c |  33 --
> >   src/mesa/main/polygon.c |  23 ---
> >   src/mesa/main/program_resource.c|  33 --
> >   src/mesa/main/queryobj.c|  31 -
> >   src/mesa/main/readpix.c |   7 ---
> >   src/mesa/main/robustness.c  |  14 +
> >   src/mesa/main/samplerobj.c  |   3 -
> >   src/mesa/main/scissor.c |  11 
> >   src/mesa/main/shaderapi.c   |  16 -
> >   src/mesa/main/state.c   |   3 -
> >   src/mesa/main/stencil.c |  27 
> >   src/mesa/main/texenv.c  |   7 ---
> >   src/mesa/main/texgen.c  |   7 ---
> >   src/mesa/main/texgetimage.c |  17 -
> >   src/mesa/main/teximage.c|  52 ---
> >   src/mesa/main/texobj.c  |  29 -
> >   src/mesa/main/texstate.c|   8 ---
> >   src/mesa/main/texstorage.c  |  13 
> >   src/mesa/main/textureview.c |   6 --
> >   src/mesa/main/varray.c  |   6 --
> >   src/mesa/main/viewport.c|  28 -
> >   src/mesa/tnl/t_vb_render.c  |   5 --
> >   src/mesa/vbo/vbo_exec_array.c   | 122 
> > 
> >   47 files changed, 2 insertions(+), 879 deletions(-)
> > 
> > diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
> > index 8e738c91c8f..bb230404d29 100644
> > --- a/src/mesa/main/attrib.c
> > +++ b/src/mesa/main/attrib.c
> > @@ -245,9 +245,6 @@ _mesa_PushAttrib(GLbitfield mask)
> >   
> >  GET_CURRENT_CONTEXT(ctx);
> >   
> > -   if (MESA_VERBOSE & VERBOSE_API)
> > -  _mesa_debug(ctx, "glPushAttrib %x\n", (int) mask);
> > -
> >  if (ctx->AttribStackDepth >= MAX_ATTRIB_STACK_DEPTH) {
> > _mesa_error( ctx, GL_STACK_OVERFLOW, "glPushAttrib" );
> 

Re: [Mesa-dev] [PATCH 2/2] glsl: get rid of values_for_type()

2017-04-20 Thread Edward O'Callaghan
Reviewed-by: Edward O'Callaghan 

On 04/21/2017 11:20 AM, Timothy Arceri wrote:
> Looks ok to me. Series:
> 
> Reviewed-by: Timothy Arceri 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev



signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] glsl: get rid of values_for_type()

2017-04-20 Thread Timothy Arceri

Looks ok to me. Series:

Reviewed-by: Timothy Arceri 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] RFC: mesa: remove MESA_VERBOSE env variable

2017-04-20 Thread Timothy Arceri
Thanks for doing this. Looks like you forgot to updated the docs, with 
that fixed.


Acked-by: Timothy Arceri 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v5 0/3] asynchronous pbo transfer with glthread

2017-04-20 Thread Marek Olšák
On Thu, Apr 20, 2017 at 9:44 PM, gregory hainaut
 wrote:
> On Thu, 20 Apr 2017 20:01:00 +0200
> Marek Olšák  wrote:
>
>> On Thu, Apr 20, 2017 at 6:53 PM, gregory hainaut
>>  wrote:
>> > On Thu, 20 Apr 2017 11:57:08 +0200
>> > Marek Olšák  wrote:
>> >
>> >> On Thu, Apr 20, 2017 at 10:28 AM, gregory hainaut
>> >>  wrote:
>> >> > On Thu, 20 Apr 2017 12:29:11 +0900
>> >> > Michel Dänzer  wrote:
>> >> >
>> >> >> On 20/04/17 01:54 AM, Gregory Hainaut wrote:
>> >> >> > Hello,
>> >> >> >
>> >> >> > Please find the latest version that include a small fix for hash 
>> >> >> > deletion. I
>> >> >> > think the series is good now. Please review/ack it.
>> >> >>
>> >> >> I'm afraid I have to NACK it. As discussed in the v4 cover letter
>> >> >> thread, Mesa's glthread cannot make any libX11 API calls.
>> >> >>
>> >> >>
>> >> >
>> >> > Hello Michel,
>> >> >
>> >> > Just to be sure we are on the same line, let's me do a summary.
>> >> >
>> >> > PCSX2 does the following bad pattern
>> >> > 1/ no call to XInitThread
>> >> > 2/ XGetGeometry to get the size of the surface
>> >> > 3/ glDrawArray into the default framebuffer 0 (the window, I'm not sure 
>> >> > how to call it)
>> >> >=> which seem to call DRI2GetBuffersWithFormat under the hood. I 
>> >> > guess to get the
>> >> >   associated buffer of the window.
>> >> >
>> >> >
>> >> > So far it was (kind of) working fine because PCSX2 does tons of PBO 
>> >> > transfer. So glthread
>> >> > was mostly synchronous.
>> >> >
>> >> > This series removes the (useless) PBO transfer synchronization. So now 
>> >> > glthread is really
>> >> > asynchronous and the above bad pattern crash as expected.
>> >> >
>> >> > I didn't add any libX11 API call on the patches. And I don't think we 
>> >> > can remvove the DRI stuff.
>> >> >
>> >> > Hum, I don't know what will be the impact on the perf but potentially 
>> >> > we can force a synchronization
>> >> > when there is a draw to framebuffer 0.
>> >>
>> >> Can you send us the backtrace when DRI2GetBuffersWithFormat is called
>> >> by glDrawArrays?
>> >>
>> >> Marek
>> >
>> > Hello Marek, Michel,
>> >
>> > Here the full backtrace.
>> >
>> > #0  DRI2GetBuffersWithFormat (dpy=0xc6307e48, drawable=104857784, 
>> > width=0xdef348c0, height=0xdef348c4, attachments=0xdcc15c88, count=1, 
>> > outCount=0xdcc15c74) at dri2.c:464
>> > #1  0xf05bac45 in dri2GetBuffersWithFormat (driDrawable=0xdef348a8, 
>> > width=0xdef348c0, height=0xdef348c4, attachments=0xdcc15c88, count=1, 
>> > out_count=0xdcc15c74, loaderPrivate=0xdefc6ef0) at dri2_glx.c:894
>> > #2  0xe3ec3111 in dri2_drawable_get_buffers (count=, 
>> > atts=0xdef252f8, drawable=0xdefc7b08) at dri2.c:285
>> > #3  dri2_allocate_textures (ctx=0xdef15928, drawable=0xdefc7b08, 
>> > statts=0xdef252f8, statts_count=2) at dri2.c:480
>> > #4  0xe3ebcbb0 in dri_st_framebuffer_validate (stctx=0xdef4ab10, 
>> > stfbi=0xdefc7b08, statts=0xdef252f8, count=2, out=0xdcc15d78) at 
>> > dri_drawable.c:83
>> > #5  0xe3d37afc in st_framebuffer_validate (stfb=stfb@entry=0xdef24f58, 
>> > st=st@entry=0xdef4ab10) at state_tracker/st_manager.c:189
>> > Note "print stfb->Base->Name" give me 0
>> > #6  0xe3d38649 in st_manager_validate_framebuffers (st=0xdef4ab10) at 
>> > state_tracker/st_manager.c:869
>> > #7  0xe3cd0580 in st_validate_state (st=0xdef4ab10, 
>> > pipeline=ST_PIPELINE_RENDER) at state_tracker/st_atom.c:174
>> > #8  0xe3cf935d in st_draw_vbo (ctx=0xdef7f8f8, prims=0xdcc15f40, 
>> > nr_prims=1, ib=0x0, index_bounds_valid=1 '\001', min_index=39911, 
>> > max_index=39914, tfb_vertcount=0x0, stream=0, indirect=0x0) at 
>> > state_tracker/st_draw.c:191
>> > #9  0xe3caf35e in vbo_draw_arrays (baseInstance=0, numInstances=1, 
>> > count=4, start=39911, mode=5, ctx=0xdef7f8f8) at vbo/vbo_exec_array.c:427
>> > #10 vbo_exec_DrawArrays (mode=5, start=39911, count=4) at 
>> > vbo/vbo_exec_array.c:575
>> > #11 0xe3be4f07 in _mesa_unmarshal_DrawArrays (ctx=0xdef7f8f8, 
>> > cmd=0xc41574b0) at main/marshal_generated.c:26644
>> > #12 _mesa_unmarshal_dispatch_cmd (ctx=0xdef7f8f8, cmd=0xc41574b0) at 
>> > main/marshal_generated.c:42457
>> > #13 0xe3ba8af0 in glthread_unmarshal_batch (release_batch=true, 
>> > batch=0xc4157410, ctx=0xdef7f8f8) at main/glthread.c:64
>> > #14 glthread_worker (data=0xdef7f8f8) at main/glthread.c:110
>> >
>> >
>> >> If this DRI2GetBuffersWithFormat call results in libX11 API calls on the
>> >> glthread, that's a bug which needs to be fixed, either by moving the
>> >> DRI2GetBuffersWithFormat call to the main thread or (if possible) by
>> >> changing DRI2GetBuffersWithFormat to use XCB directly.
>>
>> First, we need to understand why it's happening. Does the app use the
>> front buffer? Does it ever call glDrawBuffer(GL_FRONT) or
>> glReadBuffer(GL_FRONT)?
>
> Hello Marek,
>
> No, I don't use the front buffer. I don't call glDrawBuffer for the 

[Mesa-dev] [PATCH] configure.ac: Fix typos.

2017-04-20 Thread Vinson Lee
Signed-off-by: Vinson Lee 
---
 configure.ac | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index 957d15df8caa..e42fcfff7757 100644
--- a/configure.ac
+++ b/configure.ac
@@ -724,7 +724,7 @@ dnl Arch/platform-specific settings
 dnl
 AC_ARG_ENABLE([asm],
 [AS_HELP_STRING([--disable-asm],
-[disable assembly usage @<:@default=enabled on supported 
plaforms@:>@])],
+[disable assembly usage @<:@default=enabled on supported 
platforms@:>@])],
 [enable_asm="$enableval"],
 [enable_asm=yes]
 )
@@ -2146,7 +2146,7 @@ dnl DEPRECATED: EGL Platforms configuration
 dnl
 AC_ARG_WITH([egl-platforms],
 [AS_HELP_STRING([--with-egl-platforms@<:@=DIRS...@:>@],
-[DEPRECATED: use --with-plaforms instead@<:@default=auto@:>@])],
+[DEPRECATED: use --with-platforms instead@<:@default=auto@:>@])],
 [with_egl_platforms="$withval"],
 [with_egl_platforms=auto])
 
@@ -2161,7 +2161,7 @@ if test "x$with_egl_platforms" = xauto; then
 with_egl_platforms=""
 fi
 else
-AC_MSG_WARN([--with-egl-platforms is deprecated. Use --with-plaforms 
instead.])
+AC_MSG_WARN([--with-egl-platforms is deprecated. Use --with-platforms 
instead.])
 fi
 
 dnl
-- 
2.9.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] mesa: replace _mesa_index_buffer::type with index_size

2017-04-20 Thread Kenneth Graunke
On Friday, April 14, 2017 8:06:21 AM PDT Marek Olšák wrote:
> From: Marek Olšák 
> 
> This avoids repeated translations of the enum.

Reviewed-by: Kenneth Graunke 


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 100741] Chromium - Memory leak

2017-04-20 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=100741

Bug ID: 100741
   Summary: Chromium - Memory leak
   Product: Mesa
   Version: git
  Hardware: x86-64 (AMD64)
OS: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: Mesa core
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: bartosz.tomczy...@gmail.com
QA Contact: mesa-dev@lists.freedesktop.org

Hi,

I observe huge memory leak in chromium browser:

Memory allocation:
==19259== 593,808 (590,328 direct, 3,480 indirect) bytes in 2,733 blocks are
definitely lost in loss record 5,325 of 5,331
==19259==at 0x4C2CF35: calloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==19259==by 0x9BAC22F: r600_create_surface_custom (r600_texture.c:1935)
==19259==by 0x9BACA94: r600_create_surface (r600_texture.c:1989)
==19259==by 0x97CE93F: st_framebuffer_validate (st_manager.c:222)
==19259==by 0x97CFBEE: st_api_make_current (st_manager.c:808)
==19259==by 0x9931097: dri_make_current (dri_context.c:258)
==19259==by 0x992FD64: driBindContext (dri_util.c:555)
==19259==by 0x7E5A19E: dri3_bind_context (dri3_glx.c:235)
==19259==by 0x7E27AB3: MakeContextCurrent (glxcurrent.c:228)

Memory not freed in:
st_renderbuffer_delete+0x243b58: state_tracker/st_cb_fbo.c:246
_mesa_reference_renderbuffer_+0x1997d0: main/renderbuffer.c:212
_mesa_reference_renderbuffer+0x1259d1: main/renderbuffer.h:72
_mesa_free_framebuffer_data+0x1259d1: main/framebuffer.c:223
_mesa_destroy_framebuffer+0x125ad0: main/framebuffer.c:199
_mesa_reference_framebuffer_+0x125b78: main/framebuffer.c:256
_mesa_reference_framebuffer+0x8b5b7: main/framebuffer.h:63
_mesa_make_current+0x8b5b7: main/context.c:1674
st_api_make_current+0x294cba: state_tracker/st_manager.c:827
dri_unbind_context+0x3f5f7d: src/gallium/state_trackers/dri/dri_context.c:217
driUnbindContext+0x3f57ec: src/mesa/drivers/dri/common/dri_util.c:591
MakeContextCurrent+0x164: src/glx/glxcurrent.c:214

Memory is not freed in st_renderbuffer_delete because ctx is NULL.
if (ctx) {
   struct st_context *st = st_context(ctx);
   pipe_surface_release(st->pipe, >surface);
}

Changing above part of code to :
pipe_surface_reference(>surface, NULL);
or
pipe_surface_release(strb->surface->context, >surface);

fixes problem for me, but I'm not sure if it is correct way of doing it.
Could someone comment on this?

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are the QA Contact for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] radv: Prefetch compute shader too.

2017-04-20 Thread Bas Nieuwenhuizen
For consistency, doesn't really impact performance.

Signed-off-by: Bas Nieuwenhuizen 
---
 src/amd/vulkan/radv_cmd_buffer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
index 958ae6e361e..40e6e432ae7 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -2169,6 +2169,7 @@ radv_emit_compute_pipeline(struct radv_cmd_buffer 
*cmd_buffer)
va = ws->buffer_get_va(compute_shader->bo);
 
ws->cs_add_buffer(cmd_buffer->cs, compute_shader->bo, 8);
+   si_cp_dma_prefetch(cmd_buffer, va, compute_shader->code_size);
 
MAYBE_UNUSED unsigned cdw_max = 
radeon_check_space(cmd_buffer->device->ws,
   cmd_buffer->cs, 16);
-- 
2.12.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] radv/ac: use tex_lz if we can.

2017-04-20 Thread Dave Airlie
From: Dave Airlie 

Looking at some Talos shaders vs radeonsi, I noticed they use
tex_lz in a few places, so we should be able to as well.

Signed-off-by: Dave Airlie 
---
 src/amd/common/ac_nir_to_llvm.c | 22 --
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 6062d3a..514c9e9 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -2049,6 +2049,7 @@ static LLVMValueRef radv_lower_gather4_integer(struct 
nir_to_llvm_context *ctx,
 
 static LLVMValueRef build_tex_intrinsic(struct nir_to_llvm_context *ctx,
nir_tex_instr *instr,
+   bool lod_is_zero,
struct ac_image_args *args)
 {
if (instr->sampler_dim == GLSL_SAMPLER_DIM_BUF) {
@@ -2074,7 +2075,10 @@ static LLVMValueRef build_tex_intrinsic(struct 
nir_to_llvm_context *ctx,
args->bias = true;
break;
case nir_texop_txl:
-   args->lod = true;
+   if (lod_is_zero)
+   args->level_zero = true;
+   else
+   args->lod = true;
break;
case nir_texop_txs:
case nir_texop_query_levels:
@@ -4203,7 +4207,7 @@ static void visit_tex(struct nir_to_llvm_context *ctx, 
nir_tex_instr *instr)
LLVMValueRef derivs[6];
unsigned chan, count = 0;
unsigned const_src = 0, num_deriv_comp = 0;
-
+   bool lod_is_zero = false;
tex_fetch_ptrs(ctx, instr, _ptr, _ptr, _ptr);
 
for (unsigned i = 0; i < instr->num_srcs; i++) {
@@ -4223,9 +4227,14 @@ static void visit_tex(struct nir_to_llvm_context *ctx, 
nir_tex_instr *instr)
case nir_tex_src_bias:
bias = get_src(ctx, instr->src[i].src);
break;
-   case nir_tex_src_lod:
+   case nir_tex_src_lod: {
+   nir_const_value *val = 
nir_src_as_const_value(instr->src[i].src);
+
+   if (val && val->i32[0] == 0)
+   lod_is_zero = true;
lod = get_src(ctx, instr->src[i].src);
break;
+   }
case nir_tex_src_ms_index:
sample_index = get_src(ctx, instr->src[i].src);
break;
@@ -4370,7 +4379,8 @@ static void visit_tex(struct nir_to_llvm_context *ctx, 
nir_tex_instr *instr)
}
 
/* Pack LOD */
-   if ((instr->op == nir_texop_txl || instr->op == nir_texop_txf) && lod) {
+   if (lod && ((instr->op == nir_texop_txl && !lod_is_zero) ||
+   instr->op == nir_texop_txf)) {
address[count++] = lod;
} else if (instr->op == nir_texop_txf_ms && sample_index) {
address[count++] = sample_index;
@@ -4401,7 +4411,7 @@ static void visit_tex(struct nir_to_llvm_context *ctx, 
nir_tex_instr *instr)
   fmask_ptr, NULL,
   txf_address, txf_count, 0xf);
 
-   result = build_tex_intrinsic(ctx, instr, _args);
+   result = build_tex_intrinsic(ctx, instr, false, _args);
 
result = LLVMBuildExtractElement(ctx->builder, result, 
ctx->i32zero, "");
result = emit_int_cmp(ctx, LLVMIntEQ, result, ctx->i32zero);
@@ -4446,7 +4456,7 @@ static void visit_tex(struct nir_to_llvm_context *ctx, 
nir_tex_instr *instr)
set_tex_fetch_args(ctx, , instr, instr->op,
   res_ptr, samp_ptr, address, count, dmask);
 
-   result = build_tex_intrinsic(ctx, instr, );
+   result = build_tex_intrinsic(ctx, instr, lod_is_zero, );
 
if (instr->op == nir_texop_query_levels)
result = LLVMBuildExtractElement(ctx->builder, result, 
LLVMConstInt(ctx->i32, 3, false), "");
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v5 0/3] asynchronous pbo transfer with glthread

2017-04-20 Thread gregory hainaut
On Thu, 20 Apr 2017 20:01:00 +0200
Marek Olšák  wrote:

> On Thu, Apr 20, 2017 at 6:53 PM, gregory hainaut
>  wrote:
> > On Thu, 20 Apr 2017 11:57:08 +0200
> > Marek Olšák  wrote:
> >
> >> On Thu, Apr 20, 2017 at 10:28 AM, gregory hainaut
> >>  wrote:
> >> > On Thu, 20 Apr 2017 12:29:11 +0900
> >> > Michel Dänzer  wrote:
> >> >
> >> >> On 20/04/17 01:54 AM, Gregory Hainaut wrote:
> >> >> > Hello,
> >> >> >
> >> >> > Please find the latest version that include a small fix for hash 
> >> >> > deletion. I
> >> >> > think the series is good now. Please review/ack it.
> >> >>
> >> >> I'm afraid I have to NACK it. As discussed in the v4 cover letter
> >> >> thread, Mesa's glthread cannot make any libX11 API calls.
> >> >>
> >> >>
> >> >
> >> > Hello Michel,
> >> >
> >> > Just to be sure we are on the same line, let's me do a summary.
> >> >
> >> > PCSX2 does the following bad pattern
> >> > 1/ no call to XInitThread
> >> > 2/ XGetGeometry to get the size of the surface
> >> > 3/ glDrawArray into the default framebuffer 0 (the window, I'm not sure 
> >> > how to call it)
> >> >=> which seem to call DRI2GetBuffersWithFormat under the hood. I 
> >> > guess to get the
> >> >   associated buffer of the window.
> >> >
> >> >
> >> > So far it was (kind of) working fine because PCSX2 does tons of PBO 
> >> > transfer. So glthread
> >> > was mostly synchronous.
> >> >
> >> > This series removes the (useless) PBO transfer synchronization. So now 
> >> > glthread is really
> >> > asynchronous and the above bad pattern crash as expected.
> >> >
> >> > I didn't add any libX11 API call on the patches. And I don't think we 
> >> > can remvove the DRI stuff.
> >> >
> >> > Hum, I don't know what will be the impact on the perf but potentially we 
> >> > can force a synchronization
> >> > when there is a draw to framebuffer 0.
> >>
> >> Can you send us the backtrace when DRI2GetBuffersWithFormat is called
> >> by glDrawArrays?
> >>
> >> Marek
> >
> > Hello Marek, Michel,
> >
> > Here the full backtrace.
> >
> > #0  DRI2GetBuffersWithFormat (dpy=0xc6307e48, drawable=104857784, 
> > width=0xdef348c0, height=0xdef348c4, attachments=0xdcc15c88, count=1, 
> > outCount=0xdcc15c74) at dri2.c:464
> > #1  0xf05bac45 in dri2GetBuffersWithFormat (driDrawable=0xdef348a8, 
> > width=0xdef348c0, height=0xdef348c4, attachments=0xdcc15c88, count=1, 
> > out_count=0xdcc15c74, loaderPrivate=0xdefc6ef0) at dri2_glx.c:894
> > #2  0xe3ec3111 in dri2_drawable_get_buffers (count=, 
> > atts=0xdef252f8, drawable=0xdefc7b08) at dri2.c:285
> > #3  dri2_allocate_textures (ctx=0xdef15928, drawable=0xdefc7b08, 
> > statts=0xdef252f8, statts_count=2) at dri2.c:480
> > #4  0xe3ebcbb0 in dri_st_framebuffer_validate (stctx=0xdef4ab10, 
> > stfbi=0xdefc7b08, statts=0xdef252f8, count=2, out=0xdcc15d78) at 
> > dri_drawable.c:83
> > #5  0xe3d37afc in st_framebuffer_validate (stfb=stfb@entry=0xdef24f58, 
> > st=st@entry=0xdef4ab10) at state_tracker/st_manager.c:189
> > Note "print stfb->Base->Name" give me 0
> > #6  0xe3d38649 in st_manager_validate_framebuffers (st=0xdef4ab10) at 
> > state_tracker/st_manager.c:869
> > #7  0xe3cd0580 in st_validate_state (st=0xdef4ab10, 
> > pipeline=ST_PIPELINE_RENDER) at state_tracker/st_atom.c:174
> > #8  0xe3cf935d in st_draw_vbo (ctx=0xdef7f8f8, prims=0xdcc15f40, 
> > nr_prims=1, ib=0x0, index_bounds_valid=1 '\001', min_index=39911, 
> > max_index=39914, tfb_vertcount=0x0, stream=0, indirect=0x0) at 
> > state_tracker/st_draw.c:191
> > #9  0xe3caf35e in vbo_draw_arrays (baseInstance=0, numInstances=1, count=4, 
> > start=39911, mode=5, ctx=0xdef7f8f8) at vbo/vbo_exec_array.c:427
> > #10 vbo_exec_DrawArrays (mode=5, start=39911, count=4) at 
> > vbo/vbo_exec_array.c:575
> > #11 0xe3be4f07 in _mesa_unmarshal_DrawArrays (ctx=0xdef7f8f8, 
> > cmd=0xc41574b0) at main/marshal_generated.c:26644
> > #12 _mesa_unmarshal_dispatch_cmd (ctx=0xdef7f8f8, cmd=0xc41574b0) at 
> > main/marshal_generated.c:42457
> > #13 0xe3ba8af0 in glthread_unmarshal_batch (release_batch=true, 
> > batch=0xc4157410, ctx=0xdef7f8f8) at main/glthread.c:64
> > #14 glthread_worker (data=0xdef7f8f8) at main/glthread.c:110
> >
> >
> >> If this DRI2GetBuffersWithFormat call results in libX11 API calls on the
> >> glthread, that's a bug which needs to be fixed, either by moving the
> >> DRI2GetBuffersWithFormat call to the main thread or (if possible) by
> >> changing DRI2GetBuffersWithFormat to use XCB directly.
> 
> First, we need to understand why it's happening. Does the app use the
> front buffer? Does it ever call glDrawBuffer(GL_FRONT) or
> glReadBuffer(GL_FRONT)?

Hello Marek,

No, I don't use the front buffer. I don't call glDrawBuffer for the FB 0. So I 
think,
it should use GL_BACK.
Hum except if some 3rparty libs do something in my back.

In case it have any impact, I'm using either glXSwapIntervalEXT or 
glXSwapIntervalMESA (if the 

Re: [Mesa-dev] [PATCH] RFC: mesa: remove MESA_VERBOSE env variable

2017-04-20 Thread Samuel Pitoiset
I have used it sometimes, but since VERBOSE_API is missing in a bunch of 
places, that's quite useless. :)


On 04/20/2017 04:12 PM, Emil Velikov wrote:

From: Emil Velikov 

The env variable was useful in the early days of mesa development, when
tools such at Apitrace were not available.

Nowadays, out of the 12 options only a third are used. With nearly all
instances printing the API state. And even then not all entry points
are annotated, thus one cannot rely upon it.

The current patch removes the variable all together alongside a few
instances (listed below) which developers may see value in keeping.

FLUSH_VERTICES/FLUSH_CURRENT macros in src/mesa/main/context.h
_mesa_GetGraphicsResetStatusARB in src/mesa/main/robustness.c
_mesa_print_state() in src/mesa/main/state.c
_mesa_notifySwapBuffers() in src/mesa/main/context.c

Do we want to keep any of the MESA_VERBOSE instances? If so documenting
the variable seems reasonable.

Cc: Brian Paul 
Signed-off-by: Emil Velikov 
---
Brian, I assume that you're the author for most of these. How do you
feel on the topic?
---
  src/mesa/main/attrib.c  |   8 ---
  src/mesa/main/blend.c   |  40 
  src/mesa/main/blit.c|  17 -
  src/mesa/main/bufferobj.c   |  38 ---
  src/mesa/main/buffers.c |   7 ---
  src/mesa/main/clear.c   |   3 -
  src/mesa/main/compute.c |  13 
  src/mesa/main/context.c |  19 +-
  src/mesa/main/context.h |   4 --
  src/mesa/main/copyimage.c   |  10 ---
  src/mesa/main/debug.c   |  42 -
  src/mesa/main/depth.c   |  12 
  src/mesa/main/dlist.c   |  17 -
  src/mesa/main/drawpix.c |  21 ---
  src/mesa/main/enable.c  |   6 --
  src/mesa/main/fbobject.c|  32 --
  src/mesa/main/feedback.c|   3 -
  src/mesa/main/getstring.c   |   6 --
  src/mesa/main/hint.c|   5 --
  src/mesa/main/light.c   |  14 -
  src/mesa/main/lines.c   |   6 --
  src/mesa/main/matrix.c  |  29 -
  src/mesa/main/mtypes.h  |  22 ---
  src/mesa/main/performance_monitor.c |   6 --
  src/mesa/main/pipelineobj.c |  33 --
  src/mesa/main/polygon.c |  23 ---
  src/mesa/main/program_resource.c|  33 --
  src/mesa/main/queryobj.c|  31 -
  src/mesa/main/readpix.c |   7 ---
  src/mesa/main/robustness.c  |  14 +
  src/mesa/main/samplerobj.c  |   3 -
  src/mesa/main/scissor.c |  11 
  src/mesa/main/shaderapi.c   |  16 -
  src/mesa/main/state.c   |   3 -
  src/mesa/main/stencil.c |  27 
  src/mesa/main/texenv.c  |   7 ---
  src/mesa/main/texgen.c  |   7 ---
  src/mesa/main/texgetimage.c |  17 -
  src/mesa/main/teximage.c|  52 ---
  src/mesa/main/texobj.c  |  29 -
  src/mesa/main/texstate.c|   8 ---
  src/mesa/main/texstorage.c  |  13 
  src/mesa/main/textureview.c |   6 --
  src/mesa/main/varray.c  |   6 --
  src/mesa/main/viewport.c|  28 -
  src/mesa/tnl/t_vb_render.c  |   5 --
  src/mesa/vbo/vbo_exec_array.c   | 122 
  47 files changed, 2 insertions(+), 879 deletions(-)

diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
index 8e738c91c8f..bb230404d29 100644
--- a/src/mesa/main/attrib.c
+++ b/src/mesa/main/attrib.c
@@ -245,9 +245,6 @@ _mesa_PushAttrib(GLbitfield mask)
  
 GET_CURRENT_CONTEXT(ctx);
  
-   if (MESA_VERBOSE & VERBOSE_API)

-  _mesa_debug(ctx, "glPushAttrib %x\n", (int) mask);
-
 if (ctx->AttribStackDepth >= MAX_ATTRIB_STACK_DEPTH) {
_mesa_error( ctx, GL_STACK_OVERFLOW, "glPushAttrib" );
return;
@@ -935,11 +932,6 @@ _mesa_PopAttrib(void)
  
 while (attr) {
  
-  if (MESA_VERBOSE & VERBOSE_API) {

- _mesa_debug(ctx, "glPopAttrib %s\n",
- _mesa_enum_to_string(attr->kind));
-  }
-
switch (attr->kind) {
   case DUMMY_BIT:
  /* do nothing */
diff --git a/src/mesa/main/blend.c b/src/mesa/main/blend.c
index 955fda1158c..52c473a9d8b 100644
--- a/src/mesa/main/blend.c
+++ b/src/mesa/main/blend.c
@@ -220,13 +220,6 @@ _mesa_BlendFuncSeparate( GLenum sfactorRGB, GLenum 
dfactorRGB,
 unsigned buf;
 bool changed = false;
  
-   if (MESA_VERBOSE & VERBOSE_API)

-  _mesa_debug(ctx, "glBlendFuncSeparate %s %s %s %s\n",
-  _mesa_enum_to_string(sfactorRGB),
-  _mesa_enum_to_string(dfactorRGB),
-  _mesa_enum_to_string(sfactorA),
-  _mesa_enum_to_string(dfactorA));
-

Re: [Mesa-dev] [PATCH] RFC: mesa: remove MESA_VERBOSE env variable

2017-04-20 Thread Eric Anholt
Emil Velikov  writes:

> From: Emil Velikov 
>
> The env variable was useful in the early days of mesa development, when
> tools such at Apitrace were not available.
>
> Nowadays, out of the 12 options only a third are used. With nearly all
> instances printing the API state. And even then not all entry points
> are annotated, thus one cannot rely upon it.
>
> The current patch removes the variable all together alongside a few
> instances (listed below) which developers may see value in keeping.
>
> FLUSH_VERTICES/FLUSH_CURRENT macros in src/mesa/main/context.h
> _mesa_GetGraphicsResetStatusARB in src/mesa/main/robustness.c
> _mesa_print_state() in src/mesa/main/state.c
> _mesa_notifySwapBuffers() in src/mesa/main/context.c
>
> Do we want to keep any of the MESA_VERBOSE instances? If so documenting
> the variable seems reasonable.

FWIW, I'm happy to see VERBOSE_API go. I've never used it, because I
don't trust it.  If we wanted to support it, I'd rather we worked on
some sort of dispatch table stacking so that we could code-gen the
VERBOSE_API printfs.


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [RFC] - Rewrite mesa website in Sphinx

2017-04-20 Thread Brian Paul

On 04/19/2017 05:23 AM, Emil Velikov wrote:

On 11 April 2017 at 13:10, Jean Hertel  wrote:

Any more thoughts on this?


I would really appreciate feedback from more contributors.


Indeed.

Brian, Eric, others - how do you feel with the Sphinx edition of the site?
If the current theme feels a bit off there's others available [1].
Then again, that can be changed/polished at any later stage.


I guess it looks OK to me.  I was looking more closely at Sphinx markup 
and it seems pretty simple.


I think we'll want the mesa3d.org website to auto-update when someone 
checks in changes to the Sphinx files.  Hopefully, the fd.o admins can 
rig that up without too much trouble.


One note: I've always referred to the project as just "Mesa" or "The 
Mesa 3-D Graphics Library".  I only used mesa3d.org for the website 
because mesa.org was taken.  So I'd suggest replacing the string "Mesa 
3D" with one of the others.


Also, could we use a slightly smaller font?  The text seems a bit larger 
than necessary.


-Brian

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [RFC 2/2] glx: drop GLX_MESA_multithread_makecurrent support

2017-04-20 Thread Eric Anholt
Timothy Arceri  writes:

> On 20/04/17 10:53, Eric Anholt wrote:
>> Timothy Arceri  writes:
>> 
>>> This extension is not supported by GLVND, also as far
>>> as I can tell this extension requires us to do extra
>>> locking for objects that are not normaly shared across
>>> contexts, like vertex array and pipeline objects.
>> 
>> Can you explain how it would require extra locking?  The extension
>> requires that the user not enter the same GL context twice at the same
>> time -- is there something else missing?
>> 
>
> So you can't end up with tread 1 calling glGenVertexArrays() while 
> thread 2 does some other call that requires a lookup of the vertex array 
> object hash table (which could currently be in the middle of rehashing 
> from the insert in glGenVertexArrays())?
>
> I didn't see anything that forbids this. It's totally possible I'm 
> misunderstanding this extension, if you could set me straight that would 
> be great.

From the spec:

A direct rendering context may be
current to multiple threads, with synchronization of access to
the context thruogh the GL managed by the application through
mutexes.

The application must use a mutex (or something) to keep itself from
entering the context twice.


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] mesa/glthread: correctly comparae thread handles

2017-04-20 Thread Eric Anholt
Emil Velikov  writes:

> From: Emil Velikov 
>
> As mentioned in the manual - comparing pthread_t handles via the C
> comparison operator is incorrect and pthread_equal() should be used
> instead.

Seems fine

Reviewed-by: Eric Anholt 


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v4 0/3] asynchronous pbo transfer with glthread

2017-04-20 Thread gregory hainaut
On Thu, 20 Apr 2017 10:56:34 +0900
Michel Dänzer  wrote:

> On 20/04/17 01:43 AM, gregory hainaut wrote:
> > Hello All,
> > 
> > I ported PCSX2 to xcb (at least the non-glx part). Crash is gone :)
> > So I can send the v5 with the hash delete fix.
> > 
> > However, Mesa might receive crash bug report when glthread is enabled
> > on a random app that doesn't use xcb/XinitThread properly.
> 
> This means it's still a bug in Mesa, you're just working around it in
> your application.
> 
> As we've explained, Mesa's glthread cannot make any libX11 API calls.
>
 
Yes. And unfortunately the crash is still here (seem less often).

 
> > Maybe it would be better to always enable the XInitThread mode by default 
> > on the X11 library.
> > If performance of X11 is critical, it would be better to switch to xcb 
> > anyway.
> 
> There has been talk about making that change, but there's not even a
> specific plan yet for making it happen upstream. It doesn't change the
> situation with currently shipping libX11.
> 
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v5 0/3] asynchronous pbo transfer with glthread

2017-04-20 Thread Marek Olšák
On Thu, Apr 20, 2017 at 6:53 PM, gregory hainaut
 wrote:
> On Thu, 20 Apr 2017 11:57:08 +0200
> Marek Olšák  wrote:
>
>> On Thu, Apr 20, 2017 at 10:28 AM, gregory hainaut
>>  wrote:
>> > On Thu, 20 Apr 2017 12:29:11 +0900
>> > Michel Dänzer  wrote:
>> >
>> >> On 20/04/17 01:54 AM, Gregory Hainaut wrote:
>> >> > Hello,
>> >> >
>> >> > Please find the latest version that include a small fix for hash 
>> >> > deletion. I
>> >> > think the series is good now. Please review/ack it.
>> >>
>> >> I'm afraid I have to NACK it. As discussed in the v4 cover letter
>> >> thread, Mesa's glthread cannot make any libX11 API calls.
>> >>
>> >>
>> >
>> > Hello Michel,
>> >
>> > Just to be sure we are on the same line, let's me do a summary.
>> >
>> > PCSX2 does the following bad pattern
>> > 1/ no call to XInitThread
>> > 2/ XGetGeometry to get the size of the surface
>> > 3/ glDrawArray into the default framebuffer 0 (the window, I'm not sure 
>> > how to call it)
>> >=> which seem to call DRI2GetBuffersWithFormat under the hood. I guess 
>> > to get the
>> >   associated buffer of the window.
>> >
>> >
>> > So far it was (kind of) working fine because PCSX2 does tons of PBO 
>> > transfer. So glthread
>> > was mostly synchronous.
>> >
>> > This series removes the (useless) PBO transfer synchronization. So now 
>> > glthread is really
>> > asynchronous and the above bad pattern crash as expected.
>> >
>> > I didn't add any libX11 API call on the patches. And I don't think we can 
>> > remvove the DRI stuff.
>> >
>> > Hum, I don't know what will be the impact on the perf but potentially we 
>> > can force a synchronization
>> > when there is a draw to framebuffer 0.
>>
>> Can you send us the backtrace when DRI2GetBuffersWithFormat is called
>> by glDrawArrays?
>>
>> Marek
>
> Hello Marek, Michel,
>
> Here the full backtrace.
>
> #0  DRI2GetBuffersWithFormat (dpy=0xc6307e48, drawable=104857784, 
> width=0xdef348c0, height=0xdef348c4, attachments=0xdcc15c88, count=1, 
> outCount=0xdcc15c74) at dri2.c:464
> #1  0xf05bac45 in dri2GetBuffersWithFormat (driDrawable=0xdef348a8, 
> width=0xdef348c0, height=0xdef348c4, attachments=0xdcc15c88, count=1, 
> out_count=0xdcc15c74, loaderPrivate=0xdefc6ef0) at dri2_glx.c:894
> #2  0xe3ec3111 in dri2_drawable_get_buffers (count=, 
> atts=0xdef252f8, drawable=0xdefc7b08) at dri2.c:285
> #3  dri2_allocate_textures (ctx=0xdef15928, drawable=0xdefc7b08, 
> statts=0xdef252f8, statts_count=2) at dri2.c:480
> #4  0xe3ebcbb0 in dri_st_framebuffer_validate (stctx=0xdef4ab10, 
> stfbi=0xdefc7b08, statts=0xdef252f8, count=2, out=0xdcc15d78) at 
> dri_drawable.c:83
> #5  0xe3d37afc in st_framebuffer_validate (stfb=stfb@entry=0xdef24f58, 
> st=st@entry=0xdef4ab10) at state_tracker/st_manager.c:189
> Note "print stfb->Base->Name" give me 0
> #6  0xe3d38649 in st_manager_validate_framebuffers (st=0xdef4ab10) at 
> state_tracker/st_manager.c:869
> #7  0xe3cd0580 in st_validate_state (st=0xdef4ab10, 
> pipeline=ST_PIPELINE_RENDER) at state_tracker/st_atom.c:174
> #8  0xe3cf935d in st_draw_vbo (ctx=0xdef7f8f8, prims=0xdcc15f40, nr_prims=1, 
> ib=0x0, index_bounds_valid=1 '\001', min_index=39911, max_index=39914, 
> tfb_vertcount=0x0, stream=0, indirect=0x0) at state_tracker/st_draw.c:191
> #9  0xe3caf35e in vbo_draw_arrays (baseInstance=0, numInstances=1, count=4, 
> start=39911, mode=5, ctx=0xdef7f8f8) at vbo/vbo_exec_array.c:427
> #10 vbo_exec_DrawArrays (mode=5, start=39911, count=4) at 
> vbo/vbo_exec_array.c:575
> #11 0xe3be4f07 in _mesa_unmarshal_DrawArrays (ctx=0xdef7f8f8, cmd=0xc41574b0) 
> at main/marshal_generated.c:26644
> #12 _mesa_unmarshal_dispatch_cmd (ctx=0xdef7f8f8, cmd=0xc41574b0) at 
> main/marshal_generated.c:42457
> #13 0xe3ba8af0 in glthread_unmarshal_batch (release_batch=true, 
> batch=0xc4157410, ctx=0xdef7f8f8) at main/glthread.c:64
> #14 glthread_worker (data=0xdef7f8f8) at main/glthread.c:110
>
>
>> If this DRI2GetBuffersWithFormat call results in libX11 API calls on the
>> glthread, that's a bug which needs to be fixed, either by moving the
>> DRI2GetBuffersWithFormat call to the main thread or (if possible) by
>> changing DRI2GetBuffersWithFormat to use XCB directly.

First, we need to understand why it's happening. Does the app use the
front buffer? Does it ever call glDrawBuffer(GL_FRONT) or
glReadBuffer(GL_FRONT)?

>
> Ok. On one hand, moving DRI2GetBuffersWithFormat to main thread won't be 
> easy. I think we need to
> keep track of the current bound framebuffer on the app thread. So we can 
> force a sync
> on gl operation that will access the framebuffer 0.
>
> On the other hand, GLX/XLIB/XCB interactions are  quite foggy for me. It seem 
> there already some xcb codes
> in various place typically EGL and the xcb_dri2 extension. So maybe there is 
> a reason that code wasn't ported here.
> In particular, I saw this option in configure
>   

Re: [Mesa-dev] [RFC PATCH 03/26] glsl: introduce new base types for bindless samplers/images

2017-04-20 Thread Samuel Pitoiset



On 04/20/2017 07:47 PM, Nicolai Hähnle wrote:

On 20.04.2017 16:54, Samuel Pitoiset wrote:
[snip]
Yes, contains_opaque() not is_opaque(). Well, how do you plan to 
handle

the fact that bindless sampler types are 64-bit? It seemed quite
logical
to make glsl_type::is_64bit() returns true for them.


So, for the purposes of src/compiler/glsl/, I think it simply
shouldn't matter whether is_64bit returns true or not. However, I
agree that for st/glsl_to_tgsi, it'd probably be very helpful to make
is_64bit return true for them.


One new restriction is glsl_type::component_slots(). With the new base
types it's easy to return something similar to uint64_t (ie. 2
components if it's BINDLESS_SAMPLER). However, without a separate type
it returns 0 for samplers and 1 for images.

glsl_type is a quite good interface for base types, but for bindless
sampler we just can't store the information there. Currently, the only
way to know if a sampler is bindless is to have access to a
ir_variable object.

I don't see how to solve this properly without adding hacks. The way
glsl_type is designed is one of the argument for the current solution.

Any thoughts?


It's allowed by the spec to count samplers/images as two components. So
component_slots() can return N*2 like uint64_t. However,
glsl_type::components() currently returns 0 for sampler types because
matrix_columns and vector_elements are 0 (while 1 for image types).

I think this needs to be clarified and components() should return 1 for
sampler and image types.


I agree that this makes more sense.

Just to bring some more of the offline discussion back to the mailing 
list, we also realized that in current master, every sampler uniform 
actually occupies 16 bytes (one vec4 slot) in the constant buffer, so 
having the component_slots() increased from 1 to 2 is not a regression 
in terms of performance or anything like that.


It'd make sense to improve the code that maps default block uniforms to 
driver memory in a way such that old-style samplers / bound_samplers do 
not take up any space at all (and while we're at it, we might want to 
pack uniforms that are smaller than a vec4), but I think that that's a 
separate concern from ARB_bindless_texture.


Sure, this can be improved later.



Cheers,
Nicolai

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [RFC PATCH 03/26] glsl: introduce new base types for bindless samplers/images

2017-04-20 Thread Nicolai Hähnle

On 20.04.2017 16:54, Samuel Pitoiset wrote:
[snip]

Yes, contains_opaque() not is_opaque(). Well, how do you plan to handle
the fact that bindless sampler types are 64-bit? It seemed quite
logical
to make glsl_type::is_64bit() returns true for them.


So, for the purposes of src/compiler/glsl/, I think it simply
shouldn't matter whether is_64bit returns true or not. However, I
agree that for st/glsl_to_tgsi, it'd probably be very helpful to make
is_64bit return true for them.


One new restriction is glsl_type::component_slots(). With the new base
types it's easy to return something similar to uint64_t (ie. 2
components if it's BINDLESS_SAMPLER). However, without a separate type
it returns 0 for samplers and 1 for images.

glsl_type is a quite good interface for base types, but for bindless
sampler we just can't store the information there. Currently, the only
way to know if a sampler is bindless is to have access to a
ir_variable object.

I don't see how to solve this properly without adding hacks. The way
glsl_type is designed is one of the argument for the current solution.

Any thoughts?


It's allowed by the spec to count samplers/images as two components. So
component_slots() can return N*2 like uint64_t. However,
glsl_type::components() currently returns 0 for sampler types because
matrix_columns and vector_elements are 0 (while 1 for image types).

I think this needs to be clarified and components() should return 1 for
sampler and image types.


I agree that this makes more sense.

Just to bring some more of the offline discussion back to the mailing 
list, we also realized that in current master, every sampler uniform 
actually occupies 16 bytes (one vec4 slot) in the constant buffer, so 
having the component_slots() increased from 1 to 2 is not a regression 
in terms of performance or anything like that.


It'd make sense to improve the code that maps default block uniforms to 
driver memory in a way such that old-style samplers / bound_samplers do 
not take up any space at all (and while we're at it, we might want to 
pack uniforms that are smaller than a vec4), but I think that that's a 
separate concern from ARB_bindless_texture.


Cheers,
Nicolai
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/2] i965/vec4: set swizzle when loading an uniform

2017-04-20 Thread Francisco Jerez
Samuel Iglesias Gonsálvez  writes:

> It was setting XYWZ swizzle to all uniforms, no matter if they were
> a vector or not.
>
> Signed-off-by: Samuel Iglesias Gonsálvez 
> Cc: curroje...@riseup.net

Don't you need to CC mesa-stable here and in the next patch?

> ---
>  src/intel/compiler/brw_vec4_nir.cpp | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/intel/compiler/brw_vec4_nir.cpp 
> b/src/intel/compiler/brw_vec4_nir.cpp
> index a82d52088a8..5f4488c7e86 100644
> --- a/src/intel/compiler/brw_vec4_nir.cpp
> +++ b/src/intel/compiler/brw_vec4_nir.cpp
> @@ -863,6 +863,7 @@ vec4_visitor::nir_emit_intrinsic(nir_intrinsic_instr 
> *instr)
>   unsigned offset = const_offset->u32[0] + shift * 4;
>   src.offset = ROUND_DOWN_TO(offset, 16);
>   shift = (offset % 16) / 4;
> + src.swizzle = brw_swizzle_for_size(instr->num_components);

What about the indirect case a few lines below?  Isn't the swizzle passed
to the mov indirect instruction still bogus?

>   src.swizzle += BRW_SWIZZLE4(shift, shift, shift, shift);
>  
>   emit(MOV(dest, src));
> -- 
> 2.11.0


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] RFC: mesa: remove MESA_VERBOSE env variable

2017-04-20 Thread Brian Paul

On 04/20/2017 08:12 AM, Emil Velikov wrote:

From: Emil Velikov 

The env variable was useful in the early days of mesa development, when
tools such at Apitrace were not available.

Nowadays, out of the 12 options only a third are used. With nearly all
instances printing the API state. And even then not all entry points
are annotated, thus one cannot rely upon it.

The current patch removes the variable all together alongside a few
instances (listed below) which developers may see value in keeping.

FLUSH_VERTICES/FLUSH_CURRENT macros in src/mesa/main/context.h
_mesa_GetGraphicsResetStatusARB in src/mesa/main/robustness.c
_mesa_print_state() in src/mesa/main/state.c
_mesa_notifySwapBuffers() in src/mesa/main/context.c

Do we want to keep any of the MESA_VERBOSE instances? If so documenting
the variable seems reasonable.

Cc: Brian Paul 
Signed-off-by: Emil Velikov 
---
Brian, I assume that you're the author for most of these. How do you
feel on the topic?


I think Keith was the original author of MESA_VERBOSE (but that may have 
been a clean-up of some of even earlier code of mine).


I haven't used this in years so I'm OK with removing it.

-Brian


---
  src/mesa/main/attrib.c  |   8 ---
  src/mesa/main/blend.c   |  40 
  src/mesa/main/blit.c|  17 -
  src/mesa/main/bufferobj.c   |  38 ---
  src/mesa/main/buffers.c |   7 ---
  src/mesa/main/clear.c   |   3 -
  src/mesa/main/compute.c |  13 
  src/mesa/main/context.c |  19 +-
  src/mesa/main/context.h |   4 --
  src/mesa/main/copyimage.c   |  10 ---
  src/mesa/main/debug.c   |  42 -
  src/mesa/main/depth.c   |  12 
  src/mesa/main/dlist.c   |  17 -
  src/mesa/main/drawpix.c |  21 ---
  src/mesa/main/enable.c  |   6 --
  src/mesa/main/fbobject.c|  32 --
  src/mesa/main/feedback.c|   3 -
  src/mesa/main/getstring.c   |   6 --
  src/mesa/main/hint.c|   5 --
  src/mesa/main/light.c   |  14 -
  src/mesa/main/lines.c   |   6 --
  src/mesa/main/matrix.c  |  29 -
  src/mesa/main/mtypes.h  |  22 ---
  src/mesa/main/performance_monitor.c |   6 --
  src/mesa/main/pipelineobj.c |  33 --
  src/mesa/main/polygon.c |  23 ---
  src/mesa/main/program_resource.c|  33 --
  src/mesa/main/queryobj.c|  31 -
  src/mesa/main/readpix.c |   7 ---
  src/mesa/main/robustness.c  |  14 +
  src/mesa/main/samplerobj.c  |   3 -
  src/mesa/main/scissor.c |  11 
  src/mesa/main/shaderapi.c   |  16 -
  src/mesa/main/state.c   |   3 -
  src/mesa/main/stencil.c |  27 
  src/mesa/main/texenv.c  |   7 ---
  src/mesa/main/texgen.c  |   7 ---
  src/mesa/main/texgetimage.c |  17 -
  src/mesa/main/teximage.c|  52 ---
  src/mesa/main/texobj.c  |  29 -
  src/mesa/main/texstate.c|   8 ---
  src/mesa/main/texstorage.c  |  13 
  src/mesa/main/textureview.c |   6 --
  src/mesa/main/varray.c  |   6 --
  src/mesa/main/viewport.c|  28 -
  src/mesa/tnl/t_vb_render.c  |   5 --
  src/mesa/vbo/vbo_exec_array.c   | 122 
  47 files changed, 2 insertions(+), 879 deletions(-)

diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
index 8e738c91c8f..bb230404d29 100644
--- a/src/mesa/main/attrib.c
+++ b/src/mesa/main/attrib.c
@@ -245,9 +245,6 @@ _mesa_PushAttrib(GLbitfield mask)

 GET_CURRENT_CONTEXT(ctx);

-   if (MESA_VERBOSE & VERBOSE_API)
-  _mesa_debug(ctx, "glPushAttrib %x\n", (int) mask);
-
 if (ctx->AttribStackDepth >= MAX_ATTRIB_STACK_DEPTH) {
_mesa_error( ctx, GL_STACK_OVERFLOW, "glPushAttrib" );
return;
@@ -935,11 +932,6 @@ _mesa_PopAttrib(void)

 while (attr) {

-  if (MESA_VERBOSE & VERBOSE_API) {
- _mesa_debug(ctx, "glPopAttrib %s\n",
- _mesa_enum_to_string(attr->kind));
-  }
-
switch (attr->kind) {
   case DUMMY_BIT:
  /* do nothing */
diff --git a/src/mesa/main/blend.c b/src/mesa/main/blend.c
index 955fda1158c..52c473a9d8b 100644
--- a/src/mesa/main/blend.c
+++ b/src/mesa/main/blend.c
@@ -220,13 +220,6 @@ _mesa_BlendFuncSeparate( GLenum sfactorRGB, GLenum 
dfactorRGB,
 unsigned buf;
 bool changed = false;

-   if (MESA_VERBOSE & VERBOSE_API)
-  _mesa_debug(ctx, "glBlendFuncSeparate %s %s %s %s\n",
-  _mesa_enum_to_string(sfactorRGB),
-  _mesa_enum_to_string(dfactorRGB),
-  

Re: [Mesa-dev] [PATCH 1/2] c11/threads: implement thrd_current on Windows

2017-04-20 Thread Brian Paul

Series LGTM.  Jose should probably take a look too, though.

One typo below?

Reviewed-by: Brian Paul 

On 04/20/2017 08:15 AM, Emil Velikov wrote:

From: Emil Velikov 

Earlier commit removed the implementation due to a subtle bug, in the
existing code. Namely:

One was comparing directly the thread handle/IDs rather than using
thrd_equal(). As the pseudo-handles never matched things went south.

That bug was resolved with commit 458c7490c29 "mapi: rewrite
u_current_init() function without u_thread_self()" thus we can bring the
thrd_current implementation back, whist preserving the correct


s/whist/while/ ?


thrd_equal implementation as-is.

With this in place, we can remove a handful of the remaining pthread vs
WIN32 specifics.

Cc: Brian Paul 
Cc: José Fonseca 
Signed-off-by: Emil Velikov 
---
  include/c11/threads_win32.h | 35 ++-
  1 file changed, 6 insertions(+), 29 deletions(-)

diff --git a/include/c11/threads_win32.h b/include/c11/threads_win32.h
index d017c31c34e..f0eb5c77c60 100644
--- a/include/c11/threads_win32.h
+++ b/include/c11/threads_win32.h
@@ -494,42 +494,19 @@ thrd_create(thrd_t *thr, thrd_start_t func, void *arg)
  return thrd_success;
  }

-#if 0
  // 7.25.5.2
  static inline thrd_t
  thrd_current(void)
  {
-HANDLE hCurrentThread;
-BOOL bRet;
-
-/* GetCurrentThread() returns a pseudo-handle, which is useless.  We need
- * to call DuplicateHandle to get a real handle.  However the handle value
- * will not match the one returned by thread_create.
- *
- * Other potential solutions would be:
- * - define thrd_t as a thread Ids, but this would mean we'd need to 
OpenThread for many operations
- * - use malloc'ed memory for thrd_t. This would imply using TLS for 
current thread.
- *
- * Neither is particularly nice.
+/* GetCurrentThread() returns a pseudo-handle, which will not match the
+ * one returned by thread_create.
   *
- * Life would be much easier if C11 threads had different abstractions for
- * threads and thread IDs, just like C++11 threads does...
+ * At the same time, the only way that one should compare thread handle
+ * and/or IDs is via thrd_equal. That in itself attributes for the above
+ * situation.
   */
-
-bRet = DuplicateHandle(GetCurrentProcess(), // source process (pseudo) 
handle
-   GetCurrentThread(), // source (pseudo) handle
-   GetCurrentProcess(), // target process
-   , // target handle
-   0,
-   FALSE,
-   DUPLICATE_SAME_ACCESS);
-assert(bRet);
-if (!bRet) {
-   hCurrentThread = GetCurrentThread();
-}
-return hCurrentThread;
+return GetCurrentThread();
  }
-#endif

  // 7.25.5.3
  static inline int



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] mesa/glthread: correctly comparae thread handles

2017-04-20 Thread Brian Paul

In the subject line "compare"

Reviewed-by: Brian Paul 


On 04/20/2017 09:24 AM, Emil Velikov wrote:

From: Emil Velikov 

As mentioned in the manual - comparing pthread_t handles via the C
comparison operator is incorrect and pthread_equal() should be used
instead.

Cc: Timothy Arceri 
Cc: Eric Anholt 
Fixes: d8d81fbc316 ("mesa: Add infrastructure for a worker thread to process GL 
commands.")
Signed-off-by: Emil Velikov 
---
  src/mesa/main/glthread.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/main/glthread.c b/src/mesa/main/glthread.c
index c4d3f4a4349..455b829cd8d 100644
--- a/src/mesa/main/glthread.c
+++ b/src/mesa/main/glthread.c
@@ -265,7 +265,7 @@ _mesa_glthread_finish(struct gl_context *ctx)
  * dri interface entrypoints), in which case we don't need to actually
  * synchronize against ourself.
  */
-   if (pthread_self() == glthread->thread)
+   if (pthread_equal(pthread_self(), glthread->thread))
return;

 pthread_mutex_lock(>mutex);



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 02/12] i965/cnl: Add gen10.xml

2017-04-20 Thread Anuj Phogat
On Thu, Apr 20, 2017 at 9:55 AM, Emil Velikov  wrote:
> Hi Anuj,
>
> On 15 April 2017 at 01:35, Anuj Phogat  wrote:
>> From: Jason Ekstrand 
>>
>> Signed-off-by: Anuj Phogat 
>> ---
>>  src/intel/Makefile.sources |3 +-
>>  src/intel/genxml/gen10.xml | 3557 
>> 
>>  2 files changed, 3559 insertions(+), 1 deletion(-)
>>  create mode 100644 src/intel/genxml/gen10.xml
>>
>> diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources
>> index c568916..57eb157 100644
>> --- a/src/intel/Makefile.sources
>> +++ b/src/intel/Makefile.sources
>> @@ -117,7 +117,8 @@ GENXML_XML_FILES = \
>> genxml/gen7.xml \
>> genxml/gen75.xml \
>> genxml/gen8.xml \
>> -   genxml/gen9.xml
>> +   genxml/gen9.xml \
>> +   genxml/gen10.xml
>>
> Please add a few lines analogous to src/intel/Android.genxml.mk.
> Copy the gen9.xml hunk and s/gen9/gen10/.
>
Yes, I'll update the Android.genxml.mk.

> Thanks
> Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 100693] 'identifier "L__FUNCTION__" is undefined' error for Debug builds using ICC on Windows

2017-04-20 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=100693

--- Comment #8 from sav...@ukr.net ---
ICC Developers figured out, that source of this issue is code:
/* XXX: Use standard `__func__` instead */
#ifndef __FUNCTION__
#  define __FUNCTION__ __func__
#endif

in file 'src/gallium/include/pipe/p_compiler.h'.

If commented, MESA Debug build using ICC finishes successfully. Expected to be
fixed in future ICC releases
(https://software.intel.com/en-us/forums/intel-c-compiler/topic/731357#comment-1903194).

-- 
You are receiving this mail because:
You are the assignee for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] mesa/glthread: correctly comparae thread handles

2017-04-20 Thread Manolova, Plamena
Looks good to me :)

Reviewed-by: Plamena Manolova 

On Thu, Apr 20, 2017 at 8:24 AM, Emil Velikov 
wrote:

> From: Emil Velikov 
>
> As mentioned in the manual - comparing pthread_t handles via the C
> comparison operator is incorrect and pthread_equal() should be used
> instead.
>
> Cc: Timothy Arceri 
> Cc: Eric Anholt 
> Fixes: d8d81fbc316 ("mesa: Add infrastructure for a worker thread to
> process GL commands.")
> Signed-off-by: Emil Velikov 
> ---
>  src/mesa/main/glthread.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/mesa/main/glthread.c b/src/mesa/main/glthread.c
> index c4d3f4a4349..455b829cd8d 100644
> --- a/src/mesa/main/glthread.c
> +++ b/src/mesa/main/glthread.c
> @@ -265,7 +265,7 @@ _mesa_glthread_finish(struct gl_context *ctx)
>  * dri interface entrypoints), in which case we don't need to actually
>  * synchronize against ourself.
>  */
> -   if (pthread_self() == glthread->thread)
> +   if (pthread_equal(pthread_self(), glthread->thread))
>return;
>
> pthread_mutex_lock(>mutex);
> --
> 2.12.2
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 30/35] i965: Port gen6+ emit vertices code to genxml.

2017-04-20 Thread Kenneth Graunke
On Wednesday, April 19, 2017 4:56:23 PM PDT Rafael Antognolli wrote:
[snip]
> +#ifndef NDEBUG
> +static bool
> +is_passthru_format(uint32_t format)

Wrapping this in ifndef NDEBUG breaks release builds - for example,
genX(uploads_needed) uses it outside of an assertion.  If you want to
shut up warnings about the unused function, you can declare it as
"UNUSED static bool".

> +{
> +   switch (format) {
> +   case ISL_FORMAT_R64_PASSTHRU:
> +   case ISL_FORMAT_R64G64_PASSTHRU:
> +   case ISL_FORMAT_R64G64B64_PASSTHRU:
> +   case ISL_FORMAT_R64G64B64A64_PASSTHRU:
> +  return true;
> +   default:
> +  return false;
> +   }
> +}
> +#endif
> +
> +#if GEN_GEN < 8
> +static int
> +genX(uploads_needed)(uint32_t format)
> +{
> +   if (!is_passthru_format(format))
> +  return 1;
> +
> +   switch (format) {
> +   case ISL_FORMAT_R64_PASSTHRU:
> +   case ISL_FORMAT_R64G64_PASSTHRU:
> +  return 1;
> +   case ISL_FORMAT_R64G64B64_PASSTHRU:
> +   case ISL_FORMAT_R64G64B64A64_PASSTHRU:
> +  return 2;
> +   default:
> +  unreachable("not reached");
> +   }
> +}
> +
> +/*
> + * Returns the format that we are finally going to use when upload a vertex
> + * element. It will only change if we are using *64*PASSTHRU formats, as for
> + * gen < 8 they need to be splitted on two *32*FLOAT formats.
> + *
> + * @upload points in which upload we are. Valid values are [0,1]
> + */
> +static uint32_t
> +downsize_format_if_needed(uint32_t format,
> +  int upload)
> +{
> +   assert(upload == 0 || upload == 1);
> +
> +   if (!is_passthru_format(format))
> +  return format;
> +
> +   switch (format) {
> +   case ISL_FORMAT_R64_PASSTHRU:
> +  return ISL_FORMAT_R32G32_FLOAT;
> +   case ISL_FORMAT_R64G64_PASSTHRU:
> +  return ISL_FORMAT_R32G32B32A32_FLOAT;
> +   case ISL_FORMAT_R64G64B64_PASSTHRU:
> +  return !upload ? ISL_FORMAT_R32G32B32A32_FLOAT
> + : ISL_FORMAT_R32G32_FLOAT;
> +   case ISL_FORMAT_R64G64B64A64_PASSTHRU:
> +  return ISL_FORMAT_R32G32B32A32_FLOAT;
> +   default:
> +  unreachable("not reached");
> +   }
> +}


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] question about container_of

2017-04-20 Thread Kristian H. Kristensen
Emil Velikov  writes:

> On 18 April 2017 at 13:55, Pekka Paalanen  wrote:
>> On Mon, 27 Feb 2017 13:26:11 +
>> Emil Velikov  wrote:
>>
>>> Hi Julien,
>>>
>>> On 27 February 2017 at 12:08, Julien Isorce  wrote:
>>> > Hi,
>>> >
>>> > Since 2012 commit ccff74971203b533bf16b46b49a9e61753f75e6c it is said:
>>> > "sample must be initialized, or else the result is undefined" in the
>>> > description of mesa/src/util/list.h::container_of .
>>> >
>>> > But I can find a few places where it is used without initializing that
>>> > second parameter, i.e. like:
>>> >
>>> > struct A a;
>>> > container_of(ptr, a, member);
>>> >
>>> > Then I can add the "= NULL" but should it be just
>>> > container_of(ptr, struct A, member);
>>> > like in the kernel and some other places in mesa ?
>>> >
>>> Strictly peaking these are toolchain (ASAN iirc) bugs, since there is
>>> no pointer deref, as we're doing pointer arithmetic.
>>
>> Hi Emil,
>>
>> that's what people would usually think. It used to work with GCC. Then
>> came Clang.
>>
>>> Afaict the general decision was to to merge the patch(es) since they
>>> will make actual bugs stand out amongst the noise. In the long run,
>>> it's better to fix the tool (ASAN/other) than trying to "fix" all the
>>> cases in mesa and dozens of other projects. But until then patches are
>>> safe enough ;-)
>>>
>>> That's my take on it, at least.
>>
>> It depends on how container_of() has been defined. For more details,
>> see e.g.:
>> https://cgit.freedesktop.org/wayland/wayland/commit/?id=a18e34417ba3fefeb81d891235e8ebf394a20a74
>>
>> The comment in the code in Mesa is correct for the fallback
>> implementation it has. Maybe things rely on the fallback implementation
>> never being used when compiling with Clang.
>>
>> Julien, some alternatives for container_of use a pointer, others expect
>> a type instead. I believe one must use the correct form.
>>
> Thanks for the correction Pekka - yes, the issue (to deref or not) is
> implementation specific.
> A 'minor' detail that I've missed.

The issue isn't whether it derefs the pointer or not. The undefined
version looks like this:

 #define wl_container_of(ptr, sample, member)
(__typeof__(sample))((char *)(ptr)  -
 ((char *)&(sample)->member - (char *)(sample)))

and the problem is that it expects an uninitialized variable (sample) to
have consistent values across two references ( (char *)&(sample)->member and
(char *)sample ), but referencing an uninitialized variable even once is
undefined behavior.

Kristian

> Seems like we use the more prone one - having the second argument
> being a pointer as, opposed to a type.
> Perhaps we should reconsider and update mesa, sooner rather than later.
>
> Thanks again!
> Emil
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 1/2] glsl: make component_slots() returns 1 for sampler types

2017-04-20 Thread Samuel Pitoiset
It looks inconsistent to return 1 for image types and 0 for
sampler types. Especially because component_slots() is mostly
used by values_for_type() which always returns 1 for samplers.

For bindless, this value will be bumped to 2 because the
ARB_bindless_texture states that bindless samplers/images
should consume two components.

Signed-off-by: Samuel Pitoiset 
---
 src/compiler/glsl_types.cpp | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp
index db65bb0e00..0480bef80e 100644
--- a/src/compiler/glsl_types.cpp
+++ b/src/compiler/glsl_types.cpp
@@ -1296,13 +1296,12 @@ glsl_type::component_slots() const
case GLSL_TYPE_ARRAY:
   return this->length * this->fields.array->component_slots();
 
+   case GLSL_TYPE_SAMPLER:
case GLSL_TYPE_IMAGE:
-  return 1;
case GLSL_TYPE_SUBROUTINE:
- return 1;
+  return 1;
 
case GLSL_TYPE_FUNCTION:
-   case GLSL_TYPE_SAMPLER:
case GLSL_TYPE_ATOMIC_UINT:
case GLSL_TYPE_VOID:
case GLSL_TYPE_ERROR:
-- 
2.12.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 2/2] glsl: get rid of values_for_type()

2017-04-20 Thread Samuel Pitoiset
This function is actually a wrapper for component_slots()
and it always returns 1 (or N) for samplers. Since
component_slots() now return 1 for samplers, it can go.

Signed-off-by: Samuel Pitoiset 
---
 src/compiler/glsl/link_uniforms.cpp | 19 ++-
 src/compiler/glsl/linker.h  |  3 ---
 src/compiler/glsl/shader_cache.cpp  |  4 ++--
 3 files changed, 4 insertions(+), 22 deletions(-)

diff --git a/src/compiler/glsl/link_uniforms.cpp 
b/src/compiler/glsl/link_uniforms.cpp
index c29fbed743..925699641e 100644
--- a/src/compiler/glsl/link_uniforms.cpp
+++ b/src/compiler/glsl/link_uniforms.cpp
@@ -42,21 +42,6 @@
  */
 #define UNMAPPED_UNIFORM_LOC ~0u
 
-/**
- * Count the backing storage requirements for a type
- */
-unsigned
-values_for_type(const glsl_type *type)
-{
-   if (type->is_sampler()) {
-  return 1;
-   } else if (type->is_array() && type->fields.array->is_sampler()) {
-  return type->array_size();
-   } else {
-  return type->component_slots();
-   }
-}
-
 void
 program_resource_visitor::process(const glsl_type *type, const char *name)
 {
@@ -351,7 +336,7 @@ private:
* uniform for multiple shader targets, but in this case we want to
* count it for each shader target.
*/
-  const unsigned values = values_for_type(type);
+  const unsigned values = type->component_slots();
   if (type->contains_subroutine()) {
  this->num_shader_subroutines += values;
   } else if (type->contains_sampler()) {
@@ -813,7 +798,7 @@ private:
   if (!this->uniforms[id].builtin &&
   !this->uniforms[id].is_shader_storage &&
   this->buffer_block_index == -1)
- this->values += values_for_type(type);
+ this->values += type->component_slots();
}
 
/**
diff --git a/src/compiler/glsl/linker.h b/src/compiler/glsl/linker.h
index d06f419cf6..dd627be5f1 100644
--- a/src/compiler/glsl/linker.h
+++ b/src/compiler/glsl/linker.h
@@ -75,9 +75,6 @@ void
 validate_interstage_uniform_blocks(struct gl_shader_program *prog,
gl_linked_shader **stages);
 
-unsigned
-values_for_type(const glsl_type *type);
-
 extern void
 link_assign_atomic_counter_resources(struct gl_context *ctx,
  struct gl_shader_program *prog);
diff --git a/src/compiler/glsl/shader_cache.cpp 
b/src/compiler/glsl/shader_cache.cpp
index 738e5488ac..19d69c36fc 100644
--- a/src/compiler/glsl/shader_cache.cpp
+++ b/src/compiler/glsl/shader_cache.cpp
@@ -599,7 +599,7 @@ write_uniforms(struct blob *metadata, struct 
gl_shader_program *prog)
   !prog->data->UniformStorage[i].is_shader_storage &&
   prog->data->UniformStorage[i].block_index == -1) {
  unsigned vec_size =
-values_for_type(prog->data->UniformStorage[i].type) *
+prog->data->UniformStorage[i].type->component_slots() *
 MAX2(prog->data->UniformStorage[i].array_elements, 1);
  blob_write_bytes(metadata, prog->data->UniformStorage[i].storage,
   sizeof(union gl_constant_value) * vec_size);
@@ -659,7 +659,7 @@ read_uniforms(struct blob_reader *metadata, struct 
gl_shader_program *prog)
   !prog->data->UniformStorage[i].is_shader_storage &&
   prog->data->UniformStorage[i].block_index == -1) {
  unsigned vec_size =
-values_for_type(prog->data->UniformStorage[i].type) *
+prog->data->UniformStorage[i].type->component_slots() *
 MAX2(prog->data->UniformStorage[i].array_elements, 1);
  blob_copy_bytes(metadata,
  (uint8_t *) prog->data->UniformStorage[i].storage,
-- 
2.12.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 0/2] glsl: some sort of preliminary work for bindless

2017-04-20 Thread Samuel Pitoiset
Hi,

Found by inspection while working on the GLSL bindless alternative
(ie. don't introduce new base types). A full piglit run doesn't
report any regressions using RadeonSI.

Please review,
Thanks!

Samuel Pitoiset (2):
  glsl: make component_slots() returns 1 for sampler types
  glsl: get rid of values_for_type()

 src/compiler/glsl/link_uniforms.cpp | 19 ++-
 src/compiler/glsl/linker.h  |  3 ---
 src/compiler/glsl/shader_cache.cpp  |  4 ++--
 src/compiler/glsl_types.cpp |  5 ++---
 4 files changed, 6 insertions(+), 25 deletions(-)

-- 
2.12.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 22/35] i965: Port gen6+ 3DSTATE_VS to genxml.

2017-04-20 Thread Kristian H. Kristensen
Rafael Antognolli  writes:

> Emit 3DSTATE_VS on Gen6+ using brw_batch_emit helper, that uses pack
> structs from genxml.
>
> Signed-off-by: Rafael Antognolli 
> ---
>  src/mesa/drivers/dri/i965/Makefile.sources|   2 +-
>  src/mesa/drivers/dri/i965/brw_state.h |   3 +-
>  src/mesa/drivers/dri/i965/gen6_vs_state.c | 113 +---
>  src/mesa/drivers/dri/i965/gen7_vs_state.c |  87 +---
>  src/mesa/drivers/dri/i965/gen8_vs_state.c |  96 +
>  src/mesa/drivers/dri/i965/genX_state_upload.c | 110 +-
>  6 files changed, 107 insertions(+), 304 deletions(-)
>  delete mode 100644 src/mesa/drivers/dri/i965/gen7_vs_state.c
>  delete mode 100644 src/mesa/drivers/dri/i965/gen8_vs_state.c
>
> diff --git a/src/mesa/drivers/dri/i965/Makefile.sources 
> b/src/mesa/drivers/dri/i965/Makefile.sources
> index 0f893d6..eec63f8 100644
> --- a/src/mesa/drivers/dri/i965/Makefile.sources
> +++ b/src/mesa/drivers/dri/i965/Makefile.sources
> @@ -102,7 +102,6 @@ i965_FILES = \
>   gen7_te_state.c \
>   gen7_urb.c \
>   gen7_viewport_state.c \
> - gen7_vs_state.c \
>   gen7_wm_surface_state.c \
>   gen8_blend_state.c \
>   gen8_depth_state.c \
> @@ -113,7 +112,6 @@ i965_FILES = \
>   gen8_multisample_state.c \
>   gen8_surface_state.c \
>   gen8_viewport_state.c \
> - gen8_vs_state.c \
>   hsw_queryobj.c \
>   hsw_sol.c \
>   intel_batchbuffer.c \
> diff --git a/src/mesa/drivers/dri/i965/brw_state.h 
> b/src/mesa/drivers/dri/i965/brw_state.h
> index 71ec9fb..306bfc5 100644
> --- a/src/mesa/drivers/dri/i965/brw_state.h
> +++ b/src/mesa/drivers/dri/i965/brw_state.h
> @@ -123,7 +123,6 @@ extern const struct brw_tracked_state gen6_sf_vp;
>  extern const struct brw_tracked_state gen6_urb;
>  extern const struct brw_tracked_state gen6_viewport_state;
>  extern const struct brw_tracked_state gen6_vs_push_constants;
> -extern const struct brw_tracked_state gen6_vs_state;
>  extern const struct brw_tracked_state gen6_wm_push_constants;
>  extern const struct brw_tracked_state gen7_depthbuffer;
>  extern const struct brw_tracked_state gen7_ds_state;
> @@ -136,7 +135,6 @@ extern const struct brw_tracked_state 
> gen7_sf_clip_viewport;
>  extern const struct brw_tracked_state gen7_te_state;
>  extern const struct brw_tracked_state gen7_tes_push_constants;
>  extern const struct brw_tracked_state gen7_urb;
> -extern const struct brw_tracked_state gen7_vs_state;
>  extern const struct brw_tracked_state haswell_cut_index;
>  extern const struct brw_tracked_state gen8_blend_state;
>  extern const struct brw_tracked_state gen8_ds_state;
> @@ -149,7 +147,6 @@ extern const struct brw_tracked_state gen8_ps_blend;
>  extern const struct brw_tracked_state gen8_sf_clip_viewport;
>  extern const struct brw_tracked_state gen8_vertices;
>  extern const struct brw_tracked_state gen8_vf_topology;
> -extern const struct brw_tracked_state gen8_vs_state;
>  extern const struct brw_tracked_state brw_cs_work_groups_surface;
>  
>  static inline bool
> diff --git a/src/mesa/drivers/dri/i965/gen6_vs_state.c 
> b/src/mesa/drivers/dri/i965/gen6_vs_state.c
> index 17b8118..b2d2306 100644
> --- a/src/mesa/drivers/dri/i965/gen6_vs_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_vs_state.c
> @@ -68,116 +68,3 @@ const struct brw_tracked_state gen6_vs_push_constants = {
> },
> .emit = gen6_upload_vs_push_constants,
>  };
> -
> -static void
> -upload_vs_state(struct brw_context *brw)
> -{
> -   const struct gen_device_info *devinfo = >screen->devinfo;
> -   const struct brw_stage_state *stage_state = >vs.base;
> -   const struct brw_stage_prog_data *prog_data = stage_state->prog_data;
> -   const struct brw_vue_prog_data *vue_prog_data =
> -  brw_vue_prog_data(stage_state->prog_data);
> -   uint32_t floating_point_mode = 0;
> -
> -   /* From the BSpec, 3D Pipeline > Geometry > Vertex Shader > State,
> -* 3DSTATE_VS, Dword 5.0 "VS Function Enable":
> -*
> -*   [DevSNB] A pipeline flush must be programmed prior to a 3DSTATE_VS
> -*   command that causes the VS Function Enable to toggle. Pipeline
> -*   flush can be executed by sending a PIPE_CONTROL command with CS
> -*   stall bit set and a post sync operation.
> -*
> -* We've already done such a flush at the start of state upload, so we
> -* don't need to do another one here.
> -*/
> -
> -   if (stage_state->push_const_size == 0) {
> -  /* Disable the push constant buffers. */
> -  BEGIN_BATCH(5);
> -  OUT_BATCH(_3DSTATE_CONSTANT_VS << 16 | (5 - 2));
> -  OUT_BATCH(0);
> -  OUT_BATCH(0);
> -  OUT_BATCH(0);
> -  OUT_BATCH(0);
> -  ADVANCE_BATCH();
> -   } else {
> -  BEGIN_BATCH(5);
> -  OUT_BATCH(_3DSTATE_CONSTANT_VS << 16 |
> - GEN6_CONSTANT_BUFFER_0_ENABLE |
> - (5 - 2));
> -  /* Pointer to the VS constant buffer.  

Re: [Mesa-dev] [PATCH 02/12] i965/cnl: Add gen10.xml

2017-04-20 Thread Emil Velikov
Hi Anuj,

On 15 April 2017 at 01:35, Anuj Phogat  wrote:
> From: Jason Ekstrand 
>
> Signed-off-by: Anuj Phogat 
> ---
>  src/intel/Makefile.sources |3 +-
>  src/intel/genxml/gen10.xml | 3557 
> 
>  2 files changed, 3559 insertions(+), 1 deletion(-)
>  create mode 100644 src/intel/genxml/gen10.xml
>
> diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources
> index c568916..57eb157 100644
> --- a/src/intel/Makefile.sources
> +++ b/src/intel/Makefile.sources
> @@ -117,7 +117,8 @@ GENXML_XML_FILES = \
> genxml/gen7.xml \
> genxml/gen75.xml \
> genxml/gen8.xml \
> -   genxml/gen9.xml
> +   genxml/gen9.xml \
> +   genxml/gen10.xml
>
Please add a few lines analogous to src/intel/Android.genxml.mk.
Copy the gen9.xml hunk and s/gen9/gen10/.

Thanks
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v5 0/3] asynchronous pbo transfer with glthread

2017-04-20 Thread gregory hainaut
On Thu, 20 Apr 2017 11:57:08 +0200
Marek Olšák  wrote:

> On Thu, Apr 20, 2017 at 10:28 AM, gregory hainaut
>  wrote:
> > On Thu, 20 Apr 2017 12:29:11 +0900
> > Michel Dänzer  wrote:
> >
> >> On 20/04/17 01:54 AM, Gregory Hainaut wrote:
> >> > Hello,
> >> >
> >> > Please find the latest version that include a small fix for hash 
> >> > deletion. I
> >> > think the series is good now. Please review/ack it.
> >>
> >> I'm afraid I have to NACK it. As discussed in the v4 cover letter
> >> thread, Mesa's glthread cannot make any libX11 API calls.
> >>
> >>
> >
> > Hello Michel,
> >
> > Just to be sure we are on the same line, let's me do a summary.
> >
> > PCSX2 does the following bad pattern
> > 1/ no call to XInitThread
> > 2/ XGetGeometry to get the size of the surface
> > 3/ glDrawArray into the default framebuffer 0 (the window, I'm not sure how 
> > to call it)
> >=> which seem to call DRI2GetBuffersWithFormat under the hood. I guess 
> > to get the
> >   associated buffer of the window.
> >
> >
> > So far it was (kind of) working fine because PCSX2 does tons of PBO 
> > transfer. So glthread
> > was mostly synchronous.
> >
> > This series removes the (useless) PBO transfer synchronization. So now 
> > glthread is really
> > asynchronous and the above bad pattern crash as expected.
> >
> > I didn't add any libX11 API call on the patches. And I don't think we can 
> > remvove the DRI stuff.
> >
> > Hum, I don't know what will be the impact on the perf but potentially we 
> > can force a synchronization
> > when there is a draw to framebuffer 0.
> 
> Can you send us the backtrace when DRI2GetBuffersWithFormat is called
> by glDrawArrays?
> 
> Marek

Hello Marek, Michel,

Here the full backtrace.

#0  DRI2GetBuffersWithFormat (dpy=0xc6307e48, drawable=104857784, 
width=0xdef348c0, height=0xdef348c4, attachments=0xdcc15c88, count=1, 
outCount=0xdcc15c74) at dri2.c:464
#1  0xf05bac45 in dri2GetBuffersWithFormat (driDrawable=0xdef348a8, 
width=0xdef348c0, height=0xdef348c4, attachments=0xdcc15c88, count=1, 
out_count=0xdcc15c74, loaderPrivate=0xdefc6ef0) at dri2_glx.c:894
#2  0xe3ec3111 in dri2_drawable_get_buffers (count=, 
atts=0xdef252f8, drawable=0xdefc7b08) at dri2.c:285
#3  dri2_allocate_textures (ctx=0xdef15928, drawable=0xdefc7b08, 
statts=0xdef252f8, statts_count=2) at dri2.c:480
#4  0xe3ebcbb0 in dri_st_framebuffer_validate (stctx=0xdef4ab10, 
stfbi=0xdefc7b08, statts=0xdef252f8, count=2, out=0xdcc15d78) at 
dri_drawable.c:83
#5  0xe3d37afc in st_framebuffer_validate (stfb=stfb@entry=0xdef24f58, 
st=st@entry=0xdef4ab10) at state_tracker/st_manager.c:189
Note "print stfb->Base->Name" give me 0
#6  0xe3d38649 in st_manager_validate_framebuffers (st=0xdef4ab10) at 
state_tracker/st_manager.c:869
#7  0xe3cd0580 in st_validate_state (st=0xdef4ab10, 
pipeline=ST_PIPELINE_RENDER) at state_tracker/st_atom.c:174
#8  0xe3cf935d in st_draw_vbo (ctx=0xdef7f8f8, prims=0xdcc15f40, nr_prims=1, 
ib=0x0, index_bounds_valid=1 '\001', min_index=39911, max_index=39914, 
tfb_vertcount=0x0, stream=0, indirect=0x0) at state_tracker/st_draw.c:191
#9  0xe3caf35e in vbo_draw_arrays (baseInstance=0, numInstances=1, count=4, 
start=39911, mode=5, ctx=0xdef7f8f8) at vbo/vbo_exec_array.c:427
#10 vbo_exec_DrawArrays (mode=5, start=39911, count=4) at 
vbo/vbo_exec_array.c:575
#11 0xe3be4f07 in _mesa_unmarshal_DrawArrays (ctx=0xdef7f8f8, cmd=0xc41574b0) 
at main/marshal_generated.c:26644
#12 _mesa_unmarshal_dispatch_cmd (ctx=0xdef7f8f8, cmd=0xc41574b0) at 
main/marshal_generated.c:42457
#13 0xe3ba8af0 in glthread_unmarshal_batch (release_batch=true, 
batch=0xc4157410, ctx=0xdef7f8f8) at main/glthread.c:64
#14 glthread_worker (data=0xdef7f8f8) at main/glthread.c:110


> If this DRI2GetBuffersWithFormat call results in libX11 API calls on the
> glthread, that's a bug which needs to be fixed, either by moving the
> DRI2GetBuffersWithFormat call to the main thread or (if possible) by
> changing DRI2GetBuffersWithFormat to use XCB directly.

Ok. On one hand, moving DRI2GetBuffersWithFormat to main thread won't be easy. 
I think we need to 
keep track of the current bound framebuffer on the app thread. So we can force 
a sync
on gl operation that will access the framebuffer 0.

On the other hand, GLX/XLIB/XCB interactions are  quite foggy for me. It seem 
there already some xcb codes
in various place typically EGL and the xcb_dri2 extension. So maybe there is a 
reason that code wasn't ported here.
In particular, I saw this option in configure
  --enable-glx[=dri|xlib|gallium-xlib] enable the GLX library and choose an 
implementation
  [default=auto]


> It sounds like the glthread bug above exists independently from this
> patch series, which might just make it more likely to result in a crash.
> I think it would still be better to fix the bug before landing this series.
Yes it increases the probability to trigger the bug.


Cheers,
Gregory

Re: [Mesa-dev] [PATCH 3/3] egl/android: Dequeue buffers inside EGL calls (v2)

2017-04-20 Thread Mauro Rossi
2017-04-20 4:18 GMT+02:00 Tomasz Figa :

> On Wed, Apr 19, 2017 at 11:51 PM, Emil Velikov 
> wrote:
> > Hi Tomasz,
> >
> > On 19 April 2017 at 08:00, Tomasz Figa  wrote:
> >> Android buffer queues can be abandoned, which results in failing to
> >> dequeue next buffer. Currently this would fail somewhere deep within
> >> the DRI stack calling loader's getBuffers*(), without any error
> >> reporting to the client app. However Android framework code relies on
> >> proper signaling of this event, so we move buffer dequeue to
> >> createWindowSurface() and swapBuffers() call, which can generate proper
> >> EGL errors. To keep the performance benefits of delayed buffer handling,
> >> if any, fence wait and DRI image creation is kept delayed until
> >> getBuffers*() is called by the DRI driver.
> >>
> >> v2:
> >>  - add missing fence wait in DRI loader case,
> >>  - split simple code moving to a separate patch (Emil),
> >>  - fix previous rebase error.
> >>
> >> Signed-off-by: Tomasz Figa 
> >> ---
> >>  src/egl/drivers/dri2/egl_dri2.h |  1 +
> >>  src/egl/drivers/dri2/platform_android.c | 94
> +++--
> >>  2 files changed, 54 insertions(+), 41 deletions(-)
> >>
> >> diff --git a/src/egl/drivers/dri2/egl_dri2.h
> b/src/egl/drivers/dri2/egl_dri2.h
> >> index f16663712d..92b234d622 100644
> >> --- a/src/egl/drivers/dri2/egl_dri2.h
> >> +++ b/src/egl/drivers/dri2/egl_dri2.h
> >> @@ -288,6 +288,7 @@ struct dri2_egl_surface
> >>  #ifdef HAVE_ANDROID_PLATFORM
> >> struct ANativeWindow *window;
> >> struct ANativeWindowBuffer *buffer;
> >> +   int acquire_fence_fd;
> >> __DRIimage *dri_image_back;
> >> __DRIimage *dri_image_front;
> >>
> >> diff --git a/src/egl/drivers/dri2/platform_android.c
> b/src/egl/drivers/dri2/platform_android.c
> >> index 9a84a4c43d..0a75d790c5 100644
> >> --- a/src/egl/drivers/dri2/platform_android.c
> >> +++ b/src/egl/drivers/dri2/platform_android.c
> >> @@ -189,15 +189,9 @@ droid_free_local_buffers(struct dri2_egl_surface
> *dri2_surf)
> >> }
> >>  }
> >>
> >> -static EGLBoolean
> >> -droid_window_dequeue_buffer(struct dri2_egl_surface *dri2_surf)
> >> +static void
> >> +wait_and_close_acquire_fence(struct dri2_egl_surface *dri2_surf)
> >>  {
> >> -   int fence_fd;
> >> -
> >> -   if (dri2_surf->window->dequeueBuffer(dri2_surf->window,
> _surf->buffer,
> >> -_fd))
> >> -  return EGL_FALSE;
> >> -
> >> /* If access to the buffer is controlled by a sync fence, then
> block on the
> >>  * fence.
> >>  *
> >> @@ -215,15 +209,25 @@ droid_window_dequeue_buffer(struct
> dri2_egl_surface *dri2_surf)
> >>  *any value except -1) then the caller is responsible for
> closing the
> >>  *file descriptor.
> >>  */
> >> -if (fence_fd >= 0) {
> >> +if (dri2_surf->acquire_fence_fd >= 0) {
> >> /* From the SYNC_IOC_WAIT documentation in :
> >>  *
> >>  *Waits indefinitely if timeout < 0.
> >>  */
> >>  int timeout = -1;
> >> -sync_wait(fence_fd, timeout);
> >> -close(fence_fd);
> >> +sync_wait(dri2_surf->acquire_fence_fd, timeout);
> >> +close(dri2_surf->acquire_fence_fd);
> >> +dri2_surf->acquire_fence_fd = -1;
> >> }
> >> +}
> >> +
> >> +static EGLBoolean
> >> +droid_window_dequeue_buffer(_EGLDisplay *disp,
> >> +struct dri2_egl_surface *dri2_surf)
> >> +{
> >> +   if (dri2_surf->window->dequeueBuffer(dri2_surf->window,
> _surf->buffer,
> >> +_surf->acquire_fence_fd))
> >> +  return EGL_FALSE;
> >>
> >> dri2_surf->buffer->common.incRef(_surf->buffer->common);
> >>
> >> @@ -254,6 +258,14 @@ droid_window_dequeue_buffer(struct
> dri2_egl_surface *dri2_surf)
> >>dri2_surf->back = _surf->color_buffers[0];
> >> }
> >>
> >> +   /* free outdated buffers and update the surface size */
> >> +   if (dri2_surf->base.Width != dri2_surf->buffer->width ||
> >> +   dri2_surf->base.Height != dri2_surf->buffer->height) {
> >> +  droid_free_local_buffers(dri2_surf);
> >> +  dri2_surf->base.Width = dri2_surf->buffer->width;
> >> +  dri2_surf->base.Height = dri2_surf->buffer->height;
> >> +   }
> >> +
> >> return EGL_TRUE;
> >>  }
> >>
> >> @@ -263,6 +275,9 @@ droid_window_enqueue_buffer(_EGLDisplay *disp,
> >>  {
> >> struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
> >>
> >> +   /* In case we haven't done any rendering. */
> >> +   wait_and_close_acquire_fence(dri2_surf);
> >> +
> >> /* To avoid blocking other EGL calls, release the display mutex
> before
> >>  * we enter droid_window_enqueue_buffer() and re-acquire the mutex
> upon
> >>  * return.
> >> @@ -319,6 +334,7 @@ droid_create_surface(_EGLDriver *drv, _EGLDisplay
> *disp, EGLint type,
> >>_eglError(EGL_BAD_ALLOC, 

Re: [Mesa-dev] question about container_of

2017-04-20 Thread Emil Velikov
On 18 April 2017 at 13:55, Pekka Paalanen  wrote:
> On Mon, 27 Feb 2017 13:26:11 +
> Emil Velikov  wrote:
>
>> Hi Julien,
>>
>> On 27 February 2017 at 12:08, Julien Isorce  wrote:
>> > Hi,
>> >
>> > Since 2012 commit ccff74971203b533bf16b46b49a9e61753f75e6c it is said:
>> > "sample must be initialized, or else the result is undefined" in the
>> > description of mesa/src/util/list.h::container_of .
>> >
>> > But I can find a few places where it is used without initializing that
>> > second parameter, i.e. like:
>> >
>> > struct A a;
>> > container_of(ptr, a, member);
>> >
>> > Then I can add the "= NULL" but should it be just
>> > container_of(ptr, struct A, member);
>> > like in the kernel and some other places in mesa ?
>> >
>> Strictly peaking these are toolchain (ASAN iirc) bugs, since there is
>> no pointer deref, as we're doing pointer arithmetic.
>
> Hi Emil,
>
> that's what people would usually think. It used to work with GCC. Then
> came Clang.
>
>> Afaict the general decision was to to merge the patch(es) since they
>> will make actual bugs stand out amongst the noise. In the long run,
>> it's better to fix the tool (ASAN/other) than trying to "fix" all the
>> cases in mesa and dozens of other projects. But until then patches are
>> safe enough ;-)
>>
>> That's my take on it, at least.
>
> It depends on how container_of() has been defined. For more details,
> see e.g.:
> https://cgit.freedesktop.org/wayland/wayland/commit/?id=a18e34417ba3fefeb81d891235e8ebf394a20a74
>
> The comment in the code in Mesa is correct for the fallback
> implementation it has. Maybe things rely on the fallback implementation
> never being used when compiling with Clang.
>
> Julien, some alternatives for container_of use a pointer, others expect
> a type instead. I believe one must use the correct form.
>
Thanks for the correction Pekka - yes, the issue (to deref or not) is
implementation specific.
A 'minor' detail that I've missed.

Seems like we use the more prone one - having the second argument
being a pointer as, opposed to a type.
Perhaps we should reconsider and update mesa, sooner rather than later.

Thanks again!
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/7] gallium: fold u_trim_pipe_prim call from st/mesa to drivers

2017-04-20 Thread Ilia Mirkin
On Thu, Apr 20, 2017 at 12:00 PM, Marek Olšák  wrote:
> On Fri, Apr 14, 2017 at 6:47 PM, Ilia Mirkin  wrote:
>> On Fri, Apr 14, 2017 at 12:42 PM, Marek Olšák  wrote:
>>> On Fri, Apr 14, 2017 at 5:45 PM, Ilia Mirkin  wrote:
 On Fri, Apr 14, 2017 at 11:07 AM, Marek Olšák  wrote:
> diff --git a/src/gallium/drivers/nouveau/nv30/nv30_vbo.c 
> b/src/gallium/drivers/nouveau/nv30/nv30_vbo.c
> index bc9b9a1..295c394 100644
> --- a/src/gallium/drivers/nouveau/nv30/nv30_vbo.c
> +++ b/src/gallium/drivers/nouveau/nv30/nv30_vbo.c
> @@ -543,20 +543,23 @@ nv30_draw_elements(struct nv30_context *nv30, bool 
> shorten,
> }
>  }
>
>  static void
>  nv30_draw_vbo(struct pipe_context *pipe, const struct pipe_draw_info 
> *info)
>  {
> struct nv30_context *nv30 = nv30_context(pipe);
> struct nouveau_pushbuf *push = nv30->base.pushbuf;
> int i;
>
> +   if (!u_trim_pipe_prim(info->mode, (unsigned*)>count))
> +  return;
> +

 Should this also have a !info->primitive_restart? It's supported on
 nv4x (covered by this driver).
>>>
>>> In that case, I wonder if u_trim_pipe_prim is required with this
>>> driver. It might be better to just remove that call.
>>
>> Based on a quick look, this seems to exist to prevent short draws and
>> trim the count to the nearest prim size, i.e. if you try to draw a tri
>> with %3 != 0 vertices, or a line with %2 != 0? I'm not 100% sure that
>> the NV30 HW handles those correctly, but it probably does. I can
>> double-check tonight, as I have one plugged in these days.
>
> I'll just add this. Is it OK?

Yeah, that's fine. Sorry, I totally spaced on testing out the hw. I'll
make a note to do that in a place where I'll hopefully remember, and
remove the whole thing if it's all supported.

>
> diff --git a/src/gallium/drivers/nouveau/nv30/nv30_vbo.c
> b/src/gallium/drivers/nouveau/nv30/nv30_vbo.c
> index 295c394..316f868 100644
> --- a/src/gallium/drivers/nouveau/nv30/nv30_vbo.c
> +++ b/src/gallium/drivers/nouveau/nv30/nv30_vbo.c
> @@ -550,7 +550,8 @@ nv30_draw_vbo(struct pipe_context *pipe, const
> struct pipe_draw_info *info)
> struct nouveau_pushbuf *push = nv30->base.pushbuf;
> int i;
>
> -   if (!u_trim_pipe_prim(info->mode, (unsigned*)>count))
> +   if (!info->primitive_restart &&
> +   !u_trim_pipe_prim(info->mode, (unsigned*)>count))
>return;
>
> /* For picking only a few vertices from a large user buffer, push is 
> better,
>
> Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/7] gallium: fold u_trim_pipe_prim call from st/mesa to drivers

2017-04-20 Thread Marek Olšák
On Fri, Apr 14, 2017 at 6:47 PM, Ilia Mirkin  wrote:
> On Fri, Apr 14, 2017 at 12:42 PM, Marek Olšák  wrote:
>> On Fri, Apr 14, 2017 at 5:45 PM, Ilia Mirkin  wrote:
>>> On Fri, Apr 14, 2017 at 11:07 AM, Marek Olšák  wrote:
 diff --git a/src/gallium/drivers/nouveau/nv30/nv30_vbo.c 
 b/src/gallium/drivers/nouveau/nv30/nv30_vbo.c
 index bc9b9a1..295c394 100644
 --- a/src/gallium/drivers/nouveau/nv30/nv30_vbo.c
 +++ b/src/gallium/drivers/nouveau/nv30/nv30_vbo.c
 @@ -543,20 +543,23 @@ nv30_draw_elements(struct nv30_context *nv30, bool 
 shorten,
 }
  }

  static void
  nv30_draw_vbo(struct pipe_context *pipe, const struct pipe_draw_info 
 *info)
  {
 struct nv30_context *nv30 = nv30_context(pipe);
 struct nouveau_pushbuf *push = nv30->base.pushbuf;
 int i;

 +   if (!u_trim_pipe_prim(info->mode, (unsigned*)>count))
 +  return;
 +
>>>
>>> Should this also have a !info->primitive_restart? It's supported on
>>> nv4x (covered by this driver).
>>
>> In that case, I wonder if u_trim_pipe_prim is required with this
>> driver. It might be better to just remove that call.
>
> Based on a quick look, this seems to exist to prevent short draws and
> trim the count to the nearest prim size, i.e. if you try to draw a tri
> with %3 != 0 vertices, or a line with %2 != 0? I'm not 100% sure that
> the NV30 HW handles those correctly, but it probably does. I can
> double-check tonight, as I have one plugged in these days.

I'll just add this. Is it OK?

diff --git a/src/gallium/drivers/nouveau/nv30/nv30_vbo.c
b/src/gallium/drivers/nouveau/nv30/nv30_vbo.c
index 295c394..316f868 100644
--- a/src/gallium/drivers/nouveau/nv30/nv30_vbo.c
+++ b/src/gallium/drivers/nouveau/nv30/nv30_vbo.c
@@ -550,7 +550,8 @@ nv30_draw_vbo(struct pipe_context *pipe, const
struct pipe_draw_info *info)
struct nouveau_pushbuf *push = nv30->base.pushbuf;
int i;

-   if (!u_trim_pipe_prim(info->mode, (unsigned*)>count))
+   if (!info->primitive_restart &&
+   !u_trim_pipe_prim(info->mode, (unsigned*)>count))
   return;

/* For picking only a few vertices from a large user buffer, push is better,

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/3] genxml/pack: Allow hex values in the XML

2017-04-20 Thread Dylan Baker
Quoting Jason Ekstrand (2017-04-19 17:17:25)
> ---
>  src/intel/genxml/gen_pack_header.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/intel/genxml/gen_pack_header.py 
> b/src/intel/genxml/gen_pack_header.py
> index 2a70945..5b55143 100644
> --- a/src/intel/genxml/gen_pack_header.py
> +++ b/src/intel/genxml/gen_pack_header.py
> @@ -7,6 +7,7 @@ import xml.parsers.expat
>  import re
>  import sys
>  import copy
> +import ast

please sort the imports :)

with that minor nit:
Reviewed-by: Dylan Baker 

>  
>  license =  """/*
>   * Copyright (C) 2016 Intel Corporation
> @@ -476,7 +477,7 @@ class Group(object):
>  class Value(object):
>  def __init__(self, attrs):
>  self.name = safe_name(attrs["name"])
> -self.value = int(attrs["value"])
> +self.value = ast.literal_eval(attrs["value"])
>  
>  class Parser(object):
>  def __init__(self):
> -- 
> 2.5.0.400.gff86faf
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


signature.asc
Description: signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] mesa/glthread: correctly comparae thread handles

2017-04-20 Thread Emil Velikov
From: Emil Velikov 

As mentioned in the manual - comparing pthread_t handles via the C
comparison operator is incorrect and pthread_equal() should be used
instead.

Cc: Timothy Arceri 
Cc: Eric Anholt 
Fixes: d8d81fbc316 ("mesa: Add infrastructure for a worker thread to process GL 
commands.")
Signed-off-by: Emil Velikov 
---
 src/mesa/main/glthread.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/main/glthread.c b/src/mesa/main/glthread.c
index c4d3f4a4349..455b829cd8d 100644
--- a/src/mesa/main/glthread.c
+++ b/src/mesa/main/glthread.c
@@ -265,7 +265,7 @@ _mesa_glthread_finish(struct gl_context *ctx)
 * dri interface entrypoints), in which case we don't need to actually
 * synchronize against ourself.
 */
-   if (pthread_self() == glthread->thread)
+   if (pthread_equal(pthread_self(), glthread->thread))
   return;
 
pthread_mutex_lock(>mutex);
-- 
2.12.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] configure.ac: Refuse to build Vulkan without DRI3

2017-04-20 Thread ville . syrjala
From: Ville Syrjälä 

Trying to build the Vulkan WSI code results in a failure without DRI3.
Make configure check that DRI3 is available before trying to build
the Vulkan drivers.

Signed-off-by: Ville Syrjälä 
---
 configure.ac | 4 
 1 file changed, 4 insertions(+)

diff --git a/configure.ac b/configure.ac
index 957d15df8caa..248736c63aaa 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1844,6 +1844,10 @@ if test -n "$with_vulkan_drivers"; then
 AC_MSG_ERROR([Vulkan drivers require the dl_iterate_phdr function])
 fi
 
+if test "x$enable_dri3" = xno; then
+AC_MSG_ERROR([Vulkan requires DRI3])
+fi
+
 VULKAN_DRIVERS=`IFS=', '; echo $with_vulkan_drivers`
 for driver in $VULKAN_DRIVERS; do
 case "x$driver" in
-- 
2.10.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [RFC PATCH 03/26] glsl: introduce new base types for bindless samplers/images

2017-04-20 Thread Samuel Pitoiset



On 04/20/2017 12:47 AM, Samuel Pitoiset wrote:



On 04/19/2017 11:14 AM, Nicolai Hähnle wrote:

On 19.04.2017 09:51, Samuel Pitoiset wrote:



On 04/18/2017 11:26 PM, Nicolai Hähnle wrote:

On 18.04.2017 21:49, Dave Airlie wrote:

On 19 April 2017 at 05:30, Samuel Pitoiset
 wrote:



On 04/18/2017 08:14 PM, Nicolai Hähnle wrote:


On 11.04.2017 18:48, Samuel Pitoiset wrote:


Bindless sampler/image types are really different from the existing
sampler/image types. They are considered 64-bit unsigned integers,
they can be declared as temporary, shader inputs/outputs and are
non-opaque types.

For these reasons, it looks more convenient to introduce new
internal base types to the GLSL compiler, called
GLSL_TYPE_BINDLESS_SAMPLER and respectively 
GLSL_TYPE_BINDLESS_IMAGE.



Sorry for taking so long to get to this series, but could you
explain the
rationale here a bit more?



No worries, all feedbacks are always welcome, even late. :)

So, the bindless sampler/image types introduced in
ARB_bindless_texture are
really different from the "standard" ones.

They are considered as 64-bit unsigned integers (not 32-bit) and
they are
non-opaque types. The latter means they can be declared as temporary
variables, as shader inputs/outputs, inside an interface block, etc.


I don't know that that's the best way to think about it at the GLSL
level. Mostly they're still opaque types, it's just that they can be
explicitly converted to/from 64-bit ints (or uvec2... not having an
interaction with GL_ARB_gpu_shader_int64 seems like a spec oversight).


Well, it's not *only* about the explicit conversions,
ARB_bindless_texture allows more flexibility for sampler/image types.

An oversight? Unfortunately, it looks like to me it's not the only 
one...


Yes, it does look like several places in the spec could use some 
clarification...




That said, the current sampler/image types are opaque (cf,
glsl_type::is_opaque()) and it seemed quite impossible to change the
glsl_type helpers to fit my needs.


I see no is_opaque, maybe you mean contains_opaque? I agree that it's
annoying that the restrictions expressed in ast_to_hir.cpp need to be
modified. What other helpers are an issue?


Yes, contains_opaque() not is_opaque(). Well, how do you plan to handle
the fact that bindless sampler types are 64-bit? It seemed quite logical
to make glsl_type::is_64bit() returns true for them.


So, for the purposes of src/compiler/glsl/, I think it simply 
shouldn't matter whether is_64bit returns true or not. However, I 
agree that for st/glsl_to_tgsi, it'd probably be very helpful to make 
is_64bit return true for them.


One new restriction is glsl_type::component_slots(). With the new base 
types it's easy to return something similar to uint64_t (ie. 2 
components if it's BINDLESS_SAMPLER). However, without a separate type 
it returns 0 for samplers and 1 for images.


glsl_type is a quite good interface for base types, but for bindless 
sampler we just can't store the information there. Currently, the only 
way to know if a sampler is bindless is to have access to a ir_variable 
object.


I don't see how to solve this properly without adding hacks. The way 
glsl_type is designed is one of the argument for the current solution.


Any thoughts?


It's allowed by the spec to count samplers/images as two components. So 
component_slots() can return N*2 like uint64_t. However, 
glsl_type::components() currently returns 0 for sampler types because 
matrix_columns and vector_elements are 0 (while 1 for image types).


I think this needs to be clarified and components() should return 1 for 
sampler and image types.








I don't remember all the restrictions to be honest (but they are many
others) because this solution has been adopted for weeks. But the way
the compiler is implemented, doing the other solution *really* need a
ton of changes.





I tried many different solutions before figuring this one which
seems better
for some reasons:

- easy to make bindless sampler/image types 64-bit unsigned int
- easy to make bindless sampler/image types non-opaque
- should avoid breakage because the base type is different
- reduce the amount of changes in most places in the compiler


I still don't see a *positive* justification for having the two
different type families. Where do you actually need to *distinguish*
between bindless and bound samplers in the compiler? The spec really
doesn't make that distinction at all, except in one single place:

"These modifiers control whether default-block uniforms of
 the corresponding types may have their values set via both
 UniformHandle* and Uniform1i (bindless_sampler and
 bindless_image) or only via Uniform1i (bound_sampler and
 bound_image)."

It seems like the only place to distinguish between the two is
actually *outside* the compiler, in the uniforms API. And that's not
even really have about the *type* of the variable; it's about the
variable's memory 

Re: [Mesa-dev] [PATCH] docs/envvars: order INTEL_DEBUG envvars by name

2017-04-20 Thread Samuel Iglesias Gonsálvez
On Thu, 2017-04-20 at 16:11 +0200, Iago Toral wrote:
> s/order/sort :)
> 

Right! I unconsciously got it wrong with the similar word in Spanish
('ordenar'), which also has the meaning of 'to sort'.

> Reviewed-by: Iago Toral Quiroga 
> 

Thanks!

Sam

> 
> On Thu, 2017-04-20 at 14:02 +0200, Samuel Iglesias Gonsálvez wrote:
> > It helps to find the envvar you are looking for.
> > 
> > Signed-off-by: Samuel Iglesias Gonsálvez 
> > ---
> >  docs/envvars.html | 58 +++
> > 
> >  1 file changed, 29 insertions(+), 29 deletions(-)
> > 
> > diff --git a/docs/envvars.html b/docs/envvars.html
> > index a064f569c8b..05afd2d5529 100644
> > --- a/docs/envvars.html
> > +++ b/docs/envvars.html
> > @@ -163,47 +163,47 @@ See the Xlib
> > software
> > driver page for details.
> > This is useful for debugging hangs, etc.
> >  INTEL_DEBUG - a comma-separated list of named flags, which do
> > various things:
> >  
> > -   color - use color in output
> > -   tex - emit messages about textures.
> > -   state - emit messages about state flag tracking
> > -   blit - emit messages about blit operations
> > -   miptree - emit messages about miptrees
> > -   perf - emit messages about performance issues
> > -   perfmon - emit messages about AMD_performance_monitor
> > +   ann - annotate IR in assembly dumps
> > +   aub - dump batches into an AUB trace for use with
> > simulation
> > tools
> > bat - emit batch information
> > -   pix - emit messages about pixel operations
> > +   blit - emit messages about blit operations
> > +   blorp - emit messages about the blorp operations (blits
> > 
> > clears)
> > buf - emit messages about buffer objects
> > +   clip - emit messages about the clip unit (for old gens,
> > includes the CLIP program)
> > +   color - use color in output
> > +   cs - dump shader assembly for compute shaders
> > +   do32 - generate compute shader SIMD32 programs even if
> > workgroup size doesn't exceed the SIMD16 limit
> > +   dri - emit messages about the DRI interface
> > fbo - emit messages about framebuffers
> > fs - dump shader assembly for fragment shaders
> > gs - dump shader assembly for geometry shaders
> > -   sync - after sending each batch, emit a message and wait
> > for
> > that batch to finish rendering
> > -   prim - emit messages about drawing primitives
> > -   vert - emit messages about vertex assembly
> > -   dri - emit messages about the DRI interface
> > -   sf - emit messages about the strips  fans unit (for
> > old
> > gens, includes the SF program)
> > -   stats - enable statistics counters. you probably actually
> > want perfmon or intel_gpu_top instead.
> > -   urb - emit messages about URB setup
> > -   vs - dump shader assembly for vertex shaders
> > -   clip - emit messages about the clip unit (for old gens,
> > includes the CLIP program)
> > -   aub - dump batches into an AUB trace for use with
> > simulation
> > tools
> > -   shader_time - record how much GPU time is spent in each
> > shader
> > +   hex - print instruction hex dump with the disassembly
> > +   l3 - emit messages about the new L3 state during
> > transitions
> > +   miptree - emit messages about miptrees
> > +   no8 - don't generate SIMD8 fragment shader
> > no16 - suppress generation of 16-wide fragment shaders.
> > useful for debugging broken shaders
> > -   blorp - emit messages about the blorp operations (blits
> > 
> > clears)
> > +   nocompact - disable instruction compaction
> > nodualobj - suppress generation of dual-object geometry
> > shader code
> > +   norbc - disable single sampled render buffer
> > compression
> > optimizer - dump shader assembly to files at each
> > optimization pass and iteration that make progress
> > -   ann - annotate IR in assembly dumps
> > -   no8 - don't generate SIMD8 fragment shader
> > -   vec4 - force vec4 mode in vertex shader
> > +   perf - emit messages about performance issues
> > +   perfmon - emit messages about AMD_performance_monitor
> > +   pix - emit messages about pixel operations
> > +   prim - emit messages about drawing primitives
> > +   sf - emit messages about the strips  fans unit (for
> > old
> > gens, includes the SF program)
> > +   shader_time - record how much GPU time is spent in each
> > shader
> > spill_fs - force spilling of all registers in the scalar
> > backend (useful to debug spilling code)
> > spill_vec4 - force spilling of all registers in the vec4
> > backend (useful to debug spilling code)
> > -   cs - dump shader assembly for compute shaders
> > -   hex - print instruction hex dump with the disassembly
> > -   nocompact - disable instruction compaction
> > +   state - emit messages about state flag tracking
> > +   stats - enable statistics counters. you probably actually
> > want perfmon or intel_gpu_top instead.
> > +   sync - after sending each batch, emit a message and wait
> > for
> > that batch to 

[Mesa-dev] [PATCH 2/2] mapi: use the C11 thrd_current/thrd_equal

2017-04-20 Thread Emil Velikov
From: Emil Velikov 

As of earlier commit the implementations do the correct thing, so
there's no need to have the PTHREAD/WIN32 specifics in the current code.

Cc: Brian Paul 
Cc: José Fonseca 
Signed-off-by: Emil Velikov 
---
There's an open question about how lightweight GetCurrentThread +
GetThreadId is wrt GetCurrentThreadId. If anyone can share their input
and/or preference I'm all for it.
---
 src/mapi/u_current.c | 42 +++---
 1 file changed, 3 insertions(+), 39 deletions(-)

diff --git a/src/mapi/u_current.c b/src/mapi/u_current.c
index 7e7e275f2e3..1c568a2ff26 100644
--- a/src/mapi/u_current.c
+++ b/src/mapi/u_current.c
@@ -145,42 +145,6 @@ u_current_init_tsd(void)
 static mtx_t ThreadCheckMutex = _MTX_INITIALIZER_NP;
 
 
-#ifdef _WIN32
-typedef DWORD thread_id;
-#else
-typedef thrd_t thread_id;
-#endif
-
-
-static inline thread_id
-get_thread_id(void)
-{
-   /*
-* XXX: Callers of of this function assume it is a lightweight function.
-* But unfortunately C11's thrd_current() gives no such guarantees.  In
-* fact, it's pretty hard to have a compliant implementation of
-* thrd_current() on Windows with such characteristics.  So for now, we
-* side-step this mess and use Windows thread primitives directly here.
-*/
-#ifdef _WIN32
-   return GetCurrentThreadId();
-#else
-   return thrd_current();
-#endif
-}
-
-
-static inline int
-thread_id_equal(thread_id t1, thread_id t2)
-{
-#ifdef _WIN32
-   return t1 == t2;
-#else
-   return thrd_equal(t1, t2);
-#endif
-}
-
-
 /**
  * We should call this periodically from a function such as glXMakeCurrent
  * in order to test if multiple threads are being used.
@@ -188,7 +152,7 @@ thread_id_equal(thread_id t1, thread_id t2)
 void
 u_current_init(void)
 {
-   static thread_id knownID;
+   static thrd_t knownID;
static int firstCall = 1;
 
if (ThreadSafe)
@@ -198,10 +162,10 @@ u_current_init(void)
if (firstCall) {
   u_current_init_tsd();
 
-  knownID = get_thread_id();
+  knownID = thrd_current();
   firstCall = 0;
}
-   else if (!thread_id_equal(knownID, get_thread_id())) {
+   else if (!thrd_equal(knownID, thrd_current())) {
   ThreadSafe = 1;
   u_current_set_table(NULL);
   u_current_set_context(NULL);
-- 
2.12.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 1/2] c11/threads: implement thrd_current on Windows

2017-04-20 Thread Emil Velikov
From: Emil Velikov 

Earlier commit removed the implementation due to a subtle bug, in the
existing code. Namely:

One was comparing directly the thread handle/IDs rather than using
thrd_equal(). As the pseudo-handles never matched things went south.

That bug was resolved with commit 458c7490c29 "mapi: rewrite
u_current_init() function without u_thread_self()" thus we can bring the
thrd_current implementation back, whist preserving the correct
thrd_equal implementation as-is.

With this in place, we can remove a handful of the remaining pthread vs
WIN32 specifics.

Cc: Brian Paul 
Cc: José Fonseca 
Signed-off-by: Emil Velikov 
---
 include/c11/threads_win32.h | 35 ++-
 1 file changed, 6 insertions(+), 29 deletions(-)

diff --git a/include/c11/threads_win32.h b/include/c11/threads_win32.h
index d017c31c34e..f0eb5c77c60 100644
--- a/include/c11/threads_win32.h
+++ b/include/c11/threads_win32.h
@@ -494,42 +494,19 @@ thrd_create(thrd_t *thr, thrd_start_t func, void *arg)
 return thrd_success;
 }
 
-#if 0
 // 7.25.5.2
 static inline thrd_t
 thrd_current(void)
 {
-HANDLE hCurrentThread;
-BOOL bRet;
-
-/* GetCurrentThread() returns a pseudo-handle, which is useless.  We need
- * to call DuplicateHandle to get a real handle.  However the handle value
- * will not match the one returned by thread_create.
- *
- * Other potential solutions would be:
- * - define thrd_t as a thread Ids, but this would mean we'd need to 
OpenThread for many operations
- * - use malloc'ed memory for thrd_t. This would imply using TLS for 
current thread.
- *
- * Neither is particularly nice.
+/* GetCurrentThread() returns a pseudo-handle, which will not match the
+ * one returned by thread_create.
  *
- * Life would be much easier if C11 threads had different abstractions for
- * threads and thread IDs, just like C++11 threads does...
+ * At the same time, the only way that one should compare thread handle
+ * and/or IDs is via thrd_equal. That in itself attributes for the above
+ * situation.
  */
-
-bRet = DuplicateHandle(GetCurrentProcess(), // source process (pseudo) 
handle
-   GetCurrentThread(), // source (pseudo) handle
-   GetCurrentProcess(), // target process
-   , // target handle
-   0,
-   FALSE,
-   DUPLICATE_SAME_ACCESS);
-assert(bRet);
-if (!bRet) {
-   hCurrentThread = GetCurrentThread();
-}
-return hCurrentThread;
+return GetCurrentThread();
 }
-#endif
 
 // 7.25.5.3
 static inline int
-- 
2.12.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] RFC: mesa: remove MESA_VERBOSE env variable

2017-04-20 Thread Emil Velikov
From: Emil Velikov 

The env variable was useful in the early days of mesa development, when
tools such at Apitrace were not available.

Nowadays, out of the 12 options only a third are used. With nearly all
instances printing the API state. And even then not all entry points
are annotated, thus one cannot rely upon it.

The current patch removes the variable all together alongside a few
instances (listed below) which developers may see value in keeping.

FLUSH_VERTICES/FLUSH_CURRENT macros in src/mesa/main/context.h
_mesa_GetGraphicsResetStatusARB in src/mesa/main/robustness.c
_mesa_print_state() in src/mesa/main/state.c
_mesa_notifySwapBuffers() in src/mesa/main/context.c

Do we want to keep any of the MESA_VERBOSE instances? If so documenting
the variable seems reasonable.

Cc: Brian Paul 
Signed-off-by: Emil Velikov 
---
Brian, I assume that you're the author for most of these. How do you
feel on the topic?
---
 src/mesa/main/attrib.c  |   8 ---
 src/mesa/main/blend.c   |  40 
 src/mesa/main/blit.c|  17 -
 src/mesa/main/bufferobj.c   |  38 ---
 src/mesa/main/buffers.c |   7 ---
 src/mesa/main/clear.c   |   3 -
 src/mesa/main/compute.c |  13 
 src/mesa/main/context.c |  19 +-
 src/mesa/main/context.h |   4 --
 src/mesa/main/copyimage.c   |  10 ---
 src/mesa/main/debug.c   |  42 -
 src/mesa/main/depth.c   |  12 
 src/mesa/main/dlist.c   |  17 -
 src/mesa/main/drawpix.c |  21 ---
 src/mesa/main/enable.c  |   6 --
 src/mesa/main/fbobject.c|  32 --
 src/mesa/main/feedback.c|   3 -
 src/mesa/main/getstring.c   |   6 --
 src/mesa/main/hint.c|   5 --
 src/mesa/main/light.c   |  14 -
 src/mesa/main/lines.c   |   6 --
 src/mesa/main/matrix.c  |  29 -
 src/mesa/main/mtypes.h  |  22 ---
 src/mesa/main/performance_monitor.c |   6 --
 src/mesa/main/pipelineobj.c |  33 --
 src/mesa/main/polygon.c |  23 ---
 src/mesa/main/program_resource.c|  33 --
 src/mesa/main/queryobj.c|  31 -
 src/mesa/main/readpix.c |   7 ---
 src/mesa/main/robustness.c  |  14 +
 src/mesa/main/samplerobj.c  |   3 -
 src/mesa/main/scissor.c |  11 
 src/mesa/main/shaderapi.c   |  16 -
 src/mesa/main/state.c   |   3 -
 src/mesa/main/stencil.c |  27 
 src/mesa/main/texenv.c  |   7 ---
 src/mesa/main/texgen.c  |   7 ---
 src/mesa/main/texgetimage.c |  17 -
 src/mesa/main/teximage.c|  52 ---
 src/mesa/main/texobj.c  |  29 -
 src/mesa/main/texstate.c|   8 ---
 src/mesa/main/texstorage.c  |  13 
 src/mesa/main/textureview.c |   6 --
 src/mesa/main/varray.c  |   6 --
 src/mesa/main/viewport.c|  28 -
 src/mesa/tnl/t_vb_render.c  |   5 --
 src/mesa/vbo/vbo_exec_array.c   | 122 
 47 files changed, 2 insertions(+), 879 deletions(-)

diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
index 8e738c91c8f..bb230404d29 100644
--- a/src/mesa/main/attrib.c
+++ b/src/mesa/main/attrib.c
@@ -245,9 +245,6 @@ _mesa_PushAttrib(GLbitfield mask)
 
GET_CURRENT_CONTEXT(ctx);
 
-   if (MESA_VERBOSE & VERBOSE_API)
-  _mesa_debug(ctx, "glPushAttrib %x\n", (int) mask);
-
if (ctx->AttribStackDepth >= MAX_ATTRIB_STACK_DEPTH) {
   _mesa_error( ctx, GL_STACK_OVERFLOW, "glPushAttrib" );
   return;
@@ -935,11 +932,6 @@ _mesa_PopAttrib(void)
 
while (attr) {
 
-  if (MESA_VERBOSE & VERBOSE_API) {
- _mesa_debug(ctx, "glPopAttrib %s\n",
- _mesa_enum_to_string(attr->kind));
-  }
-
   switch (attr->kind) {
  case DUMMY_BIT:
 /* do nothing */
diff --git a/src/mesa/main/blend.c b/src/mesa/main/blend.c
index 955fda1158c..52c473a9d8b 100644
--- a/src/mesa/main/blend.c
+++ b/src/mesa/main/blend.c
@@ -220,13 +220,6 @@ _mesa_BlendFuncSeparate( GLenum sfactorRGB, GLenum 
dfactorRGB,
unsigned buf;
bool changed = false;
 
-   if (MESA_VERBOSE & VERBOSE_API)
-  _mesa_debug(ctx, "glBlendFuncSeparate %s %s %s %s\n",
-  _mesa_enum_to_string(sfactorRGB),
-  _mesa_enum_to_string(dfactorRGB),
-  _mesa_enum_to_string(sfactorA),
-  _mesa_enum_to_string(dfactorA));
-
/* Check if we're really changing any state.  If not, return early. */
if (ctx->Color._BlendFuncPerBuffer) {
   /* Check all per-buffer states */
@@ -417,10 +410,6 @@ _mesa_BlendEquation( GLenum mode )

Re: [Mesa-dev] [PATCH] docs/envvars: order INTEL_DEBUG envvars by name

2017-04-20 Thread Iago Toral
s/order/sort :)

Reviewed-by: Iago Toral Quiroga 


On Thu, 2017-04-20 at 14:02 +0200, Samuel Iglesias Gonsálvez wrote:
> It helps to find the envvar you are looking for.
> 
> Signed-off-by: Samuel Iglesias Gonsálvez 
> ---
>  docs/envvars.html | 58 +++
> 
>  1 file changed, 29 insertions(+), 29 deletions(-)
> 
> diff --git a/docs/envvars.html b/docs/envvars.html
> index a064f569c8b..05afd2d5529 100644
> --- a/docs/envvars.html
> +++ b/docs/envvars.html
> @@ -163,47 +163,47 @@ See the Xlib software
> driver page for details.
> This is useful for debugging hangs, etc.
>  INTEL_DEBUG - a comma-separated list of named flags, which do
> various things:
>  
> -   color - use color in output
> -   tex - emit messages about textures.
> -   state - emit messages about state flag tracking
> -   blit - emit messages about blit operations
> -   miptree - emit messages about miptrees
> -   perf - emit messages about performance issues
> -   perfmon - emit messages about AMD_performance_monitor
> +   ann - annotate IR in assembly dumps
> +   aub - dump batches into an AUB trace for use with simulation
> tools
> bat - emit batch information
> -   pix - emit messages about pixel operations
> +   blit - emit messages about blit operations
> +   blorp - emit messages about the blorp operations (blits 
> clears)
> buf - emit messages about buffer objects
> +   clip - emit messages about the clip unit (for old gens,
> includes the CLIP program)
> +   color - use color in output
> +   cs - dump shader assembly for compute shaders
> +   do32 - generate compute shader SIMD32 programs even if
> workgroup size doesn't exceed the SIMD16 limit
> +   dri - emit messages about the DRI interface
> fbo - emit messages about framebuffers
> fs - dump shader assembly for fragment shaders
> gs - dump shader assembly for geometry shaders
> -   sync - after sending each batch, emit a message and wait for
> that batch to finish rendering
> -   prim - emit messages about drawing primitives
> -   vert - emit messages about vertex assembly
> -   dri - emit messages about the DRI interface
> -   sf - emit messages about the strips  fans unit (for old
> gens, includes the SF program)
> -   stats - enable statistics counters. you probably actually
> want perfmon or intel_gpu_top instead.
> -   urb - emit messages about URB setup
> -   vs - dump shader assembly for vertex shaders
> -   clip - emit messages about the clip unit (for old gens,
> includes the CLIP program)
> -   aub - dump batches into an AUB trace for use with simulation
> tools
> -   shader_time - record how much GPU time is spent in each
> shader
> +   hex - print instruction hex dump with the disassembly
> +   l3 - emit messages about the new L3 state during
> transitions
> +   miptree - emit messages about miptrees
> +   no8 - don't generate SIMD8 fragment shader
> no16 - suppress generation of 16-wide fragment shaders.
> useful for debugging broken shaders
> -   blorp - emit messages about the blorp operations (blits 
> clears)
> +   nocompact - disable instruction compaction
> nodualobj - suppress generation of dual-object geometry
> shader code
> +   norbc - disable single sampled render buffer compression
> optimizer - dump shader assembly to files at each
> optimization pass and iteration that make progress
> -   ann - annotate IR in assembly dumps
> -   no8 - don't generate SIMD8 fragment shader
> -   vec4 - force vec4 mode in vertex shader
> +   perf - emit messages about performance issues
> +   perfmon - emit messages about AMD_performance_monitor
> +   pix - emit messages about pixel operations
> +   prim - emit messages about drawing primitives
> +   sf - emit messages about the strips  fans unit (for old
> gens, includes the SF program)
> +   shader_time - record how much GPU time is spent in each
> shader
> spill_fs - force spilling of all registers in the scalar
> backend (useful to debug spilling code)
> spill_vec4 - force spilling of all registers in the vec4
> backend (useful to debug spilling code)
> -   cs - dump shader assembly for compute shaders
> -   hex - print instruction hex dump with the disassembly
> -   nocompact - disable instruction compaction
> +   state - emit messages about state flag tracking
> +   stats - enable statistics counters. you probably actually
> want perfmon or intel_gpu_top instead.
> +   sync - after sending each batch, emit a message and wait for
> that batch to finish rendering
> tcs - dump shader assembly for tessellation control
> shaders
> tes - dump shader assembly for tessellation evaluation
> shaders
> -   l3 - emit messages about the new L3 state during
> transitions
> -   do32 - generate compute shader SIMD32 programs even if
> workgroup size doesn't exceed the SIMD16 limit
> -   norbc - disable single sampled render buffer compression
> +   tex - emit messages about textures.
> + 

Re: [Mesa-dev] [PATCH] Adding support for EXT_sRGB for Opengl ES

2017-04-20 Thread Tapani Pälli


On 04/07/2017 07:20 AM, Harish Krupo wrote:

This addes support for the GL_EXT_sRGB extension for OpengGL ES 1.0 and above.
With this patch this test passes in dEQP:
dEQP-GLES2.capability.extensions.uncompressed_texture_formats.GL_EXT_sRGB


When searching for possible other cases these changes will affect I 
noticed there are also some sRGB related cases in fbo completeness that 
regress:


please run deqp-gles2 with:

--deqp-case=dEQP-GLES2.functional.fbo.completeness*srgb*

to see these.


Signed-off-by: Harish Krupo 
---
  src/mapi/glapi/gen/es_EXT.xml| 8 
  src/mesa/drivers/dri/i965/intel_extensions.c | 1 +
  src/mesa/main/extensions_table.h | 1 +
  src/mesa/main/fbobject.c | 2 +-
  src/mesa/main/genmipmap.c| 3 +++
  src/mesa/main/mtypes.h   | 1 +
  6 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/src/mapi/glapi/gen/es_EXT.xml b/src/mapi/glapi/gen/es_EXT.xml
index 3e705eb409..a6fd7c755a 100644
--- a/src/mapi/glapi/gen/es_EXT.xml
+++ b/src/mapi/glapi/gen/es_EXT.xml
@@ -795,6 +795,14 @@
  
  
  
+

+
+
+
+
+
+
+
  
  
  
diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c 
b/src/mesa/drivers/dri/i965/intel_extensions.c
index 30f2c37695..2b49e6f8db 100644
--- a/src/mesa/drivers/dri/i965/intel_extensions.c
+++ b/src/mesa/drivers/dri/i965/intel_extensions.c
@@ -109,6 +109,7 @@ intelInitExtensions(struct gl_context *ctx)
 ctx->Extensions.EXT_texture_shared_exponent = true;
 ctx->Extensions.EXT_texture_snorm = true;
 ctx->Extensions.EXT_texture_sRGB = true;
+   ctx->Extensions.EXT_sRGB = true;
 ctx->Extensions.EXT_texture_sRGB_decode = true;
 ctx->Extensions.EXT_texture_swizzle = true;
 ctx->Extensions.EXT_vertex_array_bgra = true;
diff --git a/src/mesa/main/extensions_table.h b/src/mesa/main/extensions_table.h
index ec717912cb..7fde0a3127 100644
--- a/src/mesa/main/extensions_table.h
+++ b/src/mesa/main/extensions_table.h
@@ -245,6 +245,7 @@ EXT(EXT_shader_integer_mix  , 
EXT_shader_integer_mix
  EXT(EXT_shader_io_blocks, dummy_true  
   ,  x ,  x ,  x ,  31, 2014)
  EXT(EXT_shader_samples_identical, EXT_shader_samples_identical
   , GLL, GLC,  x ,  31, 2015)
  EXT(EXT_shadow_funcs, ARB_shadow  
   , GLL,  x ,  x ,  x , 2002)
+EXT(EXT_sRGB, EXT_sRGB 
  ,  x ,  x , ES1, ES2, 2011)
  EXT(EXT_stencil_two_side, EXT_stencil_two_side
   , GLL,  x ,  x ,  x , 2001)
  EXT(EXT_stencil_wrap, dummy_true  
   , GLL,  x ,  x ,  x , 2002)
  EXT(EXT_subtexture  , dummy_true  
   , GLL,  x ,  x ,  x , 1995)
diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index f73a009d62..caa4828111 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -3898,7 +3898,7 @@ _mesa_get_framebuffer_attachment_parameter(struct 
gl_context *ctx,
   }
}
else {
- if (ctx->Extensions.EXT_framebuffer_sRGB) {
+ if (ctx->Extensions.EXT_framebuffer_sRGB || 
!ctx->Extensions.EXT_sRGB) {
  *params =
 _mesa_get_format_color_encoding(att->Renderbuffer->Format);
   }
diff --git a/src/mesa/main/genmipmap.c b/src/mesa/main/genmipmap.c
index 6021c026f5..d451ea40be 100644
--- a/src/mesa/main/genmipmap.c
+++ b/src/mesa/main/genmipmap.c
@@ -96,6 +96,9 @@ _mesa_is_valid_generate_texture_mipmap_internalformat(struct 
gl_context *ctx,
   (_mesa_is_es3_color_renderable(internalformat) &&
_mesa_is_es3_texture_filterable(ctx, internalformat));
 }
+   else if (!_mesa_is_desktop_gl(ctx) && ctx->Extensions.EXT_sRGB && 
(internalformat == GL_SRGB)) {
+  return GL_INVALID_OPERATION;
+   }
  
 return (!_mesa_is_enum_format_integer(internalformat) &&

 !_mesa_is_depthstencil_format(internalformat) &&
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index be78b96810..b938ed8958 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -3963,6 +3963,7 @@ struct gl_extensions
 GLboolean EXT_provoking_vertex;
 GLboolean EXT_shader_integer_mix;
 GLboolean EXT_shader_samples_identical;
+   GLboolean EXT_sRGB;
 GLboolean EXT_stencil_two_side;
 GLboolean EXT_texture_array;
 GLboolean EXT_texture_compression_latc;


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] docs/envvars: order INTEL_DEBUG envvars by name

2017-04-20 Thread Samuel Iglesias Gonsálvez
It helps to find the envvar you are looking for.

Signed-off-by: Samuel Iglesias Gonsálvez 
---
 docs/envvars.html | 58 +++
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/docs/envvars.html b/docs/envvars.html
index a064f569c8b..05afd2d5529 100644
--- a/docs/envvars.html
+++ b/docs/envvars.html
@@ -163,47 +163,47 @@ See the Xlib software driver 
page for details.
This is useful for debugging hangs, etc.
 INTEL_DEBUG - a comma-separated list of named flags, which do various 
things:
 
-   color - use color in output
-   tex - emit messages about textures.
-   state - emit messages about state flag tracking
-   blit - emit messages about blit operations
-   miptree - emit messages about miptrees
-   perf - emit messages about performance issues
-   perfmon - emit messages about AMD_performance_monitor
+   ann - annotate IR in assembly dumps
+   aub - dump batches into an AUB trace for use with simulation tools
bat - emit batch information
-   pix - emit messages about pixel operations
+   blit - emit messages about blit operations
+   blorp - emit messages about the blorp operations (blits  
clears)
buf - emit messages about buffer objects
+   clip - emit messages about the clip unit (for old gens, includes the 
CLIP program)
+   color - use color in output
+   cs - dump shader assembly for compute shaders
+   do32 - generate compute shader SIMD32 programs even if workgroup size 
doesn't exceed the SIMD16 limit
+   dri - emit messages about the DRI interface
fbo - emit messages about framebuffers
fs - dump shader assembly for fragment shaders
gs - dump shader assembly for geometry shaders
-   sync - after sending each batch, emit a message and wait for that batch 
to finish rendering
-   prim - emit messages about drawing primitives
-   vert - emit messages about vertex assembly
-   dri - emit messages about the DRI interface
-   sf - emit messages about the strips  fans unit (for old gens, 
includes the SF program)
-   stats - enable statistics counters. you probably actually want perfmon 
or intel_gpu_top instead.
-   urb - emit messages about URB setup
-   vs - dump shader assembly for vertex shaders
-   clip - emit messages about the clip unit (for old gens, includes the 
CLIP program)
-   aub - dump batches into an AUB trace for use with simulation tools
-   shader_time - record how much GPU time is spent in each shader
+   hex - print instruction hex dump with the disassembly
+   l3 - emit messages about the new L3 state during transitions
+   miptree - emit messages about miptrees
+   no8 - don't generate SIMD8 fragment shader
no16 - suppress generation of 16-wide fragment shaders. useful for 
debugging broken shaders
-   blorp - emit messages about the blorp operations (blits  
clears)
+   nocompact - disable instruction compaction
nodualobj - suppress generation of dual-object geometry shader code
+   norbc - disable single sampled render buffer compression
optimizer - dump shader assembly to files at each optimization pass and 
iteration that make progress
-   ann - annotate IR in assembly dumps
-   no8 - don't generate SIMD8 fragment shader
-   vec4 - force vec4 mode in vertex shader
+   perf - emit messages about performance issues
+   perfmon - emit messages about AMD_performance_monitor
+   pix - emit messages about pixel operations
+   prim - emit messages about drawing primitives
+   sf - emit messages about the strips  fans unit (for old gens, 
includes the SF program)
+   shader_time - record how much GPU time is spent in each shader
spill_fs - force spilling of all registers in the scalar backend 
(useful to debug spilling code)
spill_vec4 - force spilling of all registers in the vec4 backend 
(useful to debug spilling code)
-   cs - dump shader assembly for compute shaders
-   hex - print instruction hex dump with the disassembly
-   nocompact - disable instruction compaction
+   state - emit messages about state flag tracking
+   stats - enable statistics counters. you probably actually want perfmon 
or intel_gpu_top instead.
+   sync - after sending each batch, emit a message and wait for that batch 
to finish rendering
tcs - dump shader assembly for tessellation control shaders
tes - dump shader assembly for tessellation evaluation shaders
-   l3 - emit messages about the new L3 state during transitions
-   do32 - generate compute shader SIMD32 programs even if workgroup size 
doesn't exceed the SIMD16 limit
-   norbc - disable single sampled render buffer compression
+   tex - emit messages about textures.
+   urb - emit messages about URB setup
+   vec4 - force vec4 mode in vertex shader
+   vert - emit messages about vertex assembly
+   vs - dump shader assembly for vertex shaders
 
 INTEL_PRECISE_TRIG - if set to 1, true or yes, then the driver prefers
accuracy over performance in trig functions.
-- 
2.11.0


Re: [Mesa-dev] [PATCH 0/3] Fix window surface handling in Android EGL code

2017-04-20 Thread Tapani Pälli
Went through these changes and everything looks good to me, only one 
typo (that causes compilation to fail) spotted on the 3rd patch. With 
these changes default wallpaper works properly \o/


For the series:
Reviewed-by: Tapani Pälli 


On 04/19/2017 10:00 AM, Tomasz Figa wrote:

This series aims to improve standard conformance and application
compatibility of Mesa EGL Android code, by changing the window surface
code to properly handle cases when the buffer queue backing the native
window is abandoned, which is relied on camera and video frameworks.

Changes from previous RFC patch:
  - split trivial code moving into a separate patch (Emil),
  - fix previous rebase error,
  - use cancelBuffer when destroying the surface to not confuse the
consumer (video framework relies on this behavior).

Tomasz Figa (3):
   egl/android: Move droid_{alloc,free}_local_buffers up the file
   egl/android; Use ANativeWindow::cancelBuffer in destroySurface
   egl/android: Dequeue buffers inside EGL calls (v2)

  src/egl/drivers/dri2/egl_dri2.h |   1 +
  src/egl/drivers/dri2/platform_android.c | 182 +---
  2 files changed, 97 insertions(+), 86 deletions(-)


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 100708] Trine 2 doesn't start on radeonsi on mesa 17

2017-04-20 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=100708

--- Comment #6 from Vedran Miletić  ---
(In reply to Nikita Krupenko from comment #5)
> Requested 'libdrm >= 2.4.79' but version of libdrm is 2.4.75
> Requested 'libdrm_amdgpu >= 2.4.79' but version of libdrm_amdgpu is 2.4.75

Can you ask Mageia to upgrade libdrm?

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are the QA Contact for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] docs/features: mark KHR_no_error as started

2017-04-20 Thread Kai Wasserbäch
The OpenGL extension KHR_no_error is exposed since commit
d42d150ad26e29d9e894ba9f9e28f8134e2e5393 by Timothy Arceri. Therefore it
should be marked as "started" in the features.txt

Signed-off-by: Kai Wasserbäch 
---

Hey,
this is a rather trivial "patch" (doesn't really deserve that designation), but
I like the documentation to be in line with reality. AFAICT "started" is the
correct state for now. If it is considered "DONE" already, let me know and I
send a v2.

Cheers,
Kai

P.S.: Please push this for me, if you accept it, since I do not have commit 
access.


 docs/features.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/features.txt b/docs/features.txt
index 5f63632e82..a0d7c1be48 100644
--- a/docs/features.txt
+++ b/docs/features.txt
@@ -305,7 +305,7 @@ Khronos, ARB, and OES extensions that are not part of any 
OpenGL or OpenGL ES ve
   GL_ARB_texture_filter_minmax  not started
   GL_ARB_transform_feedback_overflow_query  DONE (i965/gen6+)
   GL_KHR_blend_equation_advanced_coherent   DONE (i965/gen9+)
-  GL_KHR_no_error   not started
+  GL_KHR_no_error   started (Timothy 
Arceri)
   GL_KHR_texture_compression_astc_hdr   DONE (core only)
   GL_KHR_texture_compression_astc_sliced_3d not started
   GL_OES_depth_texture_cube_map DONE (all drivers that 
support GLSL 1.30+)
-- 
2.11.0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 3/3] egl/android: Dequeue buffers inside EGL calls (v2)

2017-04-20 Thread Tapani Pälli



On 04/19/2017 10:00 AM, Tomasz Figa wrote:

Android buffer queues can be abandoned, which results in failing to
dequeue next buffer. Currently this would fail somewhere deep within
the DRI stack calling loader's getBuffers*(), without any error
reporting to the client app. However Android framework code relies on
proper signaling of this event, so we move buffer dequeue to
createWindowSurface() and swapBuffers() call, which can generate proper
EGL errors. To keep the performance benefits of delayed buffer handling,
if any, fence wait and DRI image creation is kept delayed until
getBuffers*() is called by the DRI driver.

v2:
  - add missing fence wait in DRI loader case,
  - split simple code moving to a separate patch (Emil),
  - fix previous rebase error.

Signed-off-by: Tomasz Figa 
---
  src/egl/drivers/dri2/egl_dri2.h |  1 +
  src/egl/drivers/dri2/platform_android.c | 94 +++--
  2 files changed, 54 insertions(+), 41 deletions(-)

diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
index f16663712d..92b234d622 100644
--- a/src/egl/drivers/dri2/egl_dri2.h
+++ b/src/egl/drivers/dri2/egl_dri2.h
@@ -288,6 +288,7 @@ struct dri2_egl_surface
  #ifdef HAVE_ANDROID_PLATFORM
 struct ANativeWindow *window;
 struct ANativeWindowBuffer *buffer;
+   int acquire_fence_fd;
 __DRIimage *dri_image_back;
 __DRIimage *dri_image_front;
  
diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c

index 9a84a4c43d..0a75d790c5 100644
--- a/src/egl/drivers/dri2/platform_android.c
+++ b/src/egl/drivers/dri2/platform_android.c
@@ -189,15 +189,9 @@ droid_free_local_buffers(struct dri2_egl_surface 
*dri2_surf)
 }
  }
  
-static EGLBoolean

-droid_window_dequeue_buffer(struct dri2_egl_surface *dri2_surf)
+static void
+wait_and_close_acquire_fence(struct dri2_egl_surface *dri2_surf)
  {
-   int fence_fd;
-
-   if (dri2_surf->window->dequeueBuffer(dri2_surf->window, _surf->buffer,
-_fd))
-  return EGL_FALSE;
-
 /* If access to the buffer is controlled by a sync fence, then block on the
  * fence.
  *
@@ -215,15 +209,25 @@ droid_window_dequeue_buffer(struct dri2_egl_surface 
*dri2_surf)
  *any value except -1) then the caller is responsible for closing the
  *file descriptor.
  */
-if (fence_fd >= 0) {
+if (dri2_surf->acquire_fence_fd >= 0) {
 /* From the SYNC_IOC_WAIT documentation in :
  *
  *Waits indefinitely if timeout < 0.
  */
  int timeout = -1;
-sync_wait(fence_fd, timeout);
-close(fence_fd);
+sync_wait(dri2_surf->acquire_fence_fd, timeout);
+close(dri2_surf->acquire_fence_fd);
+dri2_surf->acquire_fence_fd = -1;
 }
+}
+
+static EGLBoolean
+droid_window_dequeue_buffer(_EGLDisplay *disp,
+struct dri2_egl_surface *dri2_surf)
+{
+   if (dri2_surf->window->dequeueBuffer(dri2_surf->window, _surf->buffer,
+_surf->acquire_fence_fd))
+  return EGL_FALSE;
  
 dri2_surf->buffer->common.incRef(_surf->buffer->common);
  
@@ -254,6 +258,14 @@ droid_window_dequeue_buffer(struct dri2_egl_surface *dri2_surf)

dri2_surf->back = _surf->color_buffers[0];
 }
  
+   /* free outdated buffers and update the surface size */

+   if (dri2_surf->base.Width != dri2_surf->buffer->width ||
+   dri2_surf->base.Height != dri2_surf->buffer->height) {
+  droid_free_local_buffers(dri2_surf);
+  dri2_surf->base.Width = dri2_surf->buffer->width;
+  dri2_surf->base.Height = dri2_surf->buffer->height;
+   }
+
 return EGL_TRUE;
  }
  
@@ -263,6 +275,9 @@ droid_window_enqueue_buffer(_EGLDisplay *disp,

  {
 struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
  
+   /* In case we haven't done any rendering. */

+   wait_and_close_acquire_fence(dri2_surf);
+
 /* To avoid blocking other EGL calls, release the display mutex before
  * we enter droid_window_enqueue_buffer() and re-acquire the mutex upon
  * return.
@@ -319,6 +334,7 @@ droid_create_surface(_EGLDriver *drv, _EGLDisplay *disp, 
EGLint type,
_eglError(EGL_BAD_ALLOC, "droid_create_surface");
return NULL;
 }
+   dri2_surf->acquire_fence_fd = -1;
  
 if (!_eglInitSurface(_surf->base, disp, type, conf, attrib_list))

goto cleanup_surface;
@@ -360,10 +376,18 @@ droid_create_surface(_EGLDriver *drv, _EGLDisplay *disp, 
EGLint type,
 if (window) {
window->common.incRef(>common);
dri2_surf->window = window;
+  if (!droid_window_dequeue_buffer(disp, dri2_surf)) {
+ _eglError(EGL_BAD_SURFACE, "Could not dequeue buffer from native 
window");
+ goto cleanup_window:


typo there, should be ; not :


+  }
 }
  
 return _surf->base;
  
+cleanup_window:

+   window->common.decRef(>common);
+   

[Mesa-dev] [Bug 100708] Trine 2 doesn't start on radeonsi on mesa 17

2017-04-20 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=100708

--- Comment #5 from Nikita Krupenko  ---
(In reply to Michel Dänzer from comment #3)
> Nikita, can you try if it still happens with Mesa Git master?

It doesn't compile:

Requested 'libdrm >= 2.4.79' but version of libdrm is 2.4.75
Requested 'libdrm_amdgpu >= 2.4.79' but version of libdrm_amdgpu is 2.4.75

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are the QA Contact for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v5 0/3] asynchronous pbo transfer with glthread

2017-04-20 Thread Marek Olšák
On Thu, Apr 20, 2017 at 10:28 AM, gregory hainaut
 wrote:
> On Thu, 20 Apr 2017 12:29:11 +0900
> Michel Dänzer  wrote:
>
>> On 20/04/17 01:54 AM, Gregory Hainaut wrote:
>> > Hello,
>> >
>> > Please find the latest version that include a small fix for hash deletion. 
>> > I
>> > think the series is good now. Please review/ack it.
>>
>> I'm afraid I have to NACK it. As discussed in the v4 cover letter
>> thread, Mesa's glthread cannot make any libX11 API calls.
>>
>>
>
> Hello Michel,
>
> Just to be sure we are on the same line, let's me do a summary.
>
> PCSX2 does the following bad pattern
> 1/ no call to XInitThread
> 2/ XGetGeometry to get the size of the surface
> 3/ glDrawArray into the default framebuffer 0 (the window, I'm not sure how 
> to call it)
>=> which seem to call DRI2GetBuffersWithFormat under the hood. I guess to 
> get the
>   associated buffer of the window.
>
>
> So far it was (kind of) working fine because PCSX2 does tons of PBO transfer. 
> So glthread
> was mostly synchronous.
>
> This series removes the (useless) PBO transfer synchronization. So now 
> glthread is really
> asynchronous and the above bad pattern crash as expected.
>
> I didn't add any libX11 API call on the patches. And I don't think we can 
> remvove the DRI stuff.
>
> Hum, I don't know what will be the impact on the perf but potentially we can 
> force a synchronization
> when there is a draw to framebuffer 0.

Can you send us the backtrace when DRI2GetBuffersWithFormat is called
by glDrawArrays?

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 100708] Trine 2 doesn't start on radeonsi on mesa 17

2017-04-20 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=100708

--- Comment #4 from Marek Olšák  ---
That commit is unlikely to cause any issues with X11.

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are the QA Contact for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 2/2] i965/vec4: load dvec3/4 uniforms first in the push constant buffer

2017-04-20 Thread Samuel Iglesias Gonsálvez
Reorder the uniforms to load first the dvec4-aligned variables
in the push constant buffer and then push the vec4-aligned ones.

This fixes a bug were the dvec3/4 might be loaded one part on a GRF and
the rest in next GRF, so the region parameters to read that could break
the HW rules.

Signed-off-by: Samuel Iglesias Gonsálvez 
Cc: curroje...@riseup.net
---
 src/intel/compiler/brw_vec4.cpp | 86 +++--
 1 file changed, 65 insertions(+), 21 deletions(-)

diff --git a/src/intel/compiler/brw_vec4.cpp b/src/intel/compiler/brw_vec4.cpp
index 0b92ba704e5..4ba59f54313 100644
--- a/src/intel/compiler/brw_vec4.cpp
+++ b/src/intel/compiler/brw_vec4.cpp
@@ -583,16 +583,43 @@ vec4_visitor::split_uniform_registers()
}
 }
 
+/* This function returns the register number where we placed the uniform */
+static int
+set_push_constant_loc(const int nr_uniforms, int *new_uniform_count,
+  const int src, const int size,
+  int *new_loc, int *new_chan,
+  int *new_chans_used)
+{
+   int dst;
+   /* Find the lowest place we can slot this uniform in. */
+   for (dst = *new_uniform_count; dst < nr_uniforms; dst++) {
+  if (new_chans_used[dst] + size <= 4)
+ break;
+   }
+
+   assert(dst < nr_uniforms);
+
+   new_loc[src] = dst;
+   new_chan[src] = new_chans_used[dst];
+
+   *new_uniform_count = MAX2(*new_uniform_count, dst + 1);
+   return dst;
+}
+
 void
 vec4_visitor::pack_uniform_registers()
 {
uint8_t chans_used[this->uniforms];
int new_loc[this->uniforms];
int new_chan[this->uniforms];
+   bool is_aligned_to_dvec4[this->uniforms];
+   int new_chans_used[this->uniforms];
 
memset(chans_used, 0, sizeof(chans_used));
memset(new_loc, 0, sizeof(new_loc));
memset(new_chan, 0, sizeof(new_chan));
+   memset(new_chans_used, 0, sizeof(new_chans_used));
+   memset(is_aligned_to_dvec4, 0, sizeof(is_aligned_to_dvec4));
 
/* Find which uniform vectors are actually used by the program.  We
 * expect unused vector elements when we've moved array access out
@@ -631,10 +658,13 @@ vec4_visitor::pack_uniform_registers()
 
 unsigned channel = BRW_GET_SWZ(inst->src[i].swizzle, c) + 1;
 unsigned used = MAX2(chans_used[reg], channel * channel_size);
-if (used <= 4)
+if (used <= 4) {
chans_used[reg] = used;
-else
+} else {
+   is_aligned_to_dvec4[reg] = true;
+   is_aligned_to_dvec4[reg + 1] = true;
chans_used[reg + 1] = used - 4;
+}
  }
   }
 
@@ -659,42 +689,56 @@ vec4_visitor::pack_uniform_registers()
 
int new_uniform_count = 0;
 
+   /* As the uniforms are going to be reordered, take the data from a temporary
+* copy of the original param[].
+*/
+   gl_constant_value **param = ralloc_array(NULL, gl_constant_value*,
+stage_prog_data->nr_params);
+   memcpy(param, stage_prog_data->param,
+  sizeof(gl_constant_value*) * stage_prog_data->nr_params);
+
/* Now, figure out a packing of the live uniform vectors into our
-* push constants.
+* push constants. Start with dvec{3,4} because they are aligned to
+* dvec4 size (2 vec4).
 */
for (int src = 0; src < uniforms; src++) {
   int size = chans_used[src];
 
-  if (size == 0)
+  if (size == 0 || !is_aligned_to_dvec4[src])
  continue;
 
-  int dst;
-  /* Find the lowest place we can slot this uniform in. */
-  for (dst = 0; dst < src; dst++) {
- if (chans_used[dst] + size <= 4)
-break;
+  int dst = set_push_constant_loc(uniforms, _uniform_count,
+  src, size, new_loc, new_chan,
+  new_chans_used);
+  if (dst != src) {
+ /* Move the references to the data */
+ for (int j = 0; j < size; j++) {
+stage_prog_data->param[dst * 4 + new_chan[src] + j] =
+   param[src * 4 + j];
+ }
   }
+   }
 
-  if (src == dst) {
- new_loc[src] = dst;
- new_chan[src] = 0;
-  } else {
- new_loc[src] = dst;
- new_chan[src] = chans_used[dst];
+   /* Continue with the rest of data, which is aligned to vec4. */
+   for (int src = 0; src < uniforms; src++) {
+  int size = chans_used[src];
+
+  if (size == 0 || is_aligned_to_dvec4[src])
+ continue;
 
+  int dst = set_push_constant_loc(uniforms, _uniform_count,
+  src, size, new_loc, new_chan,
+  new_chans_used);
+  if (dst != src) {
  /* Move the references to the data */
  for (int j = 0; j < size; j++) {
 stage_prog_data->param[dst * 4 + new_chan[src] + j] =
-   stage_prog_data->param[src * 4 + j];
+   param[src * 4 + j];

[Mesa-dev] [PATCH 1/2] i965/vec4: set swizzle when loading an uniform

2017-04-20 Thread Samuel Iglesias Gonsálvez
It was setting XYWZ swizzle to all uniforms, no matter if they were
a vector or not.

Signed-off-by: Samuel Iglesias Gonsálvez 
Cc: curroje...@riseup.net
---
 src/intel/compiler/brw_vec4_nir.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/intel/compiler/brw_vec4_nir.cpp 
b/src/intel/compiler/brw_vec4_nir.cpp
index a82d52088a8..5f4488c7e86 100644
--- a/src/intel/compiler/brw_vec4_nir.cpp
+++ b/src/intel/compiler/brw_vec4_nir.cpp
@@ -863,6 +863,7 @@ vec4_visitor::nir_emit_intrinsic(nir_intrinsic_instr *instr)
  unsigned offset = const_offset->u32[0] + shift * 4;
  src.offset = ROUND_DOWN_TO(offset, 16);
  shift = (offset % 16) / 4;
+ src.swizzle = brw_swizzle_for_size(instr->num_components);
  src.swizzle += BRW_SWIZZLE4(shift, shift, shift, shift);
 
  emit(MOV(dest, src));
-- 
2.11.0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [RFC 1/2] Revert "mapi: rewrite u_current_init() function without u_thread_self()"

2017-04-20 Thread Emil Velikov
On 20 April 2017 at 00:36, Timothy Arceri  wrote:
> On 19/04/17 20:32, Emil Velikov wrote:
>>
>> On 19 April 2017 at 08:45, Timothy Arceri  wrote:
>>>
>>> This reverts commit 458c7490c29ef2960a33a089f65490e044da5d27.
>>>
>>> The commit did not revert cleanly so this was fixed up by hand.
>>>
>>> u_thread_self() will be used by the following patch.
>>
>> I see your interesting in dropping the legacy/unused extension with
>> 2/2 but this commit is bonkers :-(
>
>
> This just reverts the code to how it used to be.
>
Bringing bugs back is never a good idea.

>>
>>
>>> -   else if (!thread_id_equal(knownID, get_thread_id())) {
>>> +   else if (knownID != u_thread_self()) {
>>
>> NACK. Direct comparison as seen here is broken and if it works that's
>> only by coincidence.
>
>
> In what way is it broken?
>
To quote the pthread manual:

pthread_self
... Therefore, variables of type pthread_t can't portably be compared
using the C equality operator (==); use pthread_equal(3) instead.

pthread_equal
...The  pthread_equal() function is necessary because thread IDs
should be considered opaque: there is no portable way for applications
to directly compare two pthread_t values.

On the Windows side things are somewhat similar.

-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v2] ac: fix build after LLVM 5.0 SVN r300718

2017-04-20 Thread Mike Lothian
Thanks for this

Tested-and-reviewed-by: Mike Lothian 

On Thu, 20 Apr 2017 at 09:48 Nicolai Hähnle  wrote:

> On 20.04.2017 10:34, Christoph Haag wrote:
> > v2: previously getWithDereferenceableBytes() exists, but addAttr()
> doesn't take that type
> >
> > Signed-off-by: Christoph Haag 
>
> Reviewed-by: Nicolai Hähnle 
>
>
> > ---
> >  src/amd/common/ac_llvm_helper.cpp | 4 
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/src/amd/common/ac_llvm_helper.cpp
> b/src/amd/common/ac_llvm_helper.cpp
> > index d9ea4b162e..11fa80920d 100644
> > --- a/src/amd/common/ac_llvm_helper.cpp
> > +++ b/src/amd/common/ac_llvm_helper.cpp
> > @@ -44,9 +44,13 @@ typedef AttributeSet AttributeList;
> >  void ac_add_attr_dereferenceable(LLVMValueRef val, uint64_t bytes)
> >  {
> > llvm::Argument *A = llvm::unwrap(val);
> > +#if HAVE_LLVM < 0x0500
> > llvm::AttrBuilder B;
> > B.addDereferenceableAttr(bytes);
> > A->addAttr(llvm::AttributeList::get(A->getContext(), A->getArgNo() +
> 1,  B));
> > +#else
> > +
>  A->addAttr(llvm::Attribute::getWithDereferenceableBytes(A->getContext(),
> bytes));
> > +#endif
> >  }
> >
> >  bool ac_is_sgpr_param(LLVMValueRef arg)
> >
>
>
> --
> Lerne, wie die Welt wirklich ist,
> Aber vergiss niemals, wie sie sein sollte.
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v2] ac: fix build after LLVM 5.0 SVN r300718

2017-04-20 Thread Nicolai Hähnle

On 20.04.2017 10:34, Christoph Haag wrote:

v2: previously getWithDereferenceableBytes() exists, but addAttr() doesn't take 
that type

Signed-off-by: Christoph Haag 


Reviewed-by: Nicolai Hähnle 



---
 src/amd/common/ac_llvm_helper.cpp | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/amd/common/ac_llvm_helper.cpp 
b/src/amd/common/ac_llvm_helper.cpp
index d9ea4b162e..11fa80920d 100644
--- a/src/amd/common/ac_llvm_helper.cpp
+++ b/src/amd/common/ac_llvm_helper.cpp
@@ -44,9 +44,13 @@ typedef AttributeSet AttributeList;
 void ac_add_attr_dereferenceable(LLVMValueRef val, uint64_t bytes)
 {
llvm::Argument *A = llvm::unwrap(val);
+#if HAVE_LLVM < 0x0500
llvm::AttrBuilder B;
B.addDereferenceableAttr(bytes);
A->addAttr(llvm::AttributeList::get(A->getContext(), A->getArgNo() + 1,  
B));
+#else
+   A->addAttr(llvm::Attribute::getWithDereferenceableBytes(A->getContext(), 
bytes));
+#endif
 }

 bool ac_is_sgpr_param(LLVMValueRef arg)




--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH v2] ac: fix build after LLVM 5.0 SVN r300718

2017-04-20 Thread Christoph Haag
v2: previously getWithDereferenceableBytes() exists, but addAttr() doesn't take 
that type

Signed-off-by: Christoph Haag 
---
 src/amd/common/ac_llvm_helper.cpp | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/amd/common/ac_llvm_helper.cpp 
b/src/amd/common/ac_llvm_helper.cpp
index d9ea4b162e..11fa80920d 100644
--- a/src/amd/common/ac_llvm_helper.cpp
+++ b/src/amd/common/ac_llvm_helper.cpp
@@ -44,9 +44,13 @@ typedef AttributeSet AttributeList;
 void ac_add_attr_dereferenceable(LLVMValueRef val, uint64_t bytes)
 {
llvm::Argument *A = llvm::unwrap(val);
+#if HAVE_LLVM < 0x0500
llvm::AttrBuilder B;
B.addDereferenceableAttr(bytes);
A->addAttr(llvm::AttributeList::get(A->getContext(), A->getArgNo() + 1,  
B));
+#else
+   A->addAttr(llvm::Attribute::getWithDereferenceableBytes(A->getContext(), 
bytes));
+#endif
 }
 
 bool ac_is_sgpr_param(LLVMValueRef arg)
-- 
2.12.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v5 0/3] asynchronous pbo transfer with glthread

2017-04-20 Thread gregory hainaut
On Thu, 20 Apr 2017 12:29:11 +0900
Michel Dänzer  wrote:

> On 20/04/17 01:54 AM, Gregory Hainaut wrote:
> > Hello,
> > 
> > Please find the latest version that include a small fix for hash deletion. I
> > think the series is good now. Please review/ack it.
> 
> I'm afraid I have to NACK it. As discussed in the v4 cover letter
> thread, Mesa's glthread cannot make any libX11 API calls.
> 
> 

Hello Michel,

Just to be sure we are on the same line, let's me do a summary.

PCSX2 does the following bad pattern
1/ no call to XInitThread
2/ XGetGeometry to get the size of the surface
3/ glDrawArray into the default framebuffer 0 (the window, I'm not sure how to 
call it)
   => which seem to call DRI2GetBuffersWithFormat under the hood. I guess to 
get the
  associated buffer of the window.


So far it was (kind of) working fine because PCSX2 does tons of PBO transfer. 
So glthread
was mostly synchronous.

This series removes the (useless) PBO transfer synchronization. So now glthread 
is really
asynchronous and the above bad pattern crash as expected. 

I didn't add any libX11 API call on the patches. And I don't think we can 
remvove the DRI stuff.

Hum, I don't know what will be the impact on the perf but potentially we can 
force a synchronization
when there is a draw to framebuffer 0.

Cheers,
Gregory
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] ac: fix build after LLVM 5.0 SVN r300718

2017-04-20 Thread Christoph Haag
From: Christoph Haag 

Signed-off-by: Christoph Haag 
---
 src/amd/common/ac_llvm_helper.cpp | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/amd/common/ac_llvm_helper.cpp 
b/src/amd/common/ac_llvm_helper.cpp
index d9ea4b162e..69d65ddffe 100644
--- a/src/amd/common/ac_llvm_helper.cpp
+++ b/src/amd/common/ac_llvm_helper.cpp
@@ -44,9 +44,7 @@ typedef AttributeSet AttributeList;
 void ac_add_attr_dereferenceable(LLVMValueRef val, uint64_t bytes)
 {
llvm::Argument *A = llvm::unwrap(val);
-   llvm::AttrBuilder B;
-   B.addDereferenceableAttr(bytes);
-   A->addAttr(llvm::AttributeList::get(A->getContext(), A->getArgNo() + 1,  
B));
+   A->addAttr(llvm::Attribute::getWithDereferenceableBytes(A->getContext(), 
bytes));
 }
 
 bool ac_is_sgpr_param(LLVMValueRef arg)
-- 
2.12.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/3] genxml/pack: Allow hex values in the XML

2017-04-20 Thread Iago Toral
All 3 patches (assuming Dylan's fine with patch 1) are:

Reviewed by: Iago Toral Quiroga 


On Wed, 2017-04-19 at 17:17 -0700, Jason Ekstrand wrote:
> ---
>  src/intel/genxml/gen_pack_header.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/intel/genxml/gen_pack_header.py
> b/src/intel/genxml/gen_pack_header.py
> index 2a70945..5b55143 100644
> --- a/src/intel/genxml/gen_pack_header.py
> +++ b/src/intel/genxml/gen_pack_header.py
> @@ -7,6 +7,7 @@ import xml.parsers.expat
>  import re
>  import sys
>  import copy
> +import ast
>  
>  license =  """/*
>   * Copyright (C) 2016 Intel Corporation
> @@ -476,7 +477,7 @@ class Group(object):
>  class Value(object):
>  def __init__(self, attrs):
>  self.name = safe_name(attrs["name"])
> -self.value = int(attrs["value"])
> +self.value = ast.literal_eval(attrs["value"])
>  
>  class Parser(object):
>  def __init__(self):
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v2 01/18] anv: Add the pci_id into the shader cache UUID

2017-04-20 Thread Tapani Pälli



On 03/14/2017 04:26 AM, Jason Ekstrand wrote:

This prevents a user from using a cache created on one hardware
generation on a different one.  Of course, with Intel hardware, this
requires moving their drive from one machine to another but it's still
possible and we should prevent it.

Reviewed-by: Chad Versace 
---
  src/intel/vulkan/anv_device.c | 22 +-
  1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
index be2b2f3..5db0a6c 100644
--- a/src/intel/vulkan/anv_device.c
+++ b/src/intel/vulkan/anv_device.c
@@ -32,6 +32,7 @@
  #include "util/strtod.h"
  #include "util/debug.h"
  #include "util/build_id.h"
+#include "util/mesa-sha1.h"
  #include "util/vk_util.h"
  
  #include "genxml/gen7_pack.h"

@@ -53,17 +54,28 @@ compiler_perf_log(void *data, const char *fmt, ...)
  }
  
  static bool

-anv_device_get_cache_uuid(void *uuid)
+anv_device_get_cache_uuid(void *uuid, uint16_t pci_id)
  {
 const struct build_id_note *note = 
build_id_find_nhdr("libvulkan_intel.so");
 if (!note)
return false;
  
-   unsigned len = build_id_length(note);

-   if (len < VK_UUID_SIZE)
+   unsigned build_id_len = build_id_length(note);
+   if (build_id_len < 20) /* It should be a SHA-1 */


This line here causes issues for Android, for us the build_id_len seems 
to be 16 (and worked before nicely as check was against VK_UUID_SIZE 
which is 16). I just bisected this and haven't yet figured out why but 
will check.


// Tapani
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] i965: Solve Android native fence fd double close issue

2017-04-20 Thread Tapani Pälli

Ping Chad

On 04/20/2017 08:20 AM, Xu, Randy wrote:

Any comments to this patch?

Thanks,
Randy



-Original Message-
From: Xu, Randy
Sent: Tuesday, April 18, 2017 2:27 PM
To: mesa-dev@lists.freedesktop.org
Cc: Palli, Tapani ; Xu, Randy 
Subject: [PATCH] i965: Solve Android native fence fd double close issue

The Android native fence in i965 driver has two fd, one is from App and
stored in _EGLSync.SyncFd; Another one brw_fence->sync_fd should be the
dup of the first one, not direct copy.
These two fds are closed in dri2_egl_unref_sync in sequence.

Test: Run Vulkan and GLES stress test and no crash.

Signed-off-by: Randy Xu 
---
  src/mesa/drivers/dri/i965/brw_sync.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/brw_sync.c
b/src/mesa/drivers/dri/i965/brw_sync.c
index 5b78503..a8356c3 100644
--- a/src/mesa/drivers/dri/i965/brw_sync.c
+++ b/src/mesa/drivers/dri/i965/brw_sync.c
@@ -470,7 +470,7 @@ brw_dri_create_fence_fd(__DRIcontext *dri_ctx, int
fd)
   goto fail;
 } else {
/* Import the sync fd as an in-fence. */
-  fence->sync_fd = fd;
+  fence->sync_fd = dup(fd);
 }

 assert(fence->sync_fd != -1);
--
2.7.4



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev