Re: [Mesa-dev] gallium endianness and hw drivers

[Marek Olšák]

On Thu, Jan 16, 2014 at 8:43 AM, Michel Dänzer  wrote:
> On Mit, 2014-01-15 at 14:27 +0100, Marek Olšák wrote:
>> On Wed, Jan 15, 2014 at 7:07 AM, Michel Dänzer  wrote:
>> > On Die, 2014-01-14 at 00:22 +0100, Marek Olšák wrote:
>> >> I think the format conversion functions should look like:
>> >>
>> >> #ifdef BIG_ENDIAN
>> >>case PIPE_FORMAT_A8B8G8R8_UNORM:
>> >>   return hw_format_for_R8G8B8A8_UNORM;
>> >> ...
>> >> #else
>> >>case PIPE_FORMAT_R8G8B8A8_UNORM:
>> >>   return hw_format_for_R8G8B8A8_UNORM;
>> >> #endif
>> >>
>> >> which can be simplified to:
>> >>
>> >>case PIPE_FORMAT_RGBA_UNORM:
>> >>   return hw_format_for_R8G8B8A8_UNORM;
>> >>
>> >> So that the GPU can see the same formats, but they are different for the 
>> >> CPU.
>> >>
>> >> What do you think?
>> >
>> > That might be an option, but PIPE_FORMAT_R8G8B8A8_UNORM is useful on big
>> > endian hosts as well, e.g. it matches GL_RGBA / GL_UNSIGNED_BYTE
>> > exactly.
>>
>> I wouldn't worry about such optimizations when we don't even have
>> proper big endian support.
>
> I'd consider it part of proper big endian support though.
>
>
>> I'd rather stick to the simplest solution for the old hardware.
>
> What do you mean by old hardware? Remember that SI and newer have
> basically no hardware byteswapping facilities anymore, and PowerPC
> machines with PCIe slots are still being produced and sold.

R300-R500.

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

[Mesa-dev] [PATCH V2 7/9] glsl: Aggregate initializer support for arrays of array

[Timothy Arceri]

V2 dont create a second process_array_type function

Signed-off-by: Timothy Arceri 
---
 src/glsl/ast.h  | 19 +++-
 src/glsl/ast_function.cpp   | 14 +++--
 src/glsl/ast_to_hir.cpp | 29 -
 src/glsl/glsl_parser.yy | 36 -
 src/glsl/glsl_parser_extras.cpp | 69 +++--
 5 files changed, 139 insertions(+), 28 deletions(-)

diff --git a/src/glsl/ast.h b/src/glsl/ast.h
index bbae9cd..931fafb 100644
--- a/src/glsl/ast.h
+++ b/src/glsl/ast.h
@@ -308,10 +308,16 @@ public:
   : ast_expression(ast_aggregate, NULL, NULL, NULL),
 constructor_type(NULL)
{
-  /* empty */
+  array_dimension = 1;
+  is_array = false;
}
 
ast_type_specifier *constructor_type;
+   unsigned array_dimension;
+
+   ast_array_specifier *array_specifier;
+   bool is_array;
+
virtual ir_rvalue *hir(exec_list *instructions,
   struct _mesa_glsl_parse_state *state);
 };
@@ -587,6 +593,11 @@ public:
 struct _mesa_glsl_parse_state *state)
   const;
 
+   const struct glsl_type *glsl_type(const char **name,
+ struct _mesa_glsl_parse_state *state,
+ bool skip_outer_dimension)
+  const;
+
virtual void print(void) const;
 
ir_rvalue *hir(exec_list *, struct _mesa_glsl_parse_state *);
@@ -1004,4 +1015,10 @@ extern void
 check_builtin_array_max_size(const char *name, unsigned size,
  YYLTYPE loc, struct _mesa_glsl_parse_state 
*state);
 
+extern const glsl_type *
+process_array_type(YYLTYPE *loc, const glsl_type *base,
+   ast_array_specifier *array_specifier,
+  struct _mesa_glsl_parse_state *state,
+   bool skip_first_dim);
+
 #endif /* AST_H */
diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
index 57aa45f..f245e32 100644
--- a/src/glsl/ast_function.cpp
+++ b/src/glsl/ast_function.cpp
@@ -1731,8 +1731,18 @@ ast_aggregate_initializer::hir(exec_list *instructions,
   _mesa_glsl_error(&loc, state, "type of C-style initializer unknown");
   return ir_rvalue::error_value(ctx);
}
-   const glsl_type *const constructor_type =
-  this->constructor_type->glsl_type(&name, state);
+   const glsl_type *constructor_type =
+  this->constructor_type->glsl_type(&name, state,
+this->array_dimension > 1 && 
!this->is_array);
+
+   /* This only handles "vec4 foo[..]".  The earlier 
constructor_type->glsl_type(...)
+* call already handled the "vec4[..] foo" case.
+*/
+   if (this->is_array) {
+  constructor_type =
+ process_array_type(&loc, constructor_type, this->array_specifier,
+state, this->array_dimension > 1);
+   }
 
if (!state->ARB_shading_language_420pack_enable) {
   _mesa_glsl_error(&loc, state, "C-style initialization requires the "
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 359a0b8..6c0c65b 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -1817,10 +1817,11 @@ process_array_size(exec_node *node,
return result;
 }
 
-static const glsl_type *
+const glsl_type *
 process_array_type(YYLTYPE *loc, const glsl_type *base,
ast_array_specifier *array_specifier,
-   struct _mesa_glsl_parse_state *state)
+   struct _mesa_glsl_parse_state *state,
+   bool skip_first_dim)
 {
const glsl_type *array_type = NULL;
const glsl_type *element_type = base;
@@ -1871,6 +1872,8 @@ process_array_type(YYLTYPE *loc, const glsl_type *base,
 
   unsigned array_size;
   for (/* nothing */; !node->is_head_sentinel(); node = node->prev) {
+ if (skip_first_dim && node->prev->is_head_sentinel())
+break;
  array_size = process_array_size(node, state);
  array_type_temp = glsl_type::get_array_instance(array_type_temp,
  array_size,
@@ -1902,6 +1905,14 @@ const glsl_type *
 ast_type_specifier::glsl_type(const char **name,
  struct _mesa_glsl_parse_state *state) const
 {
+   return this->glsl_type(name, state, false);
+}
+
+const glsl_type *
+ast_type_specifier::glsl_type(const char **name,
+ struct _mesa_glsl_parse_state *state,
+  bool skip_outer_dim) const
+{
const struct glsl_type *type;
 
type = state->symbols->get_type(this->type_name);
@@ -1909,7 +1920,8 @@ ast_type_specifier::glsl_type(const char **name,
 
if (this->is_array) {
   YYLTYPE loc = this->get_location();
-  type = process_array_type(&loc, type, this->array_specifier, state);
+  type = process_array_type(&loc, type, this->array_specifier,
+state, skip_outer_dim);

Re: [Mesa-dev] [RFC] freedreno: add tgsi lowering pass

[Marek Olšák]

On Thu, Jan 16, 2014 at 6:41 AM, Matt Turner  wrote:
> On Wed, Jan 15, 2014 at 5:40 AM, Rob Clark  wrote:
>> From: Rob Clark 
>>
>> So, as I found myself needing to lower a few more TGSI instructions,
>> and noticing yet again that I would have to do the same lowering as
>> various other gallium drivers already do,  I decided that maybe it
>> makes sense to do this instead generically in a way that could maybe
>> help other drivers too.
>>
>> This currently lowers the following instructions:
>>
>>DST, XPD, SCS, LRP, FRC, POW, LIT, EXP, LOG
>
> I think we already have GLSL IR level lowering passes for lrp, pow,
> exp, and log. Can't you use those?

I guess he can, but the ARB programs as well as the fixed-function
vertex program don't use the GLSL IR.

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

[Mesa-dev] [PATCH] glsl: Assert buildin uniform variables

[Juha-Pekka Heikkila]

Signed-off-by: Juha-Pekka Heikkila 
---
 src/glsl/builtin_variables.cpp | 24 
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/src/glsl/builtin_variables.cpp b/src/glsl/builtin_variables.cpp
index f630923..9f4f7c7 100644
--- a/src/glsl/builtin_variables.cpp
+++ b/src/glsl/builtin_variables.cpp
@@ -669,7 +669,11 @@ void
 builtin_variable_generator::generate_uniforms()
 {
add_uniform(int_t, "gl_NumSamples");
-   add_uniform(type("gl_DepthRangeParameters"), "gl_DepthRange");
+
+   const glsl_type *const depth_range_parameters_type =
+  type("gl_DepthRangeParameters");
+   assert(depth_range_parameters_type);
+   add_uniform(depth_range_parameters_type, "gl_DepthRange");
add_uniform(array(vec4_t, VERT_ATTRIB_MAX), "gl_CurrentAttribVertMESA");
add_uniform(array(vec4_t, VARYING_SLOT_MAX), "gl_CurrentAttribFragMESA");
 
@@ -688,7 +692,11 @@ builtin_variable_generator::generate_uniforms()
   add_uniform(mat4_t, "gl_ProjectionMatrixInverseTranspose");
   add_uniform(mat4_t, "gl_ModelViewProjectionMatrixInverseTranspose");
   add_uniform(float_t, "gl_NormalScale");
-  add_uniform(type("gl_LightModelParameters"), "gl_LightModel");
+
+  const glsl_type *const light_model_parameters_type =
+ type("gl_LightModelParameters");
+  assert(light_model_parameters_type);
+  add_uniform(light_model_parameters_type, "gl_LightModel");
   add_uniform(vec2_t, "gl_BumpRotMatrix0MESA");
   add_uniform(vec2_t, "gl_BumpRotMatrix1MESA");
   add_uniform(vec4_t, "gl_FogParamsOptimizedMESA");
@@ -701,10 +709,15 @@ builtin_variable_generator::generate_uniforms()
   add_uniform(mat4_array_type, "gl_TextureMatrixInverseTranspose");
 
   add_uniform(array(vec4_t, state->Const.MaxClipPlanes), "gl_ClipPlane");
-  add_uniform(type("gl_PointParameters"), "gl_Point");
+
+  const glsl_type *const point_parameters_type =
+ type("gl_PointParameters");
+  assert(point_parameters_type);
+  add_uniform(point_parameters_type, "gl_Point");
 
   const glsl_type *const material_parameters_type =
 type("gl_MaterialParameters");
+  assert(material_parameters_type);
   add_uniform(material_parameters_type, "gl_FrontMaterial");
   add_uniform(material_parameters_type, "gl_BackMaterial");
 
@@ -714,6 +727,7 @@ builtin_variable_generator::generate_uniforms()
 
   const glsl_type *const light_model_products_type =
  type("gl_LightModelProducts");
+  assert(light_model_products_type);
   add_uniform(light_model_products_type, "gl_FrontLightModelProduct");
   add_uniform(light_model_products_type, "gl_BackLightModelProduct");
 
@@ -736,7 +750,9 @@ builtin_variable_generator::generate_uniforms()
   add_uniform(texcoords_vec4, "gl_ObjectPlaneR");
   add_uniform(texcoords_vec4, "gl_ObjectPlaneQ");
 
-  add_uniform(type("gl_FogParameters"), "gl_Fog");
+  const glsl_type *const fog_parameters_type = type("gl_FogParameters");
+  assert(fog_parameters_type);
+  add_uniform(fog_parameters_type, "gl_Fog");
}
 }
 
-- 
1.8.1.2

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

Re: [Mesa-dev] [RFC] freedreno: add tgsi lowering pass

[Rob Clark]

On Thu, Jan 16, 2014 at 4:51 AM, Marek Olšák  wrote:
> On Thu, Jan 16, 2014 at 6:41 AM, Matt Turner  wrote:
>> On Wed, Jan 15, 2014 at 5:40 AM, Rob Clark  wrote:
>>> From: Rob Clark 
>>>
>>> So, as I found myself needing to lower a few more TGSI instructions,
>>> and noticing yet again that I would have to do the same lowering as
>>> various other gallium drivers already do,  I decided that maybe it
>>> makes sense to do this instead generically in a way that could maybe
>>> help other drivers too.
>>>
>>> This currently lowers the following instructions:
>>>
>>>DST, XPD, SCS, LRP, FRC, POW, LIT, EXP, LOG
>>
>> I think we already have GLSL IR level lowering passes for lrp, pow,
>> exp, and log. Can't you use those?
>
> I guess he can, but the ARB programs as well as the fixed-function
> vertex program don't use the GLSL IR.

a lot of the oddball instructions seem to show up from ARB piglit
tests.. so sounds like using GLSL IR would not be too useful.  The
only other reasonable alternative that I saw was to build these
lowering directly into the tgsi generation directly, but seemed safer
/ less-intrusive to have it as a separate pass on the side.  And I
suppose doing it in GLSL IR -> TGSI would also not help for ARB or
fixed function shaders, I think?

Either way, I'm starting to play around with making a proper compiler
(ie. one that can actually do instruction scheduling and register
assignment), and getting rid of a ton of boring lowering code in my
compiler makes for less code to rewrite... which is a good thing.  I
don't mind keeping it in freedreno (since a few of the lowerings I
also need for a2xx as well as a3xx), but seems like it is something
that would probably be useful to other drivers too.

BR,
-R

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

Re: [Mesa-dev] [wip 9/9] mesa: OES_get_program_binary extension functionality

[Tapani Pälli]

On 01/15/2014 06:13 PM, Paul Berry wrote:
On 2 January 2014 03:58, Tapani Pälli > wrote:


Signed-off-by: Tapani Pälli mailto:tapani.pa...@intel.com>>
---
 src/mesa/main/shaderapi.c | 44
++--
 1 file changed, 38 insertions(+), 6 deletions(-)

+   char *data = mesa_program_serialize(shProg, &size);
+
+   /* we have more data that can fit to user given buffer */
+   if (size > bufSize) {
+  _mesa_error(ctx, GL_INVALID_OPERATION, __FUNCTION__);
+  if (data)
+ free(data);


Why would we ever expect mesa_program_serialize to set size to a 
nonzero value but return NULL?  It seems like this could only happen 
if there's a bug, in which case this really ought to be


assert(data !=NULL);

Also, it's safe to call free() on a NULL pointer. According to the C 
standard, freeing a NULL pointer does nothing.


sure, will fix


+  return;
+   }
+
+   if (data) {


Similarly, this if-statement is unnecessary.


it is required for memcpy but not for free


+  memcpy(binary, data, size);
+  free(data);
+   }
+
+   if (length != NULL)
+  *length = size;
+
+   *binaryFormat = 0;
 }

 void GLAPIENTRY
@@ -1647,10 +1666,23 @@ _mesa_ProgramBinary(GLuint program, GLenum
binaryFormat,
if (!shProg)
   return;

-   (void) binaryFormat;
-   (void) binary;
-   (void) length;
-   _mesa_error(ctx, GL_INVALID_OPERATION, __FUNCTION__);
+   if (length <= 0)
+  return;


In the case of an invalid length, we need to make sure to set the 
program's LinkStatus to false.  Also, the information log needs to be 
cleared, in accordance with this text from OES_get_program_binary:




OK, I'll make a common error handling for this and case you mention below ..

If ProgramBinaryOES failed, any information about a previous link 
or load of
that program object is lost.  Thus, a failed load does not restore 
the old

state of .

+
+   /* free possible existing data and initialize structure */
+   _mesa_free_shader_program_data(ctx, shProg);
+   _mesa_init_shader_program(ctx, shProg);
+
+   /* fill structure from a binary blob */
+   if (mesa_program_deserialize(shProg, binary, length)) {
+  _mesa_error(ctx, GL_INVALID_VALUE, "glProgramBinary(binary
incompatible)");


This seems wrong to me.  From the OES_get_program_binary spec:

...  and  must be
those returned by a previous call to GetProgramBinaryOES, and 
 must
be the length of the program binary as returned by 
GetProgramBinaryOES or
GetProgramiv with  PROGRAM_BINARY_LENGTH_OES.  The program 
binary

will fail to load if these conditions are not met.

...

A program object's program binary is replaced by calls to 
LinkProgram or
ProgramBinaryOES.  Either command sets the program object's 
LINK_STATUS to
TRUE or FALSE, as queried with GetProgramiv, to reflect success or 
failure.

Either command also updates its information log, queried with
GetProgramInfoLog, to provide details about warnings or errors.

I believe this means that if deserialization fails, it should not be a 
GL error.  It should simply be treated as a link failure.


+  return;
+   }
+
+   /* driver specific link, optimizations and what not */
+   ctx->Driver.LinkShader(ctx, shProg);


Now I'm confused.  I thought a major part of the purpose of this 
extension was that it would store the post-link program, so that not 
only does glProgramBinary() avoid the runtime penalty of parsing and 
compiling, it also avoids the runtime penalty of link-time 
optimizations.  Calling ctx->Driver.LinkShader seems like it defeats 
that purpose.  It seems like what we ought to be doing is store the 
data that ctx->Driver.LinkShader *produces* in the binary blob, so 
that once it's loaded there's no further linking necessary.  If there 
is a small amount of driver-specific hook necessary, that should be 
placed in a new ctx->Driver function rather than incurring the 
overhead of another link.


This is the driver specific parts which is unfortunately not yet there. 
I'm working on adding data stored in 'state_cache' (program, prog_data 
and keys) from i965 driver within the blob so that I could avoid the 
call to the driver. It could be that driver generated IR and possibly 
some other structures need caching or recreation as well to skip the 
whole call (now experiencing some gpu hangs when restoring only the 
program and prog_data parts), another hook might be needed to cover 
this. I'm currently studying the i965 driver to understand what is required.


So what's in the blob is:

* some validation data
* gl_shader_program uniform and variable hashes
* shader type + misc data for gl_shader structure
* mesa generated ir (gl_shader->ir) for each shader stage

Additionally the

[Mesa-dev] [PATCH] v2 of glx: Add missing null check in glXCreateContextAttribsARB

[Juha-Pekka Heikkila]

Thanks Brian for the comment, you are right now the null check makes more sense.

Juha-Pekka Heikkila (1):
  glx: Add missing null check in glXCreateContextAttribsARB

 src/glx/create_context.c | 3 +++
 1 file changed, 3 insertions(+)

-- 
1.8.1.2

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

[Mesa-dev] [PATCH] glx: Add missing null check in glXCreateContextAttribsARB

[Juha-Pekka Heikkila]

Signed-off-by: Juha-Pekka Heikkila 
---
 src/glx/create_context.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/glx/create_context.c b/src/glx/create_context.c
index 38e949a..de54117 100644
--- a/src/glx/create_context.c
+++ b/src/glx/create_context.c
@@ -88,6 +88,9 @@ glXCreateContextAttribsARB(Display *dpy, GLXFBConfig config,
   (const uint32_t *) attrib_list,
   &dummy_err);
 #endif
+
+  if (gc == NULL)
+ return NULL;
}
 
gc->xid = xcb_generate_id(c);
-- 
1.8.1.2

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

Re: [Mesa-dev] [PATCH 2/2] llvmpipe: handle NULL color buffer pointers

[Brian Paul]

On 01/15/2014 07:11 PM, Roland Scheidegger wrote:

Looks good overall, just some minor quibbles inline.

Roland

Am 16.01.2014 03:15, schrieb Brian Paul:

Fixes regression from 9baa45f78b8ca7d66280e36009b6a685055d7cd6
---
  src/gallium/drivers/llvmpipe/lp_rast.c  |   44 +---
  src/gallium/drivers/llvmpipe/lp_rast_priv.h |   11 +-
  src/gallium/drivers/llvmpipe/lp_scene.c |6 +-
  src/gallium/drivers/llvmpipe/lp_setup.c |2 +-
  src/gallium/drivers/llvmpipe/lp_state_fs.c  |  152 +++
  5 files changed, 126 insertions(+), 89 deletions(-)

diff --git a/src/gallium/drivers/llvmpipe/lp_rast.c 
b/src/gallium/drivers/llvmpipe/lp_rast.c
index 6feec94..3e25ff0 100644
--- a/src/gallium/drivers/llvmpipe/lp_rast.c
+++ b/src/gallium/drivers/llvmpipe/lp_rast.c
@@ -124,7 +124,8 @@ lp_rast_clear_color(struct lp_rasterizer_task *task,
unsigned i;
union util_color uc;

-  if (util_format_is_pure_integer(scene->fb.cbufs[0]->format)) {
+  if (scene->fb.cbufs[0] &&
+  util_format_is_pure_integer(scene->fb.cbufs[0]->format)) {

This isn't quite correct, since if the first fb is NULL but the second
is a pure int you'd end up in the non pure int code path below. Maybe
would be easier if just iterating over all fbs in the outermost loop and
doing that decision per fb (this only worked because mixed int/fixed fbs
weren't allowed, but it should be quite ok deciding that per fb anyway).


OK, I'll put in a loop.





   /*
* We expect int/uint clear values here, though some APIs
* might disagree (but in any case util_pack_color()
@@ -174,20 +175,22 @@ lp_rast_clear_color(struct lp_rasterizer_task *task,
  clear_color[3]);

   for (i = 0; i < scene->fb.nr_cbufs; i++) {
-util_pack_color(arg.clear_color.f,
-scene->fb.cbufs[i]->format, &uc);
-
-util_fill_box(scene->cbufs[i].map,
-  scene->fb.cbufs[i]->format,
-  scene->cbufs[i].stride,
-  scene->cbufs[i].layer_stride,
-  task->x,
-  task->y,
-  0,
-  task->width,
-  task->height,
-  scene->fb_max_layer + 1,
-  &uc);
+if (scene->fb.cbufs[i]) {
+   util_pack_color(arg.clear_color.f,
+   scene->fb.cbufs[i]->format, &uc);
+
+   util_fill_box(scene->cbufs[i].map,
+ scene->fb.cbufs[i]->format,
+ scene->cbufs[i].stride,
+ scene->cbufs[i].layer_stride,
+ task->x,
+ task->y,
+ 0,
+ task->width,
+ task->height,
+ scene->fb_max_layer + 1,
+ &uc);
+}
   }
}
 }
@@ -444,8 +447,15 @@ lp_rast_shade_quads_mask(struct lp_rasterizer_task *task,

 /* color buffer */
 for (i = 0; i < scene->fb.nr_cbufs; i++) {
-  stride[i] = scene->cbufs[i].stride;
-  color[i] = lp_rast_get_unswizzled_color_block_pointer(task, i, x, y, 
inputs->layer);
+  if (scene->fb.cbufs[i]) {
+ stride[i] = scene->cbufs[i].stride;
+ color[i] = lp_rast_get_unswizzled_color_block_pointer(task, i, x, y,
+   inputs->layer);
+  }
+  else {
+ stride[i] = 0;
+ color[i] = NULL;
+  }
 }

 /* depth buffer */
diff --git a/src/gallium/drivers/llvmpipe/lp_rast_priv.h 
b/src/gallium/drivers/llvmpipe/lp_rast_priv.h
index bc361b6..063a70e 100644
--- a/src/gallium/drivers/llvmpipe/lp_rast_priv.h
+++ b/src/gallium/drivers/llvmpipe/lp_rast_priv.h
@@ -293,8 +293,15 @@ lp_rast_shade_quads_all( struct lp_rasterizer_task *task,

 /* color buffer */
 for (i = 0; i < scene->fb.nr_cbufs; i++) {
-  stride[i] = scene->cbufs[i].stride;
-  color[i] = lp_rast_get_unswizzled_color_block_pointer(task, i, x, y, 
inputs->layer);
+  if (scene->fb.cbufs[i]) {
+ stride[i] = scene->cbufs[i].stride;
+ color[i] = lp_rast_get_unswizzled_color_block_pointer(task, i, x, y,
+   inputs->layer);
+  }
+  else {
+ stride[i] = 0;
+ color[i] = NULL;
+  }
 }

 if (scene->zsbuf.map) {
diff --git a/src/gallium/drivers/llvmpipe/lp_scene.c 
b/src/gallium/drivers/llvmpipe/lp_scene.c
index 0296b79..134ab86 100644
--- a/src/gallium/drivers/llvmpipe/lp_scene.c
+++ b/src/gallium/drivers/llvmpipe/lp_scene.c
@@ -156,6 +156,10 @@ lp_scene_begin_rasterization(struct lp_scene *scene)

 for (i = 0; i < scene->fb.nr_cbufs; i++) {
struct pipe_s

[Mesa-dev] [PATCH] llvmpipe: handle NULL color buffer pointers

[Brian Paul]

Fixes regression from 9baa45f78b8ca7d66280e36009b6a685055d7cd6

v2: incorporate a few small changes suggested by Roland.
---
 src/gallium/drivers/llvmpipe/lp_rast.c  |   62 ---
 src/gallium/drivers/llvmpipe/lp_rast_priv.h |   11 +-
 src/gallium/drivers/llvmpipe/lp_scene.c |   23 ++--
 src/gallium/drivers/llvmpipe/lp_setup.c |2 +-
 src/gallium/drivers/llvmpipe/lp_state_fs.c  |  152 +++
 5 files changed, 156 insertions(+), 94 deletions(-)

diff --git a/src/gallium/drivers/llvmpipe/lp_rast.c 
b/src/gallium/drivers/llvmpipe/lp_rast.c
index 6feec94..6ee849b 100644
--- a/src/gallium/drivers/llvmpipe/lp_rast.c
+++ b/src/gallium/drivers/llvmpipe/lp_rast.c
@@ -110,6 +110,25 @@ lp_rast_tile_begin(struct lp_rasterizer_task *task,
 
 
 /**
+ * Examine a framebuffer object to determine if any of the colorbuffers
+ * use a pure integer format.
+ * XXX this could be a gallium utility function if useful elsewhere.
+ */
+static boolean
+is_fb_pure_integer(const struct pipe_framebuffer_state *fb)
+{
+   unsigned i;
+   for (i = 0; i < fb->nr_cbufs; i++) {
+  if (fb->cbufs[i] &&
+  util_format_is_pure_integer(fb->cbufs[i]->format)) {
+ return TRUE;
+  }
+   }
+   return FALSE;
+}
+
+
+/**
  * Clear the rasterizer's current color tile.
  * This is a bin command called during bin processing.
  * Clear commands always clear all bound layers.
@@ -124,7 +143,7 @@ lp_rast_clear_color(struct lp_rasterizer_task *task,
   unsigned i;
   union util_color uc;
 
-  if (util_format_is_pure_integer(scene->fb.cbufs[0]->format)) {
+  if (is_fb_pure_integer(&scene->fb)) {
  /*
   * We expect int/uint clear values here, though some APIs
   * might disagree (but in any case util_pack_color()
@@ -174,20 +193,22 @@ lp_rast_clear_color(struct lp_rasterizer_task *task,
 clear_color[3]);
 
  for (i = 0; i < scene->fb.nr_cbufs; i++) {
-util_pack_color(arg.clear_color.f,
-scene->fb.cbufs[i]->format, &uc);
-
-util_fill_box(scene->cbufs[i].map,
-  scene->fb.cbufs[i]->format,
-  scene->cbufs[i].stride,
-  scene->cbufs[i].layer_stride,
-  task->x,
-  task->y,
-  0,
-  task->width,
-  task->height,
-  scene->fb_max_layer + 1,
-  &uc);
+if (scene->fb.cbufs[i]) {
+   util_pack_color(arg.clear_color.f,
+   scene->fb.cbufs[i]->format, &uc);
+
+   util_fill_box(scene->cbufs[i].map,
+ scene->fb.cbufs[i]->format,
+ scene->cbufs[i].stride,
+ scene->cbufs[i].layer_stride,
+ task->x,
+ task->y,
+ 0,
+ task->width,
+ task->height,
+ scene->fb_max_layer + 1,
+ &uc);
+}
  }
   }
}
@@ -444,8 +465,15 @@ lp_rast_shade_quads_mask(struct lp_rasterizer_task *task,
 
/* color buffer */
for (i = 0; i < scene->fb.nr_cbufs; i++) {
-  stride[i] = scene->cbufs[i].stride;
-  color[i] = lp_rast_get_unswizzled_color_block_pointer(task, i, x, y, 
inputs->layer);
+  if (scene->fb.cbufs[i]) {
+ stride[i] = scene->cbufs[i].stride;
+ color[i] = lp_rast_get_unswizzled_color_block_pointer(task, i, x, y,
+   inputs->layer);
+  }
+  else {
+ stride[i] = 0;
+ color[i] = NULL;
+  }
}
 
/* depth buffer */
diff --git a/src/gallium/drivers/llvmpipe/lp_rast_priv.h 
b/src/gallium/drivers/llvmpipe/lp_rast_priv.h
index bc361b6..063a70e 100644
--- a/src/gallium/drivers/llvmpipe/lp_rast_priv.h
+++ b/src/gallium/drivers/llvmpipe/lp_rast_priv.h
@@ -293,8 +293,15 @@ lp_rast_shade_quads_all( struct lp_rasterizer_task *task,
 
/* color buffer */
for (i = 0; i < scene->fb.nr_cbufs; i++) {
-  stride[i] = scene->cbufs[i].stride;
-  color[i] = lp_rast_get_unswizzled_color_block_pointer(task, i, x, y, 
inputs->layer);
+  if (scene->fb.cbufs[i]) {
+ stride[i] = scene->cbufs[i].stride;
+ color[i] = lp_rast_get_unswizzled_color_block_pointer(task, i, x, y,
+   inputs->layer);
+  }
+  else {
+ stride[i] = 0;
+ color[i] = NULL;
+  }
}
 
if (scene->zsbuf.map) {
diff --git a/src/gallium/drivers/llvmpipe/lp_scene.c 
b/src/gallium/drivers/llvmpipe/lp_scene.c
index 0296b79..9ba5f1a 100644
--- a/src/gallium/drivers/llvmpipe/lp_scene.c
+++ b/src/gallium/drivers/llvmpipe/lp_sce

Re: [Mesa-dev] [PATCH] llvmpipe: handle NULL color buffer pointers

[Jose Fonseca]

Looks good AFAICT. Thanks Brian.

Jose

- Original Message -
> Fixes regression from 9baa45f78b8ca7d66280e36009b6a685055d7cd6
> 
> v2: incorporate a few small changes suggested by Roland.
> ---
>  src/gallium/drivers/llvmpipe/lp_rast.c  |   62 ---
>  src/gallium/drivers/llvmpipe/lp_rast_priv.h |   11 +-
>  src/gallium/drivers/llvmpipe/lp_scene.c |   23 ++--
>  src/gallium/drivers/llvmpipe/lp_setup.c |2 +-
>  src/gallium/drivers/llvmpipe/lp_state_fs.c  |  152
>  +++
>  5 files changed, 156 insertions(+), 94 deletions(-)
> 
> diff --git a/src/gallium/drivers/llvmpipe/lp_rast.c
> b/src/gallium/drivers/llvmpipe/lp_rast.c
> index 6feec94..6ee849b 100644
> --- a/src/gallium/drivers/llvmpipe/lp_rast.c
> +++ b/src/gallium/drivers/llvmpipe/lp_rast.c
> @@ -110,6 +110,25 @@ lp_rast_tile_begin(struct lp_rasterizer_task *task,
>  
>  
>  /**
> + * Examine a framebuffer object to determine if any of the colorbuffers
> + * use a pure integer format.
> + * XXX this could be a gallium utility function if useful elsewhere.
> + */
> +static boolean
> +is_fb_pure_integer(const struct pipe_framebuffer_state *fb)
> +{
> +   unsigned i;
> +   for (i = 0; i < fb->nr_cbufs; i++) {
> +  if (fb->cbufs[i] &&
> +  util_format_is_pure_integer(fb->cbufs[i]->format)) {
> + return TRUE;
> +  }
> +   }
> +   return FALSE;
> +}
> +
> +
> +/**
>   * Clear the rasterizer's current color tile.
>   * This is a bin command called during bin processing.
>   * Clear commands always clear all bound layers.
> @@ -124,7 +143,7 @@ lp_rast_clear_color(struct lp_rasterizer_task *task,
>unsigned i;
>union util_color uc;
>  
> -  if (util_format_is_pure_integer(scene->fb.cbufs[0]->format)) {
> +  if (is_fb_pure_integer(&scene->fb)) {
>   /*
>* We expect int/uint clear values here, though some APIs
>* might disagree (but in any case util_pack_color()
> @@ -174,20 +193,22 @@ lp_rast_clear_color(struct lp_rasterizer_task *task,
>  clear_color[3]);
>  
>   for (i = 0; i < scene->fb.nr_cbufs; i++) {
> -util_pack_color(arg.clear_color.f,
> -scene->fb.cbufs[i]->format, &uc);
> -
> -util_fill_box(scene->cbufs[i].map,
> -  scene->fb.cbufs[i]->format,
> -  scene->cbufs[i].stride,
> -  scene->cbufs[i].layer_stride,
> -  task->x,
> -  task->y,
> -  0,
> -  task->width,
> -  task->height,
> -  scene->fb_max_layer + 1,
> -  &uc);
> +if (scene->fb.cbufs[i]) {
> +   util_pack_color(arg.clear_color.f,
> +   scene->fb.cbufs[i]->format, &uc);
> +
> +   util_fill_box(scene->cbufs[i].map,
> + scene->fb.cbufs[i]->format,
> + scene->cbufs[i].stride,
> + scene->cbufs[i].layer_stride,
> + task->x,
> + task->y,
> + 0,
> + task->width,
> + task->height,
> + scene->fb_max_layer + 1,
> + &uc);
> +}
>   }
>}
> }
> @@ -444,8 +465,15 @@ lp_rast_shade_quads_mask(struct lp_rasterizer_task
> *task,
>  
> /* color buffer */
> for (i = 0; i < scene->fb.nr_cbufs; i++) {
> -  stride[i] = scene->cbufs[i].stride;
> -  color[i] = lp_rast_get_unswizzled_color_block_pointer(task, i, x, y,
> inputs->layer);
> +  if (scene->fb.cbufs[i]) {
> + stride[i] = scene->cbufs[i].stride;
> + color[i] = lp_rast_get_unswizzled_color_block_pointer(task, i, x,
> y,
> +
> inputs->layer);
> +  }
> +  else {
> + stride[i] = 0;
> + color[i] = NULL;
> +  }
> }
>  
> /* depth buffer */
> diff --git a/src/gallium/drivers/llvmpipe/lp_rast_priv.h
> b/src/gallium/drivers/llvmpipe/lp_rast_priv.h
> index bc361b6..063a70e 100644
> --- a/src/gallium/drivers/llvmpipe/lp_rast_priv.h
> +++ b/src/gallium/drivers/llvmpipe/lp_rast_priv.h
> @@ -293,8 +293,15 @@ lp_rast_shade_quads_all( struct lp_rasterizer_task
> *task,
>  
> /* color buffer */
> for (i = 0; i < scene->fb.nr_cbufs; i++) {
> -  stride[i] = scene->cbufs[i].stride;
> -  color[i] = lp_rast_get_unswizzled_color_block_pointer(task, i, x, y,
> inputs->layer);
> +  if (scene->fb.cbufs[i]) {
> + stride[i] = scene->cbufs[i].stride;
> + color[i] = lp_rast_get_unswizzled_color_block_pointer(task, i, x,
> y,
> +
> inputs->layer);
> +  }
> +  else {
> + stride[i] = 0;
> + color[i] = NULL;
> +  }
> }
>  
> if (scene->zsbuf.ma

Re: [Mesa-dev] [PATCH] llvmpipe: fix possible constant buffer overflow

[Brian Paul]

On 01/15/2014 06:09 PM, Zack Rusin wrote:

It's possible to bind a smaller buffer as a constant buffer, than
what the shader actually uses/requires. This could cause nasty
crashes. This patch adds the architecture to pass the maximum
allowable constant buffer index to the jit to let it make
sure that the constant buffer indices are always within bounds.
The behavior follows the d3d10 spec, which says the overflow
should always return all zeros, and overflow is only defined
as access beyond the size of the currently bound buffer. Accesses
beyond the declared shader constant register size are not
considered an overflow and expected to return garbage but consistent
garbage (we follow the behavior which some wlk tests expect which
is to return the actual values from the bound buffer).

Signed-off-by: Zack Rusin 


My only suggestion is the subject line.  How about "llvmpipe: do 
constant buffer bounds checking in shaders"?  I tend to think of 
overflow as an issue when writing, not reading.


Not a big deal though.  Nice work!

-Brian

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

[Mesa-dev] [Bug 69285] Enabling LLVM results in substantially different rendering

[bugzilla-daemon]

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

--- Comment #4 from Charles Huber  ---
Still broken in 10.0.2 with LLVM 3.4.

Slightly different configure required for 3.4:

./configure \
--disable-assertions \
--enable-terminfo=no \
--enable-curses=no \

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

Re: [Mesa-dev] [Mesa PATCH 2/3] i965: Use the new drm_intel_bo offset64 field.

[Eric Anholt]

Kenneth Graunke  writes:

> On 01/15/2014 12:47 PM, Eric Anholt wrote:
>> Kenneth Graunke  writes:
>> 
>>> libdrm 2.4.52 introduces a new 'uint64_t offset64' field, intended to
>>> replace the old 'unsigned long offset' field.  To preserve ABI, libdrm
>>> continues to store the presumed offset in both locations.
>>>
>>> On Broadwell, a 64-bit kernel may place BOs at "high" (> 4G) addresses.
>>> However, with a 32-bit userspace, the 'unsigned long offset' field will
>>> only be 32-bit, which is not large enough to hold this value.  We need
>>> to use a proper uint64_t (like the kernel does).
>>>
>>> Technically, a lot of this code doesn't affect Broadwell, so we could
>>> leave it using the old field.  But it makes sense to just switch to the
>>> new, properly typed field.
>> 
>> This series is:
>> 
>> Reviewed-by: Eric Anholt 
>> 
>> I was concerned about brw_program_reloc returning uint32_t still, except
>> that on gen5+ it's always just returning the incoming prog_offset from
>> the state base address.
>
> From our conversation on IRC, it sounded like you had some ideas for
> creating a newer/better libdrm API for doing relocations, which would
> replace this.  Did you give up on that?

I've got a patch laying around, but I wrote it without your patch and I
need to rebase.  I also got distracted by the "busy" performance fix.
Of course, it doesn't let us get rid of the mesa presumed offset code
yet, but if we can get rid of the subdata-upload path of batches now
that CACHED_BATCH is going away, that would be the next step.


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

[Mesa-dev] [Bug 69285] Enabling LLVM results in substantially different rendering

[bugzilla-daemon]

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

Charles Huber  changed:

   What|Removed |Added

Version|9.2 |git

--- Comment #5 from Charles Huber  ---
Broken on current git master (188383591d6d657b557a5407ee9b7b993f79c53b) too.

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

Re: [Mesa-dev] [PATCH] llvmpipe: fix possible constant buffer overflow

[Roland Scheidegger]

Am 16.01.2014 03:09, schrieb Zack Rusin:
> It's possible to bind a smaller buffer as a constant buffer, than
> what the shader actually uses/requires. This could cause nasty
> crashes. This patch adds the architecture to pass the maximum
> allowable constant buffer index to the jit to let it make
> sure that the constant buffer indices are always within bounds.
> The behavior follows the d3d10 spec, which says the overflow
> should always return all zeros, and overflow is only defined
> as access beyond the size of the currently bound buffer. Accesses
> beyond the declared shader constant register size are not
> considered an overflow and expected to return garbage but consistent
> garbage (we follow the behavior which some wlk tests expect which
> is to return the actual values from the bound buffer).
> 
> Signed-off-by: Zack Rusin 
> ---
>  src/gallium/auxiliary/draw/draw_llvm.c | 42 ++
>  src/gallium/auxiliary/draw/draw_llvm.h | 32 +---
>  .../draw/draw_pt_fetch_shade_pipeline_llvm.c   |  6 ++
>  src/gallium/auxiliary/gallivm/lp_bld_tgsi.h|  2 +
>  src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c| 89 
> ++
>  src/gallium/drivers/llvmpipe/lp_jit.c  |  7 +-
>  src/gallium/drivers/llvmpipe/lp_jit.h  |  5 ++
>  src/gallium/drivers/llvmpipe/lp_setup.c|  7 +-
>  src/gallium/drivers/llvmpipe/lp_state_fs.c |  5 +-
>  9 files changed, 152 insertions(+), 43 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/draw/draw_llvm.c 
> b/src/gallium/auxiliary/draw/draw_llvm.c
> index 331039a..0bbb680 100644
> --- a/src/gallium/auxiliary/draw/draw_llvm.c
> +++ b/src/gallium/auxiliary/draw/draw_llvm.c
> @@ -242,17 +242,20 @@ create_jit_context_type(struct gallivm_state *gallivm,
>  {
> LLVMTargetDataRef target = gallivm->target;
> LLVMTypeRef float_type = LLVMFloatTypeInContext(gallivm->context);
> +   LLVMTypeRef int_type = LLVMInt32TypeInContext(gallivm->context);
> LLVMTypeRef elem_types[DRAW_JIT_CTX_NUM_FIELDS];
> LLVMTypeRef context_type;
>  
> elem_types[0] = LLVMArrayType(LLVMPointerType(float_type, 0), /* 
> vs_constants */
>   LP_MAX_TGSI_CONST_BUFFERS);
> -   elem_types[1] = LLVMPointerType(LLVMArrayType(LLVMArrayType(float_type, 
> 4),
> +   elem_types[1] = LLVMArrayType(int_type, /* num_vs_constants */
> + LP_MAX_TGSI_CONST_BUFFERS);
> +   elem_types[2] = LLVMPointerType(LLVMArrayType(LLVMArrayType(float_type, 
> 4),
>   DRAW_TOTAL_CLIP_PLANES), 0);
> -   elem_types[2] = LLVMPointerType(float_type, 0); /* viewport */
> -   elem_types[3] = LLVMArrayType(texture_type,
> +   elem_types[3] = LLVMPointerType(float_type, 0); /* viewport */
> +   elem_types[4] = LLVMArrayType(texture_type,
>   PIPE_MAX_SHADER_SAMPLER_VIEWS); /* textures 
> */
> -   elem_types[4] = LLVMArrayType(sampler_type,
> +   elem_types[5] = LLVMArrayType(sampler_type,
>   PIPE_MAX_SAMPLERS); /* samplers */
> context_type = LLVMStructTypeInContext(gallivm->context, elem_types,
>Elements(elem_types), 0);
> @@ -264,6 +267,8 @@ create_jit_context_type(struct gallivm_state *gallivm,
>  
> LP_CHECK_MEMBER_OFFSET(struct draw_jit_context, vs_constants,
>target, context_type, DRAW_JIT_CTX_CONSTANTS);
> +   LP_CHECK_MEMBER_OFFSET(struct draw_jit_context, num_vs_constants,
> +  target, context_type, DRAW_JIT_CTX_NUM_CONSTANTS);
> LP_CHECK_MEMBER_OFFSET(struct draw_jit_context, planes,
>target, context_type, DRAW_JIT_CTX_PLANES);
> LP_CHECK_MEMBER_OFFSET(struct draw_jit_context, viewport,
> @@ -298,20 +303,22 @@ create_gs_jit_context_type(struct gallivm_state 
> *gallivm,
>  
> elem_types[0] = LLVMArrayType(LLVMPointerType(float_type, 0), /* 
> constants */
>   LP_MAX_TGSI_CONST_BUFFERS);
> -   elem_types[1] = LLVMPointerType(LLVMArrayType(LLVMArrayType(float_type, 
> 4),
> +   elem_types[1] = LLVMArrayType(int_type, /* num_constants */
> + LP_MAX_TGSI_CONST_BUFFERS);
> +   elem_types[2] = LLVMPointerType(LLVMArrayType(LLVMArrayType(float_type, 
> 4),
>   DRAW_TOTAL_CLIP_PLANES), 0);
> -   elem_types[2] = LLVMPointerType(float_type, 0); /* viewport */
> +   elem_types[3] = LLVMPointerType(float_type, 0); /* viewport */
>  
> -   elem_types[3] = LLVMArrayType(texture_type,
> +   elem_types[4] = LLVMArrayType(texture_type,
>   PIPE_MAX_SHADER_SAMPLER_VIEWS); /* textures 
> */
> -   elem_types[4] = LLVMArrayType(sampler_type,
> +   elem_types[5] = LLVMArrayType(sampler_type,
>   PIPE_MAX_SAMPLERS); /* samplers */
> 
> -   elem_types[5] 

Re: [Mesa-dev] [PATCH 2/2] llvmpipe: handle NULL color buffer pointers

[Roland Scheidegger]

Am 16.01.2014 17:19, schrieb Brian Paul:
> On 01/15/2014 07:11 PM, Roland Scheidegger wrote:
>> Looks good overall, just some minor quibbles inline.
>>
>> Roland
>>
>> Am 16.01.2014 03:15, schrieb Brian Paul:
>>> Fixes regression from 9baa45f78b8ca7d66280e36009b6a685055d7cd6
>>> ---
>>>   src/gallium/drivers/llvmpipe/lp_rast.c  |   44 +---
>>>   src/gallium/drivers/llvmpipe/lp_rast_priv.h |   11 +-
>>>   src/gallium/drivers/llvmpipe/lp_scene.c |6 +-
>>>   src/gallium/drivers/llvmpipe/lp_setup.c |2 +-
>>>   src/gallium/drivers/llvmpipe/lp_state_fs.c  |  152
>>> +++
>>>   5 files changed, 126 insertions(+), 89 deletions(-)
>>>
>>> diff --git a/src/gallium/drivers/llvmpipe/lp_rast.c
>>> b/src/gallium/drivers/llvmpipe/lp_rast.c
>>> index 6feec94..3e25ff0 100644
>>> --- a/src/gallium/drivers/llvmpipe/lp_rast.c
>>> +++ b/src/gallium/drivers/llvmpipe/lp_rast.c
>>> @@ -124,7 +124,8 @@ lp_rast_clear_color(struct lp_rasterizer_task *task,
>>> unsigned i;
>>> union util_color uc;
>>>
>>> -  if (util_format_is_pure_integer(scene->fb.cbufs[0]->format)) {
>>> +  if (scene->fb.cbufs[0] &&
>>> +  util_format_is_pure_integer(scene->fb.cbufs[0]->format)) {
>> This isn't quite correct, since if the first fb is NULL but the second
>> is a pure int you'd end up in the non pure int code path below. Maybe
>> would be easier if just iterating over all fbs in the outermost loop and
>> doing that decision per fb (this only worked because mixed int/fixed fbs
>> weren't allowed, but it should be quite ok deciding that per fb anyway).
> 
> OK, I'll put in a loop.
> 
> 
>>
>>>/*
>>> * We expect int/uint clear values here, though some APIs
>>> * might disagree (but in any case util_pack_color()
>>> @@ -174,20 +175,22 @@ lp_rast_clear_color(struct lp_rasterizer_task
>>> *task,
>>>   clear_color[3]);
>>>
>>>for (i = 0; i < scene->fb.nr_cbufs; i++) {
>>> -util_pack_color(arg.clear_color.f,
>>> -scene->fb.cbufs[i]->format, &uc);
>>> -
>>> -util_fill_box(scene->cbufs[i].map,
>>> -  scene->fb.cbufs[i]->format,
>>> -  scene->cbufs[i].stride,
>>> -  scene->cbufs[i].layer_stride,
>>> -  task->x,
>>> -  task->y,
>>> -  0,
>>> -  task->width,
>>> -  task->height,
>>> -  scene->fb_max_layer + 1,
>>> -  &uc);
>>> +if (scene->fb.cbufs[i]) {
>>> +   util_pack_color(arg.clear_color.f,
>>> +   scene->fb.cbufs[i]->format, &uc);
>>> +
>>> +   util_fill_box(scene->cbufs[i].map,
>>> + scene->fb.cbufs[i]->format,
>>> + scene->cbufs[i].stride,
>>> + scene->cbufs[i].layer_stride,
>>> + task->x,
>>> + task->y,
>>> + 0,
>>> + task->width,
>>> + task->height,
>>> + scene->fb_max_layer + 1,
>>> + &uc);
>>> +}
>>>}
>>> }
>>>  }
>>> @@ -444,8 +447,15 @@ lp_rast_shade_quads_mask(struct
>>> lp_rasterizer_task *task,
>>>
>>>  /* color buffer */
>>>  for (i = 0; i < scene->fb.nr_cbufs; i++) {
>>> -  stride[i] = scene->cbufs[i].stride;
>>> -  color[i] = lp_rast_get_unswizzled_color_block_pointer(task, i,
>>> x, y, inputs->layer);
>>> +  if (scene->fb.cbufs[i]) {
>>> + stride[i] = scene->cbufs[i].stride;
>>> + color[i] = lp_rast_get_unswizzled_color_block_pointer(task,
>>> i, x, y,
>>> +  
>>> inputs->layer);
>>> +  }
>>> +  else {
>>> + stride[i] = 0;
>>> + color[i] = NULL;
>>> +  }
>>>  }
>>>
>>>  /* depth buffer */
>>> diff --git a/src/gallium/drivers/llvmpipe/lp_rast_priv.h
>>> b/src/gallium/drivers/llvmpipe/lp_rast_priv.h
>>> index bc361b6..063a70e 100644
>>> --- a/src/gallium/drivers/llvmpipe/lp_rast_priv.h
>>> +++ b/src/gallium/drivers/llvmpipe/lp_rast_priv.h
>>> @@ -293,8 +293,15 @@ lp_rast_shade_quads_all( struct
>>> lp_rasterizer_task *task,
>>>
>>>  /* color buffer */
>>>  for (i = 0; i < scene->fb.nr_cbufs; i++) {
>>> -  stride[i] = scene->cbufs[i].stride;
>>> -  color[i] = lp_rast_get_unswizzled_color_block_pointer(task, i,
>>> x, y, inputs->layer);
>>> +  if (scene->fb.cbufs[i]) {
>>> + stride[i] = scene->cbufs[i].stride;
>>> + color[i] = lp_rast_get_unswizzled_color_block_pointer(task,
>>> i, x, y,
>>> +  
>>> inputs->layer);
>>>

Re: [Mesa-dev] [PATCH] mesa: fix GL_COLOR_SUM enum for drivers without ARB_vertex_program

[Ian Romanick]

You're right.  I really botched that. :( The changes in enable.c look
correct, but get_hash_params.py needs the same treatment.

On 01/10/2014 11:57 AM, Ilia Mirkin wrote:
> Commit c13970808 (mesa: GL_EXT_secondary_color is not optional) changed
> 
> CHECK_EXTENSION2(EXT_secondary_color, ARB_vetex_program, cap)
> 
> to
> 
> CHECK_EXTENSION(ARB_vertex_program, cap)
> 
> However CHECK_EXTENSION2 checks that either extension is available, not
> both. Remove the extension check entirely since the intent was for it to
> always be enabled.
> 
> Signed-off-by: Ilia Mirkin 
> Cc: 9.2 10.0 
> ---
> 
> Someone mentioned that nouveau (dri) was getting errors with
> glDisable(GL_COLOR_SUM), even though the driver should have had support for
> it.
> 
>  src/mesa/main/enable.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/src/mesa/main/enable.c b/src/mesa/main/enable.c
> index bb4a23c..fca3068 100644
> --- a/src/mesa/main/enable.c
> +++ b/src/mesa/main/enable.c
> @@ -762,7 +762,6 @@ _mesa_set_enable(struct gl_context *ctx, GLenum cap, 
> GLboolean state)
>case GL_COLOR_SUM_EXT:
>   if (ctx->API != API_OPENGL_COMPAT)
>  goto invalid_enum_error;
> - CHECK_EXTENSION(ARB_vertex_program, cap);
>   if (ctx->Fog.ColorSumEnabled == state)
>  return;
>   FLUSH_VERTICES(ctx, _NEW_FOG);
> @@ -1443,7 +1442,6 @@ _mesa_IsEnabled( GLenum cap )
>case GL_COLOR_SUM_EXT:
>   if (ctx->API != API_OPENGL_COMPAT)
>  goto invalid_enum_error;
> - CHECK_EXTENSION(ARB_vertex_program);
>   return ctx->Fog.ColorSumEnabled;
>  
>/* GL_ARB_multisample */
> 

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

Re: [Mesa-dev] [PATCH] llvmpipe: handle NULL color buffer pointers

[Roland Scheidegger]

Am 16.01.2014 17:20, schrieb Brian Paul:
> Fixes regression from 9baa45f78b8ca7d66280e36009b6a685055d7cd6
> 
> v2: incorporate a few small changes suggested by Roland.
> ---
>  src/gallium/drivers/llvmpipe/lp_rast.c  |   62 ---
>  src/gallium/drivers/llvmpipe/lp_rast_priv.h |   11 +-
>  src/gallium/drivers/llvmpipe/lp_scene.c |   23 ++--
>  src/gallium/drivers/llvmpipe/lp_setup.c |2 +-
>  src/gallium/drivers/llvmpipe/lp_state_fs.c  |  152 
> +++
>  5 files changed, 156 insertions(+), 94 deletions(-)
> 
> diff --git a/src/gallium/drivers/llvmpipe/lp_rast.c 
> b/src/gallium/drivers/llvmpipe/lp_rast.c
> index 6feec94..6ee849b 100644
> --- a/src/gallium/drivers/llvmpipe/lp_rast.c
> +++ b/src/gallium/drivers/llvmpipe/lp_rast.c
> @@ -110,6 +110,25 @@ lp_rast_tile_begin(struct lp_rasterizer_task *task,
>  
>  
>  /**
> + * Examine a framebuffer object to determine if any of the colorbuffers
> + * use a pure integer format.
> + * XXX this could be a gallium utility function if useful elsewhere.
> + */
> +static boolean
> +is_fb_pure_integer(const struct pipe_framebuffer_state *fb)
> +{
> +   unsigned i;
> +   for (i = 0; i < fb->nr_cbufs; i++) {
> +  if (fb->cbufs[i] &&
> +  util_format_is_pure_integer(fb->cbufs[i]->format)) {
> + return TRUE;
> +  }
> +   }
> +   return FALSE;
> +}
> +
> +
> +/**
>   * Clear the rasterizer's current color tile.
>   * This is a bin command called during bin processing.
>   * Clear commands always clear all bound layers.
> @@ -124,7 +143,7 @@ lp_rast_clear_color(struct lp_rasterizer_task *task,
>unsigned i;
>union util_color uc;
>  
> -  if (util_format_is_pure_integer(scene->fb.cbufs[0]->format)) {
> +  if (is_fb_pure_integer(&scene->fb)) {
>   /*
>* We expect int/uint clear values here, though some APIs
>* might disagree (but in any case util_pack_color()
> @@ -174,20 +193,22 @@ lp_rast_clear_color(struct lp_rasterizer_task *task,
>  clear_color[3]);
>  
>   for (i = 0; i < scene->fb.nr_cbufs; i++) {
> -util_pack_color(arg.clear_color.f,
> -scene->fb.cbufs[i]->format, &uc);
> -
> -util_fill_box(scene->cbufs[i].map,
> -  scene->fb.cbufs[i]->format,
> -  scene->cbufs[i].stride,
> -  scene->cbufs[i].layer_stride,
> -  task->x,
> -  task->y,
> -  0,
> -  task->width,
> -  task->height,
> -  scene->fb_max_layer + 1,
> -  &uc);
> +if (scene->fb.cbufs[i]) {
> +   util_pack_color(arg.clear_color.f,
> +   scene->fb.cbufs[i]->format, &uc);
> +
> +   util_fill_box(scene->cbufs[i].map,
> + scene->fb.cbufs[i]->format,
> + scene->cbufs[i].stride,
> + scene->cbufs[i].layer_stride,
> + task->x,
> + task->y,
> + 0,
> + task->width,
> + task->height,
> + scene->fb_max_layer + 1,
> + &uc);
> +}
>   }
>}
> }
> @@ -444,8 +465,15 @@ lp_rast_shade_quads_mask(struct lp_rasterizer_task *task,
>  
> /* color buffer */
> for (i = 0; i < scene->fb.nr_cbufs; i++) {
> -  stride[i] = scene->cbufs[i].stride;
> -  color[i] = lp_rast_get_unswizzled_color_block_pointer(task, i, x, y, 
> inputs->layer);
> +  if (scene->fb.cbufs[i]) {
> + stride[i] = scene->cbufs[i].stride;
> + color[i] = lp_rast_get_unswizzled_color_block_pointer(task, i, x, y,
> +   
> inputs->layer);
> +  }
> +  else {
> + stride[i] = 0;
> + color[i] = NULL;
> +  }
> }
>  
> /* depth buffer */
> diff --git a/src/gallium/drivers/llvmpipe/lp_rast_priv.h 
> b/src/gallium/drivers/llvmpipe/lp_rast_priv.h
> index bc361b6..063a70e 100644
> --- a/src/gallium/drivers/llvmpipe/lp_rast_priv.h
> +++ b/src/gallium/drivers/llvmpipe/lp_rast_priv.h
> @@ -293,8 +293,15 @@ lp_rast_shade_quads_all( struct lp_rasterizer_task *task,
>  
> /* color buffer */
> for (i = 0; i < scene->fb.nr_cbufs; i++) {
> -  stride[i] = scene->cbufs[i].stride;
> -  color[i] = lp_rast_get_unswizzled_color_block_pointer(task, i, x, y, 
> inputs->layer);
> +  if (scene->fb.cbufs[i]) {
> + stride[i] = scene->cbufs[i].stride;
> + color[i] = lp_rast_get_unswizzled_color_block_pointer(task, i, x, y,
> +   
> inputs->layer);
> +  }
> +  else {
> +   

[Mesa-dev] [PATCH v2] mesa: fix GL_COLOR_SUM enum for drivers without ARB_vertex_program

[Ilia Mirkin]

Commit c13970808 (mesa: GL_EXT_secondary_color is not optional) changed

CHECK_EXTENSION2(EXT_secondary_color, ARB_vetex_program, cap)

to

CHECK_EXTENSION(ARB_vertex_program, cap)

However CHECK_EXTENSION2 checks that either extension is available, not
both. Remove the extension check entirely since the intent was for it to
always be enabled.

Signed-off-by: Ilia Mirkin 
Cc: 9.2 10.0 
---

I hope this is what you meant by the get_hash_params.py change. I'm not
entirely clear on when one should use NO_EXTRA vs a flush of some sort.

 src/mesa/main/enable.c   | 2 --
 src/mesa/main/get_hash_params.py | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/mesa/main/enable.c b/src/mesa/main/enable.c
index bb4a23c..fca3068 100644
--- a/src/mesa/main/enable.c
+++ b/src/mesa/main/enable.c
@@ -762,7 +762,6 @@ _mesa_set_enable(struct gl_context *ctx, GLenum cap, 
GLboolean state)
   case GL_COLOR_SUM_EXT:
  if (ctx->API != API_OPENGL_COMPAT)
 goto invalid_enum_error;
- CHECK_EXTENSION(ARB_vertex_program, cap);
  if (ctx->Fog.ColorSumEnabled == state)
 return;
  FLUSH_VERTICES(ctx, _NEW_FOG);
@@ -1443,7 +1442,6 @@ _mesa_IsEnabled( GLenum cap )
   case GL_COLOR_SUM_EXT:
  if (ctx->API != API_OPENGL_COMPAT)
 goto invalid_enum_error;
- CHECK_EXTENSION(ARB_vertex_program);
  return ctx->Fog.ColorSumEnabled;
 
   /* GL_ARB_multisample */
diff --git a/src/mesa/main/get_hash_params.py b/src/mesa/main/get_hash_params.py
index 6195d63..bc2bbaf 100644
--- a/src/mesa/main/get_hash_params.py
+++ b/src/mesa/main/get_hash_params.py
@@ -541,7 +541,7 @@ descriptor=[
   [ "TRANSPOSE_TEXTURE_MATRIX_ARB", "CONTEXT_MATRIX_T(TextureMatrixStack), 
NO_EXTRA" ],
 
 # GL_EXT_secondary_color
-  [ "COLOR_SUM", "CONTEXT_BOOL(Fog.ColorSumEnabled), extra_ARB_vertex_program" 
],
+  [ "COLOR_SUM", "CONTEXT_BOOL(Fog.ColorSumEnabled), NO_EXTRA" ],
   [ "CURRENT_SECONDARY_COLOR", 
"CONTEXT_FIELD(Current.Attrib[VERT_ATTRIB_COLOR1][0], TYPE_FLOATN_4), 
extra_flush_current" ],
   [ "SECONDARY_COLOR_ARRAY", 
"ARRAY_BOOL(VertexAttrib[VERT_ATTRIB_COLOR1].Enabled), NO_EXTRA" ],
   [ "SECONDARY_COLOR_ARRAY_TYPE", 
"ARRAY_ENUM(VertexAttrib[VERT_ATTRIB_COLOR1].Type), NO_EXTRA" ],
-- 
1.8.3.2

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

Re: [Mesa-dev] [Mesa PATCH 2/3] i965: Use the new drm_intel_bo offset64 field.

[Ian Romanick]

On 01/13/2014 03:56 PM, Kenneth Graunke wrote:
> libdrm 2.4.52 introduces a new 'uint64_t offset64' field, intended to
> replace the old 'unsigned long offset' field.  To preserve ABI, libdrm
> continues to store the presumed offset in both locations.
> 
> On Broadwell, a 64-bit kernel may place BOs at "high" (> 4G) addresses.
> However, with a 32-bit userspace, the 'unsigned long offset' field will
> only be 32-bit, which is not large enough to hold this value.  We need
> to use a proper uint64_t (like the kernel does).
> 
> Technically, a lot of this code doesn't affect Broadwell, so we could
> leave it using the old field.  But it makes sense to just switch to the
> new, properly typed field.
> 
> Signed-off-by: Kenneth Graunke 
> ---
>  configure.ac  |  2 +-
>  src/mesa/drivers/dri/i965/brw_cc.c|  2 +-
>  src/mesa/drivers/dri/i965/brw_clip_state.c|  2 +-
>  src/mesa/drivers/dri/i965/brw_context.h   |  2 +-
>  src/mesa/drivers/dri/i965/brw_sf_state.c  |  2 +-
>  src/mesa/drivers/dri/i965/brw_vs_state.c  |  4 ++--
>  src/mesa/drivers/dri/i965/brw_wm_sampler_state.c  |  2 +-
>  src/mesa/drivers/dri/i965/brw_wm_state.c  |  4 ++--
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c  | 14 +++---
>  src/mesa/drivers/dri/i965/gen6_blorp.cpp  |  4 ++--
>  src/mesa/drivers/dri/i965/gen7_blorp.cpp  |  4 ++--
>  src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 14 +++---
>  src/mesa/drivers/dri/i965/intel_batchbuffer.c |  6 +++---
>  13 files changed, 31 insertions(+), 31 deletions(-)
> 
> This was generated by temporarily removing the 'offset' field from libdrm
> and fixing all the compile errors.  Obviously, we can't actually delete the
> field, but you can at least have some confidence that I caught all the
> existing uses.

Alternately, we could use GCC extensions to mark the field as
deprecated.  Then any uses of the field will generate a warning.  This
will prevent new uses from accidentally creeping in.  It would, however,
cause spurious warnings if someone uses new libdrm with old Mesa (e.g.,
10.0.x stable branch).

Either way, all 4 patches are

Reviewed-by: Ian Romanick 

> diff --git a/configure.ac b/configure.ac
> index 4b55140..fd189ea 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -29,7 +29,7 @@ AC_SUBST([OSMESA_VERSION])
>  dnl Versions for external dependencies
>  LIBDRM_REQUIRED=2.4.24
>  LIBDRM_RADEON_REQUIRED=2.4.50
> -LIBDRM_INTEL_REQUIRED=2.4.49
> +LIBDRM_INTEL_REQUIRED=2.4.52
>  LIBDRM_NVVIEUX_REQUIRED=2.4.33
>  LIBDRM_NOUVEAU_REQUIRED="2.4.33 libdrm >= 2.4.41"
>  LIBDRM_FREEDRENO_REQUIRED=2.4.51
> diff --git a/src/mesa/drivers/dri/i965/brw_cc.c 
> b/src/mesa/drivers/dri/i965/brw_cc.c
> index 4bc3b23..497d91a 100644
> --- a/src/mesa/drivers/dri/i965/brw_cc.c
> +++ b/src/mesa/drivers/dri/i965/brw_cc.c
> @@ -215,7 +215,7 @@ static void upload_cc_unit(struct brw_context *brw)
>cc->cc5.statistics_enable = 1;
>  
> /* CACHE_NEW_CC_VP */
> -   cc->cc4.cc_viewport_state_offset = (brw->batch.bo->offset +
> +   cc->cc4.cc_viewport_state_offset = (brw->batch.bo->offset64 +
>  brw->cc.vp_offset) >> 5; /* reloc */
>  
> brw->state.dirty.cache |= CACHE_NEW_CC_UNIT;
> diff --git a/src/mesa/drivers/dri/i965/brw_clip_state.c 
> b/src/mesa/drivers/dri/i965/brw_clip_state.c
> index 66b3229..8647b0d 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip_state.c
> @@ -132,7 +132,7 @@ brw_upload_clip_unit(struct brw_context *brw)
> {
>clip->clip5.guard_band_enable = 1;
>clip->clip6.clipper_viewport_state_ptr =
> - (brw->batch.bo->offset + brw->clip.vp_offset) >> 5;
> + (brw->batch.bo->offset64 + brw->clip.vp_offset) >> 5;
>  
>/* emit clip viewport relocation */
>drm_intel_bo_emit_reloc(brw->batch.bo,
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
> b/src/mesa/drivers/dri/i965/brw_context.h
> index 63dd4a0..77c4c3e 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -1809,7 +1809,7 @@ brw_program_reloc(struct brw_context *brw, uint32_t 
> state_offset,
>  prog_offset,
>  I915_GEM_DOMAIN_INSTRUCTION, 0);
>  
> -   return brw->cache.bo->offset + prog_offset;
> +   return brw->cache.bo->offset64 + prog_offset;
>  }
>  
>  bool brw_do_cubemap_normalize(struct exec_list *instructions);
> diff --git a/src/mesa/drivers/dri/i965/brw_sf_state.c 
> b/src/mesa/drivers/dri/i965/brw_sf_state.c
> index 69093f2..9bc0cd3 100644
> --- a/src/mesa/drivers/dri/i965/brw_sf_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_sf_state.c
> @@ -173,7 +173,7 @@ static void upload_sf_unit( struct brw_context *brw )
>sf->thread4.stats_enable = 1;
>  
> /* CACHE_NEW_SF_VP */
> -   sf->sf5.sf_viewport_state_offset = (brw->batch.bo->offset +
> +

Re: [Mesa-dev] [Mesa-stable] [PATCH v2] mesa: fix GL_COLOR_SUM enum for drivers without ARB_vertex_program

[Ian Romanick]

On 01/16/2014 10:13 AM, Ilia Mirkin wrote:
> Commit c13970808 (mesa: GL_EXT_secondary_color is not optional) changed
> 
> CHECK_EXTENSION2(EXT_secondary_color, ARB_vetex_program, cap)
> 
> to
> 
> CHECK_EXTENSION(ARB_vertex_program, cap)
> 
> However CHECK_EXTENSION2 checks that either extension is available, not
> both. Remove the extension check entirely since the intent was for it to
> always be enabled.
> 
> Signed-off-by: Ilia Mirkin 
> Cc: 9.2 10.0 

Reviewed-by: Ian Romanick 

> ---
> 
> I hope this is what you meant by the get_hash_params.py change. I'm not
> entirely clear on when one should use NO_EXTRA vs a flush of some sort.

The various flushes are only needed for queries of derived state (like
the current secondary color query).

>  src/mesa/main/enable.c   | 2 --
>  src/mesa/main/get_hash_params.py | 2 +-
>  2 files changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/src/mesa/main/enable.c b/src/mesa/main/enable.c
> index bb4a23c..fca3068 100644
> --- a/src/mesa/main/enable.c
> +++ b/src/mesa/main/enable.c
> @@ -762,7 +762,6 @@ _mesa_set_enable(struct gl_context *ctx, GLenum cap, 
> GLboolean state)
>case GL_COLOR_SUM_EXT:
>   if (ctx->API != API_OPENGL_COMPAT)
>  goto invalid_enum_error;
> - CHECK_EXTENSION(ARB_vertex_program, cap);
>   if (ctx->Fog.ColorSumEnabled == state)
>  return;
>   FLUSH_VERTICES(ctx, _NEW_FOG);
> @@ -1443,7 +1442,6 @@ _mesa_IsEnabled( GLenum cap )
>case GL_COLOR_SUM_EXT:
>   if (ctx->API != API_OPENGL_COMPAT)
>  goto invalid_enum_error;
> - CHECK_EXTENSION(ARB_vertex_program);
>   return ctx->Fog.ColorSumEnabled;
>  
>/* GL_ARB_multisample */
> diff --git a/src/mesa/main/get_hash_params.py 
> b/src/mesa/main/get_hash_params.py
> index 6195d63..bc2bbaf 100644
> --- a/src/mesa/main/get_hash_params.py
> +++ b/src/mesa/main/get_hash_params.py
> @@ -541,7 +541,7 @@ descriptor=[
>[ "TRANSPOSE_TEXTURE_MATRIX_ARB", "CONTEXT_MATRIX_T(TextureMatrixStack), 
> NO_EXTRA" ],
>  
>  # GL_EXT_secondary_color
> -  [ "COLOR_SUM", "CONTEXT_BOOL(Fog.ColorSumEnabled), 
> extra_ARB_vertex_program" ],
> +  [ "COLOR_SUM", "CONTEXT_BOOL(Fog.ColorSumEnabled), NO_EXTRA" ],
>[ "CURRENT_SECONDARY_COLOR", 
> "CONTEXT_FIELD(Current.Attrib[VERT_ATTRIB_COLOR1][0], TYPE_FLOATN_4), 
> extra_flush_current" ],
>[ "SECONDARY_COLOR_ARRAY", 
> "ARRAY_BOOL(VertexAttrib[VERT_ATTRIB_COLOR1].Enabled), NO_EXTRA" ],
>[ "SECONDARY_COLOR_ARRAY_TYPE", 
> "ARRAY_ENUM(VertexAttrib[VERT_ATTRIB_COLOR1].Type), NO_EXTRA" ],
> 

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

Re: [Mesa-dev] [Mesa-stable] [PATCH v2] mesa: fix GL_COLOR_SUM enum for drivers without ARB_vertex_program

[Ilia Mirkin]

On Thu, Jan 16, 2014 at 1:34 PM, Ian Romanick  wrote:
> On 01/16/2014 10:13 AM, Ilia Mirkin wrote:
>> Commit c13970808 (mesa: GL_EXT_secondary_color is not optional) changed
>>
>> CHECK_EXTENSION2(EXT_secondary_color, ARB_vetex_program, cap)
>>
>> to
>>
>> CHECK_EXTENSION(ARB_vertex_program, cap)
>>
>> However CHECK_EXTENSION2 checks that either extension is available, not
>> both. Remove the extension check entirely since the intent was for it to
>> always be enabled.
>>
>> Signed-off-by: Ilia Mirkin 
>> Cc: 9.2 10.0 
>
> Reviewed-by: Ian Romanick 

Thanks! Can you take care of checking it in? I don't have an fd.o account.

>
>> ---
>>
>> I hope this is what you meant by the get_hash_params.py change. I'm not
>> entirely clear on when one should use NO_EXTRA vs a flush of some sort.
>
> The various flushes are only needed for queries of derived state (like
> the current secondary color query).
>
>>  src/mesa/main/enable.c   | 2 --
>>  src/mesa/main/get_hash_params.py | 2 +-
>>  2 files changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/src/mesa/main/enable.c b/src/mesa/main/enable.c
>> index bb4a23c..fca3068 100644
>> --- a/src/mesa/main/enable.c
>> +++ b/src/mesa/main/enable.c
>> @@ -762,7 +762,6 @@ _mesa_set_enable(struct gl_context *ctx, GLenum cap, 
>> GLboolean state)
>>case GL_COLOR_SUM_EXT:
>>   if (ctx->API != API_OPENGL_COMPAT)
>>  goto invalid_enum_error;
>> - CHECK_EXTENSION(ARB_vertex_program, cap);
>>   if (ctx->Fog.ColorSumEnabled == state)
>>  return;
>>   FLUSH_VERTICES(ctx, _NEW_FOG);
>> @@ -1443,7 +1442,6 @@ _mesa_IsEnabled( GLenum cap )
>>case GL_COLOR_SUM_EXT:
>>   if (ctx->API != API_OPENGL_COMPAT)
>>  goto invalid_enum_error;
>> - CHECK_EXTENSION(ARB_vertex_program);
>>   return ctx->Fog.ColorSumEnabled;
>>
>>/* GL_ARB_multisample */
>> diff --git a/src/mesa/main/get_hash_params.py 
>> b/src/mesa/main/get_hash_params.py
>> index 6195d63..bc2bbaf 100644
>> --- a/src/mesa/main/get_hash_params.py
>> +++ b/src/mesa/main/get_hash_params.py
>> @@ -541,7 +541,7 @@ descriptor=[
>>[ "TRANSPOSE_TEXTURE_MATRIX_ARB", "CONTEXT_MATRIX_T(TextureMatrixStack), 
>> NO_EXTRA" ],
>>
>>  # GL_EXT_secondary_color
>> -  [ "COLOR_SUM", "CONTEXT_BOOL(Fog.ColorSumEnabled), 
>> extra_ARB_vertex_program" ],
>> +  [ "COLOR_SUM", "CONTEXT_BOOL(Fog.ColorSumEnabled), NO_EXTRA" ],
>>[ "CURRENT_SECONDARY_COLOR", 
>> "CONTEXT_FIELD(Current.Attrib[VERT_ATTRIB_COLOR1][0], TYPE_FLOATN_4), 
>> extra_flush_current" ],
>>[ "SECONDARY_COLOR_ARRAY", 
>> "ARRAY_BOOL(VertexAttrib[VERT_ATTRIB_COLOR1].Enabled), NO_EXTRA" ],
>>[ "SECONDARY_COLOR_ARRAY_TYPE", 
>> "ARRAY_ENUM(VertexAttrib[VERT_ATTRIB_COLOR1].Type), NO_EXTRA" ],
>>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [Mesa-stable] [PATCH v2] mesa: fix GL_COLOR_SUM enum for drivers without ARB_vertex_program

[Ilia Mirkin]

On Thu, Jan 16, 2014 at 1:38 PM, Ilia Mirkin  wrote:
> On Thu, Jan 16, 2014 at 1:34 PM, Ian Romanick  wrote:
>> On 01/16/2014 10:13 AM, Ilia Mirkin wrote:
>>> Commit c13970808 (mesa: GL_EXT_secondary_color is not optional) changed
>>>
>>> CHECK_EXTENSION2(EXT_secondary_color, ARB_vetex_program, cap)
>>>
>>> to
>>>
>>> CHECK_EXTENSION(ARB_vertex_program, cap)
>>>
>>> However CHECK_EXTENSION2 checks that either extension is available, not
>>> both. Remove the extension check entirely since the intent was for it to
>>> always be enabled.
>>>
>>> Signed-off-by: Ilia Mirkin 
>>> Cc: 9.2 10.0 
>>
>> Reviewed-by: Ian Romanick 
>
> Thanks! Can you take care of checking it in? I don't have an fd.o account.
>
>>
>>> ---
>>>
>>> I hope this is what you meant by the get_hash_params.py change. I'm not
>>> entirely clear on when one should use NO_EXTRA vs a flush of some sort.
>>
>> The various flushes are only needed for queries of derived state (like
>> the current secondary color query).

Actually if the extra_ARB_vertex_program was needed before -- wouldn't
it still be needed now? Presumably it only does work if that extension
exists, and GL_COLOR_SUM somehow interacts with it. (Sorry, my
knowledge of the details is rather weak.)

>>
>>>  src/mesa/main/enable.c   | 2 --
>>>  src/mesa/main/get_hash_params.py | 2 +-
>>>  2 files changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/src/mesa/main/enable.c b/src/mesa/main/enable.c
>>> index bb4a23c..fca3068 100644
>>> --- a/src/mesa/main/enable.c
>>> +++ b/src/mesa/main/enable.c
>>> @@ -762,7 +762,6 @@ _mesa_set_enable(struct gl_context *ctx, GLenum cap, 
>>> GLboolean state)
>>>case GL_COLOR_SUM_EXT:
>>>   if (ctx->API != API_OPENGL_COMPAT)
>>>  goto invalid_enum_error;
>>> - CHECK_EXTENSION(ARB_vertex_program, cap);
>>>   if (ctx->Fog.ColorSumEnabled == state)
>>>  return;
>>>   FLUSH_VERTICES(ctx, _NEW_FOG);
>>> @@ -1443,7 +1442,6 @@ _mesa_IsEnabled( GLenum cap )
>>>case GL_COLOR_SUM_EXT:
>>>   if (ctx->API != API_OPENGL_COMPAT)
>>>  goto invalid_enum_error;
>>> - CHECK_EXTENSION(ARB_vertex_program);
>>>   return ctx->Fog.ColorSumEnabled;
>>>
>>>/* GL_ARB_multisample */
>>> diff --git a/src/mesa/main/get_hash_params.py 
>>> b/src/mesa/main/get_hash_params.py
>>> index 6195d63..bc2bbaf 100644
>>> --- a/src/mesa/main/get_hash_params.py
>>> +++ b/src/mesa/main/get_hash_params.py
>>> @@ -541,7 +541,7 @@ descriptor=[
>>>[ "TRANSPOSE_TEXTURE_MATRIX_ARB", "CONTEXT_MATRIX_T(TextureMatrixStack), 
>>> NO_EXTRA" ],
>>>
>>>  # GL_EXT_secondary_color
>>> -  [ "COLOR_SUM", "CONTEXT_BOOL(Fog.ColorSumEnabled), 
>>> extra_ARB_vertex_program" ],
>>> +  [ "COLOR_SUM", "CONTEXT_BOOL(Fog.ColorSumEnabled), NO_EXTRA" ],
>>>[ "CURRENT_SECONDARY_COLOR", 
>>> "CONTEXT_FIELD(Current.Attrib[VERT_ATTRIB_COLOR1][0], TYPE_FLOATN_4), 
>>> extra_flush_current" ],
>>>[ "SECONDARY_COLOR_ARRAY", 
>>> "ARRAY_BOOL(VertexAttrib[VERT_ATTRIB_COLOR1].Enabled), NO_EXTRA" ],
>>>[ "SECONDARY_COLOR_ARRAY_TYPE", 
>>> "ARRAY_ENUM(VertexAttrib[VERT_ATTRIB_COLOR1].Type), NO_EXTRA" ],
>>>
>>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 04/10] nv50: typecast the result of ffs() to unsigned

[Emil Velikov]

Signed-off-by: Emil Velikov 
---
 src/gallium/drivers/nouveau/nv50/nv50_shader_state.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c 
b/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c
index c44d208..c9d80ea 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c
@@ -46,7 +46,7 @@ nv50_constbufs_validate(struct nv50_context *nv50)
  p = NV50_3D_SET_PROGRAM_CB_PROGRAM_VERTEX;
 
   while (nv50->constbuf_dirty[s]) {
- const int i = ffs(nv50->constbuf_dirty[s]) - 1;
+ const unsigned i = (unsigned)ffs(nv50->constbuf_dirty[s]) - 1;
 
  assert(i < NV50_MAX_PIPE_CONSTBUFS);
  nv50->constbuf_dirty[s] &= ~(1 << i);
-- 
1.8.5.2

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

[Mesa-dev] [PATCH 07/10] nv50: assert before trying to out-of-bounds access textures

[Emil Velikov]

Signed-off-by: Emil Velikov 
---
 src/gallium/drivers/nouveau/nv50/nv50_context.c | 2 ++
 src/gallium/drivers/nouveau/nv50/nv50_state.c   | 2 ++
 src/gallium/drivers/nouveau/nv50/nv50_tex.c | 1 +
 3 files changed, 5 insertions(+)

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_context.c 
b/src/gallium/drivers/nouveau/nv50/nv50_context.c
index a4ec93a..8183b01 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_context.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_context.c
@@ -87,6 +87,7 @@ nv50_context_unreference_resources(struct nv50_context *nv50)
pipe_resource_reference(&nv50->idxbuf.buffer, NULL);
 
for (s = 0; s < 3; ++s) {
+  assert(nv50->num_textures[s] <= PIPE_MAX_SAMPLERS);
   for (i = 0; i < nv50->num_textures[s]; ++i)
  pipe_sampler_view_reference(&nv50->textures[s][i], NULL);
 
@@ -168,6 +169,7 @@ nv50_invalidate_resource_storage(struct nouveau_context 
*ctx,
 
if (res->bind & PIPE_BIND_SAMPLER_VIEW) {
   for (s = 0; s < 3; ++s) {
+  assert(nv50->num_textures[s] <= PIPE_MAX_SAMPLERS);
   for (i = 0; i < nv50->num_textures[s]; ++i) {
  if (nv50->textures[s][i] &&
  nv50->textures[s][i]->texture == res) {
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_state.c 
b/src/gallium/drivers/nouveau/nv50/nv50_state.c
index 5488cac..b07856c 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_state.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_state.c
@@ -646,6 +646,7 @@ nv50_stage_set_sampler_views(struct nv50_context *nv50, int 
s,
 {
unsigned i;
 
+   assert(nr <= PIPE_MAX_SAMPLERS);
for (i = 0; i < nr; ++i) {
   struct nv50_tic_entry *old = nv50_tic_entry(nv50->textures[s][i]);
   if (old)
@@ -654,6 +655,7 @@ nv50_stage_set_sampler_views(struct nv50_context *nv50, int 
s,
   pipe_sampler_view_reference(&nv50->textures[s][i], views[i]);
}
 
+   assert(nv50->num_textures[s] <= PIPE_MAX_SAMPLERS);
for (i = nr; i < nv50->num_textures[s]; ++i) {
   struct nv50_tic_entry *old = nv50_tic_entry(nv50->textures[s][i]);
   if (!old)
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_tex.c 
b/src/gallium/drivers/nouveau/nv50/nv50_tex.c
index f7284fa..f2325cf 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_tex.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_tex.c
@@ -222,6 +222,7 @@ nv50_validate_tic(struct nv50_context *nv50, int s)
unsigned i;
boolean need_flush = FALSE;
 
+   assert(nv50->num_textures[s] <= PIPE_MAX_SAMPLERS);
for (i = 0; i < nv50->num_textures[s]; ++i) {
   struct nv50_tic_entry *tic = nv50_tic_entry(nv50->textures[s][i]);
   struct nv04_resource *res;
-- 
1.8.5.2

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

[Mesa-dev] [PATCH 02/10] nv50: access only the available amount of constbuf

[Emil Velikov]

The textures array is defined as a number of NV50_MAX_PIPE_CONSTBUFS
per shader stage. Currently the nv50 driver handles only 3 shader
stages, thus we wreck chaos when accessing array-out-of-bounds.

Cc: 9.1 9.2 10.0 
Signed-off-by: Emil Velikov 
---
 src/gallium/drivers/nouveau/nv50/nv50_context.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_context.c 
b/src/gallium/drivers/nouveau/nv50/nv50_context.c
index eb345dc..bd00b50 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_context.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_context.c
@@ -179,7 +179,7 @@ nv50_invalidate_resource_storage(struct nouveau_context 
*ctx,
}
 
if (res->bind & PIPE_BIND_CONSTANT_BUFFER) {
-  for (s = 0; s < 5; ++s) {
+  for (s = 0; s < 3; ++s) {
   for (i = 0; i < nv50->num_vtxbufs; ++i) {
  if (!nv50->constbuf[s][i].user &&
  nv50->constbuf[s][i].u.buf == res) {
-- 
1.8.5.2

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

[Mesa-dev] [PATCH 03/10] nv50: assert before trying to out-of-bounds access constbuf

[Emil Velikov]

Signed-off-by: Emil Velikov 
---
 src/gallium/drivers/nouveau/nv50/nv50_context.c  | 1 +
 src/gallium/drivers/nouveau/nv50/nv50_shader_state.c | 2 ++
 src/gallium/drivers/nouveau/nv50/nv50_state.c| 1 +
 3 files changed, 4 insertions(+)

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_context.c 
b/src/gallium/drivers/nouveau/nv50/nv50_context.c
index bd00b50..9ea425e 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_context.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_context.c
@@ -180,6 +180,7 @@ nv50_invalidate_resource_storage(struct nouveau_context 
*ctx,
 
if (res->bind & PIPE_BIND_CONSTANT_BUFFER) {
   for (s = 0; s < 3; ++s) {
+  assert(nv50->num_vtxbufs <= NV50_MAX_PIPE_CONSTBUFS);
   for (i = 0; i < nv50->num_vtxbufs; ++i) {
  if (!nv50->constbuf[s][i].user &&
  nv50->constbuf[s][i].u.buf == res) {
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c 
b/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c
index 9144fc4..c44d208 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c
@@ -47,6 +47,8 @@ nv50_constbufs_validate(struct nv50_context *nv50)
 
   while (nv50->constbuf_dirty[s]) {
  const int i = ffs(nv50->constbuf_dirty[s]) - 1;
+
+ assert(i < NV50_MAX_PIPE_CONSTBUFS);
  nv50->constbuf_dirty[s] &= ~(1 << i);
 
  if (nv50->constbuf[s][i].user) {
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_state.c 
b/src/gallium/drivers/nouveau/nv50/nv50_state.c
index b6a180e..5488cac 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_state.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_state.c
@@ -786,6 +786,7 @@ nv50_set_constant_buffer(struct pipe_context *pipe, uint 
shader, uint index,
if (shader == PIPE_SHADER_COMPUTE)
   return;
 
+   assert(i < NV50_MAX_PIPE_CONSTBUFS);
if (nv50->constbuf[s][i].user)
   nv50->constbuf[s][i].u.buf = NULL;
else
-- 
1.8.5.2

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

[Mesa-dev] [PATCH 01/10] nv50: access only the available amount of textures

[Emil Velikov]

The textures array is defined as a number of PIPE_MAX_SAMPLERS per shader stage.
Currently nv50 driver handles only 3 shader stages, thus we wreck chaos when
accessing array-out-of-bounds.

Fixes a segfault in piglit/bin/arb_texture_buffer_object-data-sync -fbo -auto

Cc: 9.1 9.2 10.0 
Signed-off-by: Emil Velikov 
---
 src/gallium/drivers/nouveau/nv50/nv50_context.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_context.c 
b/src/gallium/drivers/nouveau/nv50/nv50_context.c
index 11afc48..eb345dc 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_context.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_context.c
@@ -165,7 +165,7 @@ nv50_invalidate_resource_storage(struct nouveau_context 
*ctx,
}
 
if (res->bind & PIPE_BIND_SAMPLER_VIEW) {
-  for (s = 0; s < 5; ++s) {
+  for (s = 0; s < 3; ++s) {
   for (i = 0; i < nv50->num_textures[s]; ++i) {
  if (nv50->textures[s][i] &&
  nv50->textures[s][i]->texture == res) {
-- 
1.8.5.2

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

[Mesa-dev] [PATCH 09/10] nv50: assert before trying to out-of-bounds access framebuffer.cbufs

[Emil Velikov]

Signed-off-by: Emil Velikov 
---
 src/gallium/drivers/nouveau/nv50/nv50_context.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_context.c 
b/src/gallium/drivers/nouveau/nv50/nv50_context.c
index 8183b01..24264d5 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_context.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_context.c
@@ -130,6 +130,7 @@ nv50_invalidate_resource_storage(struct nouveau_context 
*ctx,
unsigned s, i;
 
if (res->bind & PIPE_BIND_RENDER_TARGET) {
+  assert(nv50->framebuffer.nr_cbufs <= PIPE_MAX_COLOR_BUFS);
   for (i = 0; i < nv50->framebuffer.nr_cbufs; ++i) {
  if (nv50->framebuffer.cbufs[i] &&
  nv50->framebuffer.cbufs[i]->texture == res) {
-- 
1.8.5.2

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

[Mesa-dev] [PATCH 10/10] nv50: drop obsolete check from error path

[Emil Velikov]

At 'out_err' the nv50_context has been calloc-ated.

Signed-off-by: Emil Velikov 
---
 src/gallium/drivers/nouveau/nv50/nv50_context.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_context.c 
b/src/gallium/drivers/nouveau/nv50/nv50_context.c
index 24264d5..10e8b47 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_context.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_context.c
@@ -298,15 +298,13 @@ nv50_create(struct pipe_screen *pscreen, void *priv)
return pipe;
 
 out_err:
-   if (nv50) {
-  if (nv50->bufctx_3d)
- nouveau_bufctx_del(&nv50->bufctx_3d);
-  if (nv50->bufctx)
- nouveau_bufctx_del(&nv50->bufctx);
-  if (nv50->blit)
- FREE(nv50->blit);
-  FREE(nv50);
-   }
+   if (nv50->bufctx_3d)
+  nouveau_bufctx_del(&nv50->bufctx_3d);
+   if (nv50->bufctx)
+  nouveau_bufctx_del(&nv50->bufctx);
+   if (nv50->blit)
+  FREE(nv50->blit);
+   FREE(nv50);
return NULL;
 }
 
-- 
1.8.5.2

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

[Mesa-dev] [PATCH 06/10] nv50: pass vtxbuf index as unsigned

[Emil Velikov]

The index passed to the function is already unsigned, and internally
we threat it as unsigned.

Signed-off-by: Emil Velikov 
---
 src/gallium/drivers/nouveau/nv50/nv50_vbo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_vbo.c 
b/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
index 1dcccfe..208c116 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
@@ -189,7 +189,7 @@ nv50_emit_vtxattr(struct nv50_context *nv50, struct 
pipe_vertex_buffer *vb,
 }
 
 static INLINE void
-nv50_user_vbuf_range(struct nv50_context *nv50, int vbi,
+nv50_user_vbuf_range(struct nv50_context *nv50, unsigned vbi,
  uint32_t *base, uint32_t *size)
 {
assert(vbi < PIPE_MAX_ATTRIBS);
-- 
1.8.5.2

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

[Mesa-dev] [PATCH 00/10] nv50: out-of-bounds access of context specific attribs

[Emil Velikov]

Subject: [PATCH 00/10] nv50: out-of-bounds access of context specific attribs
In-Reply-To:

So here it goes, a few serious patches and some not as much

Patches 1, 2 correct the range to be within the defined boundaries
when invalidating the textures/constbufs. Most likely a copy/paste
typo from the nvc0 driver.

Patches 3, 5, 7, 8, 9 introduce asserts which may not be stricly
needed. I'm open to suggestions

Patches 4, 6 changes the type of variables used to store the index
to be unsigned.

And patch 10 provides a small/trivial cleanup at the context_ctor
errot path.

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

[Mesa-dev] [PATCH 2/3] mesa: Fix extension dependency for half-float TexBOs

[Ian Romanick]

From: Ian Romanick 

Half-float TexBOs should require both GL_ARB_half_float_pixel and
GL_ARB_texture_float.  This doesn't matter much in practice.  Every
driver that supports GL_ARB_texture_buffer_object already supports
GL_ARB_half_float_pixel.  We only expose the TexBO extension in core
profiles, and those require GL_ARB_texture_float.

Signed-off-by: Ian Romanick 
---
 src/mesa/main/teximage.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
index 8aac54e..bbd5006 100644
--- a/src/mesa/main/teximage.c
+++ b/src/mesa/main/teximage.c
@@ -4150,7 +4150,18 @@ _mesa_validate_texbuffer_format(const struct gl_context 
*ctx,
if (datatype == GL_FLOAT && !ctx->Extensions.ARB_texture_float)
   return MESA_FORMAT_NONE;
 
-   if (datatype == GL_HALF_FLOAT && !ctx->Extensions.ARB_half_float_pixel)
+   /* The GL_ARB_texture_buffer_object spec says:
+*
+* "If ARB_texture_float is not supported, references to the
+* floating-point internal formats provided by that extension should be
+* removed, and such formats may not be passed to TexBufferARB."
+*
+* As a result, GL_HALF_FLOAT internal format depends on both
+* GL_ARB_texture_float and GL_ARB_half_float_pixel.
+*/
+   if (datatype == GL_HALF_FLOAT &&
+   !(ctx->Extensions.ARB_half_float_pixel
+ && ctx->Extensions.ARB_texture_float))
   return MESA_FORMAT_NONE;
 
if (!ctx->Extensions.ARB_texture_rg) {
-- 
1.8.1.4

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

[Mesa-dev] [PATCH 3/3] mesa: GL_ARB_half_float_pixel is not optional

[Ian Romanick]

From: Ian Romanick 

Almost every driver already supported it.  All current and future
Gallium drivers always support it, and most existing classic drivers
support it.

This only changes radeon and nouveau.

This extension only adds data types that can be passed to, for example,
glTexImage2D.  It does not add internal formats.  Since you can already
pass GL_FLOAT to glTexImage2D this shouldn't pose any additional issues
with those drivers.  Note that r200 and i915 already supported this
extension, and they don't support floating-point textures either.

Signed-off-by: Ian Romanick 
---
 docs/GL3.txt |  2 +-
 src/mesa/drivers/dri/i915/intel_extensions.c |  1 -
 src/mesa/drivers/dri/i965/intel_extensions.c |  1 -
 src/mesa/drivers/dri/r200/r200_context.c |  1 -
 src/mesa/main/extensions.c   |  3 +--
 src/mesa/main/glformats.c| 24 ++--
 src/mesa/main/mtypes.h   |  1 -
 src/mesa/main/teximage.c | 17 ++---
 src/mesa/main/version.c  |  1 -
 src/mesa/state_tracker/st_extensions.c   |  1 -
 10 files changed, 10 insertions(+), 42 deletions(-)

diff --git a/docs/GL3.txt b/docs/GL3.txt
index be78652..4902f76b 100644
--- a/docs/GL3.txt
+++ b/docs/GL3.txt
@@ -30,7 +30,7 @@ GL 3.0 --- all DONE: i965, nv50, nvc0, r600, radeonsi
   GL_EXT_texture_shared_exponentDONE (swrast)
   Float depth buffers (GL_ARB_depth_buffer_float)   DONE ()
   Framebuffer objects (GL_ARB_framebuffer_object)   DONE (r300, swrast)
-  GL_ARB_half_float_pixel   DONE (r300, swrast)
+  GL_ARB_half_float_pixel   DONE (all drivers)
   GL_ARB_half_float_vertex  DONE (r300, swrast)
   GL_EXT_texture_integerDONE ()
   GL_EXT_texture_array  DONE ()
diff --git a/src/mesa/drivers/dri/i915/intel_extensions.c 
b/src/mesa/drivers/dri/i915/intel_extensions.c
index a5cff70..13a5c82 100644
--- a/src/mesa/drivers/dri/i915/intel_extensions.c
+++ b/src/mesa/drivers/dri/i915/intel_extensions.c
@@ -47,7 +47,6 @@ intelInitExtensions(struct gl_context *ctx)
ctx->Extensions.ARB_draw_elements_base_vertex = true;
ctx->Extensions.ARB_explicit_attrib_location = true;
ctx->Extensions.ARB_framebuffer_object = true;
-   ctx->Extensions.ARB_half_float_pixel = true;
ctx->Extensions.ARB_internalformat_query = true;
ctx->Extensions.ARB_map_buffer_range = true;
ctx->Extensions.ARB_point_sprite = true;
diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c 
b/src/mesa/drivers/dri/i965/intel_extensions.c
index de07b7f..cdafba8 100644
--- a/src/mesa/drivers/dri/i965/intel_extensions.c
+++ b/src/mesa/drivers/dri/i965/intel_extensions.c
@@ -174,7 +174,6 @@ intelInitExtensions(struct gl_context *ctx)
ctx->Extensions.ARB_fragment_program_shadow = true;
ctx->Extensions.ARB_fragment_shader = true;
ctx->Extensions.ARB_framebuffer_object = true;
-   ctx->Extensions.ARB_half_float_pixel = true;
ctx->Extensions.ARB_half_float_vertex = true;
ctx->Extensions.ARB_instanced_arrays = true;
ctx->Extensions.ARB_internalformat_query = true;
diff --git a/src/mesa/drivers/dri/r200/r200_context.c 
b/src/mesa/drivers/dri/r200/r200_context.c
index f82424b..a6fff09 100644
--- a/src/mesa/drivers/dri/r200/r200_context.c
+++ b/src/mesa/drivers/dri/r200/r200_context.c
@@ -365,7 +365,6 @@ GLboolean r200CreateContext( gl_api api,
_math_matrix_ctr( &rmesa->tmpmat );
_math_matrix_set_identity( &rmesa->tmpmat );
 
-   ctx->Extensions.ARB_half_float_pixel = true;
ctx->Extensions.ARB_occlusion_query = true;
ctx->Extensions.ARB_point_sprite = true;
ctx->Extensions.ARB_texture_border_clamp = true;
diff --git a/src/mesa/main/extensions.c b/src/mesa/main/extensions.c
index 2e0ccc3..7882bb6 100644
--- a/src/mesa/main/extensions.c
+++ b/src/mesa/main/extensions.c
@@ -104,7 +104,7 @@ static const struct extension extension_table[] = {
{ "GL_ARB_framebuffer_sRGB",o(EXT_framebuffer_sRGB),
GL, 1998 },
{ "GL_ARB_get_program_binary",  o(dummy_true),  
GL, 2010 },
{ "GL_ARB_gpu_shader5", o(ARB_gpu_shader5), 
GL, 2010 },
-   { "GL_ARB_half_float_pixel",o(ARB_half_float_pixel),
GL, 2003 },
+   { "GL_ARB_half_float_pixel",o(dummy_true),  
GL, 2003 },
{ "GL_ARB_half_float_vertex",   o(ARB_half_float_vertex),   
GL, 2008 },
{ "GL_ARB_instanced_arrays",o(ARB_instanced_arrays),
GL, 2008 },
{ "GL_ARB_internalformat_query",

[Mesa-dev] [PATCH 1/3] docs: Update GL3.txt due to recent work

[Ian Romanick]

From: Ian Romanick 

Signed-off-by: Ian Romanick 
---
 docs/GL3.txt | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/docs/GL3.txt b/docs/GL3.txt
index 0672ec7..be78652 100644
--- a/docs/GL3.txt
+++ b/docs/GL3.txt
@@ -55,7 +55,7 @@ GL 3.1 --- all DONE: i965, nv50, nvc0, r600, radeonsi
   Forward compatible context support/deprecations   DONE ()
   Instanced drawing (GL_ARB_draw_instanced) DONE (swrast)
   Buffer copying (GL_ARB_copy_buffer)   DONE (r300, swrast)
-  Primitive restart (GL_NV_primitive_restart)   DONE (r300, )
+  Primitive restart (GL_NV_primitive_restart)   DONE (r300)
   16 vertex texture image units DONE ()
   Texture buffer objs (GL_ARB_texture_buffer_object)DONE for OpenGL 3.1 
contexts ()
   Rectangular textures (GL_ARB_texture_rectangle)   DONE (r300, swrast)
@@ -83,9 +83,9 @@ GL 3.3 --- all DONE: i965
 
   GLSL 3.30 DONE ()
   GL_ARB_blend_func_extendedDONE (nv50, nvc0, 
r600, radeonsi, softpipe)
-  GL_ARB_explicit_attrib_location   DONE (i915, nv50, 
nvc0, r300, r600, radeonsi, swrast)
+  GL_ARB_explicit_attrib_location   DONE (all drivers that 
support GLSL)
   GL_ARB_occlusion_query2   DONE (nv50, nvc0, 
r300, r600, radeonsi, swrast)
-  GL_ARB_sampler_objectsDONE (nv50, nvc0, 
r300, r600, radeonsi)
+  GL_ARB_sampler_objectsDONE (all drivers)
   GL_ARB_shader_bit_encodingDONE (nv50, nvc0, 
r600, radeonsi)
   GL_ARB_texture_rgb10_a2ui DONE (r600, radeonsi)
   GL_ARB_texture_swizzleDONE (nv50, nvc0, 
r300, r600, radeonsi, swrast)
@@ -117,10 +117,10 @@ GL 4.1:
   GLSL 4.1 not started
   GL_ARB_ES2_compatibility DONE (i965, r300, r600, 
radeonsi)
   GL_ARB_get_program_binaryDONE (0 binary formats)
-  GL_ARB_separate_shader_objects   some infrastructure done
+  GL_ARB_separate_shader_objects   started (Ian Romanick, 
Gregory Hainaut)
   GL_ARB_shader_precision  not started
   GL_ARB_vertex_attrib_64bit   not started
-  GL_ARB_viewport_arraynot started
+  GL_ARB_viewport_arraystarted (Ian Romanick, 
Courtney Goeltzenleuchter)
 
 
 GL 4.2:
@@ -144,8 +144,8 @@ GL 4.3:
   GLSL 4.3 not started
   GL_ARB_arrays_of_arrays  not started
   GL_ARB_ES3_compatibility DONE (i965)
-  GL_ARB_clear_buffer_object   not started
-  GL_ARB_compute_shadernot started
+  GL_ARB_clear_buffer_object   DONE (all drivers)
+  GL_ARB_compute_shaderstarted (Paul Berry)
   GL_ARB_copy_imagenot started
   GL_KHR_debug DONE (all drivers)
   GL_ARB_explicit_uniform_location not started
@@ -162,7 +162,7 @@ GL 4.3:
   GL_ARB_texture_buffer_range  DONE (nv50, nvc0, i965, 
r600, radeonsi)
   GL_ARB_texture_query_levels  DONE (i965)
   GL_ARB_texture_storage_multisample   DONE (all drivers that 
support GL_ARB_texture_multisample)
-  GL_ARB_texture_view  not started
+  GL_ARB_texture_view  started (Courtney 
Goeltzenleuchter, Chris Forbes)
   GL_ARB_vertex_attrib_binding DONE (all drivers)
 
 
@@ -173,7 +173,7 @@ GL 4.4:
   GL_ARB_buffer_storagenot started
   GL_ARB_clear_texture not started
   GL_ARB_enhanced_layouts  not started
-  GL_ARB_multi_bindnot started
+  GL_ARB_multi_bindstarted (Maxence Le 
Doré)
   GL_ARB_query_buffer_object   not started
   GL_ARB_texture_mirror_clamp_to_edge  DONE (i965, nv30, nv50, 
nvc0, r300, r600, radeonsi, swrast)
   GL_ARB_texture_stencil8  not started
-- 
1.8.1.4

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

[Mesa-dev] [PATCH 08/10] nv50: assert before trying to out-of-bounds access samplers

[Emil Velikov]

Signed-off-by: Emil Velikov 
---
 src/gallium/drivers/nouveau/nv50/nv50_state.c | 3 +++
 src/gallium/drivers/nouveau/nv50/nv50_tex.c   | 1 +
 2 files changed, 4 insertions(+)

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_state.c 
b/src/gallium/drivers/nouveau/nv50/nv50_state.c
index b07856c..02755a5 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_state.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_state.c
@@ -557,6 +557,7 @@ nv50_sampler_state_delete(struct pipe_context *pipe, void 
*hwcso)
unsigned s, i;
 
for (s = 0; s < 3; ++s)
+  assert(nv50_context(pipe)->num_samplers[s] <= PIPE_MAX_SAMPLERS);
   for (i = 0; i < nv50_context(pipe)->num_samplers[s]; ++i)
  if (nv50_context(pipe)->samplers[s][i] == hwcso)
 nv50_context(pipe)->samplers[s][i] = NULL;
@@ -572,6 +573,7 @@ nv50_stage_sampler_states_bind(struct nv50_context *nv50, 
int s,
 {
unsigned i;
 
+   assert(nr <= PIPE_MAX_SAMPLERS);
for (i = 0; i < nr; ++i) {
   struct nv50_tsc_entry *old = nv50->samplers[s][i];
 
@@ -579,6 +581,7 @@ nv50_stage_sampler_states_bind(struct nv50_context *nv50, 
int s,
   if (old)
  nv50_screen_tsc_unlock(nv50->screen, old);
}
+   assert(nv50->num_samplers[s] <= PIPE_MAX_SAMPLERS);
for (; i < nv50->num_samplers[s]; ++i)
   if (nv50->samplers[s][i])
  nv50_screen_tsc_unlock(nv50->screen, nv50->samplers[s][i]);
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_tex.c 
b/src/gallium/drivers/nouveau/nv50/nv50_tex.c
index f2325cf..bd47bf8 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_tex.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_tex.c
@@ -309,6 +309,7 @@ nv50_validate_tsc(struct nv50_context *nv50, int s)
unsigned i;
boolean need_flush = FALSE;
 
+   assert(nv50->num_samplers[s] <= PIPE_MAX_SAMPLERS);
for (i = 0; i < nv50->num_samplers[s]; ++i) {
   struct nv50_tsc_entry *tsc = nv50_tsc_entry(nv50->samplers[s][i]);
 
-- 
1.8.5.2

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

Re: [Mesa-dev] [PATCH] glsl: Assert buildin uniform variables

[Matt Turner]

On Thu, Jan 16, 2014 at 2:59 AM, Juha-Pekka Heikkila
 wrote:
> Signed-off-by: Juha-Pekka Heikkila 
> ---
>  src/glsl/builtin_variables.cpp | 24 
>  1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/src/glsl/builtin_variables.cpp b/src/glsl/builtin_variables.cpp
> index f630923..9f4f7c7 100644
> --- a/src/glsl/builtin_variables.cpp
> +++ b/src/glsl/builtin_variables.cpp
> @@ -669,7 +669,11 @@ void
>  builtin_variable_generator::generate_uniforms()
>  {
> add_uniform(int_t, "gl_NumSamples");
> -   add_uniform(type("gl_DepthRangeParameters"), "gl_DepthRange");
> +
> +   const glsl_type *const depth_range_parameters_type =
> +  type("gl_DepthRangeParameters");
> +   assert(depth_range_parameters_type);
> +   add_uniform(depth_range_parameters_type, "gl_DepthRange");
> add_uniform(array(vec4_t, VERT_ATTRIB_MAX), "gl_CurrentAttribVertMESA");
> add_uniform(array(vec4_t, VARYING_SLOT_MAX), "gl_CurrentAttribFragMESA");
>
> @@ -688,7 +692,11 @@ builtin_variable_generator::generate_uniforms()
>add_uniform(mat4_t, "gl_ProjectionMatrixInverseTranspose");
>add_uniform(mat4_t, "gl_ModelViewProjectionMatrixInverseTranspose");
>add_uniform(float_t, "gl_NormalScale");
> -  add_uniform(type("gl_LightModelParameters"), "gl_LightModel");
> +
> +  const glsl_type *const light_model_parameters_type =
> + type("gl_LightModelParameters");
> +  assert(light_model_parameters_type);
> +  add_uniform(light_model_parameters_type, "gl_LightModel");
>add_uniform(vec2_t, "gl_BumpRotMatrix0MESA");
>add_uniform(vec2_t, "gl_BumpRotMatrix1MESA");
>add_uniform(vec4_t, "gl_FogParamsOptimizedMESA");
> @@ -701,10 +709,15 @@ builtin_variable_generator::generate_uniforms()
>add_uniform(mat4_array_type, "gl_TextureMatrixInverseTranspose");
>
>add_uniform(array(vec4_t, state->Const.MaxClipPlanes), "gl_ClipPlane");
> -  add_uniform(type("gl_PointParameters"), "gl_Point");
> +
> +  const glsl_type *const point_parameters_type =
> + type("gl_PointParameters");
> +  assert(point_parameters_type);
> +  add_uniform(point_parameters_type, "gl_Point");
>
>const glsl_type *const material_parameters_type =
>  type("gl_MaterialParameters");
> +  assert(material_parameters_type);
>add_uniform(material_parameters_type, "gl_FrontMaterial");
>add_uniform(material_parameters_type, "gl_BackMaterial");
>
> @@ -714,6 +727,7 @@ builtin_variable_generator::generate_uniforms()
>
>const glsl_type *const light_model_products_type =
>   type("gl_LightModelProducts");
> +  assert(light_model_products_type);
>add_uniform(light_model_products_type, "gl_FrontLightModelProduct");
>add_uniform(light_model_products_type, "gl_BackLightModelProduct");
>
> @@ -736,7 +750,9 @@ builtin_variable_generator::generate_uniforms()
>add_uniform(texcoords_vec4, "gl_ObjectPlaneR");
>add_uniform(texcoords_vec4, "gl_ObjectPlaneQ");
>
> -  add_uniform(type("gl_FogParameters"), "gl_Fog");
> +  const glsl_type *const fog_parameters_type = type("gl_FogParameters");
> +  assert(fog_parameters_type);
> +  add_uniform(fog_parameters_type, "gl_Fog");
> }
>  }
>
> --
> 1.8.1.2

Does this help something? asserts are no-ops in optimized builds, so
adding them won't fix klocwork issues.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 05/10] nv50: assert before trying to out-of-bounds access vtxbuf

[Emil Velikov]

Signed-off-by: Emil Velikov 
---
 src/gallium/drivers/nouveau/nv50/nv50_context.c |  2 ++
 src/gallium/drivers/nouveau/nv50/nv50_push.c|  1 +
 src/gallium/drivers/nouveau/nv50/nv50_vbo.c | 12 +++-
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_context.c 
b/src/gallium/drivers/nouveau/nv50/nv50_context.c
index 9ea425e..a4ec93a 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_context.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_context.c
@@ -80,6 +80,7 @@ nv50_context_unreference_resources(struct nv50_context *nv50)
 
util_unreference_framebuffer_state(&nv50->framebuffer);
 
+   assert(nv50->num_vtxbufs <= PIPE_MAX_ATTRIBS);
for (i = 0; i < nv50->num_vtxbufs; ++i)
   pipe_resource_reference(&nv50->vtxbuf[i].buffer, NULL);
 
@@ -149,6 +150,7 @@ nv50_invalidate_resource_storage(struct nouveau_context 
*ctx,
}
 
if (res->bind & PIPE_BIND_VERTEX_BUFFER) {
+  assert(nv50->num_vtxbufs <= PIPE_MAX_ATTRIBS);
   for (i = 0; i < nv50->num_vtxbufs; ++i) {
  if (nv50->vtxbuf[i].buffer == res) {
 nv50->dirty |= NV50_NEW_ARRAYS;
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_push.c 
b/src/gallium/drivers/nouveau/nv50/nv50_push.c
index 3e9a409..a3a397c 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_push.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_push.c
@@ -219,6 +219,7 @@ nv50_push_vbo(struct nv50_context *nv50, const struct 
pipe_draw_info *info)
ctx.packet_vertex_limit = nv50->vertex->packet_vertex_limit;
ctx.vertex_words = nv50->vertex->vertex_size;
 
+   assert(nv50->num_vtxbufs <= PIPE_MAX_ATTRIBS);
for (i = 0; i < nv50->num_vtxbufs; ++i) {
   const struct pipe_vertex_buffer *vb = &nv50->vtxbuf[i];
   const uint8_t *data;
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_vbo.c 
b/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
index 947c67d..1dcccfe 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
@@ -192,6 +192,7 @@ static INLINE void
 nv50_user_vbuf_range(struct nv50_context *nv50, int vbi,
  uint32_t *base, uint32_t *size)
 {
+   assert(vbi < PIPE_MAX_ATTRIBS);
if (unlikely(nv50->vertex->instance_bufs & (1 << vbi))) {
   /* TODO: use min and max instance divisor to get a proper range */
   *base = 0;
@@ -211,6 +212,7 @@ nv50_upload_user_buffers(struct nv50_context *nv50,
 {
unsigned b;
 
+   assert(nv50->num_vtxbufs <= PIPE_MAX_ATTRIBS);
for (b = 0; b < nv50->num_vtxbufs; ++b) {
   struct nouveau_bo *bo;
   const struct pipe_vertex_buffer *vb = &nv50->vtxbuf[b];
@@ -241,9 +243,12 @@ nv50_update_user_vbufs(struct nv50_context *nv50)
for (i = 0; i < nv50->vertex->num_elements; ++i) {
   struct pipe_vertex_element *ve = &nv50->vertex->element[i].pipe;
   const unsigned b = ve->vertex_buffer_index;
-  struct pipe_vertex_buffer *vb = &nv50->vtxbuf[b];
+  struct pipe_vertex_buffer *vb;
   uint32_t base, size;
 
+  assert(b < PIPE_MAX_ATTRIBS);
+  vb = &nv50->vtxbuf[b];
+
   if (!(nv50->vbo_user & (1 << b)))
  continue;
 
@@ -306,6 +311,7 @@ nv50_vertex_arrays_validate(struct nv50_context *nv50)
 
if (!nv50->vbo_fifo) {
   /* if vertex buffer was written by GPU - flush VBO cache */
+  assert(nv50->num_vtxbufs <= PIPE_MAX_ATTRIBS);
   for (i = 0; i < nv50->num_vtxbufs; ++i) {
  struct nv04_resource *buf = nv04_resource(nv50->vtxbuf[i].buffer);
  if (buf && buf->status & NOUVEAU_BUFFER_STATUS_GPU_WRITING) {
@@ -332,6 +338,8 @@ nv50_vertex_arrays_validate(struct nv50_context *nv50)
}
for (i = 0; i < vertex->num_elements; ++i) {
   const unsigned b = vertex->element[i].pipe.vertex_buffer_index;
+
+  assert(b < PIPE_MAX_ATTRIBS);
   ve = &vertex->element[i];
   vb = &nv50->vtxbuf[b];
 
@@ -360,6 +368,8 @@ nv50_vertex_arrays_validate(struct nv50_context *nv50)
for (i = 0; i < vertex->num_elements; ++i) {
   uint64_t address, limit;
   const unsigned b = vertex->element[i].pipe.vertex_buffer_index;
+
+  assert(b < PIPE_MAX_ATTRIBS);
   ve = &vertex->element[i];
   vb = &nv50->vtxbuf[b];
 
-- 
1.8.5.2

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

Re: [Mesa-dev] [PATCH 05/10] nv50: assert before trying to out-of-bounds access vtxbuf

[Ilia Mirkin]

On Thu, Jan 16, 2014 at 1:44 PM, Emil Velikov  wrote:
> Signed-off-by: Emil Velikov 
> ---
>  src/gallium/drivers/nouveau/nv50/nv50_context.c |  2 ++
>  src/gallium/drivers/nouveau/nv50/nv50_push.c|  1 +
>  src/gallium/drivers/nouveau/nv50/nv50_vbo.c | 12 +++-
>  3 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_context.c 
> b/src/gallium/drivers/nouveau/nv50/nv50_context.c
> index 9ea425e..a4ec93a 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_context.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_context.c
> @@ -80,6 +80,7 @@ nv50_context_unreference_resources(struct nv50_context 
> *nv50)
>
> util_unreference_framebuffer_state(&nv50->framebuffer);
>
> +   assert(nv50->num_vtxbufs <= PIPE_MAX_ATTRIBS);
> for (i = 0; i < nv50->num_vtxbufs; ++i)
>pipe_resource_reference(&nv50->vtxbuf[i].buffer, NULL);
>
> @@ -149,6 +150,7 @@ nv50_invalidate_resource_storage(struct nouveau_context 
> *ctx,
> }
>
> if (res->bind & PIPE_BIND_VERTEX_BUFFER) {
> +  assert(nv50->num_vtxbufs <= PIPE_MAX_ATTRIBS);
>for (i = 0; i < nv50->num_vtxbufs; ++i) {
>   if (nv50->vtxbuf[i].buffer == res) {
>  nv50->dirty |= NV50_NEW_ARRAYS;
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_push.c 
> b/src/gallium/drivers/nouveau/nv50/nv50_push.c
> index 3e9a409..a3a397c 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_push.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_push.c
> @@ -219,6 +219,7 @@ nv50_push_vbo(struct nv50_context *nv50, const struct 
> pipe_draw_info *info)
> ctx.packet_vertex_limit = nv50->vertex->packet_vertex_limit;
> ctx.vertex_words = nv50->vertex->vertex_size;
>
> +   assert(nv50->num_vtxbufs <= PIPE_MAX_ATTRIBS);
> for (i = 0; i < nv50->num_vtxbufs; ++i) {
>const struct pipe_vertex_buffer *vb = &nv50->vtxbuf[i];
>const uint8_t *data;
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_vbo.c 
> b/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
> index 947c67d..1dcccfe 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
> @@ -192,6 +192,7 @@ static INLINE void
>  nv50_user_vbuf_range(struct nv50_context *nv50, int vbi,
>   uint32_t *base, uint32_t *size)
>  {
> +   assert(vbi < PIPE_MAX_ATTRIBS);

Also make the parameter a uint?

> if (unlikely(nv50->vertex->instance_bufs & (1 << vbi))) {
>/* TODO: use min and max instance divisor to get a proper range */
>*base = 0;
> @@ -211,6 +212,7 @@ nv50_upload_user_buffers(struct nv50_context *nv50,
>  {
> unsigned b;
>
> +   assert(nv50->num_vtxbufs <= PIPE_MAX_ATTRIBS);
> for (b = 0; b < nv50->num_vtxbufs; ++b) {
>struct nouveau_bo *bo;
>const struct pipe_vertex_buffer *vb = &nv50->vtxbuf[b];
> @@ -241,9 +243,12 @@ nv50_update_user_vbufs(struct nv50_context *nv50)
> for (i = 0; i < nv50->vertex->num_elements; ++i) {
>struct pipe_vertex_element *ve = &nv50->vertex->element[i].pipe;
>const unsigned b = ve->vertex_buffer_index;
> -  struct pipe_vertex_buffer *vb = &nv50->vtxbuf[b];
> +  struct pipe_vertex_buffer *vb;
>uint32_t base, size;
>
> +  assert(b < PIPE_MAX_ATTRIBS);
> +  vb = &nv50->vtxbuf[b];
> +
>if (!(nv50->vbo_user & (1 << b)))
>   continue;
>
> @@ -306,6 +311,7 @@ nv50_vertex_arrays_validate(struct nv50_context *nv50)
>
> if (!nv50->vbo_fifo) {
>/* if vertex buffer was written by GPU - flush VBO cache */
> +  assert(nv50->num_vtxbufs <= PIPE_MAX_ATTRIBS);
>for (i = 0; i < nv50->num_vtxbufs; ++i) {
>   struct nv04_resource *buf = nv04_resource(nv50->vtxbuf[i].buffer);
>   if (buf && buf->status & NOUVEAU_BUFFER_STATUS_GPU_WRITING) {
> @@ -332,6 +338,8 @@ nv50_vertex_arrays_validate(struct nv50_context *nv50)
> }
> for (i = 0; i < vertex->num_elements; ++i) {
>const unsigned b = vertex->element[i].pipe.vertex_buffer_index;
> +
> +  assert(b < PIPE_MAX_ATTRIBS);
>ve = &vertex->element[i];
>vb = &nv50->vtxbuf[b];
>
> @@ -360,6 +368,8 @@ nv50_vertex_arrays_validate(struct nv50_context *nv50)
> for (i = 0; i < vertex->num_elements; ++i) {
>uint64_t address, limit;
>const unsigned b = vertex->element[i].pipe.vertex_buffer_index;
> +
> +  assert(b < PIPE_MAX_ATTRIBS);
>ve = &vertex->element[i];
>vb = &nv50->vtxbuf[b];
>
> --
> 1.8.5.2
>
> ___
> 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 00/10] nv50: out-of-bounds access of context specific attribs

[Ilia Mirkin]

On Thu, Jan 16, 2014 at 1:44 PM, Emil Velikov  wrote:
> Subject: [PATCH 00/10] nv50: out-of-bounds access of context specific attribs
> In-Reply-To:
>
> So here it goes, a few serious patches and some not as much
>
> Patches 1, 2 correct the range to be within the defined boundaries
> when invalidating the textures/constbufs. Most likely a copy/paste
> typo from the nvc0 driver.
>
> Patches 3, 5, 7, 8, 9 introduce asserts which may not be stricly
> needed. I'm open to suggestions
>
> Patches 4, 6 changes the type of variables used to store the index
> to be unsigned.
>
> And patch 10 provides a small/trivial cleanup at the context_ctor
> errot path.

Series is

Reviewed-by: Ilia Mirkin 

Nice cleanups! And it should make future bugs a bit more obvious.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/3] docs: Update GL3.txt due to recent work

[Matt Turner]

On Thu, Jan 16, 2014 at 10:44 AM, Ian Romanick  wrote:
> From: Ian Romanick 
>
> Signed-off-by: Ian Romanick 
> ---
>  docs/GL3.txt | 18 +-
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/docs/GL3.txt b/docs/GL3.txt
> index 0672ec7..be78652 100644
> --- a/docs/GL3.txt
> +++ b/docs/GL3.txt
> @@ -55,7 +55,7 @@ GL 3.1 --- all DONE: i965, nv50, nvc0, r600, radeonsi
>Forward compatible context support/deprecations   DONE ()
>Instanced drawing (GL_ARB_draw_instanced) DONE (swrast)
>Buffer copying (GL_ARB_copy_buffer)   DONE (r300, swrast)
> -  Primitive restart (GL_NV_primitive_restart)   DONE (r300, )
> +  Primitive restart (GL_NV_primitive_restart)   DONE (r300)
>16 vertex texture image units DONE ()
>Texture buffer objs (GL_ARB_texture_buffer_object)DONE for OpenGL 3.1 
> contexts ()
>Rectangular textures (GL_ARB_texture_rectangle)   DONE (r300, swrast)
> @@ -83,9 +83,9 @@ GL 3.3 --- all DONE: i965
>
>GLSL 3.30 DONE ()
>GL_ARB_blend_func_extendedDONE (nv50, nvc0, 
> r600, radeonsi, softpipe)
> -  GL_ARB_explicit_attrib_location   DONE (i915, nv50, 
> nvc0, r300, r600, radeonsi, swrast)
> +  GL_ARB_explicit_attrib_location   DONE (all drivers 
> that support GLSL)
>GL_ARB_occlusion_query2   DONE (nv50, nvc0, 
> r300, r600, radeonsi, swrast)
> -  GL_ARB_sampler_objectsDONE (nv50, nvc0, 
> r300, r600, radeonsi)
> +  GL_ARB_sampler_objectsDONE (all drivers)
>GL_ARB_shader_bit_encodingDONE (nv50, nvc0, 
> r600, radeonsi)
>GL_ARB_texture_rgb10_a2ui DONE (r600, radeonsi)
>GL_ARB_texture_swizzleDONE (nv50, nvc0, 
> r300, r600, radeonsi, swrast)
> @@ -117,10 +117,10 @@ GL 4.1:
>GLSL 4.1 not started
>GL_ARB_ES2_compatibility DONE (i965, r300, 
> r600, radeonsi)
>GL_ARB_get_program_binaryDONE (0 binary 
> formats)
> -  GL_ARB_separate_shader_objects   some infrastructure 
> done
> +  GL_ARB_separate_shader_objects   started (Ian 
> Romanick, Gregory Hainaut)
>GL_ARB_shader_precision  not started
>GL_ARB_vertex_attrib_64bit   not started
> -  GL_ARB_viewport_arraynot started
> +  GL_ARB_viewport_arraystarted (Ian 
> Romanick, Courtney Goeltzenleuchter)
>
>
>  GL 4.2:
> @@ -144,8 +144,8 @@ GL 4.3:
>GLSL 4.3 not started
>GL_ARB_arrays_of_arrays  not started
>GL_ARB_ES3_compatibility DONE (i965)
> -  GL_ARB_clear_buffer_object   not started
> -  GL_ARB_compute_shadernot started
> +  GL_ARB_clear_buffer_object   DONE (all drivers)
> +  GL_ARB_compute_shaderstarted (Paul Berry)
>GL_ARB_copy_imagenot started
>GL_KHR_debug DONE (all drivers)
>GL_ARB_explicit_uniform_location not started
> @@ -162,7 +162,7 @@ GL 4.3:
>GL_ARB_texture_buffer_range  DONE (nv50, nvc0, 
> i965, r600, radeonsi)
>GL_ARB_texture_query_levels  DONE (i965)
>GL_ARB_texture_storage_multisample   DONE (all drivers 
> that support GL_ARB_texture_multisample)
> -  GL_ARB_texture_view  not started
> +  GL_ARB_texture_view  started (Courtney 
> Goeltzenleuchter, Chris Forbes)
>GL_ARB_vertex_attrib_binding DONE (all drivers)
>
>
> @@ -173,7 +173,7 @@ GL 4.4:
>GL_ARB_buffer_storagenot started
>GL_ARB_clear_texture not started
>GL_ARB_enhanced_layouts  not started
> -  GL_ARB_multi_bindnot started
> +  GL_ARB_multi_bindstarted (Maxence Le 
> Doré)

I don't think we were going forward with Maxence's patches, and rather
going with Fredrik's (Cc'd). Fredrik, what's the current status?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] glsl: Assert buildin uniform variables

[Juha-Pekka Heikkilä]

On Thu, Jan 16, 2014 at 8:48 PM, Matt Turner  wrote:
> On Thu, Jan 16, 2014 at 2:59 AM, Juha-Pekka Heikkila
>  wrote:
>> Signed-off-by: Juha-Pekka Heikkila 
>> ---
>>  src/glsl/builtin_variables.cpp | 24 
>>  1 file changed, 20 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/glsl/builtin_variables.cpp b/src/glsl/builtin_variables.cpp
>> index f630923..9f4f7c7 100644
>> --- a/src/glsl/builtin_variables.cpp
>> +++ b/src/glsl/builtin_variables.cpp
>> @@ -669,7 +669,11 @@ void
>>  builtin_variable_generator::generate_uniforms()
>>  {
>> add_uniform(int_t, "gl_NumSamples");
>> -   add_uniform(type("gl_DepthRangeParameters"), "gl_DepthRange");
>> +
>> +   const glsl_type *const depth_range_parameters_type =
>> +  type("gl_DepthRangeParameters");
>> +   assert(depth_range_parameters_type);
>> +   add_uniform(depth_range_parameters_type, "gl_DepthRange");
>> add_uniform(array(vec4_t, VERT_ATTRIB_MAX), "gl_CurrentAttribVertMESA");
>> add_uniform(array(vec4_t, VARYING_SLOT_MAX), "gl_CurrentAttribFragMESA");
>>
>> @@ -688,7 +692,11 @@ builtin_variable_generator::generate_uniforms()
>>add_uniform(mat4_t, "gl_ProjectionMatrixInverseTranspose");
>>add_uniform(mat4_t, "gl_ModelViewProjectionMatrixInverseTranspose");
>>add_uniform(float_t, "gl_NormalScale");
>> -  add_uniform(type("gl_LightModelParameters"), "gl_LightModel");
>> +
>> +  const glsl_type *const light_model_parameters_type =
>> + type("gl_LightModelParameters");
>> +  assert(light_model_parameters_type);
>> +  add_uniform(light_model_parameters_type, "gl_LightModel");
>>add_uniform(vec2_t, "gl_BumpRotMatrix0MESA");
>>add_uniform(vec2_t, "gl_BumpRotMatrix1MESA");
>>add_uniform(vec4_t, "gl_FogParamsOptimizedMESA");
>> @@ -701,10 +709,15 @@ builtin_variable_generator::generate_uniforms()
>>add_uniform(mat4_array_type, "gl_TextureMatrixInverseTranspose");
>>
>>add_uniform(array(vec4_t, state->Const.MaxClipPlanes), 
>> "gl_ClipPlane");
>> -  add_uniform(type("gl_PointParameters"), "gl_Point");
>> +
>> +  const glsl_type *const point_parameters_type =
>> + type("gl_PointParameters");
>> +  assert(point_parameters_type);
>> +  add_uniform(point_parameters_type, "gl_Point");
>>
>>const glsl_type *const material_parameters_type =
>>  type("gl_MaterialParameters");
>> +  assert(material_parameters_type);
>>add_uniform(material_parameters_type, "gl_FrontMaterial");
>>add_uniform(material_parameters_type, "gl_BackMaterial");
>>
>> @@ -714,6 +727,7 @@ builtin_variable_generator::generate_uniforms()
>>
>>const glsl_type *const light_model_products_type =
>>   type("gl_LightModelProducts");
>> +  assert(light_model_products_type);
>>add_uniform(light_model_products_type, "gl_FrontLightModelProduct");
>>add_uniform(light_model_products_type, "gl_BackLightModelProduct");
>>
>> @@ -736,7 +750,9 @@ builtin_variable_generator::generate_uniforms()
>>add_uniform(texcoords_vec4, "gl_ObjectPlaneR");
>>add_uniform(texcoords_vec4, "gl_ObjectPlaneQ");
>>
>> -  add_uniform(type("gl_FogParameters"), "gl_Fog");
>> +  const glsl_type *const fog_parameters_type = type("gl_FogParameters");
>> +  assert(fog_parameters_type);
>> +  add_uniform(fog_parameters_type, "gl_Fog");
>> }
>>  }
>>
>> --
>> 1.8.1.2
>
> Does this help something? asserts are no-ops in optimized builds, so
> adding them won't fix klocwork issues.

I did not think checking these build in types outside debug builds
would make much sense thus only assert. I can of course change them to
normal if(null) checks but I was thinking if these ever fail outside
development work there is something very wrong.

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

Re: [Mesa-dev] [PATCH] glsl: Assert buildin uniform variables

[Matt Turner]

On Thu, Jan 16, 2014 at 12:37 PM, Juha-Pekka Heikkilä
 wrote:
> On Thu, Jan 16, 2014 at 8:48 PM, Matt Turner  wrote:
>> On Thu, Jan 16, 2014 at 2:59 AM, Juha-Pekka Heikkila
>>  wrote:
>>> Signed-off-by: Juha-Pekka Heikkila 
>>> ---
>>>  src/glsl/builtin_variables.cpp | 24 
>>>  1 file changed, 20 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/glsl/builtin_variables.cpp b/src/glsl/builtin_variables.cpp
>>> index f630923..9f4f7c7 100644
>>> --- a/src/glsl/builtin_variables.cpp
>>> +++ b/src/glsl/builtin_variables.cpp
>>> @@ -669,7 +669,11 @@ void
>>>  builtin_variable_generator::generate_uniforms()
>>>  {
>>> add_uniform(int_t, "gl_NumSamples");
>>> -   add_uniform(type("gl_DepthRangeParameters"), "gl_DepthRange");
>>> +
>>> +   const glsl_type *const depth_range_parameters_type =
>>> +  type("gl_DepthRangeParameters");
>>> +   assert(depth_range_parameters_type);
>>> +   add_uniform(depth_range_parameters_type, "gl_DepthRange");
>>> add_uniform(array(vec4_t, VERT_ATTRIB_MAX), "gl_CurrentAttribVertMESA");
>>> add_uniform(array(vec4_t, VARYING_SLOT_MAX), 
>>> "gl_CurrentAttribFragMESA");
>>>
>>> @@ -688,7 +692,11 @@ builtin_variable_generator::generate_uniforms()
>>>add_uniform(mat4_t, "gl_ProjectionMatrixInverseTranspose");
>>>add_uniform(mat4_t, "gl_ModelViewProjectionMatrixInverseTranspose");
>>>add_uniform(float_t, "gl_NormalScale");
>>> -  add_uniform(type("gl_LightModelParameters"), "gl_LightModel");
>>> +
>>> +  const glsl_type *const light_model_parameters_type =
>>> + type("gl_LightModelParameters");
>>> +  assert(light_model_parameters_type);
>>> +  add_uniform(light_model_parameters_type, "gl_LightModel");
>>>add_uniform(vec2_t, "gl_BumpRotMatrix0MESA");
>>>add_uniform(vec2_t, "gl_BumpRotMatrix1MESA");
>>>add_uniform(vec4_t, "gl_FogParamsOptimizedMESA");
>>> @@ -701,10 +709,15 @@ builtin_variable_generator::generate_uniforms()
>>>add_uniform(mat4_array_type, "gl_TextureMatrixInverseTranspose");
>>>
>>>add_uniform(array(vec4_t, state->Const.MaxClipPlanes), 
>>> "gl_ClipPlane");
>>> -  add_uniform(type("gl_PointParameters"), "gl_Point");
>>> +
>>> +  const glsl_type *const point_parameters_type =
>>> + type("gl_PointParameters");
>>> +  assert(point_parameters_type);
>>> +  add_uniform(point_parameters_type, "gl_Point");
>>>
>>>const glsl_type *const material_parameters_type =
>>>  type("gl_MaterialParameters");
>>> +  assert(material_parameters_type);
>>>add_uniform(material_parameters_type, "gl_FrontMaterial");
>>>add_uniform(material_parameters_type, "gl_BackMaterial");
>>>
>>> @@ -714,6 +727,7 @@ builtin_variable_generator::generate_uniforms()
>>>
>>>const glsl_type *const light_model_products_type =
>>>   type("gl_LightModelProducts");
>>> +  assert(light_model_products_type);
>>>add_uniform(light_model_products_type, "gl_FrontLightModelProduct");
>>>add_uniform(light_model_products_type, "gl_BackLightModelProduct");
>>>
>>> @@ -736,7 +750,9 @@ builtin_variable_generator::generate_uniforms()
>>>add_uniform(texcoords_vec4, "gl_ObjectPlaneR");
>>>add_uniform(texcoords_vec4, "gl_ObjectPlaneQ");
>>>
>>> -  add_uniform(type("gl_FogParameters"), "gl_Fog");
>>> +  const glsl_type *const fog_parameters_type = 
>>> type("gl_FogParameters");
>>> +  assert(fog_parameters_type);
>>> +  add_uniform(fog_parameters_type, "gl_Fog");
>>> }
>>>  }
>>>
>>> --
>>> 1.8.1.2
>>
>> Does this help something? asserts are no-ops in optimized builds, so
>> adding them won't fix klocwork issues.
>
> I did not think checking these build in types outside debug builds
> would make much sense thus only assert. I can of course change them to
> normal if(null) checks but I was thinking if these ever fail outside
> development work there is something very wrong.

I agree -- if these fail something is terribly wrong. Since they're
hardcoded built-in variables I think there's zero chance that they'll
fail, so I'd ignore this klocwork issue.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 73714] New account for Ilia Mirkin

[bugzilla-daemon]

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

Ilia Mirkin  changed:

   What|Removed |Added

   Assignee|sitewranglers@lists.freedes |mesa-dev@lists.freedesktop.
   |ktop.org|org
Product|freedesktop.org |Mesa
  Component|New Accounts|Other

--- Comment #2 from Ilia Mirkin  ---
I've been doing a bunch of work on nouveau, have a large patch series pending,
and it's becoming a bit of a drag to get other people to commit stuff for me.

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

Re: [Mesa-dev] mesa-dev Digest, Vol 46, Issue 190

[Maxence Le Doré]

Those of Fredrik of course, since his implementation is better, I no
more work on multi bind.

2014/1/16  :
> Send mesa-dev mailing list submissions to
> mesa-dev@lists.freedesktop.org
>
> To subscribe or unsubscribe via the World Wide Web, visit
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> or, via email, send a message with subject or body 'help' to
> mesa-dev-requ...@lists.freedesktop.org
>
> You can reach the person managing the list at
> mesa-dev-ow...@lists.freedesktop.org
>
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of mesa-dev digest..."
>
>
> Today's Topics:
>
>1. [PATCH 2/3] mesa: Fix extension dependency forhalf-float
>   TexBOs (Ian Romanick)
>2. Re: [PATCH] glsl: Assert buildin uniform variables (Matt Turner)
>3. Re: [PATCH 05/10] nv50: assert before trying to out-of-bounds
>   access vtxbuf (Ilia Mirkin)
>4. Re: [PATCH 00/10] nv50: out-of-bounds access of context
>   specific attribs (Ilia Mirkin)
>5. Re: [PATCH 1/3] docs: Update GL3.txt due to recent work
>   (Matt Turner)
>
>
> --
>
> Message: 1
> Date: Thu, 16 Jan 2014 10:44:50 -0800
> From: "Ian Romanick" 
> To: mesa-dev@lists.freedesktop.org
> Cc: Ian Romanick 
> Subject: [Mesa-dev] [PATCH 2/3] mesa: Fix extension dependency for
> half-float TexBOs
> Message-ID: <1389897891-6088-2-git-send-email-...@freedesktop.org>
>
> From: Ian Romanick 
>
> Half-float TexBOs should require both GL_ARB_half_float_pixel and
> GL_ARB_texture_float.  This doesn't matter much in practice.  Every
> driver that supports GL_ARB_texture_buffer_object already supports
> GL_ARB_half_float_pixel.  We only expose the TexBO extension in core
> profiles, and those require GL_ARB_texture_float.
>
> Signed-off-by: Ian Romanick 
> ---
>  src/mesa/main/teximage.c | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
> index 8aac54e..bbd5006 100644
> --- a/src/mesa/main/teximage.c
> +++ b/src/mesa/main/teximage.c
> @@ -4150,7 +4150,18 @@ _mesa_validate_texbuffer_format(const struct 
> gl_context *ctx,
> if (datatype == GL_FLOAT && !ctx->Extensions.ARB_texture_float)
>return MESA_FORMAT_NONE;
>
> -   if (datatype == GL_HALF_FLOAT && !ctx->Extensions.ARB_half_float_pixel)
> +   /* The GL_ARB_texture_buffer_object spec says:
> +*
> +* "If ARB_texture_float is not supported, references to the
> +* floating-point internal formats provided by that extension should 
> be
> +* removed, and such formats may not be passed to TexBufferARB."
> +*
> +* As a result, GL_HALF_FLOAT internal format depends on both
> +* GL_ARB_texture_float and GL_ARB_half_float_pixel.
> +*/
> +   if (datatype == GL_HALF_FLOAT &&
> +   !(ctx->Extensions.ARB_half_float_pixel
> + && ctx->Extensions.ARB_texture_float))
>return MESA_FORMAT_NONE;
>
> if (!ctx->Extensions.ARB_texture_rg) {
> --
> 1.8.1.4
>
>
>
> --
>
> Message: 2
> Date: Thu, 16 Jan 2014 10:48:20 -0800
> From: Matt Turner 
> To: Juha-Pekka Heikkila 
> Cc: "mesa-dev@lists.freedesktop.org" 
> Subject: Re: [Mesa-dev] [PATCH] glsl: Assert buildin uniform variables
> Message-ID:
> 
> Content-Type: text/plain; charset=ISO-8859-1
>
> On Thu, Jan 16, 2014 at 2:59 AM, Juha-Pekka Heikkila
>  wrote:
>> Signed-off-by: Juha-Pekka Heikkila 
>> ---
>>  src/glsl/builtin_variables.cpp | 24 
>>  1 file changed, 20 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/glsl/builtin_variables.cpp b/src/glsl/builtin_variables.cpp
>> index f630923..9f4f7c7 100644
>> --- a/src/glsl/builtin_variables.cpp
>> +++ b/src/glsl/builtin_variables.cpp
>> @@ -669,7 +669,11 @@ void
>>  builtin_variable_generator::generate_uniforms()
>>  {
>> add_uniform(int_t, "gl_NumSamples");
>> -   add_uniform(type("gl_DepthRangeParameters"), "gl_DepthRange");
>> +
>> +   const glsl_type *const depth_range_parameters_type =
>> +  type("gl_DepthRangeParameters");
>> +   assert(depth_range_parameters_type);
>> +   add_uniform(depth_range_parameters_type, "gl_DepthRange");
>> add_uniform(array(vec4_t, VERT_ATTRIB_MAX), "gl_CurrentAttribVertMESA");
>> add_uniform(array(vec4_t, VARYING_SLOT_MAX), "gl_CurrentAttribFragMESA");
>>
>> @@ -688,7 +692,11 @@ builtin_variable_generator::generate_uniforms()
>>add_uniform(mat4_t, "gl_ProjectionMatrixInverseTranspose");
>>add_uniform(mat4_t, "gl_ModelViewProjectionMatrixInverseTranspose");
>>add_uniform(float_t, "gl_NormalScale");
>> -  add_uniform(type("gl_LightModelParameters"), "gl_LightModel");
>> +
>> +  const glsl_type *const light_model_parameters_type =
>> + type("gl_LightModelParameters");
>> +  assert(light_model_parameters_type);
>> +  add_uniform(light_model_

Re: [Mesa-dev] [PATCHv2] i965: Avoid extraneous fast depth clears

[Chad Versace]

On Fri, Jan 03, 2014 at 02:28:45PM +0800, Chia-I Wu wrote:
> When the depth buffer is already cleared, skip GEN6_HIZ_OP_DEPTH_CLEAR.  This
> is made possible by tracking which slices have been cleared in
> "struct intel_mipmap_level".  The hiz_cleared flag is unset when the depth
> buffer is rendered to or when a HiZ resolve is needed.
> 
> For Unigine Tropics, the FPS improvement is 1.32134% +/- 0.161878% (n=13).
> 
> v2:
> - unset hiz_cleared automatically in intel_miptree_slice_set_needs_hiz_resolve
> - set/unset hiz_cleared with intel_renderbuffer_att_set_needs_depth_resolve
> 
> Signed-off-by: Chia-I Wu 
> ---
>  src/mesa/drivers/dri/i965/brw_clear.c | 54 
> +++
>  src/mesa/drivers/dri/i965/brw_draw.c  |  2 +-
>  src/mesa/drivers/dri/i965/intel_fbo.c | 18 -
>  src/mesa/drivers/dri/i965/intel_fbo.h |  4 +-
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 39 +++
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 21 +++
>  6 files changed, 118 insertions(+), 20 deletions(-)

This patch looks good to me.
Reviewed-by: Chad Versace 

By the way, the patch no longer cleanly applies to master, but the
conflicts look trivial.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 2/2] svga: fix crash when clearing null color buffer

[Brian Paul]

Fixes regression since 9baa45f78b8ca7d66280e36009b6a685055d7cd6
but some of the piglit fbo-drawbuffers-none tests still don't
pass.
---
 src/gallium/drivers/svga/svga_pipe_clear.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/gallium/drivers/svga/svga_pipe_clear.c 
b/src/gallium/drivers/svga/svga_pipe_clear.c
index 47f6258..ebdce3b 100644
--- a/src/gallium/drivers/svga/svga_pipe_clear.c
+++ b/src/gallium/drivers/svga/svga_pipe_clear.c
@@ -111,9 +111,13 @@ svga_clear(struct pipe_context *pipe, unsigned buffers,
struct svga_context *svga = svga_context( pipe );
enum pipe_error ret;
 
-   if (buffers & PIPE_CLEAR_COLOR)
-  SVGA_DBG(DEBUG_DMA, "clear sid %p\n",
-   svga_surface(svga->curr.framebuffer.cbufs[0])->handle);
+   if (buffers & PIPE_CLEAR_COLOR) {
+  struct svga_winsys_handle *h = NULL;
+  if (svga->curr.framebuffer.cbufs[0]) {
+ h = svga_surface(svga->curr.framebuffer.cbufs[0])->handle;
+  }
+  SVGA_DBG(DEBUG_DMA, "clear sid %p\n", h);
+   }
 
/* flush any queued prims (don't want them to appear after the clear!) */
svga_hwtnl_flush_retry(svga);
-- 
1.8.3.2

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

[Mesa-dev] [PATCH 1/2] svga: replace an assertion with warning

[Brian Paul]

We trip on this in some environments but it's not a fatal error.

Signed-off-by: Brian Paul 
---
 src/gallium/drivers/svga/svga_resource_texture.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/svga/svga_resource_texture.c 
b/src/gallium/drivers/svga/svga_resource_texture.c
index 4feca53..109897f 100644
--- a/src/gallium/drivers/svga/svga_resource_texture.c
+++ b/src/gallium/drivers/svga/svga_resource_texture.c
@@ -205,7 +205,10 @@ svga_texture_get_handle(struct pipe_screen *screen,
struct svga_winsys_screen *sws = svga_winsys_screen(texture->screen);
unsigned stride;
 
-   assert(svga_texture(texture)->key.cachable == 0);
+   if (svga_texture(texture)->key.cachable) {
+  debug_warn_once("svga: texture->key.cachable=1");
+   }
+
svga_texture(texture)->key.cachable = 0;
stride = util_format_get_nblocksx(texture->format, texture->width0) *
 util_format_get_blocksize(texture->format);
-- 
1.8.3.2

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

Re: [Mesa-dev] [PATCH 01/10] loader: introduce the loader util lib

[Kristian Høgsberg]

On Mon, Jan 13, 2014 at 12:01:03AM +, Emil Velikov wrote:
> All the various window system integration layers duplicate roughly the
> same code for figuring out device and driver name, pci-id's, etc.  Which
> is sad.  So extract it out into a loader util lib.
> 
> Signed-off-by: Rob Clark 
> 
> v2 (Emil)
> * Separate the introduction of libloader from the code de-duplication.
> * Strip out non-pci devices support.
> * Add scons + Android build system support.
> * Add VISIBILITY_CFLAGS to avoid exporting the loader funcs.
> 
> v3 (Emil)
> * PIPE_OS_ANDROID is undefined at this scope, use ANDROID
> * Make sure we define _EGL_NO_DRM when building only swrast
> 
> Signed-off-by: Emil Velikov 
> ---
>  Android.mk|   1 +
>  configure.ac  |   1 +
>  include/pci_ids/pci_id_driver_map.h   |  24 ++-
>  src/Makefile.am   |   2 +-
>  src/SConscript|   1 +
>  src/egl/drivers/dri2/Android.mk   |   1 +
>  src/egl/drivers/dri2/Makefile.am  |   1 +
>  src/gallium/auxiliary/pipe-loader/Makefile.am |   1 +
>  src/gallium/state_trackers/egl/Makefile.am|   1 +
>  src/gallium/state_trackers/egl/SConscript |   1 +
>  src/gallium/targets/egl-static/Android.mk |   1 +
>  src/gallium/targets/egl-static/Makefile.am|   1 +
>  src/gallium/targets/egl-static/SConscript |   1 +
>  src/gbm/Makefile.am   |   6 +-
>  src/glx/Makefile.am   |   1 +
>  src/glx/SConscript|   1 +
>  src/loader/Android.mk |  48 +
>  src/loader/Makefile.am|  37 
>  src/loader/Makefile.sources   |   2 +
>  src/loader/SConscript |  28 +++
>  src/loader/loader.c   | 264 
> ++
>  src/loader/loader.h   |  57 ++
>  22 files changed, 466 insertions(+), 15 deletions(-)
>  create mode 100644 src/loader/Android.mk
>  create mode 100644 src/loader/Makefile.am
>  create mode 100644 src/loader/Makefile.sources
>  create mode 100644 src/loader/SConscript
>  create mode 100644 src/loader/loader.c
>  create mode 100644 src/loader/loader.h
> 
> diff --git a/Android.mk b/Android.mk
> index c3c1f08..05ed62f 100644
> --- a/Android.mk
> +++ b/Android.mk
> @@ -78,6 +78,7 @@ endif
>  ifneq ($(strip $(MESA_GPU_DRIVERS)),)
>  
>  SUBDIRS := \
> + src/loader \
>   src/mapi \
>   src/glsl \
>   src/mesa \
> diff --git a/configure.ac b/configure.ac
> index 4b55140..ce677d8 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -2059,6 +2059,7 @@ AC_CONFIG_FILES([Makefile
>   src/glx/Makefile
>   src/glx/tests/Makefile
>   src/gtest/Makefile
> + src/loader/Makefile
>   src/mapi/Makefile
>   src/mapi/es1api/Makefile
>   src/mapi/es1api/glesv1_cm.pc
> diff --git a/include/pci_ids/pci_id_driver_map.h 
> b/include/pci_ids/pci_id_driver_map.h
> index 8a97c6f..2e88451 100644
> --- a/include/pci_ids/pci_id_driver_map.h
> +++ b/include/pci_ids/pci_id_driver_map.h
> @@ -2,6 +2,7 @@
>  #define _PCI_ID_DRIVER_MAP_H_
>  
>  #include 
> +#include "loader.h"
>  
>  #ifndef ARRAY_SIZE
>  #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
> @@ -19,7 +20,6 @@ static const int i965_chip_ids[] = {
>  #undef CHIPSET
>  };
>  
> -#ifndef DRIVER_MAP_GALLIUM_ONLY
>  static const int r100_chip_ids[] = {
>  #define CHIPSET(chip, name, family) chip,
>  #include "pci_ids/radeon_pci_ids.h"
> @@ -31,7 +31,6 @@ static const int r200_chip_ids[] = {
>  #include "pci_ids/r200_pci_ids.h"
>  #undef CHIPSET
>  };
> -#endif
>  
>  static const int r300_chip_ids[] = {
>  #define CHIPSET(chip, name, family) chip,
> @@ -62,18 +61,17 @@ static const struct {
> const char *driver;
> const int *chip_ids;
> int num_chips_ids;
> +   unsigned driver_types;
>  } driver_map[] = {
> -   { 0x8086, "i915", i915_chip_ids, ARRAY_SIZE(i915_chip_ids) },
> -   { 0x8086, "i965", i965_chip_ids, ARRAY_SIZE(i965_chip_ids) },
> -#ifndef DRIVER_MAP_GALLIUM_ONLY
> -   { 0x1002, "radeon", r100_chip_ids, ARRAY_SIZE(r100_chip_ids) },
> -   { 0x1002, "r200", r200_chip_ids, ARRAY_SIZE(r200_chip_ids) },
> -#endif
> -   { 0x1002, "r300", r300_chip_ids, ARRAY_SIZE(r300_chip_ids) },
> -   { 0x1002, "r600", r600_chip_ids, ARRAY_SIZE(r600_chip_ids) },
> -   { 0x1002, "radeonsi", radeonsi_chip_ids, ARRAY_SIZE(radeonsi_chip_ids) },
> -   { 0x10de, "nouveau", NULL, -1 },
> -   { 0x15ad, "vmwgfx", vmwgfx_chip_ids, ARRAY_SIZE(vmwgfx_chip_ids) },
> +   { 0x8086, "i915", i915_chip_ids, ARRAY_SIZE(i915_chip_ids), _LOADER_DRI | 
> _LOADER_GALLIUM },
> +   { 0x8086, "i965", i965_chip_ids, ARRAY_SIZE(i965_chip_ids), _LOADER_DRI | 
> _LOADER_GALLIUM },
> +   { 0x1002, "radeon", r100_chip_ids, ARRAY_SIZE(r100_chip_ids), _LOADER_DRI 
> },
> +   {

Re: [Mesa-dev] [PATCH 02/10] glx: use the loader util lib

[Kristian Høgsberg]

On Sat, Jan 11, 2014 at 04:54:43PM +, Emil Velikov wrote:
> Signed-off-by: Emil Velikov 
> ---
>  src/glx/Makefile.am   |   5 +-
>  src/glx/SConscript|   2 +-
>  src/glx/dri3_common.c | 146 
> --
>  src/glx/dri3_glx.c|   3 +-
>  src/glx/dri3_priv.h   |   3 --
>  5 files changed, 6 insertions(+), 153 deletions(-)
>  delete mode 100644 src/glx/dri3_common.c
> 
> diff --git a/src/glx/Makefile.am b/src/glx/Makefile.am
> index 6d81471..54a0cc0 100644
> --- a/src/glx/Makefile.am
> +++ b/src/glx/Makefile.am
> @@ -99,10 +99,11 @@ libglx_la_SOURCES = \
>  
>  if HAVE_DRI3
>  libglx_la_SOURCES += \
> -  dri3_glx.c \
> -  dri3_common.c
> +  dri3_glx.c
>  endif
>  
> +libglx_la_LIBADD = $(top_builddir)/src/loader/libloader.la
> +
>  GL_LIBS = \
>   libglx.la \
>   $(SHARED_GLAPI_LIBS) \
> diff --git a/src/glx/SConscript b/src/glx/SConscript
> index 309a648..cc36bf0 100644
> --- a/src/glx/SConscript
> +++ b/src/glx/SConscript
> @@ -27,6 +27,7 @@ env.Append(CPPDEFINES = [
>  ])
>  
>  env.Prepend(LIBS = [
> +libloader,
>  glapi
>  ])
>  
> @@ -82,7 +83,6 @@ sources = [
>  'dri2.c',
>  'dri2_query_renderer.c',
>  #'dri3_glx.c',
> -#'dri3_common.c',
>  'applegl_glx.c',
>  ]
>  
> diff --git a/src/glx/dri3_common.c b/src/glx/dri3_common.c
> deleted file mode 100644
> index c758f96..000
> --- a/src/glx/dri3_common.c
> +++ /dev/null
> @@ -1,146 +0,0 @@
> -/*
> - * Copyright © 2013 Keith Packard
> - *
> - * Permission to use, copy, modify, distribute, and sell this software and 
> its
> - * documentation for any purpose is hereby granted without fee, provided that
> - * the above copyright notice appear in all copies and that both that 
> copyright
> - * notice and this permission notice appear in supporting documentation, and
> - * that the name of the copyright holders not be used in advertising or
> - * publicity pertaining to distribution of the software without specific,
> - * written prior permission.  The copyright holders make no representations
> - * about the suitability of this software for any purpose.  It is provided 
> "as
> - * is" without express or implied warranty.
> - *
> - * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS 
> SOFTWARE,
> - * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
> - * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
> - * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF 
> USE,
> - * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
> - * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR 
> PERFORMANCE
> - * OF THIS SOFTWARE.
> - */
> -
> -/*
> - * This code is derived from src/egl/drivers/dri2/common.c which
> - * carries the following copyright:
> - * 
> - * Copyright © 2011 Intel Corporation
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a
> - * copy of this software and associated documentation files (the "Software"),
> - * to deal in the Software without restriction, including without limitation
> - * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> - * and/or sell copies of the Software, and to permit persons to whom the
> - * Software is furnished to do so, subject to the following conditions:
> - *
> - * The above copyright notice and this permission notice (including the next
> - * paragraph) shall be included in all copies or substantial portions of the
> - * Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> - * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> - * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> - * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> - * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> - * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> - * DEALINGS IN THE SOFTWARE.
> - *
> - * Authors:
> - *Kristian Høgsberg 
> - *Benjamin Franzke 
> - */
> -
> -#include 
> -#include 
> -
> -#include 
> -#include 
> -#include 
> -#include 
> -#include "glapi.h"
> -#include "glxclient.h"
> -#include "xf86dri.h"
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include "xf86drm.h"
> -#include "dri_common.h"
> -#include "dri3_priv.h"
> -
> -#define DRIVER_MAP_DRI3_ONLY
> -#include "pci_ids/pci_id_driver_map.h"
> -
> -#include 
> -
> -static struct udev_device *
> -dri3_udev_device_new_from_fd(struct udev *udev, int fd)
> -{
> -   struct udev_device *device;
> -   struct stat buf;
> -
> -   if (fstat(fd, &buf) < 0) {
> -  ErrorMessageF("DRI3: failed to stat fd %d", fd);
> -  return NULL;
> -   }
> -
> -   device = udev_device_new_from_devnum(udev, 'c', buf.st_rdev);
> -   if (device == NULL) {
> -  ErrorMessageF("DRI3: could not

Re: [Mesa-dev] [PATCH 03/10] gbm: use the loader util lib

[Kristian Høgsberg]

On Sat, Jan 11, 2014 at 04:54:44PM +, Emil Velikov wrote:
> Additionally this commit removes the following exported functions
> 
>_gbm_udev_device_new_from_fd()
>_gbm_fd_get_device_name()
>_gbm_log()
> 
> All three were erroneously marked as exported since their inception.
> Neither of them has ever been a part of the API thus there should be
> no users of them.
> 
> Cc: Chad Versace 
> Cc: Kristian Høgsberg 
> Signed-off-by: Emil Velikov 
> ---
> 
> Chad, Kristian
> 
> Can you give us some tips of there are any non-mesa users of the
> above functions. They seems to have never been exposed by gbm api
> but were marked as exported since day one :\

They were exported so that external gbm modules could call them.
Today we compile the gbm dri backend into libgbm, so it's not an
issue.  Nothing outside mesa should use it.

Reviewed-by: Kristian Høgsberg 

> Thanks
> Emil
> ---
> 
>  .../state_trackers/gbm/gbm_gallium_drmint.h|  1 -
>  src/gbm/Makefile.am|  7 +-
>  src/gbm/backends/dri/driver_name.c | 89 
> --
>  src/gbm/backends/dri/gbm_dri.c |  3 +-
>  src/gbm/backends/dri/gbm_driint.h  |  4 -
>  src/gbm/main/common.c  | 88 -
>  src/gbm/main/common.h  | 42 --
>  src/gbm/main/gbm.c |  1 -
>  8 files changed, 5 insertions(+), 230 deletions(-)
>  delete mode 100644 src/gbm/backends/dri/driver_name.c
>  delete mode 100644 src/gbm/main/common.c
>  delete mode 100644 src/gbm/main/common.h
> 
> diff --git a/src/gallium/state_trackers/gbm/gbm_gallium_drmint.h 
> b/src/gallium/state_trackers/gbm/gbm_gallium_drmint.h
> index a5d6d83..4534dd8 100644
> --- a/src/gallium/state_trackers/gbm/gbm_gallium_drmint.h
> +++ b/src/gallium/state_trackers/gbm/gbm_gallium_drmint.h
> @@ -32,7 +32,6 @@
>  
>  #include "gbmint.h"
>  
> -#include "common.h"
>  #include "common_drm.h"
>  
>  struct gbm_gallium_drm_device {
> diff --git a/src/gbm/Makefile.am b/src/gbm/Makefile.am
> index 5ae7a68..4030cc6 100644
> --- a/src/gbm/Makefile.am
> +++ b/src/gbm/Makefile.am
> @@ -19,10 +19,10 @@ include_HEADERS = main/gbm.h
>  
>  libgbm_la_SOURCES = \
>   main/gbm.c \
> - main/backend.c \
> - main/common.c
> + main/backend.c
>  libgbm_la_LDFLAGS = -version-info 1:0
>  libgbm_la_LIBADD = \
> + $(top_builddir)/src/loader/libloader.la \
>   $(LIBUDEV_LIBS) \
>   $(LIBKMS_LIBS) \
>   $(DLOPEN_LIBS)
> @@ -36,8 +36,7 @@ endif
>  if HAVE_DRI
>  noinst_LTLIBRARIES = libgbm_dri.la
>  libgbm_dri_la_SOURCES = \
> - backends/dri/gbm_dri.c \
> - backends/dri/driver_name.c
> + backends/dri/gbm_dri.c
>  
>  libgbm_dri_la_CFLAGS = \
>   $(AM_CFLAGS) \
> diff --git a/src/gbm/backends/dri/driver_name.c 
> b/src/gbm/backends/dri/driver_name.c
> deleted file mode 100644
> index 2ed209f..000
> --- a/src/gbm/backends/dri/driver_name.c
> +++ /dev/null
> @@ -1,89 +0,0 @@
> -/*
> - * Copyright © 2011 Intel Corporation
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a
> - * copy of this software and associated documentation files (the "Software"),
> - * to deal in the Software without restriction, including without limitation
> - * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> - * and/or sell copies of the Software, and to permit persons to whom the
> - * Software is furnished to do so, subject to the following conditions:
> - *
> - * The above copyright notice and this permission notice (including the next
> - * paragraph) shall be included in all copies or substantial portions of the
> - * Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> - * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> - * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> - * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> - * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> - * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> - * DEALINGS IN THE SOFTWARE.
> - *
> - * Authors:
> - *Kristian Høgsberg 
> - *Benjamin Franzke 
> - */
> -
> -#include 
> -#include 
> -
> -#include 
> -
> -#include "gbm_driint.h"
> -#define DRIVER_MAP_DRI2_ONLY
> -#include "pci_ids/pci_id_driver_map.h"
> -
> -char *
> -dri_fd_get_driver_name(int fd)
> -{
> -   struct udev *udev;
> -   struct udev_device *device, *parent;
> -   const char *pci_id;
> -   char *driver = NULL;
> -   int vendor_id, chip_id, i, j;
> -
> -   udev = udev_new();
> -   device = _gbm_udev_device_new_from_fd(udev, fd);
> -   if (device == NULL)
> -  return NULL;
> -
> -   parent = udev_device_get_parent(device);
> -   if (parent == NULL) {
> -  fprintf(stderr, "gbm: could not get parent device");
> - 

Re: [Mesa-dev] code de-duplication and non-pci support v2

[Kristian Høgsberg]

On Sat, Jan 11, 2014 at 04:51:42PM -0500, Rob Clark wrote:
> On Sat, Jan 11, 2014 at 11:54 AM, Emil Velikov  
> wrote:
> > This is an updated series of Rob's patches
> >
> > * The introduction of the util library is separated from the
> > de-duplication.
> > * Each commit targets individual part of mesa and it should
> > build/work regardless of build system/options.
> > * Handles a couple more cases of de-duplication.
> > * Hides the loader funcs so that they are not exported.
> > * Building platform_android, will correctly set the logger to
> > _eglLog(), which on itself is rapped around Androids (A)LOG.
> > * Non-pci devices support has been ripped out and is left at
> > the end of the series.
> > * automake and scons build tested, Android should after
> > correcting the following
> > defined(PIPE_OS_ANDROID) && !defined(_EGL_NO_DRM)
> 
> Very cool, thanks for taking this and running with it.  Especially the
> other build systems which I have no clue about :-)
> 
> I had a look at the patches, and they look good.  So with the
> disclaimer that I am certainly not expert on these parts, for the
> series:
> 
> Reviewed-by: Rob Clark 

I'm not familiar with the other build systems either, but aside for
those two issues I pointed out

Reviewed-by: Kristian Høgsberg 

for the series.

> > Brief list of which patches affect which build system
> > (android A, automake M, scons S)
> > patch 1 - A, M, S
> > patch 2 - M, S
> > patch 3 - M
> > patch 4 - A, M, S
> > patch 5 - M, S
> > patch 6 - M
> > patch 7 - A, M
> > patch 8 - S
> >
> > Notes:
> > * Eric's comment about moving the driver_name allocation to
> > egl_dri2.c does not seem easily achiveable due to platform_x11.
> >
> > * Keith's patch can be relatively easily rebased on top of this.
> >
> > * Andoid logging should work via (A)LOG.
> >
> > Cheers,
> > Emil
> >
> > Emil Velikov (9):
> >   loader: introduce the loader util lib
> >   glx: use the loader util lib
> >   gbm: use the loader util lib
> >   egl-static: use loader util lib
> >   st/egl: use loader util lib
> >   pipe-loader: use loader util lib
> >   egl_dri2: use loader util lib
> >   pci_ids: no not include loader.h
> >   pipe-loader: add support for non-pci (platform) devices
> >
> > Rob Clark (1):
> >   loader: fallback to drmGetVersion() for non-pci devices
> >
> >  Android.mk |   1 +
> >  configure.ac   |   1 +
> >  include/pci_ids/pci_id_driver_map.h|  27 +-
> >  src/Makefile.am|   2 +-
> >  src/SConscript |   1 +
> >  src/egl/drivers/dri2/Android.mk|   5 +-
> >  src/egl/drivers/dri2/Makefile.am   |   5 +-
> >  src/egl/drivers/dri2/common.c  | 144 ---
> >  src/egl/drivers/dri2/egl_dri2.h|   5 -
> >  src/egl/drivers/dri2/platform_android.c| 107 +---
> >  src/egl/drivers/dri2/platform_drm.c|   5 +-
> >  src/egl/drivers/dri2/platform_wayland.c|   5 +-
> >  src/egl/main/Android.mk|   1 +
> >  src/gallium/auxiliary/pipe-loader/Makefile.am  |   8 +-
> >  src/gallium/auxiliary/pipe-loader/pipe_loader.h|   1 +
> >  .../auxiliary/pipe-loader/pipe_loader_drm.c|  92 +--
> >  src/gallium/state_trackers/clover/core/device.cpp  |   2 +
> >  src/gallium/state_trackers/egl/Makefile.am |   2 +
> >  src/gallium/state_trackers/egl/SConscript  |   4 +
> >  src/gallium/state_trackers/egl/drm/native_drm.c|  44 +---
> >  .../state_trackers/gbm/gbm_gallium_drmint.h|   1 -
> >  src/gallium/targets/egl-static/Android.mk  |   1 +
> >  src/gallium/targets/egl-static/Makefile.am |   2 +
> >  src/gallium/targets/egl-static/SConscript  |   2 +
> >  src/gallium/targets/egl-static/egl.c   | 185 +-
> >  src/gbm/Makefile.am|  13 +-
> >  src/gbm/backends/dri/driver_name.c |  89 ---
> >  src/gbm/backends/dri/gbm_dri.c |   3 +-
> >  src/gbm/backends/dri/gbm_driint.h  |   4 -
> >  src/gbm/main/common.c  |  88 ---
> >  src/gbm/main/common.h  |  42 
> >  src/gbm/main/gbm.c |   1 -
> >  src/glx/Makefile.am|   6 +-
> >  src/glx/SConscript |   3 +-
> >  src/glx/dri3_common.c  | 146 ---
> >  src/glx/dri3_glx.c |   3 +-
> >  src/glx/dri3_priv.h|   3 -
> >  src/loader/Android.mk  |  43 
> >  src/loader/Makefile.am |  37 +++
> >  src/loader/Makefile.sources|   2 +

Re: [Mesa-dev] [PATCH 03/10] gbm: use the loader util lib

[Emil Velikov]

On 16/01/14 23:03, Kristian Høgsberg wrote:
> On Sat, Jan 11, 2014 at 04:54:44PM +, Emil Velikov wrote:
>> Additionally this commit removes the following exported functions
>>
>>_gbm_udev_device_new_from_fd()
>>_gbm_fd_get_device_name()
>>_gbm_log()
>>
>> All three were erroneously marked as exported since their inception.
>> Neither of them has ever been a part of the API thus there should be
>> no users of them.
>>
>> Cc: Chad Versace 
>> Cc: Kristian Høgsberg 
>> Signed-off-by: Emil Velikov 
>> ---
>>
>> Chad, Kristian
>>
>> Can you give us some tips of there are any non-mesa users of the
>> above functions. They seems to have never been exposed by gbm api
>> but were marked as exported since day one :\
> 
> They were exported so that external gbm modules could call them.
> Today we compile the gbm dri backend into libgbm, so it's not an
> issue.  Nothing outside mesa should use it.
> 
If that's the case I think you won't mind if I follow up with another
patch that removes the rest of the functions that are not in the API/are
used only by the backend(s).

Thanks
Emil

> Reviewed-by: Kristian Høgsberg 
> 
>> Thanks
>> Emil
>> ---
>>
>>  .../state_trackers/gbm/gbm_gallium_drmint.h|  1 -
>>  src/gbm/Makefile.am|  7 +-
>>  src/gbm/backends/dri/driver_name.c | 89 
>> --
>>  src/gbm/backends/dri/gbm_dri.c |  3 +-
>>  src/gbm/backends/dri/gbm_driint.h  |  4 -
>>  src/gbm/main/common.c  | 88 
>> -
>>  src/gbm/main/common.h  | 42 --
>>  src/gbm/main/gbm.c |  1 -
>>  8 files changed, 5 insertions(+), 230 deletions(-)
>>  delete mode 100644 src/gbm/backends/dri/driver_name.c
>>  delete mode 100644 src/gbm/main/common.c
>>  delete mode 100644 src/gbm/main/common.h
>>
>> diff --git a/src/gallium/state_trackers/gbm/gbm_gallium_drmint.h 
>> b/src/gallium/state_trackers/gbm/gbm_gallium_drmint.h
>> index a5d6d83..4534dd8 100644
>> --- a/src/gallium/state_trackers/gbm/gbm_gallium_drmint.h
>> +++ b/src/gallium/state_trackers/gbm/gbm_gallium_drmint.h
>> @@ -32,7 +32,6 @@
>>  
>>  #include "gbmint.h"
>>  
>> -#include "common.h"
>>  #include "common_drm.h"
>>  
>>  struct gbm_gallium_drm_device {
>> diff --git a/src/gbm/Makefile.am b/src/gbm/Makefile.am
>> index 5ae7a68..4030cc6 100644
>> --- a/src/gbm/Makefile.am
>> +++ b/src/gbm/Makefile.am
>> @@ -19,10 +19,10 @@ include_HEADERS = main/gbm.h
>>  
>>  libgbm_la_SOURCES = \
>>  main/gbm.c \
>> -main/backend.c \
>> -main/common.c
>> +main/backend.c
>>  libgbm_la_LDFLAGS = -version-info 1:0
>>  libgbm_la_LIBADD = \
>> +$(top_builddir)/src/loader/libloader.la \
>>  $(LIBUDEV_LIBS) \
>>  $(LIBKMS_LIBS) \
>>  $(DLOPEN_LIBS)
>> @@ -36,8 +36,7 @@ endif
>>  if HAVE_DRI
>>  noinst_LTLIBRARIES = libgbm_dri.la
>>  libgbm_dri_la_SOURCES = \
>> -backends/dri/gbm_dri.c \
>> -backends/dri/driver_name.c
>> +backends/dri/gbm_dri.c
>>  
>>  libgbm_dri_la_CFLAGS = \
>>  $(AM_CFLAGS) \
>> diff --git a/src/gbm/backends/dri/driver_name.c 
>> b/src/gbm/backends/dri/driver_name.c
>> deleted file mode 100644
>> index 2ed209f..000
>> --- a/src/gbm/backends/dri/driver_name.c
>> +++ /dev/null
>> @@ -1,89 +0,0 @@
>> -/*
>> - * Copyright © 2011 Intel Corporation
>> - *
>> - * Permission is hereby granted, free of charge, to any person obtaining a
>> - * copy of this software and associated documentation files (the 
>> "Software"),
>> - * to deal in the Software without restriction, including without limitation
>> - * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> - * and/or sell copies of the Software, and to permit persons to whom the
>> - * Software is furnished to do so, subject to the following conditions:
>> - *
>> - * The above copyright notice and this permission notice (including the next
>> - * paragraph) shall be included in all copies or substantial portions of the
>> - * Software.
>> - *
>> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> - * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> - * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>> - * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
>> - * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
>> - * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>> - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>> - * DEALINGS IN THE SOFTWARE.
>> - *
>> - * Authors:
>> - *Kristian Høgsberg 
>> - *Benjamin Franzke 
>> - */
>> -
>> -#include 
>> -#include 
>> -
>> -#include 
>> -
>> -#include "gbm_driint.h"
>> -#define DRIVER_MAP_DRI2_ONLY
>> -#include "pci_ids/pci_id_driver_map.h"
>> -
>> -char *
>> -dri_fd_get_driver_name(int fd)
>> -{
>> -   struct udev *udev;
>> -   struct udev_device *device, *parent;
>> -   c

Re: [Mesa-dev] code de-duplication and non-pci support v2

[Emil Velikov]

On 16/01/14 23:29, Kristian Høgsberg wrote:
> On Sat, Jan 11, 2014 at 04:51:42PM -0500, Rob Clark wrote:
>> On Sat, Jan 11, 2014 at 11:54 AM, Emil Velikov  
>> wrote:
>>> This is an updated series of Rob's patches
>>>
>>> * The introduction of the util library is separated from the
>>> de-duplication.
>>> * Each commit targets individual part of mesa and it should
>>> build/work regardless of build system/options.
>>> * Handles a couple more cases of de-duplication.
>>> * Hides the loader funcs so that they are not exported.
>>> * Building platform_android, will correctly set the logger to
>>> _eglLog(), which on itself is rapped around Androids (A)LOG.
>>> * Non-pci devices support has been ripped out and is left at
>>> the end of the series.
>>> * automake and scons build tested, Android should after
>>> correcting the following
>>> defined(PIPE_OS_ANDROID) && !defined(_EGL_NO_DRM)
>>
>> Very cool, thanks for taking this and running with it.  Especially the
>> other build systems which I have no clue about :-)
>>
>> I had a look at the patches, and they look good.  So with the
>> disclaimer that I am certainly not expert on these parts, for the
>> series:
>>
>> Reviewed-by: Rob Clark 
> 
> I'm not familiar with the other build systems either, but aside for
> those two issues I pointed out
> 
Good points Kristian I'll update patches 1 & 2.

I'm suspecting that there will not be many more people interested in
reviewing/testing the series, so unless someone has anything to add I
plan on pushing it this Saturday.

Cheers
Emil

> Reviewed-by: Kristian Høgsberg 
> 
> for the series.
> 
>>> Brief list of which patches affect which build system
>>> (android A, automake M, scons S)
>>> patch 1 - A, M, S
>>> patch 2 - M, S
>>> patch 3 - M
>>> patch 4 - A, M, S
>>> patch 5 - M, S
>>> patch 6 - M
>>> patch 7 - A, M
>>> patch 8 - S
>>>
>>> Notes:
>>> * Eric's comment about moving the driver_name allocation to
>>> egl_dri2.c does not seem easily achiveable due to platform_x11.
>>>
>>> * Keith's patch can be relatively easily rebased on top of this.
>>>
>>> * Andoid logging should work via (A)LOG.
>>>
>>> Cheers,
>>> Emil
>>>
>>> Emil Velikov (9):
>>>   loader: introduce the loader util lib
>>>   glx: use the loader util lib
>>>   gbm: use the loader util lib
>>>   egl-static: use loader util lib
>>>   st/egl: use loader util lib
>>>   pipe-loader: use loader util lib
>>>   egl_dri2: use loader util lib
>>>   pci_ids: no not include loader.h
>>>   pipe-loader: add support for non-pci (platform) devices
>>>
>>> Rob Clark (1):
>>>   loader: fallback to drmGetVersion() for non-pci devices
>>>
>>>  Android.mk |   1 +
>>>  configure.ac   |   1 +
>>>  include/pci_ids/pci_id_driver_map.h|  27 +-
>>>  src/Makefile.am|   2 +-
>>>  src/SConscript |   1 +
>>>  src/egl/drivers/dri2/Android.mk|   5 +-
>>>  src/egl/drivers/dri2/Makefile.am   |   5 +-
>>>  src/egl/drivers/dri2/common.c  | 144 ---
>>>  src/egl/drivers/dri2/egl_dri2.h|   5 -
>>>  src/egl/drivers/dri2/platform_android.c| 107 +---
>>>  src/egl/drivers/dri2/platform_drm.c|   5 +-
>>>  src/egl/drivers/dri2/platform_wayland.c|   5 +-
>>>  src/egl/main/Android.mk|   1 +
>>>  src/gallium/auxiliary/pipe-loader/Makefile.am  |   8 +-
>>>  src/gallium/auxiliary/pipe-loader/pipe_loader.h|   1 +
>>>  .../auxiliary/pipe-loader/pipe_loader_drm.c|  92 +--
>>>  src/gallium/state_trackers/clover/core/device.cpp  |   2 +
>>>  src/gallium/state_trackers/egl/Makefile.am |   2 +
>>>  src/gallium/state_trackers/egl/SConscript  |   4 +
>>>  src/gallium/state_trackers/egl/drm/native_drm.c|  44 +---
>>>  .../state_trackers/gbm/gbm_gallium_drmint.h|   1 -
>>>  src/gallium/targets/egl-static/Android.mk  |   1 +
>>>  src/gallium/targets/egl-static/Makefile.am |   2 +
>>>  src/gallium/targets/egl-static/SConscript  |   2 +
>>>  src/gallium/targets/egl-static/egl.c   | 185 +-
>>>  src/gbm/Makefile.am|  13 +-
>>>  src/gbm/backends/dri/driver_name.c |  89 ---
>>>  src/gbm/backends/dri/gbm_dri.c |   3 +-
>>>  src/gbm/backends/dri/gbm_driint.h  |   4 -
>>>  src/gbm/main/common.c  |  88 ---
>>>  src/gbm/main/common.h  |  42 
>>>  src/gbm/main/gbm.c |   1 -
>>>  src/glx/Makefile.am|   6 +-
>>>  src/glx/SConscript |   3 +-
>>>  src/glx/dri3_common.c  | 146 ---
>>>  src/glx/dri3_gl

Re: [Mesa-dev] [PATCH 03/10] gbm: use the loader util lib

[Emil Velikov]

On 16/01/14 23:45, Emil Velikov wrote:
>> > They were exported so that external gbm modules could call them.
>> > Today we compile the gbm dri backend into libgbm, so it's not an
>> > issue.  Nothing outside mesa should use it.
>> > 
> If that's the case I think you won't mind if I follow up with another
> patch that removes the rest of the functions that are not in the API/are
> used only by the backend(s).
> 
> Thanks
> Emil
> 
And by "remove" I mean make sure they are not exported.

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

Re: [Mesa-dev] [PATCH 1/9] Mesa: Add ARB_arrays_of_arrays

[Matt Turner]

> Mesa: Add ARB_arrays_of_arrays

Nit picky, but "mesa" like the rest of the commit messages already in git?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [Mesa-stable] [PATCH v2] mesa: fix GL_COLOR_SUM enum for drivers without ARB_vertex_program

[Ian Romanick]

On 01/16/2014 10:42 AM, Ilia Mirkin wrote:
> On Thu, Jan 16, 2014 at 1:38 PM, Ilia Mirkin  wrote:
>> On Thu, Jan 16, 2014 at 1:34 PM, Ian Romanick  wrote:
>>> On 01/16/2014 10:13 AM, Ilia Mirkin wrote:
 Commit c13970808 (mesa: GL_EXT_secondary_color is not optional) changed

 CHECK_EXTENSION2(EXT_secondary_color, ARB_vetex_program, cap)

 to

 CHECK_EXTENSION(ARB_vertex_program, cap)

 However CHECK_EXTENSION2 checks that either extension is available, not
 both. Remove the extension check entirely since the intent was for it to
 always be enabled.

 Signed-off-by: Ilia Mirkin 
 Cc: 9.2 10.0 
>>>
>>> Reviewed-by: Ian Romanick 
>>
>> Thanks! Can you take care of checking it in? I don't have an fd.o account.
>>
>>>
 ---

 I hope this is what you meant by the get_hash_params.py change. I'm not
 entirely clear on when one should use NO_EXTRA vs a flush of some sort.
>>>
>>> The various flushes are only needed for queries of derived state (like
>>> the current secondary color query).
> 
> Actually if the extra_ARB_vertex_program was needed before -- wouldn't
> it still be needed now? Presumably it only does work if that extension
> exists, and GL_COLOR_SUM somehow interacts with it. (Sorry, my
> knowledge of the details is rather weak.)

I was just looking at the EXT_secondary_color spec:

Accepted by the  parameter of Enable, Disable, and IsEnabled,
and by the  parameter of GetBooleanv, GetIntegerv, GetFloatv,
and GetDoublev:

COLOR_SUM_EXT   0x8458

Since GL_EXT_secondary_color is always available, the enable, disable,
and query calls should always work.

I'll push your patch in a couple minutes...

  src/mesa/main/enable.c   | 2 --
  src/mesa/main/get_hash_params.py | 2 +-
  2 files changed, 1 insertion(+), 3 deletions(-)

 diff --git a/src/mesa/main/enable.c b/src/mesa/main/enable.c
 index bb4a23c..fca3068 100644
 --- a/src/mesa/main/enable.c
 +++ b/src/mesa/main/enable.c
 @@ -762,7 +762,6 @@ _mesa_set_enable(struct gl_context *ctx, GLenum cap, 
 GLboolean state)
case GL_COLOR_SUM_EXT:
   if (ctx->API != API_OPENGL_COMPAT)
  goto invalid_enum_error;
 - CHECK_EXTENSION(ARB_vertex_program, cap);
   if (ctx->Fog.ColorSumEnabled == state)
  return;
   FLUSH_VERTICES(ctx, _NEW_FOG);
 @@ -1443,7 +1442,6 @@ _mesa_IsEnabled( GLenum cap )
case GL_COLOR_SUM_EXT:
   if (ctx->API != API_OPENGL_COMPAT)
  goto invalid_enum_error;
 - CHECK_EXTENSION(ARB_vertex_program);
   return ctx->Fog.ColorSumEnabled;

/* GL_ARB_multisample */
 diff --git a/src/mesa/main/get_hash_params.py 
 b/src/mesa/main/get_hash_params.py
 index 6195d63..bc2bbaf 100644
 --- a/src/mesa/main/get_hash_params.py
 +++ b/src/mesa/main/get_hash_params.py
 @@ -541,7 +541,7 @@ descriptor=[
[ "TRANSPOSE_TEXTURE_MATRIX_ARB", 
 "CONTEXT_MATRIX_T(TextureMatrixStack), NO_EXTRA" ],

  # GL_EXT_secondary_color
 -  [ "COLOR_SUM", "CONTEXT_BOOL(Fog.ColorSumEnabled), 
 extra_ARB_vertex_program" ],
 +  [ "COLOR_SUM", "CONTEXT_BOOL(Fog.ColorSumEnabled), NO_EXTRA" ],
[ "CURRENT_SECONDARY_COLOR", 
 "CONTEXT_FIELD(Current.Attrib[VERT_ATTRIB_COLOR1][0], TYPE_FLOATN_4), 
 extra_flush_current" ],
[ "SECONDARY_COLOR_ARRAY", 
 "ARRAY_BOOL(VertexAttrib[VERT_ATTRIB_COLOR1].Enabled), NO_EXTRA" ],
[ "SECONDARY_COLOR_ARRAY_TYPE", 
 "ARRAY_ENUM(VertexAttrib[VERT_ATTRIB_COLOR1].Type), NO_EXTRA" ],

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

Re: [Mesa-dev] [PATCH 2/9] glsl: add dimension_count to glsl_types

[Matt Turner]

On Wed, Jan 15, 2014 at 10:27 PM, Timothy Arceri  wrote:
> Signed-off-by: Timothy Arceri 
> ---
>  src/glsl/glsl_types.cpp | 15 +++
>  src/glsl/glsl_types.h   | 17 -
>  2 files changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
> index 12d4ac0..1c9add7 100644
> --- a/src/glsl/glsl_types.cpp
> +++ b/src/glsl/glsl_types.cpp
> @@ -277,12 +277,13 @@ _mesa_glsl_release_types(void)
>  }
>
>
> -glsl_type::glsl_type(const glsl_type *array, unsigned length) :
> +glsl_type::glsl_type(const glsl_type *array, unsigned length,
> + unsigned dimension_count) :
> base_type(GLSL_TYPE_ARRAY),
> sampler_dimensionality(0), sampler_shadow(0), sampler_array(0),
> sampler_type(0), interface_packing(0),
> vector_elements(0), matrix_columns(0),
> -   name(NULL), length(length)
> +   name(NULL), length(length), dimension_count(dimension_count)
>  {
> this->fields.array = array;
> /* Inherit the gl type of the base. The GL type is used for
> @@ -416,10 +417,16 @@ glsl_type::get_instance(unsigned base_type, unsigned 
> rows, unsigned columns)
> return error_type;
>  }
>
> -

Unrelated whitespace change.

>  const glsl_type *
>  glsl_type::get_array_instance(const glsl_type *base, unsigned array_size)
>  {
> +   return get_array_instance(base, array_size, 1);
> +}
> +
> +const glsl_type *
> +glsl_type::get_array_instance(const glsl_type *base, unsigned array_size,
> +  unsigned dimension_count)
> +{

Just add dimension_count=1 as a default parameter to the existing
get_array_instance() method.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 4/9] glsl: Add array specifier to ast code

[Matt Turner]

On Wed, Jan 15, 2014 at 10:27 PM, Timothy Arceri  wrote:
> Signed-off-by: Timothy Arceri 
> ---
>  src/glsl/ast.h  |  45 +++
>  src/glsl/ast_array_index.cpp|  13 +++
>  src/glsl/ast_to_hir.cpp | 173 
> +++-
>  src/glsl/ast_type.cpp   |   8 +-
>  src/glsl/glsl_parser_extras.cpp |  30 +++
>  src/glsl/glsl_parser_extras.h   |   2 +
>  6 files changed, 179 insertions(+), 92 deletions(-)
>
> diff --git a/src/glsl/ast.h b/src/glsl/ast.h
> index 76911f0..bbae9cd 100644
> --- a/src/glsl/ast.h
> +++ b/src/glsl/ast.h
> @@ -276,6 +276,21 @@ private:
> bool cons;
>  };
>
> +class ast_array_specifier : public ast_node {
> +public:
> +   ast_array_specifier()
> +  : ast_node()
> +   {
> +  dimension_count = 1;
> +   }
> +
> +   virtual void print(void) const;
> +
> +   unsigned dimension_count;
> +   bool is_unsized_array;
> +   exec_list array_dimensions;
> +};
> +
>  /**
>   * C-style aggregate initialization class
>   *
> @@ -325,14 +340,15 @@ public:
>
>  class ast_declaration : public ast_node {
>  public:
> -   ast_declaration(const char *identifier, bool is_array, ast_expression 
> *array_size,
> -  ast_expression *initializer);
> +   ast_declaration(const char *identifier, bool is_array,
> +   ast_array_specifier *array_specifier,
> +   ast_expression *initializer);
> virtual void print(void) const;
>
> const char *identifier;
>
> bool is_array;
> -   ast_expression *array_size;
> +   ast_array_specifier *array_specifier;
>
> ast_expression *initializer;
>  };
> @@ -531,7 +547,6 @@ public:
>  };
>
>
> -
>  class ast_type_specifier : public ast_node {
>  public:
> /**
> @@ -542,9 +557,9 @@ public:
>  * be modified. Zeros the inherited ast_node's fields.
>  */
> ast_type_specifier(const ast_type_specifier *that, bool is_array,
> -  ast_expression *array_size)
> +  ast_array_specifier *array_specifier)
>: ast_node(), type_name(that->type_name), structure(that->structure),
> -is_array(is_array), array_size(array_size),
> +is_array(is_array), array_specifier(array_specifier),
>  default_precision(that->default_precision)
> {
>/* empty */
> @@ -553,7 +568,7 @@ public:
> /** Construct a type specifier from a type name */
> ast_type_specifier(const char *name)
>: type_name(name), structure(NULL),
> -   is_array(false), array_size(NULL),
> +   is_array(false), array_specifier(NULL),
> default_precision(ast_precision_none)
> {
>/* empty */
> @@ -562,7 +577,7 @@ public:
> /** Construct a type specifier from a structure definition */
> ast_type_specifier(ast_struct_specifier *s)
>: type_name(s->name), structure(s),
> -   is_array(false), array_size(NULL),
> +   is_array(false), array_specifier(NULL),
> default_precision(ast_precision_none)
> {
>/* empty */
> @@ -580,7 +595,7 @@ public:
> ast_struct_specifier *structure;
>
> bool is_array;
> -   ast_expression *array_size;
> +   ast_array_specifier *array_specifier;
>
> /** For precision statements, this is the given precision; otherwise 
> none. */
> unsigned default_precision:2;
> @@ -634,7 +649,7 @@ public:
>type(NULL),
>identifier(NULL),
>is_array(false),
> -  array_size(NULL),
> +  array_specifier(NULL),
>formal_parameter(false),
>is_void(false)
> {
> @@ -649,7 +664,7 @@ public:
> ast_fully_specified_type *type;
> const char *identifier;
> bool is_array;
> -   ast_expression *array_size;
> +   ast_array_specifier *array_specifier;
>
> static void parameters_to_hir(exec_list *ast_parameters,
>  bool formal, exec_list *ir_parameters,
> @@ -897,12 +912,12 @@ public:
> ast_interface_block(ast_type_qualifier layout,
> const char *instance_name,
> bool is_array,
> -   ast_expression *array_size)
> +   ast_array_specifier *array_specifier)
> : layout(layout), block_name(NULL), instance_name(instance_name),
> - is_array(is_array), array_size(array_size)
> + is_array(is_array), array_specifier(array_specifier)
> {
>if (!is_array)
> - assert(array_size == NULL);
> + assert(array_specifier == NULL);
> }
>
> virtual ir_rvalue *hir(exec_list *instructions,
> @@ -937,7 +952,7 @@ public:
>  * If the block is not declared as an array or if the block instance array
>  * is unsized, this field will be \c NULL.
>  */
> -   ast_expression *array_size;
> +   ast_array_specifier *array_specifier;
>  };
>
>
> diff --git a/src/glsl/ast_array_index.cpp b/src/glsl/ast_array_index.cpp
> index a5f2320..f3b060e 100644
> --- a/src/glsl/ast_array_index.cpp
> +++ b/src/glsl/ast_array_index.cpp
> @

Re: [Mesa-dev] [PATCH 6/9] glsl: Implement ARB_arrays_of_arrays support for constructors

[Matt Turner]

On Wed, Jan 15, 2014 at 10:27 PM, Timothy Arceri  wrote:
> Signed-off-by: Timothy Arceri 
> ---
>  src/glsl/ast_function.cpp | 54 
> ---
>  1 file changed, 46 insertions(+), 8 deletions(-)
>
> diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
> index 2d05d07..57aa45f 100644
> --- a/src/glsl/ast_function.cpp
> +++ b/src/glsl/ast_function.cpp
> @@ -734,6 +734,7 @@ process_array_constructor(exec_list *instructions,
>  *Section 4.1.10 "Implicit Conversions.""
>  */
> exec_list actual_parameters;
> +
> const unsigned parameter_count =
>process_parameters(instructions, &actual_parameters, parameters, 
> state);
> bool is_unsized_array = constructor_type->is_unsized_array();
> @@ -752,8 +753,10 @@ process_array_constructor(exec_list *instructions,
>
> if (is_unsized_array) {
>constructor_type =
> -glsl_type::get_array_instance(constructor_type->element_type(),
> -  parameter_count);
> + glsl_type::get_array_instance(constructor_type->element_type(),
> +   parameter_count,
> +   constructor_type->dimension_count);
> +
>assert(constructor_type != NULL);
>assert(constructor_type->length == parameter_count);
> }
> @@ -782,14 +785,49 @@ process_array_constructor(exec_list *instructions,
>  }
>}
>
> -  if (result->type != constructor_type->element_type()) {
> -_mesa_glsl_error(loc, state, "type error in array constructor: "
> - "expected: %s, found %s",
> - constructor_type->element_type()->name,
> - result->type->name);
> +  if (constructor_type->dimension_count == 1 &&
> +  result->type != constructor_type->element_type()) {
> + _mesa_glsl_error(loc, state, "type error in array constructor: "
> +  "expected: %s, found %s",
> +  constructor_type->element_type()->name,
> +  result->type->name);
>   return ir_rvalue::error_value(ctx);
>}
>
> +  /* compare arrays of arrays dimensions, element type, and sizes*/
> +  if (result->type->is_array()) {
> +
> + if (result->type->dimension_count != 
> (constructor_type->dimension_count-1)) {
> + _mesa_glsl_error(loc, state, "type error in array constructor: "
> + "expected array with: %u dimension(s),"
> +  " found %u dimension(s)",
> +  constructor_type->dimension_count-1,
> +  result->type->dimension_count);
> + return ir_rvalue::error_value(ctx);
> + }
> +
> + const glsl_type *expected_type = constructor_type->element_type();
> + const glsl_type *result_type = result->type;
> + for (unsigned i=0; itype->dimension_count; i++) {
> +if (result_type->length != expected_type->length) {
> +   _mesa_glsl_error(loc, state, "type error in array 
> constructor: "
> +"expected array with size: %u,"
> +" found size %u",
> +expected_type->length,
> +result_type->length);
> +   return ir_rvalue::error_value(ctx);
> +}
> +expected_type = expected_type->element_type();
> +result_type = result_type->element_type();
> + }
> +
> + if (expected_type != result_type) {
> +_mesa_glsl_error(loc, state, "type error in array constructor: "
> +"expected: %s",
> + expected_type->name);
> + }
> +  }
> +
>/* Attempt to convert the parameter to a constant valued expression.
> * After doing so, track whether or not all the parameters to the
> * constructor are trivially constant valued expressions.
> @@ -808,7 +846,7 @@ process_array_constructor(exec_list *instructions,
>return new(ctx) ir_constant(constructor_type, &actual_parameters);
>
> ir_variable *var = new(ctx) ir_variable(constructor_type, "array_ctor",
> -  ir_var_temporary);
> +   ir_var_temporary);
> instructions->push_tail(var);
>
> int i = 0;
> --
> 1.8.3.1

There are just generally a bunch of unrelated whitespace changes in
this patch. I see some of them are replacing tabs with spaces, but
this just makes finding the functional differences in the patch
harder. If you're already modifying a line, feel free to fix up its
whitespace, but not on otherwise untouched lines. Actually, there are
some added lines with tabs too..
___
mesa-dev mailing list
mesa-dev@lists

[Mesa-dev] [PATCH] llvmpipe: fix large point rasterization with point_quad_rasterization

[sroland]

From: Roland Scheidegger 

The whole round-pointsize-to-int stuff must only be done with GL legacy
rules (no point_quad_rasterization) or all the wrong edges are lit up.
This was previously in a private branch (d3d pointsprite test complains
loudly otherwise) and got lost in a merge. However, it should certainly
apply to GL point sprite rasterization as well.
---
 src/gallium/drivers/llvmpipe/lp_setup_point.c |   31 +++--
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/src/gallium/drivers/llvmpipe/lp_setup_point.c 
b/src/gallium/drivers/llvmpipe/lp_setup_point.c
index 3610c5c..f065676 100644
--- a/src/gallium/drivers/llvmpipe/lp_setup_point.c
+++ b/src/gallium/drivers/llvmpipe/lp_setup_point.c
@@ -307,12 +307,13 @@ subpixel_snap(float a)
  */
 static void
 print_point(struct lp_setup_context *setup,
-const float (*v0)[4])
+const float (*v0)[4],
+const float size)
 {
const struct lp_setup_variant_key *key = &setup->setup.variant->key;
uint i;
 
-   debug_printf("llvmpipe point\n");
+   debug_printf("llvmpipe point, width %f\n", size);
for (i = 0; i < 1 + key->num_inputs; i++) {
   debug_printf("  v0[%d]:  %f %f %f %f\n", i,
v0[i][0], v0[i][1], v0[i][2], v0[i][3]);
@@ -331,12 +332,6 @@ try_setup_point( struct lp_setup_context *setup,
const float size
   = (setup->point_size_per_vertex && sizeAttr > 0) ? v0[sizeAttr][0]
   : setup->point_size;
-   
-   /* Point size as fixed point integer, remove rounding errors 
-* and gives minimum width for very small points
-*/
-   int fixed_width = MAX2(FIXED_ONE,
-  (subpixel_snap(size) + FIXED_ONE/2 - 1) & 
~(FIXED_ONE-1));
 
/* Yes this is necessary to accurately calculate bounding boxes
 * with the two fill-conventions we support.  GL (normally) ends
@@ -353,6 +348,7 @@ try_setup_point( struct lp_setup_context *setup,
struct point_info info;
unsigned viewport_index = 0;
unsigned layer = 0;
+   int fixed_width;
 
if (setup->viewport_index_slot > 0) {
   unsigned *udata = (unsigned*)v0[setup->viewport_index_slot];
@@ -364,7 +360,7 @@ try_setup_point( struct lp_setup_context *setup,
}
 
if (0)
-  print_point(setup, v0);
+  print_point(setup, v0, size);
 
/* Bounding rectangle (in pixels) */
if (!lp_context->rasterizer ||
@@ -372,9 +368,14 @@ try_setup_point( struct lp_setup_context *setup,
   /*
* Rasterize points as quads.
*/
+  int x0, y0;
+  /* Point size as fixed point integer, remove rounding errors
+   * and gives minimum width for very small points.
+   */
+  fixed_width = MAX2(FIXED_ONE, subpixel_snap(size));
 
-  const int x0 = subpixel_snap(v0[0][0] - setup->pixel_offset) - 
fixed_width/2;
-  const int y0 = subpixel_snap(v0[0][1] - setup->pixel_offset) - 
fixed_width/2;
+  x0 = subpixel_snap(v0[0][0] - setup->pixel_offset) - fixed_width/2;
+  y0 = subpixel_snap(v0[0][1] - setup->pixel_offset) - fixed_width/2;
 
   bbox.x0 = (x0 + (FIXED_ONE-1)) >> FIXED_ORDER;
   bbox.x1 = (x0 + fixed_width + (FIXED_ONE-1)) >> FIXED_ORDER;
@@ -398,7 +399,13 @@ try_setup_point( struct lp_setup_context *setup,
   const int x0 = subpixel_snap(v0[0][0]);
   const int y0 = subpixel_snap(v0[0][1]) - adj;
 
-  int int_width = fixed_width >> FIXED_ORDER;
+  int int_width;
+  /* Point size as fixed point integer. For GL legacy points
+   * the point size is always a whole integer.
+   */
+  fixed_width = MAX2(FIXED_ONE,
+ (subpixel_snap(size) + FIXED_ONE/2 - 1) & 
~(FIXED_ONE-1));
+  int_width = fixed_width >> FIXED_ORDER;
 
   assert(setup->pixel_offset != 0);
 
-- 
1.7.9.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v3 1/1] clover: Don't crash on NULL global buffer objects.

[Jan Vesely]

Specs say "If the argument is a buffer object, the arg_value
pointer can be NULL or point to a NULL value in which case a NULL
value will be used as the value for the argument declared as a
pointer to __global or __constant memory in the kernel."

So don't crash when somebody does that.

v2: Insert NULL into input buffer instead of buffer handle pair
Fix constant_argument too
Drop r600 driver changes

v3: Fix inserting NULL pointer

Signed-off-by: Jan Vesely 
---
 src/gallium/state_trackers/clover/core/kernel.cpp | 34 +++
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/src/gallium/state_trackers/clover/core/kernel.cpp 
b/src/gallium/state_trackers/clover/core/kernel.cpp
index 58780d6..fb826c1 100644
--- a/src/gallium/state_trackers/clover/core/kernel.cpp
+++ b/src/gallium/state_trackers/clover/core/kernel.cpp
@@ -327,7 +327,7 @@ kernel::global_argument::set(size_t size, const void 
*value) {
if (size != sizeof(cl_mem))
   throw error(CL_INVALID_ARG_SIZE);
 
-   buf = &obj(*(cl_mem *)value);
+   buf = pobj(value ? *(cl_mem *)value : NULL);
_set = true;
 }
 
@@ -335,8 +335,14 @@ void
 kernel::global_argument::bind(exec_context &ctx,
   const module::argument &marg) {
align(ctx.input, marg.target_align);
-   ctx.g_handles.push_back(allocate(ctx.input, marg.target_size));
-   ctx.g_buffers.push_back(buf->resource(*ctx.q).pipe);
+
+   if (buf) {
+  ctx.g_handles.push_back(allocate(ctx.input, marg.target_size));
+  ctx.g_buffers.push_back(buf->resource(*ctx.q).pipe);
+   } else {
+  //NULL pointer
+  allocate(ctx.input, marg.target_size);
+   }
 }
 
 void
@@ -379,22 +385,28 @@ kernel::constant_argument::set(size_t size, const void 
*value) {
if (size != sizeof(cl_mem))
   throw error(CL_INVALID_ARG_SIZE);
 
-   buf = &obj(*(cl_mem *)value);
+   buf = pobj(value ? *(cl_mem *)value : NULL);
_set = true;
 }
 
 void
 kernel::constant_argument::bind(exec_context &ctx,
 const module::argument &marg) {
-   auto v = bytes(ctx.resources.size() << 24);
-
-   extend(v, module::argument::zero_ext, marg.target_size);
-   byteswap(v, ctx.q->dev.endianness());
align(ctx.input, marg.target_align);
-   insert(ctx.input, v);
 
-   st = buf->resource(*ctx.q).bind_surface(*ctx.q, false);
-   ctx.resources.push_back(st);
+   if (buf) {
+  auto v = bytes(ctx.resources.size() << 24);
+
+  extend(v, module::argument::zero_ext, marg.target_size);
+  byteswap(v, ctx.q->dev.endianness());
+  insert(ctx.input, v);
+
+  st = buf->resource(*ctx.q).bind_surface(*ctx.q, false);
+  ctx.resources.push_back(st);
+   } else {
+  //NULL pointer
+  allocate(ctx.input, marg.target_size);
+   }
 }
 
 void
-- 
1.8.4.2

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

Re: [Mesa-dev] [PATCH v2 RFC 1/1] clover: Don't crash on NULL global buffer objects.

[Jan Vesely]

On Wed, 2014-01-15 at 18:41 +0100, Francisco Jerez wrote:
> Jan Vesely  writes:
> 
> > Specs say "If the argument is a buffer object, the arg_value
> > pointer can be NULL or point to a NULL value in which case a NULL
> > value will be used as the value for the argument declared as a
> > pointer to __global or __constant memory in the kernel."
> >
> > So don't crash when somebody does that.
> >
> > v2: Insert NULL into input buffer instead of buffer handle pair
> > Fix constant_argument too
> > Drop r600 driver changes
> >
> > Signed-off-by: Jan Vesely 
> > ---
> > Hi Francisco,
> >
> Hi Jan,
> 
> > I could not find much info about how ctx.input is supposed to be used.
> > It looks like it stores begin-end intervals of parameters so NULL, NULL
> > seemed appropriate.
> >
> 
> It's just an array of bytes that is passed to the driver as input for
> the kernel.  Some suggestions more below.

Like an image of stack-passed parameters?
So inserting two NULLs produced two 0 bytes, and the only reason it
worked was because of the alignment for the next argument?

thanks,
Jan

> 
> > Is this closer to what you had in mind? This patch fixes my issue
> > even without the r600 changes.
> >
> > regards,
> > Jan
> >
> >  src/gallium/state_trackers/clover/core/kernel.cpp | 34 
> > +--
> >  1 file changed, 20 insertions(+), 14 deletions(-)
> >
> > diff --git a/src/gallium/state_trackers/clover/core/kernel.cpp 
> > b/src/gallium/state_trackers/clover/core/kernel.cpp
> > index 58780d6..d4942b3 100644
> > --- a/src/gallium/state_trackers/clover/core/kernel.cpp
> > +++ b/src/gallium/state_trackers/clover/core/kernel.cpp
> > @@ -327,16 +327,19 @@ kernel::global_argument::set(size_t size, const void 
> > *value) {
> > if (size != sizeof(cl_mem))
> >throw error(CL_INVALID_ARG_SIZE);
> >  
> > -   buf = &obj(*(cl_mem *)value);
> > +   buf = pobj(value ? *(cl_mem *)value : NULL);
> > _set = true;
> >  }
> >  
> >  void
> >  kernel::global_argument::bind(exec_context &ctx,
> >const module::argument &marg) {
> > -   align(ctx.input, marg.target_align);
> 
> As null pointers have the same alignment restrictions as normal pointers,
> you should keep this line here before the 'if (buf)' conditional.
> 
> > -   ctx.g_handles.push_back(allocate(ctx.input, marg.target_size));
> > -   ctx.g_buffers.push_back(buf->resource(*ctx.q).pipe);
> > +   if (buf) {
> > +  align(ctx.input, marg.target_align);
> > +  ctx.g_handles.push_back(allocate(ctx.input, marg.target_size));
> > +  ctx.g_buffers.push_back(buf->resource(*ctx.q).pipe);
> > +   } else
> 
> For consistency use braces around the else block of an if block that
> uses braces.
> 
> > +  insert(ctx.input, {NULL, NULL});
> 
> I don't think this does what you want.  The easiest way to append zeroes
> to the input buffer is:
> 
> |} else {
> |   // Null global pointer.
> |   allocate(ctx.input, marg.target_size);
> |}
> 
> >  }
> >  
> >  void
> > @@ -379,22 +382,25 @@ kernel::constant_argument::set(size_t size, const 
> > void *value) {
> > if (size != sizeof(cl_mem))
> >throw error(CL_INVALID_ARG_SIZE);
> >  
> > -   buf = &obj(*(cl_mem *)value);
> > +   buf = pobj(value ? *(cl_mem *)value : NULL);
> > _set = true;
> >  }
> >  
> >  void
> >  kernel::constant_argument::bind(exec_context &ctx,
> >  const module::argument &marg) {
> > -   auto v = bytes(ctx.resources.size() << 24);
> > -
> > -   extend(v, module::argument::zero_ext, marg.target_size);
> > -   byteswap(v, ctx.q->dev.endianness());
> > -   align(ctx.input, marg.target_align);
> > -   insert(ctx.input, v);
> > -
> > -   st = buf->resource(*ctx.q).bind_surface(*ctx.q, false);
> > -   ctx.resources.push_back(st);
> > +   if (buf) {
> > +  auto v = bytes(ctx.resources.size() << 24);
> > +
> > +  extend(v, module::argument::zero_ext, marg.target_size);
> > +  byteswap(v, ctx.q->dev.endianness());
> > +  align(ctx.input, marg.target_align);
> 
> This align() call should be kept outside the conditional too.
> 
> > +  insert(ctx.input, v);
> > +
> > +  st = buf->resource(*ctx.q).bind_surface(*ctx.q, false);
> > +  ctx.resources.push_back(st);
> > +   } else
> > +  insert(ctx.input, {NULL, NULL});
> 
> Same comment as before.
> 
> Thank you.
> 
> >  }
> >  
> >  void
> > -- 
> > 1.8.4.2

-- 
Jan Vesely 


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

[Mesa-dev] [Bug 73714] New account for Ilia Mirkin

[bugzilla-daemon]

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

Ben Skeggs  changed:

   What|Removed |Added

   Assignee|mesa-dev@lists.freedesktop. |sitewranglers@lists.freedes
   |org |ktop.org
Product|Mesa|freedesktop.org
  Component|Other   |New Accounts

--- Comment #3 from Ben Skeggs  ---
ACK

-- 
You are receiving this mail because:
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 v2] nv50, nvc0: clear out RT on a null cbuf

[Ilia Mirkin]

This is needed since commit 9baa45f78b (st/mesa: bind NULL colorbuffers
as specified by glDrawBuffers).

This implementation is highly based on a larger commit by
Christoph Bumiller  in his gallium-nine
branch.

Signed-off-by: Ilia Mirkin 
---
 src/gallium/drivers/nouveau/nv50/nv50_defs.xml.h   |  1 +
 src/gallium/drivers/nouveau/nv50/nv50_formats.c|  1 -
 .../drivers/nouveau/nv50/nv50_state_validate.c | 30 +++---
 .../drivers/nouveau/nvc0/nvc0_state_validate.c | 28 +---
 4 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_defs.xml.h 
b/src/gallium/drivers/nouveau/nv50/nv50_defs.xml.h
index 2e42843..80de3be 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_defs.xml.h
+++ b/src/gallium/drivers/nouveau/nv50/nv50_defs.xml.h
@@ -78,6 +78,7 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE 
SOFTWARE.
 #define NV50_VSTATUS_BLOCKED   0x0005
 #define NV50_VSTATUS_FAULTED   0x0006
 #define NV50_VSTATUS_PAUSED0x0007
+#define NV50_SURFACE_FORMAT_NONE   0x
 #define NV50_SURFACE_FORMAT_BITMAP 0x001c
 #define NV50_SURFACE_FORMAT_UNK1D  0x001d
 #define NV50_SURFACE_FORMAT_RGBA32_FLOAT   0x00c0
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_formats.c 
b/src/gallium/drivers/nouveau/nv50/nv50_formats.c
index 0a7e812..d21905d 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_formats.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_formats.c
@@ -71,7 +71,6 @@
 # define U_tV  U_V
 #endif
 
-#define NV50_SURFACE_FORMAT_NONE 0
 #define NV50_ZETA_FORMAT_NONE 0
 
 /* for vertex buffers: */
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c 
b/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
index 86b9a23..bb05f03 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_state_validate.c
@@ -1,6 +1,19 @@
 
 #include "nv50/nv50_context.h"
-#include "os/os_time.h"
+#include "nv50/nv50_defs.xml.h"
+
+static INLINE void
+nv50_fb_set_null_rt(struct nouveau_pushbuf *push, unsigned i)
+{
+   BEGIN_NV04(push, NV50_3D(RT_ADDRESS_HIGH(i)), 4);
+   PUSH_DATA (push, 0);
+   PUSH_DATA (push, 0);
+   PUSH_DATA (push, NV50_SURFACE_FORMAT_NONE);
+   PUSH_DATA (push, 0);
+   BEGIN_NV04(push, NV50_3D(RT_HORIZ(i)), 2);
+   PUSH_DATA (push, 64);
+   PUSH_DATA (push, 0);
+}
 
 static void
 nv50_validate_fb(struct nv50_context *nv50)
@@ -20,9 +33,18 @@ nv50_validate_fb(struct nv50_context *nv50)
PUSH_DATA (push, fb->height << 16);
 
for (i = 0; i < fb->nr_cbufs; ++i) {
-  struct nv50_miptree *mt = nv50_miptree(fb->cbufs[i]->texture);
-  struct nv50_surface *sf = nv50_surface(fb->cbufs[i]);
-  struct nouveau_bo *bo = mt->base.bo;
+  struct nv50_miptree *mt;
+  struct nv50_surface *sf;
+  struct nouveau_bo *bo;
+
+  if (!fb->cbufs[i]) {
+ nv50_fb_set_null_rt(push, i);
+ continue;
+  }
+
+  mt = nv50_miptree(fb->cbufs[i]->texture);
+  sf = nv50_surface(fb->cbufs[i]);
+  bo = mt->base.bo;
 
   array_size = MIN2(array_size, sf->depth);
   if (mt->layout_3d)
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c
index 0ba4bad..dd71c65 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c
@@ -2,6 +2,7 @@
 #include "util/u_math.h"
 
 #include "nvc0/nvc0_context.h"
+#include "nv50/nv50_defs.xml.h"
 
 #if 0
 static void
@@ -54,6 +55,18 @@ nvc0_validate_zcull(struct nvc0_context *nvc0)
 }
 #endif
 
+static INLINE void
+nvc0_fb_set_null_rt(struct nouveau_pushbuf *push, unsigned i)
+{
+   BEGIN_NVC0(push, NVC0_3D(RT_ADDRESS_HIGH(i)), 6);
+   PUSH_DATA (push, 0);
+   PUSH_DATA (push, 0);
+   PUSH_DATA (push, 64);
+   PUSH_DATA (push, 0);
+   PUSH_DATA (push, NV50_SURFACE_FORMAT_NONE);
+   PUSH_DATA (push, 0);
+}
+
 static void
 nvc0_validate_fb(struct nvc0_context *nvc0)
 {
@@ -72,9 +85,18 @@ nvc0_validate_fb(struct nvc0_context *nvc0)
 PUSH_DATA (push, fb->height << 16);
 
 for (i = 0; i < fb->nr_cbufs; ++i) {
-struct nv50_surface *sf = nv50_surface(fb->cbufs[i]);
-struct nv04_resource *res = nv04_resource(sf->base.texture);
-struct nouveau_bo *bo = res->bo;
+struct nv50_surface *sf;
+struct nv04_resource *res;
+struct nouveau_bo *bo;
+
+if (!fb->cbufs[i]) {
+   nvc0_fb_set_null_rt(push, i);
+   continue;
+}
+
+sf = nv50_surface(fb->cbufs[i]);
+res = nv04_resource(sf->base.texture);
+bo = res->bo;
 
 BEGIN_NVC0(push, NVC0_3D(RT_ADDRESS_HIGH(i)), 9);
 PUSH_DATAh(push, res->address + sf->offset);
-- 
1.8.3.2

__

[Mesa-dev] [PATCH] nv50, nvc0: only clear out the buffers that we were asked to clear

[Ilia Mirkin]

Fixes fbo-drawbuffers-none glClearBuffer piglit test.

Signed-off-by: Ilia Mirkin 
---

Only tested on nv50, but implementations seem similar enough.

 src/gallium/drivers/nouveau/nv50/nv50_surface.c | 17 +
 src/gallium/drivers/nouveau/nvc0/nvc0_surface.c | 17 +
 2 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_surface.c 
b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
index 358f57a..eb27429 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_surface.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
@@ -408,9 +408,6 @@ nv50_clear(struct pipe_context *pipe, unsigned buffers,
   PUSH_DATAf(push, color->f[1]);
   PUSH_DATAf(push, color->f[2]);
   PUSH_DATAf(push, color->f[3]);
-  mode =
- NV50_3D_CLEAR_BUFFERS_R | NV50_3D_CLEAR_BUFFERS_G |
- NV50_3D_CLEAR_BUFFERS_B | NV50_3D_CLEAR_BUFFERS_A;
}
 
if (buffers & PIPE_CLEAR_DEPTH) {
@@ -425,12 +422,16 @@ nv50_clear(struct pipe_context *pipe, unsigned buffers,
   mode |= NV50_3D_CLEAR_BUFFERS_S;
}
 
-   BEGIN_NV04(push, NV50_3D(CLEAR_BUFFERS), 1);
-   PUSH_DATA (push, mode);
-
-   for (i = 1; i < fb->nr_cbufs; i++) {
+   if (mode) {
   BEGIN_NV04(push, NV50_3D(CLEAR_BUFFERS), 1);
-  PUSH_DATA (push, (i << 6) | 0x3c);
+  PUSH_DATA (push, mode);
+   }
+
+   for (i = 0; i < fb->nr_cbufs; i++) {
+  if (buffers & (PIPE_CLEAR_COLOR0 << i)) {
+ BEGIN_NV04(push, NV50_3D(CLEAR_BUFFERS), 1);
+ PUSH_DATA (push, (i << 6) | 0x3c);
+  }
}
 }
 
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
index 5375bd4..0c12bce 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
@@ -427,9 +427,6 @@ nvc0_clear(struct pipe_context *pipe, unsigned buffers,
   PUSH_DATAf(push, color->f[1]);
   PUSH_DATAf(push, color->f[2]);
   PUSH_DATAf(push, color->f[3]);
-  mode =
- NVC0_3D_CLEAR_BUFFERS_R | NVC0_3D_CLEAR_BUFFERS_G |
- NVC0_3D_CLEAR_BUFFERS_B | NVC0_3D_CLEAR_BUFFERS_A;
}
 
if (buffers & PIPE_CLEAR_DEPTH) {
@@ -444,12 +441,16 @@ nvc0_clear(struct pipe_context *pipe, unsigned buffers,
   mode |= NVC0_3D_CLEAR_BUFFERS_S;
}
 
-   BEGIN_NVC0(push, NVC0_3D(CLEAR_BUFFERS), 1);
-   PUSH_DATA (push, mode);
-
-   for (i = 1; i < fb->nr_cbufs; i++) {
+   if (mode) {
   BEGIN_NVC0(push, NVC0_3D(CLEAR_BUFFERS), 1);
-  PUSH_DATA (push, (i << 6) | 0x3c);
+  PUSH_DATA (push, mode);
+   }
+
+   for (i = 0; i < fb->nr_cbufs; i++) {
+  if (buffers & (PIPE_CLEAR_COLOR0 << i)) {
+ BEGIN_NVC0(push, NVC0_3D(CLEAR_BUFFERS), 1);
+ PUSH_DATA (push, (i << 6) | 0x3c);
+  }
}
 }
 
-- 
1.8.3.2

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

[Mesa-dev] [PATCH 2/3] svga: rename color output variables

[Brian Paul]

Just to be bit more readable.
---
 src/gallium/drivers/svga/svga_tgsi_decl_sm30.c | 6 +++---
 src/gallium/drivers/svga/svga_tgsi_emit.h  | 5 +++--
 src/gallium/drivers/svga/svga_tgsi_insn.c  | 8 
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/src/gallium/drivers/svga/svga_tgsi_decl_sm30.c 
b/src/gallium/drivers/svga/svga_tgsi_decl_sm30.c
index 7faa275..e0a30a5 100644
--- a/src/gallium/drivers/svga/svga_tgsi_decl_sm30.c
+++ b/src/gallium/drivers/svga/svga_tgsi_decl_sm30.c
@@ -332,9 +332,9 @@ ps30_output(struct svga_shader_emitter *emit,
 
  emit->output_map[idx] = dst_register( SVGA3DREG_TEMP,
emit->nr_hw_temp++ );
- emit->temp_col[idx] = emit->output_map[idx];
- emit->true_col[idx] = dst_register( SVGA3DREG_COLOROUT, 
-  semantic.Index );
+ emit->temp_color_output[idx] = emit->output_map[idx];
+ emit->true_color_output[idx] = dst_register(SVGA3DREG_COLOROUT, 
+ semantic.Index);
   }
   else {
  emit->output_map[idx] = dst_register( SVGA3DREG_COLOROUT, 
diff --git a/src/gallium/drivers/svga/svga_tgsi_emit.h 
b/src/gallium/drivers/svga/svga_tgsi_emit.h
index e36a955..28b8e69 100644
--- a/src/gallium/drivers/svga/svga_tgsi_emit.h
+++ b/src/gallium/drivers/svga/svga_tgsi_emit.h
@@ -119,8 +119,9 @@ struct svga_shader_emitter
/* shared output for depth and fog */
SVGA3dShaderDestToken vs_depth_fog;
 
-   SVGA3dShaderDestToken temp_col[PIPE_MAX_COLOR_BUFS];
-   SVGA3dShaderDestToken true_col[PIPE_MAX_COLOR_BUFS];
+   /* PS output colors */
+   SVGA3dShaderDestToken temp_color_output[PIPE_MAX_COLOR_BUFS];
+   SVGA3dShaderDestToken true_color_output[PIPE_MAX_COLOR_BUFS];
 
SVGA3dShaderDestToken temp_psiz;
SVGA3dShaderDestToken true_psiz;
diff --git a/src/gallium/drivers/svga/svga_tgsi_insn.c 
b/src/gallium/drivers/svga/svga_tgsi_insn.c
index 0865095..2143546 100644
--- a/src/gallium/drivers/svga/svga_tgsi_insn.c
+++ b/src/gallium/drivers/svga/svga_tgsi_insn.c
@@ -3059,7 +3059,7 @@ emit_ps_postamble(struct svga_shader_emitter *emit)
}
 
for (i = 0; i < PIPE_MAX_COLOR_BUFS; i++) {
-  if (SVGA3dShaderGetRegType(emit->true_col[i].value) != 0) {
+  if (SVGA3dShaderGetRegType(emit->true_color_output[i].value) != 0) {
  /* Potentially override output colors with white for XOR
   * logicop workaround.
   */
@@ -3070,15 +3070,15 @@ emit_ps_postamble(struct svga_shader_emitter *emit)
 
 if (!submit_op1( emit,
  inst_token(SVGA3DOP_MOV),
- emit->true_col[i],
+ emit->true_color_output[i],
  one ))
return FALSE;
  }
  else {
 if (!submit_op1( emit,
  inst_token(SVGA3DOP_MOV),
- emit->true_col[i],
- src(emit->temp_col[i]) ))
+ emit->true_color_output[i],
+ src(emit->temp_color_output[i]) ))
return FALSE;
  }
   }
-- 
1.8.3.2

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

[Mesa-dev] [PATCH 3/3] svga: implement TGSI_PROPERTY_FS_COLOR0_WRITES_ALL_CBUFS

[Brian Paul]

Fixes several colorbuffer tests, including piglit "fbo-drawbuffers-none"
for "gl_FragColor" and "glDrawPixels" cases.
---
 src/gallium/drivers/svga/svga_state_fs.c   |  7 +
 src/gallium/drivers/svga/svga_tgsi.h   |  2 ++
 src/gallium/drivers/svga/svga_tgsi_decl_sm30.c | 37 --
 src/gallium/drivers/svga/svga_tgsi_insn.c  |  9 +++
 4 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/src/gallium/drivers/svga/svga_state_fs.c 
b/src/gallium/drivers/svga/svga_state_fs.c
index 51d0eb5..6497d74 100644
--- a/src/gallium/drivers/svga/svga_state_fs.c
+++ b/src/gallium/drivers/svga/svga_state_fs.c
@@ -276,6 +276,11 @@ make_fs_key(const struct svga_context *svga,
key->sprite_origin_lower_left = (svga->curr.rast->templ.sprite_coord_mode
 == PIPE_SPRITE_COORD_LOWER_LEFT);
 
+   key->write_color_to_all = fs->base.info.color0_writes_all_cbufs;
+
+   /* SVGA_NEW_FRAME_BUFFER */
+   key->num_cbufs = svga->curr.framebuffer.nr_cbufs;
+
return PIPE_OK;
 }
 
@@ -296,6 +301,7 @@ emit_hw_fs(struct svga_context *svga, unsigned dirty)
 * SVGA_NEW_RAST
 * SVGA_NEW_NEED_SWTNL
 * SVGA_NEW_SAMPLER
+* SVGA_NEW_FRAME_BUFFER
 */
ret = make_fs_key( svga, fs, &key );
if (ret != PIPE_OK)
@@ -335,6 +341,7 @@ struct svga_tracked_state svga_hw_fs =
 SVGA_NEW_NEED_SWTNL |
 SVGA_NEW_RAST |
 SVGA_NEW_SAMPLER |
+SVGA_NEW_FRAME_BUFFER |
 SVGA_NEW_BLEND),
emit_hw_fs
 };
diff --git a/src/gallium/drivers/svga/svga_tgsi.h 
b/src/gallium/drivers/svga/svga_tgsi.h
index 0e06dbf..0f6a0e7 100644
--- a/src/gallium/drivers/svga/svga_tgsi.h
+++ b/src/gallium/drivers/svga/svga_tgsi.h
@@ -56,6 +56,8 @@ struct svga_fs_compile_key
unsigned light_twoside:1;
unsigned front_ccw:1;
unsigned white_fragments:1;
+   unsigned write_color_to_all:1;
+   unsigned num_cbufs:3;
unsigned num_textures:8;
unsigned num_unnormalized_coords:8;
unsigned sprite_origin_lower_left:1;
diff --git a/src/gallium/drivers/svga/svga_tgsi_decl_sm30.c 
b/src/gallium/drivers/svga/svga_tgsi_decl_sm30.c
index e0a30a5..dba9994 100644
--- a/src/gallium/drivers/svga/svga_tgsi_decl_sm30.c
+++ b/src/gallium/drivers/svga/svga_tgsi_decl_sm30.c
@@ -327,14 +327,35 @@ ps30_output(struct svga_shader_emitter *emit,
 {
switch (semantic.Name) {
case TGSI_SEMANTIC_COLOR:
-  if (emit->unit == PIPE_SHADER_FRAGMENT &&
-  emit->key.fkey.white_fragments) {
-
- emit->output_map[idx] = dst_register( SVGA3DREG_TEMP,
-   emit->nr_hw_temp++ );
- emit->temp_color_output[idx] = emit->output_map[idx];
- emit->true_color_output[idx] = dst_register(SVGA3DREG_COLOROUT, 
- semantic.Index);
+  if (emit->unit == PIPE_SHADER_FRAGMENT) {
+ if (emit->key.fkey.white_fragments) {
+/* Used for XOR logicop mode */
+emit->output_map[idx] = dst_register( SVGA3DREG_TEMP,
+  emit->nr_hw_temp++ );
+emit->temp_color_output[idx] = emit->output_map[idx];
+emit->true_color_output[idx] = dst_register(SVGA3DREG_COLOROUT, 
+semantic.Index);
+ }
+ else if (emit->key.fkey.write_color_to_all) {
+/* We'll write color output [0] to all render targets.
+ * Prepare all the output registers here, but only when the
+ * semantic.Index == 0 so we don't do this more than once.
+ */
+if (semantic.Index == 0) {
+   unsigned i;
+   for (i = 0; i < emit->key.fkey.num_cbufs; i++) {
+  emit->output_map[i] = dst_register(SVGA3DREG_TEMP,
+ emit->nr_hw_temp++);
+  emit->temp_color_output[i] = emit->output_map[i];
+  emit->true_color_output[i] = dst_register(SVGA3DREG_COLOROUT,
+i);
+   }
+}
+ }
+ else {
+emit->output_map[idx] =
+   dst_register(SVGA3DREG_COLOROUT, semantic.Index);
+ }
   }
   else {
  emit->output_map[idx] = dst_register( SVGA3DREG_COLOROUT, 
diff --git a/src/gallium/drivers/svga/svga_tgsi_insn.c 
b/src/gallium/drivers/svga/svga_tgsi_insn.c
index 2143546..a5769b7 100644
--- a/src/gallium/drivers/svga/svga_tgsi_insn.c
+++ b/src/gallium/drivers/svga/svga_tgsi_insn.c
@@ -3074,6 +3074,15 @@ emit_ps_postamble(struct svga_shader_emitter *emit)
  one ))
return FALSE;
  }
+ else if (emit->unit == PIPE_SHADER_FRAGMENT &&
+  emit->key.fkey.write_color_to_all) {
+/* Write temp color output [0] to true output [i] */
+if (!submit_op1(emit, inst_token(SVGA3

[Mesa-dev] [PATCH 1/3] svga: fix clearing for null color buffers

[Brian Paul]

Fixes piglit "fbo-drawbuffers-none glClear" test.
---
 src/gallium/drivers/svga/svga_pipe_clear.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/gallium/drivers/svga/svga_pipe_clear.c 
b/src/gallium/drivers/svga/svga_pipe_clear.c
index 5deebb2..21869e9 100644
--- a/src/gallium/drivers/svga/svga_pipe_clear.c
+++ b/src/gallium/drivers/svga/svga_pipe_clear.c
@@ -59,12 +59,12 @@ try_clear(struct svga_context *svga,
   }
}
 
-   if ((buffers & PIPE_CLEAR_COLOR) && fb->cbufs[0]) {
+   if (buffers & PIPE_CLEAR_COLOR) {
   flags |= SVGA3D_CLEAR_COLOR;
   util_pack_color(color->f, PIPE_FORMAT_B8G8R8A8_UNORM, &uc);
 
-  rect.w = fb->cbufs[0]->width;
-  rect.h = fb->cbufs[0]->height;
+  rect.w = fb->width;
+  rect.h = fb->height;
}
 
if ((buffers & PIPE_CLEAR_DEPTHSTENCIL) && fb->zsbuf) {
-- 
1.8.3.2

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

[Mesa-dev] [PATCH] st/vdpau: check that the format/bindings are valid

[Ilia Mirkin]

It's a bit unreasonable to rely on applications doing the queries and
then obeying their results.

Signed-off-by: Ilia Mirkin 
---

We're starting to see people complaining about flash with newer versions of
nouveau on nv30 cards. These cards don't actually expose any hw caps via
vdpau, but the API is still free to use the surfaces/etc. We were seeing some
errors in the logs that appear to only be able to happen if someone has
managed to create an illegal-type surface. (And nv30 is pretty limited in its
supported renderable format list.)

This adds checks to make sure that the surfaces are valid for the screen in
question. Hopefully I haven't misunderstood gallium or the vdpau state
tracker... My quick test with mplayer (but without hw accel) seems to work.

 src/gallium/state_trackers/vdpau/bitmap.c|  5 +++--
 src/gallium/state_trackers/vdpau/output.c| 16 ++--
 src/gallium/state_trackers/vdpau/vdpau_private.h |  8 
 3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/src/gallium/state_trackers/vdpau/bitmap.c 
b/src/gallium/state_trackers/vdpau/bitmap.c
index 469f3e8..24dbc42 100644
--- a/src/gallium/state_trackers/vdpau/bitmap.c
+++ b/src/gallium/state_trackers/vdpau/bitmap.c
@@ -43,7 +43,7 @@ vlVdpBitmapSurfaceCreate(VdpDevice device,
  VdpBitmapSurface *surface)
 {
struct pipe_context *pipe;
-   struct pipe_resource res_tmpl, *res;
+   struct pipe_resource res_tmpl, *res = NULL;
struct pipe_sampler_view sv_templ;
 
vlVdpBitmapSurface *vlsurface = NULL;
@@ -79,7 +79,8 @@ vlVdpBitmapSurfaceCreate(VdpDevice device,
res_tmpl.usage = frequently_accessed ? PIPE_USAGE_DYNAMIC : 
PIPE_USAGE_STATIC;
 
pipe_mutex_lock(dev->mutex);
-   res = pipe->screen->resource_create(pipe->screen, &res_tmpl);
+   if (CheckSurfaceParams(pipe->screen, &res_tmpl))
+  res = pipe->screen->resource_create(pipe->screen, &res_tmpl);
if (!res) {
   pipe_mutex_unlock(dev->mutex);
   FREE(dev);
diff --git a/src/gallium/state_trackers/vdpau/output.c 
b/src/gallium/state_trackers/vdpau/output.c
index e4e1433..76c4611 100644
--- a/src/gallium/state_trackers/vdpau/output.c
+++ b/src/gallium/state_trackers/vdpau/output.c
@@ -48,7 +48,7 @@ vlVdpOutputSurfaceCreate(VdpDevice device,
  VdpOutputSurface  *surface)
 {
struct pipe_context *pipe;
-   struct pipe_resource res_tmpl, *res;
+   struct pipe_resource res_tmpl, *res = NULL;
struct pipe_sampler_view sv_templ;
struct pipe_surface surf_templ;
 
@@ -83,7 +83,9 @@ vlVdpOutputSurfaceCreate(VdpDevice device,
res_tmpl.usage = PIPE_USAGE_STATIC;
 
pipe_mutex_lock(dev->mutex);
-   res = pipe->screen->resource_create(pipe->screen, &res_tmpl);
+
+   if (CheckSurfaceParams(pipe->screen, &res_tmpl))
+  res = pipe->screen->resource_create(pipe->screen, &res_tmpl);
if (!res) {
   pipe_mutex_unlock(dev->mutex);
   FREE(dev);
@@ -117,7 +119,7 @@ vlVdpOutputSurfaceCreate(VdpDevice device,
   FREE(dev);
   return VDP_STATUS_ERROR;
}
-   
+
pipe_resource_reference(&res, NULL);
 
vl_compositor_init_state(&vlsurface->cstate, pipe);
@@ -278,7 +280,7 @@ vlVdpOutputSurfacePutBitsIndexed(VdpOutputSurface surface,
enum pipe_format index_format;
enum pipe_format colortbl_format;
 
-   struct pipe_resource *res, res_tmpl;
+   struct pipe_resource *res = NULL, res_tmpl;
struct pipe_sampler_view sv_tmpl;
struct pipe_sampler_view *sv_idx = NULL, *sv_tbl = NULL;
 
@@ -326,7 +328,8 @@ vlVdpOutputSurfacePutBitsIndexed(VdpOutputSurface surface,
pipe_mutex_lock(vlsurface->device->mutex);
vlVdpResolveDelayedRendering(vlsurface->device, NULL, NULL);
 
-   res = context->screen->resource_create(context->screen, &res_tmpl);
+   if (CheckSurfaceParams(context->screen, &res_tmpl))
+  res = context->screen->resource_create(context->screen, &res_tmpl);
if (!res)
   goto error_resource;
 
@@ -359,7 +362,8 @@ vlVdpOutputSurfacePutBitsIndexed(VdpOutputSurface surface,
res_tmpl.usage = PIPE_USAGE_STAGING;
res_tmpl.bind = PIPE_BIND_SAMPLER_VIEW;
 
-   res = context->screen->resource_create(context->screen, &res_tmpl);
+   if (CheckSurfaceParams(context->screen, &res_tmpl))
+  res = context->screen->resource_create(context->screen, &res_tmpl);
if (!res)
   goto error_resource;
 
diff --git a/src/gallium/state_trackers/vdpau/vdpau_private.h 
b/src/gallium/state_trackers/vdpau/vdpau_private.h
index 60196ac..6c2c019 100644
--- a/src/gallium/state_trackers/vdpau/vdpau_private.h
+++ b/src/gallium/state_trackers/vdpau/vdpau_private.h
@@ -332,6 +332,14 @@ RectToPipeBox(const VdpRect *rect, struct pipe_resource 
*res)
return box;
 }
 
+static inline bool
+CheckSurfaceParams(struct pipe_screen *screen,
+   const struct pipe_resource *templ)
+{
+   return screen->is_format_supported(
+ screen, templ->format, templ->target, templ->nr_samples, templ->bind);
+}
+
 typedef struct
 {

[Mesa-dev] [V3 PATCH 7/8] mesa: Remove/update related comments and rename MESA_FORMAT_XBGR A Type MESA_FORMATs to match naming spec as follows: s/\bMESA_FORMAT_XBGR8888_SNORM\b/MESA_FORMAT_RGBX_SNORM8

[Mark Mueller]

Signed-off-by: Mark Mueller 
---
 src/mesa/drivers/dri/i965/brw_surface_formats.c |  24 +++---
 src/mesa/main/format_pack.c |  60 +++---
 src/mesa/main/format_unpack.c   |  36 -
 src/mesa/main/formats.c | 100 
 src/mesa/main/formats.h |  24 +++---
 src/mesa/main/texformat.c   |  20 ++---
 src/mesa/main/texstore.c|  70 -
 src/mesa/state_tracker/st_format.c  |  48 ++--
 src/mesa/swrast/s_texfetch.c|  24 +++---
 9 files changed, 203 insertions(+), 203 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_surface_formats.c 
b/src/mesa/drivers/dri/i965/brw_surface_formats.c
index 2620131..f7afce2 100644
--- a/src/mesa/drivers/dri/i965/brw_surface_formats.c
+++ b/src/mesa/drivers/dri/i965/brw_surface_formats.c
@@ -507,19 +507,19 @@ brw_format_for_mesa_format(mesa_format mesa_format)
 
   [MESA_FORMAT_B4G4R4X4_UNORM] = 0,
   [MESA_FORMAT_B5G5R5X1_UNORM] = BRW_SURFACEFORMAT_B5G5R5X1_UNORM,
-  [MESA_FORMAT_XBGR_SNORM] = 0,
-  [MESA_FORMAT_XBGR_SRGB] = 0,
-  [MESA_FORMAT_XBGR_UINT] = 0,
-  [MESA_FORMAT_XBGR_SINT] = 0,
+  [MESA_FORMAT_RGBX_SNORM8] = 0,
+  [MESA_FORMAT_SRGBX_UNORM8] = 0,
+  [MESA_FORMAT_RGBX_UINT8] = 0,
+  [MESA_FORMAT_RGBX_SINT8] = 0,
   [MESA_FORMAT_B10G10R10X2_UNORM] = BRW_SURFACEFORMAT_B10G10R10X2_UNORM,
-  [MESA_FORMAT_XBGR16161616_UNORM] = BRW_SURFACEFORMAT_R16G16B16X16_UNORM,
-  [MESA_FORMAT_XBGR16161616_SNORM] = 0,
-  [MESA_FORMAT_XBGR16161616_FLOAT] = BRW_SURFACEFORMAT_R16G16B16X16_FLOAT,
-  [MESA_FORMAT_XBGR16161616_UINT] = 0,
-  [MESA_FORMAT_XBGR16161616_SINT] = 0,
-  [MESA_FORMAT_XBGR32323232_FLOAT] = BRW_SURFACEFORMAT_R32G32B32X32_FLOAT,
-  [MESA_FORMAT_XBGR32323232_UINT] = 0,
-  [MESA_FORMAT_XBGR32323232_SINT] = 0,
+  [MESA_FORMAT_RGBX_UNORM16] = BRW_SURFACEFORMAT_R16G16B16X16_UNORM,
+  [MESA_FORMAT_RGBX_SNORM16] = 0,
+  [MESA_FORMAT_RGBX_FLOAT16] = BRW_SURFACEFORMAT_R16G16B16X16_FLOAT,
+  [MESA_FORMAT_RGBX_UINT16] = 0,
+  [MESA_FORMAT_RGBX_SINT16] = 0,
+  [MESA_FORMAT_RGBX_FLOAT32] = BRW_SURFACEFORMAT_R32G32B32X32_FLOAT,
+  [MESA_FORMAT_RGBX_UINT32] = 0,
+  [MESA_FORMAT_RGBX_SINT32] = 0,
};
assert(mesa_format < MESA_FORMAT_COUNT);
return table[mesa_format];
diff --git a/src/mesa/main/format_pack.c b/src/mesa/main/format_pack.c
index d80e25c..e604cd3 100644
--- a/src/mesa/main/format_pack.c
+++ b/src/mesa/main/format_pack.c
@@ -1711,7 +1711,7 @@ pack_float_XRGB1555_UNORM(const GLfloat src[4], void *dst)
 
 
 /*
- * MESA_FORMAT_XBGR_SNORM
+ * MESA_FORMAT_RGBX_SNORM8
  */
 
 static void
@@ -1726,7 +1726,7 @@ pack_float_XBGR_SNORM(const GLfloat src[4], void *dst)
 
 
 /*
- * MESA_FORMAT_XBGR_SRGB
+ * MESA_FORMAT_SRGBX_UNORM8
  */
 
 static void
@@ -1764,7 +1764,7 @@ pack_float_XRGB2101010_UNORM(const GLfloat src[4], void 
*dst)
 }
 
 
-/* MESA_FORMAT_XBGR16161616_UNORM */
+/* MESA_FORMAT_RGBX_UNORM16 */
 
 static void
 pack_ubyte_XBGR16161616_UNORM(const GLubyte src[4], void *dst)
@@ -1787,7 +1787,7 @@ pack_float_XBGR16161616_UNORM(const GLfloat src[4], void 
*dst)
 }
 
 
-/* MESA_FORMAT_XBGR16161616_SNORM */
+/* MESA_FORMAT_RGBX_SNORM16 */
 
 static void
 pack_float_XBGR16161616_SNORM(const GLfloat src[4], void *dst)
@@ -1800,7 +1800,7 @@ pack_float_XBGR16161616_SNORM(const GLfloat src[4], void 
*dst)
 }
 
 
-/* MESA_FORMAT_XBGR16161616_FLOAT */
+/* MESA_FORMAT_RGBX_FLOAT16 */
 
 static void
 pack_float_XBGR16161616_FLOAT(const GLfloat src[4], void *dst)
@@ -1812,7 +1812,7 @@ pack_float_XBGR16161616_FLOAT(const GLfloat src[4], void 
*dst)
d[3] = _mesa_float_to_half(1.0);
 }
 
-/* MESA_FORMAT_XBGR32323232_FLOAT */
+/* MESA_FORMAT_RGBX_FLOAT32 */
 
 static void
 pack_float_XBGR32323232_FLOAT(const GLfloat src[4], void *dst)
@@ -2014,19 +2014,19 @@ _mesa_get_pack_ubyte_rgba_function(mesa_format format)
 
   table[MESA_FORMAT_B4G4R4X4_UNORM] = pack_ubyte_XRGB_UNORM;
   table[MESA_FORMAT_B5G5R5X1_UNORM] = pack_ubyte_XRGB1555_UNORM;
-  table[MESA_FORMAT_XBGR_SNORM] = NULL;
-  table[MESA_FORMAT_XBGR_SRGB] = NULL;
-  table[MESA_FORMAT_XBGR_UINT] = NULL;
-  table[MESA_FORMAT_XBGR_SINT] = NULL;
+  table[MESA_FORMAT_RGBX_SNORM8] = NULL;
+  table[MESA_FORMAT_SRGBX_UNORM8] = NULL;
+  table[MESA_FORMAT_RGBX_UINT8] = NULL;
+  table[MESA_FORMAT_RGBX_SINT8] = NULL;
   table[MESA_FORMAT_B10G10R10X2_UNORM] = pack_ubyte_XRGB2101010_UNORM;
-  table[MESA_FORMAT_XBGR16161616_UNORM] = pack_ubyte_XBGR16161616_UNORM;
-  table[MESA_FORMAT_XBGR16161616_SNORM] = NULL;
-  table[MESA_FORMAT_XBGR16161616_FLOAT] = NULL;
-  table[MESA_FORMAT_XBGR16161616_UINT] = NULL;
-  table[MESA_FORMAT_XBGR16161616_SINT] = NULL;
-  table[MESA_FORMAT_XBGR32323232_FLOAT] = NULL;
-

[Mesa-dev] [V3 PATCH 0/8] mesa: Naming MESA_FORMATs to a specification

[Mark Mueller]

This series introduces a specification for 3 types of MESA_FORMAT names -
Type A (array formats), Type C (compressed formats), and Type P (packed
formats), and then performs a series of substitutions grouped by type. Builds
of all default gallium and DRI drivers were verified and no regressions
were observed w/piglit tests on the i965 driver.

The format_unpack functions were used to verify component orders, except
with some _REV formats, which appeared to be incorrect, but not used in
normal testing.

Mark Mueller (8):
 1 's/\bgl_format\b/mesa_format/g'. Use better name for Mesa Formats enum
 
 2 Change all 4 color component unsigned byte formats to meet spec:
s/MESA_FORMAT_RGBA\b/MESA_FORMAT_ABGR_UNORM8/g
s/MESA_FORMAT_RGBA_REV\b/MESA_FORMAT_RGBA_UNORM8/g
s/MESA_FORMAT_ARGB\b/MESA_FORMAT_BGRA_UNORM8/g
s/MESA_FORMAT_ARGB_REV\b/MESA_FORMAT_ARGB_UNORM8/g
s/MESA_FORMAT_RGBX\b/MESA_FORMAT_XBGR_UNORM8/g
s/MESA_FORMAT_RGBX_REV\b/MESA_FORMAT_RGBX_UNORM8/g
s/MESA_FORMAT_XRGB\b/MESA_FORMAT_BGRX_UNORM8/g
s/MESA_FORMAT_XRGB_REV\b/MESA_FORMAT_XRGB_UNORM8/g

 3 Update comments. Conversion of the following Type A formats:
s/MESA_FORMAT_RGB888\b/MESA_FORMAT_BGR_UNORM8/g
s/MESA_FORMAT_BGR888\b/MESA_FORMAT_RGB_UNORM8/g
s/MESA_FORMAT_AL88\b/MESA_FORMAT_LA_UNORM8/g
s/MESA_FORMAT_AL88_REV\b/MESA_FORMAT_AL_UNORM8/g
s/MESA_FORMAT_AL1616\b/MESA_FORMAT_LA_UNORM16/g
s/MESA_FORMAT_AL1616_REV\b/MESA_FORMAT_AL_UNORM16/g
s/MESA_FORMAT_A8\b/MESA_FORMAT_A_UNORM8/g
s/MESA_FORMAT_A16\b/MESA_FORMAT_A_UNORM16/g
s/MESA_FORMAT_L8\b/MESA_FORMAT_L_UNORM8/g
s/MESA_FORMAT_L16\b/MESA_FORMAT_L_UNORM16/g
s/MESA_FORMAT_I8\b/MESA_FORMAT_I_UNORM8/g
s/MESA_FORMAT_I16\b/MESA_FORMAT_I_UNORM16/g
s/MESA_FORMAT_R8\b/MESA_FORMAT_R_UNORM8/g
s/MESA_FORMAT_GR88\b/MESA_FORMAT_RG_UNORM8/g
s/MESA_FORMAT_RG88\b/MESA_FORMAT_GR_UNORM8/g
s/MESA_FORMAT_R16\b/MESA_FORMAT_R_UNORM16/g
s/MESA_FORMAT_GR1616\b/MESA_FORMAT_RG_UNORM16/g
s/MESA_FORMAT_RG1616\b/MESA_FORMAT_GR_UNORM16/g
s/MESA_FORMAT_Z16\b/MESA_FORMAT_Z_UNORM16/g
s/MESA_FORMAT_Z32\b/MESA_FORMAT_Z_UNORM32/g
s/MESA_FORMAT_S8\b/MESA_FORMAT_S_UINT8/g
s/MESA_FORMAT_SRGBA8\b/MESA_FORMAT_SABGR_UNORM8/g
s/MESA_FORMAT_SRGB8\b/MESA_FORMAT_SBGR_UNORM8/g
s/MESA_FORMAT_SARGB8\b/MESA_FORMAT_SBGRA_UNORM8/g
s/MESA_FORMAT_SL8\b/MESA_FORMAT_SL_UNORM8/g
s/MESA_FORMAT_SLA8\b/MESA_FORMAT_SLA_UNORM8/g
s/MESA_FORMAT_Z32_FLOAT\b/MESA_FORMAT_Z_FLOAT32/g

 4 Conversion of Type P formats as follows (w/related comment fixes):
s/\bMESA_FORMAT_RGB565\b/MESA_FORMAT_B5G6R5_UNORM/g
s/\bMESA_FORMAT_RGB565_REV\b/MESA_FORMAT_R5G6B5_UNORM/g
s/\bMESA_FORMAT_ARGB\b/MESA_FORMAT_B4G4R4A4_UNORM/g
s/\bMESA_FORMAT_ARGB_REV\b/MESA_FORMAT_A4R4G4B4_UNORM/g
s/\bMESA_FORMAT_RGBA5551\b/MESA_FORMAT_A1B5G5R5_UNORM/g
s/\bMESA_FORMAT_ARGB1555\b/MESA_FORMAT_B5G5R5A1_UNORM/g
s/\bMESA_FORMAT_ARGB1555_REV\b/MESA_FORMAT_A1R5G5B5_UNORM/g
s/\bMESA_FORMAT_AL44\b/MESA_FORMAT_L4A4_UNORM/g
s/\bMESA_FORMAT_RGB332\b/MESA_FORMAT_B2G3R3_UNORM/g
s/\bMESA_FORMAT_ARGB2101010\b/MESA_FORMAT_B10G10R10A2_UNORM/g
s/\bMESA_FORMAT_Z24_S8\b/MESA_FORMAT_S8_UINT_Z24_UNORM/g
s/\bMESA_FORMAT_S8_Z24\b/MESA_FORMAT_Z24_UNORM_S8_UINT/g
s/\bMESA_FORMAT_X8_Z24\b/MESA_FORMAT_Z24_UNORM_X8_UINT/g
s/\bMESA_FORMAT_Z24_X8\b/MESA_FORMAT_X8Z24_UNORM/g
s/\bMESA_FORMAT_RGB9_E5_FLOAT\b/MESA_FORMAT_R9G9B9E5_FLOAT/g
s/\bMESA_FORMAT_R11_G11_B10_FLOAT\b/MESA_FORMAT_R11G11B10_FLOAT/g   

s/\bMESA_FORMAT_Z32_FLOAT_X24S8\b/MESA_FORMAT_Z32_FLOAT_S8X24_UINT/g
   
s/\bMESA_FORMAT_ABGR2101010_UINT\b/MESA_FORMAT_R10G10B10A2_UINT/g   
 s/\bMESA_FORMAT_XRGB_UNORM\b/MESA_FORMAT_B4G4R4X4_UNORM/g
s/\bMESA_FORMAT_XRGB1555_UNORM\b/MESA_FORMAT_B5G5R5X1_UNORM/g
s/\bMESA_FORMAT_XRGB2101010_UNORM\b/MESA_FORMAT_B10G10R10X2_UNORM/g

 5 Compressed spelled out color components ALPHA, INTENSITY, and
LUMINANCE to A, I, and L:
s/\bMESA_FORMAT_ALPHA_UINT8\b/MESA_FORMAT_A_UINT8/g
s/\bMESA_FORMAT_ALPHA_UINT16\b/MESA_FORMAT_A_UINT16/g
s/\bMESA_FORMAT_ALPHA_UINT32\b/MESA_FORMAT_A_UINT32/g
s/\bMESA_FORMAT_ALPHA_INT32\b/MESA_FORMAT_A_INT32/g
s/\bMESA_FORMAT_ALPHA_INT16\b/MESA_FORMAT_A_INT16/g
s/\bMESA_FORMAT_ALPHA_INT8\b/MESA_FORMAT_A_INT8/g
s/\bMESA_FORMAT_INTESITY_UINT8\b/MESA_FORMAT_I_UINT8/g
s/\bMESA_FORMAT_INTESITY_UINT16\b/MESA_FORMAT_I_UINT16/g
s/\bMESA_FORMAT_INTESITY_UINT32\b/MESA_FORMAT_I_UINT32/g
s/\bMESA_FORMAT_INTESITY_INT32\b/MESA_FORMAT_I_INT32/g
s/\bMESA_FORMAT_INTESITY_INT16\b/MESA_FORMAT_I_INT16/g
s/\bMESA_FORMAT_INTESITY_INT8\b/MESA_FORMAT_I

Re: [Mesa-dev] [PATCH 2/3] nv50: add more RGB10A2 formats

[Ilia Mirkin]

On Wed, Jan 15, 2014 at 6:37 AM, Ilia Mirkin  wrote:
> On Wed, Dec 25, 2013 at 11:53 AM, Christoph Bumiller
>  wrote:
>> ---
>>  src/gallium/drivers/nouveau/nv50/nv50_formats.c | 7 +++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_formats.c 
>> b/src/gallium/drivers/nouveau/nv50/nv50_formats.c
>> index 0a7e812..b301890 100644
>> --- a/src/gallium/drivers/nouveau/nv50/nv50_formats.c
>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_formats.c
>> @@ -202,6 +202,8 @@ const struct nv50_format 
>> nv50_format_table[PIPE_FORMAT_COUNT] =
>> TBV, 1),
>> C4A(R10G10B10A2_SNORM, NONE, C0, C1, C2, C3, SNORM, 10_10_10_2, TV, 0),
>> C4A(B10G10R10A2_SNORM, NONE, C2, C1, C0, C3, SNORM, 10_10_10_2, TV, 1),
>> +   C4A(R10G10B10A2_UINT, RGB10_A2_UINT, C0, C1, C2, C3, UINT, 10_10_10_2, 
>> TRV, 0),
>> +   C4A(B10G10R10A2_UINT, RGB10_A2_UINT, C2, C1, C0, C3, UINT, 10_10_10_2, 
>> TV, 0),
>>
>> F3B(R11G11B10_FLOAT, R11G11B10_FLOAT, C0, C1, C2, xx, FLOAT, 11_11_10, 
>> IB),
>>
>> @@ -394,6 +396,11 @@ const struct nv50_format 
>> nv50_format_table[PIPE_FORMAT_COUNT] =
>> F1A(R16_SSCALED, NONE, C0, xx, xx, xx, SSCALED, 16, V),
>> F1A(R16_USCALED, NONE, C0, xx, xx, xx, USCALED, 16, V),
>>
>> +   C4A(R10G10B10A2_USCALED, NONE, C0, C1, C2, C3, USCALED, 10_10_10_2, V, 
>> 0),
>> +   C4A(R10G10B10A2_SSCALED, NONE, C0, C1, C2, C3, SSCALED, 10_10_10_2, V, 
>> 0),
>> +   C4A(B10G10R10A2_USCALED, NONE, C0, C1, C2, C3, USCALED, 10_10_10_2, V, 
>> 1),
>> +   C4A(B10G10R10A2_SSCALED, NONE, C0, C1, C2, C3, SSCALED, 10_10_10_2, V, 
>> 1),
>> +
>> C4A(R8G8B8A8_SSCALED, NONE, C0, C1, C2, C3, SSCALED, 8_8_8_8, V, 0),
>> C4A(R8G8B8A8_USCALED, NONE, C0, C1, C2, C3, USCALED, 8_8_8_8, V, 0),
>> F3A(R8G8B8_UNORM, NONE, C0, C1, C2, xx, UNORM, 8_8_8, V),
>
> FWIW I ran piglit on this (with your other patches that cause the
> RGB10A2 extensions to actually be turned on), and I saw this failure:
>
> $ bin/draw-vertices-2101010 -auto
> Int vertices - 2/10/10/10
> Unsigned Int vertices - 2/10/10/10
> Int Color - 2/10/10/10
> Probe color at (45,5)
>   Expected: 1.00 0.00 0.00 0.333000
>   Observed: 1.00 0.00 0.00 0.00
> Unsigned Int Color - 2/10/10/10
> Int BGRA Color - 2/10/10/10
> Probe color at (85,5)
>   Expected: 0.00 0.00 1.00 0.333000
>   Observed: 0.00 0.00 1.00 0.00
> Unsigned Int BGRA Color - 2/10/10/10
> Int 2/10/10/10 - test ABI
> Unsigned 2/10/10/10 - test ABI

BTW, I tried to look into this...

(a) Shouldn't the second C4A in the first hunk have a 1 for the BGRA
bit? And similarly, shouldn't the components be flipped for the BGRA
formats in the second hunk? Doing this made the
GL_ARB_texture_rgb10_a2ui tests pass (well, mostly -- projective mode,
whatever that is, still doesn't work, but that wasn't tested before)

(b) I think that the draw-vertices test might be broken, but not in
any way that matters:
  - its i32to2 function is wrong. but it's fine for 0 which is what's
being passed in
  - 0 for a signed (2's complement) A2 component should become
0.6, not 0.3. I think. It's the third of 4 values -- -2, -1,
0, 1. But that's not what we're seeing anyways.

(c) When I ran on the blob earlier, it had identical failures, and so
does llvmpipe.

>
> There were also ARB_texture_rgb10_a2ui failures with texwrap
> bordercolor's, but there are lots of other such failures with integer
> formats, so I assume that's all part of the same thing. For reference:
>
> $ bin/texwrap GL_ARB_texture_rgb10_a2ui bordercolor -fbo -auto
> Testing GL_ARB_texture_rgb10_a2ui.
> Testing the border color only.
> Testing GL_RGB10_A2UI, border color only
> Fail with NEAREST and CLAMP_TO_BORDER at (95,36) @ 0,0
>   Expected: 25 229 127 170
>   Observed: 0 0 0 0
> Fail with NEAREST and MIRROR_CLAMP_TO_BORDER_EXT at (273,36) @ 0,0
>   Expected: 25 229 127 170
>   Observed: 0 0 0 0
>
> $ bin/texwrap GL_ARB_texture_rgb10_a2ui bordercolor swizzled -fbo -auto
> Testing GL_ARB_texture_rgb10_a2ui.
> Testing the border color only.
> Using texture swizzling.
> Testing GL_RGB10_A2UI, swizzled, border color only
> Fail with NEAREST and CLAMP_TO_BORDER at (95,36) @ 0,0
>   Expected: 127 25 229 170
>   Observed: 0 0 0 0
> Fail with NEAREST and MIRROR_CLAMP_TO_BORDER_EXT at (273,36) @ 0,0
>   Expected: 127 25 229 170
>   Observed: 0 0 0 0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev