Re: [Mesa-dev] [PATCH 28/29] i965: Define implementation constants for ARB_shader_image_load_store.

[Kenneth Graunke]

On Saturday, May 02, 2015 06:29:55 PM Francisco Jerez wrote:
> Reviewed-by: Paul Berry 
> 
> v2: Drop VS support pre-Gen8, drop GS support.
> ---
>  src/mesa/drivers/dri/i965/brw_context.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c 
> b/src/mesa/drivers/dri/i965/brw_context.c
> index 30263d0..bd8aa32 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -501,6 +501,18 @@ brw_initialize_context_constants(struct brw_context *brw)
>ctx->Const.Program[MESA_SHADER_GEOMETRY].MaxAtomicBuffers = 
> BRW_MAX_ABO;
>ctx->Const.Program[MESA_SHADER_COMPUTE].MaxAtomicBuffers = BRW_MAX_ABO;
>ctx->Const.MaxCombinedAtomicBuffers = 3 * BRW_MAX_ABO;
> +
> +  ctx->Const.Program[MESA_SHADER_FRAGMENT].MaxImageUniforms =
> + BRW_MAX_IMAGES;
> +  ctx->Const.Program[MESA_SHADER_VERTEX].MaxImageUniforms =
> + (brw->scalar_vs ? BRW_MAX_IMAGES : 0);
> +  ctx->Const.Program[MESA_SHADER_COMPUTE].MaxImageUniforms =
> + BRW_MAX_IMAGES;
> +  ctx->Const.MaxImageUnits = MAX_IMAGE_UNITS;
> +  ctx->Const.MaxCombinedImageUnitsAndFragmentOutputs =
> + MAX_IMAGE_UNITS + BRW_MAX_DRAW_BUFFERS;
> +  ctx->Const.MaxImageSamples = 0;
> +  ctx->Const.MaxCombinedImageUniforms = 3 * BRW_MAX_IMAGES;
> }
>  
> /* Gen6 converts quads to polygon in beginning of 3D pipeline,
> 

Reviewed-by: Kenneth Graunke 


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

Re: [Mesa-dev] [PATCH 8/8] st/mesa: add double input support including lowering (v3)

[Ilia Mirkin]

On Mon, May 4, 2015 at 7:33 PM, Dave Airlie  wrote:
> On 30 April 2015 at 13:23, Ilia Mirkin  wrote:
>> On Wed, Apr 29, 2015 at 9:14 PM, Dave Airlie  wrote:
>>> From: Dave Airlie 
>>>
>>> This takes a different approach to previously, we cannot index into the
>>> inputMapping with anything but the mesa attribute index, so we can't use
>>> the just add one to index trick, we need more info to add one to it
>>> after we've mapped the input.
>>
>> Almost certainly a failing on my part, but the above makes little sense to 
>> me.
>
> Clarifying this is also the answer to all the questions below as well I think,
>
> Due to the double taking one attribute location, but using two hw slots, I 
> can't
> just use attr + 1, VERT_ATTRIB_GENERIC0 could be a dvec4 attribute,
> and VERT_ATTRIB_GENERIC1 could also be a different dvec4 attribute.
>
> So the mapping from TGSI input where we want
> GENERIC0 to map to input 0,1
> and
> GENERIC1 to map to input 2,3
>
> So input 1, can map to GENERIC0, not GENERIC0+1
>
> We also don't want to process each mesa attribute twice, so when we find
> a mesa attribute that is a double one, we do the double processing at
> that point,
>
> so we have the placeholder, which means this TGSI input is just the second
> part of a mesa attribute you already processed, no need to process it again,
> so we just return a NULL client array for it so we skip it.
>
> Not sure if this helps make more or less sense :-)

Yes, thanks. This all looks good to me... would still like a comment
about double_reg2 (at the definition site in the code) as well as a
newline after get_client_array. With those minor bits fixed,

Reviewed-by: Ilia Mirkin 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 6/8] glsl: check total count of multi-slot double vertex attribs

[Dave Airlie]

On 5 May 2015 at 09:47, Ian Romanick  wrote:
> On 04/29/2015 06:14 PM, Dave Airlie wrote:
>> From: Dave Airlie 
>>
>> The spec is vague all over the place about this, but this seems
>> to be the intent, we can probably make this optional later if
>> someone makes hw that cares and writes a driver.
>>
>> Basically we need to double count some of the d types but
>> only for totalling not for slot number assignment.
>>
>> Signed-off-by: Dave Airlie 
>> ---
>>  src/glsl/linker.cpp | 40 +++-
>>  1 file changed, 39 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
>> index 21fde94..5803f82 100644
>> --- a/src/glsl/linker.cpp
>> +++ b/src/glsl/linker.cpp
>> @@ -1975,6 +1975,7 @@ assign_attribute_or_color_locations(gl_shader_program 
>> *prog,
>> } to_assign[16];
>>
>> unsigned num_attr = 0;
>> +   unsigned total_attribs_size = 0;
>>
>> foreach_in_list(ir_instruction, node, sh->ir) {
>>ir_variable *const var = node->as_variable();
>> @@ -2016,12 +2017,41 @@ 
>> assign_attribute_or_color_locations(gl_shader_program *prog,
>>}
>>}
>>
>> +  const unsigned slots = var->type->count_attribute_slots();
>> +
>> +  /* From GL4.5 core spec, section 11.1.1 (Vertex Attributes):
>> +   *
>> +   * "A program with more than the value of MAX_VERTEX_ATTRIBS active
>> +   * attribute variables may fail to link, unless device-dependent
>> +   * optimizations are able to make the program fit within available
>> +   * hardware resources. For the purposes of this test, attribute 
>> variables
>> +   * of the type dvec3, dvec4, dmat2x3, dmat2x4, dmat3, dmat3x4, 
>> dmat4x3,
>> +   * and dmat4 may count as consuming twice as many attributes as 
>> equivalent
>> +   * single-precision types. While these types use the same number of
>> +   * generic attributes as their single-precision equivalents,
>> +   * implementations are permitted to consume two single-precision 
>> vectors
>> +   * of internal storage for each three- or four-component 
>> double-precision
>> +   * vector."
>> +   * Until someone has a good reason in Mesa, enforce that now.
>> +   */
>> +  if (target_index == MESA_SHADER_VERTEX) {
>> +  total_attribs_size += slots;
>> +  if (var->type->without_array() == glsl_type::dvec3_type ||
>> +  var->type->without_array() == glsl_type::dvec4_type ||
>> +  var->type->without_array() == glsl_type::dmat2x3_type ||
>> +  var->type->without_array() == glsl_type::dmat2x4_type ||
>> +  var->type->without_array() == glsl_type::dmat3_type ||
>> +  var->type->without_array() == glsl_type::dmat3x4_type ||
>> +  var->type->without_array() == glsl_type::dmat4x3_type ||
>> +  var->type->without_array() == glsl_type::dmat4_type)
>
> I think doing
>
>if (var->type->without_array()->base_type == GLSL_TYPE_DOUBLE)
>
> should be sufficient here.

No it is only the double types that traverse two slots that matter,
the spec is quite clear that double and dvec2 don't get this treatment,
hence the cut-n-paste above.
>
> Also... we may have discussed this w.r.t. varyings in the original fp64
> series, why not just make count_attribute_slots() return 2*count for
> doubles?

Because they only take one attribute slot, if you create a shader
like

in dvec4 d1;
in dvec4 d2;

the GetAttribLocation should give you back 0, 1, not 0 and 2.

also BindAttribLocation with 0,1 should work, and explicit locations
of 0 and 1 should work.

So they can take up the space for two slots out of the 16 total,
but they can't use the indexes for two.

(I'll remove the extra blank line).

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

Re: [Mesa-dev] [PATCH 5/8] glsl: track which program inputs are doubles

[Dave Airlie]

On 5 May 2015 at 09:53, Ian Romanick  wrote:
> On 04/29/2015 06:14 PM, Dave Airlie wrote:
>> From: Dave Airlie 
>>
>> instead of doing the attempts at dual slot handling here,
>> let the backend do it.
>
> Just curious... why?  It seems like everyone wants the same mapping, so
> doing it one place ought to be better.  There is a lot of stuff in the
> GL specs to anticipate hardware that has never materialized.

Primarily to have the mesa core and glsl core bits be consistent,
since we need for all the query interfaces etc to work as per the GL API

which is double/dvec2/dvec3/dvec4 take one attribute slot at the GL API,

So keeping all the mesa/glsl code based around that reasoning, and
lowering in the backends seems less likely to have corner cases bugs
in things like Get/Bind areas.

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

Re: [Mesa-dev] [PATCH 4/8] glsl: add ARB_vertex_attrib_64bit support.

[Dave Airlie]

On 5 May 2015 at 09:40, Ian Romanick  wrote:
> On 04/29/2015 06:13 PM, Dave Airlie wrote:
>> From: Dave Airlie 
>>
>> Just more boilerplate stuff.
>>
>> Signed-off-by: Dave Airlie 
>> ---
>>  src/glsl/ast_to_hir.cpp | 3 +++
>>  src/glsl/glcpp/glcpp-parse.y| 3 +++
>>  src/glsl/glsl_parser_extras.cpp | 1 +
>>  src/glsl/glsl_parser_extras.h   | 2 ++
>>  4 files changed, 9 insertions(+)
>>
>> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>> index 18b82e3..d1a5eaf 100644
>> --- a/src/glsl/ast_to_hir.cpp
>> +++ b/src/glsl/ast_to_hir.cpp
>> @@ -3537,6 +3537,9 @@ ast_declarator_list::hir(exec_list *instructions,
>>  switch (check_type->base_type) {
>>  case GLSL_TYPE_FLOAT:
>>  break;
>> +case GLSL_TYPE_DOUBLE:
>> +   if (state->is_version(410, 0) || 
>> state->ARB_vertex_attrib_64bit_enable)
>> +  break;
>
> If you fall through here, it will bail from the state->is_version(120,
> 300) below.  That's not right.
>
Indeed you are correct,

I've fixed it up, its a little ugly though, maybe something to refactor later.

will resend,

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

Re: [Mesa-dev] [PATCH 5/8] glsl: track which program inputs are doubles

[Ian Romanick]

On 04/29/2015 06:14 PM, Dave Airlie wrote:
> From: Dave Airlie 
> 
> instead of doing the attempts at dual slot handling here,
> let the backend do it.

Just curious... why?  It seems like everyone wants the same mapping, so
doing it one place ought to be better.  There is a lot of stuff in the
GL specs to anticipate hardware that has never materialized.

> Signed-off-by: Dave Airlie 
> ---
>  src/glsl/ir_set_program_inouts.cpp | 14 ++
>  src/mesa/main/mtypes.h |  1 +
>  2 files changed, 3 insertions(+), 12 deletions(-)
> 
> diff --git a/src/glsl/ir_set_program_inouts.cpp 
> b/src/glsl/ir_set_program_inouts.cpp
> index e877a20..0a410ab 100644
> --- a/src/glsl/ir_set_program_inouts.cpp
> +++ b/src/glsl/ir_set_program_inouts.cpp
> @@ -105,13 +105,10 @@ mark(struct gl_program *prog, ir_variable *var, int 
> offset, int len,
>int idx = var->data.location + var->data.index + offset + i;
>GLbitfield64 bitfield = BITFIELD64_BIT(idx);
>  
> -  /* dvec3 and dvec4 take up 2 slots */
> -  if (dual_slot) {
> - idx += i;
> - bitfield |= bitfield << 1;
> -  }
>if (var->data.mode == ir_var_shader_in) {
>prog->InputsRead |= bitfield;
> +  if (dual_slot)
> + prog->DoubleInputsRead |= bitfield;
>   if (is_fragment_shader) {
>  gl_fragment_program *fprog = (gl_fragment_program *) prog;
>  fprog->InterpQualifier[idx] =
> @@ -120,13 +117,6 @@ mark(struct gl_program *prog, ir_variable *var, int 
> offset, int len,
> fprog->IsCentroid |= bitfield;
>  if (var->data.sample)
> fprog->IsSample |= bitfield;
> -
> -/* Set the InterpQualifier of the next slot to the same as the
> - * current one, since dvec3 and dvec4 spans 2 slots.
> - */
> -if (dual_slot)
> -   fprog->InterpQualifier[idx + 1] =
> -  (glsl_interp_qualifier) var->data.interpolation;
>   }
>} else if (var->data.mode == ir_var_system_value) {
>   prog->SystemValuesRead |= bitfield;
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index efc723a..3860ab9 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -2090,6 +2090,7 @@ struct gl_program
> struct nir_shader *nir;
>  
> GLbitfield64 InputsRead; /**< Bitmask of which input regs are read */
> +   GLbitfield64 DoubleInputsRead; /**< Bitmask of which input regs are 
> read  and are doubles */
> GLbitfield64 OutputsWritten; /**< Bitmask of which output regs are 
> written */
> GLbitfield SystemValuesRead;   /**< Bitmask of SYSTEM_VALUE_x inputs used 
> */
> GLbitfield InputFlags[MAX_PROGRAM_INPUTS];   /**< PROG_PARAM_BIT_x flags 
> */
> 

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

Re: [Mesa-dev] ARB_vertex_attrib_64bit support (2nd posting I think)

[Ian Romanick]

Patches 2, 3, and 7 are

Reviewed-by: Ian Romanick 

Comments sent on patch 4, 5, and 6.

I don't know anything about patches 1 or 8.

On 04/29/2015 06:13 PM, Dave Airlie wrote:
> The biggest change in this series is the attribute slot counting
> code in patch 6, and some cleanups in patch 8.
> 
> I think this isn't going to improve itself much more out of tree,
> 
> Dave.
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

Re: [Mesa-dev] [PATCH 6/8] glsl: check total count of multi-slot double vertex attribs

[Ian Romanick]

On 04/29/2015 06:14 PM, Dave Airlie wrote:
> From: Dave Airlie 
> 
> The spec is vague all over the place about this, but this seems
> to be the intent, we can probably make this optional later if
> someone makes hw that cares and writes a driver.
> 
> Basically we need to double count some of the d types but
> only for totalling not for slot number assignment.
> 
> Signed-off-by: Dave Airlie 
> ---
>  src/glsl/linker.cpp | 40 +++-
>  1 file changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index 21fde94..5803f82 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -1975,6 +1975,7 @@ assign_attribute_or_color_locations(gl_shader_program 
> *prog,
> } to_assign[16];
>  
> unsigned num_attr = 0;
> +   unsigned total_attribs_size = 0;
>  
> foreach_in_list(ir_instruction, node, sh->ir) {
>ir_variable *const var = node->as_variable();
> @@ -2016,12 +2017,41 @@ assign_attribute_or_color_locations(gl_shader_program 
> *prog,
>}
>}
>  
> +  const unsigned slots = var->type->count_attribute_slots();
> +
> +  /* From GL4.5 core spec, section 11.1.1 (Vertex Attributes):
> +   *
> +   * "A program with more than the value of MAX_VERTEX_ATTRIBS active
> +   * attribute variables may fail to link, unless device-dependent
> +   * optimizations are able to make the program fit within available
> +   * hardware resources. For the purposes of this test, attribute 
> variables
> +   * of the type dvec3, dvec4, dmat2x3, dmat2x4, dmat3, dmat3x4, dmat4x3,
> +   * and dmat4 may count as consuming twice as many attributes as 
> equivalent
> +   * single-precision types. While these types use the same number of
> +   * generic attributes as their single-precision equivalents,
> +   * implementations are permitted to consume two single-precision 
> vectors
> +   * of internal storage for each three- or four-component 
> double-precision
> +   * vector."
> +   * Until someone has a good reason in Mesa, enforce that now.
> +   */
> +  if (target_index == MESA_SHADER_VERTEX) {
> +  total_attribs_size += slots;
> +  if (var->type->without_array() == glsl_type::dvec3_type ||
> +  var->type->without_array() == glsl_type::dvec4_type ||
> +  var->type->without_array() == glsl_type::dmat2x3_type ||
> +  var->type->without_array() == glsl_type::dmat2x4_type ||
> +  var->type->without_array() == glsl_type::dmat3_type ||
> +  var->type->without_array() == glsl_type::dmat3x4_type ||
> +  var->type->without_array() == glsl_type::dmat4x3_type ||
> +  var->type->without_array() == glsl_type::dmat4_type)

I think doing

   if (var->type->without_array()->base_type == GLSL_TYPE_DOUBLE)

should be sufficient here.

Also... we may have discussed this w.r.t. varyings in the original fp64
series, why not just make count_attribute_slots() return 2*count for
doubles?

> + total_attribs_size += slots;
> +  }
> +
>/* If the variable is not a built-in and has a location statically
> * assigned in the shader (presumably via a layout qualifier), make 
> sure
> * that it doesn't collide with other assigned locations.  Otherwise,
> * add it to the list of variables that need linker-assigned locations.
> */
> -  const unsigned slots = var->type->count_attribute_slots();
>if (var->data.location != -1) {
>if (var->data.location >= generic_base && var->data.index < 1) {
>   /* From page 61 of the OpenGL 4.0 spec:
> @@ -2141,6 +2171,14 @@ assign_attribute_or_color_locations(gl_shader_program 
> *prog,
>num_attr++;
> }
>  
> +   if (target_index == MESA_SHADER_VERTEX) {
> +  if (total_attribs_size > max_index) {
> +  linker_error(prog,
> +   "attempt to use %d vertex attribute slots only %d 
> available ",
> +   total_attribs_size, max_index);
> +  return false;
> +  }
> +   }

Blank line here.

> /* If all of the attributes were assigned locations by the application (or
>  * are built-in attributes with fixed locations), return early.  This 
> should
>  * be the common case.
> 

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

Re: [Mesa-dev] [PATCH 4/8] glsl: add ARB_vertex_attrib_64bit support.

[Ian Romanick]

On 04/29/2015 06:13 PM, Dave Airlie wrote:
> From: Dave Airlie 
> 
> Just more boilerplate stuff.
> 
> Signed-off-by: Dave Airlie 
> ---
>  src/glsl/ast_to_hir.cpp | 3 +++
>  src/glsl/glcpp/glcpp-parse.y| 3 +++
>  src/glsl/glsl_parser_extras.cpp | 1 +
>  src/glsl/glsl_parser_extras.h   | 2 ++
>  4 files changed, 9 insertions(+)
> 
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 18b82e3..d1a5eaf 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -3537,6 +3537,9 @@ ast_declarator_list::hir(exec_list *instructions,
>  switch (check_type->base_type) {
>  case GLSL_TYPE_FLOAT:
>  break;
> +case GLSL_TYPE_DOUBLE:
> +   if (state->is_version(410, 0) || 
> state->ARB_vertex_attrib_64bit_enable)
> +  break;

If you fall through here, it will bail from the state->is_version(120,
300) below.  That's not right.

>  case GLSL_TYPE_UINT:
>  case GLSL_TYPE_INT:
> if (state->is_version(120, 300))
> diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
> index cfceca6..a11b6b2 100644
> --- a/src/glsl/glcpp/glcpp-parse.y
> +++ b/src/glsl/glcpp/glcpp-parse.y
> @@ -2448,6 +2448,9 @@ _glcpp_parser_handle_version_declaration(glcpp_parser_t 
> *parser, intmax_t versio
>if (extensions->ARB_gpu_shader_fp64)
>   add_builtin_define(parser, "GL_ARB_gpu_shader_fp64", 1);
>  
> +   if (extensions->ARB_vertex_attrib_64bit)
> +  add_builtin_define(parser, "GL_ARB_vertex_attrib_64bit", 1);
> +
> if (extensions->AMD_vertex_shader_layer)
>add_builtin_define(parser, "GL_AMD_vertex_shader_layer", 1);
>  
> diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
> index 0aa3c54..67c0f58 100644
> --- a/src/glsl/glsl_parser_extras.cpp
> +++ b/src/glsl/glsl_parser_extras.cpp
> @@ -565,6 +565,7 @@ static const _mesa_glsl_extension 
> _mesa_glsl_supported_extensions[] = {
> EXT(ARB_texture_query_lod,  true,  false, 
> ARB_texture_query_lod),
> EXT(ARB_texture_rectangle,  true,  false, dummy_true),
> EXT(ARB_uniform_buffer_object,  true,  false, 
> ARB_uniform_buffer_object),
> +   EXT(ARB_vertex_attrib_64bit,true,  false, 
> ARB_vertex_attrib_64bit),
> EXT(ARB_viewport_array, true,  false, ARB_viewport_array),
>  
> /* KHR extensions go here, sorted alphabetically.
> diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
> index dae7864..c3b9098 100644
> --- a/src/glsl/glsl_parser_extras.h
> +++ b/src/glsl/glsl_parser_extras.h
> @@ -458,6 +458,8 @@ struct _mesa_glsl_parse_state {
> bool ARB_texture_rectangle_warn;
> bool ARB_uniform_buffer_object_enable;
> bool ARB_uniform_buffer_object_warn;
> +   bool ARB_vertex_attrib_64bit_enable;
> +   bool ARB_vertex_attrib_64bit_warn;
> bool ARB_viewport_array_enable;
> bool ARB_viewport_array_warn;
>  
> 

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

Re: [Mesa-dev] [PATCH 8/8] st/mesa: add double input support including lowering (v3)

[Dave Airlie]

On 30 April 2015 at 13:23, Ilia Mirkin  wrote:
> On Wed, Apr 29, 2015 at 9:14 PM, Dave Airlie  wrote:
>> From: Dave Airlie 
>>
>> This takes a different approach to previously, we cannot index into the
>> inputMapping with anything but the mesa attribute index, so we can't use
>> the just add one to index trick, we need more info to add one to it
>> after we've mapped the input.
>
> Almost certainly a failing on my part, but the above makes little sense to me.

Clarifying this is also the answer to all the questions below as well I think,

Due to the double taking one attribute location, but using two hw slots, I can't
just use attr + 1, VERT_ATTRIB_GENERIC0 could be a dvec4 attribute,
and VERT_ATTRIB_GENERIC1 could also be a different dvec4 attribute.

So the mapping from TGSI input where we want
GENERIC0 to map to input 0,1
and
GENERIC1 to map to input 2,3

So input 1, can map to GENERIC0, not GENERIC0+1

We also don't want to process each mesa attribute twice, so when we find
a mesa attribute that is a double one, we do the double processing at
that point,

so we have the placeholder, which means this TGSI input is just the second
part of a mesa attribute you already processed, no need to process it again,
so we just return a NULL client array for it so we skip it.

Not sure if this helps make more or less sense :-)

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

Re: [Mesa-dev] i965 implementation of the ARB_shader_image_load_store built-ins. (v2)

[Kenneth Graunke]

On Saturday, May 02, 2015 06:29:27 PM Francisco Jerez wrote:
> This v2 is motivated by the less than enthusiastic reception of my
> last two series implementing the image load, store and atomic
> intrinsics in the i965 back-end.  It substitutes both.
> 
> The built-ins are now implemented in the scalar back-end only, what
> means that IVB and HSW are no longer able to expose image support for
> the VS and GS stages -- behaviour acceptable according to the
> specification.  BDW and up should still be able to expose images in
> the VS stage, and possibly in the GS stage at some point too.  CS
> images can be supported on all gens (that support CS that is).  The
> benefit is that the FS/VEC4 abstraction becomes unnecessary and the
> overall set of changes amounts to less code.
> 
> It can be found in the following branch along with its dependencies:
> http://cgit.freedesktop.org/~currojerez/mesa/log/?h=image-load-store-scalar
> 
> src/mesa/drivers/dri/i965/Makefile.sources   |4 +
> src/mesa/drivers/dri/i965/brw_context.c  |   12 ++
> src/mesa/drivers/dri/i965/brw_fs.cpp |   43 -
> src/mesa/drivers/dri/i965/brw_fs.h   |   19 +-
> src/mesa/drivers/dri/i965/brw_fs_builder.h   |  617 
> ++
> src/mesa/drivers/dri/i965/brw_fs_generator.cpp   |8 +-
> src/mesa/drivers/dri/i965/brw_fs_nir.cpp |   56 --
> src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp | 1144 
> +++
> src/mesa/drivers/dri/i965/brw_fs_surface_builder.h   |  233 
> 
> src/mesa/drivers/dri/i965/brw_fs_visitor.cpp |  466 
> ---
> src/mesa/drivers/dri/i965/brw_ir_fs.h|   56 ++
> src/mesa/drivers/dri/i965/brw_ir_vec4.h  |   55 ++
> src/mesa/drivers/dri/i965/brw_shader.cpp |   32 
> src/mesa/drivers/dri/i965/brw_shader.h   |6 +
> src/mesa/drivers/dri/i965/brw_vec4.cpp   |   10 +
> src/mesa/drivers/dri/i965/brw_vec4.h |6 +
> src/mesa/drivers/dri/i965/brw_vec4_builder.h |  579 
> ++
> src/mesa/drivers/dri/i965/brw_vec4_generator.cpp |8 +-
> src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp   |  179 ++
> src/mesa/drivers/dri/i965/intel_extensions.c |2 +
> 20 files changed, 3224 insertions(+), 311 deletions(-)
> 
> [PATCH 01/29] i965/fs: Have component() set the register stride to zero.
> [PATCH 02/29] i965: Add register constructors taking an backend_reg as 
> argument.
> [PATCH 03/29] i965: Define consistent interface to disable control flow 
> execution masking.
> [PATCH 04/29] i965: Define consistent interface to predicate an instruction.
> [PATCH 05/29] i965: Define consistent interface to enable instruction 
> conditional modifiers.
> [PATCH 06/29] i965: Define consistent interface to enable instruction result 
> saturation.
> [PATCH 07/29] i965/fs: Introduce FS IR builder.
> [PATCH 08/29] i965/vec4: Introduce VEC4 IR builder.
> [PATCH 09/29] i965: Define the setup_vector_uniform_values() backend_visitor 
> interface.
> [PATCH 10/29] i965: Add support for handling image uniforms to the GLSL IR 
> visitors.
> [PATCH 11/29] i965: Add a visitor method to extract the result of a visit.
> [PATCH 12/29] i965/fs: Obtain atomic counter locations by recursing through 
> the visitor.
> [PATCH 13/29] i965/vec4: Obtain atomic counter locations by recursing through 
> the visitor.
> [PATCH 14/29] i965: Lift the constness restriction on surface indices passed 
> to untyped ops.
> [PATCH 15/29] i965/fs: Import array utils for the surface message builder.
> [PATCH 16/29] i965/fs: Import helpers to convert vectors into arrays and back.
> [PATCH 17/29] i965/fs: Import surface message builder functions.
> [PATCH 18/29] i965/fs: Import image access validity checks.
> [PATCH 19/29] i965/fs: Import image memory offset calculation code.
> [PATCH 20/29] i965/fs: Import image format metadata queries.
> [PATCH 21/29] i965/fs: Import image format conversion primitives.
> [PATCH 22/29] i965/fs: Implement image load, store and atomic.
> [PATCH 23/29] i965/fs: Revisit GLSL IR atomic counter intrinsic translation.
> [PATCH 24/29] i965/fs: Revisit NIR atomic counter intrinsic translation.
> [PATCH 25/29] i965/fs: Import GLSL IR image intrinsic translation code.
> [PATCH 26/29] i965/fs: Import GLSL IR memory barrier intrinsic translation 
> code.
> [PATCH 27/29] i965/fs: Drop unused untyped surface read and atomic emit 
> methods.
> [PATCH 28/29] i965: Define implementation constants for 
> ARB_shader_image_load_store.
> [PATCH 29/29] i965: Expose ARB_shader_image_load_store.

Hi Curro,

Thanks for sending this out.  I'm definitely more comfortable with 

Re: [Mesa-dev] [PATCH 6/9] glsl: mark named uniform block arrays as active if defined with shared or std140 layout qualifier

[Matt Turner]

On Tue, Feb 24, 2015 at 10:02 AM, Eduardo Lima Mitev  wrote:
> From: Samuel Iglesias Gonsalvez 
>
> If the named uniform block is an array declared with a shared or
> std140 layout qualifier and no shader instruction has referenced it,
> then it is never marked as active, then num_array_elements is zero.
>
> Section 2.12.6 (Uniform Variables) of the OpenGL ES 3.0.4 spec says
> that is should be considered active:
>
>  "All members of a named uniform block declared with a shared or
>   std140 layout qualifier are considered active, even if they are not
>   referenced in any shader in the program. The uniform block itself is
>   also considered active, even if no member of the block is
>   referenced."
>
> Fixes:
>
> dEQP-GLES3.functional.ubo.random.basic_instance_arrays.18
>
> Signed-off-by: Samuel Iglesias Gonsalvez 
> ---
>  src/glsl/link_uniform_blocks.cpp | 26 ++
>  1 file changed, 26 insertions(+)
>
> diff --git a/src/glsl/link_uniform_blocks.cpp 
> b/src/glsl/link_uniform_blocks.cpp
> index f5fc502..67a3f00 100644
> --- a/src/glsl/link_uniform_blocks.cpp
> +++ b/src/glsl/link_uniform_blocks.cpp
> @@ -213,6 +213,32 @@ link_uniform_blocks(void *mem_ctx,
>const glsl_type *const block_type =
>   b->type->is_array() ? b->type->fields.array : b->type;
>
> +  /* Section 2.12.6 (Uniform Variables) of the OpenGL ES 3.0.4 spec says:
> +   *
> +   *   "All members of a named uniform block declared with a shared or
> +   *   std140 layout qualifier are considered active, even if they are 
> not
> +   *   referenced in any shader in the program. The uniform block itself 
> is
> +   *   also considered active, even if no member of the block is
> +   *   referenced."
> +   *
> +   * If the named uniform block is an array declared with a shared or
> +   * std140 layout qualifier and no shader instruction has referenced it,
> +   * then it is never marked as active, then num_array_elements is zero.
> +   */

As far as I can tell, this is a difference between Desktop GL and ES.
If that is indeed the case, shouldn't the block below contain a
_mesa_is_gles3() check?

> +  if (b->num_array_elements == 0 && b->type->is_array() &&
> +  (b->type->interface_packing == GLSL_INTERFACE_PACKING_STD140 ||
> +   b->type->interface_packing == GLSL_INTERFACE_PACKING_SHARED)) {
> + struct link_uniform_block_active *const block =
> +(struct link_uniform_block_active *) entry->data;
> + block->num_array_elements = block->type->length;
> + block->array_elements = reralloc(mem_ctx,
> +  block->array_elements,
> +  unsigned,
> +  block->num_array_elements);
> + /* Mark the entire array as used. */
> + for (unsigned i = 0; i < block->num_array_elements; i++)
> +block->array_elements[i] = i;
> +  }
>assert((b->num_array_elements > 0) == b->type->is_array());
>
>block_size.num_active_uniforms = 0;
> --
> 2.1.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 5/9] mesa: fix deletion of inactive bound transform feedback object

[Matt Turner]

On Tue, Feb 24, 2015 at 10:02 AM, Eduardo Lima Mitev  wrote:
> From: Samuel Iglesias Gonsalvez 
>
> When a transform feedback object is bound and not active, the OpenGL ES
> 3.0 and GL_ARB_transform_feedback2 specs don't explicitly disallow its
> deletion. Only the deletion of the default framebuffer object is
> forbidden.
>
> This patch follows what it is done for glDeleteTextures(), i.e.
> the binding reverts to 0 (the default framebuffer object).

Is this necessary to make the test pass? I wouldn't have thought this
was correct, from the spec:

"If an active transform feedback object is deleted its name
immediately becomes unused, but the underlying object is not deleted
until it is no longer active"

I don't see any evidence that we should unbind it.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 09/29] i965: Define the setup_vector_uniform_values() backend_visitor interface.

[Kenneth Graunke]

On Saturday, May 02, 2015 06:29:36 PM Francisco Jerez wrote:
> This cleans up the VEC4 implementation of setup_uniform_values()
> somewhat and will avoid duplication of the image uniform upload code
> by having a common interface to upload a vector of uniforms on either
> back-end.
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp   | 12 
>  src/mesa/drivers/dri/i965/brw_fs.h |  4 +++
>  src/mesa/drivers/dri/i965/brw_shader.h |  4 +++
>  src/mesa/drivers/dri/i965/brw_vec4.h   |  3 ++
>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 40 
> ++
>  5 files changed, 44 insertions(+), 19 deletions(-)

Definitely nicer.

Reviewed-by: Kenneth Graunke 



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

Re: [Mesa-dev] [PATCH 14/29] i965: Lift the constness restriction on surface indices passed to untyped ops.

[Kenneth Graunke]

On Saturday, May 02, 2015 06:29:41 PM Francisco Jerez wrote:
> ---
>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp   | 8 ++--
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 4 
>  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 8 ++--
>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp   | 4 
>  4 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp 
> b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> index d476c92..6dd14c2 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> @@ -2015,19 +2015,15 @@ fs_generator::generate_code(const cfg_t *cfg, int 
> dispatch_width)
>   break;
>  
>case SHADER_OPCODE_UNTYPED_ATOMIC:
> - assert(src[1].file == BRW_IMMEDIATE_VALUE &&
> -src[2].file == BRW_IMMEDIATE_VALUE);
> + assert(src[2].file == BRW_IMMEDIATE_VALUE);
>   brw_untyped_atomic(p, dst, src[0], src[1], src[2].dw1.ud,
>  inst->mlen, !inst->dst.is_null());
> - brw_mark_surface_used(prog_data, src[1].dw1.ud);
>   break;
>  
>case SHADER_OPCODE_UNTYPED_SURFACE_READ:
> - assert(src[1].file == BRW_IMMEDIATE_VALUE &&
> -src[2].file == BRW_IMMEDIATE_VALUE);
> + assert(src[2].file == BRW_IMMEDIATE_VALUE);
>   brw_untyped_surface_read(p, dst, src[0], src[1],
>inst->mlen, src[2].dw1.ud);
> - brw_mark_surface_used(prog_data, src[1].dw1.ud);
>   break;
>  
>case SHADER_OPCODE_UNTYPED_SURFACE_WRITE:
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp 
> b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> index 12d02d3..9c394ba 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -157,6 +157,10 @@ fs_visitor::visit(ir_variable *ir)
>if (ir->type->contains_atomic()) {
>   reg = new(this->mem_ctx) fs_reg(ir->data.atomic.offset);
>  
> + brw_mark_surface_used(stage_prog_data,
> +   stage_prog_data->binding_table.abo_start +
> +   ir->data.binding);
> +
>} else if (ir->is_in_uniform_block() || type_size(ir->type) == 0) {
>   /* Thanks to the lower_ubo_reference pass, we will see only
>* ir_binop_ubo_load expressions and not ir_dereference_variable 
> for UBO
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp 
> b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> index 9d37c93..b8f546f 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> @@ -1469,19 +1469,15 @@ vec4_generator::generate_code(const cfg_t *cfg)
>   break;
>  
>case SHADER_OPCODE_UNTYPED_ATOMIC:
> - assert(src[1].file == BRW_IMMEDIATE_VALUE &&
> -src[2].file == BRW_IMMEDIATE_VALUE);
> + assert(src[2].file == BRW_IMMEDIATE_VALUE);
>   brw_untyped_atomic(p, dst, src[0], src[1], src[2].dw1.ud, 
> inst->mlen,
>  !inst->dst.is_null());
> - brw_mark_surface_used(&prog_data->base, src[1].dw1.ud);
>   break;
>  
>case SHADER_OPCODE_UNTYPED_SURFACE_READ:
> - assert(src[1].file == BRW_IMMEDIATE_VALUE &&
> -src[2].file == BRW_IMMEDIATE_VALUE);
> + assert(src[2].file == BRW_IMMEDIATE_VALUE);
>   brw_untyped_surface_read(p, dst, src[0], src[1], inst->mlen,
>src[2].dw1.ud);
> - brw_mark_surface_used(&prog_data->base, src[1].dw1.ud);
>   break;
>  
>case SHADER_OPCODE_UNTYPED_SURFACE_WRITE:
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp 
> b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> index 22586de..1f87d99 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> @@ -1069,6 +1069,10 @@ vec4_visitor::visit(ir_variable *ir)
>if (ir->type->contains_atomic()) {
>   reg = new(this->mem_ctx) dst_reg(ir->data.atomic.offset);
>  
> + brw_mark_surface_used(stage_prog_data,
> +   stage_prog_data->binding_table.abo_start +
> +   ir->data.binding);
> +
>} else if (ir->is_in_uniform_block() || type_size(ir->type) == 0) {
>   /* Thanks to the lower_ubo_reference pass, we will see only
>* ir_binop_ubo_load expressions and not ir_dereference_variable for
> 

This change looks good, but don't you need to make brw_fs_nir.cpp call
brw_mark_surface_used() as well?  It supports atomic counters, today.

With that fixed (or an explanation why it's unnecessary), this is:
Reviewed-by: Kenneth Graunke 


signature.asc
Description: This is a digitally signed message part.

Re: [Mesa-dev] [PATCH 2/9] i965: Fix textureGrad with cube samplers

[Roland Scheidegger]

Am 05.05.2015 um 00:16 schrieb Matt Turner:
> On Tue, Feb 24, 2015 at 10:39 AM, Roland Scheidegger  
> wrote:
>> As a side note (not really directly related to the patch) I think this
>> could benefit from some optimization, unless there's some passes
>> somewhere which can already do this.
>>
>> The non-scalar (non-cube) calculation does this:
>> lod_info.lod = log2(max(sqrt(dot1), sqrt(dot2)))
>> The second sqrt can be trivially eliminated:
>> = log2(sqrt(max(dot1, dot2)))
> 
> I don't think that's valid, since sqrt(x) is greater than x for x < 1.0.
That relationship is true, but not relevant, since
when (x >= y)
-> sqrt(x) >= sqrt(y) still holds true (for non-negative numbers anyway)

Roland



> 
>> And furthermore the remaining sqrt can be killed off too thanks to
>> n*log2(x) = log2(x^n)
>> = 0.5*log2(max(dot1, dot2)))
>>
>> I don't really know anything about the hw, but I would think this should
>> be faster not only on the 1 mathbox-per-gpu ancient igps ;-).
> 
> FWIW, I implemented that optimization in NIR based on this email in
> 
> commit 099c729b4cb6863e8ddbdb9afe7fd7bd53c11ee1
> Author: Matt Turner 
> Date:   Thu Mar 26 10:07:58 2015 -0700
> 
> nir: Add identities for the log function.
> 
> and it didn't find any instances of log(sqrt(x)) in our shader-db. :(
> 
> It did find some rcp(log(x)) though!
> 

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

Re: [Mesa-dev] [PATCH 03/29] i965: Define consistent interface to disable control flow execution masking.

[Kenneth Graunke]

On Saturday, May 02, 2015 06:29:30 PM Francisco Jerez wrote:
> ---
>  src/mesa/drivers/dri/i965/brw_ir_fs.h   | 10 ++
>  src/mesa/drivers/dri/i965/brw_ir_vec4.h |  9 +
>  2 files changed, 19 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_ir_fs.h 
> b/src/mesa/drivers/dri/i965/brw_ir_fs.h
> index ce23fc5..6c65632 100644
> --- a/src/mesa/drivers/dri/i965/brw_ir_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_ir_fs.h
> @@ -258,4 +258,14 @@ public:
> bool pi_noperspective:1;   /**< Pixel interpolator noperspective flag */
>  };
>  
> +/**
> + * Disable per-channel control flow execution masking on \p inst.
> + */
> +static inline fs_inst *
> +exec_all(fs_inst *inst)
> +{
> +   inst->force_writemask_all = true;
> +   return inst;
> +}
> +
>  #endif
> diff --git a/src/mesa/drivers/dri/i965/brw_ir_vec4.h 
> b/src/mesa/drivers/dri/i965/brw_ir_vec4.h
> index 36a8224..48dd90f 100644
> --- a/src/mesa/drivers/dri/i965/brw_ir_vec4.h
> +++ b/src/mesa/drivers/dri/i965/brw_ir_vec4.h
> @@ -192,6 +192,15 @@ public:
> }
>  };
>  
> +/**
> + * Disable per-channel control flow execution masking on \p inst.
> + */
> +inline vec4_instruction *
> +exec_all(vec4_instruction *inst)
> +{
> +   inst->force_writemask_all = true;
> +   return inst;
> +}
>  } /* namespace brw */
>  
>  #endif
> 

Patches 3-6 are:
Reviewed-by: Kenneth Graunke 

Matt and I were confused about the "exec_" prefix in the functions
though - why "exec"?  "exec_all" makes a lot of sense here - execute on
all channels - but "exec_saturate" seems a little odd to me.

Perhaps we can find a different prefix (or you can convince us).


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

Re: [Mesa-dev] [PATCH 2/9] i965: Fix textureGrad with cube samplers

[Matt Turner]

On Tue, Feb 24, 2015 at 10:39 AM, Roland Scheidegger  wrote:
> As a side note (not really directly related to the patch) I think this
> could benefit from some optimization, unless there's some passes
> somewhere which can already do this.
>
> The non-scalar (non-cube) calculation does this:
> lod_info.lod = log2(max(sqrt(dot1), sqrt(dot2)))
> The second sqrt can be trivially eliminated:
> = log2(sqrt(max(dot1, dot2)))

I don't think that's valid, since sqrt(x) is greater than x for x < 1.0.

> And furthermore the remaining sqrt can be killed off too thanks to
> n*log2(x) = log2(x^n)
> = 0.5*log2(max(dot1, dot2)))
>
> I don't really know anything about the hw, but I would think this should
> be faster not only on the 1 mathbox-per-gpu ancient igps ;-).

FWIW, I implemented that optimization in NIR based on this email in

commit 099c729b4cb6863e8ddbdb9afe7fd7bd53c11ee1
Author: Matt Turner 
Date:   Thu Mar 26 10:07:58 2015 -0700

nir: Add identities for the log function.

and it didn't find any instances of log(sqrt(x)) in our shader-db. :(

It did find some rcp(log(x)) though!
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 01/29] i965/fs: Have component() set the register stride to zero.

[Kenneth Graunke]

On Saturday, May 02, 2015 06:29:28 PM Francisco Jerez wrote:
> ---
>  src/mesa/drivers/dri/i965/brw_ir_fs.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_ir_fs.h 
> b/src/mesa/drivers/dri/i965/brw_ir_fs.h
> index e2d2617..32fab65 100644
> --- a/src/mesa/drivers/dri/i965/brw_ir_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_ir_fs.h
> @@ -164,6 +164,7 @@ component(fs_reg reg, unsigned idx)
> assert(idx < reg.width);
> reg.subreg_offset = idx * type_sz(reg.type);
> reg.width = 1;
> +   reg.stride = 0;
> return reg;
>  }

Reviewed-by: Kenneth Graunke 


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

Re: [Mesa-dev] [PATCH 1/9] glsl: don't lower fragdata array if the output data types don't match

[Matt Turner]

On Tue, Feb 24, 2015 at 10:02 AM, Eduardo Lima Mitev  wrote:
> From: Samuel Iglesias Gonsalvez 
>
> Commit 7e414b58640aee6e243d337e72cea290c354f632 broke the gl_FragData array
> into separate gl_FragData[i] variables, so drivers can eliminate useless
> writes to gl_FragData improving their performance.
>
> The problem occurs when GLSL IR code is linked in the following case:
>
> * The FS output variable base data type does not match gl_FragData one (float
>   vector)
> * The FS output variable is replaced by gl_out_FragDataX because of commit
>   7e414b58640aee6 with X from 0 to GL_MAX_DRAW_BUFFERS.
>
> Then the FS output variable base data type is lost in the resulting GLSL IR,
> making that the driver does a wrong assignment to gl_out_FragData components
> because of unmatching data types.
>
> This patch reverts the fragdata array lowering when the output var base data 
> type
> doesn't match gl_out_FragData, i.e., when output variable base data type is
> not a float or a float vector.
>
> This patch fixes 250 dEQP tests (tested in an Intel Haswell machine)
>
> dEQP-GLES3.functional.fragment_out.random.* (22 failed tests)
> dEQP-GLES3.functional.fragment_out.array.uint.* (120 failed tests)
> dEQP-GLES3.functional.fragment_out.array.int.* (108 failed tests)

All of these pass for me on Haswell with today's master branch --
finally a case of NIR's screwing up the types helping something :)

I've confirmed that this generates the same code with non-NIR and NIR
after this patch.

Reviewed-by: Matt Turner 

>
> Signed-off-by: Samuel Iglesias Gonsalvez 
> ---
>  src/glsl/opt_dead_builtin_varyings.cpp | 10 ++
>  1 file changed, 10 insertions(+)
>
> diff --git a/src/glsl/opt_dead_builtin_varyings.cpp 
> b/src/glsl/opt_dead_builtin_varyings.cpp
> index 50c8aa7..4919c8b 100644
> --- a/src/glsl/opt_dead_builtin_varyings.cpp
> +++ b/src/glsl/opt_dead_builtin_varyings.cpp
> @@ -99,6 +99,16 @@ public:
>   }
>   else {
>  this->fragdata_usage |= 1 << index->get_uint_component(0);
> +/* Don't lowered fragdata array if the output variable

typo: lower
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] mesa: fix shininess check for ffvertex_prog v2

[Ilia Mirkin]

On Mon, May 4, 2015 at 5:54 PM, Tim Rowley  wrote:
> Switch to using VERT_BIT_GENERIC macro.  Downloading piglet and
> thought about how to test for this, but it doesn't look like I can
> make a test pass/fail based on the tgsi generated (which is the most
> straightforward way of testing this).

No, it would have to be based on correct/incorrect rendering.

Anyways, this is Reviewed-by: Ilia Mirkin 

>
> diff --git a/src/mesa/main/ffvertex_prog.c b/src/mesa/main/ffvertex_prog.c
> index 7fdd9ba..70adaf8 100644
> --- a/src/mesa/main/ffvertex_prog.c
> +++ b/src/mesa/main/ffvertex_prog.c
> @@ -135,7 +135,7 @@ static GLboolean check_active_shininess( struct 
> gl_context *ctx,
> (key->light_color_material_mask & (1 << attr)))
>return GL_TRUE;
>
> -   if (key->varying_vp_inputs & VERT_ATTRIB_GENERIC(attr))
> +   if (key->varying_vp_inputs & VERT_BIT_GENERIC(attr))
>return GL_TRUE;
>
> if (ctx->Light.Material.Attrib[attr][0] != 0.0F)
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] mesa: fix shininess check for ffvertex_prog v2

[Tim Rowley]

Switch to using VERT_BIT_GENERIC macro.  Downloading piglet and
thought about how to test for this, but it doesn't look like I can
make a test pass/fail based on the tgsi generated (which is the most
straightforward way of testing this).

diff --git a/src/mesa/main/ffvertex_prog.c b/src/mesa/main/ffvertex_prog.c
index 7fdd9ba..70adaf8 100644
--- a/src/mesa/main/ffvertex_prog.c
+++ b/src/mesa/main/ffvertex_prog.c
@@ -135,7 +135,7 @@ static GLboolean check_active_shininess( struct gl_context 
*ctx,
(key->light_color_material_mask & (1 << attr)))
   return GL_TRUE;
 
-   if (key->varying_vp_inputs & VERT_ATTRIB_GENERIC(attr))
+   if (key->varying_vp_inputs & VERT_BIT_GENERIC(attr))
   return GL_TRUE;
 
if (ctx->Light.Material.Attrib[attr][0] != 0.0F)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 00/20] Begin enabling OpenGL ES 3.1

[Ian Romanick]

On 04/30/2015 07:47 AM, Mike Lothian wrote:
> Isn't that override to change the GL version rather than the GLES version?

Patch 20 causes MESA_GL_VERSION_OVERRIDE to affect the GLES version as well.

> On Thu, 30 Apr 2015 00:26 Ian Romanick  > wrote:
> 
> There's still a fair amount functionality left to be implemented before
> GLES 3.1 can actually be enabled.  Compute shaders and SSBOs are the
> biggest things left to finish.  This series just allows people to start
> testing the things that are implemented.  To get a GLES 3.1 context, set
> the environment variable
> 
> MESA_GL_VERSION_OVERRIDE=3.1
> 
> The GLES3 experience taught me that there is a huge pile of little
> differences between GLES and desktop GL... and it takes a really, really
> long time to get those ironed out enough to pass Khronos conformance
> tests.  Getting the initial boiler plate stuff out now lets people start
> testing and fixing sooner.
> 
> This is also available in the gles3.1-enabling branch of my fd.o tree at
> git://people.freedesktop.org/~idr/mesa
> .
> 
> I have tested this with the glsl-es-3.10 patches that I just sent to the
> piglit list.  I get the expected set of passes and failures from the
> minimum-maximums.txt test.
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org 
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 

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

Re: [Mesa-dev] [PATCH] i965: Sort extension enable lists

[Matt Turner]

On Mon, May 4, 2015 at 1:51 PM, Ian Romanick  wrote:
> From: Ian Romanick 
>
> Sort by GEN, then sort by extension name.
>
> Signed-off-by: Ian Romanick 
> ---

Alphabetize! \o/

Reviewed-by: Matt Turner 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] i965: Sort extension enable lists

[Ian Romanick]

From: Ian Romanick 

Sort by GEN, then sort by extension name.

Signed-off-by: Ian Romanick 
---
I had intended to send this out with the GLES3.1 stuff, but, for some
reason, I forgot.

 src/mesa/drivers/dri/i965/intel_extensions.c | 69 ++--
 1 file changed, 35 insertions(+), 34 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c 
b/src/mesa/drivers/dri/i965/intel_extensions.c
index c28c171..b45e848 100644
--- a/src/mesa/drivers/dri/i965/intel_extensions.c
+++ b/src/mesa/drivers/dri/i965/intel_extensions.c
@@ -232,6 +232,7 @@ intelInitExtensions(struct gl_context *ctx)
ctx->Extensions.EXT_pixel_buffer_object = true;
ctx->Extensions.EXT_point_parameters = true;
ctx->Extensions.EXT_provoking_vertex = true;
+   ctx->Extensions.EXT_stencil_two_side = true;
ctx->Extensions.EXT_texture_array = true;
ctx->Extensions.EXT_texture_env_dot3 = true;
ctx->Extensions.EXT_texture_filter_anisotropic = true;
@@ -241,7 +242,6 @@ intelInitExtensions(struct gl_context *ctx)
ctx->Extensions.EXT_texture_sRGB = true;
ctx->Extensions.EXT_texture_sRGB_decode = true;
ctx->Extensions.EXT_texture_swizzle = true;
-   ctx->Extensions.EXT_stencil_two_side = true;
ctx->Extensions.EXT_vertex_array_bgra = true;
ctx->Extensions.AMD_seamless_cubemap_per_texture = true;
ctx->Extensions.APPLE_object_purgeable = true;
@@ -254,10 +254,14 @@ intelInitExtensions(struct gl_context *ctx)
ctx->Extensions.NV_texture_rectangle = true;
ctx->Extensions.TDFX_texture_compression_FXT1 = true;
ctx->Extensions.OES_compressed_ETC1_RGB8_texture = true;
-   ctx->Extensions.OES_EGL_image = true;
ctx->Extensions.OES_draw_texture = true;
-   ctx->Extensions.OES_standard_derivatives = true;
+   ctx->Extensions.OES_EGL_image = true;
ctx->Extensions.OES_EGL_image_external = true;
+   ctx->Extensions.OES_standard_derivatives = true;
+   ctx->Extensions.OES_texture_float = true;
+   ctx->Extensions.OES_texture_float_linear = true;
+   ctx->Extensions.OES_texture_half_float = true;
+   ctx->Extensions.OES_texture_half_float_linear = true;
 
if (brw->gen >= 6)
   ctx->Const.GLSLVersion = 330;
@@ -265,52 +269,62 @@ intelInitExtensions(struct gl_context *ctx)
   ctx->Const.GLSLVersion = 120;
_mesa_override_glsl_version(&ctx->Const);
 
+   if (brw->gen >= 5) {
+  ctx->Extensions.ARB_texture_query_levels = ctx->Const.GLSLVersion >= 130;
+  ctx->Extensions.ARB_texture_query_lod = true;
+  ctx->Extensions.EXT_shader_integer_mix = ctx->Const.GLSLVersion >= 130;
+  ctx->Extensions.EXT_timer_query = true;
+
+  if (brw->gen == 5 || can_write_oacontrol(brw)) {
+ ctx->Extensions.AMD_performance_monitor = true;
+ ctx->Extensions.INTEL_performance_query = true;
+  }
+   }
+
if (brw->gen >= 6) {
   uint64_t dummy;
 
-  ctx->Extensions.EXT_framebuffer_multisample = true;
-  ctx->Extensions.EXT_transform_feedback = true;
-  ctx->Extensions.EXT_framebuffer_multisample_blit_scaled = true;
-  ctx->Extensions.ARB_blend_func_extended = 
!driQueryOptionb(&brw->optionCache, "disable_blend_func_extended");
+  ctx->Extensions.ARB_blend_func_extended =
+ !driQueryOptionb(&brw->optionCache, "disable_blend_func_extended");
+  ctx->Extensions.ARB_conditional_render_inverted = true;
   ctx->Extensions.ARB_draw_buffers_blend = true;
   ctx->Extensions.ARB_ES3_compatibility = true;
-  ctx->Extensions.ARB_uniform_buffer_object = true;
+  ctx->Extensions.ARB_sample_shading = true;
   ctx->Extensions.ARB_shading_language_420pack = true;
+  ctx->Extensions.ARB_shading_language_packing = true;
   ctx->Extensions.ARB_texture_buffer_object = true;
   ctx->Extensions.ARB_texture_buffer_object_rgb32 = true;
   ctx->Extensions.ARB_texture_buffer_range = true;
   ctx->Extensions.ARB_texture_cube_map_array = true;
-  ctx->Extensions.OES_depth_texture_cube_map = true;
-  ctx->Extensions.ARB_shading_language_packing = true;
-  ctx->Extensions.ARB_texture_multisample = true;
-  ctx->Extensions.ARB_sample_shading = true;
   ctx->Extensions.ARB_texture_gather = true;
-  ctx->Extensions.ARB_conditional_render_inverted = true;
+  ctx->Extensions.ARB_texture_multisample = true;
+  ctx->Extensions.ARB_uniform_buffer_object = true;
+
   ctx->Extensions.AMD_vertex_shader_layer = true;
+  ctx->Extensions.EXT_framebuffer_multisample = true;
+  ctx->Extensions.EXT_framebuffer_multisample_blit_scaled = true;
   ctx->Extensions.EXT_polygon_offset_clamp = true;
+  ctx->Extensions.EXT_transform_feedback = true;
+  ctx->Extensions.OES_depth_texture_cube_map = true;
 
   /* Test if the kernel has the ioctl. */
   if (drm_intel_reg_read(brw->bufmgr, TIMESTAMP, &dummy) == 0)
  ctx->Extensions.ARB_timer_query = true;
}
 
-   if (brw->gen >= 5) {
-  ctx->Extensions.ARB_texture_query_lod = true;
-  ctx->Extensions.

Re: [Mesa-dev] [PATCH] mesa: fix shininess check for ffvertex_prog

[Ilia Mirkin]

On Mon, May 4, 2015 at 3:57 PM, Tim Rowley  wrote:
> This was working before because it failed into generating the more
> general case lighting equation.
>
> diff --git a/src/mesa/main/ffvertex_prog.c b/src/mesa/main/ffvertex_prog.c
> index 7fdd9ba..cce0636 100644
> --- a/src/mesa/main/ffvertex_prog.c
> +++ b/src/mesa/main/ffvertex_prog.c
> @@ -135,7 +135,7 @@ static GLboolean check_active_shininess( struct 
> gl_context *ctx,
> (key->light_color_material_mask & (1 << attr)))
>return GL_TRUE;
>
> -   if (key->varying_vp_inputs & VERT_ATTRIB_GENERIC(attr))
> +   if (key->varying_vp_inputs & (1 << VERT_ATTRIB_GENERIC(attr)))

Unfortunately this would have to be 1ULL. To avoid this whole
situation, why not just use VERT_BIT_GENERIC(attr)?

BTW if it's easy, please add a piglit test (or modify existing one) to
hit this. If it's not easy, then... meh. This is a pretty obvious fix,
since:

src/mesa/main/mtypes.h:   GLbitfield64 varying_vp_inputs;  /**< mask
of VERT_BIT_* flags */


  -ilia

>return GL_TRUE;
>
> if (ctx->Light.Material.Attrib[attr][0] != 0.0F)
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] mesa: fix shininess check for ffvertex_prog

[Tim Rowley]

This was working before because it failed into generating the more
general case lighting equation.

diff --git a/src/mesa/main/ffvertex_prog.c b/src/mesa/main/ffvertex_prog.c
index 7fdd9ba..cce0636 100644
--- a/src/mesa/main/ffvertex_prog.c
+++ b/src/mesa/main/ffvertex_prog.c
@@ -135,7 +135,7 @@ static GLboolean check_active_shininess( struct gl_context 
*ctx,
(key->light_color_material_mask & (1 << attr)))
   return GL_TRUE;
 
-   if (key->varying_vp_inputs & VERT_ATTRIB_GENERIC(attr))
+   if (key->varying_vp_inputs & (1 << VERT_ATTRIB_GENERIC(attr)))
   return GL_TRUE;
 
if (ctx->Light.Material.Attrib[attr][0] != 0.0F)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 06/20] glsl: Add glsl_parser_state::has_atomic_counters helper

[Ian Romanick]

On 04/29/2015 11:42 PM, Tapani Pälli wrote:
> 
> 
> On 04/30/2015 02:25 AM, Ian Romanick wrote:
>> From: Ian Romanick 
>>
>> Signed-off-by: Ian Romanick 
>> ---
>>   src/glsl/builtin_functions.cpp | 2 +-
>>   src/glsl/builtin_types.cpp | 2 +-
>>   src/glsl/builtin_variables.cpp | 2 +-
>>   src/glsl/glsl_parser.yy| 4 ++--
>>   src/glsl/glsl_parser_extras.h  | 5 +
>>   5 files changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/glsl/builtin_functions.cpp
>> b/src/glsl/builtin_functions.cpp
>> index 524b8d6..5ce8112 100644
>> --- a/src/glsl/builtin_functions.cpp
>> +++ b/src/glsl/builtin_functions.cpp
>> @@ -359,7 +359,7 @@ tex3d_lod(const _mesa_glsl_parse_state *state)
>>   static bool
>>   shader_atomic_counters(const _mesa_glsl_parse_state *state)
>>   {
>> -   return state->ARB_shader_atomic_counters_enable;
>> +   return state->has_atomic_counters();
>>   }
>>
>>   static bool
>> diff --git a/src/glsl/builtin_types.cpp b/src/glsl/builtin_types.cpp
>> index fef86df..d92e2eb 100644
>> --- a/src/glsl/builtin_types.cpp
>> +++ b/src/glsl/builtin_types.cpp
>> @@ -372,7 +372,7 @@ _mesa_glsl_initialize_types(struct
>> _mesa_glsl_parse_state *state)
>> add_type(symbols, glsl_type::uimage2DMSArray_type);
>>  }
>>
>> -   if (state->ARB_shader_atomic_counters_enable) {
>> +   if (state->has_atomic_counters()) {
>> add_type(symbols, glsl_type::atomic_uint_type);
>>  }
>>
>> diff --git a/src/glsl/builtin_variables.cpp
>> b/src/glsl/builtin_variables.cpp
>> index 21e7331..9d0b272 100644
>> --- a/src/glsl/builtin_variables.cpp
>> +++ b/src/glsl/builtin_variables.cpp
>> @@ -653,7 +653,7 @@ builtin_variable_generator::generate_constants()
>> add_const("gl_MaxTextureCoords", state->Const.MaxTextureCoords);
>>  }
>>
>> -   if (state->ARB_shader_atomic_counters_enable) {
>> +   if (state->has_atomic_counters()) {
>> add_const("gl_MaxVertexAtomicCounters",
>>   state->Const.MaxVertexAtomicCounters);
>> add_const("gl_MaxGeometryAtomicCounters",
>> diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
>> index aceb3b9..b34d7ef 100644
>> --- a/src/glsl/glsl_parser.yy
>> +++ b/src/glsl/glsl_parser.yy
>> @@ -1404,13 +1404,13 @@ layout_qualifier_id:
>> }
>>
>> if ((state->ARB_shading_language_420pack_enable ||
>> -   state->ARB_shader_atomic_counters_enable) &&
>> +   state->has_atomic_counters()) &&
>> match_layout_qualifier("binding", $1, state) == 0) {
>>$$.flags.q.explicit_binding = 1;
>>$$.binding = $3;
>> }
>>
>> -  if (state->ARB_shader_atomic_counters_enable &&
>> +  if (state->has_atomic_counters() &&
>> match_layout_qualifier("offset", $1, state) == 0) {
>>$$.flags.q.explicit_offset = 1;
>>$$.offset = $3;
>> diff --git a/src/glsl/glsl_parser_extras.h
>> b/src/glsl/glsl_parser_extras.h
>> index dae7864..382c200 100644
>> --- a/src/glsl/glsl_parser_extras.h
>> +++ b/src/glsl/glsl_parser_extras.h
>> @@ -186,6 +186,11 @@ struct _mesa_glsl_parse_state {
>> return true;
>>  }
>>
>> +   bool has_atomic_counters() const
>> +   {
>> +  return ARB_shader_atomic_counters_enable || is_version(400, 0);
> 
> IMO version should be 420 as the ATOMIC_UINT keyword 'allowed_glsl'
> field is also set 420 in glsl_lexer.ll.

Yeah, Ilia pointed that out later in the series when I add ES3.1
support.  I believe you're both correct.  With this changed to
is_version(420, 0), is this Reviewed-by?

>> +   }
>> +
>>  bool has_explicit_attrib_stream() const
>>  {
>> return ARB_gpu_shader5_enable || is_version(400, 0);
>>

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

Re: [Mesa-dev] [PATCH] mesa: Restore functionality to dispatch sanity test

[Ian Romanick]

On 04/29/2015 02:44 PM, Brian Paul wrote:
> On 04/29/2015 02:53 PM, Ian Romanick wrote:
>> On 04/29/2015 12:07 PM, Ian Romanick wrote:
>>> From: Ian Romanick 
>>>
>>> Along with a couple secondary goals, the dispatch sanity test had two
>>> major, primary goals.
>>>
>>> 1. Ensure that all functions part of an API version are set in the
>>> dispatch table.
>>>
>>> 2. Ensure that functions that cannot be part of an API version are not
>>> set in the dispatch table.
>>>
>>> Commit 4bdbb58 removed the tests ability to fulfill either of its
>>
>> This commit also breaks binary compatibility between old libGL and new
>> DRI driver.
>>
>> $ EGL_LOG_LEVEL=debug es2_info
>> libEGL debug: Native platform type: x11 (autodetected)
>> libEGL debug: EGL search path is /opt/xorg-master-x86_64/lib64/egl
>> libEGL debug: added egl_dri2 to module array
>> libEGL debug: failed to open
>> /home/idr/devel/graphics/Mesa/BUILD/10.4-64/lib64/i965_dri.so:
>> /home/idr/devel/graphics/Mesa/BUILD/10.4-64/lib64/i965_dri.so:
>> undefined symbol: _glapi_set_nop_handler
>>
>> That's not ok. :(
>>
>> Brian: Can you propose an alternate solution to your original problem
>> that doesn't break compatibility?  Otherwise, I'm going to have to just
>> revert 4bdbb58.
> 
> Please hold off on that.  I'm going to be off-line until next week and
> won't have time to work on this until then at the earliest.

As it turns out, I spent the rest of the week sick in bed, so I held off
on it. :)

>>> primary goals by removing anything that used _mesa_generic_nop().  It
>>> seems like the problem on Windows could have been resolved by adding the
>>> NULL context pointer check from nop_handler to _mesa_generic_nop().
> 
> Unfortunately, no.  The problem is the the OpenGL API uses __stdcall
> convention on Windows (the callee has to restore the stack).  That means
> plugging a single, generic no-op handler into all dispatch slots will
> not work.  The no-op function will not properly clean up the stack and
> we'll crash.  We found this with a professional CAD app on Windows so
> the fix is critical to those users.

Oh... that's annoying, but makes sense.

> A temporary work-around may be to only call _glapi_set_nop_handler() for
> Windows.  Then, maybe remove the #ifdef _WIN32 at some point down the road.

Either that or condition it on DRI builds.  Other Mesa builds won't have
this particular issue.  Other loader / driver interfaces are dynamic.
If we really want to enable this feature in DRI builds, we can do it.
It will just require someone to do a bunch of typing.

> How often do you test mixing old libGL with newer drivers?  I've always
> suspected this is a bit fragile.  Nobody else seems to have noticed.

Periodically.  I generally have a bunch of Mesa branches built at once
that I test for various things.  I mostly end up using mismatched
libraries when I have to use EGL because, for some reason, using
non-installed libEGL is really painful.

> -Brian
> 
> PS: the patch looks OK to me.
> 
> Reviewed-by: Brian Paul 
> 
> 
>>> There is, however, some debugging benefit to actually getting the
>>> (supposed) function name logged in the "unsupported function called"
>>> message.
>>>
>>> The preceding commit added a function, _glapi_new_nop_table, that
>>> allocates a table of per-entry point no-op functions.  Restore the
>>> ability to actually validate the sanity of the dispatch table by using
>>> _glapi_new_nop_table.
>>>
>>> Previous to this commit removing a function from one of the
>>> *_functions_possible lists would not cause the test to fail.  With this
>>> commit removing such a function will result in failure, as is expected.
>>>
>>> Signed-off-by: Ian Romanick 
>>> Cc: Brian Paul 
>>> ---
>>>   src/mesa/main/tests/dispatch_sanity.cpp | 49
>>> +
>>>   1 file changed, 43 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/src/mesa/main/tests/dispatch_sanity.cpp
>>> b/src/mesa/main/tests/dispatch_sanity.cpp
>>> index 946eabb..2a5afcd 100644
>>> --- a/src/mesa/main/tests/dispatch_sanity.cpp
>>> +++ b/src/mesa/main/tests/dispatch_sanity.cpp
>>> @@ -82,6 +82,7 @@ public:
>>>  struct dd_function_table driver_functions;
>>>  struct gl_context share_list;
>>>  struct gl_context ctx;
>>> +   _glapi_proc *nop_table;
>>>   };
>>>
>>>   void
>>> @@ -93,6 +94,9 @@ DispatchSanity_test::SetUp()
>>>  memset(&ctx, 0, sizeof(ctx));
>>>
>>>  _mesa_init_driver_functions(&driver_functions);
>>> +
>>> +   const unsigned size = _glapi_get_dispatch_table_size();
>>> +   nop_table = (_glapi_proc *) _glapi_new_nop_table(size);
>>>   }
>>>
>>>   void
>>> @@ -122,11 +126,18 @@ offset_to_proc_name_safe(unsigned offset)
>>>* _glapi_proc *table exist.
>>>*/
>>>   static void
>>> -validate_functions(struct gl_context *ctx, const struct function
>>> *function_table)
>>> +validate_functions(struct gl_context *ctx, const struct function
>>> *function_table,
>>> +   const _glapi_proc *nop_table)
>>>

Re: [Mesa-dev] [PATCH] clover: add --with-icd-file-dir option

[Aaron Watry]

On Mon, May 4, 2015 at 12:11 PM, Ilia Mirkin  wrote:

> On Mon, May 4, 2015 at 12:47 PM, Tom Stellard  wrote:
> > On Mon, May 04, 2015 at 10:13:19AM -0400, Ilia Mirkin wrote:
> >> On Mon, May 4, 2015 at 10:04 AM, Tom Stellard  wrote:
> >> > On Sat, May 02, 2015 at 01:31:41PM -0400, Ilia Mirkin wrote:
> >> >> On Sat, May 2, 2015 at 1:19 PM, EdB  wrote:
> >> >> > The standard ICD file path is /etc/OpenCL/vendor/.
> >> >> > However it doesn't fit well with custom build.
> >> >> > This option allow ICD vendor file installation path override
> >> >> > ---
> >> >> >  configure.ac   | 6 ++
> >> >> >  src/gallium/targets/opencl/Makefile.am | 2 +-
> >> >> >  2 files changed, 7 insertions(+), 1 deletion(-)
> >> >> >
> >> >> > diff --git a/configure.ac b/configure.ac
> >> >> > index 095e23e..bf08d76 100644
> >> >> > --- a/configure.ac
> >> >> > +++ b/configure.ac
> >> >> > @@ -2005,6 +2005,12 @@ AC_ARG_WITH([d3d-libdir],
> >> >> >  [D3D_DRIVER_INSTALL_DIR="$withval"],
> >> >> >  [D3D_DRIVER_INSTALL_DIR="${libdir}/d3d"])
> >> >> >  AC_SUBST([D3D_DRIVER_INSTALL_DIR])
> >> >> > +AC_ARG_WITH([icd-file-dir],
> >> >> > +[AS_HELP_STRING([--with-icd-file-dir=DIR],
> >> >> > +[directory for the OpenCL ICD vendor file
> @<:@/etc/OpenCL/vendors@:>@])],
> >> >> > +[ICD_FILE_INSTALL_DIR="$withval"],
> >> >> > +[ICD_FILE_INSTALL_DIR="/etc/OpenCL/vendors"])
> >> >>
> >> >> What about making this default to ${sysconfdir}/OpenCL/vendors ? That
> >> >> way using --prefix should auto-make it go into the prefix instead of
> >> >> unexpectedly installing things outside of the specified prefix? That
> >> >> way a distro build which specifies --sysconfdir as /etc will get it
> in
> >> >> the right place, while by default it'll go into /usr/local/etc and a
> >> >> user can override the icd loader's default behaviour with
> >> >> OPENCL_VENDOR_PATH?
> >> >>
> >> >
> >> > I would prefer not to make this the default behavior, because it
> violates the spec
> >> > and there could potentially be multiple icd implementations, which
> may or may not have
> >> > the overrides.
> >> >
> >> > I think the best solution would be to rename the option to something
> like
> >> > --enable-ocl-icd-respect-prefix (suggestions for other names
> encouraged).
> >> > and have the option enable the behavior that Ilia is describing.
> >> >
> >> > This will give distros and advanced users a way to setup their system
> >> > the way they want.
> >>
> >> It's just a very anti-autoconf thing to do to have "make install" fail
> >> by default unless you specify some "hey, i actually want make install
> >> to work" option.
> >>
> >> I think it's crazy to expect that, by default, people will want to
> >> write over their system installs, and having things go outside of the
> >> specified --prefix is very surprising (unless you force some other
> >> option). And asking the user to run "make install" as root is even
> >> crazier.
> >>
> >
> > My expectation is that, by default, when people specify
> --enable-opencl-icd
> > they want an implementation that conforms to the specification.
> > Unfortunately, this means installing icd files to /etc.
>
> Oh, does this only happen when I supply some option? I.e. if I just do
> ./configure --prefix=... it'll still work, I have to do
> --enable-opencl-icd and only *then* does it install the other thing?
> That might be more acceptable, although still not really.
>
> >
> > There is no good solution here, but I'd rather have users specify a flag
> > to get a sane build system, than requiring them to set a flag and set
> > an environment variable just to get working OpenCL with the ICD loader.
>
> I know exceedingly little about OpenCL. I'm just coming at this from a
> generic "build shouldn't try to install things outside the prefix"
> perspective. Make install shouldn't fail, and users shouldn't be
> tempted into accidentally overwriting their system installs even
> though they set a prefix but then foolishly ran 'sudo make install'.
>
> Autoconf is a pretty common build helper, nearly every sane package
> with a unix build target uses it, and I think people are used to how
> it works. One of the cool things is that you can do "./configure
> --prefix=/home/user/install; make install" and expect it to work (as
> in, things landing in /home/user/install/..., not in /). Or perhaps
> you use a system like 'stow' which takes care of all the
> symlinking/etc and allows you to easily have multiple versions of the
> same software.
>
> Having things go outside of the configured directories goes against
> the autoconf standard... perhaps it'd be OK if you added some giant
> warning at the end of configure output saying "DANGER DANGER DANGER
> will do unexpected things by default!".
>
> >
> >> I guess I haven't hit this yet because there's no OpenCL support in
> >> nouveau or freedreno, but I made the same stink about vdpau when Emil
> >> tried to make it install to some system location by 

Re: [Mesa-dev] [PATCH] clover: add --with-icd-file-dir option

[Ilia Mirkin]

On Mon, May 4, 2015 at 12:47 PM, Tom Stellard  wrote:
> On Mon, May 04, 2015 at 10:13:19AM -0400, Ilia Mirkin wrote:
>> On Mon, May 4, 2015 at 10:04 AM, Tom Stellard  wrote:
>> > On Sat, May 02, 2015 at 01:31:41PM -0400, Ilia Mirkin wrote:
>> >> On Sat, May 2, 2015 at 1:19 PM, EdB  wrote:
>> >> > The standard ICD file path is /etc/OpenCL/vendor/.
>> >> > However it doesn't fit well with custom build.
>> >> > This option allow ICD vendor file installation path override
>> >> > ---
>> >> >  configure.ac   | 6 ++
>> >> >  src/gallium/targets/opencl/Makefile.am | 2 +-
>> >> >  2 files changed, 7 insertions(+), 1 deletion(-)
>> >> >
>> >> > diff --git a/configure.ac b/configure.ac
>> >> > index 095e23e..bf08d76 100644
>> >> > --- a/configure.ac
>> >> > +++ b/configure.ac
>> >> > @@ -2005,6 +2005,12 @@ AC_ARG_WITH([d3d-libdir],
>> >> >  [D3D_DRIVER_INSTALL_DIR="$withval"],
>> >> >  [D3D_DRIVER_INSTALL_DIR="${libdir}/d3d"])
>> >> >  AC_SUBST([D3D_DRIVER_INSTALL_DIR])
>> >> > +AC_ARG_WITH([icd-file-dir],
>> >> > +[AS_HELP_STRING([--with-icd-file-dir=DIR],
>> >> > +[directory for the OpenCL ICD vendor file 
>> >> > @<:@/etc/OpenCL/vendors@:>@])],
>> >> > +[ICD_FILE_INSTALL_DIR="$withval"],
>> >> > +[ICD_FILE_INSTALL_DIR="/etc/OpenCL/vendors"])
>> >>
>> >> What about making this default to ${sysconfdir}/OpenCL/vendors ? That
>> >> way using --prefix should auto-make it go into the prefix instead of
>> >> unexpectedly installing things outside of the specified prefix? That
>> >> way a distro build which specifies --sysconfdir as /etc will get it in
>> >> the right place, while by default it'll go into /usr/local/etc and a
>> >> user can override the icd loader's default behaviour with
>> >> OPENCL_VENDOR_PATH?
>> >>
>> >
>> > I would prefer not to make this the default behavior, because it violates 
>> > the spec
>> > and there could potentially be multiple icd implementations, which may or 
>> > may not have
>> > the overrides.
>> >
>> > I think the best solution would be to rename the option to something like
>> > --enable-ocl-icd-respect-prefix (suggestions for other names encouraged).
>> > and have the option enable the behavior that Ilia is describing.
>> >
>> > This will give distros and advanced users a way to setup their system
>> > the way they want.
>>
>> It's just a very anti-autoconf thing to do to have "make install" fail
>> by default unless you specify some "hey, i actually want make install
>> to work" option.
>>
>> I think it's crazy to expect that, by default, people will want to
>> write over their system installs, and having things go outside of the
>> specified --prefix is very surprising (unless you force some other
>> option). And asking the user to run "make install" as root is even
>> crazier.
>>
>
> My expectation is that, by default, when people specify --enable-opencl-icd
> they want an implementation that conforms to the specification.
> Unfortunately, this means installing icd files to /etc.

Oh, does this only happen when I supply some option? I.e. if I just do
./configure --prefix=... it'll still work, I have to do
--enable-opencl-icd and only *then* does it install the other thing?
That might be more acceptable, although still not really.

>
> There is no good solution here, but I'd rather have users specify a flag
> to get a sane build system, than requiring them to set a flag and set
> an environment variable just to get working OpenCL with the ICD loader.

I know exceedingly little about OpenCL. I'm just coming at this from a
generic "build shouldn't try to install things outside the prefix"
perspective. Make install shouldn't fail, and users shouldn't be
tempted into accidentally overwriting their system installs even
though they set a prefix but then foolishly ran 'sudo make install'.

Autoconf is a pretty common build helper, nearly every sane package
with a unix build target uses it, and I think people are used to how
it works. One of the cool things is that you can do "./configure
--prefix=/home/user/install; make install" and expect it to work (as
in, things landing in /home/user/install/..., not in /). Or perhaps
you use a system like 'stow' which takes care of all the
symlinking/etc and allows you to easily have multiple versions of the
same software.

Having things go outside of the configured directories goes against
the autoconf standard... perhaps it'd be OK if you added some giant
warning at the end of configure output saying "DANGER DANGER DANGER
will do unexpected things by default!".

>
>> I guess I haven't hit this yet because there's no OpenCL support in
>> nouveau or freedreno, but I made the same stink about vdpau when Emil
>> tried to make it install to some system location by default. At least
>> a few people seemed to agree with me back then...
>>
>
> Does the vdpau spec also require installation to a specific system director
> (e.g. /etc/) ?

The vdpau loader (libvdpau) loads from a fixed (and arbitrari

Re: [Mesa-dev] [PATCH] clover: add --with-icd-file-dir option

[Tom Stellard]

On Mon, May 04, 2015 at 10:13:19AM -0400, Ilia Mirkin wrote:
> On Mon, May 4, 2015 at 10:04 AM, Tom Stellard  wrote:
> > On Sat, May 02, 2015 at 01:31:41PM -0400, Ilia Mirkin wrote:
> >> On Sat, May 2, 2015 at 1:19 PM, EdB  wrote:
> >> > The standard ICD file path is /etc/OpenCL/vendor/.
> >> > However it doesn't fit well with custom build.
> >> > This option allow ICD vendor file installation path override
> >> > ---
> >> >  configure.ac   | 6 ++
> >> >  src/gallium/targets/opencl/Makefile.am | 2 +-
> >> >  2 files changed, 7 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/configure.ac b/configure.ac
> >> > index 095e23e..bf08d76 100644
> >> > --- a/configure.ac
> >> > +++ b/configure.ac
> >> > @@ -2005,6 +2005,12 @@ AC_ARG_WITH([d3d-libdir],
> >> >  [D3D_DRIVER_INSTALL_DIR="$withval"],
> >> >  [D3D_DRIVER_INSTALL_DIR="${libdir}/d3d"])
> >> >  AC_SUBST([D3D_DRIVER_INSTALL_DIR])
> >> > +AC_ARG_WITH([icd-file-dir],
> >> > +[AS_HELP_STRING([--with-icd-file-dir=DIR],
> >> > +[directory for the OpenCL ICD vendor file 
> >> > @<:@/etc/OpenCL/vendors@:>@])],
> >> > +[ICD_FILE_INSTALL_DIR="$withval"],
> >> > +[ICD_FILE_INSTALL_DIR="/etc/OpenCL/vendors"])
> >>
> >> What about making this default to ${sysconfdir}/OpenCL/vendors ? That
> >> way using --prefix should auto-make it go into the prefix instead of
> >> unexpectedly installing things outside of the specified prefix? That
> >> way a distro build which specifies --sysconfdir as /etc will get it in
> >> the right place, while by default it'll go into /usr/local/etc and a
> >> user can override the icd loader's default behaviour with
> >> OPENCL_VENDOR_PATH?
> >>
> >
> > I would prefer not to make this the default behavior, because it violates 
> > the spec
> > and there could potentially be multiple icd implementations, which may or 
> > may not have
> > the overrides.
> >
> > I think the best solution would be to rename the option to something like
> > --enable-ocl-icd-respect-prefix (suggestions for other names encouraged).
> > and have the option enable the behavior that Ilia is describing.
> >
> > This will give distros and advanced users a way to setup their system
> > the way they want.
> 
> It's just a very anti-autoconf thing to do to have "make install" fail
> by default unless you specify some "hey, i actually want make install
> to work" option.
> 
> I think it's crazy to expect that, by default, people will want to
> write over their system installs, and having things go outside of the
> specified --prefix is very surprising (unless you force some other
> option). And asking the user to run "make install" as root is even
> crazier.
> 

My expectation is that, by default, when people specify --enable-opencl-icd
they want an implementation that conforms to the specification.
Unfortunately, this means installing icd files to /etc.

There is no good solution here, but I'd rather have users specify a flag
to get a sane build system, than requiring them to set a flag and set
an environment variable just to get working OpenCL with the ICD loader.

> I guess I haven't hit this yet because there's no OpenCL support in
> nouveau or freedreno, but I made the same stink about vdpau when Emil
> tried to make it install to some system location by default. At least
> a few people seemed to agree with me back then...
> 

Does the vdpau spec also require installation to a specific system director
(e.g. /etc/) ?

-Tom

>   -ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 08/19] gallium: add set_tess_state to configure default tessellation parameters

[Roland Scheidegger]

AFAIK this isn't possible with d3d11, but if hw can do it it makes sense
indeed.

For the series:
Reviewed-by: Roland Scheidegger 


Am 04.05.2015 um 15:33 schrieb Marek Olšák:
> Hi Roland,
> 
> It depends on the hardware. On Radeon, the tessellation evaluation
> shader can read vertex shader outputs directly. That means the
> tessellation control shader is not necessary if the tessellation
> levels are fixed. set_tess_state sets the fixed tessellation levels
> for exactly this case.
> 
> Other hardware may need to generate a pass-through tessellation
> control shader and upload these parameters as constants for the shader
> to use them.
> 
> Marek
> 
> On Mon, May 4, 2015 at 1:29 PM, Roland Scheidegger  wrote:
>> Seems a bit awkward that you have to set such state separately. I don't
>> know much about how this is supposed to work though, some quick grep
>> says these parameters are used when there's no tessellation control
>> shader, so I guess that's why it is separate state.
>> How does that actually work in hw, can you just switch tesselation
>> control shaders off or do you have to provide some kind of a default
>> tesselation ctrl shader?
>>
>> Roland
>>
>> Am 02.05.2015 um 22:16 schrieb Ilia Mirkin:
>>> Signed-off-by: Ilia Mirkin 
>>> ---
>>>  src/gallium/docs/source/context.rst  | 5 +
>>>  src/gallium/include/pipe/p_context.h | 4 
>>>  2 files changed, 9 insertions(+)
>>>
>>> diff --git a/src/gallium/docs/source/context.rst 
>>> b/src/gallium/docs/source/context.rst
>>> index 5861f46..0908ee7 100644
>>> --- a/src/gallium/docs/source/context.rst
>>> +++ b/src/gallium/docs/source/context.rst
>>> @@ -79,6 +79,11 @@ objects. They all follow simple, one-method binding 
>>> calls, e.g.
>>>should be the same as the number of set viewports and can be up to
>>>PIPE_MAX_VIEWPORTS.
>>>  * ``set_viewport_states``
>>> +* ``set_tess_state`` configures the default tessellation parameters:
>>> +  * ``default_outer_level`` is the default value for the outer tessellation
>>> +levels. This corresponds to GL's ``PATCH_DEFAULT_OUTER_LEVEL``.
>>> +  * ``default_inner_level`` is the default value for the inner tessellation
>>> +levels. This corresponds to GL's ``PATCH_DEFAULT_INNER_LEVEL``.
>>>
>>>
>>>  Sampler Views
>>> diff --git a/src/gallium/include/pipe/p_context.h 
>>> b/src/gallium/include/pipe/p_context.h
>>> index 74c2f2f..afc5d7b 100644
>>> --- a/src/gallium/include/pipe/p_context.h
>>> +++ b/src/gallium/include/pipe/p_context.h
>>> @@ -231,6 +231,10 @@ struct pipe_context {
>>>   unsigned start_slot, unsigned num_views,
>>>   struct pipe_sampler_view **);
>>>
>>> +   void (*set_tess_state)(struct pipe_context *,
>>> +  float default_outer_level[4],
>>> +  float default_inner_level[2]);
>>> +
>>> /**
>>>  * Bind an array of shader resources that will be used by the
>>>  * graphics pipeline.  Any resources that were previously bound to
>>>
>>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=AwIBaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Vjtt0vs_iqoI31UfJxBl7yv9I2FeiaeAYgMTLKRBc_I&m=HzX4eWNBFKn5LqsznbaYEi0lFpm532okt85OoH3eZJI&s=iRWQuZvAY3E-f6Wp5kG0zNIpnmAYKK3H_jjKT3cdgPs&e=
>>  

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

Re: [Mesa-dev] [PATCH 07/29] i965/fs: Introduce FS IR builder.

[Jason Ekstrand]

On Sat, May 2, 2015 at 8:29 AM, Francisco Jerez  wrote:
> The purpose of this change is threefold: First, it improves the
> modularity of the compiler back-end by separating the functionality
> required to construct an i965 IR program from the rest of the visitor
> god-object, what in turn will reduce the coupling between other
> components and the visitor allowing a more modular design.  This patch
> doesn't yet remove the equivalent functionality from the visitor
> classes, that task will be undertaken by a separate series, as it
> involves major back-end surgery.

First off, I'm going to say that I like splitting this out as a
separate object and, apart from the comment or two below, I think this
looks pretty good.  It will be especially nice for optimization or
lowering passes that want to be able to insert stuff at a particular
point.

I would, however, like to ask the more general question of "What do we
want it to look like in the end?"  I want to get a little discussion
of that so that we can figure it out and just get the refactoring done
instead of Curro sending patches and waiting to see if they get
rejected.  That's a really bad way to discuss architecting things.
Here's what I would propose that we work towards.  I'll use the
example of the FS but vec4 would be the same:

 1) fs_shader.  Represents a shader.  This would have the CFG, the
allocator, dispatch width, shader stage, uses_kill, etc.  In other
words, the things local to a shader that we have in fs_visitor right
now.
 2) fs_builder.  Basically what Curro cooked up right here.  It would
be created on a particular fs_shader and would be used for building
things.
 3) fs_visitor.  This would *just* be for GLSL IR -> FS.
 4) fs_nir_visitor.  This would be for NIR -> FS.  We may not bother
with a class at all for this given that NIR is in C.  I really don't
care whether or not we do.
 5) Optimization passes could be part of fs_shader or we could make
them just functions that take a fs_shader as a parameter.  I'd be fine
with either and doing the former would probably be easier for the
initial refactor.
 6) The various helper functions shared between GLSL IR and NIR
visitors such as emit_fragcoord_setup would be static functions that
take a builder as their first parameter instead of being associated
with any given class.  We could make them part of the fs_builder or
fs_shader but I really see no need for that.

Thoughts?
--Jason

> Second, it improves consistency between the scalar and vector
> back-ends in order to enable back-end agnostic IR generation.  The FS
> and VEC4 builders can both be used to generate scalar code with a
> compatible interface or they can be used to generate natural vector
> width code -- 1 or 4 components respectively according to
> traits::chan_size.
>
> Third, the approach to IR construction is somewhat different to what
> the visitor classes currently do.  All parameters affecting code
> generation (execution size, half control, point in the program where
> new instructions are inserted, etc.) are encapsulated in a stand-alone
> object rather than being quasi-global state (yes, anything defined in
> one of the visitor classes is effectively global due to the tight
> coupling with virtually everything else in the compiler back-end).
> This object is lightweight and can be copied, mutated and passed
> around, making helper IR-building functions more flexible because they
> can now simply take a builder object as argument and will inherit its
> IR generation properties in exactly the same way that a discrete
> instruction would from the same builder object.
>
> The emit_typed_write() function from my image-load-store branch is an
> example that illustrates the usefulness of the latter point: Due to
> hardware limitations the function may have to split the untyped
> surface message in 8-wide chunks.  That means that the several
> functions called to help with the construction of the message payload
> are themselves required to set the execution width and half control
> correctly on the instructions they emit, and to allocate all registers
> with half the default width.  With the previous approach this would
> require the used helper functions to be aware of the parameters that
> might differ from the default state and explicitly set the instruction
> bits accordingly.  With the new approach they would get a modified
> builder object as argument that would influence all instructions
> emitted by the helper function as if it were the default state.
>
> Another example is the fs_visitor::VARYING_PULL_CONSTANT_LOAD()
> method.  It doesn't actually emit any instructions, they are simply
> created and inserted into an exec_list which is returned for the
> caller to emit at some location of the program.  This sort of two-step
> emission becomes unnecessary with the builder interface because the
> insertion point is one more of the code generation parameters which
> are part of the builder object.  The caller can simply pa

Re: [Mesa-dev] [PATCH] main/queryobj: add GL_QUERY_TARGET support to GetQueryObjectiv()

[Ilia Mirkin]

Reviewed-by: Ilia Mirkin 

On Mon, May 4, 2015 at 10:06 AM, Martin Peres
 wrote:
> This was missing from my patchset to support the query-related entry
> points of Direct State Access.
>
> Reported-by: Ilia Mirkin 
> Signed-off-by: Martin Peres 
> ---
>  src/mesa/main/queryobj.c | 12 
>  1 file changed, 12 insertions(+)
>
> diff --git a/src/mesa/main/queryobj.c b/src/mesa/main/queryobj.c
> index fbccf3f..5ff1b95 100644
> --- a/src/mesa/main/queryobj.c
> +++ b/src/mesa/main/queryobj.c
> @@ -776,6 +776,9 @@ _mesa_GetQueryObjectiv(GLuint id, GLenum pname, GLint 
> *params)
>  ctx->Driver.CheckQuery( ctx, q );
>   *params = q->Ready;
>   break;
> +  case GL_QUERY_TARGET:
> + *params = q->Target;
> + break;
>default:
>   _mesa_error(ctx, GL_INVALID_ENUM, "glGetQueryObjectivARB(pname)");
>   return;
> @@ -827,6 +830,9 @@ _mesa_GetQueryObjectuiv(GLuint id, GLenum pname, GLuint 
> *params)
>  ctx->Driver.CheckQuery( ctx, q );
>   *params = q->Ready;
>   break;
> +  case GL_QUERY_TARGET:
> + *params = q->Target;
> + break;
>default:
>   _mesa_error(ctx, GL_INVALID_ENUM, "glGetQueryObjectuivARB(pname)");
>   return;
> @@ -867,6 +873,9 @@ _mesa_GetQueryObjecti64v(GLuint id, GLenum pname, 
> GLint64EXT *params)
>  ctx->Driver.CheckQuery( ctx, q );
>   *params = q->Ready;
>   break;
> +  case GL_QUERY_TARGET:
> + *params = q->Target;
> + break;
>default:
>   _mesa_error(ctx, GL_INVALID_ENUM, "glGetQueryObjecti64vARB(pname)");
>   return;
> @@ -907,6 +916,9 @@ _mesa_GetQueryObjectui64v(GLuint id, GLenum pname, 
> GLuint64EXT *params)
>  ctx->Driver.CheckQuery( ctx, q );
>   *params = q->Ready;
>   break;
> +  case GL_QUERY_TARGET:
> + *params = q->Target;
> + break;
>default:
>   _mesa_error(ctx, GL_INVALID_ENUM, 
> "glGetQueryObjectui64vARB(pname)");
>   return;
> --
> 2.3.7
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] clover: add --with-icd-file-dir option

[Ilia Mirkin]

On Mon, May 4, 2015 at 10:04 AM, Tom Stellard  wrote:
> On Sat, May 02, 2015 at 01:31:41PM -0400, Ilia Mirkin wrote:
>> On Sat, May 2, 2015 at 1:19 PM, EdB  wrote:
>> > The standard ICD file path is /etc/OpenCL/vendor/.
>> > However it doesn't fit well with custom build.
>> > This option allow ICD vendor file installation path override
>> > ---
>> >  configure.ac   | 6 ++
>> >  src/gallium/targets/opencl/Makefile.am | 2 +-
>> >  2 files changed, 7 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/configure.ac b/configure.ac
>> > index 095e23e..bf08d76 100644
>> > --- a/configure.ac
>> > +++ b/configure.ac
>> > @@ -2005,6 +2005,12 @@ AC_ARG_WITH([d3d-libdir],
>> >  [D3D_DRIVER_INSTALL_DIR="$withval"],
>> >  [D3D_DRIVER_INSTALL_DIR="${libdir}/d3d"])
>> >  AC_SUBST([D3D_DRIVER_INSTALL_DIR])
>> > +AC_ARG_WITH([icd-file-dir],
>> > +[AS_HELP_STRING([--with-icd-file-dir=DIR],
>> > +[directory for the OpenCL ICD vendor file 
>> > @<:@/etc/OpenCL/vendors@:>@])],
>> > +[ICD_FILE_INSTALL_DIR="$withval"],
>> > +[ICD_FILE_INSTALL_DIR="/etc/OpenCL/vendors"])
>>
>> What about making this default to ${sysconfdir}/OpenCL/vendors ? That
>> way using --prefix should auto-make it go into the prefix instead of
>> unexpectedly installing things outside of the specified prefix? That
>> way a distro build which specifies --sysconfdir as /etc will get it in
>> the right place, while by default it'll go into /usr/local/etc and a
>> user can override the icd loader's default behaviour with
>> OPENCL_VENDOR_PATH?
>>
>
> I would prefer not to make this the default behavior, because it violates the 
> spec
> and there could potentially be multiple icd implementations, which may or may 
> not have
> the overrides.
>
> I think the best solution would be to rename the option to something like
> --enable-ocl-icd-respect-prefix (suggestions for other names encouraged).
> and have the option enable the behavior that Ilia is describing.
>
> This will give distros and advanced users a way to setup their system
> the way they want.

It's just a very anti-autoconf thing to do to have "make install" fail
by default unless you specify some "hey, i actually want make install
to work" option.

I think it's crazy to expect that, by default, people will want to
write over their system installs, and having things go outside of the
specified --prefix is very surprising (unless you force some other
option). And asking the user to run "make install" as root is even
crazier.

I guess I haven't hit this yet because there's no OpenCL support in
nouveau or freedreno, but I made the same stink about vdpau when Emil
tried to make it install to some system location by default. At least
a few people seemed to agree with me back then...

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

Re: [Mesa-dev] [PATCH 0/9] Some egl/wayland patches

[Daniel Stone]

Hi,

On 2 May 2015 at 11:15, Axel Davy  wrote:
> Since gallium egl state tracker was dropped,
> there was no way to use swrast with wayland,
> since it was not implemented for egl_dri2.
> https://bugs.freedesktop.org/show_bug.cgi?id=86701
>
> This patch serie aimed at implementing swrast support
> for the wayland egl_dri2 backend.
> But I also went at fixing a few other related things.
>
> I also took the opportunity to make new versions of my
> patches from last year to implement render-nodes and
> DRI_PRIME support. The only major difference is that
> it doesn't have fallback when blitImage is unimplemented
> by driver. It is implemented by gallium drivers only.
> It just means for now that you can use a radeon or nouveau
> card with DRI_PRIME, but not an intel card. So all normal
> use cases are supported.
>
> The swrast __DRIswrastLoaderExtension API seems to have been
> designed with X11 in mind. As a result, it doesn't fit perfectly
> wayland: a copy could be avoided by upgrading this API.
> There doesn't seem to be interest in doing this work for a small
> gain for something that's not as efficient as hw rendering anyway.

Patches 1-7 are Reviewed-by: Daniel Stone .
Patch 8 looks OK, except that it seems like the anonymous-file helper
could be made shared somewhere; if so, brilliant, then if not, R-b
anyway. I'm not really competent to review patch 9 if I'm honest.

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

[Mesa-dev] [PATCH] main/queryobj: add GL_QUERY_TARGET support to GetQueryObjectiv()

[Martin Peres]

This was missing from my patchset to support the query-related entry
points of Direct State Access.

Reported-by: Ilia Mirkin 
Signed-off-by: Martin Peres 
---
 src/mesa/main/queryobj.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/src/mesa/main/queryobj.c b/src/mesa/main/queryobj.c
index fbccf3f..5ff1b95 100644
--- a/src/mesa/main/queryobj.c
+++ b/src/mesa/main/queryobj.c
@@ -776,6 +776,9 @@ _mesa_GetQueryObjectiv(GLuint id, GLenum pname, GLint 
*params)
 ctx->Driver.CheckQuery( ctx, q );
  *params = q->Ready;
  break;
+  case GL_QUERY_TARGET:
+ *params = q->Target;
+ break;
   default:
  _mesa_error(ctx, GL_INVALID_ENUM, "glGetQueryObjectivARB(pname)");
  return;
@@ -827,6 +830,9 @@ _mesa_GetQueryObjectuiv(GLuint id, GLenum pname, GLuint 
*params)
 ctx->Driver.CheckQuery( ctx, q );
  *params = q->Ready;
  break;
+  case GL_QUERY_TARGET:
+ *params = q->Target;
+ break;
   default:
  _mesa_error(ctx, GL_INVALID_ENUM, "glGetQueryObjectuivARB(pname)");
  return;
@@ -867,6 +873,9 @@ _mesa_GetQueryObjecti64v(GLuint id, GLenum pname, 
GLint64EXT *params)
 ctx->Driver.CheckQuery( ctx, q );
  *params = q->Ready;
  break;
+  case GL_QUERY_TARGET:
+ *params = q->Target;
+ break;
   default:
  _mesa_error(ctx, GL_INVALID_ENUM, "glGetQueryObjecti64vARB(pname)");
  return;
@@ -907,6 +916,9 @@ _mesa_GetQueryObjectui64v(GLuint id, GLenum pname, 
GLuint64EXT *params)
 ctx->Driver.CheckQuery( ctx, q );
  *params = q->Ready;
  break;
+  case GL_QUERY_TARGET:
+ *params = q->Target;
+ break;
   default:
  _mesa_error(ctx, GL_INVALID_ENUM, "glGetQueryObjectui64vARB(pname)");
  return;
-- 
2.3.7

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

Re: [Mesa-dev] [PATCH] clover: add --with-icd-file-dir option

[Tom Stellard]

On Sat, May 02, 2015 at 01:31:41PM -0400, Ilia Mirkin wrote:
> On Sat, May 2, 2015 at 1:19 PM, EdB  wrote:
> > The standard ICD file path is /etc/OpenCL/vendor/.
> > However it doesn't fit well with custom build.
> > This option allow ICD vendor file installation path override
> > ---
> >  configure.ac   | 6 ++
> >  src/gallium/targets/opencl/Makefile.am | 2 +-
> >  2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index 095e23e..bf08d76 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -2005,6 +2005,12 @@ AC_ARG_WITH([d3d-libdir],
> >  [D3D_DRIVER_INSTALL_DIR="$withval"],
> >  [D3D_DRIVER_INSTALL_DIR="${libdir}/d3d"])
> >  AC_SUBST([D3D_DRIVER_INSTALL_DIR])
> > +AC_ARG_WITH([icd-file-dir],
> > +[AS_HELP_STRING([--with-icd-file-dir=DIR],
> > +[directory for the OpenCL ICD vendor file 
> > @<:@/etc/OpenCL/vendors@:>@])],
> > +[ICD_FILE_INSTALL_DIR="$withval"],
> > +[ICD_FILE_INSTALL_DIR="/etc/OpenCL/vendors"])
> 
> What about making this default to ${sysconfdir}/OpenCL/vendors ? That
> way using --prefix should auto-make it go into the prefix instead of
> unexpectedly installing things outside of the specified prefix? That
> way a distro build which specifies --sysconfdir as /etc will get it in
> the right place, while by default it'll go into /usr/local/etc and a
> user can override the icd loader's default behaviour with
> OPENCL_VENDOR_PATH?
> 

I would prefer not to make this the default behavior, because it violates the 
spec
and there could potentially be multiple icd implementations, which may or may 
not have
the overrides.

I think the best solution would be to rename the option to something like
--enable-ocl-icd-respect-prefix (suggestions for other names encouraged).
and have the option enable the behavior that Ilia is describing.

This will give distros and advanced users a way to setup their system
the way they want.  

-Tom

> > +AC_SUBST([ICD_FILE_INSTALL_DIR])
> >
> >  dnl
> >  dnl Gallium helper functions
> > diff --git a/src/gallium/targets/opencl/Makefile.am 
> > b/src/gallium/targets/opencl/Makefile.am
> > index 5daf327..9f0e58e 100644
> > --- a/src/gallium/targets/opencl/Makefile.am
> > +++ b/src/gallium/targets/opencl/Makefile.am
> > @@ -47,7 +47,7 @@ EXTRA_lib@OPENCL_LIBNAME@_la_DEPENDENCIES = opencl.sym
> >  EXTRA_DIST = mesa.icd opencl.sym
> >
> >  if HAVE_CLOVER_ICD
> > -icddir = /etc/OpenCL/vendors/
> > +icddir = $(ICD_FILE_INSTALL_DIR)
> >  icd_DATA = mesa.icd
> >  endif
> >
> > --
> > 2.1.0
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] mesa: Always return a value in _mesa_format_from_format_and_type

[Iago Toral]

On Mon, 2015-05-04 at 05:52 -0700, Jason Ekstrand wrote:
> 
> On May 4, 2015 4:25 AM, "Iago Toral Quiroga" 
> wrote:
> >
> > Return MESA_FORMAT_NONE if no matching type was found. We have an
> > assertion here, but we should return something anyway to avoid
> > confusion with non-debug builds.
> > ---
> >  src/mesa/main/glformats.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
> > index 8ced579..fddd048 100644
> > --- a/src/mesa/main/glformats.c
> > +++ b/src/mesa/main/glformats.c
> > @@ -2752,4 +2752,5 @@ _mesa_format_from_format_and_type(GLenum
> format, GLenum type)
> >  * format in that case.
> >  */
> > unreachable("Unsupported format");
> > +   return MESA_FORMAT_NONE;
> 
> If we want this to have well-defined results, this should not be an
> unreachable().  The semantics of unreachable allow the compiler to
> generate wrong code for the case where it does get reached.  We could
> switch it to an assert or we could just drop this patch. I'd be a fan
> of the latter.
> --Jason

Ok, let's drop this patch then.

Iago

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

Re: [Mesa-dev] [PATCH 4/7] i965: Perform basic optimizations on the FIND_LIVE_CHANNEL opcode.

[Francisco Jerez]

Kenneth Graunke  writes:

> On Thursday, April 30, 2015 04:59:49 PM Francisco Jerez wrote:
>> Matt Turner  writes:
>> 
>> > On Fri, Feb 20, 2015 at 11:49 AM, Francisco Jerez  
>> > wrote:
>> >> ---
>> >>  src/mesa/drivers/dri/i965/brw_fs.cpp   | 49 
>> >> ++
>> >>  src/mesa/drivers/dri/i965/brw_fs.h |  1 +
>> >>  src/mesa/drivers/dri/i965/brw_fs_cse.cpp   |  1 +
>> >>  src/mesa/drivers/dri/i965/brw_vec4.cpp | 41 +
>> >>  src/mesa/drivers/dri/i965/brw_vec4.h   |  1 +
>> >>  src/mesa/drivers/dri/i965/brw_vec4_cse.cpp |  1 +
>> >>  6 files changed, 94 insertions(+)
>> >>
>> >> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
>> >> b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> >> index d567c2b..4537900 100644
>> >> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> >> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> >> @@ -2734,6 +2734,54 @@ fs_visitor::compute_to_mrf()
>> >>  }
>> >>
>> >>  /**
>> >> + * Eliminate FIND_LIVE_CHANNEL instructions occurring outside any control
>> >> + * flow.  We could probably do better here with some form of divergence
>> >> + * analysis.
>> >> + */
>> >> +bool
>> >> +fs_visitor::eliminate_find_live_channel()
>> >> +{
>> >> +   bool progress = false;
>> >> +   unsigned depth = 0;
>> >> +
>> >> +   foreach_block_and_inst_safe(block, fs_inst, inst, cfg) {
>> >> +  switch (inst->opcode) {
>> >> +  case BRW_OPCODE_IF:
>> >> +  case BRW_OPCODE_DO:
>> >> + depth++;
>> >> + break;
>> >> +
>> >> +  case BRW_OPCODE_ENDIF:
>> >> +  case BRW_OPCODE_WHILE:
>> >> + depth--;
>> >> + break;
>> >> +
>> >> +  case FS_OPCODE_DISCARD_JUMP:
>> >> + /* This can potentially make control flow non-uniform until the 
>> >> end
>> >> +  * of the program.
>> >> +  */
>> >> + depth++;
>> >> + break;
>
> Why not just "return progress" at this point?  depth++ just makes it
> effectively never do anything again, but continue iterating over the
> rest of the program.
>
Heh, indeed.

> Otherwise, this all seems good to me.  This patch is
> Reviewed-by: Kenneth Graunke 
>
Thank you both for having a look. :)

>> >> +
>> >> +  case SHADER_OPCODE_FIND_LIVE_CHANNEL:
>> >> + if (depth == 0) {
>> >> +inst->opcode = BRW_OPCODE_MOV;
>> >> +inst->src[0] = fs_reg(0);
>> >
>> > How does this work if we're on a primitive edge and channel zero is
>> > disabled? Does the EU not actually disable those channels except in
>> > the framebuffer write or something?
>> >
>> Yes, exactly, samples outside the primitive (or samples discarded by the
>> early depth/stencil test) have the corresponding execution mask bit
>> enabled, otherwise it wouldn't be possible to calculate derivatives near
>> the edge of a primitive.  Whole subspans not covered by the primitive or
>> dropped by early fragment tests are never dispatched to any PS thread,
>> so channel "0" should always have valid contents except when disabled by
>> control flow.
>> 
>> >> +inst->sources = 1;
>> >> +inst->force_writemask_all = true;
>> >> +progress = true;
>> >> + }
>> >> + break;
>> >> +
>> >> +  default:
>> >> + break;
>> >> +  }
>> >> +   }
>> >> +
>> >> +   return progress;
>> >> +}
>> >
>> > I wouldn't mind if this was just part of opt_algebraic. Less code that
>> > way and one fewer pass over the instruction list. What do you think,
>> > Ken?
>> 
>> I shortly considered doing that but finally decided not to because:
>>  - It's not an algebraic instruction.
>>  - The optimization is non-local unlike the other fully self-contained
>>algebraic optimizations, as it needs to keep track of the control
>>flow nesting level.


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

Re: [Mesa-dev] [PATCH 7/7] i965/sync: Implement DRI2_Fence extension

[Daniel Stone]

Hi,

On 1 May 2015 at 21:02, Chad Versace  wrote:
> +static bool
> +brw_fence_has_completed(struct brw_fence *fence)
> +{
> +   if (fence->signalled)
> +  return true;
> +
> +   if (fence->batch_bo && !drm_intel_bo_busy(fence->batch_bo)) {
> +  drm_intel_bo_unreference(fence->batch_bo);
> +  fence->batch_bo = NULL;

Should this branch be setting fence->signalled = true as well, to
match client_wait?

The rest looks good to me, going on what I remember of having looked
at this about 5 years ago, so:
Reviewed-by: Daniel Stone 

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

Re: [Mesa-dev] [PATCH 08/19] gallium: add set_tess_state to configure default tessellation parameters

[Ilia Mirkin]

FWIW nvidia also does not require a special TCS in this case and can
program the tessellation levels directly from the cmd stream.
[Actually it might need a TCS with only an "exit" instruction, I
should double-check.] However AFAIK Intel hardware requires a more
involved passthrough TCS which handles setting the levels and
potentially copying the per-vertex values over, not sure about the
latter bit. Here is the nvc0 impl:

https://github.com/imirkin/mesa/commit/3b7b3887f2dc106305efc3b49b295551e24d2790#diff-21f2776e5f66dfdae77438adc195ba2aR539

On Mon, May 4, 2015 at 9:33 AM, Marek Olšák  wrote:
> Hi Roland,
>
> It depends on the hardware. On Radeon, the tessellation evaluation
> shader can read vertex shader outputs directly. That means the
> tessellation control shader is not necessary if the tessellation
> levels are fixed. set_tess_state sets the fixed tessellation levels
> for exactly this case.
>
> Other hardware may need to generate a pass-through tessellation
> control shader and upload these parameters as constants for the shader
> to use them.
>
> Marek
>
> On Mon, May 4, 2015 at 1:29 PM, Roland Scheidegger  wrote:
>> Seems a bit awkward that you have to set such state separately. I don't
>> know much about how this is supposed to work though, some quick grep
>> says these parameters are used when there's no tessellation control
>> shader, so I guess that's why it is separate state.
>> How does that actually work in hw, can you just switch tesselation
>> control shaders off or do you have to provide some kind of a default
>> tesselation ctrl shader?
>>
>> Roland
>>
>> Am 02.05.2015 um 22:16 schrieb Ilia Mirkin:
>>> Signed-off-by: Ilia Mirkin 
>>> ---
>>>  src/gallium/docs/source/context.rst  | 5 +
>>>  src/gallium/include/pipe/p_context.h | 4 
>>>  2 files changed, 9 insertions(+)
>>>
>>> diff --git a/src/gallium/docs/source/context.rst 
>>> b/src/gallium/docs/source/context.rst
>>> index 5861f46..0908ee7 100644
>>> --- a/src/gallium/docs/source/context.rst
>>> +++ b/src/gallium/docs/source/context.rst
>>> @@ -79,6 +79,11 @@ objects. They all follow simple, one-method binding 
>>> calls, e.g.
>>>should be the same as the number of set viewports and can be up to
>>>PIPE_MAX_VIEWPORTS.
>>>  * ``set_viewport_states``
>>> +* ``set_tess_state`` configures the default tessellation parameters:
>>> +  * ``default_outer_level`` is the default value for the outer tessellation
>>> +levels. This corresponds to GL's ``PATCH_DEFAULT_OUTER_LEVEL``.
>>> +  * ``default_inner_level`` is the default value for the inner tessellation
>>> +levels. This corresponds to GL's ``PATCH_DEFAULT_INNER_LEVEL``.
>>>
>>>
>>>  Sampler Views
>>> diff --git a/src/gallium/include/pipe/p_context.h 
>>> b/src/gallium/include/pipe/p_context.h
>>> index 74c2f2f..afc5d7b 100644
>>> --- a/src/gallium/include/pipe/p_context.h
>>> +++ b/src/gallium/include/pipe/p_context.h
>>> @@ -231,6 +231,10 @@ struct pipe_context {
>>>   unsigned start_slot, unsigned num_views,
>>>   struct pipe_sampler_view **);
>>>
>>> +   void (*set_tess_state)(struct pipe_context *,
>>> +  float default_outer_level[4],
>>> +  float default_inner_level[2]);
>>> +
>>> /**
>>>  * Bind an array of shader resources that will be used by the
>>>  * graphics pipeline.  Any resources that were previously bound to
>>>
>>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 08/19] gallium: add set_tess_state to configure default tessellation parameters

[Marek Olšák]

Hi Roland,

It depends on the hardware. On Radeon, the tessellation evaluation
shader can read vertex shader outputs directly. That means the
tessellation control shader is not necessary if the tessellation
levels are fixed. set_tess_state sets the fixed tessellation levels
for exactly this case.

Other hardware may need to generate a pass-through tessellation
control shader and upload these parameters as constants for the shader
to use them.

Marek

On Mon, May 4, 2015 at 1:29 PM, Roland Scheidegger  wrote:
> Seems a bit awkward that you have to set such state separately. I don't
> know much about how this is supposed to work though, some quick grep
> says these parameters are used when there's no tessellation control
> shader, so I guess that's why it is separate state.
> How does that actually work in hw, can you just switch tesselation
> control shaders off or do you have to provide some kind of a default
> tesselation ctrl shader?
>
> Roland
>
> Am 02.05.2015 um 22:16 schrieb Ilia Mirkin:
>> Signed-off-by: Ilia Mirkin 
>> ---
>>  src/gallium/docs/source/context.rst  | 5 +
>>  src/gallium/include/pipe/p_context.h | 4 
>>  2 files changed, 9 insertions(+)
>>
>> diff --git a/src/gallium/docs/source/context.rst 
>> b/src/gallium/docs/source/context.rst
>> index 5861f46..0908ee7 100644
>> --- a/src/gallium/docs/source/context.rst
>> +++ b/src/gallium/docs/source/context.rst
>> @@ -79,6 +79,11 @@ objects. They all follow simple, one-method binding 
>> calls, e.g.
>>should be the same as the number of set viewports and can be up to
>>PIPE_MAX_VIEWPORTS.
>>  * ``set_viewport_states``
>> +* ``set_tess_state`` configures the default tessellation parameters:
>> +  * ``default_outer_level`` is the default value for the outer tessellation
>> +levels. This corresponds to GL's ``PATCH_DEFAULT_OUTER_LEVEL``.
>> +  * ``default_inner_level`` is the default value for the inner tessellation
>> +levels. This corresponds to GL's ``PATCH_DEFAULT_INNER_LEVEL``.
>>
>>
>>  Sampler Views
>> diff --git a/src/gallium/include/pipe/p_context.h 
>> b/src/gallium/include/pipe/p_context.h
>> index 74c2f2f..afc5d7b 100644
>> --- a/src/gallium/include/pipe/p_context.h
>> +++ b/src/gallium/include/pipe/p_context.h
>> @@ -231,6 +231,10 @@ struct pipe_context {
>>   unsigned start_slot, unsigned num_views,
>>   struct pipe_sampler_view **);
>>
>> +   void (*set_tess_state)(struct pipe_context *,
>> +  float default_outer_level[4],
>> +  float default_inner_level[2]);
>> +
>> /**
>>  * Bind an array of shader resources that will be used by the
>>  * graphics pipeline.  Any resources that were previously bound to
>>
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] swrast: Fix rgba_draw_pixels with GL_COLOR_INDEX

[Jason Ekstrand]

On May 4, 2015 6:41 AM, "Juha-Pekka Heikkila" 
wrote:
>
> These two patches
>
> Reviewed-and-tested-by: Juha-Pekka Heikkila 

Those should probably be sperate tags.  Also,

Reviewed-by: Jason Ekstrand 

> On 04.05.2015 12:24, Iago Toral Quiroga wrote:
> > When we implemented the format conversion rewrite we forgot to handle
> > GL_COLOR_INDEX here, which needs special handling.
> >
> > Fixes the following piglit test:
> > bin/gl-1.0-drawpixels-color-index -auto -fbo
> >
> > Buzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90213
> > ---
> >  src/mesa/swrast/s_drawpix.c | 29 ++---
> >  1 file changed, 26 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/mesa/swrast/s_drawpix.c b/src/mesa/swrast/s_drawpix.c
> > index bf42726..fb677ee 100644
> > --- a/src/mesa/swrast/s_drawpix.c
> > +++ b/src/mesa/swrast/s_drawpix.c
> > @@ -448,14 +448,34 @@ draw_rgba_pixels( struct gl_context *ctx, GLint
x, GLint y,
> > {
> >const GLbitfield interpMask = span.interpMask;
> >const GLbitfield arrayMask = span.arrayMask;
> > -  const GLint srcStride
> > - = _mesa_image_row_stride(unpack, width, format, type);
> >GLint skipPixels = 0;
> >/* use span array for temp color storage */
> >GLfloat *rgba = (GLfloat *)
span.array->attribs[VARYING_SLOT_COL0];
> >void *tempImage = NULL;
> >
> > -  if (unpack->SwapBytes) {
> > +  /* We have to deal with GL_COLOR_INDEX manually because
> > +   * _mesa_format_convert does not handle this format. So what we
do here is
> > +   * convert it to RGBA ubyte first and then convert from that to
dst as
> > +   * usual.
> > +   */
> > +  if (format == GL_COLOR_INDEX) {
> > + /* This will handle byte swapping and transferops if needed */
> > + tempImage =
> > +_mesa_unpack_color_index_to_rgba_ubyte(ctx, 2,
> > +   pixels, format,
type,
> > +   width, height, 1,
> > +   unpack,
> > +   transferOps);
> > + if (!tempImage) {
> > +_mesa_error(ctx, GL_OUT_OF_MEMORY, "glDrawPixels");
> > +return;
> > + }
> > +
> > + transferOps = 0;
> > + pixels = tempImage;
> > + format = GL_RGBA;
> > + type = GL_UNSIGNED_BYTE;
> > +  } else if (unpack->SwapBytes) {
> >   /* We have to handle byte-swapping scenarios before calling
> >* _mesa_format_convert
> >*/
> > @@ -476,6 +496,9 @@ draw_rgba_pixels( struct gl_context *ctx, GLint x,
GLint y,
> >   }
> >}
> >
> > +  const GLint srcStride
> > + = _mesa_image_row_stride(unpack, width, format, type);
> > +
> >/* if the span is wider than SWRAST_MAX_WIDTH we have to do it
in chunks */
> >while (skipPixels < width) {
> >   const GLint spanWidth = MIN2(width - skipPixels,
SWRAST_MAX_WIDTH);
> >
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] mesa: Always return a value in _mesa_format_from_format_and_type

[Jason Ekstrand]

On May 4, 2015 4:25 AM, "Iago Toral Quiroga"  wrote:
>
> Return MESA_FORMAT_NONE if no matching type was found. We have an
> assertion here, but we should return something anyway to avoid
> confusion with non-debug builds.
> ---
>  src/mesa/main/glformats.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
> index 8ced579..fddd048 100644
> --- a/src/mesa/main/glformats.c
> +++ b/src/mesa/main/glformats.c
> @@ -2752,4 +2752,5 @@ _mesa_format_from_format_and_type(GLenum format,
GLenum type)
>  * format in that case.
>  */
> unreachable("Unsupported format");
> +   return MESA_FORMAT_NONE;

If we want this to have well-defined results, this should not be an
unreachable().  The semantics of unreachable allow the compiler to generate
wrong code for the case where it does get reached.  We could switch it to
an assert or we could just drop this patch. I'd be a fan of the latter.
--Jason

>  }
> --
> 1.9.1
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/7] glsl: Forbid opaque variables as operands of the ternary operator.

[Francisco Jerez]

Matt Turner  writes:

> On Sat, Jan 31, 2015 at 12:54 PM, Francisco Jerez  
> wrote:
>> ---
>>  src/glsl/ast_to_hir.cpp | 12 
>>  1 file changed, 12 insertions(+)
>
> Patches 1-2 were already reviewed and committed.
>
> Patches 3-7 are
>
> Reviewed-by: Matt Turner 

Thanks!


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

Re: [Mesa-dev] [PATCH 6/8] mesa: Implement image uniform queries.

[Francisco Jerez]

Matt Turner  writes:

> On Sat, Jan 31, 2015 at 10:27 AM, Francisco Jerez  
> wrote:
>> ---
>>  src/mesa/main/uniform_query.cpp | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/main/uniform_query.cpp 
>> b/src/mesa/main/uniform_query.cpp
>> index 32870d0..82e5e38 100644
>> --- a/src/mesa/main/uniform_query.cpp
>> +++ b/src/mesa/main/uniform_query.cpp
>> @@ -362,7 +362,8 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint 
>> program, GLint location,
>>   &&
>>   (uni->type->base_type == GLSL_TYPE_INT
>>|| uni->type->base_type == GLSL_TYPE_UINT
>> -  || uni->type->base_type == GLSL_TYPE_SAMPLER))) {
>> +  || uni->type->base_type == GLSL_TYPE_SAMPLER
>> +   || uni->type->base_type == GLSL_TYPE_IMAGE))) {
>>  memcpy(paramsOut, src, bytes);
>>} else {
>>  union gl_constant_value *const dst =
>> @@ -381,6 +382,7 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint 
>> program, GLint location,
>>   break;
>>case GLSL_TYPE_INT:
>>case GLSL_TYPE_SAMPLER:
>> +  case GLSL_TYPE_IMAGE:
>>   dst[i].f = (float) src[i].i;
>>   break;
>>case GLSL_TYPE_BOOL:
>> --
>> 2.1.3
>
> Remove the tabs in the couple of lines you're adding in this patch,
> and this series is
>
> Reviewed-by: Matt Turner 

Thank you Matt.


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

Re: [Mesa-dev] [PATCH 12/13] i965: Add typed surface access opcodes.

[Pohjolainen, Topi]

On Mon, May 04, 2015 at 02:56:33PM +0300, Francisco Jerez wrote:
> "Pohjolainen, Topi"  writes:
> 
> > On Mon, May 04, 2015 at 02:18:37PM +0300, Francisco Jerez wrote:
> >> "Pohjolainen, Topi"  writes:
> >> 
> >> > On Sat, Apr 25, 2015 at 01:45:36AM +0300, Francisco Jerez wrote:
> >> >> "Pohjolainen, Topi"  writes:
> >> >> 
> >> >> > On Fri, Feb 27, 2015 at 05:34:55PM +0200, Francisco Jerez wrote:
> >> >> >> ---
> >> >> >>  src/mesa/drivers/dri/i965/brw_defines.h|   4 +
> >> >> >>  src/mesa/drivers/dri/i965/brw_eu.h |  24 +++
> >> >> >>  src/mesa/drivers/dri/i965/brw_eu_emit.c| 169 
> >> >> >> +
> >> >> >>  src/mesa/drivers/dri/i965/brw_fs.cpp   |  12 ++
> >> >> >>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp |  17 +++
> >> >> >>  .../drivers/dri/i965/brw_schedule_instructions.cpp |   3 +
> >> >> >>  src/mesa/drivers/dri/i965/brw_shader.cpp   |   8 +
> >> >> >>  src/mesa/drivers/dri/i965/brw_vec4.cpp |   6 +
> >> >> >>  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp   |  18 +++
> >> >> >>  9 files changed, 261 insertions(+)
> >> >> >> 
> >> >> >> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h 
> >> >> >> b/src/mesa/drivers/dri/i965/brw_defines.h
> >> >> >> index e56f49c..cf07da9 100644
> >> >> >> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> >> >> >> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> >> >> >> @@ -906,6 +906,10 @@ enum opcode {
> >> >> >> SHADER_OPCODE_UNTYPED_SURFACE_READ,
> >> >> >> SHADER_OPCODE_UNTYPED_SURFACE_WRITE,
> >> >> >>  
> >> >> >> +   SHADER_OPCODE_TYPED_ATOMIC,
> >> >> >> +   SHADER_OPCODE_TYPED_SURFACE_READ,
> >> >> >> +   SHADER_OPCODE_TYPED_SURFACE_WRITE,
> >> >> >> +
> >> >> >> SHADER_OPCODE_GEN4_SCRATCH_READ,
> >> >> >> SHADER_OPCODE_GEN4_SCRATCH_WRITE,
> >> >> >> SHADER_OPCODE_GEN7_SCRATCH_READ,
> >> >> >> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h 
> >> >> >> b/src/mesa/drivers/dri/i965/brw_eu.h
> >> >> >> index cad956b..ce9554b 100644
> >> >> >> --- a/src/mesa/drivers/dri/i965/brw_eu.h
> >> >> >> +++ b/src/mesa/drivers/dri/i965/brw_eu.h
> >> >> >> @@ -421,6 +421,30 @@ brw_untyped_surface_write(struct brw_compile *p,
> >> >> >>unsigned num_channels);
> >> >> >>  
> >> >> >>  void
> >> >> >> +brw_typed_atomic(struct brw_compile *p,
> >> >> >> + struct brw_reg dst,
> >> >> >> + struct brw_reg payload,
> >> >> >> + struct brw_reg surface,
> >> >> >> + unsigned atomic_op,
> >> >> >> + unsigned msg_length,
> >> >> >> + bool response_expected);
> >> >> >> +
> >> >> >> +void
> >> >> >> +brw_typed_surface_read(struct brw_compile *p,
> >> >> >> +   struct brw_reg dst,
> >> >> >> +   struct brw_reg payload,
> >> >> >> +   struct brw_reg surface,
> >> >> >> +   unsigned msg_length,
> >> >> >> +   unsigned num_channels);
> >> >> >> +
> >> >> >> +void
> >> >> >> +brw_typed_surface_write(struct brw_compile *p,
> >> >> >> +struct brw_reg payload,
> >> >> >> +struct brw_reg surface,
> >> >> >> +unsigned msg_length,
> >> >> >> +unsigned num_channels);
> >> >> >> +
> >> >> >> +void
> >> >> >>  brw_pixel_interpolator_query(struct brw_compile *p,
> >> >> >>   struct brw_reg dest,
> >> >> >>   struct brw_reg mrf,
> >> >> >> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c 
> >> >> >> b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> >> >> >> index f5b8fa9..74f1fc1 100644
> >> >> >> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> >> >> >> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> >> >> >> @@ -2944,6 +2944,175 @@ brw_untyped_surface_write(struct brw_compile 
> >> >> >> *p,
> >> >> >>p, insn, num_channels);
> >> >> >>  }
> >> >> >>  
> >> >> >> +static void
> >> >> >> +brw_set_dp_typed_atomic_message(struct brw_compile *p,
> >> >> >> +struct brw_inst *insn,
> >> >> >> +unsigned atomic_op,
> >> >> >> +bool response_expected)
> >> >> >> +{
> >> >> >> +   const struct brw_context *brw = p->brw;
> >> >> >> +   unsigned msg_control =
> >> >> >> +  atomic_op | /* Atomic Operation Type: BRW_AOP_* */
> >> >> >> +  (response_expected ? 1 << 5 : 0); /* Return data expected */
> >> >> >> +
> >> >> >> +   if (brw->gen >= 8 || brw->is_haswell) {
> >> >> >> +  if (brw_inst_access_mode(brw, p->current) == BRW_ALIGN_1) {
> >> >> >> + if (brw_inst_qtr_control(brw, p->current) == 
> >> >> >> GEN6_COMPRESSION_2Q)
> >> >> >> +msg_control |= 1 << 4; /* Use high 8 slots of the 
> >> >> >> sample mask */
> >> >> >> +
> >> >> >> + brw_inst_set_dp_msg_type(brw, insn,
> >> >>

Re: [Mesa-dev] [PATCH 12/13] i965: Add typed surface access opcodes.

[Francisco Jerez]

"Pohjolainen, Topi"  writes:

> On Mon, May 04, 2015 at 02:18:37PM +0300, Francisco Jerez wrote:
>> "Pohjolainen, Topi"  writes:
>> 
>> > On Sat, Apr 25, 2015 at 01:45:36AM +0300, Francisco Jerez wrote:
>> >> "Pohjolainen, Topi"  writes:
>> >> 
>> >> > On Fri, Feb 27, 2015 at 05:34:55PM +0200, Francisco Jerez wrote:
>> >> >> ---
>> >> >>  src/mesa/drivers/dri/i965/brw_defines.h|   4 +
>> >> >>  src/mesa/drivers/dri/i965/brw_eu.h |  24 +++
>> >> >>  src/mesa/drivers/dri/i965/brw_eu_emit.c| 169 
>> >> >> +
>> >> >>  src/mesa/drivers/dri/i965/brw_fs.cpp   |  12 ++
>> >> >>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp |  17 +++
>> >> >>  .../drivers/dri/i965/brw_schedule_instructions.cpp |   3 +
>> >> >>  src/mesa/drivers/dri/i965/brw_shader.cpp   |   8 +
>> >> >>  src/mesa/drivers/dri/i965/brw_vec4.cpp |   6 +
>> >> >>  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp   |  18 +++
>> >> >>  9 files changed, 261 insertions(+)
>> >> >> 
>> >> >> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h 
>> >> >> b/src/mesa/drivers/dri/i965/brw_defines.h
>> >> >> index e56f49c..cf07da9 100644
>> >> >> --- a/src/mesa/drivers/dri/i965/brw_defines.h
>> >> >> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
>> >> >> @@ -906,6 +906,10 @@ enum opcode {
>> >> >> SHADER_OPCODE_UNTYPED_SURFACE_READ,
>> >> >> SHADER_OPCODE_UNTYPED_SURFACE_WRITE,
>> >> >>  
>> >> >> +   SHADER_OPCODE_TYPED_ATOMIC,
>> >> >> +   SHADER_OPCODE_TYPED_SURFACE_READ,
>> >> >> +   SHADER_OPCODE_TYPED_SURFACE_WRITE,
>> >> >> +
>> >> >> SHADER_OPCODE_GEN4_SCRATCH_READ,
>> >> >> SHADER_OPCODE_GEN4_SCRATCH_WRITE,
>> >> >> SHADER_OPCODE_GEN7_SCRATCH_READ,
>> >> >> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h 
>> >> >> b/src/mesa/drivers/dri/i965/brw_eu.h
>> >> >> index cad956b..ce9554b 100644
>> >> >> --- a/src/mesa/drivers/dri/i965/brw_eu.h
>> >> >> +++ b/src/mesa/drivers/dri/i965/brw_eu.h
>> >> >> @@ -421,6 +421,30 @@ brw_untyped_surface_write(struct brw_compile *p,
>> >> >>unsigned num_channels);
>> >> >>  
>> >> >>  void
>> >> >> +brw_typed_atomic(struct brw_compile *p,
>> >> >> + struct brw_reg dst,
>> >> >> + struct brw_reg payload,
>> >> >> + struct brw_reg surface,
>> >> >> + unsigned atomic_op,
>> >> >> + unsigned msg_length,
>> >> >> + bool response_expected);
>> >> >> +
>> >> >> +void
>> >> >> +brw_typed_surface_read(struct brw_compile *p,
>> >> >> +   struct brw_reg dst,
>> >> >> +   struct brw_reg payload,
>> >> >> +   struct brw_reg surface,
>> >> >> +   unsigned msg_length,
>> >> >> +   unsigned num_channels);
>> >> >> +
>> >> >> +void
>> >> >> +brw_typed_surface_write(struct brw_compile *p,
>> >> >> +struct brw_reg payload,
>> >> >> +struct brw_reg surface,
>> >> >> +unsigned msg_length,
>> >> >> +unsigned num_channels);
>> >> >> +
>> >> >> +void
>> >> >>  brw_pixel_interpolator_query(struct brw_compile *p,
>> >> >>   struct brw_reg dest,
>> >> >>   struct brw_reg mrf,
>> >> >> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c 
>> >> >> b/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> >> >> index f5b8fa9..74f1fc1 100644
>> >> >> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> >> >> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> >> >> @@ -2944,6 +2944,175 @@ brw_untyped_surface_write(struct brw_compile 
>> >> >> *p,
>> >> >>p, insn, num_channels);
>> >> >>  }
>> >> >>  
>> >> >> +static void
>> >> >> +brw_set_dp_typed_atomic_message(struct brw_compile *p,
>> >> >> +struct brw_inst *insn,
>> >> >> +unsigned atomic_op,
>> >> >> +bool response_expected)
>> >> >> +{
>> >> >> +   const struct brw_context *brw = p->brw;
>> >> >> +   unsigned msg_control =
>> >> >> +  atomic_op | /* Atomic Operation Type: BRW_AOP_* */
>> >> >> +  (response_expected ? 1 << 5 : 0); /* Return data expected */
>> >> >> +
>> >> >> +   if (brw->gen >= 8 || brw->is_haswell) {
>> >> >> +  if (brw_inst_access_mode(brw, p->current) == BRW_ALIGN_1) {
>> >> >> + if (brw_inst_qtr_control(brw, p->current) == 
>> >> >> GEN6_COMPRESSION_2Q)
>> >> >> +msg_control |= 1 << 4; /* Use high 8 slots of the sample 
>> >> >> mask */
>> >> >> +
>> >> >> + brw_inst_set_dp_msg_type(brw, insn,
>> >> >> +  
>> >> >> HSW_DATAPORT_DC_PORT1_TYPED_ATOMIC_OP);
>> >> >> +  } else {
>> >> >> + brw_inst_set_dp_msg_type(brw, insn,
>> >> >> +  
>> >> >> HSW_DATAPORT_DC_PORT1_TYPED_ATOMIC_OP_SIMD4X2);
>>

Re: [Mesa-dev] [PATCH 12/13] i965: Add typed surface access opcodes.

[Pohjolainen, Topi]

On Mon, May 04, 2015 at 02:47:33PM +0300, Pohjolainen, Topi wrote:
> On Mon, May 04, 2015 at 02:18:37PM +0300, Francisco Jerez wrote:
> > "Pohjolainen, Topi"  writes:
> > 
> > > On Sat, Apr 25, 2015 at 01:45:36AM +0300, Francisco Jerez wrote:
> > >> "Pohjolainen, Topi"  writes:
> > >> 
> > >> > On Fri, Feb 27, 2015 at 05:34:55PM +0200, Francisco Jerez wrote:
> > >> >> ---
> > >> >>  src/mesa/drivers/dri/i965/brw_defines.h|   4 +
> > >> >>  src/mesa/drivers/dri/i965/brw_eu.h |  24 +++
> > >> >>  src/mesa/drivers/dri/i965/brw_eu_emit.c| 169 
> > >> >> +
> > >> >>  src/mesa/drivers/dri/i965/brw_fs.cpp   |  12 ++
> > >> >>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp |  17 +++
> > >> >>  .../drivers/dri/i965/brw_schedule_instructions.cpp |   3 +
> > >> >>  src/mesa/drivers/dri/i965/brw_shader.cpp   |   8 +
> > >> >>  src/mesa/drivers/dri/i965/brw_vec4.cpp |   6 +
> > >> >>  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp   |  18 +++
> > >> >>  9 files changed, 261 insertions(+)
> > >> >> 
> > >> >> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h 
> > >> >> b/src/mesa/drivers/dri/i965/brw_defines.h
> > >> >> index e56f49c..cf07da9 100644
> > >> >> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> > >> >> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> > >> >> @@ -906,6 +906,10 @@ enum opcode {
> > >> >> SHADER_OPCODE_UNTYPED_SURFACE_READ,
> > >> >> SHADER_OPCODE_UNTYPED_SURFACE_WRITE,
> > >> >>  
> > >> >> +   SHADER_OPCODE_TYPED_ATOMIC,
> > >> >> +   SHADER_OPCODE_TYPED_SURFACE_READ,
> > >> >> +   SHADER_OPCODE_TYPED_SURFACE_WRITE,
> > >> >> +
> > >> >> SHADER_OPCODE_GEN4_SCRATCH_READ,
> > >> >> SHADER_OPCODE_GEN4_SCRATCH_WRITE,
> > >> >> SHADER_OPCODE_GEN7_SCRATCH_READ,
> > >> >> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h 
> > >> >> b/src/mesa/drivers/dri/i965/brw_eu.h
> > >> >> index cad956b..ce9554b 100644
> > >> >> --- a/src/mesa/drivers/dri/i965/brw_eu.h
> > >> >> +++ b/src/mesa/drivers/dri/i965/brw_eu.h
> > >> >> @@ -421,6 +421,30 @@ brw_untyped_surface_write(struct brw_compile *p,
> > >> >>unsigned num_channels);
> > >> >>  
> > >> >>  void
> > >> >> +brw_typed_atomic(struct brw_compile *p,
> > >> >> + struct brw_reg dst,
> > >> >> + struct brw_reg payload,
> > >> >> + struct brw_reg surface,
> > >> >> + unsigned atomic_op,
> > >> >> + unsigned msg_length,
> > >> >> + bool response_expected);
> > >> >> +
> > >> >> +void
> > >> >> +brw_typed_surface_read(struct brw_compile *p,
> > >> >> +   struct brw_reg dst,
> > >> >> +   struct brw_reg payload,
> > >> >> +   struct brw_reg surface,
> > >> >> +   unsigned msg_length,
> > >> >> +   unsigned num_channels);
> > >> >> +
> > >> >> +void
> > >> >> +brw_typed_surface_write(struct brw_compile *p,
> > >> >> +struct brw_reg payload,
> > >> >> +struct brw_reg surface,
> > >> >> +unsigned msg_length,
> > >> >> +unsigned num_channels);
> > >> >> +
> > >> >> +void
> > >> >>  brw_pixel_interpolator_query(struct brw_compile *p,
> > >> >>   struct brw_reg dest,
> > >> >>   struct brw_reg mrf,
> > >> >> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c 
> > >> >> b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> > >> >> index f5b8fa9..74f1fc1 100644
> > >> >> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> > >> >> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> > >> >> @@ -2944,6 +2944,175 @@ brw_untyped_surface_write(struct brw_compile 
> > >> >> *p,
> > >> >>p, insn, num_channels);
> > >> >>  }
> > >> >>  
> > >> >> +static void
> > >> >> +brw_set_dp_typed_atomic_message(struct brw_compile *p,
> > >> >> +struct brw_inst *insn,
> > >> >> +unsigned atomic_op,
> > >> >> +bool response_expected)
> > >> >> +{
> > >> >> +   const struct brw_context *brw = p->brw;
> > >> >> +   unsigned msg_control =
> > >> >> +  atomic_op | /* Atomic Operation Type: BRW_AOP_* */
> > >> >> +  (response_expected ? 1 << 5 : 0); /* Return data expected */
> > >> >> +
> > >> >> +   if (brw->gen >= 8 || brw->is_haswell) {
> > >> >> +  if (brw_inst_access_mode(brw, p->current) == BRW_ALIGN_1) {
> > >> >> + if (brw_inst_qtr_control(brw, p->current) == 
> > >> >> GEN6_COMPRESSION_2Q)
> > >> >> +msg_control |= 1 << 4; /* Use high 8 slots of the sample 
> > >> >> mask */
> > >> >> +
> > >> >> + brw_inst_set_dp_msg_type(brw, insn,
> > >> >> +  
> > >> >> HSW_DATAPORT_DC_PORT1_TYPED_ATOMIC_OP);
> > >> >> +  } else {
> > >> >> +

Re: [Mesa-dev] [PATCH 12/13] i965: Add typed surface access opcodes.

[Pohjolainen, Topi]

On Mon, May 04, 2015 at 02:18:37PM +0300, Francisco Jerez wrote:
> "Pohjolainen, Topi"  writes:
> 
> > On Sat, Apr 25, 2015 at 01:45:36AM +0300, Francisco Jerez wrote:
> >> "Pohjolainen, Topi"  writes:
> >> 
> >> > On Fri, Feb 27, 2015 at 05:34:55PM +0200, Francisco Jerez wrote:
> >> >> ---
> >> >>  src/mesa/drivers/dri/i965/brw_defines.h|   4 +
> >> >>  src/mesa/drivers/dri/i965/brw_eu.h |  24 +++
> >> >>  src/mesa/drivers/dri/i965/brw_eu_emit.c| 169 
> >> >> +
> >> >>  src/mesa/drivers/dri/i965/brw_fs.cpp   |  12 ++
> >> >>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp |  17 +++
> >> >>  .../drivers/dri/i965/brw_schedule_instructions.cpp |   3 +
> >> >>  src/mesa/drivers/dri/i965/brw_shader.cpp   |   8 +
> >> >>  src/mesa/drivers/dri/i965/brw_vec4.cpp |   6 +
> >> >>  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp   |  18 +++
> >> >>  9 files changed, 261 insertions(+)
> >> >> 
> >> >> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h 
> >> >> b/src/mesa/drivers/dri/i965/brw_defines.h
> >> >> index e56f49c..cf07da9 100644
> >> >> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> >> >> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> >> >> @@ -906,6 +906,10 @@ enum opcode {
> >> >> SHADER_OPCODE_UNTYPED_SURFACE_READ,
> >> >> SHADER_OPCODE_UNTYPED_SURFACE_WRITE,
> >> >>  
> >> >> +   SHADER_OPCODE_TYPED_ATOMIC,
> >> >> +   SHADER_OPCODE_TYPED_SURFACE_READ,
> >> >> +   SHADER_OPCODE_TYPED_SURFACE_WRITE,
> >> >> +
> >> >> SHADER_OPCODE_GEN4_SCRATCH_READ,
> >> >> SHADER_OPCODE_GEN4_SCRATCH_WRITE,
> >> >> SHADER_OPCODE_GEN7_SCRATCH_READ,
> >> >> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h 
> >> >> b/src/mesa/drivers/dri/i965/brw_eu.h
> >> >> index cad956b..ce9554b 100644
> >> >> --- a/src/mesa/drivers/dri/i965/brw_eu.h
> >> >> +++ b/src/mesa/drivers/dri/i965/brw_eu.h
> >> >> @@ -421,6 +421,30 @@ brw_untyped_surface_write(struct brw_compile *p,
> >> >>unsigned num_channels);
> >> >>  
> >> >>  void
> >> >> +brw_typed_atomic(struct brw_compile *p,
> >> >> + struct brw_reg dst,
> >> >> + struct brw_reg payload,
> >> >> + struct brw_reg surface,
> >> >> + unsigned atomic_op,
> >> >> + unsigned msg_length,
> >> >> + bool response_expected);
> >> >> +
> >> >> +void
> >> >> +brw_typed_surface_read(struct brw_compile *p,
> >> >> +   struct brw_reg dst,
> >> >> +   struct brw_reg payload,
> >> >> +   struct brw_reg surface,
> >> >> +   unsigned msg_length,
> >> >> +   unsigned num_channels);
> >> >> +
> >> >> +void
> >> >> +brw_typed_surface_write(struct brw_compile *p,
> >> >> +struct brw_reg payload,
> >> >> +struct brw_reg surface,
> >> >> +unsigned msg_length,
> >> >> +unsigned num_channels);
> >> >> +
> >> >> +void
> >> >>  brw_pixel_interpolator_query(struct brw_compile *p,
> >> >>   struct brw_reg dest,
> >> >>   struct brw_reg mrf,
> >> >> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c 
> >> >> b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> >> >> index f5b8fa9..74f1fc1 100644
> >> >> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> >> >> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> >> >> @@ -2944,6 +2944,175 @@ brw_untyped_surface_write(struct brw_compile *p,
> >> >>p, insn, num_channels);
> >> >>  }
> >> >>  
> >> >> +static void
> >> >> +brw_set_dp_typed_atomic_message(struct brw_compile *p,
> >> >> +struct brw_inst *insn,
> >> >> +unsigned atomic_op,
> >> >> +bool response_expected)
> >> >> +{
> >> >> +   const struct brw_context *brw = p->brw;
> >> >> +   unsigned msg_control =
> >> >> +  atomic_op | /* Atomic Operation Type: BRW_AOP_* */
> >> >> +  (response_expected ? 1 << 5 : 0); /* Return data expected */
> >> >> +
> >> >> +   if (brw->gen >= 8 || brw->is_haswell) {
> >> >> +  if (brw_inst_access_mode(brw, p->current) == BRW_ALIGN_1) {
> >> >> + if (brw_inst_qtr_control(brw, p->current) == 
> >> >> GEN6_COMPRESSION_2Q)
> >> >> +msg_control |= 1 << 4; /* Use high 8 slots of the sample 
> >> >> mask */
> >> >> +
> >> >> + brw_inst_set_dp_msg_type(brw, insn,
> >> >> +  
> >> >> HSW_DATAPORT_DC_PORT1_TYPED_ATOMIC_OP);
> >> >> +  } else {
> >> >> + brw_inst_set_dp_msg_type(brw, insn,
> >> >> +  
> >> >> HSW_DATAPORT_DC_PORT1_TYPED_ATOMIC_OP_SIMD4X2);
> >> >> +  }
> >> >> +
> >> >> +   } else {
> >> >> +  brw_inst_set_dp_msg_type(brw, insn,
> >> >> +   GEN7_DATAP

Re: [Mesa-dev] [PATCH 1/2] swrast: Fix rgba_draw_pixels with GL_COLOR_INDEX

[Juha-Pekka Heikkila]

These two patches

Reviewed-and-tested-by: Juha-Pekka Heikkila 

On 04.05.2015 12:24, Iago Toral Quiroga wrote:
> When we implemented the format conversion rewrite we forgot to handle
> GL_COLOR_INDEX here, which needs special handling.
> 
> Fixes the following piglit test:
> bin/gl-1.0-drawpixels-color-index -auto -fbo
> 
> Buzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90213
> ---
>  src/mesa/swrast/s_drawpix.c | 29 ++---
>  1 file changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/src/mesa/swrast/s_drawpix.c b/src/mesa/swrast/s_drawpix.c
> index bf42726..fb677ee 100644
> --- a/src/mesa/swrast/s_drawpix.c
> +++ b/src/mesa/swrast/s_drawpix.c
> @@ -448,14 +448,34 @@ draw_rgba_pixels( struct gl_context *ctx, GLint x, 
> GLint y,
> {
>const GLbitfield interpMask = span.interpMask;
>const GLbitfield arrayMask = span.arrayMask;
> -  const GLint srcStride
> - = _mesa_image_row_stride(unpack, width, format, type);
>GLint skipPixels = 0;
>/* use span array for temp color storage */
>GLfloat *rgba = (GLfloat *) span.array->attribs[VARYING_SLOT_COL0];
>void *tempImage = NULL;
>  
> -  if (unpack->SwapBytes) {
> +  /* We have to deal with GL_COLOR_INDEX manually because
> +   * _mesa_format_convert does not handle this format. So what we do 
> here is
> +   * convert it to RGBA ubyte first and then convert from that to dst as
> +   * usual.
> +   */
> +  if (format == GL_COLOR_INDEX) {
> + /* This will handle byte swapping and transferops if needed */
> + tempImage =
> +_mesa_unpack_color_index_to_rgba_ubyte(ctx, 2,
> +   pixels, format, type,
> +   width, height, 1,
> +   unpack,
> +   transferOps);
> + if (!tempImage) {
> +_mesa_error(ctx, GL_OUT_OF_MEMORY, "glDrawPixels");
> +return;
> + }
> +
> + transferOps = 0;
> + pixels = tempImage;
> + format = GL_RGBA;
> + type = GL_UNSIGNED_BYTE;
> +  } else if (unpack->SwapBytes) {
>   /* We have to handle byte-swapping scenarios before calling
>* _mesa_format_convert
>*/
> @@ -476,6 +496,9 @@ draw_rgba_pixels( struct gl_context *ctx, GLint x, GLint 
> y,
>   }
>}
>  
> +  const GLint srcStride
> + = _mesa_image_row_stride(unpack, width, format, type);
> +
>/* if the span is wider than SWRAST_MAX_WIDTH we have to do it in 
> chunks */
>while (skipPixels < width) {
>   const GLint spanWidth = MIN2(width - skipPixels, SWRAST_MAX_WIDTH);
> 

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

Re: [Mesa-dev] [PATCH 08/19] gallium: add set_tess_state to configure default tessellation parameters

[Roland Scheidegger]

Seems a bit awkward that you have to set such state separately. I don't
know much about how this is supposed to work though, some quick grep
says these parameters are used when there's no tessellation control
shader, so I guess that's why it is separate state.
How does that actually work in hw, can you just switch tesselation
control shaders off or do you have to provide some kind of a default
tesselation ctrl shader?

Roland

Am 02.05.2015 um 22:16 schrieb Ilia Mirkin:
> Signed-off-by: Ilia Mirkin 
> ---
>  src/gallium/docs/source/context.rst  | 5 +
>  src/gallium/include/pipe/p_context.h | 4 
>  2 files changed, 9 insertions(+)
> 
> diff --git a/src/gallium/docs/source/context.rst 
> b/src/gallium/docs/source/context.rst
> index 5861f46..0908ee7 100644
> --- a/src/gallium/docs/source/context.rst
> +++ b/src/gallium/docs/source/context.rst
> @@ -79,6 +79,11 @@ objects. They all follow simple, one-method binding calls, 
> e.g.
>should be the same as the number of set viewports and can be up to
>PIPE_MAX_VIEWPORTS.
>  * ``set_viewport_states``
> +* ``set_tess_state`` configures the default tessellation parameters:
> +  * ``default_outer_level`` is the default value for the outer tessellation
> +levels. This corresponds to GL's ``PATCH_DEFAULT_OUTER_LEVEL``.
> +  * ``default_inner_level`` is the default value for the inner tessellation
> +levels. This corresponds to GL's ``PATCH_DEFAULT_INNER_LEVEL``.
>  
>  
>  Sampler Views
> diff --git a/src/gallium/include/pipe/p_context.h 
> b/src/gallium/include/pipe/p_context.h
> index 74c2f2f..afc5d7b 100644
> --- a/src/gallium/include/pipe/p_context.h
> +++ b/src/gallium/include/pipe/p_context.h
> @@ -231,6 +231,10 @@ struct pipe_context {
>   unsigned start_slot, unsigned num_views,
>   struct pipe_sampler_view **);
>  
> +   void (*set_tess_state)(struct pipe_context *,
> +  float default_outer_level[4],
> +  float default_inner_level[2]);
> +
> /**
>  * Bind an array of shader resources that will be used by the
>  * graphics pipeline.  Any resources that were previously bound to
> 

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

Re: [Mesa-dev] [PATCH 00/19] gallium: basic tessellation support

[Roland Scheidegger]

Hmm someting like TESSEVAL is fine by me but I'd prefer something a bit
more descriptive than TESSC/TESSE...

Roland

Am 03.05.2015 um 17:59 schrieb Marek Olšák:
> Renaming everything would be slightly annoying. I personally prefer
> shorter names anyway (TESSC/TESSE, PIPE_SHADER_TESSEVAL, etc.). I
> don't think longer names would make any difference in readability.
> 
> Marek
> 
> On Sun, May 3, 2015 at 5:06 PM, Ilia Mirkin  wrote:
>> On Sun, May 3, 2015 at 6:18 AM, Glenn Kennard  
>> wrote:
>>> On Sat, 02 May 2015 22:16:24 +0200, Ilia Mirkin 
>>> wrote:
>>>
 This series adds tokens and updates some helper gallium functions to
 know about tessellation. This provides no actual support for
 tessellation in either core or drivers, however this will make it
 possible to work on the core and driver pieces without crazy
 interdependencies, as well as be landed separately and without
 (direct) dependency.

 Most of these patches have existed for about a year already, and have
 been part of my and Marek's trees enabling tessellation in the nvc0
 and radeonsi drivers. I've taken this opportunity to fix up and fold
 some of them though.

 This should be pretty safe to land, since even if I messed something
 up, having this in-tree will make it easier for others to identify and
 fix any issues collaboratively.

 Ilia Mirkin (11):
   gallium: add tessellation shader types
   gallium: add new PATCHES primitive type
   gallium: add new semantics for tessellation
   gallium: add interfaces for controlling tess program state
   gallium: add tessellation shader properties
   gallium: add patch_vertices to draw info
   gallium: add set_tess_state to configure default tessellation
 parameters
   tgsi/scan: allow scanning tessellation shaders
   tgsi/sanity: set implicit in/out array sizes based on patch sizes
   tgsi/ureg: allow ureg_dst to have dimension indices
   tgsi/dump: fix declaration printing of tessellation inputs/outputs

 Marek Olšák (8):
   gallium: bump shader input and output limits
   trace: implement new tessellation functions
   gallium/util: print patch_vertices in util_dump_draw_info
   gallium/u_blitter: disable tessellation for all operations
   gallium/cso: add support for tessellation shaders
   gallium/cso: set NULL shaders at context destruction
   gallium: disable tessellation shaders for meta ops
   tgsi/ureg: use correct limit for max input count

  src/gallium/auxiliary/cso_cache/cso_context.c | 100
 ++
  src/gallium/auxiliary/cso_cache/cso_context.h |  12 
  src/gallium/auxiliary/hud/hud_context.c   |   6 ++
  src/gallium/auxiliary/postprocess/pp_run.c|   6 ++
  src/gallium/auxiliary/tgsi/tgsi_dump.c|  20 +-
  src/gallium/auxiliary/tgsi/tgsi_info.c|   4 ++
  src/gallium/auxiliary/tgsi/tgsi_sanity.c  |  36 --
  src/gallium/auxiliary/tgsi/tgsi_scan.c|   6 +-
  src/gallium/auxiliary/tgsi/tgsi_strings.c |  19 -
  src/gallium/auxiliary/tgsi/tgsi_strings.h |   2 +-
  src/gallium/auxiliary/tgsi/tgsi_ureg.c|  26 ++-
  src/gallium/auxiliary/tgsi/tgsi_ureg.h|  59 +--
  src/gallium/auxiliary/util/u_blit.c   |   6 ++
  src/gallium/auxiliary/util/u_blitter.c|  27 +++
  src/gallium/auxiliary/util/u_blitter.h|  16 -
  src/gallium/auxiliary/util/u_dump_state.c |   2 +
  src/gallium/docs/source/context.rst   |   5 ++
  src/gallium/docs/source/tgsi.rst  |  70 ++
  src/gallium/drivers/trace/tr_context.c|  26 +++
  src/gallium/drivers/trace/tr_dump_state.c |   2 +
  src/gallium/include/pipe/p_context.h  |  14 
  src/gallium/include/pipe/p_defines.h  |  16 -
  src/gallium/include/pipe/p_shader_tokens.h|  18 -
  src/gallium/include/pipe/p_state.h|   6 +-
  src/mesa/state_tracker/st_cb_bitmap.c |   8 ++-
  src/mesa/state_tracker/st_cb_clear.c  |   6 ++
  src/mesa/state_tracker/st_cb_drawpixels.c |   8 ++-
  src/mesa/state_tracker/st_cb_drawtex.c|   6 ++
  28 files changed, 501 insertions(+), 31 deletions(-)

>>>
>>> Some minor nits for patches 1, 6 and 7, see separate mails
>>>
>>> Patches 2-5, 8-19 are
>>> Reviewed-by: Glenn Kennard 
>>
>> Thanks for taking a look! It's easy enough to make the naming changes
>> you proposed (just sed -i all the patches, and git am them), but I'd
>> rather not do so repeatedly. Marek, thoughts on Glenn's naming
>> suggestions?
>>
>>   -ilia
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.freedesktop.org_mailm

Re: [Mesa-dev] [PATCH 12/13] i965: Add typed surface access opcodes.

[Francisco Jerez]

"Pohjolainen, Topi"  writes:

> Unfortunately mail server has purged the original patch number 13. I took
> a look instead in your recent image-load-store-nir branch.

You probably meant the image-load-store-scalar branch?

> I studied the fence mechanism a little in bspec, and based on that I
> can't see anything obvious amiss. Small nit below but:
>
> Reviewed-by: Topi Pohjolainen 
>
> Sorry for dropping the ball for such a long time.
>
> +void
> +brw_memory_fence(struct brw_codegen *p,
> + struct brw_reg dst)
> +{
> +   const struct brw_device_info *devinfo = p->devinfo;
> +   const bool commit_enable = (devinfo->gen == 7 && !devinfo->is_haswell);
>
> You can drop the extra ().

Thank you Topi, I've fixed that locally.


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

Re: [Mesa-dev] [PATCH 12/13] i965: Add typed surface access opcodes.

[Francisco Jerez]

"Pohjolainen, Topi"  writes:

> On Sat, Apr 25, 2015 at 01:45:36AM +0300, Francisco Jerez wrote:
>> "Pohjolainen, Topi"  writes:
>> 
>> > On Fri, Feb 27, 2015 at 05:34:55PM +0200, Francisco Jerez wrote:
>> >> ---
>> >>  src/mesa/drivers/dri/i965/brw_defines.h|   4 +
>> >>  src/mesa/drivers/dri/i965/brw_eu.h |  24 +++
>> >>  src/mesa/drivers/dri/i965/brw_eu_emit.c| 169 
>> >> +
>> >>  src/mesa/drivers/dri/i965/brw_fs.cpp   |  12 ++
>> >>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp |  17 +++
>> >>  .../drivers/dri/i965/brw_schedule_instructions.cpp |   3 +
>> >>  src/mesa/drivers/dri/i965/brw_shader.cpp   |   8 +
>> >>  src/mesa/drivers/dri/i965/brw_vec4.cpp |   6 +
>> >>  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp   |  18 +++
>> >>  9 files changed, 261 insertions(+)
>> >> 
>> >> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h 
>> >> b/src/mesa/drivers/dri/i965/brw_defines.h
>> >> index e56f49c..cf07da9 100644
>> >> --- a/src/mesa/drivers/dri/i965/brw_defines.h
>> >> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
>> >> @@ -906,6 +906,10 @@ enum opcode {
>> >> SHADER_OPCODE_UNTYPED_SURFACE_READ,
>> >> SHADER_OPCODE_UNTYPED_SURFACE_WRITE,
>> >>  
>> >> +   SHADER_OPCODE_TYPED_ATOMIC,
>> >> +   SHADER_OPCODE_TYPED_SURFACE_READ,
>> >> +   SHADER_OPCODE_TYPED_SURFACE_WRITE,
>> >> +
>> >> SHADER_OPCODE_GEN4_SCRATCH_READ,
>> >> SHADER_OPCODE_GEN4_SCRATCH_WRITE,
>> >> SHADER_OPCODE_GEN7_SCRATCH_READ,
>> >> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h 
>> >> b/src/mesa/drivers/dri/i965/brw_eu.h
>> >> index cad956b..ce9554b 100644
>> >> --- a/src/mesa/drivers/dri/i965/brw_eu.h
>> >> +++ b/src/mesa/drivers/dri/i965/brw_eu.h
>> >> @@ -421,6 +421,30 @@ brw_untyped_surface_write(struct brw_compile *p,
>> >>unsigned num_channels);
>> >>  
>> >>  void
>> >> +brw_typed_atomic(struct brw_compile *p,
>> >> + struct brw_reg dst,
>> >> + struct brw_reg payload,
>> >> + struct brw_reg surface,
>> >> + unsigned atomic_op,
>> >> + unsigned msg_length,
>> >> + bool response_expected);
>> >> +
>> >> +void
>> >> +brw_typed_surface_read(struct brw_compile *p,
>> >> +   struct brw_reg dst,
>> >> +   struct brw_reg payload,
>> >> +   struct brw_reg surface,
>> >> +   unsigned msg_length,
>> >> +   unsigned num_channels);
>> >> +
>> >> +void
>> >> +brw_typed_surface_write(struct brw_compile *p,
>> >> +struct brw_reg payload,
>> >> +struct brw_reg surface,
>> >> +unsigned msg_length,
>> >> +unsigned num_channels);
>> >> +
>> >> +void
>> >>  brw_pixel_interpolator_query(struct brw_compile *p,
>> >>   struct brw_reg dest,
>> >>   struct brw_reg mrf,
>> >> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c 
>> >> b/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> >> index f5b8fa9..74f1fc1 100644
>> >> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> >> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> >> @@ -2944,6 +2944,175 @@ brw_untyped_surface_write(struct brw_compile *p,
>> >>p, insn, num_channels);
>> >>  }
>> >>  
>> >> +static void
>> >> +brw_set_dp_typed_atomic_message(struct brw_compile *p,
>> >> +struct brw_inst *insn,
>> >> +unsigned atomic_op,
>> >> +bool response_expected)
>> >> +{
>> >> +   const struct brw_context *brw = p->brw;
>> >> +   unsigned msg_control =
>> >> +  atomic_op | /* Atomic Operation Type: BRW_AOP_* */
>> >> +  (response_expected ? 1 << 5 : 0); /* Return data expected */
>> >> +
>> >> +   if (brw->gen >= 8 || brw->is_haswell) {
>> >> +  if (brw_inst_access_mode(brw, p->current) == BRW_ALIGN_1) {
>> >> + if (brw_inst_qtr_control(brw, p->current) == 
>> >> GEN6_COMPRESSION_2Q)
>> >> +msg_control |= 1 << 4; /* Use high 8 slots of the sample 
>> >> mask */
>> >> +
>> >> + brw_inst_set_dp_msg_type(brw, insn,
>> >> +  HSW_DATAPORT_DC_PORT1_TYPED_ATOMIC_OP);
>> >> +  } else {
>> >> + brw_inst_set_dp_msg_type(brw, insn,
>> >> +  
>> >> HSW_DATAPORT_DC_PORT1_TYPED_ATOMIC_OP_SIMD4X2);
>> >> +  }
>> >> +
>> >> +   } else {
>> >> +  brw_inst_set_dp_msg_type(brw, insn,
>> >> +   GEN7_DATAPORT_RC_TYPED_ATOMIC_OP);
>> >> +
>> >> +  if (brw_inst_qtr_control(brw, p->current) == GEN6_COMPRESSION_2Q)
>> >> + msg_control |= 1 << 4; /* Use high 8 slots of the sample mask */
>> >> +   }
>> >> +
>> >> +   brw_inst_set_dp_msg_control(brw, insn, msg_control);
>> >> +}
>> >> +

Re: [Mesa-dev] [PATCH] glsl: mark special built-in inputs referenced by vertex stage

[Martin Peres]

On 30/04/15 09:27, Tapani Pälli wrote:

Refactoring done on active attribute queries did not take in to
account special built-in inputs for the vertex stage. This commit
sets them referenced by vertex stage so that they get enumerated
properly.

Fixes Piglit test 'get-active-attrib-returns-all-inputs' failure.

Signed-off-by: Tapani Pälli 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90243
---
  src/glsl/linker.cpp | 7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 21fde94..6064f2f 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -2556,6 +2556,7 @@ add_interface_variables(struct gl_shader_program *shProg,
  {
 foreach_in_list(ir_instruction, node, sh->ir) {
ir_variable *var = node->as_variable();
+  uint8_t mask = 0;
  
if (!var)

   continue;
@@ -2571,6 +2572,10 @@ add_interface_variables(struct gl_shader_program *shProg,
   var->data.location != SYSTEM_VALUE_VERTEX_ID_ZERO_BASE &&
   var->data.location != SYSTEM_VALUE_INSTANCE_ID)
  continue;
+ /* Mark special built-in inputs referenced by the vertex stage so
+  * that they are considered active by the shader queries.
+  */
+ mask = (1 << (MESA_SHADER_VERTEX));


I cringed when I saw that you explicitly set it only for the vertex 
shader, but this is indeed not a problem because only VertexID and 
InstanceID can go through here.


Then I wondered why you ignored all the other builtins. The spec is 
however quite specific about which builtins should be listed. No idea why.


Anyway, you get my R-b!

Reviewed-By: Martin Peres 

   /* FALLTHROUGH */
case ir_var_shader_in:
   if (programInterface != GL_PROGRAM_INPUT)
@@ -2585,7 +2590,7 @@ add_interface_variables(struct gl_shader_program *shProg,
};
  
if (!add_program_resource(shProg, programInterface, var,

-build_stageref(shProg, var->name)))
+build_stageref(shProg, var->name) | mask))
   return false;
 }
 return true;


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

[Mesa-dev] [Bug 90213] glDrawPixels with GL_COLOR_INDEX never returns.

[bugzilla-daemon]

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

--- Comment #5 from Iago Toral  ---
Sent a couple of patches to the mailing list for review:

http://lists.freedesktop.org/archives/mesa-dev/2015-May/083334.html
http://lists.freedesktop.org/archives/mesa-dev/2015-May/083335.html

The first one fixes the attached test program and also this piglit test:
bin/gl-1.0-drawpixels-color-index -auto -fbo

-- 
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
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 2/2] mesa: Always return a value in _mesa_format_from_format_and_type

[Iago Toral Quiroga]

Return MESA_FORMAT_NONE if no matching type was found. We have an
assertion here, but we should return something anyway to avoid
confusion with non-debug builds.
---
 src/mesa/main/glformats.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
index 8ced579..fddd048 100644
--- a/src/mesa/main/glformats.c
+++ b/src/mesa/main/glformats.c
@@ -2752,4 +2752,5 @@ _mesa_format_from_format_and_type(GLenum format, GLenum 
type)
 * format in that case.
 */
unreachable("Unsupported format");
+   return MESA_FORMAT_NONE;
 }
-- 
1.9.1

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

[Mesa-dev] [PATCH 1/2] swrast: Fix rgba_draw_pixels with GL_COLOR_INDEX

[Iago Toral Quiroga]

When we implemented the format conversion rewrite we forgot to handle
GL_COLOR_INDEX here, which needs special handling.

Fixes the following piglit test:
bin/gl-1.0-drawpixels-color-index -auto -fbo

Buzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90213
---
 src/mesa/swrast/s_drawpix.c | 29 ++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/src/mesa/swrast/s_drawpix.c b/src/mesa/swrast/s_drawpix.c
index bf42726..fb677ee 100644
--- a/src/mesa/swrast/s_drawpix.c
+++ b/src/mesa/swrast/s_drawpix.c
@@ -448,14 +448,34 @@ draw_rgba_pixels( struct gl_context *ctx, GLint x, GLint 
y,
{
   const GLbitfield interpMask = span.interpMask;
   const GLbitfield arrayMask = span.arrayMask;
-  const GLint srcStride
- = _mesa_image_row_stride(unpack, width, format, type);
   GLint skipPixels = 0;
   /* use span array for temp color storage */
   GLfloat *rgba = (GLfloat *) span.array->attribs[VARYING_SLOT_COL0];
   void *tempImage = NULL;
 
-  if (unpack->SwapBytes) {
+  /* We have to deal with GL_COLOR_INDEX manually because
+   * _mesa_format_convert does not handle this format. So what we do here 
is
+   * convert it to RGBA ubyte first and then convert from that to dst as
+   * usual.
+   */
+  if (format == GL_COLOR_INDEX) {
+ /* This will handle byte swapping and transferops if needed */
+ tempImage =
+_mesa_unpack_color_index_to_rgba_ubyte(ctx, 2,
+   pixels, format, type,
+   width, height, 1,
+   unpack,
+   transferOps);
+ if (!tempImage) {
+_mesa_error(ctx, GL_OUT_OF_MEMORY, "glDrawPixels");
+return;
+ }
+
+ transferOps = 0;
+ pixels = tempImage;
+ format = GL_RGBA;
+ type = GL_UNSIGNED_BYTE;
+  } else if (unpack->SwapBytes) {
  /* We have to handle byte-swapping scenarios before calling
   * _mesa_format_convert
   */
@@ -476,6 +496,9 @@ draw_rgba_pixels( struct gl_context *ctx, GLint x, GLint y,
  }
   }
 
+  const GLint srcStride
+ = _mesa_image_row_stride(unpack, width, format, type);
+
   /* if the span is wider than SWRAST_MAX_WIDTH we have to do it in chunks 
*/
   while (skipPixels < width) {
  const GLint spanWidth = MIN2(width - skipPixels, SWRAST_MAX_WIDTH);
-- 
1.9.1

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

Re: [Mesa-dev] [PATCH 12/13] i965: Add typed surface access opcodes.

[Pohjolainen, Topi]

Unfortunately mail server has purged the original patch number 13. I took
a look instead in your recent image-load-store-nir branch. I studied the
fence mechanism a little in bspec, and based on that I can't see anything
obvious amiss. Small nit below but:

Reviewed-by: Topi Pohjolainen 

Sorry for dropping the ball for such a long time.

+void
+brw_memory_fence(struct brw_codegen *p,
+ struct brw_reg dst)
+{
+   const struct brw_device_info *devinfo = p->devinfo;
+   const bool commit_enable = (devinfo->gen == 7 && !devinfo->is_haswell);

You can drop the extra ().
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 12/13] i965: Add typed surface access opcodes.

[Pohjolainen, Topi]

On Sat, Apr 25, 2015 at 01:45:36AM +0300, Francisco Jerez wrote:
> "Pohjolainen, Topi"  writes:
> 
> > On Fri, Feb 27, 2015 at 05:34:55PM +0200, Francisco Jerez wrote:
> >> ---
> >>  src/mesa/drivers/dri/i965/brw_defines.h|   4 +
> >>  src/mesa/drivers/dri/i965/brw_eu.h |  24 +++
> >>  src/mesa/drivers/dri/i965/brw_eu_emit.c| 169 
> >> +
> >>  src/mesa/drivers/dri/i965/brw_fs.cpp   |  12 ++
> >>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp |  17 +++
> >>  .../drivers/dri/i965/brw_schedule_instructions.cpp |   3 +
> >>  src/mesa/drivers/dri/i965/brw_shader.cpp   |   8 +
> >>  src/mesa/drivers/dri/i965/brw_vec4.cpp |   6 +
> >>  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp   |  18 +++
> >>  9 files changed, 261 insertions(+)
> >> 
> >> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h 
> >> b/src/mesa/drivers/dri/i965/brw_defines.h
> >> index e56f49c..cf07da9 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> >> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> >> @@ -906,6 +906,10 @@ enum opcode {
> >> SHADER_OPCODE_UNTYPED_SURFACE_READ,
> >> SHADER_OPCODE_UNTYPED_SURFACE_WRITE,
> >>  
> >> +   SHADER_OPCODE_TYPED_ATOMIC,
> >> +   SHADER_OPCODE_TYPED_SURFACE_READ,
> >> +   SHADER_OPCODE_TYPED_SURFACE_WRITE,
> >> +
> >> SHADER_OPCODE_GEN4_SCRATCH_READ,
> >> SHADER_OPCODE_GEN4_SCRATCH_WRITE,
> >> SHADER_OPCODE_GEN7_SCRATCH_READ,
> >> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h 
> >> b/src/mesa/drivers/dri/i965/brw_eu.h
> >> index cad956b..ce9554b 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_eu.h
> >> +++ b/src/mesa/drivers/dri/i965/brw_eu.h
> >> @@ -421,6 +421,30 @@ brw_untyped_surface_write(struct brw_compile *p,
> >>unsigned num_channels);
> >>  
> >>  void
> >> +brw_typed_atomic(struct brw_compile *p,
> >> + struct brw_reg dst,
> >> + struct brw_reg payload,
> >> + struct brw_reg surface,
> >> + unsigned atomic_op,
> >> + unsigned msg_length,
> >> + bool response_expected);
> >> +
> >> +void
> >> +brw_typed_surface_read(struct brw_compile *p,
> >> +   struct brw_reg dst,
> >> +   struct brw_reg payload,
> >> +   struct brw_reg surface,
> >> +   unsigned msg_length,
> >> +   unsigned num_channels);
> >> +
> >> +void
> >> +brw_typed_surface_write(struct brw_compile *p,
> >> +struct brw_reg payload,
> >> +struct brw_reg surface,
> >> +unsigned msg_length,
> >> +unsigned num_channels);
> >> +
> >> +void
> >>  brw_pixel_interpolator_query(struct brw_compile *p,
> >>   struct brw_reg dest,
> >>   struct brw_reg mrf,
> >> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c 
> >> b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> >> index f5b8fa9..74f1fc1 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> >> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> >> @@ -2944,6 +2944,175 @@ brw_untyped_surface_write(struct brw_compile *p,
> >>p, insn, num_channels);
> >>  }
> >>  
> >> +static void
> >> +brw_set_dp_typed_atomic_message(struct brw_compile *p,
> >> +struct brw_inst *insn,
> >> +unsigned atomic_op,
> >> +bool response_expected)
> >> +{
> >> +   const struct brw_context *brw = p->brw;
> >> +   unsigned msg_control =
> >> +  atomic_op | /* Atomic Operation Type: BRW_AOP_* */
> >> +  (response_expected ? 1 << 5 : 0); /* Return data expected */
> >> +
> >> +   if (brw->gen >= 8 || brw->is_haswell) {
> >> +  if (brw_inst_access_mode(brw, p->current) == BRW_ALIGN_1) {
> >> + if (brw_inst_qtr_control(brw, p->current) == GEN6_COMPRESSION_2Q)
> >> +msg_control |= 1 << 4; /* Use high 8 slots of the sample mask 
> >> */
> >> +
> >> + brw_inst_set_dp_msg_type(brw, insn,
> >> +  HSW_DATAPORT_DC_PORT1_TYPED_ATOMIC_OP);
> >> +  } else {
> >> + brw_inst_set_dp_msg_type(brw, insn,
> >> +  
> >> HSW_DATAPORT_DC_PORT1_TYPED_ATOMIC_OP_SIMD4X2);
> >> +  }
> >> +
> >> +   } else {
> >> +  brw_inst_set_dp_msg_type(brw, insn,
> >> +   GEN7_DATAPORT_RC_TYPED_ATOMIC_OP);
> >> +
> >> +  if (brw_inst_qtr_control(brw, p->current) == GEN6_COMPRESSION_2Q)
> >> + msg_control |= 1 << 4; /* Use high 8 slots of the sample mask */
> >> +   }
> >> +
> >> +   brw_inst_set_dp_msg_control(brw, insn, msg_control);
> >> +}
> >> +
> >> +void
> >> +brw_typed_atomic(struct brw_compile *p,
> >> + struct brw_reg dst,
> >> + struct brw_reg payload,
> >>