Re: [Mesa-dev] [PATCH 2/6] anv/cmd_buffer: Add some helpers for working with descriptor sets

[Jordan Justen]

Reviewed-by: Jordan Justen 

On 2017-12-01 17:20:05, Jason Ekstrand wrote:
> ---
>  src/intel/vulkan/genX_cmd_buffer.c | 45 
> --
>  1 file changed, 34 insertions(+), 11 deletions(-)
> 
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
> b/src/intel/vulkan/genX_cmd_buffer.c
> index ab5590d..e4362d1 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -1432,6 +1432,35 @@ cmd_buffer_alloc_push_constants(struct anv_cmd_buffer 
> *cmd_buffer)
> cmd_buffer->state.push_constants_dirty |= VK_SHADER_STAGE_ALL_GRAPHICS;
>  }
>  
> +static const struct anv_descriptor *
> +anv_descriptor_for_binding(const struct anv_cmd_buffer *cmd_buffer,
> +   const struct anv_pipeline_binding *binding)
> +{
> +   assert(binding->set < MAX_SETS);
> +   const struct anv_descriptor_set *set =
> +  cmd_buffer->state.descriptors[binding->set];
> +   const uint32_t offset =
> +  set->layout->binding[binding->binding].descriptor_index;
> +   return >descriptors[offset + binding->index];
> +}
> +
> +static uint32_t
> +dynamic_offset_for_binding(const struct anv_cmd_buffer *cmd_buffer,
> +   const struct anv_pipeline *pipeline,
> +   const struct anv_pipeline_binding *binding)
> +{
> +   assert(binding->set < MAX_SETS);
> +   const struct anv_descriptor_set *set =
> +  cmd_buffer->state.descriptors[binding->set];
> +
> +   uint32_t dynamic_offset_idx =
> +  pipeline->layout->set[binding->set].dynamic_offset_start +
> +  set->layout->binding[binding->binding].dynamic_offset_index +
> +  binding->index;
> +
> +   return cmd_buffer->state.dynamic_offsets[dynamic_offset_idx];
> +}
> +
>  static VkResult
>  emit_binding_table(struct anv_cmd_buffer *cmd_buffer,
> gl_shader_stage stage,
> @@ -1534,10 +1563,8 @@ emit_binding_table(struct anv_cmd_buffer *cmd_buffer,
>   continue;
>}
>  
> -  struct anv_descriptor_set *set =
> - cmd_buffer->state.descriptors[binding->set];
> -  uint32_t offset = 
> set->layout->binding[binding->binding].descriptor_index;
> -  struct anv_descriptor *desc = >descriptors[offset + 
> binding->index];
> +  const struct anv_descriptor *desc =
> + anv_descriptor_for_binding(cmd_buffer, binding);
>  
>switch (desc->type) {
>case VK_DESCRIPTOR_TYPE_SAMPLER:
> @@ -1611,14 +1638,10 @@ emit_binding_table(struct anv_cmd_buffer *cmd_buffer,
>  
>case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC:
>case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC: {
> - uint32_t dynamic_offset_idx =
> -pipeline->layout->set[binding->set].dynamic_offset_start +
> -set->layout->binding[binding->binding].dynamic_offset_index +
> -binding->index;
> -
>   /* Compute the offset within the buffer */
> - uint64_t offset = desc->offset +
> -cmd_buffer->state.dynamic_offsets[dynamic_offset_idx];
> + uint32_t dynamic_offset =
> +dynamic_offset_for_binding(cmd_buffer, pipeline, binding);
> + uint64_t offset = desc->offset + dynamic_offset;
>   /* Clamp to the buffer size */
>   offset = MIN2(offset, desc->buffer->size);
>   /* Clamp the range to the buffer size */
> -- 
> 2.5.0.400.gff86faf
> 
> ___
> 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 1/6] anv/pipeline: Translate vulkan_resource_index to a constant when possible

[Jordan Justen]

On 2017-12-01 17:20:04, Jason Ekstrand wrote:
> We want to call brw_nir_analyze_ubo_ranges immedately after
> anv_nir_apply_pipeline_layout and it badly wants constants.  We could
> run an optimization step and let constant folding do it but that's way
> more expensive than needed.  It's really easy to just handle constants
> in apply_pipeline_layout.
> ---
>  src/intel/vulkan/anv_nir_apply_pipeline_layout.c | 15 +++
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/src/intel/vulkan/anv_nir_apply_pipeline_layout.c 
> b/src/intel/vulkan/anv_nir_apply_pipeline_layout.c
> index f8d8164..4f7680b 100644
> --- a/src/intel/vulkan/anv_nir_apply_pipeline_layout.c
> +++ b/src/intel/vulkan/anv_nir_apply_pipeline_layout.c
> @@ -116,12 +116,19 @@ lower_res_index_intrinsic(nir_intrinsic_instr *intrin,
> uint32_t array_size =
>state->layout->set[set].layout->binding[binding].array_size;
>  
> -   nir_ssa_def *block_index = nir_ssa_for_src(b, intrin->src[0], 1);
> +   nir_const_value *const_block_index = 
> nir_src_as_const_value(intrin->src[0]);
>  
> -   if (state->add_bounds_checks)
> -  block_index = nir_umin(b, block_index, nir_imm_int(b, array_size - 1));
> +   nir_ssa_def *block_index;
> +   if (const_block_index) {
> +  block_index = nir_imm_int(b, surface_index + 
> const_block_index->u32[0]);

I'm guessing that if this is constant, then it'll have been bounds
checked elsewhere? If not, I guess you could do something easy here
with array_size.

Reviewed-by: Jordan Justen 

> +   } else {
> +  block_index = nir_ssa_for_src(b, intrin->src[0], 1);
>  
> -   block_index = nir_iadd(b, nir_imm_int(b, surface_index), block_index);
> +  if (state->add_bounds_checks)
> + block_index = nir_umin(b, block_index, nir_imm_int(b, array_size - 
> 1));
> +
> +  block_index = nir_iadd(b, nir_imm_int(b, surface_index), block_index);
> +   }
>  
> assert(intrin->dest.is_ssa);
> nir_ssa_def_rewrite_uses(>dest.ssa, nir_src_for_ssa(block_index));
> -- 
> 2.5.0.400.gff86faf
> 
> ___
> 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 7/9] mesa: move ATI_fs state compile changes after the error checks

[Miklós Máté]

Both in setup and arithmetic instructions. Also, remove the useless
new_*_inst() functions, and refactor check_arith_arg(), because it did
two completely different things.

Piglit: spec/ati_fragment_shader/error04-endshader

Signed-off-by: Miklós Máté 
---
 src/mesa/main/atifragshader.c | 107 +-
 1 file changed, 53 insertions(+), 54 deletions(-)

diff --git a/src/mesa/main/atifragshader.c b/src/mesa/main/atifragshader.c
index 4aebaf659e..8538e3a53e 100644
--- a/src/mesa/main/atifragshader.c
+++ b/src/mesa/main/atifragshader.c
@@ -73,21 +73,6 @@ _mesa_delete_ati_fragment_shader(struct gl_context *ctx, 
struct ati_fragment_sha
 }
 
 
-
-static void
-new_arith_inst(struct ati_fragment_shader *prog)
-{
-/* set "default" instruction as not all may get defined.
-   there is no specified way to express a nop with ati fragment shaders we use
-   GL_NONE as the op enum and just set some params to 0 - so nothing to do 
here */
-   prog->numArithInstr[prog->cur_pass >> 1]++;
-}
-
-static void
-new_tex_inst(struct ati_fragment_shader *prog)
-{
-}
-
 static void match_pair_inst(struct ati_fragment_shader *curProg, GLuint optype)
 {
if (optype == curProg->last_optype) {
@@ -159,8 +144,8 @@ static void debug_op(GLint optype, GLuint arg_count, GLenum 
op, GLuint dst,
 }
 #endif
 
-static int check_arith_arg(struct ati_fragment_shader *curProg,
-   GLuint optype, GLuint arg, GLuint argRep)
+static int
+check_arith_arg(GLuint optype, GLuint arg, GLuint argRep)
 {
GET_CURRENT_CONTEXT(ctx);
 
@@ -188,13 +173,17 @@ static int check_arith_arg(struct ati_fragment_shader 
*curProg,
  return 0;
   }
}
-   if ((curProg->cur_pass == 1) &&
-  ((arg == GL_PRIMARY_COLOR_ARB) || (arg == 
GL_SECONDARY_INTERPOLATOR_ATI))) {
-  curProg->interpinp1 = GL_TRUE;
-   }
return 1;
 }
 
+static GLboolean
+check_arg_color(GLubyte pass, GLuint arg)
+{
+   if (pass == 1 && (arg == GL_PRIMARY_COLOR_ARB || arg == 
GL_SECONDARY_INTERPOLATOR_ATI))
+ return GL_TRUE;
+   return GL_FALSE;
+}
+
 GLuint GLAPIENTRY
 _mesa_GenFragmentShadersATI(GLuint range)
 {
@@ -441,18 +430,17 @@ _mesa_PassTexCoordATI(GLuint dst, GLuint coord, GLenum 
swizzle)
GET_CURRENT_CONTEXT(ctx);
struct ati_fragment_shader *curProg = ctx->ATIFragmentShader.Current;
struct atifs_setupinst *curI;
+   GLubyte new_pass = curProg->cur_pass;
 
if (!ctx->ATIFragmentShader.Compiling) {
   _mesa_error(ctx, GL_INVALID_OPERATION, 
"glPassTexCoordATI(outsideShader)");
   return;
}
 
-   if (curProg->cur_pass == 1) {
-  match_pair_inst(curProg, 0);
-  curProg->cur_pass = 2;
-   }
-   if ((curProg->cur_pass > 2) ||
-  ((1 << (dst - GL_REG_0_ATI)) & curProg->regsAssigned[curProg->cur_pass 
>> 1])) {
+   if (curProg->cur_pass == 1)
+  new_pass = 2;
+   if ((new_pass > 2) ||
+  ((1 << (dst - GL_REG_0_ATI)) & curProg->regsAssigned[new_pass >> 1])) {
   _mesa_error(ctx, GL_INVALID_OPERATION, "glPassTexCoord(pass)");
   return;
}
@@ -467,7 +455,7 @@ _mesa_PassTexCoordATI(GLuint dst, GLuint coord, GLenum 
swizzle)
   _mesa_error(ctx, GL_INVALID_ENUM, "glPassTexCoordATI(coord)");
   return;
}
-   if ((curProg->cur_pass == 0) && (coord >= GL_REG_0_ATI)) {
+   if ((new_pass == 0) && (coord >= GL_REG_0_ATI)) {
   _mesa_error(ctx, GL_INVALID_OPERATION, "glPassTexCoordATI(coord)");
   return;
}
@@ -490,8 +478,10 @@ _mesa_PassTexCoordATI(GLuint dst, GLuint coord, GLenum 
swizzle)
   }
}
 
+   if (curProg->cur_pass == 1)
+  match_pair_inst(curProg, 0);
+   curProg->cur_pass = new_pass;
curProg->regsAssigned[curProg->cur_pass >> 1] |= 1 << (dst - GL_REG_0_ATI);
-   new_tex_inst(curProg);
 
/* add the instructions */
curI = >SetupInst[curProg->cur_pass >> 1][dst - GL_REG_0_ATI];
@@ -513,18 +503,17 @@ _mesa_SampleMapATI(GLuint dst, GLuint interp, GLenum 
swizzle)
GET_CURRENT_CONTEXT(ctx);
struct ati_fragment_shader *curProg = ctx->ATIFragmentShader.Current;
struct atifs_setupinst *curI;
+   GLubyte new_pass = curProg->cur_pass;
 
if (!ctx->ATIFragmentShader.Compiling) {
   _mesa_error(ctx, GL_INVALID_OPERATION, "glSampleMapATI(outsideShader)");
   return;
}
 
-   if (curProg->cur_pass == 1) {
-  match_pair_inst(curProg, 0);
-  curProg->cur_pass = 2;
-   }
-   if ((curProg->cur_pass > 2) ||
-  ((1 << (dst - GL_REG_0_ATI)) & curProg->regsAssigned[curProg->cur_pass 
>> 1])) {
+   if (curProg->cur_pass == 1)
+  new_pass = 2;
+   if ((new_pass > 2) ||
+  ((1 << (dst - GL_REG_0_ATI)) & curProg->regsAssigned[new_pass >> 1])) {
   _mesa_error(ctx, GL_INVALID_OPERATION, "glSampleMapATI(pass)");
   return;
}
@@ -540,7 +529,7 @@ _mesa_SampleMapATI(GLuint dst, GLuint interp, GLenum 
swizzle)
   _mesa_error(ctx, GL_INVALID_ENUM, "glSampleMapATI(interp)");
   return;
}
-   if ((curProg->cur_pass == 0) && (interp >= GL_REG_0_ATI)) {
+ 

[Mesa-dev] [PATCH 9/9] mesa: always compare optype with symbolic name in ATI_fs

[Miklós Máté]

Signed-off-by: Miklós Máté 
---
 src/mesa/main/atifragshader.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/mesa/main/atifragshader.c b/src/mesa/main/atifragshader.c
index 8538e3a53e..6b636f1dc7 100644
--- a/src/mesa/main/atifragshader.c
+++ b/src/mesa/main/atifragshader.c
@@ -76,7 +76,7 @@ _mesa_delete_ati_fragment_shader(struct gl_context *ctx, 
struct ati_fragment_sha
 static void match_pair_inst(struct ati_fragment_shader *curProg, GLuint optype)
 {
if (optype == curProg->last_optype) {
-  curProg->last_optype = 1;
+  curProg->last_optype = ATI_FRAGMENT_SHADER_ALPHA_OP;
}
 }
 
@@ -125,7 +125,7 @@ static void debug_op(GLint optype, GLuint arg_count, GLenum 
op, GLuint dst,
   
   fprintf(stderr, "%s(%s, %s", op_name, _mesa_enum_to_string(op),
  _mesa_enum_to_string(dst));
-  if (!optype)
+  if (optype == ATI_FRAGMENT_SHADER_COLOR_OP)
 fprintf(stderr, ", %d", dstMask);
   
   fprintf(stderr, ", %s", create_dst_mod_str(dstMod));
@@ -631,7 +631,7 @@ _mesa_FragmentOpXATI(GLint optype, GLuint arg_count, GLenum 
op, GLuint dst,
   _mesa_error(ctx, GL_INVALID_ENUM, "C/AFragmentOpATI(op)");
   return;
}
-   if (optype == 1) {
+   if (optype == ATI_FRAGMENT_SHADER_ALPHA_OP) {
   if (((op == GL_DOT2_ADD_ATI) && (curI->Opcode[0] != GL_DOT2_ADD_ATI)) ||
 ((op == GL_DOT3_ATI) && (curI->Opcode[0] != GL_DOT3_ATI)) ||
 ((op == GL_DOT4_ATI) && (curI->Opcode[0] != GL_DOT4_ATI)) ||
-- 
2.15.0.rc0

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

[Mesa-dev] [PATCH 8/9] mesa: document ati_fragment_shader::cur_pass and swizzlerq

[Miklós Máté]

Signed-off-by: Miklós Máté 
---
 src/mesa/main/mtypes.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 0e8a05359a..8c8151c496 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2390,10 +2390,14 @@ struct ati_fragment_shader
GLubyte numArithInstr[2];
GLubyte regsAssigned[2];
GLubyte NumPasses; /**< 1 or 2 */
+   /** Current compile stage: 0 setup pass1, 1 arith pass1, 2 setup pass2, 3 
arith pass2 */
GLubyte cur_pass;
GLubyte last_optype;
GLboolean interpinp1;
GLboolean isValid;
+   /** Array of 2 bit values for each tex unit to remember whether
+* STR or STQ swizzle was used
+*/
GLuint swizzlerq;
struct gl_program *Program;
 };
-- 
2.15.0.rc0

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

[Mesa-dev] [PATCH 0/9] Fixes for ATI_fragment_shader

[Miklós Máté]

These are fixes for the problems my Piglit tests uncovered so far.
After this series all gallium drivers (I could only test softpipe,
llvmpipe and radeonsi) and swrast should pass all tests.
Unfortunately I can't test on r200.

Miklós Máté (9):
  mesa: add fallback texture for SampleMapATI if there is nothing
  mesa: fix crash when an ATI_fs pass begins with an alpha inst
  mesa: fix typo in ATI_fs dstMod error checking
  mesa: fix validate for secondary interpolator
  mesa: fix not having secondary color in ATI_fs in swrast
  tnl: fix not having texture coords in ATI_fs in swrast
  mesa: move ATI_fs state compile changes after the error checks
  mesa: document ati_fragment_shader::cur_pass and swizzlerq
  mesa: always compare optype with symbolic name in ATI_fs

 src/mesa/main/atifragshader.c | 163 +++---
 src/mesa/main/mtypes.h|   4 ++
 src/mesa/main/state.h |  17 +++--
 src/mesa/main/texstate.c  |  30 
 src/mesa/tnl/t_context.c  |   3 +-
 5 files changed, 136 insertions(+), 81 deletions(-)

-- 
2.15.0.rc0

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

[Mesa-dev] [PATCH 2/9] mesa: fix crash when an ATI_fs pass begins with an alpha inst

[Miklós Máté]

This fixes crash when:
- first pass begins with alpha inst
- first pass ends with color inst, second pass begins with alpha inst
Also, use the symbolic name instead of a number.

Piglit: spec/ati_fragment_shader/api-alphafirst

v2: fixed formatting

Signed-off-by: Miklós Máté 
---
 src/mesa/main/atifragshader.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/mesa/main/atifragshader.c b/src/mesa/main/atifragshader.c
index 49ddb6e5af..d61455e12f 100644
--- a/src/mesa/main/atifragshader.c
+++ b/src/mesa/main/atifragshader.c
@@ -597,9 +597,13 @@ _mesa_FragmentOpXATI(GLint optype, GLuint arg_count, 
GLenum op, GLuint dst,
else if (curProg->cur_pass==2)
   curProg->cur_pass=3;
 
-   /* decide whether this is a new instruction or not ... all color 
instructions are new,
-  and alpha instructions might also be new if there was no preceding color 
inst */
-   if ((optype == 0) || (curProg->last_optype == optype)) {
+   /* Decide whether this is a new instruction or not. All color instructions
+* are new, and alpha instructions might also be new if there was no
+* preceding color inst. This may also be the first inst of the pass
+*/
+   if (optype == ATI_FRAGMENT_SHADER_COLOR_OP ||
+   curProg->last_optype == optype ||
+   curProg->numArithInstr[curProg->cur_pass >> 1] == 0) {
   if (curProg->numArithInstr[curProg->cur_pass >> 1] > 7) {
 _mesa_error(ctx, GL_INVALID_OPERATION, "C/AFragmentOpATI(instrCount)");
 return;
-- 
2.15.0.rc0

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

[Mesa-dev] [PATCH 5/9] mesa: fix not having secondary color in ATI_fs in swrast

[Miklós Máté]

ATI_fs in swrast only had secondary color if GL_COLOR_SUM was enabled.
This patch probably fixes the same issue in r200.

Piglit: spec/ati_fragment_shader/render-sources and render-precedence

Signed-off-by: Miklós Máté 
---
 src/mesa/main/state.h | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/src/mesa/main/state.h b/src/mesa/main/state.h
index 9d4591790a..5814c659a3 100644
--- a/src/mesa/main/state.h
+++ b/src/mesa/main/state.h
@@ -46,6 +46,13 @@ extern void
 _mesa_set_vp_override(struct gl_context *ctx, GLboolean flag);
 
 
+static inline bool
+_mesa_ati_fragment_shader_enabled(const struct gl_context *ctx)
+{
+   return ctx->ATIFragmentShader.Enabled &&
+  ctx->ATIFragmentShader.Current->Instructions[0];
+}
+
 /**
  * Is the secondary color needed?
  */
@@ -69,6 +76,9 @@ _mesa_need_secondary_color(const struct gl_context *ctx)
(ctx->FragmentProgram._Current->info.inputs_read & VARYING_BIT_COL1))
   return GL_TRUE;
 
+   if (_mesa_ati_fragment_shader_enabled(ctx))
+  return GL_TRUE;
+
return GL_FALSE;
 }
 
@@ -107,11 +117,4 @@ _mesa_arb_fragment_program_enabled(const struct gl_context 
*ctx)
   ctx->FragmentProgram.Current->arb.Instructions;
 }
 
-static inline bool
-_mesa_ati_fragment_shader_enabled(const struct gl_context *ctx)
-{
-   return ctx->ATIFragmentShader.Enabled &&
-  ctx->ATIFragmentShader.Current->Instructions[0];
-}
-
 #endif
-- 
2.15.0.rc0

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

[Mesa-dev] [PATCH 4/9] mesa: fix validate for secondary interpolator

[Miklós Máté]

This patch fixes multiple problems:
- the interpolator check was duplicated
- both had arg instead of argRep
- I split it into color and alpha for better readability and error msg
- the DOT4 check only applies to color instruction according to the spec
- made the DOT4 check fatal, and improved the error msg

Piglit: spec/ati_fragment_shader/error08-secondary

v2: fixed formatting, added spec quotations

Signed-off-by: Miklós Máté 
---
 src/mesa/main/atifragshader.c | 40 +++-
 1 file changed, 27 insertions(+), 13 deletions(-)

diff --git a/src/mesa/main/atifragshader.c b/src/mesa/main/atifragshader.c
index 5ddd83bec4..4aebaf659e 100644
--- a/src/mesa/main/atifragshader.c
+++ b/src/mesa/main/atifragshader.c
@@ -171,15 +171,22 @@ static int check_arith_arg(struct ati_fragment_shader 
*curProg,
   _mesa_error(ctx, GL_INVALID_ENUM, "C/AFragmentOpATI(arg)");
   return 0;
}
-   if ((arg == GL_SECONDARY_INTERPOLATOR_ATI) && (((optype == 0) && (argRep == 
GL_ALPHA)) ||
-  ((optype == 1) && ((arg == GL_ALPHA) || (argRep == GL_NONE) {
-  _mesa_error(ctx, GL_INVALID_OPERATION, "C/AFragmentOpATI(sec_interp)");
-  return 0;
-   }
-   if ((arg == GL_SECONDARY_INTERPOLATOR_ATI) && (((optype == 0) && (argRep == 
GL_ALPHA)) ||
-  ((optype == 1) && ((arg == GL_ALPHA) || (argRep == GL_NONE) {
-  _mesa_error(ctx, GL_INVALID_OPERATION, "C/AFragmentOpATI(sec_interp)");
-  return 0;
+   /* The ATI_fragment_shader spec says:
+*
+*The error INVALID_OPERATION is generated by
+*ColorFragmentOp[1..3]ATI if  is SECONDARY_INTERPOLATOR_ATI
+*and  is ALPHA, or by AlphaFragmentOp[1..3]ATI if 
+*is SECONDARY_INTERPOLATOR_ATI and  is ALPHA or NONE, ...
+*/
+   if (arg == GL_SECONDARY_INTERPOLATOR_ATI) {
+  if (optype == ATI_FRAGMENT_SHADER_COLOR_OP && argRep == GL_ALPHA) {
+ _mesa_error(ctx, GL_INVALID_OPERATION, "CFragmentOpATI(sec_interp)");
+ return 0;
+  } else if (optype == ATI_FRAGMENT_SHADER_ALPHA_OP &&
+ (argRep == GL_ALPHA || argRep == GL_NONE)) {
+ _mesa_error(ctx, GL_INVALID_OPERATION, "AFragmentOpATI(sec_interp)");
+ return 0;
+  }
}
if ((curProg->cur_pass == 1) &&
   ((arg == GL_PRIMARY_COLOR_ARB) || (arg == 
GL_SECONDARY_INTERPOLATOR_ATI))) {
@@ -644,10 +651,17 @@ _mesa_FragmentOpXATI(GLint optype, GLuint arg_count, 
GLenum op, GLuint dst,
 return;
   }
}
-   if ((op == GL_DOT4_ATI) &&
-  (((arg1 == GL_SECONDARY_INTERPOLATOR_ATI) && ((arg1Rep == GL_ALPHA) || 
(arg1Rep == GL_NONE))) ||
-  (((arg2 == GL_SECONDARY_INTERPOLATOR_ATI) && ((arg2Rep == GL_ALPHA) || 
(arg2Rep == GL_NONE)) {
-  _mesa_error(ctx, GL_INVALID_OPERATION, "C/AFragmentOpATI(sec_interp)");
+   /* The ATI_fragment_shader spec says:
+*
+*The error INVALID_OPERATION is generated by... ColorFragmentOp2ATI
+*if  is DOT4_ATI and  is SECONDARY_INTERPOLATOR_ATI and
+* is ALPHA or NONE.
+*/
+   if (optype == ATI_FRAGMENT_SHADER_COLOR_OP && op == GL_DOT4_ATI &&
+   ((arg1 == GL_SECONDARY_INTERPOLATOR_ATI && (arg1Rep == GL_ALPHA || 
arg1Rep == GL_NONE)) ||
+   (arg2 == GL_SECONDARY_INTERPOLATOR_ATI && (arg2Rep == GL_ALPHA || 
arg2Rep == GL_NONE {
+  _mesa_error(ctx, GL_INVALID_OPERATION, 
"C/AFragmentOpATI(sec_interpDOT4)");
+  return;
}
 
if (!check_arith_arg(curProg, optype, arg1, arg1Rep)) {
-- 
2.15.0.rc0

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

[Mesa-dev] [PATCH 1/9] mesa: add fallback texture for SampleMapATI if there is nothing

[Miklós Máté]

This fixes crash in the state tracker.

Piglit: spec/ati_fragment_shader/render-notexture

v2: fixed formatting, moved stuff inside the loop,
moved the fallback later to fix more cases

Signed-off-by: Miklós Máté 
---
 src/mesa/main/texstate.c | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/src/mesa/main/texstate.c b/src/mesa/main/texstate.c
index 2146723d08..8f87cac0ce 100644
--- a/src/mesa/main/texstate.c
+++ b/src/mesa/main/texstate.c
@@ -819,6 +819,29 @@ update_ff_texture_state(struct gl_context *ctx,
}
 }
 
+static void
+fix_missing_textures_for_atifs(struct gl_context *ctx,
+   struct gl_program *prog,
+   BITSET_WORD *enabled_texture_units)
+{
+   GLbitfield mask = prog->SamplersUsed;
+
+   while (mask) {
+  const int s = u_bit_scan();
+  const int unit = prog->SamplerUnits[s];
+  const gl_texture_index target_index = ffs(prog->TexturesUsed[unit]) - 1;
+
+  if (!ctx->Texture.Unit[unit]._Current) {
+ struct gl_texture_object *texObj =
+_mesa_get_fallback_texture(ctx, target_index);
+ _mesa_reference_texobj(>Texture.Unit[unit]._Current, texObj);
+ BITSET_SET(enabled_texture_units, unit);
+ ctx->Texture._MaxEnabledTexImageUnit =
+MAX2(ctx->Texture._MaxEnabledTexImageUnit, (int)unit);
+  }
+   }
+}
+
 /**
  * \note This routine refers to derived texture matrix values to
  * compute the ENABLE_TEXMAT flags, but is only called on
@@ -875,6 +898,13 @@ _mesa_update_texture_state(struct gl_context *ctx)
   _mesa_reference_texobj(>Texture.Unit[i]._Current, NULL);
}
 
+   /* add fallback texture for SampleMapATI if there is nothing */
+   if (_mesa_ati_fragment_shader_enabled(ctx) &&
+   ctx->ATIFragmentShader.Current->Program)
+  fix_missing_textures_for_atifs(ctx,
+ ctx->ATIFragmentShader.Current->Program,
+ enabled_texture_units);
+
if (!prog[MESA_SHADER_FRAGMENT] || !prog[MESA_SHADER_VERTEX])
   update_texgen(ctx);
 }
-- 
2.15.0.rc0

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

[Mesa-dev] [PATCH 6/9] tnl: fix not having texture coords in ATI_fs in swrast

[Miklós Máté]

ATI_fs in swrast only had access to texture coordinates if there was a
valid texture bound and texturing was enabled.

Piglit: spec/ati_fragment_shader/render-sources and render-notexture

Signed-off-by: Miklós Máté 
---
 src/mesa/tnl/t_context.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/mesa/tnl/t_context.c b/src/mesa/tnl/t_context.c
index ec18fa01b2..bb5d9fc07b 100644
--- a/src/mesa/tnl/t_context.c
+++ b/src/mesa/tnl/t_context.c
@@ -157,7 +157,8 @@ _tnl_InvalidateState( struct gl_context *ctx, GLuint 
new_state )
 
for (i = 0; i < ctx->Const.MaxTextureCoordUnits; i++) {
  if (ctx->Texture._EnabledCoordUnits & (1 << i) ||
-(fp && fp->info.inputs_read & VARYING_BIT_TEX(i))) {
+(fp && fp->info.inputs_read & VARYING_BIT_TEX(i)) ||
+ _mesa_ati_fragment_shader_enabled(ctx)) {
tnl->render_inputs_bitset |= BITFIELD64_BIT(_TNL_ATTRIB_TEX(i));
  }
}
-- 
2.15.0.rc0

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

[Mesa-dev] [PATCH 3/9] mesa: fix typo in ATI_fs dstMod error checking

[Miklós Máté]

Piglit: spec/ati_fragment_shader/error14-invalidmod

Signed-off-by: Miklós Máté 
Reviewed-by: Ian Romanick 
---
 src/mesa/main/atifragshader.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/main/atifragshader.c b/src/mesa/main/atifragshader.c
index d61455e12f..5ddd83bec4 100644
--- a/src/mesa/main/atifragshader.c
+++ b/src/mesa/main/atifragshader.c
@@ -625,7 +625,7 @@ _mesa_FragmentOpXATI(GLint optype, GLuint arg_count, GLenum 
op, GLuint dst,
}
if ((modtemp != GL_NONE) && (modtemp != GL_2X_BIT_ATI) &&
   (modtemp != GL_4X_BIT_ATI) && (modtemp != GL_8X_BIT_ATI) &&
-  (modtemp != GL_HALF_BIT_ATI) && !(modtemp != GL_QUARTER_BIT_ATI) &&
+  (modtemp != GL_HALF_BIT_ATI) && (modtemp != GL_QUARTER_BIT_ATI) &&
   (modtemp != GL_EIGHTH_BIT_ATI)) {
   _mesa_error(ctx, GL_INVALID_ENUM, "C/AFragmentOpATI(dstMod)%x", modtemp);
   return;
-- 
2.15.0.rc0

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

Re: [Mesa-dev] [PATCH 1/2] glsl: Match order of gl_LightSourceParameters elements.

[Fabian Bieler]

Thanks again for the review. Since I don't have Mesa git access (only
Piglit) a little push would be appreciated. ;)

I'll send the commit IDs to mesa-stable as suggested though I'm not sure
if the gl_NormalScale fix is worth the trouble. I don't think _anyone_
is using that feature.

On 2017-11-29 23:56, Brian Paul wrote:
> Reviewed-by: Brian Paul 
> 
> I think you could tag both of your patches for the stable branch.
> 
> On 11/23/2017 01:48 PM, Fabian Bieler wrote:
>> spotExponent and spotCosCutoff were swapped in the
>> gl_builtin_uniform_element
>> struct.
>> Now the order matches across gl_builtin_uniform_element,
>> glsl_struct_field and
>> the spec.
>> ---
>>   src/compiler/glsl/builtin_variables.cpp | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/compiler/glsl/builtin_variables.cpp
>> b/src/compiler/glsl/builtin_variables.cpp
>> index 00bc99d..a885f32 100644
>> --- a/src/compiler/glsl/builtin_variables.cpp
>> +++ b/src/compiler/glsl/builtin_variables.cpp
>> @@ -90,9 +90,9 @@ static const struct gl_builtin_uniform_element
>> gl_LightSource_elements[] = {
>>     SWIZZLE_Y,
>>     SWIZZLE_Z,
>>     SWIZZLE_Z)},
>> -   {"spotCosCutoff", {STATE_LIGHT, 0, STATE_SPOT_DIRECTION},
>> SWIZZLE_},
>> -   {"spotCutoff", {STATE_LIGHT, 0, STATE_SPOT_CUTOFF}, SWIZZLE_},
>>  {"spotExponent", {STATE_LIGHT, 0, STATE_ATTENUATION}, SWIZZLE_},
>> +   {"spotCutoff", {STATE_LIGHT, 0, STATE_SPOT_CUTOFF}, SWIZZLE_},
>> +   {"spotCosCutoff", {STATE_LIGHT, 0, STATE_SPOT_DIRECTION},
>> SWIZZLE_},
>>  {"constantAttenuation", {STATE_LIGHT, 0, STATE_ATTENUATION},
>> SWIZZLE_},
>>  {"linearAttenuation", {STATE_LIGHT, 0, STATE_ATTENUATION},
>> SWIZZLE_},
>>  {"quadraticAttenuation", {STATE_LIGHT, 0, STATE_ATTENUATION},
>> SWIZZLE_},
>>
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] nir/gather_info: Handle double inputs in set_io_mask

[Jason Ekstrand]

Ugh... This patch, though obviously correct if taken on its own, doesn't
wok.  The double_inputs_read field is more subtle and bonkers than I
thought.

On Sat, Dec 2, 2017 at 9:32 AM, Jason Ekstrand  wrote:

> It's a little sketchy to put the logic in set_io_mask but due to a
> detail of the GLSL spec, it's safe to check for 64-bit without knowing
> the full access chain.  This fixes a bug where try_mask_partial_io
> succeeded at marking a portion of the input as used but then we came
> along and marked the whole thing in double_inputs_read.  By putting it
> in set_io_mask, we get the correct flagging even for partial reads.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103241
> Cc: mesa-sta...@lists.freedesktop.org
> Cc: Timothy Arceri 
> Cc: Alejandro Piñeiro 
> ---
>  src/compiler/nir/nir_gather_info.c | 28 +---
>  1 file changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/src/compiler/nir/nir_gather_info.c
> b/src/compiler/nir/nir_gather_info.c
> index 9469396..bab37f3 100644
> --- a/src/compiler/nir/nir_gather_info.c
> +++ b/src/compiler/nir/nir_gather_info.c
> @@ -54,6 +54,23 @@ set_io_mask(nir_shader *shader, nir_variable *var, int
> offset, int len,
>   else
>  shader->info.inputs_read |= bitfield;
>
> + /* From the GLSL 4.60 spec, section 4.3.4 "Input Variables":
> +  *
> +  *It is a compile-time error to declare a vertex shader input
> +  *containing any of the  following:
> +  *
> +  * - A Boolean type (bool, bvec2, bvec3, bvec4)
> +  * - An opaque type
> +  * - A structure
> +  *
> +  * Therefore, when looking for 64-bit attributes, we need only
> +  * consider arrays of vectors.
> +  */
> + if (shader->info.stage == MESA_SHADER_VERTEX &&
> + glsl_type_is_dual_slot(glsl_without_array(var->type))) {
> +shader->info.double_inputs_read |= bitfield;
> + }
> +
>   if (shader->info.stage == MESA_SHADER_FRAGMENT) {
>  shader->info.fs.uses_sample_qualifier |= var->data.sample;
>   }
> @@ -226,17 +243,6 @@ gather_intrinsic_info(nir_intrinsic_instr *instr,
> nir_shader *shader)
>
>   if (!try_mask_partial_io(shader, instr->variables[0],
> is_output_read))
>  mark_whole_variable(shader, var, is_output_read);
> -
> - /* We need to track which input_reads bits correspond to a
> -  * dvec3/dvec4 input attribute */
> - if (shader->info.stage == MESA_SHADER_VERTEX &&
> - var->data.mode == nir_var_shader_in &&
> - glsl_type_is_dual_slot(glsl_without_array(var->type))) {
> -for (uint i = 0; i < glsl_count_attribute_slots(var->type,
> false); i++) {
> -   int idx = var->data.location + i;
> -   shader->info.double_inputs_read |= BITFIELD64_BIT(idx);
> -}
> - }
>}
>break;
> }
> --
> 2.5.0.400.gff86faf
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/3] nv50/ir: Fix unused var warnings in release build

[Ilia Mirkin]

On Sat, Dec 2, 2017 at 1:22 PM, Rhys Kidd  wrote:
> Signed-off-by: Rhys Kidd 
> ---
>  src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nv50.cpp | 2 +-
>  src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nv50.cpp 
> b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nv50.cpp
> index cc2a88eb40..139ff4a31d 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nv50.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nv50.cpp
> @@ -621,7 +621,7 @@ void
>  CodeEmitterNV50::emitLOAD(const Instruction *i)
>  {
> DataFile sf = i->src(0).getFile();
> -   int32_t offset = i->getSrc(0)->reg.data.offset;
> +   MAYBE_UNUSED int32_t offset = i->getSrc(0)->reg.data.offset;
>
> switch (sf) {
> case FILE_SHADER_INPUT:
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp 
> b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> index 61d4e6a2d0..f49459b969 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> @@ -3308,7 +3308,7 @@ PostRaLoadPropagation::handleMADforNV50(Instruction *i)
>   i->setSrc(1, def->getSrc(0));
>} else {
>   ImmediateValue val;
> - bool ret = def->src(0).getImmediate(val);
> + MAYBE_UNUSED bool ret = def->src(0).getImmediate(val);

Can you write a comment mentioning that getImmediate() has
side-effects on the argument, so this *shouldn't* get folded into the
assert? My concern is that with the MAYBE_UNUSED there, someone with a
twitchy thumb may be tempted to press that button...

  -ilia

>   assert(ret);
>   if (i->getSrc(1)->reg.data.id & 1)
>  val.reg.data.u32 >>= 16;
> --
> 2.14.1
>
> ___
> 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 0/3] nouveau compiler warning cleanups

[Rhys Kidd]

Couple of little compiler warning cleanups so that nouveau builds without any
warnings for meson's debug and release builds with gcc 7.2.0

Rhys Kidd (3):
  nv50: Fix unused var warning in release build
  nvc0: Fix unused var warnings in release build
  nv50/ir: Fix unused var warnings in release build

 src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nv50.cpp | 2 +-
 src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp  | 2 +-
 src/gallium/drivers/nouveau/nv50/nv98_video.c | 3 ++-
 src/gallium/drivers/nouveau/nvc0/nvc0_video.c | 7 ---
 4 files changed, 8 insertions(+), 6 deletions(-)

-- 
2.14.1

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

[Mesa-dev] [PATCH 1/3] nv50: Fix unused var warning in release build

[Rhys Kidd]

Signed-off-by: Rhys Kidd 
---
 src/gallium/drivers/nouveau/nv50/nv98_video.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/nouveau/nv50/nv98_video.c 
b/src/gallium/drivers/nouveau/nv50/nv98_video.c
index 92526d9f64..da0267e646 100644
--- a/src/gallium/drivers/nouveau/nv50/nv98_video.c
+++ b/src/gallium/drivers/nouveau/nv50/nv98_video.c
@@ -40,7 +40,8 @@ nv98_decoder_decode_bitstream(struct pipe_video_codec 
*decoder,
uint32_t comm_seq = ++dec->fence_seq;
union pipe_desc desc;
 
-   unsigned vp_caps, is_ref, ret;
+   unsigned vp_caps, is_ref;
+   MAYBE_UNUSED unsigned ret; /* used in debug checks */
struct nouveau_vp3_video_buffer *refs[16] = {};
 
desc.base = picture;
-- 
2.14.1

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

[Mesa-dev] [PATCH 3/3] nv50/ir: Fix unused var warnings in release build

[Rhys Kidd]

Signed-off-by: Rhys Kidd 
---
 src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nv50.cpp | 2 +-
 src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nv50.cpp 
b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nv50.cpp
index cc2a88eb40..139ff4a31d 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nv50.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nv50.cpp
@@ -621,7 +621,7 @@ void
 CodeEmitterNV50::emitLOAD(const Instruction *i)
 {
DataFile sf = i->src(0).getFile();
-   int32_t offset = i->getSrc(0)->reg.data.offset;
+   MAYBE_UNUSED int32_t offset = i->getSrc(0)->reg.data.offset;
 
switch (sf) {
case FILE_SHADER_INPUT:
diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp 
b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
index 61d4e6a2d0..f49459b969 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
@@ -3308,7 +3308,7 @@ PostRaLoadPropagation::handleMADforNV50(Instruction *i)
  i->setSrc(1, def->getSrc(0));
   } else {
  ImmediateValue val;
- bool ret = def->src(0).getImmediate(val);
+ MAYBE_UNUSED bool ret = def->src(0).getImmediate(val);
  assert(ret);
  if (i->getSrc(1)->reg.data.id & 1)
 val.reg.data.u32 >>= 16;
-- 
2.14.1

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

[Mesa-dev] [PATCH 2/3] nvc0: Fix unused var warnings in release build

[Rhys Kidd]

Signed-off-by: Rhys Kidd 
---
 src/gallium/drivers/nouveau/nvc0/nvc0_video.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_video.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_video.c
index b5e7bba5f9..5c1cf899ca 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_video.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_video.c
@@ -32,7 +32,7 @@ nvc0_decoder_begin_frame(struct pipe_video_codec *decoder,
 {
struct nouveau_vp3_decoder *dec = (struct nouveau_vp3_decoder *)decoder;
uint32_t comm_seq = ++dec->fence_seq;
-   unsigned ret = 0;
+   MAYBE_UNUSED unsigned ret = 0; /* used in debug checks */
 
assert(dec);
assert(target);
@@ -53,7 +53,7 @@ nvc0_decoder_decode_bitstream(struct pipe_video_codec 
*decoder,
 {
struct nouveau_vp3_decoder *dec = (struct nouveau_vp3_decoder *)decoder;
uint32_t comm_seq = dec->fence_seq;
-   unsigned ret = 0;
+   MAYBE_UNUSED unsigned ret = 0; /* used in debug checks */
 
assert(decoder);
 
@@ -72,7 +72,8 @@ nvc0_decoder_end_frame(struct pipe_video_codec *decoder,
uint32_t comm_seq = dec->fence_seq;
union pipe_desc desc;
 
-   unsigned vp_caps, is_ref, ret;
+   unsigned vp_caps, is_ref;
+   MAYBE_UNUSED unsigned ret; /* used in debug checks */
struct nouveau_vp3_video_buffer *refs[16] = {};
 
desc.base = picture;
-- 
2.14.1

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

[Mesa-dev] [PATCH] nir/gather_info: Handle double inputs in set_io_mask

[Jason Ekstrand]

It's a little sketchy to put the logic in set_io_mask but due to a
detail of the GLSL spec, it's safe to check for 64-bit without knowing
the full access chain.  This fixes a bug where try_mask_partial_io
succeeded at marking a portion of the input as used but then we came
along and marked the whole thing in double_inputs_read.  By putting it
in set_io_mask, we get the correct flagging even for partial reads.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103241
Cc: mesa-sta...@lists.freedesktop.org
Cc: Timothy Arceri 
Cc: Alejandro Piñeiro 
---
 src/compiler/nir/nir_gather_info.c | 28 +---
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/src/compiler/nir/nir_gather_info.c 
b/src/compiler/nir/nir_gather_info.c
index 9469396..bab37f3 100644
--- a/src/compiler/nir/nir_gather_info.c
+++ b/src/compiler/nir/nir_gather_info.c
@@ -54,6 +54,23 @@ set_io_mask(nir_shader *shader, nir_variable *var, int 
offset, int len,
  else
 shader->info.inputs_read |= bitfield;
 
+ /* From the GLSL 4.60 spec, section 4.3.4 "Input Variables":
+  *
+  *It is a compile-time error to declare a vertex shader input
+  *containing any of the  following:
+  *
+  * - A Boolean type (bool, bvec2, bvec3, bvec4)
+  * - An opaque type
+  * - A structure
+  *
+  * Therefore, when looking for 64-bit attributes, we need only
+  * consider arrays of vectors.
+  */
+ if (shader->info.stage == MESA_SHADER_VERTEX &&
+ glsl_type_is_dual_slot(glsl_without_array(var->type))) {
+shader->info.double_inputs_read |= bitfield;
+ }
+
  if (shader->info.stage == MESA_SHADER_FRAGMENT) {
 shader->info.fs.uses_sample_qualifier |= var->data.sample;
  }
@@ -226,17 +243,6 @@ gather_intrinsic_info(nir_intrinsic_instr *instr, 
nir_shader *shader)
 
  if (!try_mask_partial_io(shader, instr->variables[0], is_output_read))
 mark_whole_variable(shader, var, is_output_read);
-
- /* We need to track which input_reads bits correspond to a
-  * dvec3/dvec4 input attribute */
- if (shader->info.stage == MESA_SHADER_VERTEX &&
- var->data.mode == nir_var_shader_in &&
- glsl_type_is_dual_slot(glsl_without_array(var->type))) {
-for (uint i = 0; i < glsl_count_attribute_slots(var->type, false); 
i++) {
-   int idx = var->data.location + i;
-   shader->info.double_inputs_read |= BITFIELD64_BIT(idx);
-}
- }
   }
   break;
}
-- 
2.5.0.400.gff86faf

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

Re: [Mesa-dev] [PATCH v2 09/32] anv: Require a dedicated allocation for modified images

[Chad Versace]

On Wed 29 Nov 2017, Jason Ekstrand wrote:
> On Wed, Nov 29, 2017 at 2:24 PM, Chad Versace <[1]chadvers...@chromium.org>
> wrote:
> 
> On Tue 28 Nov 2017, Jason Ekstrand wrote:
> > This lets us set the BO tiling when we allocate the memory.  This is
> > required for GL to work properly.
> > ---
> >  src/intel/vulkan/anv_device.c | 53 ++
> +
> >  1 file changed, 49 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/intel/vulkan/anv_device.c 
> b/src/intel/vulkan/anv_device.
> c
> > index df929e4..d82d1f7 100644
> > --- a/src/intel/vulkan/anv_device.c
> > +++ b/src/intel/vulkan/anv_device.c
> > @@ -29,6 +29,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> >  #include "anv_private.h"
> >  #include "util/strtod.h"
> > @@ -1612,6 +1613,40 @@ VkResult anv_AllocateMemory(
> >                                    >bo);
> >        if (result != VK_SUCCESS)
> >           goto fail;
> > +
> > +      const VkMemoryDedicatedAllocateInfoKHR *dedicated_info =
> > +         vk_find_struct_const(pAllocateInfo->pNext,
> MEMORY_DEDICATED_ALLOCATE_INFO_KHR);
> > +      if (dedicated_info && dedicated_info->image != VK_NULL_HANDLE) {
> > +         ANV_FROM_HANDLE(anv_image, image, dedicated_info->image);
> > +
> > +         /* For images using modifiers, we require a dedicated
> allocation
> > +          * and we set the BO tiling to match the tiling of the
> underlying
> > +          * modifier.  This is a bit unfortunate as this is completely
> > +          * pointless for Vulkan.  However, GL needs to be able to map
> things
> > +          * so it needs the tiling to be set.  The only way to do this
> in a
> > +          * non-racy way is to set the tiling in the creator of the BO
> so that
> > +          * makes it our job.
> > +          *
> > +          * One of these days, once the GL driver learns to not map
> things
> > +          * through the GTT in random places, we can drop this and 
> start
> > +          * allowing multiple modified images in the same BO.
> > +          */
> > +         if (image->drm_format_mod != DRM_FORMAT_MOD_INVALID) {
> > +            assert(isl_drm_modifier_get_info(image->drm_format_mod)->
> tiling ==
> > +                   image->planes[0].surface.isl.tiling);
> > +            const uint32_t i915_tiling =
> > +               isl_tiling_to_i915_tiling(image->planes[0].surface.isl.
> tiling);
> > +            int ret = anv_gem_set_tiling(device, mem->bo->gem_handle,
> > +                                         image->planes[0].surface.isl.
> row_pitch,
> > +                                         i915_tiling);
> > +            if (ret) {
> > +               anv_bo_cache_release(device, >bo_cache, 
> mem->bo);
> > +               return vk_errorf(device->instance, NULL,
> > +                                VK_ERROR_OUT_OF_DEVICE_MEMORY,
> > +                                "failed to set BO tiling: %m");
> > +            }
> > +         }
> > +      }
> >     }
> 
> This hunk of code is run only for non-imported memory. It's in the
> 'else' branch of `if (fd_info && fd_info->handleTypes)`. I believe it
> needs to be run in both branches. Otherwise, if an app creates
> a dma_buf, imports it into Vulkan, binds the image to the dma_buf, and
> afterwards imports the same dma_buf into GL as an EGLImage, then
> intel_create_image_from_fds_common() will fail to discover the bo's
> tiling.
> 
> 
> Yes, that was a bit intentional because setting it any time other than image
> creation is racy.  If we were going to set it on import as well, we may as 
> well
> just do that in the GL driver and call it a day.
>  
> 
> Everything else in this patch looks correct to me, even though it makes
> me sad.

I started writing a mini-essay in reply to this patch. Since that
reply's summary is/was

In general, for the upcoming modifiers extension, this patch is
incorrect. But, for the limited usecase of the pseudo WSI layer,
everything should work correctly.

then I'm ok with landing this patch as-is in this series.

Reviewed-by: Chad Versace 

But we should chat later (not while I'm making breakfast for my kid
(which is right now)) about the best way to fix the general problem,
which becomes real when VK_EXT_image_drm_format_modifier arrives.

I think I've reviewed all the patches now. If I've forgotten any, let me
know... or just push anyway.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 103955] Using array in structure results in wrong GLSL compilation output

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=103955

--- Comment #8 from Ilia Mirkin  ---
This patch fixes the sample case for me:

https://patchwork.freedesktop.org/patch/191532/

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

[Mesa-dev] [PATCH] st/mesa: swizzle argument when there's a vector size mismatch

[Ilia Mirkin]

GLSL IR operation arguments can sometimes have an implicit swizzle as a
result of a vector arg and a scalar arg, where the scalar argument is
implicitly expanded to the size of the vector argument.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103955
Signed-off-by: Ilia Mirkin 
---

I'm *pretty* sure I looked at all of ir_validate to make sure that this will
be fine, but I wouldn't exclude the possibility that I missed some extremely
odd case. Could use a piglit run.

 src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 21 +
 1 file changed, 21 insertions(+)

diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp 
b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index 0772b736275..8ca337d16ec 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -1342,9 +1342,30 @@ glsl_to_tgsi_visitor::visit_expression(ir_expression* 
ir, st_src_reg *op)
 
int vector_elements = ir->operands[0]->type->vector_elements;
if (ir->operands[1]) {
+  st_src_reg *swz_op = NULL;
+  if (vector_elements > ir->operands[1]->type->vector_elements) {
+ assert(ir->operands[1]->type->vector_elements == 1);
+ swz_op = [1];
+  } else if (vector_elements < ir->operands[1]->type->vector_elements) {
+ assert(ir->operands[0]->type->vector_elements == 1);
+ swz_op = [0];
+  }
+  if (swz_op) {
+ uint16_t swizzle_x = GET_SWZ(swz_op->swizzle, 0);
+ swz_op->swizzle = MAKE_SWIZZLE4(swizzle_x, swizzle_x,
+ swizzle_x, swizzle_x);
+  }
   vector_elements = MAX2(vector_elements,
  ir->operands[1]->type->vector_elements);
}
+   if (ir->operands[2] &&
+   ir->operands[2]->type->vector_elements != vector_elements) {
+  /* This can happen with ir_triop_lrp, i.e. glsl mix */
+  assert(ir->operands[2]->type->vector_elements == 1);
+  uint16_t swizzle_x = GET_SWZ(op[2]->swizzle, 0);
+  op[2].swizzle = MAKE_SWIZZLE4(swizzle_x, swizzle_x,
+swizzle_x, swizzle_x);
+   }
 
this->result.file = PROGRAM_UNDEFINED;
 
-- 
2.13.6

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

Re: [Mesa-dev] [PATCH v2 00/32] vulkan/wsi: Rework WSI to look a lot more like a layer

[Chad Versace]

On Wed 29 Nov 2017, Dave Airlie wrote:
> On 29 November 2017 at 10:28, Jason Ekstrand  wrote:
> > This patch series is a v2 of the one I sent out a couple weeks ago to
> > rewrite Vulkan window system integration.  The original patch series can be
> > found on patchwork here:
> >
> > https://patchwork.freedesktop.org/series/33961/
> >
> > This series can be found as a git branch here:
> >
> > https://cgit.freedesktop.org/~jekstrand/mesa/log/?h=review/vulkan-wsi-rewrite-v2
> >
> > Notable changes in v2:
> 
> Thanks for finished this off and making it more presentable,
> 
> You might want to wait for Daniel or Chad to look some of it over, but
> I'm happy to
> give
> 
> Reviewed-by: Dave Airlie 
> 
> for the whole series, it really makes the WSI and prime into a contained 
> space,
> and is what I wished the initial modifier series had looked like :-)

I really like the clean seperation created by this series. I hope the
Android winsys extension (and the upcoming one too) can be rewritten
similarly and placed into src/vulkan/android, so anv and radv can share
it.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 32/32] radv: Implement VK_KHR_get_surface_capabilities2

[Chad Versace]

On Tue 28 Nov 2017, Jason Ekstrand wrote:
> The WSI core code does all the hard work.  Just add the wrappers and
> turn it on.
> ---
>  src/amd/vulkan/radv_extensions.py |  1 +
>  src/amd/vulkan/radv_wsi.c | 26 ++
>  2 files changed, 27 insertions(+)

Reviewed-by: Chad Versace 

This is the end of the series, but I've left a few patches unreviewed.
I'll revisit them now.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 31/32] vulkan/wsi: Initialize individual WSI interfaces in wsi_device_init

[Chad Versace]

On Tue 28 Nov 2017, Jason Ekstrand wrote:
> Now that we have anv_device_init/finish functions, there's no reason to
> have the individual driver do any more work than that.

You mean "wsi_device_init/finish" functions.

> ---
>  src/amd/vulkan/radv_wsi.c   | 36 ++--
>  src/intel/vulkan/anv_wsi.c  | 36 ++--
>  src/vulkan/wsi/wsi_common.c | 37 
> +++--
>  src/vulkan/wsi/wsi_common.h | 19 +++
>  src/vulkan/wsi/wsi_common_private.h | 10 ++
>  5 files changed, 64 insertions(+), 74 deletions(-)


> -void
> +VkResult
>  wsi_device_init(struct wsi_device *wsi,
>  VkPhysicalDevice pdevice,
> -WSI_FN_GetPhysicalDeviceProcAddr proc_addr)
> +WSI_FN_GetPhysicalDeviceProcAddr proc_addr,
> +const VkAllocationCallbacks *alloc)
>  {
> +   VkResult result;
> +
> memset(wsi, 0, sizeof(*wsi));
>  
>  #define WSI_GET_CB(func) \
> @@ -69,6 +72,36 @@ wsi_device_init(struct wsi_device *wsi,
> WSI_GET_CB(QueueSubmit);
> WSI_GET_CB(WaitForFences);
>  #undef WSI_GET_CB
> +
> +#ifdef VK_USE_PLATFORM_XCB_KHR
> +   result = wsi_x11_init_wsi(wsi, alloc);
> +   if (result != VK_SUCCESS)
> +  return result;
> +#endif
> +
> +#ifdef VK_USE_PLATFORM_WAYLAND_KHR
> +   result = wsi_wl_init_wsi(wsi, alloc, pdevice);
> +   if (result != VK_SUCCESS) {
> +#ifdef VK_USE_PLATFORM_XCB_KHR
> +  wsi_x11_finish_wsi(wsi, alloc);
> +#endif
> +  return result;
> +   }
> +#endif

This looks like a bug. wsi_device_init() fails if either of x11 or
wayland initialization fails, even if the user requested only one of
VK_KHR_{xcb,xlib,wayland}_surface.

I filed https://bugs.freedesktop.org/show_bug.cgi?id=104039 .

But this patch is just a refactor patch, and it preserves the existing
(and possibly buggy) behavior. With the commit message fixed,
Reviewed-by: Chad Versace 


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

[Mesa-dev] [Bug 104038] mesa: Need a new Bugzilla component Drivers/Vulkan/Common

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=104038

Chad Versace  changed:

   What|Removed |Added

   See Also||https://bugs.freedesktop.or
   ||g/show_bug.cgi?id=104039

-- 
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] [Bug 104038] mesa: Need a new Bugzilla component Drivers/Vulkan/Common

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=104038

Chad Versace  changed:

   What|Removed |Added

 CC||airl...@freedesktop.org,
   ||chadvers...@chromium.org,
   ||emil.l.veli...@gmail.com,
   ||ja...@jlekstrand.net

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

[Mesa-dev] [Bug 104038] mesa: Need a new Bugzilla component Drivers/Vulkan/Common

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=104038

Bug ID: 104038
   Summary: mesa: Need a new Bugzilla component
Drivers/Vulkan/Common
   Product: Mesa
   Version: git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: Other
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: chadvers...@chromium.org
QA Contact: mesa-dev@lists.freedesktop.org

The Intel and AMD Vulkan drivers share enough code now that we need a Bugzilla
component for the the shared code.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
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 v2 29/32] vulkan/wsi: Add wrappers for all of the surface queries

[Chad Versace]

On Tue 28 Nov 2017, Jason Ekstrand wrote:
> This lets us move wsi_interface to wsi_common_private.h
> ---
>  src/amd/vulkan/radv_wsi.c   | 41 ++--
>  src/intel/vulkan/anv_wsi.c  | 51 +++-
>  src/vulkan/wsi/wsi_common.c | 77 
> +
>  src/vulkan/wsi/wsi_common.h | 72 +-
>  src/vulkan/wsi/wsi_common_private.h | 34 
>  5 files changed, 192 insertions(+), 83 deletions(-)

Patches 28 & 29 are
Reviewed-by: Chad Versace 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 27/32] vulkan/wsi: Move wsi_swapchain to wsi_common_private.h

[Chad Versace]

On Tue 28 Nov 2017, Jason Ekstrand wrote:
> The drivers no longer poke at this directly.
> ---
>  src/vulkan/wsi/wsi_common.h | 46 
> +
>  src/vulkan/wsi/wsi_common_private.h | 46 
> +
>  2 files changed, 47 insertions(+), 45 deletions(-)

Yay. It's becoming more layer-like.

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

Re: [Mesa-dev] [PATCH v2 26/32] vulkan/wsi: Add a helper for AcquireNextImage

[Chad Versace]

On Tue 28 Nov 2017, Jason Ekstrand wrote:
> Unfortunately, due to the fact that AcquireNextImage does not take a
> queue, the ANV trick for triggering the fence won't work in general.  We
> leave dealing with the fence up to the caller for now.
> ---
>  src/amd/vulkan/radv_wsi.c   | 15 ++-
>  src/intel/vulkan/anv_wsi.c  | 19 +++
>  src/vulkan/wsi/wsi_common.c | 14 ++
>  src/vulkan/wsi/wsi_common.h |  8 
>  4 files changed, 43 insertions(+), 13 deletions(-)

Patch 26 is
Reviewed-by: Chad Versace 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 25/32] vulkan/wsi: move swapchain create/destroy to common code

[Chad Versace]

On Tue 28 Nov 2017, Jason Ekstrand wrote:
> From: Dave Airlie 
> 
> v2 (Jason Ekstrand):
>  - Rebase
>  - Alter the names of the helpers to better match the vulkan entrypoints
>  - Use the helpers in anv
> ---
>  src/amd/vulkan/radv_wsi.c   | 42 --
>  src/intel/vulkan/anv_wsi.c  | 35 +--
>  src/vulkan/wsi/wsi_common.c | 38 ++
>  src/vulkan/wsi/wsi_common.h | 12 
>  4 files changed, 63 insertions(+), 64 deletions(-)

> @@ -115,6 +115,9 @@ fail:
>  void
>  wsi_swapchain_finish(struct wsi_swapchain *chain)
>  {
> +   for (unsigned i = 0; i < ARRAY_SIZE(chain->fences); i++)
> +  chain->wsi->DestroyFence(chain->device, chain->fences[i], 
> >alloc);
> +

Yes. I've been waiting to see DestroyFence here since patch 17.

> for (uint32_t i = 0; i < chain->wsi->queue_family_count; i++) {
>chain->wsi->DestroyCommandPool(chain->device, chain->cmd_pools[i],
>   >alloc);
> @@ -485,6 +488,41 @@ wsi_destroy_image(const struct wsi_swapchain *chain,

Patch 25 is
Reviewed-by: Chad Versace 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 23/32] vulkan/wsi: Move get_images into common code

[Chad Versace]

On Tue 28 Nov 2017, Jason Ekstrand wrote:
> This moves bits out of all four corners (anv, radv, x11, wayland) and
> into the wsi common code.  We also switch to using an outarray to ensure
> we get our return code right.
> ---
>  src/amd/vulkan/radv_wsi.c   |  7 +++
>  src/intel/vulkan/anv_wsi.c  |  7 +++
>  src/vulkan/wsi/wsi_common.c | 17 +
>  src/vulkan/wsi/wsi_common.h |  9 +++--
>  src/vulkan/wsi/wsi_common_wayland.c | 28 +---
>  src/vulkan/wsi/wsi_common_x11.c | 29 +
>  6 files changed, 40 insertions(+), 57 deletions(-)

Patches 22 and 23 are
Reviewed-by: Chad Versace 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 20/32] vulkan/wsi: Set a proper pWaitDstStageMask on the dummy submit

[Chad Versace]

On Fri 01 Dec 2017, Chad Versace wrote:
> On Tue 28 Nov 2017, Jason Ekstrand wrote:
> > Neither mesa driver really cares, but we should set it none the less for
> > the sake of correctness.
> > ---
> >  src/vulkan/wsi/wsi_common.c | 17 +
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/src/vulkan/wsi/wsi_common.c b/src/vulkan/wsi/wsi_common.c
> > index 4f6648f..e5a9a28 100644
> > --- a/src/vulkan/wsi/wsi_common.c
> > +++ b/src/vulkan/wsi/wsi_common.c
> > @@ -542,14 +542,31 @@ wsi_common_queue_present(const struct wsi_device *wsi,
> >   .sType = VK_STRUCTURE_TYPE_SUBMIT_INFO,
> >   .pNext = NULL,
> >};
> > +  VkPipelineStageFlags *stage_flags = NULL;
> >if (i == 0) {
> >   /* We only need/want to wait on semaphores once.  After that, 
> > we're
> >* guaranteed ordering since it all happens on the same queue.
> >*/
> >   submit_info.waitSemaphoreCount = pPresentInfo->waitSemaphoreCount,
> >   submit_info.pWaitSemaphores = pPresentInfo->pWaitSemaphores,
> > +
> > + /* Set up the pWaitDstStageMasks */
> > + stage_flags = vk_alloc(>alloc,
> > +sizeof(VkPipelineStageFlags) *
> > +pPresentInfo->waitSemaphoreCount,
> > +8,
> > +VK_SYSTEM_ALLOCATION_SCOPE_COMMAND);
> > + if (!stage_flags) {
> > +result = VK_ERROR_OUT_OF_HOST_MEMORY;
> > +goto fail_present;
> > + }
> > + for (uint32_t s = 0; s < pPresentInfo->waitSemaphoreCount; s++)
> > +stage_flags[s] = VK_PIPELINE_STAGE_ALL_GRAPHICS_BIT;
> > +
> > + submit_info.pWaitDstStageMask = stage_flags;
> 
> Since VkSwapchain is required to be externally synchronized, you could
> embed stage_flags directly in struct wsi_swapchain, doubling its size
> when needed. But meh.

Since stage_flags gets freed at the end of the function, you could make
the code simpler by using the stack.

VkPipelineStageFlags stage_flags[submit_info.waitSemaphoreCount];

But some people don't like stack-allocated arrays.

> Reviewed-by: Chad Versace 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/1] Use a freelist in nir_opt_dce

[Thomas Helland]

2017-12-02 15:49 GMT+01:00 Thomas Helland :
> This patch tries to reduce the number of calls to ralloc in nir_opt_dce.
> Especially with scalarized shaders we have a bunch of calls to ralloc
> in this pass, hurting us quite bad. See the commit message for details.
>
> The other large caller to ralloc is nir_alu_instr_create, and it would
> be nice if we could allocate groups at a time also here. I'm not sure
> how we can deal with that though, as it does not allocate the same
> number of items each time. I'm also working on a similar approach for
 ^^^
That should be "number of bytes".

> the symbol table, but that is not quite ready yet.
>
> Thomas Helland (1):
>   nir: Use a freelist in nir_opt_dce to avoid spamming ralloc
>
>  src/compiler/nir/nir_opt_dce.c | 47 
> --
>  1 file changed, 32 insertions(+), 15 deletions(-)
>
> --
> 2.15.0
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 0/1] Use a freelist in nir_opt_dce

[Thomas Helland]

This patch tries to reduce the number of calls to ralloc in nir_opt_dce.
Especially with scalarized shaders we have a bunch of calls to ralloc
in this pass, hurting us quite bad. See the commit message for details.

The other large caller to ralloc is nir_alu_instr_create, and it would
be nice if we could allocate groups at a time also here. I'm not sure
how we can deal with that though, as it does not allocate the same
number of items each time. I'm also working on a similar approach for
the symbol table, but that is not quite ready yet.

Thomas Helland (1):
  nir: Use a freelist in nir_opt_dce to avoid spamming ralloc

 src/compiler/nir/nir_opt_dce.c | 47 --
 1 file changed, 32 insertions(+), 15 deletions(-)

-- 
2.15.0

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

[Mesa-dev] [PATCH 1/1] nir: Use a freelist in nir_opt_dce to avoid spamming ralloc

[Thomas Helland]

Also, allocate worklist_elem in groups of 20, to reduce the burden of 
allocation. Do not use rzalloc, as there is no need. This lets us drop 
the number of calls to ralloc from aproximately 10% of all calls to 
ralloc(130 000 calls), down to a mere 2000 calls to ralloc_array_size. 
This cuts the runtime of shader-db by 1%, while at the same time 
reducing the number of stalled cycles, executed cycles, and executed 
instructions by about 1 % as reported by perf. I did a five-run 
benchmark pre and post and got a statistical variance less than 0.1% pre 
and post. This was with i965's ir validation polluting the benchmark, so 
the numbers are even better in release builds.

Performance change as found with perf-diff:
4.74% -0.23%  libc-2.26.so[.] _int_malloc
1.88% -0.21%  libc-2.26.so[.] malloc
2.27% +0.16%  libmesa_dri_drivers.so  [.] match_value.part.7
2.95% -0.12%  libc-2.26.so[.] _int_free
  +0.11%  libmesa_dri_drivers.so  [.] worklist_push
1.22% -0.08%  libc-2.26.so[.] malloc_consolidate
0.16% -0.06%  libmesa_dri_drivers.so  [.] mark_live_cb
1.21% +0.06%  libmesa_dri_drivers.so  [.] match_expression.part.6
0.75% -0.05%  libc-2.26.so[.] cfree@GLIBC_2.2.5
0.50% -0.05%  libmesa_dri_drivers.so  [.] ralloc_size
0.57% +0.04%  libmesa_dri_drivers.so  [.] nir_replace_instr
1.29% -0.04%  libmesa_dri_drivers.so  [.] unsafe_free
---
 src/compiler/nir/nir_opt_dce.c | 47 --
 1 file changed, 32 insertions(+), 15 deletions(-)

diff --git a/src/compiler/nir/nir_opt_dce.c b/src/compiler/nir/nir_opt_dce.c
index 5cefba3a72..f9285fe4ac 100644
--- a/src/compiler/nir/nir_opt_dce.c
+++ b/src/compiler/nir/nir_opt_dce.c
@@ -29,32 +29,46 @@
 
 /* SSA-based mark-and-sweep dead code elimination */
 
+typedef struct {
+   struct exec_list *worklist;
+   struct exec_list *free_nodes;
+} worklist;
+
 typedef struct {
struct exec_node node;
nir_instr *instr;
 } worklist_elem;
 
 static void
-worklist_push(struct exec_list *worklist, nir_instr *instr)
+worklist_push(worklist *worklist, nir_instr *instr)
 {
-   worklist_elem *elem = ralloc(worklist, worklist_elem);
+   if (exec_list_is_empty(worklist->free_nodes)) {
+  worklist_elem *elements = ralloc_array(worklist, worklist_elem, 20);
+  for (int i = 0; i < 20; i++)
+ exec_list_push_tail(worklist->free_nodes, [i].node);
+   }
+
+   struct exec_node *node = exec_list_pop_head(worklist->free_nodes);
+   worklist_elem *elem = exec_node_data(worklist_elem, node, node);
elem->instr = instr;
instr->pass_flags = 1;
-   exec_list_push_tail(worklist, >node);
+   exec_list_push_tail(worklist->worklist, >node);
 }
 
 static nir_instr *
-worklist_pop(struct exec_list *worklist)
+worklist_pop(worklist *worklist)
 {
-   struct exec_node *node = exec_list_pop_head(worklist);
+   
+   struct exec_node *node = exec_list_pop_head(worklist->worklist);
worklist_elem *elem = exec_node_data(worklist_elem, node, node);
+   exec_list_push_head(worklist->free_nodes, node);
return elem->instr;
 }
 
 static bool
 mark_live_cb(nir_src *src, void *_state)
 {
-   struct exec_list *worklist = (struct exec_list *) _state;
+   worklist *worklist = _state;
 
if (src->is_ssa && !src->ssa->parent_instr->pass_flags) {
   worklist_push(worklist, src->ssa->parent_instr);
@@ -64,7 +78,7 @@ mark_live_cb(nir_src *src, void *_state)
 }
 
 static void
-init_instr(nir_instr *instr, struct exec_list *worklist)
+init_instr(nir_instr *instr, worklist *worklist)
 {
nir_alu_instr *alu_instr;
nir_intrinsic_instr *intrin_instr;
@@ -113,7 +127,7 @@ init_instr(nir_instr *instr, struct exec_list *worklist)
 }
 
 static bool
-init_block(nir_block *block, struct exec_list *worklist)
+init_block(nir_block *block, worklist *worklist)
 {
nir_foreach_instr(instr, block)
   init_instr(instr, worklist);
@@ -131,19 +145,22 @@ init_block(nir_block *block, struct exec_list *worklist)
 static bool
 nir_opt_dce_impl(nir_function_impl *impl)
 {
-   struct exec_list *worklist = rzalloc(NULL, struct exec_list);
-   exec_list_make_empty(worklist);
+   worklist *wl = ralloc(NULL, worklist);
+   wl->free_nodes = ralloc(wl, struct exec_list);
+   wl->worklist = ralloc(wl, struct exec_list);
+   exec_list_make_empty(wl->free_nodes);
+   exec_list_make_empty(wl->worklist);
 
nir_foreach_block(block, impl) {
-  init_block(block, worklist);
+  init_block(block, wl);
}
 
-   while (!exec_list_is_empty(worklist)) {
-  nir_instr *instr = worklist_pop(worklist);
-  nir_foreach_src(instr, mark_live_cb, worklist);
+   while (!exec_list_is_empty(wl->worklist)) {
+  nir_instr *instr = worklist_pop(wl);
+  nir_foreach_src(instr, mark_live_cb, wl);
}
 
-   ralloc_free(worklist);
+   ralloc_free(wl);
 
bool progress = false;
 
-- 
2.15.0

___
mesa-dev mailing list

Re: [Mesa-dev] [PATCH 0/3] Fix linkage for libEGL and libGLX without dri3

[Jon Turney]

On 01/12/2017 23:06, Dylan Baker wrote:

As we discussed elsewhere, this should fix the linkage of the dri3 loader and
glx and egl.

Dylan Baker (3):
   meson: Reformat meson code to match more common style
   meson: fix underlinkage without dri3
   meson: Fix overlinkage of dri3 loader

  src/egl/meson.build|  2 +-
  src/glx/meson.build| 14 +-
  src/loader/meson.build |  3 +--
  3 files changed, 11 insertions(+), 8 deletions(-)



Great, thanks!

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

Re: [Mesa-dev] [PATCH v2] nv50/ir: rework the madsp subop handling

[Tobias Klausmann]

Hi,

comments down below...


Greetings,

Tobias


On 12/2/17 12:34 PM, Karol Herbst wrote:

From: Karol Herbst 

Creating correct SubOps for OP_MADSP wasn't easy, because devs needed to know
the proper values for each data type. Also the third source doesn't know any
explicit signess and the current code tried to work around this fact.

To make the creating of the subops much easier, this commit introduces a helper
macro, which can be used in an intuitive way and eliminates the need to know
the correct numbers for creating such subops.

Values for src0 and src1 kept their meaning, src2 changed significantly:

Before:
bit 0 changed the sign of src0
bit 1 changed the sign of src1
bits 2-3 choosed the data type out of 32, 24 and 16L

Now:
bits 0-1 choose the data type out of 32, 24 and 16L

This change also lead to an additional << 2 shift in the emmiter for src2

No regressions in piglit on NVE4

v2: fixed mistake in nvc0 emiter

Signed-off-by: Karol Herbst 
---
  src/gallium/drivers/nouveau/codegen/nv50_ir.h  | 24 +-
  .../drivers/nouveau/codegen/nv50_ir_emit_gk110.cpp |  8 +++-
  .../drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp  |  8 +++-
  .../nouveau/codegen/nv50_ir_lowering_nvc0.cpp  | 10 -
  4 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir.h 
b/src/gallium/drivers/nouveau/codegen/nv50_ir.h
index bc15992df0..10b872e0dd 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir.h
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir.h
@@ -239,11 +239,25 @@ enum operation
  #define NV50_IR_SUBOP_LOAD_LOCKED1
  #define NV50_IR_SUBOP_STORE_UNLOCKED 2
  #define NV50_IR_SUBOP_MADSP_SD 0x
-// Yes, we could represent those with DataType.
-// Or put the type into operation and have a couple 1000 values in that enum.
-// This will have to do for now.
-// The bitfields are supposed to correspond to nve4 ISA.
-#define NV50_IR_SUBOP_MADSP(a,b,c) (((c) << 8) | ((b) << 4) | (a))
+#define NV50_IR_SUBOP_MADSP_SRC0_U32  0x000
+#define NV50_IR_SUBOP_MADSP_SRC0_S32  0x001
+#define NV50_IR_SUBOP_MADSP_SRC0_U24  0x002
+#define NV50_IR_SUBOP_MADSP_SRC0_S24  0x003
+#define NV50_IR_SUBOP_MADSP_SRC0_U16L 0x004
+#define NV50_IR_SUBOP_MADSP_SRC0_S16L 0x005
+#define NV50_IR_SUBOP_MADSP_SRC0_U16H 0x006
+#define NV50_IR_SUBOP_MADSP_SRC0_S16H 0x007
+#define NV50_IR_SUBOP_MADSP_SRC1_U24  0x000
+#define NV50_IR_SUBOP_MADSP_SRC1_S24  0x010
+#define NV50_IR_SUBOP_MADSP_SRC1_U16L 0x020
+#define NV50_IR_SUBOP_MADSP_SRC1_S16L 0x030
+#define NV50_IR_SUBOP_MADSP_SRC2_32   0x000
+#define NV50_IR_SUBOP_MADSP_SRC2_24   0x100
+#define NV50_IR_SUBOP_MADSP_SRC2_16L  0x200
+#define NV50_IR_SUBOP_MADSP_TUPLE(a, b, c) \



call this one _3TUPLE ?



+   ( NV50_IR_SUBOP_MADSP_SRC0_ ## a | \
+ NV50_IR_SUBOP_MADSP_SRC1_ ## b | \
+ NV50_IR_SUBOP_MADSP_SRC2_ ## c )
  #define NV50_IR_SUBOP_V1(d,a,b)(((d) << 10) | ((b) << 5) | (a) | 0x)
  #define NV50_IR_SUBOP_V2(d,a,b)(((d) << 10) | ((b) << 5) | (a) | 0x4000)
  #define NV50_IR_SUBOP_V4(d,a,b)(((d) << 10) | ((b) << 5) | (a) | 0x8000)
diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gk110.cpp 
b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gk110.cpp
index 370427d0d1..f9c34a2760 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gk110.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gk110.cpp
@@ -556,11 +556,9 @@ CodeEmitterGK110::emitMADSP(const Instruction *i)
 if (i->subOp == NV50_IR_SUBOP_MADSP_SD) {
code[1] |= 0x00c0;
 } else {
-  code[1] |= (i->subOp & 0x00f) << 19; // imadp1
-  code[1] |= (i->subOp & 0x0f0) << 20; // imadp2
-  code[1] |= (i->subOp & 0x100) << 11; // imadp3
-  code[1] |= (i->subOp & 0x200) << 15; // imadp3
-  code[1] |= (i->subOp & 0xc00) << 12; // imadp3
+  code[1] |= (i->subOp & 0x007) << 19; // src0
+  code[1] |= (i->subOp & 0x030) << 20; // src1
+  code[1] |= (i->subOp & 0x300) << 14; // src2
 }
  
 if (i->flagsDef >= 0)

diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp 
b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp
index 58594f02c7..38080f04d5 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp
@@ -826,11 +826,9 @@ CodeEmitterNVC0::emitMADSP(const Instruction *i)
 if (i->subOp == NV50_IR_SUBOP_MADSP_SD) {
code[1] |= 0x0180;
 } else {
-  code[0] |= (i->subOp & 0x00f) << 7;
-  code[0] |= (i->subOp & 0x0f0) << 1;
-  code[0] |= (i->subOp & 0x100) >> 3;
-  code[0] |= (i->subOp & 0x200) >> 2;
-  code[1] |= (i->subOp & 0xc00) << 13;
+  code[0] |= (i->subOp & 0x007) << 7;
+  code[0] |= (i->subOp & 0x030) << 1;
+  code[1] |= (i->subOp & 0x300) << 15;
 }
  
 if (i->flagsDef >= 0)

diff --git 

[Mesa-dev] [Bug 104035] When will the egl introp for vaapi be implemented

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=104035

Bug ID: 104035
   Summary: When will the egl introp for vaapi be implemented
   Product: Mesa
   Version: git
  Hardware: x86-64 (AMD64)
OS: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: EGL
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: laichiah...@outlook.com
QA Contact: mesa-dev@lists.freedesktop.org

There is no useful hardware decoding method in Linux, vdpau is too old.

-- 
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] Android: gallium/radeon: fix libmesa_amd_common dependency

[Mauro Rossi]

libmesa_amd_common static dependency is added in Android build
to avoid the following building errors:

In file included from 
external/mesa/src/gallium/drivers/radeon/r600_buffer_common.c:24:
In file included from external/mesa/src/gallium/drivers/radeonsi/si_pipe.h:26:
external/mesa/src/gallium/drivers/radeonsi/si_shader.h:138:10: fatal error: 
'ac_binary.h' file not found
 ^
1 error generated.
...
In file included from 
external/mesa/src/gallium/drivers/radeon/r600_gpu_load.c:34:
In file included from external/mesa/src/gallium/drivers/radeonsi/si_pipe.h:26:
external/mesa/src/gallium/drivers/radeonsi/si_shader.h:138:10: fatal error: 
'ac_binary.h' file not found
 ^
1 error generated.

Fixes: 950221f923 ("radeonsi: remove r600_common_screen")
---
 src/gallium/drivers/radeon/Android.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/gallium/drivers/radeon/Android.mk 
b/src/gallium/drivers/radeon/Android.mk
index 578ab0be91..5efc2033f5 100644
--- a/src/gallium/drivers/radeon/Android.mk
+++ b/src/gallium/drivers/radeon/Android.mk
@@ -30,6 +30,7 @@ include $(CLEAR_VARS)
 
 LOCAL_SRC_FILES := $(C_SOURCES)
 
+LOCAL_STATIC_LIBRARIES := libmesa_amd_common
 LOCAL_SHARED_LIBRARIES := libdrm_radeon
 LOCAL_MODULE := libmesa_pipe_radeon
 
-- 
2.14.1

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

[Mesa-dev] [PATCH v2] nv50/ir: rework the madsp subop handling

[Karol Herbst]

From: Karol Herbst 

Creating correct SubOps for OP_MADSP wasn't easy, because devs needed to know
the proper values for each data type. Also the third source doesn't know any
explicit signess and the current code tried to work around this fact.

To make the creating of the subops much easier, this commit introduces a helper
macro, which can be used in an intuitive way and eliminates the need to know
the correct numbers for creating such subops.

Values for src0 and src1 kept their meaning, src2 changed significantly:

Before:
bit 0 changed the sign of src0
bit 1 changed the sign of src1
bits 2-3 choosed the data type out of 32, 24 and 16L

Now:
bits 0-1 choose the data type out of 32, 24 and 16L

This change also lead to an additional << 2 shift in the emmiter for src2

No regressions in piglit on NVE4

v2: fixed mistake in nvc0 emiter

Signed-off-by: Karol Herbst 
---
 src/gallium/drivers/nouveau/codegen/nv50_ir.h  | 24 +-
 .../drivers/nouveau/codegen/nv50_ir_emit_gk110.cpp |  8 +++-
 .../drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp  |  8 +++-
 .../nouveau/codegen/nv50_ir_lowering_nvc0.cpp  | 10 -
 4 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir.h 
b/src/gallium/drivers/nouveau/codegen/nv50_ir.h
index bc15992df0..10b872e0dd 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir.h
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir.h
@@ -239,11 +239,25 @@ enum operation
 #define NV50_IR_SUBOP_LOAD_LOCKED1
 #define NV50_IR_SUBOP_STORE_UNLOCKED 2
 #define NV50_IR_SUBOP_MADSP_SD 0x
-// Yes, we could represent those with DataType.
-// Or put the type into operation and have a couple 1000 values in that enum.
-// This will have to do for now.
-// The bitfields are supposed to correspond to nve4 ISA.
-#define NV50_IR_SUBOP_MADSP(a,b,c) (((c) << 8) | ((b) << 4) | (a))
+#define NV50_IR_SUBOP_MADSP_SRC0_U32  0x000
+#define NV50_IR_SUBOP_MADSP_SRC0_S32  0x001
+#define NV50_IR_SUBOP_MADSP_SRC0_U24  0x002
+#define NV50_IR_SUBOP_MADSP_SRC0_S24  0x003
+#define NV50_IR_SUBOP_MADSP_SRC0_U16L 0x004
+#define NV50_IR_SUBOP_MADSP_SRC0_S16L 0x005
+#define NV50_IR_SUBOP_MADSP_SRC0_U16H 0x006
+#define NV50_IR_SUBOP_MADSP_SRC0_S16H 0x007
+#define NV50_IR_SUBOP_MADSP_SRC1_U24  0x000
+#define NV50_IR_SUBOP_MADSP_SRC1_S24  0x010
+#define NV50_IR_SUBOP_MADSP_SRC1_U16L 0x020
+#define NV50_IR_SUBOP_MADSP_SRC1_S16L 0x030
+#define NV50_IR_SUBOP_MADSP_SRC2_32   0x000
+#define NV50_IR_SUBOP_MADSP_SRC2_24   0x100
+#define NV50_IR_SUBOP_MADSP_SRC2_16L  0x200
+#define NV50_IR_SUBOP_MADSP_TUPLE(a, b, c) \
+   ( NV50_IR_SUBOP_MADSP_SRC0_ ## a | \
+ NV50_IR_SUBOP_MADSP_SRC1_ ## b | \
+ NV50_IR_SUBOP_MADSP_SRC2_ ## c )
 #define NV50_IR_SUBOP_V1(d,a,b)(((d) << 10) | ((b) << 5) | (a) | 0x)
 #define NV50_IR_SUBOP_V2(d,a,b)(((d) << 10) | ((b) << 5) | (a) | 0x4000)
 #define NV50_IR_SUBOP_V4(d,a,b)(((d) << 10) | ((b) << 5) | (a) | 0x8000)
diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gk110.cpp 
b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gk110.cpp
index 370427d0d1..f9c34a2760 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gk110.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gk110.cpp
@@ -556,11 +556,9 @@ CodeEmitterGK110::emitMADSP(const Instruction *i)
if (i->subOp == NV50_IR_SUBOP_MADSP_SD) {
   code[1] |= 0x00c0;
} else {
-  code[1] |= (i->subOp & 0x00f) << 19; // imadp1
-  code[1] |= (i->subOp & 0x0f0) << 20; // imadp2
-  code[1] |= (i->subOp & 0x100) << 11; // imadp3
-  code[1] |= (i->subOp & 0x200) << 15; // imadp3
-  code[1] |= (i->subOp & 0xc00) << 12; // imadp3
+  code[1] |= (i->subOp & 0x007) << 19; // src0
+  code[1] |= (i->subOp & 0x030) << 20; // src1
+  code[1] |= (i->subOp & 0x300) << 14; // src2
}
 
if (i->flagsDef >= 0)
diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp 
b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp
index 58594f02c7..38080f04d5 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp
@@ -826,11 +826,9 @@ CodeEmitterNVC0::emitMADSP(const Instruction *i)
if (i->subOp == NV50_IR_SUBOP_MADSP_SD) {
   code[1] |= 0x0180;
} else {
-  code[0] |= (i->subOp & 0x00f) << 7;
-  code[0] |= (i->subOp & 0x0f0) << 1;
-  code[0] |= (i->subOp & 0x100) >> 3;
-  code[0] |= (i->subOp & 0x200) >> 2;
-  code[1] |= (i->subOp & 0xc00) << 13;
+  code[0] |= (i->subOp & 0x007) << 7;
+  code[0] |= (i->subOp & 0x030) << 1;
+  code[1] |= (i->subOp & 0x300) << 15;
}
 
if (i->flagsDef >= 0)
diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp 
b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
index 7243b1d2e4..d0eff75bef 100644
--- 

Re: [Mesa-dev] [PATCH 0/5] i965: ASTC5x5 workaround

[Pohjolainen, Topi]

On Fri, Dec 01, 2017 at 07:19:17PM +0200, kevin.rogo...@intel.com wrote:
> From: Kevin Rogovin 
> 
> This patch series implements a needed workaround for Gen9 for ASTC5x5
> sampler reads. The crux of the work around is to make sure that the
> sampler does not read an ASTC5x5 texture and a surface with an auxilary
> buffer without having a texture cache invalidate between such accesses.

The solution here is to store the types of read access (aux, astc5x5) into
context. This information is then used for two purposes: 

   1) do extra tex cache flush when needed and specifically only when needed
   2) resolve surfaces when undesired combination is about to be sampled

Latter case could be addressed also with on-the-fly check in
brw_predraw_resolve_inputs(). There one goes thru all the active textures for
the next draw call and resolves when necessary. One could check for the
undesired combination of textures there. I understand we would need to walk
the textures twice, first to check for any occurences of one type and
then for the other. This, however, would fit to the way we resolve other
texture types and prevent from adding more things to check into the context.

In the first case, if one didn't optimize, i.e., do the extra flush only when
needed, one could instead check if ASTC5x5 texture is going to be sampled and
flush before and after the draw call to be safe. This is not optimal but also
code-wise that simple that I'd be curious to know how much the optimized
flushing helps.

I think the non-optimized cases should be doable also in Vulkan. Jason, what
do you think?

> 
> 
> Kevin Rogovin (5):
>   i965: define astc5x5 workaround infrastructure
>   i965: ASTC5x5 workaround logic for blorp
>   i965: set ASTC5x5 workaround texture type tracking on texture validate
>   i965: use ASTC5x5 workaround in brw_draw
>   i965: use ASTC5x5 workaround in brw_compute
> 
>  src/mesa/drivers/dri/i965/brw_compute.c  |  6 +++
>  src/mesa/drivers/dri/i965/brw_context.c  | 63 
> 
>  src/mesa/drivers/dri/i965/brw_context.h  | 23 +
>  src/mesa/drivers/dri/i965/brw_draw.c |  6 +++
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c |  5 ++
>  src/mesa/drivers/dri/i965/genX_blorp_exec.c  |  5 ++
>  src/mesa/drivers/dri/i965/intel_batchbuffer.c|  1 +
>  src/mesa/drivers/dri/i965/intel_tex_image.c  | 16 --
>  src/mesa/drivers/dri/i965/intel_tex_validate.c   | 13 +
>  9 files changed, 134 insertions(+), 4 deletions(-)
> 
> -- 
> 2.14.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