[Mesa-dev] [PATCH 02/13] mesa: Add SYSTEM_VALUE_VERTEX_ID_ZERO_BASE
From: Ian Romanick ian.d.roman...@intel.com
There exists hardware, such as i965, that does not implement the OpenGL
semantic for gl_VertexID. Instead, that hardware does not include the
value of basevertex in the gl_VertexID value.
SYSTEM_VALUE_VERTEX_ID_ZERO_BASE is the system value that represents
this semantic.
Signed-off-by: Ian Romanick ian.d.roman...@intel.com
Reviewed-by: Kenneth Graunke kenn...@whitecape.org
---
src/mesa/main/mtypes.h | 12
src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 1 +
2 files changed, 13 insertions(+)
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 207be0a..c603007 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2083,6 +2083,8 @@ typedef enum
*
* gl_VertexID gets basevertex added in. This differs from DirectX where
* SV_VertexID does \b not get basevertex added in.
+*
+* \sa SYSTEM_VALUE_VERTEX_ID_ZERO_BASE
*/
SYSTEM_VALUE_VERTEX_ID,
@@ -2114,6 +2116,16 @@ typedef enum
* Note that baseinstance is \b not included in the value of instance.
*/
SYSTEM_VALUE_INSTANCE_ID,
+
+ /**
+* DirectX-style vertex ID.
+*
+* Unlike \c SYSTEM_VALUE_VERTEX_ID, this system value does \b not include
+* the value of basevertex.
+*
+* \sa SYSTEM_VALUE_VERTEX_ID, SYSTEM_VALUE_BASE_VERTEX
+*/
+ SYSTEM_VALUE_VERTEX_ID_ZERO_BASE,
/*@}*/
/**
diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index 0290553..81f08cd 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -4171,6 +4171,7 @@ const unsigned _mesa_sysval_to_semantic[SYSTEM_VALUE_MAX]
= {
*/
TGSI_SEMANTIC_VERTEXID,
TGSI_SEMANTIC_INSTANCEID,
+ 0,
/* Geometry shader
*/
--
2.0.2
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 11/13] i965: Refactor Gen4-7 VERTEX_BUFFER_STATE emission into a helper.
We'll need to emit another VERTEX_BUFFER_STATE for gl_BaseVertex;
pulling this into a helper function will save us from having to deal
with cross-generation differences in that code.
Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
src/mesa/drivers/dri/i965/brw_draw_upload.c | 77 ++---
1 file changed, 47 insertions(+), 30 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c
b/src/mesa/drivers/dri/i965/brw_draw_upload.c
index 37a65bc..7c01d79 100644
--- a/src/mesa/drivers/dri/i965/brw_draw_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c
@@ -625,9 +625,52 @@ brw_prepare_shader_draw_parameters(struct brw_context *brw)
}
}
-static void brw_emit_vertices(struct brw_context *brw)
+/**
+ * Emit a VERTEX_BUFFER_STATE entry (part of 3DSTATE_VERTEX_BUFFERS).
+ */
+static void
+emit_vertex_buffer_state(struct brw_context *brw,
+ unsigned buffer_nr,
+ drm_intel_bo *bo,
+ unsigned bo_ending_address,
+ unsigned bo_offset,
+ unsigned stride,
+ unsigned step_rate)
{
struct gl_context *ctx = brw-ctx;
+ uint32_t dw0;
+
+ if (brw-gen = 6) {
+ dw0 = (buffer_nr GEN6_VB0_INDEX_SHIFT) |
+(step_rate ? GEN6_VB0_ACCESS_INSTANCEDATA
+ : GEN6_VB0_ACCESS_VERTEXDATA);
+ } else {
+ dw0 = (buffer_nr BRW_VB0_INDEX_SHIFT) |
+(step_rate ? BRW_VB0_ACCESS_INSTANCEDATA
+ : BRW_VB0_ACCESS_VERTEXDATA);
+ }
+
+ if (brw-gen = 7)
+ dw0 |= GEN7_VB0_ADDRESS_MODIFYENABLE;
+
+ if (brw-gen == 7)
+ dw0 |= GEN7_MOCS_L3 16;
+
+ WARN_ONCE(stride = (brw-gen = 5 ? 2048 : 2047),
+ VBO stride %d too large, bad rendering may occur\n,
+ stride);
+ OUT_BATCH(dw0 | (stride BRW_VB0_PITCH_SHIFT));
+ OUT_RELOC(bo, I915_GEM_DOMAIN_VERTEX, 0, bo_offset);
+ if (brw-gen = 5) {
+ OUT_RELOC(bo, I915_GEM_DOMAIN_VERTEX, 0, bo_ending_address);
+ } else {
+ OUT_BATCH(0);
+ }
+ OUT_BATCH(step_rate);
+}
+
+static void brw_emit_vertices(struct brw_context *brw)
+{
GLuint i, nr_elements;
brw_prepare_vertices(brw);
@@ -680,36 +723,10 @@ static void brw_emit_vertices(struct brw_context *brw)
OUT_BATCH((_3DSTATE_VERTEX_BUFFERS 16) | (4*brw-vb.nr_buffers - 1));
for (i = 0; i brw-vb.nr_buffers; i++) {
struct brw_vertex_buffer *buffer = brw-vb.buffers[i];
-uint32_t dw0;
-
-if (brw-gen = 6) {
- dw0 = buffer-step_rate
-? GEN6_VB0_ACCESS_INSTANCEDATA
-: GEN6_VB0_ACCESS_VERTEXDATA;
- dw0 |= i GEN6_VB0_INDEX_SHIFT;
-} else {
- dw0 = buffer-step_rate
-? BRW_VB0_ACCESS_INSTANCEDATA
-: BRW_VB0_ACCESS_VERTEXDATA;
- dw0 |= i BRW_VB0_INDEX_SHIFT;
-}
+ emit_vertex_buffer_state(brw, i, buffer-bo, buffer-bo-size - 1,
+ buffer-offset, buffer-stride,
+ buffer-step_rate);
-if (brw-gen = 7)
- dw0 |= GEN7_VB0_ADDRESS_MODIFYENABLE;
-
- if (brw-gen == 7)
- dw0 |= GEN7_MOCS_L3 16;
-
- WARN_ONCE(buffer-stride = (brw-gen = 5 ? 2048 : 2047),
- VBO stride %d too large, bad rendering may occur\n,
- buffer-stride);
-OUT_BATCH(dw0 | (buffer-stride BRW_VB0_PITCH_SHIFT));
-OUT_RELOC(buffer-bo, I915_GEM_DOMAIN_VERTEX, 0, buffer-offset);
-if (brw-gen = 5) {
- OUT_RELOC(buffer-bo, I915_GEM_DOMAIN_VERTEX, 0, buffer-bo-size -
1);
-} else
- OUT_BATCH(0);
-OUT_BATCH(buffer-step_rate);
}
ADVANCE_BATCH();
}
--
2.0.2
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 01/13] mesa: Document SYSTEM_VALUE_VERTEX_ID and SYSTEM_VALUE_INSTANCE_ID
From: Ian Romanick ian.d.roman...@intel.com
v2: Additions to the documentation for SYSTEM_VALUE_VERTEX_ID. Quote
the GL_ARB_shader_draw_parameters spec and mention DirectX SV_VertexID.
Signed-off-by: Ian Romanick ian.d.roman...@intel.com
Reviewed-by: Kenneth Graunke kenn...@whitecape.org
---
src/mesa/main/mtypes.h | 57 ++
1 file changed, 57 insertions(+)
This series is available as the 'basevertex-v9' branch of ~kwg/mesa
(not ~idr/mesa). Ken tested this series against Piglit on Haswell and
Broadwell, but did not test earlier hardware, nor run the ES3 tests.
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index ff130da..207be0a 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2055,7 +2055,64 @@ typedef enum
* \name Vertex shader system values
*/
/*@{*/
+ /**
+* OpenGL-style vertex ID.
+*
+* Section 2.11.7 (Shader Execution), subsection Shader Inputs, of the
+* OpenGL 3.3 core profile spec says:
+*
+* gl_VertexID holds the integer index i implicitly passed by
+* DrawArrays or one of the other drawing commands defined in section
+* 2.8.3.
+*
+* Section 2.8.3 (Drawing Commands) of the same spec says:
+*
+* The commandsare equivalent to the commands with the same base
+* name (without the BaseVertex suffix), except that the ith element
+* transferred by the corresponding draw call will be taken from
+* element indices[i] + basevertex of each enabled array.
+*
+* Additionally, the overview in the GL_ARB_shader_draw_parameters spec
+* says:
+*
+* In unextended GL, vertex shaders have inputs named gl_VertexID and
+* gl_InstanceID, which contain, respectively the index of the vertex
+* and instance. The value of gl_VertexID is the implicitly passed
+* index of the vertex being processed, which includes the value of
+* baseVertex, for those commands that accept it.
+*
+* gl_VertexID gets basevertex added in. This differs from DirectX where
+* SV_VertexID does \b not get basevertex added in.
+*/
SYSTEM_VALUE_VERTEX_ID,
+
+ /**
+* Instanced ID as supplied to gl_InstanceID
+*
+* Values assigned to gl_InstanceID always begin with zero, regardless of
+* the value of baseinstance.
+*
+* Section 11.1.3.9 (Shader Inputs) of the OpenGL 4.4 core profile spec
+* says:
+*
+* gl_InstanceID holds the integer instance number of the current
+* primitive in an instanced draw call (see section 10.5).
+*
+* Through a big chain of pseudocode, section 10.5 describes that
+* baseinstance is not counted by gl_InstanceID. In that section, notice
+*
+* If an enabled vertex attribute array is instanced (it has a
+* non-zero divisor as specified by VertexAttribDivisor), the element
+* index that is transferred to the GL, for all vertices, is given by
+*
+* floor(instance/divisor) + baseinstance
+*
+* If an array corresponding to an attribute required by a vertex
+* shader is not enabled, then the corresponding element is taken from
+* the current attribute state (see section 10.2).
+*
+* Note that baseinstance is \b not included in the value of instance.
+*/
SYSTEM_VALUE_INSTANCE_ID,
/*@}*/
--
2.0.2
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 08/13] i965: Handle SYSTEM_VALUE_VERTEX_ID_ZERO_BASE
From: Ian Romanick ian.d.roman...@intel.com
Signed-off-by: Ian Romanick ian.d.roman...@intel.com
Reviewed-by: Kenneth Graunke kenn...@whitecape.org
---
src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp
b/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp
index 64c72fd..6a1c049 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp
@@ -155,6 +155,7 @@ vec4_vs_visitor::make_reg_for_system_value(ir_variable *ir)
switch (ir-data.location) {
case SYSTEM_VALUE_VERTEX_ID:
+ case SYSTEM_VALUE_VERTEX_ID_ZERO_BASE:
reg-writemask = WRITEMASK_X;
break;
case SYSTEM_VALUE_INSTANCE_ID:
--
2.0.2
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 06/13] mesa: Replace string comparisons with SYSTEM_VALUE enum checks.
This is more efficient. Signed-off-by: Kenneth Graunke kenn...@whitecape.org --- src/mesa/main/shader_query.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) You can't see it in the diff, but this is in a switch statement on var-data.mode, in the ir_var_system_value case. diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp index 4267743..4871d09 100644 --- a/src/mesa/main/shader_query.cpp +++ b/src/mesa/main/shader_query.cpp @@ -92,8 +92,8 @@ is_active_attrib(const ir_variable *var) * are enumerated, including the special built-in inputs gl_VertexID * and gl_InstanceID. */ - return !strcmp(var-name, gl_VertexID) || - !strcmp(var-name, gl_InstanceID); + return var-data.location == SYSTEM_VALUE_VERTEX_ID || + var-data.location == SYSTEM_VALUE_INSTANCE_ID; default: return false; -- 2.0.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 04/13] glsl/linker: Make get_main_function_signature public
From: Ian Romanick ian.d.roman...@intel.com
The next patch will use this function in a different file.
Signed-off-by: Ian Romanick ian.d.roman...@intel.com
Reviewed-by: Kenneth Graunke kenn...@whitecape.org
---
src/glsl/linker.cpp | 9 +
src/glsl/linker.h | 3 +++
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 0096fb0..ae6c91a 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -1115,8 +1115,8 @@ move_non_declarations(exec_list *instructions, exec_node
*last,
/**
* Get the function signature for main from a shader
*/
-static ir_function_signature *
-get_main_function_signature(gl_shader *sh)
+ir_function_signature *
+link_get_main_function_signature(gl_shader *sh)
{
ir_function *const f = sh-symbols-get_function(main);
if (f != NULL) {
@@ -1644,7 +1644,7 @@ link_intrastage_shaders(void *mem_ctx,
*/
gl_shader *main = NULL;
for (unsigned i = 0; i num_shaders; i++) {
- if (get_main_function_signature(shader_list[i]) != NULL) {
+ if (link_get_main_function_signature(shader_list[i]) != NULL) {
main = shader_list[i];
break;
}
@@ -1673,7 +1673,8 @@ link_intrastage_shaders(void *mem_ctx,
/* The a pointer to the main function in the final linked shader (i.e., the
* copy of the original shader that contained the main function).
*/
- ir_function_signature *const main_sig = get_main_function_signature(linked);
+ ir_function_signature *const main_sig =
+ link_get_main_function_signature(linked);
/* Move any instructions other than variable declarations or function
* declarations into main.
diff --git a/src/glsl/linker.h b/src/glsl/linker.h
index 8851da6..5f9bb04 100644
--- a/src/glsl/linker.h
+++ b/src/glsl/linker.h
@@ -26,6 +26,9 @@
#ifndef GLSL_LINKER_H
#define GLSL_LINKER_H
+ir_function_signature *
+link_get_main_function_signature(gl_shader *sh);
+
extern bool
link_function_calls(gl_shader_program *prog, gl_shader *main,
gl_shader **shader_list, unsigned num_shaders);
--
2.0.2
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 10/13] i965: Make gl_BaseVertex available in a buffer object.
This will be used for GL_ARB_shader_draw_parameters, as well as fixing
gl_VertexID, which is supposed to include gl_BaseVertex's value.
For indirect draws, we simply point at the indirect buffer; for normal
draws, we upload the value via the upload buffer.
Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
src/mesa/drivers/dri/i965/brw_context.h | 7 +++
src/mesa/drivers/dri/i965/brw_draw.c| 14 ++
src/mesa/drivers/dri/i965/brw_draw_upload.c | 10 ++
3 files changed, 31 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/brw_context.h
b/src/mesa/drivers/dri/i965/brw_context.h
index eea7938..b5b5e41 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -1063,6 +1063,13 @@ struct brw_context
int start_vertex_location;
int base_vertex_location;
+
+ /**
+ * Buffer and offset used for GL_ARB_shader_draw_parameters
+ * (for now, only gl_BaseVertex).
+ */
+ drm_intel_bo *draw_params_bo;
+ uint32_t draw_params_offset;
} draw;
struct {
diff --git a/src/mesa/drivers/dri/i965/brw_draw.c
b/src/mesa/drivers/dri/i965/brw_draw.c
index d8e16a7..f0aaec7 100644
--- a/src/mesa/drivers/dri/i965/brw_draw.c
+++ b/src/mesa/drivers/dri/i965/brw_draw.c
@@ -465,6 +465,20 @@ static bool brw_try_draw_prims( struct gl_context *ctx,
brw-draw.start_vertex_location = prims[i].start;
brw-draw.base_vertex_location = prims[i].basevertex;
+ if (prims[i].is_indirect) {
+ /* Point draw_params_bo at the indirect buffer. */
+ brw-draw.draw_params_bo =
+intel_buffer_object(ctx-DrawIndirectBuffer)-buffer;
+ brw-draw.draw_params_offset =
+prims[i].indirect_offset + (prims[i].indexed ? 12 : 8);
+ } else {
+ /* Set draw_params_bo to NULL so brw_prepare_vertices knows it
+ * has to upload gl_BaseVertex and such if they're needed.
+ */
+ brw-draw.draw_params_bo = NULL;
+ brw-draw.draw_params_offset = 0;
+ }
+
if (brw-gen 6)
brw_set_prim(brw, prims[i]);
else
diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c
b/src/mesa/drivers/dri/i965/brw_draw_upload.c
index 38b1087..37a65bc 100644
--- a/src/mesa/drivers/dri/i965/brw_draw_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c
@@ -607,11 +607,21 @@ brw_prepare_vertices(struct brw_context *brw)
void
brw_prepare_shader_draw_parameters(struct brw_context *brw)
{
+ int *gl_basevertex_value;
if (brw-draw.indexed) {
brw-draw.start_vertex_location += brw-ib.start_vertex_offset;
brw-draw.base_vertex_location += brw-vb.start_vertex_bias;
+ gl_basevertex_value = brw-draw.base_vertex_location;
} else {
brw-draw.start_vertex_location += brw-vb.start_vertex_bias;
+ gl_basevertex_value = brw-draw.start_vertex_location;
+ }
+
+ /* For non-indirect draws, upload gl_BaseVertex. */
+ if (brw-vs.prog_data-uses_vertexid brw-draw.draw_params_bo == NULL) {
+ intel_upload_data(brw, gl_basevertex_value, 4, 4,
+ brw-draw.draw_params_bo,
+brw-draw.draw_params_offset);
}
}
--
2.0.2
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 12/13] i965: Expose gl_BaseVertex via a vertex attribute.
Now that we have the data available, we need to expose it to the
shaders. We can reuse the same vertex element that we use for
gl_VertexID, but we need to back it by an actual vertex buffer.
A hardware restriction requires that vertex attributes coming from a
buffer (STORE_SRC) must come before any other types (i.e. STORE_0).
So, we have to make gl_BaseVertex be the .x component of the vertex
attribute. This means moving gl_VertexID to a different component.
I chose to move gl_VertexID and gl_InstanceID to the .z and .w
components, respectively, to make room for gl_BaseInstance in the .y
component (which would also come from a buffer, and therefore be
STORE_SRC).
Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
src/mesa/drivers/dri/i965/brw_draw_upload.c | 38 ++---
src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp | 7 ++--
src/mesa/drivers/dri/i965/gen8_draw_upload.c | 40 +++
3 files changed, 65 insertions(+), 20 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c
b/src/mesa/drivers/dri/i965/brw_draw_upload.c
index 7c01d79..d59ca8b 100644
--- a/src/mesa/drivers/dri/i965/brw_draw_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c
@@ -712,15 +712,18 @@ static void brw_emit_vertices(struct brw_context *brw)
/* Now emit VB and VEP state packets.
*/
- if (brw-vb.nr_buffers) {
+ unsigned nr_buffers =
+ brw-vb.nr_buffers + brw-vs.prog_data-uses_vertexid;
+
+ if (nr_buffers) {
if (brw-gen = 6) {
-assert(brw-vb.nr_buffers = 33);
+assert(nr_buffers = 33);
} else {
-assert(brw-vb.nr_buffers = 17);
+assert(nr_buffers = 17);
}
- BEGIN_BATCH(1 + 4*brw-vb.nr_buffers);
- OUT_BATCH((_3DSTATE_VERTEX_BUFFERS 16) | (4*brw-vb.nr_buffers - 1));
+ BEGIN_BATCH(1 + 4 * nr_buffers);
+ OUT_BATCH((_3DSTATE_VERTEX_BUFFERS 16) | (4 * nr_buffers - 1));
for (i = 0; i brw-vb.nr_buffers; i++) {
struct brw_vertex_buffer *buffer = brw-vb.buffers[i];
emit_vertex_buffer_state(brw, i, buffer-bo, buffer-bo-size - 1,
@@ -728,6 +731,15 @@ static void brw_emit_vertices(struct brw_context *brw)
buffer-step_rate);
}
+
+ if (brw-vs.prog_data-uses_vertexid) {
+ emit_vertex_buffer_state(brw, brw-vb.nr_buffers,
+ brw-draw.draw_params_bo,
+ brw-draw.draw_params_bo-size - 1,
+ brw-draw.draw_params_offset,
+ 0, /* stride */
+ 0); /* step rate */
+ }
ADVANCE_BATCH();
}
@@ -815,15 +827,19 @@ static void brw_emit_vertices(struct brw_context *brw)
if (brw-vs.prog_data-uses_vertexid) {
uint32_t dw0 = 0, dw1 = 0;
- dw1 = ((BRW_VE1_COMPONENT_STORE_VID BRW_VE1_COMPONENT_0_SHIFT) |
-(BRW_VE1_COMPONENT_STORE_IID BRW_VE1_COMPONENT_1_SHIFT) |
-(BRW_VE1_COMPONENT_STORE_0 BRW_VE1_COMPONENT_2_SHIFT) |
-(BRW_VE1_COMPONENT_STORE_0 BRW_VE1_COMPONENT_3_SHIFT));
+ dw1 = (BRW_VE1_COMPONENT_STORE_SRC BRW_VE1_COMPONENT_0_SHIFT) |
+(BRW_VE1_COMPONENT_STORE_0BRW_VE1_COMPONENT_1_SHIFT) |
+(BRW_VE1_COMPONENT_STORE_VID BRW_VE1_COMPONENT_2_SHIFT) |
+(BRW_VE1_COMPONENT_STORE_IID BRW_VE1_COMPONENT_3_SHIFT);
if (brw-gen = 6) {
-dw0 |= GEN6_VE0_VALID;
+ dw0 |= GEN6_VE0_VALID |
+brw-vb.nr_buffers GEN6_VE0_INDEX_SHIFT |
+BRW_SURFACEFORMAT_R32_UINT BRW_VE0_FORMAT_SHIFT;
} else {
-dw0 |= BRW_VE0_VALID;
+ dw0 |= BRW_VE0_VALID |
+brw-vb.nr_buffers BRW_VE0_INDEX_SHIFT |
+BRW_SURFACEFORMAT_R32_UINT BRW_VE0_FORMAT_SHIFT;
dw1 |= (i * 4) BRW_VE1_DST_OFFSET_SHIFT;
}
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp
b/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp
index 6a1c049..667ed68 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp
@@ -154,12 +154,15 @@ vec4_vs_visitor::make_reg_for_system_value(ir_variable
*ir)
vs_prog_data-uses_vertexid = true;
switch (ir-data.location) {
+ case SYSTEM_VALUE_BASE_VERTEX:
+ reg-writemask = WRITEMASK_X;
+ break;
case SYSTEM_VALUE_VERTEX_ID:
case SYSTEM_VALUE_VERTEX_ID_ZERO_BASE:
- reg-writemask = WRITEMASK_X;
+ reg-writemask = WRITEMASK_Z;
break;
case SYSTEM_VALUE_INSTANCE_ID:
- reg-writemask = WRITEMASK_Y;
+ reg-writemask = WRITEMASK_W;
break;
default:
unreachable(not reached);
diff --git a/src/mesa/drivers/dri/i965/gen8_draw_upload.c
b/src/mesa/drivers/dri/i965/gen8_draw_upload.c
index 8e4fe5d..7e4c1eb 100644
--- a/src/mesa/drivers/dri/i965/gen8_draw_upload.c
+++ b/src/mesa/drivers/dri/i965/gen8_draw_upload.c
[Mesa-dev] [PATCH 09/13] i965: Calculate start/base_vertex_location after preparing vertices.
Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
src/mesa/drivers/dri/i965/brw_context.h | 8
src/mesa/drivers/dri/i965/brw_draw.c | 17 -
src/mesa/drivers/dri/i965/brw_draw.h | 2 ++
src/mesa/drivers/dri/i965/brw_draw_upload.c | 12
src/mesa/drivers/dri/i965/brw_state_upload.c | 6 +++---
src/mesa/drivers/dri/i965/gen8_draw_upload.c | 1 +
6 files changed, 34 insertions(+), 12 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_context.h
b/src/mesa/drivers/dri/i965/brw_context.h
index 1bbcf46..eea7938 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -1058,6 +1058,14 @@ struct brw_context
bool no_depth_or_stencil;
struct {
+ /** Does the current draw use the index buffer? */
+ bool indexed;
+
+ int start_vertex_location;
+ int base_vertex_location;
+ } draw;
+
+ struct {
struct brw_vertex_element inputs[VERT_ATTRIB_MAX];
struct brw_vertex_buffer buffers[VERT_ATTRIB_MAX];
diff --git a/src/mesa/drivers/dri/i965/brw_draw.c
b/src/mesa/drivers/dri/i965/brw_draw.c
index 4dae7d3..d8e16a7 100644
--- a/src/mesa/drivers/dri/i965/brw_draw.c
+++ b/src/mesa/drivers/dri/i965/brw_draw.c
@@ -167,26 +167,19 @@ static void brw_emit_prim(struct brw_context *brw,
{
int verts_per_instance;
int vertex_access_type;
- int start_vertex_location;
- int base_vertex_location;
int indirect_flag;
DBG(PRIM: %s %d %d\n, _mesa_lookup_enum_by_nr(prim-mode),
prim-start, prim-count);
- start_vertex_location = prim-start;
- base_vertex_location = prim-basevertex;
if (prim-indexed) {
vertex_access_type = brw-gen = 7 ?
GEN7_3DPRIM_VERTEXBUFFER_ACCESS_RANDOM :
GEN4_3DPRIM_VERTEXBUFFER_ACCESS_RANDOM;
- start_vertex_location += brw-ib.start_vertex_offset;
- base_vertex_location += brw-vb.start_vertex_bias;
} else {
vertex_access_type = brw-gen = 7 ?
GEN7_3DPRIM_VERTEXBUFFER_ACCESS_SEQUENTIAL :
GEN4_3DPRIM_VERTEXBUFFER_ACCESS_SEQUENTIAL;
- start_vertex_location += brw-vb.start_vertex_bias;
}
/* We only need to trim the primitive count on pre-Gen6. */
@@ -261,10 +254,10 @@ static void brw_emit_prim(struct brw_context *brw,
vertex_access_type);
}
OUT_BATCH(verts_per_instance);
- OUT_BATCH(start_vertex_location);
+ OUT_BATCH(brw-draw.start_vertex_location);
OUT_BATCH(prim-num_instances);
OUT_BATCH(prim-base_instance);
- OUT_BATCH(base_vertex_location);
+ OUT_BATCH(brw-draw.base_vertex_location);
ADVANCE_BATCH();
/* Only used on Sandybridge; harmless to set elsewhere. */
@@ -467,12 +460,18 @@ static bool brw_try_draw_prims( struct gl_context *ctx,
brw_merge_inputs(brw, arrays);
}
}
+
+ brw-draw.indexed = prims[i].indexed;
+ brw-draw.start_vertex_location = prims[i].start;
+ brw-draw.base_vertex_location = prims[i].basevertex;
+
if (brw-gen 6)
brw_set_prim(brw, prims[i]);
else
gen6_set_prim(brw, prims[i]);
retry:
+
/* Note that before the loop, brw-state.dirty.brw was set to != 0, and
* that the state updated in the loop outside of this block is that in
* *_set_prim or intel_batchbuffer_flush(), which only impacts
diff --git a/src/mesa/drivers/dri/i965/brw_draw.h
b/src/mesa/drivers/dri/i965/brw_draw.h
index 774f1d4..fc83dcd 100644
--- a/src/mesa/drivers/dri/i965/brw_draw.h
+++ b/src/mesa/drivers/dri/i965/brw_draw.h
@@ -47,6 +47,8 @@ void brw_draw_prims( struct gl_context *ctx,
void brw_draw_init( struct brw_context *brw );
void brw_draw_destroy( struct brw_context *brw );
+void brw_prepare_shader_draw_parameters(struct brw_context *);
+
/* brw_primitive_restart.c */
GLboolean
brw_handle_primitive_restart(struct gl_context *ctx,
diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c
b/src/mesa/drivers/dri/i965/brw_draw_upload.c
index 5d6b766..38b1087 100644
--- a/src/mesa/drivers/dri/i965/brw_draw_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c
@@ -604,12 +604,24 @@ brw_prepare_vertices(struct brw_context *brw)
brw-vb.nr_buffers = j;
}
+void
+brw_prepare_shader_draw_parameters(struct brw_context *brw)
+{
+ if (brw-draw.indexed) {
+ brw-draw.start_vertex_location += brw-ib.start_vertex_offset;
+ brw-draw.base_vertex_location += brw-vb.start_vertex_bias;
+ } else {
+ brw-draw.start_vertex_location += brw-vb.start_vertex_bias;
+ }
+}
+
static void brw_emit_vertices(struct brw_context *brw)
{
struct gl_context *ctx = brw-ctx;
GLuint i, nr_elements;
brw_prepare_vertices(brw);
+ brw_prepare_shader_draw_parameters(brw);
brw_emit_query_begin(brw);
diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c
b/src/mesa/drivers/dri/i965/brw_state_upload.c
index 3a452c3..e946621 100644
--- a/src/mesa/drivers/dri/i965/brw_state_upload.c
[Mesa-dev] [PATCH 07/13] mesa: Fix glGetActiveAttribute for gl_VertexID when lowered.
The lower_vertex_id pass converts uses of the gl_VertexID system value
to the gl_BaseVertex and gl_VertexIDMESA system values. Since
gl_VertexID is no longer accessed, it would not be considered active.
Of course, it should be, since the shader uses gl_VertexID.
Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
src/mesa/main/shader_query.cpp | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
Avoids regressing Piglit's gl-get-active-attrib-returns-all-inputs when
enabling lowering later in the series.
diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
index 4871d09..c395a78 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -93,6 +93,7 @@ is_active_attrib(const ir_variable *var)
* and gl_InstanceID.
*/
return var-data.location == SYSTEM_VALUE_VERTEX_ID ||
+ var-data.location == SYSTEM_VALUE_VERTEX_ID_ZERO_BASE ||
var-data.location == SYSTEM_VALUE_INSTANCE_ID;
default:
@@ -128,12 +129,22 @@ _mesa_GetActiveAttrib(GLhandleARB program, GLuint
desired_index,
foreach_in_list(ir_instruction, node, ir) {
const ir_variable *const var = node-as_variable();
+ const char *var_name = var-name;
if (!is_active_attrib(var))
continue;
if (current_index == desired_index) {
-_mesa_copy_string(name, maxLength, length, var-name);
+ /* Since gl_VertexID may be lowered to gl_VertexIDMESA, we need to
+ * consider gl_VertexIDMESA as gl_VertexID for purposes of checking
+ * active attributes.
+ */
+ if (var-data.mode == ir_var_system_value
+ var-data.location == SYSTEM_VALUE_VERTEX_ID_ZERO_BASE) {
+var_name = gl_VertexID;
+ }
+
+_mesa_copy_string(name, maxLength, length, var_name);
if (size)
*size = (var-type-is_array()) ? var-type-length : 1;
--
2.0.2
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 05/13] glsl: Add a lowering pass for gl_VertexID
From: Ian Romanick ian.d.roman...@intel.com
Converts gl_VertexID to (gl_VertexIDMESA + gl_BaseVertex). gl_VertexIDMESA
is backed by SYSTEM_VALUE_VERTEX_ID_ZERO_BASE, and gl_BaseVertex is backed
by SYSTEM_VALUE_BASE_VERTEX.
v2: Put the enum in struct gl_constants and propoerly resolve the scope
in C++ code. Fix suggested by Marek.
v3: Reabase on Matt's foreach_in_list changes (was using foreach_list).
v4 (Ken): Use a systemvalue instead of a uniform because
STATE_BASE_VERTEX has been removed.
v5: Use a boolean to select lowering, and only allow one lowering
method. Suggested by Ken.
v6 (Ken): Replace strcmp against literal gl_BaseVertex/gl_VertexID
with SYSTEM_VALUE enum checks, for efficiency.
Signed-off-by: Ian Romanick ian.d.roman...@intel.com
Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
src/glsl/Makefile.sources| 1 +
src/glsl/ir_optimization.h | 2 +
src/glsl/linker.cpp | 3 +
src/glsl/lower_vertex_id.cpp | 144 +++
src/mesa/main/context.c | 6 ++
src/mesa/main/mtypes.h | 9 +++
6 files changed, 165 insertions(+)
create mode 100644 src/glsl/lower_vertex_id.cpp
I suppose this is Signed-off-by [v4, v6] and Reviewed-by [v5]...
diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources
index 2131dda..cb8d5a6 100644
--- a/src/glsl/Makefile.sources
+++ b/src/glsl/Makefile.sources
@@ -76,6 +76,7 @@ LIBGLSL_FILES = \
$(GLSL_SRCDIR)/lower_vec_index_to_swizzle.cpp \
$(GLSL_SRCDIR)/lower_vector.cpp \
$(GLSL_SRCDIR)/lower_vector_insert.cpp \
+ $(GLSL_SRCDIR)/lower_vertex_id.cpp \
$(GLSL_SRCDIR)/lower_output_reads.cpp \
$(GLSL_SRCDIR)/lower_ubo_reference.cpp \
$(GLSL_SRCDIR)/opt_algebraic.cpp \
diff --git a/src/glsl/ir_optimization.h b/src/glsl/ir_optimization.h
index b83c225..2892dc2 100644
--- a/src/glsl/ir_optimization.h
+++ b/src/glsl/ir_optimization.h
@@ -125,6 +125,8 @@ bool optimize_redundant_jumps(exec_list *instructions);
bool optimize_split_arrays(exec_list *instructions, bool linked);
bool lower_offset_arrays(exec_list *instructions);
+bool lower_vertex_id(gl_shader *shader);
+
ir_rvalue *
compare_index_block(exec_list *instructions, ir_variable *index,
unsigned base, unsigned components, void *mem_ctx);
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index ae6c91a..aadc47d 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -1737,6 +1737,9 @@ link_intrastage_shaders(void *mem_ctx,
}
}
+ if (ctx-Const.VertexID_is_zero_based)
+ lower_vertex_id(linked);
+
/* Make a pass over all variable declarations to ensure that arrays with
* unspecified sizes have a size specified. The size is inferred from the
* max_array_access field.
diff --git a/src/glsl/lower_vertex_id.cpp b/src/glsl/lower_vertex_id.cpp
new file mode 100644
index 000..fc90bc8
--- /dev/null
+++ b/src/glsl/lower_vertex_id.cpp
@@ -0,0 +1,144 @@
+/*
+ * Copyright © 2014 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.
+ */
+
+/**
+ * \file lower_vertex_id.cpp
+ *
+ * There exists hardware, such as i965, that does not implement the OpenGL
+ * semantic for gl_VertexID. Instead, that hardware does not include the
+ * value of basevertex in the gl_VertexID value. To implement the OpenGL
+ * semantic, we'll have to convert gl_Vertex_ID to
+ * gl_VertexIDMESA+gl_BaseVertexMESA.
+ */
+
+#include glsl_symbol_table.h
+#include ir_hierarchical_visitor.h
+#include ir.h
+#include ir_builder.h
+#include linker.h
+#include program/prog_statevars.h
+
+namespace {
+
+class lower_vertex_id_visitor : public ir_hierarchical_visitor {
+public:
+ explicit lower_vertex_id_visitor(ir_function_signature *main_sig,
+exec_list *ir_list)
+ : progress(false), VertexID(NULL), gl_VertexID(NULL),
+
[Mesa-dev] [PATCH 13/13] i965: Request lowering gl_VertexID
From: Ian Romanick ian.d.roman...@intel.com Fixes the (new) piglit tests gles-3.0-drawarrays-vertexid, gl-3.0-multidrawarrays-vertexid, and gl-3.2-basevertex-vertexid. Fixes gles3conform failure in: ES3-CTS.gtf.GL3Tests.transform_feedback.transform_feedback_vertex_id Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=80247 Signed-off-by: Ian Romanick ian.d.roman...@intel.com Reviewed-by: Kenneth Graunke kenn...@whitecape.org --- src/mesa/drivers/dri/i965/brw_context.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c index 52f2557..d309c5b 100644 --- a/src/mesa/drivers/dri/i965/brw_context.c +++ b/src/mesa/drivers/dri/i965/brw_context.c @@ -458,6 +458,7 @@ brw_initialize_context_constants(struct brw_context *brw) ctx-Const.NativeIntegers = true; ctx-Const.UniformBooleanTrue = 1; + ctx-Const.VertexID_is_zero_based = true; /* From the gen4 PRM, volume 4 page 127: * -- 2.0.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 03/13] mesa: Add SYSTEM_VALUE_BASE_VERTEX
From: Ian Romanick ian.d.roman...@intel.com
This system value represents the basevertex value passed to
glDrawElementsBaseVertex and related functions.
Signed-off-by: Ian Romanick ian.d.roman...@intel.com
---
src/mesa/main/mtypes.h | 15 ++-
src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 1 +
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index c603007..99037c6 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2084,7 +2084,12 @@ typedef enum
* gl_VertexID gets basevertex added in. This differs from DirectX where
* SV_VertexID does \b not get basevertex added in.
*
-* \sa SYSTEM_VALUE_VERTEX_ID_ZERO_BASE
+* \note
+* If all system values are available, \c SYSTEM_VALUE_VERTEX_ID will be
+* equal to \c SYSTEM_VALUE_VERTEX_ID_ZERO_BASE plus
+* \c SYSTEM_VALUE_BASE_VERTEX.
+*
+* \sa SYSTEM_VALUE_VERTEX_ID_ZERO_BASE, SYSTEM_VALUE_BASE_VERTEX
*/
SYSTEM_VALUE_VERTEX_ID,
@@ -2126,6 +2131,14 @@ typedef enum
* \sa SYSTEM_VALUE_VERTEX_ID, SYSTEM_VALUE_BASE_VERTEX
*/
SYSTEM_VALUE_VERTEX_ID_ZERO_BASE,
+
+ /**
+* Value of \c basevertex passed to \c glDrawElementsBaseVertex and similar
+* functions.
+*
+* \sa SYSTEM_VALUE_VERTEX_ID, SYSTEM_VALUE_VERTEX_ID_ZERO_BASE
+*/
+ SYSTEM_VALUE_BASE_VERTEX,
/*@}*/
/**
diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index 81f08cd..68c6b59 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -4172,6 +4172,7 @@ const unsigned _mesa_sysval_to_semantic[SYSTEM_VALUE_MAX]
= {
TGSI_SEMANTIC_VERTEXID,
TGSI_SEMANTIC_INSTANCEID,
0,
+ 0,
/* Geometry shader
*/
--
2.0.2
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 03/13] mesa: Add SYSTEM_VALUE_BASE_VERTEX
On Friday, August 08, 2014 12:31:07 AM Kenneth Graunke wrote: From: Ian Romanick ian.d.roman...@intel.com This system value represents the basevertex value passed to glDrawElementsBaseVertex and related functions. Signed-off-by: Ian Romanick ian.d.roman...@intel.com --- src/mesa/main/mtypes.h | 15 ++- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 1 + 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index c603007..99037c6 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -2084,7 +2084,12 @@ typedef enum * gl_VertexID gets basevertex added in. This differs from DirectX where * SV_VertexID does \b not get basevertex added in. * -* \sa SYSTEM_VALUE_VERTEX_ID_ZERO_BASE +* \note +* If all system values are available, \c SYSTEM_VALUE_VERTEX_ID will be +* equal to \c SYSTEM_VALUE_VERTEX_ID_ZERO_BASE plus +* \c SYSTEM_VALUE_BASE_VERTEX. +* +* \sa SYSTEM_VALUE_VERTEX_ID_ZERO_BASE, SYSTEM_VALUE_BASE_VERTEX */ SYSTEM_VALUE_VERTEX_ID, @@ -2126,6 +2131,14 @@ typedef enum * \sa SYSTEM_VALUE_VERTEX_ID, SYSTEM_VALUE_BASE_VERTEX */ SYSTEM_VALUE_VERTEX_ID_ZERO_BASE, + + /** +* Value of \c basevertex passed to \c glDrawElementsBaseVertex and similar +* functions. +* +* \sa SYSTEM_VALUE_VERTEX_ID, SYSTEM_VALUE_VERTEX_ID_ZERO_BASE +*/ + SYSTEM_VALUE_BASE_VERTEX, /*@}*/ Ian, It occurred to me that we're sort of abusing this system value in the i965 patches later in this series - we're using it to store gl_BaseVertexARB, but also using it to store the first parameter for glDrawArrays. I think in the glDrawArrays case, gl_BaseVertexARB is supposed to be 0. I'm not sure what the right thing to do is. --Ken signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Mesa (master): mesa/formats: Add layout and swizzle information
On Thursday, August 07, 2014 10:41:36 PM Roland Scheidegger wrote: Am 07.08.2014 20:25, schrieb Jason Ekstrand: Michel, On Thu, Aug 7, 2014 at 12:04 AM, Michel Dänzer mic...@daenzer.net mailto:mic...@daenzer.net wrote: On 07.08.2014 02:02, Jason Ekstrand wrote: Michael, Close, but no cigar. :) I'm sorry about that. I must have read too quickly. :-/ Could you please point me at the failing tests. spec/!OpenGL 1.1/depthstencil-default_fb-drawpixels-FLOAT-and-USHORT spec/!OpenGL 1.1/draw-pixels spec/!OpenGL 1.1/stencil-drawpixels spec/!OpenGL 1.4/copy-pixels spec/ARB_depth_buffer_float/fbo-depthstencil-GL_DEPTH32F_STENCIL8-drawpixels-FLOAT-and-USHORT spec/ARB_depth_buffer_float/fbo-stencil-GL_DEPTH32F_STENCIL8-drawpixels spec/EXT_framebuffer_object/fbo-stencil-GL_STENCIL_INDEX1-drawpixels spec/EXT_framebuffer_object/fbo-stencil-GL_STENCIL_INDEX16-drawpixels spec/EXT_framebuffer_object/fbo-stencil-GL_STENCIL_INDEX14-drawpixels spec/EXT_framebuffer_object/fbo-stencil-GL_STENCIL_INDEX8-drawpixels spec/EXT_packed_depth_stencil/fbo-depthstencil-GL_DEPTH24_STENCIL8-drawpixels-FLOAT-and-USHORT spec/EXT_packed_depth_stencil/fbo-stencil-GL_DEPTH24_STENCIL8-drawpixels (The total number of regressions is around 20 because some of these are run for several numbers of samples) I don't have a radeon, but I can run with llvmpipe or dri swrast and try to find the bug that way. At least the draw-pixels test indeed regressed with llvmpipe as well. The draw pixels regression on llvmpipe is different. The changes I made to texture upload included a subtle change in the way we handle signed input data. In older GL versions there were two formulas, one which mapped [-128, 127] to [-1, 1] and one which mapped [-127, 127] to [-1, 1]. The former formula was used when uploading a non-snorm texture from signed integer data or when doing operations such as DrawPixels and ReadPixels. In GL 4.3, this first formula is going away and we will only have the later formula. (The later formula has the advantage of mapping 0 to 0.) If we think it's needed, I can add code to the swizzle_and_convert function to be able to handle the legacy formula in those cases where older GL versions say that it's needed. Yeah the two different formulas in older GL versions are quite a pity. I'm not sure if we really need to honor the old formula, but I guess if binary drivers do we might be required to as well. The gallium util code though never did. Maybe should just make the tests more lenient... Roland I have a pretty strong preference to ditch the old formula entirely: 1. OpenGL will silently promote you to a 4.2 context if you ask for less than that, because it's deemed backwards compatible (even though it strictly isn't, due to things like this). Implementing both sets of rules means that GL4-class hardware will use one formula, and GL3-class hardware will use the other...on the same driver. That seems weird. 2. OpenGL ES 3.x strictly uses the new formula. 3. The new formula matches DirectX, so it's probably what ported applications (or a virtualization environment) would expect. 4. Intel hardware cannot honor the older GL formula (we'd have to do the conversion manually, which seems like a waste). My preference would be to make tests accept either formula. --Ken 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] [PATCH] i965: Support the allow_glsl_extension_directive_midshader option.
This adds support for Marek's new driconf parameter, which avoids totally white rendering in Unigine Valley (which attempts to enable the GL_ARB_sample_shading extension in an illegal place). Signed-off-by: Kenneth Graunke kenn...@whitecape.org --- src/mesa/drivers/dri/i965/brw_context.c | 3 +++ src/mesa/drivers/dri/i965/intel_screen.c | 1 + 2 files changed, 4 insertions(+) diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c index 52f2557..402d936 100644 --- a/src/mesa/drivers/dri/i965/brw_context.c +++ b/src/mesa/drivers/dri/i965/brw_context.c @@ -570,6 +570,9 @@ brw_process_driconf_options(struct brw_context *brw) ctx-Const.DisableGLSLLineContinuations = driQueryOptionb(options, disable_glsl_line_continuations); + + ctx-Const.AllowGLSLExtensionDirectiveMidShader = + driQueryOptionb(options, allow_glsl_extension_directive_midshader); } GLboolean diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c index 5ebf1d5..ea0fc58 100644 --- a/src/mesa/drivers/dri/i965/intel_screen.c +++ b/src/mesa/drivers/dri/i965/intel_screen.c @@ -84,6 +84,7 @@ DRI_CONF_BEGIN DRI_CONF_FORCE_GLSL_EXTENSIONS_WARN(false) DRI_CONF_DISABLE_GLSL_LINE_CONTINUATIONS(false) DRI_CONF_DISABLE_BLEND_FUNC_EXTENDED(false) + DRI_CONF_ALLOW_GLSL_EXTENSION_DIRECTIVE_MIDSHADER(false) DRI_CONF_OPT_BEGIN_B(shader_precompile, true) DRI_CONF_DESC(en, Perform code generation at shader link time.) -- 2.0.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: Drop USE_IEEE define.
On Wed, Aug 06, 2014 at 04:54:38PM -0700, Matt Turner wrote: I think OpenVMS was the only platform that Mesa ran on that used a non-IEEE representation for floats. We removed OpenVMS support a while back, and this should alleviate the need to continue updating the this-platform-uses-IEEE list. OpenVMS isn't the only thing that runs on a vax, this will likely break the software rasteriser on OpenBSD and NetBSD on vax. It seems Mesa isn't currently built on OpenBSD/vax but is on NetBSD. Though NetBSD are stuck with Mesa 7.x.x due to the DRM/KMS situation. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/5] i965: Add support for ARB_copy_image
Jason Ekstrand ja...@jlekstrand.net writes:
+ if (src_mt == dst_mt src_level == dst_level src_z == dst_z) {
+ /* If we are on the same miptree, same level, and same slice, then
+ * intel_miptree_map won't let us map it twice. We have to do a
+ * single map in read-write mode.
+ */
+
+ map_x1 = MIN2(src_x, dst_x);
+ map_y1 = MIN2(src_y, dst_y);
+ map_x2 = MAX2(src_x, dst_x) + src_width;
+ map_y2 = MAX2(src_y, dst_y) + src_height;
+
+ intel_miptree_map(brw, src_mt, src_level, src_z,
+map_x1, map_y1, map_x2 - map_x1, map_y2 - map_y1,
+GL_MAP_READ_BIT | GL_MAP_WRITE_BIT,
+(void **)mapped, src_stride);
+
+ dst_stride = src_stride;
+
+ /* Set the offsets here so we don't have to think about it later */
+ src_mapped = mapped + (src_y - map_y1) * src_stride +
+(src_x - map_x1) * cpp;
+ dst_mapped = mapped + (dst_y - map_y1) * dst_stride +
+(dst_x - map_x1) * cpp;
This needs to take into account the block size of the format or it will
offset to the wrong position. I guess that is quite important as this
code path will only be used for compressed formats.
The commit message still mentions the maximum stride which is no longer
valid.
Regards,
- Neil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] egl_dri2: fix EXT_image_dma_buf_import fds
From: Pekka Paalanen pekka.paala...@collabora.co.uk
The EGL_EXT_image_dma_buf_import specification was revised (according to
its revision history) on Dec 5th, 2013, for EGL to not take ownership of
the file descriptors.
Do not close the file descriptors passed in to eglCreateImageKHR with
EGL_LINUX_DMA_BUF_EXT target.
It is assumed, that the drivers, which ultimately process the file
descriptors, do not close or modify them in any way either. This avoids
the need to dup(), as it seems we would only need to just close the
dup'd file descriptors right after.
Signed-off-by: Pekka Paalanen pekka.paala...@collabora.co.uk
---
Hi,
the corresponding Piglit fix has already been sent to the piglit mailing
list. Both this and that need to be applied to not regress Mesa' piglit run
by one test (ext_image_dma_buf_import-ownership_transfer).
This patch fixes my test case on heavily modified Weston.
Thanks,
pq
---
src/egl/drivers/dri2/egl_dri2.c | 37 ++---
1 file changed, 6 insertions(+), 31 deletions(-)
diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 5602ec3..cd85fd3 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -1678,36 +1678,13 @@ dri2_check_dma_buf_format(const _EGLImageAttribs *attrs)
/**
* The spec says:
*
- * If eglCreateImageKHR is successful for a EGL_LINUX_DMA_BUF_EXT target,
- * the EGL takes ownership of the file descriptor and is responsible for
- * closing it, which it may do at any time while the EGLDisplay is
- * initialized.
+ * If eglCreateImageKHR is successful for a EGL_LINUX_DMA_BUF_EXT target, the
+ * EGL will take a reference to the dma_buf(s) which it will release at any
+ * time while the EGLDisplay is initialized. It is the responsibility of the
+ * application to close the dma_buf file descriptors.
+ *
+ * Therefore we must never close or otherwise modify the file descriptors.
*/
-static void
-dri2_take_dma_buf_ownership(const int *fds, unsigned num_fds)
-{
- int already_closed[num_fds];
- unsigned num_closed = 0;
- unsigned i, j;
-
- for (i = 0; i num_fds; ++i) {
- /**
- * The same file descriptor can be referenced multiple times in case more
- * than one plane is found in the same buffer, just with a different
- * offset.
- */
- for (j = 0; j num_closed; ++j) {
- if (already_closed[j] == fds[i])
-break;
- }
-
- if (j == num_closed) {
- close(fds[i]);
- already_closed[num_closed++] = fds[i];
- }
- }
-}
-
static _EGLImage *
dri2_create_image_dma_buf(_EGLDisplay *disp, _EGLContext *ctx,
EGLClientBuffer buffer, const EGLint *attr_list)
@@ -1770,8 +1747,6 @@ dri2_create_image_dma_buf(_EGLDisplay *disp, _EGLContext
*ctx,
return EGL_NO_IMAGE_KHR;
res = dri2_create_image_from_dri(disp, dri_image);
- if (res)
- dri2_take_dma_buf_ownership(fds, num_fds);
return res;
}
--
1.8.5.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 03/13] mesa: Add SYSTEM_VALUE_BASE_VERTEX
Am 08.08.2014 09:37, schrieb Kenneth Graunke: On Friday, August 08, 2014 12:31:07 AM Kenneth Graunke wrote: From: Ian Romanick ian.d.roman...@intel.com This system value represents the basevertex value passed to glDrawElementsBaseVertex and related functions. Signed-off-by: Ian Romanick ian.d.roman...@intel.com --- src/mesa/main/mtypes.h | 15 ++- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 1 + 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index c603007..99037c6 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -2084,7 +2084,12 @@ typedef enum * gl_VertexID gets basevertex added in. This differs from DirectX where * SV_VertexID does \b not get basevertex added in. * -* \sa SYSTEM_VALUE_VERTEX_ID_ZERO_BASE +* \note +* If all system values are available, \c SYSTEM_VALUE_VERTEX_ID will be +* equal to \c SYSTEM_VALUE_VERTEX_ID_ZERO_BASE plus +* \c SYSTEM_VALUE_BASE_VERTEX. +* +* \sa SYSTEM_VALUE_VERTEX_ID_ZERO_BASE, SYSTEM_VALUE_BASE_VERTEX */ SYSTEM_VALUE_VERTEX_ID, @@ -2126,6 +2131,14 @@ typedef enum * \sa SYSTEM_VALUE_VERTEX_ID, SYSTEM_VALUE_BASE_VERTEX */ SYSTEM_VALUE_VERTEX_ID_ZERO_BASE, + + /** +* Value of \c basevertex passed to \c glDrawElementsBaseVertex and similar +* functions. +* +* \sa SYSTEM_VALUE_VERTEX_ID, SYSTEM_VALUE_VERTEX_ID_ZERO_BASE +*/ + SYSTEM_VALUE_BASE_VERTEX, /*@}*/ Ian, It occurred to me that we're sort of abusing this system value in the i965 patches later in this series - we're using it to store gl_BaseVertexARB, but also using it to store the first parameter for glDrawArrays. I think in the glDrawArrays case, gl_BaseVertexARB is supposed to be 0. I'm not sure what the right thing to do is. --Ken Hmm are you sure of that? I guess you're drawing that from gl_BaseVertexARB holds the integer value passed to the baseVertex parameter to the command that resulted in the current shader invocation. In the case where the command has no baseVertex parameter, the value of gl_BaseVertexARB is zero. And non-indexed draw commands have no baseVertex parameter, they have a first parameter instead. Pretty much the same thing however, just a different name. I think it would make way more sense if gl_BaseVertexARB would also account for the first parameter of non-indexed draw, so maybe it's just not cleverly worded. ARB_shader_draw_parameters specifically mentions this stuff is useful for translating from other APIs, and if it doesn't include non-indexed draw's first parameter, it can't do that. So I would suggest the docs should be clarified instead. Roland ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/3] cl workdim v2
Tom Stellard t...@stellard.net writes: On Thu, Aug 07, 2014 at 04:02:40PM +0300, Francisco Jerez wrote: Jan Vesely jan.ves...@rutgers.edu writes: This respin includes Francisco's approach of providing implicit in the arg vector passed from clover, and Tom's idea of appending implicit args after the kernel args. Hmmm... Maybe it would make sense to add some sort of versioning (e.g. as part of the target triple) to the binary interface between clover and the kernel instead, so we can handle this sort of non-backwards compatible changes and the compiler back-end and libclc have some way to find out whether some specific feature is available and e.g. some specific extension should be enabled. I was thinking the way to do this would be to use calling conventions on the kernel functions to specify which binary interface to use. However, I don't want to change the binary interface right now, because it is still missing a lot of things, and I don't want to have to change it every time we add something new. I think we should keep the current interface of: Offset | Data -|-- 0: Kernel Arguments sizeof(Kernel Inputs): work_dim sizeof(Kernel Inputs) + 4: ... We can always revisit this once clover is more mature and we think we have a binary interface that won't change. Although, personally I prefer adding implicit inputs to the end of the kernel arguments rather than having of them somewhere else. Right, that sounds reasonable to me. Thanks. -Tom I assumed it's not safe to modify exec.input, so the input vector is copied before appending work dim. Why wouldn't it be safe? You just need to make sure they're appended before the compute state is created. Passes get-work-dim piglit on turks without any regression, I have not tested SI as I don't have the hw. jan Jan Vesely (3): gallium: Pass input data size to launch_grid clover: Add work dimension implicit param to input r600,radeonsi: Copy implicit args provided by clover src/gallium/drivers/ilo/ilo_gpgpu.c | 2 +- src/gallium/drivers/nouveau/nvc0/nvc0_compute.c | 2 +- src/gallium/drivers/nouveau/nvc0/nvc0_context.h | 4 +- src/gallium/drivers/nouveau/nvc0/nve4_compute.c | 2 +- src/gallium/drivers/r600/evergreen_compute.c | 14 +- src/gallium/drivers/r600/evergreen_compute.h | 1 - src/gallium/drivers/radeonsi/si_compute.c | 6 +- src/gallium/include/pipe/p_context.h | 2 +- src/gallium/state_trackers/clover/core/kernel.cpp | 162 -- src/gallium/tests/trivial/compute.c | 40 +++--- 10 files changed, 122 insertions(+), 113 deletions(-) -- 1.9.3 pgpdo7ljkXl34.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 01/13] mesa: Document SYSTEM_VALUE_VERTEX_ID and SYSTEM_VALUE_INSTANCE_ID
The mesa parts of the series all look good to me.
We definitely want something like that in gallium too (draw fails the
vertex id tests sort of on purpose right now because we needed d3d10
behavior).
Oh and sort of off-topic but since you're familiar with it do you know
why the gl_PrimitiveIn of the geometry shader isn't a system value
whereas all the vertex_id and friends are?
Roland
Am 08.08.2014 09:31, schrieb Kenneth Graunke:
From: Ian Romanick ian.d.roman...@intel.com
v2: Additions to the documentation for SYSTEM_VALUE_VERTEX_ID. Quote
the GL_ARB_shader_draw_parameters spec and mention DirectX SV_VertexID.
Signed-off-by: Ian Romanick ian.d.roman...@intel.com
Reviewed-by: Kenneth Graunke kenn...@whitecape.org
---
src/mesa/main/mtypes.h | 57
++
1 file changed, 57 insertions(+)
This series is available as the 'basevertex-v9' branch of ~kwg/mesa
(not ~idr/mesa). Ken tested this series against Piglit on Haswell and
Broadwell, but did not test earlier hardware, nor run the ES3 tests.
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index ff130da..207be0a 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2055,7 +2055,64 @@ typedef enum
* \name Vertex shader system values
*/
/*@{*/
+ /**
+* OpenGL-style vertex ID.
+*
+* Section 2.11.7 (Shader Execution), subsection Shader Inputs, of the
+* OpenGL 3.3 core profile spec says:
+*
+* gl_VertexID holds the integer index i implicitly passed by
+* DrawArrays or one of the other drawing commands defined in section
+* 2.8.3.
+*
+* Section 2.8.3 (Drawing Commands) of the same spec says:
+*
+* The commandsare equivalent to the commands with the same base
+* name (without the BaseVertex suffix), except that the ith element
+* transferred by the corresponding draw call will be taken from
+* element indices[i] + basevertex of each enabled array.
+*
+* Additionally, the overview in the GL_ARB_shader_draw_parameters spec
+* says:
+*
+* In unextended GL, vertex shaders have inputs named gl_VertexID and
+* gl_InstanceID, which contain, respectively the index of the vertex
+* and instance. The value of gl_VertexID is the implicitly passed
+* index of the vertex being processed, which includes the value of
+* baseVertex, for those commands that accept it.
+*
+* gl_VertexID gets basevertex added in. This differs from DirectX where
+* SV_VertexID does \b not get basevertex added in.
+*/
SYSTEM_VALUE_VERTEX_ID,
+
+ /**
+* Instanced ID as supplied to gl_InstanceID
+*
+* Values assigned to gl_InstanceID always begin with zero, regardless of
+* the value of baseinstance.
+*
+* Section 11.1.3.9 (Shader Inputs) of the OpenGL 4.4 core profile spec
+* says:
+*
+* gl_InstanceID holds the integer instance number of the current
+* primitive in an instanced draw call (see section 10.5).
+*
+* Through a big chain of pseudocode, section 10.5 describes that
+* baseinstance is not counted by gl_InstanceID. In that section, notice
+*
+* If an enabled vertex attribute array is instanced (it has a
+* non-zero divisor as specified by VertexAttribDivisor), the element
+* index that is transferred to the GL, for all vertices, is given by
+*
+* floor(instance/divisor) + baseinstance
+*
+* If an array corresponding to an attribute required by a vertex
+* shader is not enabled, then the corresponding element is taken from
+* the current attribute state (see section 10.2).
+*
+* Note that baseinstance is \b not included in the value of instance.
+*/
SYSTEM_VALUE_INSTANCE_ID,
/*@}*/
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl_dri2: fix EXT_image_dma_buf_import fds
On Fri, 8 Aug 2014 17:28:59 +0300
Pekka Paalanen ppaala...@gmail.com wrote:
From: Pekka Paalanen pekka.paala...@collabora.co.uk
The EGL_EXT_image_dma_buf_import specification was revised (according to
its revision history) on Dec 5th, 2013, for EGL to not take ownership of
the file descriptors.
Do not close the file descriptors passed in to eglCreateImageKHR with
EGL_LINUX_DMA_BUF_EXT target.
It is assumed, that the drivers, which ultimately process the file
descriptors, do not close or modify them in any way either. This avoids
the need to dup(), as it seems we would only need to just close the
dup'd file descriptors right after.
Signed-off-by: Pekka Paalanen pekka.paala...@collabora.co.uk
---
Hi,
the corresponding Piglit fix has already been sent to the piglit mailing
list. Both this and that need to be applied to not regress Mesa' piglit run
by one test (ext_image_dma_buf_import-ownership_transfer).
This patch fixes my test case on heavily modified Weston.
Sorry, I forgot. This patch would apply also to the 10.0, 10.1, and
10.2 stable branches. Should it be a candidate there?
Thanks,
pq
---
src/egl/drivers/dri2/egl_dri2.c | 37 ++---
1 file changed, 6 insertions(+), 31 deletions(-)
diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 5602ec3..cd85fd3 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -1678,36 +1678,13 @@ dri2_check_dma_buf_format(const _EGLImageAttribs
*attrs)
/**
* The spec says:
*
- * If eglCreateImageKHR is successful for a EGL_LINUX_DMA_BUF_EXT target,
- * the EGL takes ownership of the file descriptor and is responsible for
- * closing it, which it may do at any time while the EGLDisplay is
- * initialized.
+ * If eglCreateImageKHR is successful for a EGL_LINUX_DMA_BUF_EXT target,
the
+ * EGL will take a reference to the dma_buf(s) which it will release at any
+ * time while the EGLDisplay is initialized. It is the responsibility of the
+ * application to close the dma_buf file descriptors.
+ *
+ * Therefore we must never close or otherwise modify the file descriptors.
*/
-static void
-dri2_take_dma_buf_ownership(const int *fds, unsigned num_fds)
-{
- int already_closed[num_fds];
- unsigned num_closed = 0;
- unsigned i, j;
-
- for (i = 0; i num_fds; ++i) {
- /**
- * The same file descriptor can be referenced multiple times in case
more
- * than one plane is found in the same buffer, just with a different
- * offset.
- */
- for (j = 0; j num_closed; ++j) {
- if (already_closed[j] == fds[i])
-break;
- }
-
- if (j == num_closed) {
- close(fds[i]);
- already_closed[num_closed++] = fds[i];
- }
- }
-}
-
static _EGLImage *
dri2_create_image_dma_buf(_EGLDisplay *disp, _EGLContext *ctx,
EGLClientBuffer buffer, const EGLint *attr_list)
@@ -1770,8 +1747,6 @@ dri2_create_image_dma_buf(_EGLDisplay *disp,
_EGLContext *ctx,
return EGL_NO_IMAGE_KHR;
res = dri2_create_image_from_dri(disp, dri_image);
- if (res)
- dri2_take_dma_buf_ownership(fds, num_fds);
return res;
}
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] clover: Add support for CL_MAP_WRITE_INVALIDATE_REGION
Bruno Jiménez brunoji...@gmail.com writes:
OpenCL 1.2 CL_MAP_WRITE_INVALIDATE_REGION sounds a lot like
PIPE_TRANSFER_DISCARD_RANGE:
From OpenCL 1.2 spec:
The contents of the region being mapped are to be discarded.
From p_defines.h:
Discards the memory within the mapped region.
v2: Move the code for validating flags to the front-end as
suggested by Francisco Jerez
Looks good to me, pushed as ec73778f1fd6e14623422d62605fc69dc8fb7aa4.
---
src/gallium/state_trackers/clover/api/transfer.cpp | 13 +
src/gallium/state_trackers/clover/core/resource.cpp | 2 ++
2 files changed, 15 insertions(+)
diff --git a/src/gallium/state_trackers/clover/api/transfer.cpp
b/src/gallium/state_trackers/clover/api/transfer.cpp
index 07d8a73..37c3074 100644
--- a/src/gallium/state_trackers/clover/api/transfer.cpp
+++ b/src/gallium/state_trackers/clover/api/transfer.cpp
@@ -168,6 +168,17 @@ namespace {
}
///
+ /// Checks that the mapping flags are correct
+ ///
+ void
+ validate_flags(const cl_map_flags flags) {
+ if (((flags CL_MAP_WRITE) || (flags CL_MAP_READ))
+ (flags CL_MAP_WRITE_INVALIDATE_REGION))
+ throw error(CL_INVALID_VALUE);
+ }
+
+
+ ///
/// Class that encapsulates the task of mapping an object of type
/// \a T. The return value of get() should be implicitly
/// convertible to \a void *.
@@ -629,6 +640,7 @@ clEnqueueMapBuffer(cl_command_queue d_q, cl_mem d_mem,
cl_bool blocking,
validate_common(q, deps);
validate_object(q, mem, obj_origin, obj_pitch, region);
+ validate_flags(flags);
void *map = mem.resource(q).add_map(q, flags, blocking, obj_origin,
region);
@@ -656,6 +668,7 @@ clEnqueueMapImage(cl_command_queue d_q, cl_mem d_mem,
cl_bool blocking,
validate_common(q, deps);
validate_object(q, img, origin, region);
+ validate_flags(flags);
void *map = img.resource(q).add_map(q, flags, blocking, origin, region);
diff --git a/src/gallium/state_trackers/clover/core/resource.cpp
b/src/gallium/state_trackers/clover/core/resource.cpp
index 7b8a40a..34c0cd5 100644
--- a/src/gallium/state_trackers/clover/core/resource.cpp
+++ b/src/gallium/state_trackers/clover/core/resource.cpp
@@ -174,6 +174,8 @@ mapping::mapping(command_queue q, resource r,
pctx(q.pipe) {
unsigned usage = ((flags CL_MAP_WRITE ? PIPE_TRANSFER_WRITE : 0 ) |
(flags CL_MAP_READ ? PIPE_TRANSFER_READ : 0 ) |
+ (flags CL_MAP_WRITE_INVALIDATE_REGION ?
+PIPE_TRANSFER_DISCARD_RANGE : 0) |
(!blocking ? PIPE_TRANSFER_UNSYNCHRONIZED : 0));
p = pctx-transfer_map(pctx, r.pipe, 0, usage,
--
2.0.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
pgp_KFzAkozfC.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/7] radeonsi/compute: Call si_pm4_free_state() after emitting compute state
This will decrement the reference count for buffers referenced in the command stream will prevent us from leaking them. CC: 10.2 mesa-sta...@lists.freedesktop.org --- src/gallium/drivers/radeonsi/si_compute.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/drivers/radeonsi/si_compute.c b/src/gallium/drivers/radeonsi/si_compute.c index 482d475..e8fc8eb 100644 --- a/src/gallium/drivers/radeonsi/si_compute.c +++ b/src/gallium/drivers/radeonsi/si_compute.c @@ -374,8 +374,8 @@ static void si_launch_grid( } #endif - FREE(pm4); FREE(kernel_args); + si_pm4_free_state(sctx, pm4, ~0); } -- 1.8.1.5 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 7/7] clover: Flush the command queue in clReleaseCommandQueue()
This is required by the spec.
CC: 10.2 mesa-sta...@lists.freedesktop.org
---
src/gallium/state_trackers/clover/api/queue.cpp | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/gallium/state_trackers/clover/api/queue.cpp
b/src/gallium/state_trackers/clover/api/queue.cpp
index a136018..06a2863 100644
--- a/src/gallium/state_trackers/clover/api/queue.cpp
+++ b/src/gallium/state_trackers/clover/api/queue.cpp
@@ -58,7 +58,11 @@ clRetainCommandQueue(cl_command_queue d_q) try {
CLOVER_API cl_int
clReleaseCommandQueue(cl_command_queue d_q) try {
- if (obj(d_q).release())
+ auto q = obj(d_q);
+
+ q.flush();
+
+ if (q.release())
delete pobj(d_q);
return CL_SUCCESS;
--
1.8.1.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 6/7] radeonsi/compute: Stop leaking the input buffer
We were leaking the input buffer used for kernel arguments and since
we were allocating it using si_upload_const_buffer() we were leaking
1 MB per kernel invocation.
CC: 10.2 mesa-sta...@lists.freedesktop.org
---
src/gallium/drivers/radeonsi/si_compute.c | 22 ++
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/src/gallium/drivers/radeonsi/si_compute.c
b/src/gallium/drivers/radeonsi/si_compute.c
index dff5ddd..01aa0c6 100644
--- a/src/gallium/drivers/radeonsi/si_compute.c
+++ b/src/gallium/drivers/radeonsi/si_compute.c
@@ -48,6 +48,7 @@ struct si_pipe_compute {
struct si_pipe_shader *kernels;
unsigned num_user_sgprs;
+ struct r600_resource *input_buffer;
struct pipe_resource *global_buffers[MAX_GLOBAL_BUFFERS];
LLVMContextRef llvm_ctx;
@@ -85,6 +86,9 @@ static void *si_create_compute_state(
LLVMDisposeModule(mod);
}
+ program-input_buffer = si_resource_create_custom(sctx-b.b.screen,
+ PIPE_USAGE_IMMUTABLE, program-input_size);
+
return program;
}
@@ -167,7 +171,7 @@ static void si_launch_grid(
struct si_context *sctx = (struct si_context*)ctx;
struct si_pipe_compute *program = sctx-cs_shader_state.program;
struct si_pm4_state *pm4 = CALLOC_STRUCT(si_pm4_state);
- struct r600_resource *kernel_args_buffer = NULL;
+ struct r600_resource *input_buffer = program-input_buffer;
unsigned kernel_args_size;
unsigned num_work_size_bytes = 36;
uint32_t kernel_args_offset = 0;
@@ -199,7 +203,8 @@ static void si_launch_grid(
/* The extra num_work_size_bytes are for work group / work item size
information */
kernel_args_size = program-input_size + num_work_size_bytes + 8 /* For
scratch va */;
- kernel_args = MALLOC(kernel_args_size);
+ kernel_args = sctx-b.ws-buffer_map(input_buffer-cs_buf,
+ sctx-b.rings.gfx.cs, PIPE_TRANSFER_WRITE);
for (i = 0; i 3; i++) {
kernel_args[i] = grid_layout[i];
kernel_args[i + 3] = grid_layout[i] * block_layout[i];
@@ -236,13 +241,13 @@ static void si_launch_grid(
kernel_args[i]);
}
- si_upload_const_buffer(sctx, kernel_args_buffer, (uint8_t*)kernel_args,
- kernel_args_size, kernel_args_offset);
- kernel_args_va = r600_resource_va(ctx-screen,
- (struct pipe_resource*)kernel_args_buffer);
+ sctx-b.ws-buffer_unmap(input_buffer-cs_buf);
+
+ kernel_args_va = r600_resource_va(ctx-screen, input_buffer-b.b);
kernel_args_va += kernel_args_offset;
- si_pm4_add_bo(pm4, kernel_args_buffer, RADEON_USAGE_READ,
RADEON_PRIO_SHADER_DATA);
+ si_pm4_add_bo(pm4, input_buffer, RADEON_USAGE_READ,
+ RADEON_PRIO_SHADER_DATA);
si_pm4_set_reg(pm4, R_00B900_COMPUTE_USER_DATA_0, kernel_args_va);
si_pm4_set_reg(pm4, R_00B900_COMPUTE_USER_DATA_0 + 4,
S_008F04_BASE_ADDRESS_HI (kernel_args_va 32) | S_008F04_STRIDE(0));
@@ -374,7 +379,6 @@ static void si_launch_grid(
}
#endif
- FREE(kernel_args);
si_pm4_free_state(sctx, pm4, ~0);
}
@@ -398,6 +402,8 @@ static void si_delete_compute_state(struct pipe_context
*ctx, void* state){
if (program-llvm_ctx){
LLVMContextDispose(program-llvm_ctx);
}
+ pipe_resource_reference(
+ (struct pipe_resource **)program-input_buffer, NULL);
//And then free the program itself.
FREE(program);
--
1.8.1.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/7] radeon/compute: Fix reported values for MAX_GLOBAL_SIZE and MAX_MEM_ALLOC_SIZE
There is a hard limit in older kernels of 256 MB for buffer allocations,
so report this value as MAX_MEM_ALLOC_SIZE and adjust MAX_GLOBAL_SIZE
to statisfy requirements of OpenCL.
CC: 10.2 mesa-sta...@lists.freedesktop.org
---
src/gallium/drivers/radeon/r600_pipe_common.c | 32 ---
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/src/gallium/drivers/radeon/r600_pipe_common.c
b/src/gallium/drivers/radeon/r600_pipe_common.c
index 3476021..0886b02 100644
--- a/src/gallium/drivers/radeon/r600_pipe_common.c
+++ b/src/gallium/drivers/radeon/r600_pipe_common.c
@@ -474,13 +474,21 @@ static int r600_get_compute_param(struct pipe_screen
*screen,
case PIPE_COMPUTE_CAP_MAX_GLOBAL_SIZE:
if (ret) {
uint64_t *max_global_size = ret;
- /* XXX: This is what the proprietary driver reports, we
-* may want to use a different value. */
- /* XXX: Not sure what to put here for SI. */
- if (rscreen-chip_class = SI)
- *max_global_size = 20;
- else
- *max_global_size = 201326592;
+ uint64_t max_mem_alloc_size;
+
+ r600_get_compute_param(screen,
+ PIPE_COMPUTE_CAP_MAX_MEM_ALLOC_SIZE,
+ max_mem_alloc_size);
+
+ /* In OpenCL, the MAX_MEM_ALLOC_SIZE must be at least
+* 1/4 of the MAX_GLOBAL_SIZE. Since the
+* MAX_MEM_ALLOC_SIZE is fixed for older kernels,
+* make sure we never report more than
+* 4 * MAX_MEM_ALLOC_SIZE.
+*/
+ *max_global_size = MIN2(4 * max_mem_alloc_size,
+ rscreen-info.gart_size +
+ rscreen-info.vram_size);
}
return sizeof(uint64_t);
@@ -504,13 +512,11 @@ static int r600_get_compute_param(struct pipe_screen
*screen,
if (ret) {
uint64_t max_global_size;
uint64_t *max_mem_alloc_size = ret;
- r600_get_compute_param(screen,
PIPE_COMPUTE_CAP_MAX_GLOBAL_SIZE, max_global_size);
- /* OpenCL requres this value be at least
-* max(MAX_GLOBAL_SIZE / 4, 128 * 1024 *1024)
-* I'm really not sure what value to report here, but
-* MAX_GLOBAL_SIZE / 4 seems resonable.
+
+ /* XXX: The limit in older kernels is 256 MB. We
+* should add a query here for newer kernels.
*/
- *max_mem_alloc_size = max_global_size / 4;
+ *max_mem_alloc_size = 256 * 1024 * 1024;
}
return sizeof(uint64_t);
--
1.8.1.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 5/7] radeonsi/compute: Whitespace fixes
CC: 10.2 mesa-sta...@lists.freedesktop.org
---
src/gallium/drivers/radeonsi/si_compute.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/gallium/drivers/radeonsi/si_compute.c
b/src/gallium/drivers/radeonsi/si_compute.c
index e8fc8eb..dff5ddd 100644
--- a/src/gallium/drivers/radeonsi/si_compute.c
+++ b/src/gallium/drivers/radeonsi/si_compute.c
@@ -48,7 +48,7 @@ struct si_pipe_compute {
struct si_pipe_shader *kernels;
unsigned num_user_sgprs;
-struct pipe_resource *global_buffers[MAX_GLOBAL_BUFFERS];
+ struct pipe_resource *global_buffers[MAX_GLOBAL_BUFFERS];
LLVMContextRef llvm_ctx;
};
@@ -392,7 +392,6 @@ static void si_delete_compute_state(struct pipe_context
*ctx, void* state){
si_pipe_shader_destroy(ctx,
program-kernels[i]);
}
}
-
FREE(program-kernels);
}
--
1.8.1.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/7] radeon/compute: Report a value for PIPE_SHADER_CAP_MAX_CONST_BUFFER_SIZE
CC: 10.2 mesa-sta...@lists.freedesktop.org
---
src/gallium/drivers/r600/r600_pipe.c | 11 ++-
src/gallium/drivers/radeonsi/si_pipe.c | 7 +++
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/src/gallium/drivers/r600/r600_pipe.c
b/src/gallium/drivers/r600/r600_pipe.c
index a08e70e..7ace671 100644
--- a/src/gallium/drivers/r600/r600_pipe.c
+++ b/src/gallium/drivers/r600/r600_pipe.c
@@ -421,7 +421,16 @@ static int r600_get_shader_param(struct pipe_screen*
pscreen, unsigned shader, e
/* XXX Isn't this equal to TEMPS? */
return 1; /* Max native address registers */
case PIPE_SHADER_CAP_MAX_CONST_BUFFER_SIZE:
- return R600_MAX_CONST_BUFFER_SIZE;
+ if (shader == PIPE_SHADER_COMPUTE) {
+ uint64_t max_const_buffer_size;
+ pscreen-get_compute_param(pscreen,
+ PIPE_COMPUTE_CAP_MAX_MEM_ALLOC_SIZE,
+ max_const_buffer_size);
+ return max_const_buffer_size;
+
+ } else {
+ return R600_MAX_CONST_BUFFER_SIZE;
+ }
case PIPE_SHADER_CAP_MAX_CONST_BUFFERS:
return R600_MAX_USER_CONST_BUFFERS;
case PIPE_SHADER_CAP_MAX_PREDS:
diff --git a/src/gallium/drivers/radeonsi/si_pipe.c
b/src/gallium/drivers/radeonsi/si_pipe.c
index 635b37d..791838f 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.c
+++ b/src/gallium/drivers/radeonsi/si_pipe.c
@@ -327,6 +327,13 @@ static int si_get_shader_param(struct pipe_screen*
pscreen, unsigned shader, enu
case PIPE_SHADER_CAP_DOUBLES:
return 0; /* XXX: Enable doubles once the compiler can
handle them. */
+ case PIPE_SHADER_CAP_MAX_CONST_BUFFER_SIZE: {
+ uint64_t max_const_buffer_size;
+ pscreen-get_compute_param(pscreen,
+ PIPE_COMPUTE_CAP_MAX_MEM_ALLOC_SIZE,
+ max_const_buffer_size);
+ return max_const_buffer_size;
+ }
default:
return 0;
}
--
1.8.1.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/7] radeonsi/compute: Update reference counts for buffers in si_set_global_binding()
CC: 10.2 mesa-sta...@lists.freedesktop.org
---
src/gallium/drivers/radeonsi/si_compute.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/gallium/drivers/radeonsi/si_compute.c
b/src/gallium/drivers/radeonsi/si_compute.c
index 42e4fec..482d475 100644
--- a/src/gallium/drivers/radeonsi/si_compute.c
+++ b/src/gallium/drivers/radeonsi/si_compute.c
@@ -105,7 +105,7 @@ static void si_set_global_binding(
if (!resources) {
for (i = first; i first + n; i++) {
- program-global_buffers[i] = NULL;
+ pipe_resource_reference(program-global_buffers[i],
NULL);
}
return;
}
@@ -113,7 +113,7 @@ static void si_set_global_binding(
for (i = first; i first + n; i++) {
uint64_t va;
uint32_t offset;
- program-global_buffers[i] = resources[i];
+ pipe_resource_reference(program-global_buffers[i],
resources[i]);
va = r600_resource_va(ctx-screen, resources[i]);
offset = util_le32_to_cpu(*handles[i]);
va += offset;
--
1.8.1.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] radeonsi/compute: Memory usage fixes
Hi, This series contains fixes for applications which allocate large amounts of memory. The first two patches fix the values reported for PIPE_COMPUTE_CAP_MAX_GLOBAL_SIZE, PIPE_COMPUTE_CAP_MAX_MEM_ALLOC_SIZE, and PIPE_SHADER_CAP_MAX_CONST_BUFFER_SIZE so that applications don't allocate more memory than is available. The next five patches eliminate some GPU buffer leaks which should fix long running applications that launch a lot of kernels. Please Review. Thanks, Tom ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Support the allow_glsl_extension_directive_midshader option.
Reviewed-by: Matt Turner matts...@gmail.com ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 7/7] clover: Flush the command queue in clReleaseCommandQueue()
Tom Stellard thomas.stell...@amd.com writes:
This is required by the spec.
CC: 10.2 mesa-sta...@lists.freedesktop.org
---
src/gallium/state_trackers/clover/api/queue.cpp | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/gallium/state_trackers/clover/api/queue.cpp
b/src/gallium/state_trackers/clover/api/queue.cpp
index a136018..06a2863 100644
--- a/src/gallium/state_trackers/clover/api/queue.cpp
+++ b/src/gallium/state_trackers/clover/api/queue.cpp
@@ -58,7 +58,11 @@ clRetainCommandQueue(cl_command_queue d_q) try {
CLOVER_API cl_int
clReleaseCommandQueue(cl_command_queue d_q) try {
- if (obj(d_q).release())
+ auto q = obj(d_q);
+
+ q.flush();
+
+ if (q.release())
delete pobj(d_q);
return CL_SUCCESS;
For this patch:
Reviewed-by: Francisco Jerez curroje...@riseup.net
Thanks.
--
1.8.1.5
pgp5VPWS2e_pg.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] clover: Add support for CL_MAP_WRITE_INVALIDATE_REGION
On Fri, 2014-08-08 at 18:10 +0300, Francisco Jerez wrote:
Bruno Jiménez brunoji...@gmail.com writes:
OpenCL 1.2 CL_MAP_WRITE_INVALIDATE_REGION sounds a lot like
PIPE_TRANSFER_DISCARD_RANGE:
From OpenCL 1.2 spec:
The contents of the region being mapped are to be discarded.
From p_defines.h:
Discards the memory within the mapped region.
v2: Move the code for validating flags to the front-end as
suggested by Francisco Jerez
Looks good to me, pushed as ec73778f1fd6e14623422d62605fc69dc8fb7aa4.
Thanks!
---
src/gallium/state_trackers/clover/api/transfer.cpp | 13 +
src/gallium/state_trackers/clover/core/resource.cpp | 2 ++
2 files changed, 15 insertions(+)
diff --git a/src/gallium/state_trackers/clover/api/transfer.cpp
b/src/gallium/state_trackers/clover/api/transfer.cpp
index 07d8a73..37c3074 100644
--- a/src/gallium/state_trackers/clover/api/transfer.cpp
+++ b/src/gallium/state_trackers/clover/api/transfer.cpp
@@ -168,6 +168,17 @@ namespace {
}
///
+ /// Checks that the mapping flags are correct
+ ///
+ void
+ validate_flags(const cl_map_flags flags) {
+ if (((flags CL_MAP_WRITE) || (flags CL_MAP_READ))
+ (flags CL_MAP_WRITE_INVALIDATE_REGION))
+ throw error(CL_INVALID_VALUE);
+ }
+
+
+ ///
/// Class that encapsulates the task of mapping an object of type
/// \a T. The return value of get() should be implicitly
/// convertible to \a void *.
@@ -629,6 +640,7 @@ clEnqueueMapBuffer(cl_command_queue d_q, cl_mem d_mem,
cl_bool blocking,
validate_common(q, deps);
validate_object(q, mem, obj_origin, obj_pitch, region);
+ validate_flags(flags);
void *map = mem.resource(q).add_map(q, flags, blocking, obj_origin,
region);
@@ -656,6 +668,7 @@ clEnqueueMapImage(cl_command_queue d_q, cl_mem d_mem,
cl_bool blocking,
validate_common(q, deps);
validate_object(q, img, origin, region);
+ validate_flags(flags);
void *map = img.resource(q).add_map(q, flags, blocking, origin, region);
diff --git a/src/gallium/state_trackers/clover/core/resource.cpp
b/src/gallium/state_trackers/clover/core/resource.cpp
index 7b8a40a..34c0cd5 100644
--- a/src/gallium/state_trackers/clover/core/resource.cpp
+++ b/src/gallium/state_trackers/clover/core/resource.cpp
@@ -174,6 +174,8 @@ mapping::mapping(command_queue q, resource r,
pctx(q.pipe) {
unsigned usage = ((flags CL_MAP_WRITE ? PIPE_TRANSFER_WRITE : 0 ) |
(flags CL_MAP_READ ? PIPE_TRANSFER_READ : 0 ) |
+ (flags CL_MAP_WRITE_INVALIDATE_REGION ?
+PIPE_TRANSFER_DISCARD_RANGE : 0) |
(!blocking ? PIPE_TRANSFER_UNSYNCHRONIZED : 0));
p = pctx-transfer_map(pctx, r.pipe, 0, usage,
--
2.0.4
___
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] RFC: glsl/glcpp: Allow for '#' characters to appear in shader body
On Fri, Aug 8, 2014 at 12:29 AM, Carl Worth cwo...@cworth.org wrote: Erik Faye-Lund kusmab...@gmail.com writes: Note that '$' is a bit different, as it's not a part of the preprocessor's character set, so using it might be interpreted as undefined behavior. Right. That could easily go either way. Is the phrase each non-white-space character restricted to characters of the original character set? This is a bit unclear. Section 2.1 [lex.phases] has the following to say: 1. Physical source file characters are mapped, in an implementation-defined manner, to the basic source character set (introducing new-line characters for end-of-line indicators) if necessary. Trigraph sequences are replaced by corresponding single-character internal representations. Any source file character not in the basic source character set is replaced by the universal-character-name that designates that character. (An implementation may use any internal encoding, so long as an actual extended character encountered in the source file, and the same extended character expressed in the source file as a universal-character-name (i.e. using the \u notation), are handled equivalently.) snip 5. Each source character set member, escape sequence, or universal-character-name in character literals and string literals is converted to the corresponding member of the execution character set; if there is no corresponding member, it is converted to an implementation-defined member other than the null (wide) character. The Any source file character not in the basic source character set is replaced by the universal-character-name that designates that character-bit seems to be the interesting part, but I'm not quite able to grok it. Appendix E lists universal-character-names, seemingly from ISO/IEC PDTR 10176 (which seems to be some kind of guidelines for programming language design). It seems to me like this is there to support non-ASCII identifiers and strings, but GLSL doesn't support either. I'm not able to come up with a conclusion here. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 80183] [llvmpipe] triangles with vertices that map to raster positions viewport width/height are not displayed
https://bugs.freedesktop.org/show_bug.cgi?id=80183 --- Comment #7 from Roland Scheidegger srol...@vmware.com --- (In reply to comment #2) I found out that the problem is the line: gl_ClipVertex = gl_ModelViewMatrix * gl_Vertex; in the fragment shader. If I remove that line, it works as expected. Since the rendering is ok on ATI/NVidia and on softpipe, I suspect that gl_ClipVertex support is broken in LLVM pipe. Actually I suspect gl_ClipVertex is mostly ok, but you're writing garbage to it and relying on the fact that user plane clipping is disabled so the output isn't actually used. Or something like that. This is where we failed (using the clipvertex output for ordinary clipping even if user plane clipping is disabled). Patch coming... -- 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 2/2] draw: (trivial) use information about gs being present from variant key
From: Roland Scheidegger srol...@vmware.com This is a purely cosmetic change. --- src/gallium/auxiliary/draw/draw_llvm.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/gallium/auxiliary/draw/draw_llvm.c b/src/gallium/auxiliary/draw/draw_llvm.c index 5a7bedb..3d9ddf2 100644 --- a/src/gallium/auxiliary/draw/draw_llvm.c +++ b/src/gallium/auxiliary/draw/draw_llvm.c @@ -1499,16 +1499,15 @@ draw_llvm_generate(struct draw_llvm *llvm, struct draw_llvm_variant *variant, LLVMValueRef fetch_max; struct lp_build_sampler_soa *sampler = 0; LLVMValueRef ret, clipmask_bool_ptr; - const struct draw_geometry_shader *gs = draw-gs.geometry_shader; struct draw_llvm_variant_key *key = variant-key; /* If geometry shader is present we need to skip both the viewport * transformation and clipping otherwise the inputs to the geometry * shader will be incorrect. */ - const boolean bypass_viewport = gs || key-bypass_viewport; - const boolean enable_cliptest = !gs (key-clip_xy || - key-clip_z || - key-clip_user); + const boolean bypass_viewport = key-has_gs || key-bypass_viewport; + const boolean enable_cliptest = !key-has_gs (key-clip_xy || +key-clip_z || +key-clip_user); LLVMValueRef variant_func; const unsigned pos = llvm-draw-vs.position_output; const unsigned cv = llvm-draw-vs.clipvertex_output; -- 1.9.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] draw: don't use clipvertex output if user plane clipping is disabled
From: Roland Scheidegger srol...@vmware.com
The non-llvm path made sure that both clip and pre_clip_pos point to the data
output by position, not clipvertex, if user based clipping is disabled.
However, the llvm path did not, which apparently led to failures if
gl_ClipVertex was written but user plane clipping not enabled (bug 80183).
Why I have no idea really, but just make it match the non-llvm behavior...
---
src/gallium/auxiliary/draw/draw_llvm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/gallium/auxiliary/draw/draw_llvm.c
b/src/gallium/auxiliary/draw/draw_llvm.c
index d29adfb..5a7bedb 100644
--- a/src/gallium/auxiliary/draw/draw_llvm.c
+++ b/src/gallium/auxiliary/draw/draw_llvm.c
@@ -1732,7 +1732,7 @@ draw_llvm_generate(struct draw_llvm *llvm, struct
draw_llvm_variant *variant,
if (pos != -1 cv != -1) {
/* store original positions in clip before further manipulation */
- store_clip(gallivm, vs_type, io, outputs, 0, cv);
+ store_clip(gallivm, vs_type, io, outputs, 0, key-clip_user ? cv :
pos);
store_clip(gallivm, vs_type, io, outputs, 1, pos);
/* do cliptest */
--
1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Allow the blorp blit between BGR and RGB
On Tuesday, July 01, 2014 04:04:56 PM Neil Roberts wrote:
FWIW, I relaxed the format restrictions in
brw_blorp_copytexsubimage, so it can handle general format
conversions as well (i.e. RGBA_FLOAT16 - RGBA_UNORM). There's
no reason we couldn't do that for BlitFramebuffer as well, I just
forgot to do it (and then we decided to make it a newbie task, and
then the newbie didn't do said task, and...oops.)
Ok, I think that is a good idea. Here is a patch to do it. I'm also
sending a test to the piglit mailing list that tries blitting between
different formats.
Regards,
- Neil
Oh, sorry I missed this! This looks good to me, and it'll be good to have this
for 10.3...
Reviewed-by: Kenneth Graunke kenn...@whitecape.org
--- 8 --- (use git am --scissors to automatically chop here)
Subject: i965: Don't check for format differences when using the blorp blitter
Previously the blorp blitter wouldn't be used if the source and destination
buffer had a different format other than swizzling between RGB and BGR and
adding or removing a dummy alpha channel. However there's no reason why the
blorp code path can't be used to do almost all format conversions so this
patch just removes the checks. However it does explicitly disable converting
to/from MESA_FORMAT_Z24_UNORM_X8_UINT because there is a similar check
brw_blorp_copytexsubimage.
This doesn't cause any Piglit test regressions at least on Ivybridge.
---
src/mesa/drivers/dri/i965/brw_blorp_blit.cpp | 66
+---
1 file changed, 12 insertions(+), 54 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
index d7f5f7d..6a4918e 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
+++ b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
@@ -120,52 +120,6 @@ do_blorp_blit(struct brw_context *brw, GLbitfield
buffer_bit,
}
static bool
-format_is_rgba_or_rgbx_byte(mesa_format format)
-{
- switch (format) {
- case MESA_FORMAT_B8G8R8X8_UNORM:
- case MESA_FORMAT_B8G8R8A8_UNORM:
- case MESA_FORMAT_R8G8B8X8_UNORM:
- case MESA_FORMAT_R8G8B8A8_UNORM:
- return true;
- default:
- return false;
- }
-}
-
-static bool
-color_formats_match(mesa_format src_format, mesa_format dst_format)
-{
- mesa_format linear_src_format = _mesa_get_srgb_format_linear(src_format);
- mesa_format linear_dst_format = _mesa_get_srgb_format_linear(dst_format);
-
- /* Normally, we require the formats to be equal. However, we also support
-* blitting from ARGB to XRGB (discarding alpha), and from XRGB to ARGB
-* (overriding alpha to 1.0 via blending) as well as swizzling between BGR
-* and RGB.
-*/
-
- return (linear_src_format == linear_dst_format ||
- (format_is_rgba_or_rgbx_byte(linear_src_format)
-format_is_rgba_or_rgbx_byte(linear_dst_format)));
-}
-
-static bool
-formats_match(GLbitfield buffer_bit, struct intel_renderbuffer *src_irb,
- struct intel_renderbuffer *dst_irb)
-{
- /* Note: don't just check gl_renderbuffer::Format, because in some cases
-* multiple gl_formats resolve to the same native type in the miptree (for
-* example MESA_FORMAT_Z24_UNORM_X8_UINT and
MESA_FORMAT_Z24_UNORM_S8_UINT), and we can blit
-* between those formats.
-*/
- mesa_format src_format = find_miptree(buffer_bit, src_irb)-format;
- mesa_format dst_format = find_miptree(buffer_bit, dst_irb)-format;
-
- return color_formats_match(src_format, dst_format);
-}
-
-static bool
try_blorp_blit(struct brw_context *brw,
GLfloat srcX0, GLfloat srcY0, GLfloat srcX1, GLfloat srcY1,
GLfloat dstX0, GLfloat dstY0, GLfloat dstX1, GLfloat dstY1,
@@ -191,16 +145,13 @@ try_blorp_blit(struct brw_context *brw,
/* Find buffers */
struct intel_renderbuffer *src_irb;
struct intel_renderbuffer *dst_irb;
+ struct intel_mipmap_tree *src_mt;
+ struct intel_mipmap_tree *dst_mt;
switch (buffer_bit) {
case GL_COLOR_BUFFER_BIT:
src_irb = intel_renderbuffer(read_fb-_ColorReadBuffer);
for (unsigned i = 0; i ctx-DrawBuffer-_NumColorDrawBuffers; ++i) {
dst_irb = intel_renderbuffer(ctx-DrawBuffer-_ColorDrawBuffers[i]);
- if (dst_irb !formats_match(buffer_bit, src_irb, dst_irb))
-return false;
- }
- for (unsigned i = 0; i ctx-DrawBuffer-_NumColorDrawBuffers; ++i) {
- dst_irb = intel_renderbuffer(ctx-DrawBuffer-_ColorDrawBuffers[i]);
if (dst_irb)
do_blorp_blit(brw, buffer_bit, src_irb, dst_irb, srcX0, srcY0,
srcX1, srcY1, dstX0, dstY0, dstX1, dstY1,
@@ -212,8 +163,17 @@ try_blorp_blit(struct brw_context *brw,
intel_renderbuffer(read_fb-Attachment[BUFFER_DEPTH].Renderbuffer);
dst_irb =
[Mesa-dev] [PATCH] configure: check for dladdr via AC_CHECK_FUNC/AC_CHECK_LIB
From: Pali Rohár pali.ro...@gmail.com Use both macros as in some cases using AC_CHECK_FUNCS alone may fail. Thus HAVE_DLADDR will not be defined, and as a result most of the code in megadriver_stub.c will not be compiled. Breakind the backwards compat with between older libGL/xserver(s) and DRI megadrivers. Cc: Jon TURNEY jon.tur...@dronecode.org.uk Cc: 10.2 mesa-sta...@lists.freedesktop.org [Emil Velikov] Commit message. Signed-off-by: Emil Velikov emil.l.veli...@gmail.com --- On the other hand we can replace _mesa_dl*, dl* and util_dl* with a single solution (based on the gallium one). Then we can drop the check altogether, and slim down the DEFINES that we're feeding to everything that we build, and drop the DLOPEN_LIBS :) Perhaps one day... -Emil configure.ac | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/configure.ac b/configure.ac index a3b3abd..85f60ed 100644 --- a/configure.ac +++ b/configure.ac @@ -535,10 +535,9 @@ AC_CHECK_FUNC([dlopen], [DEFINES=$DEFINES -DHAVE_DLOPEN], AC_SUBST([DLOPEN_LIBS]) dnl Check if that library also has dladdr -save_LDFLAGS=$LDFLAGS -LDFLAGS=$LDFLAGS $DLOPEN_LIBS -AC_CHECK_FUNCS([dladdr]) -LDFLAGS=$save_LDFLAGS +AC_CHECK_FUNC([dladdr], [DEFINES=$DEFINES -DHAVE_DLADDR], +[AC_CHECK_LIB([dl], [dladdr], + [DEFINES=$DEFINES -DHAVE_DLADDR])]) case $host_os in darwin*|mingw*) -- 2.0.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 09/13] i965: Calculate start/base_vertex_location after preparing vertices.
Reviewed-by: Ian Romanick ian.d.roman...@intel.com
It might be useful in the commit message to explain why this change is
necessary.
On 08/08/2014 12:31 AM, Kenneth Graunke wrote:
Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
src/mesa/drivers/dri/i965/brw_context.h | 8
src/mesa/drivers/dri/i965/brw_draw.c | 17 -
src/mesa/drivers/dri/i965/brw_draw.h | 2 ++
src/mesa/drivers/dri/i965/brw_draw_upload.c | 12
src/mesa/drivers/dri/i965/brw_state_upload.c | 6 +++---
src/mesa/drivers/dri/i965/gen8_draw_upload.c | 1 +
6 files changed, 34 insertions(+), 12 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_context.h
b/src/mesa/drivers/dri/i965/brw_context.h
index 1bbcf46..eea7938 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -1058,6 +1058,14 @@ struct brw_context
bool no_depth_or_stencil;
struct {
+ /** Does the current draw use the index buffer? */
+ bool indexed;
+
+ int start_vertex_location;
+ int base_vertex_location;
+ } draw;
+
+ struct {
struct brw_vertex_element inputs[VERT_ATTRIB_MAX];
struct brw_vertex_buffer buffers[VERT_ATTRIB_MAX];
diff --git a/src/mesa/drivers/dri/i965/brw_draw.c
b/src/mesa/drivers/dri/i965/brw_draw.c
index 4dae7d3..d8e16a7 100644
--- a/src/mesa/drivers/dri/i965/brw_draw.c
+++ b/src/mesa/drivers/dri/i965/brw_draw.c
@@ -167,26 +167,19 @@ static void brw_emit_prim(struct brw_context *brw,
{
int verts_per_instance;
int vertex_access_type;
- int start_vertex_location;
- int base_vertex_location;
int indirect_flag;
DBG(PRIM: %s %d %d\n, _mesa_lookup_enum_by_nr(prim-mode),
prim-start, prim-count);
- start_vertex_location = prim-start;
- base_vertex_location = prim-basevertex;
if (prim-indexed) {
vertex_access_type = brw-gen = 7 ?
GEN7_3DPRIM_VERTEXBUFFER_ACCESS_RANDOM :
GEN4_3DPRIM_VERTEXBUFFER_ACCESS_RANDOM;
- start_vertex_location += brw-ib.start_vertex_offset;
- base_vertex_location += brw-vb.start_vertex_bias;
} else {
vertex_access_type = brw-gen = 7 ?
GEN7_3DPRIM_VERTEXBUFFER_ACCESS_SEQUENTIAL :
GEN4_3DPRIM_VERTEXBUFFER_ACCESS_SEQUENTIAL;
- start_vertex_location += brw-vb.start_vertex_bias;
}
/* We only need to trim the primitive count on pre-Gen6. */
@@ -261,10 +254,10 @@ static void brw_emit_prim(struct brw_context *brw,
vertex_access_type);
}
OUT_BATCH(verts_per_instance);
- OUT_BATCH(start_vertex_location);
+ OUT_BATCH(brw-draw.start_vertex_location);
OUT_BATCH(prim-num_instances);
OUT_BATCH(prim-base_instance);
- OUT_BATCH(base_vertex_location);
+ OUT_BATCH(brw-draw.base_vertex_location);
ADVANCE_BATCH();
/* Only used on Sandybridge; harmless to set elsewhere. */
@@ -467,12 +460,18 @@ static bool brw_try_draw_prims( struct gl_context *ctx,
brw_merge_inputs(brw, arrays);
}
}
+
+ brw-draw.indexed = prims[i].indexed;
+ brw-draw.start_vertex_location = prims[i].start;
+ brw-draw.base_vertex_location = prims[i].basevertex;
+
if (brw-gen 6)
brw_set_prim(brw, prims[i]);
else
gen6_set_prim(brw, prims[i]);
retry:
+
/* Note that before the loop, brw-state.dirty.brw was set to != 0, and
* that the state updated in the loop outside of this block is that in
* *_set_prim or intel_batchbuffer_flush(), which only impacts
diff --git a/src/mesa/drivers/dri/i965/brw_draw.h
b/src/mesa/drivers/dri/i965/brw_draw.h
index 774f1d4..fc83dcd 100644
--- a/src/mesa/drivers/dri/i965/brw_draw.h
+++ b/src/mesa/drivers/dri/i965/brw_draw.h
@@ -47,6 +47,8 @@ void brw_draw_prims( struct gl_context *ctx,
void brw_draw_init( struct brw_context *brw );
void brw_draw_destroy( struct brw_context *brw );
+void brw_prepare_shader_draw_parameters(struct brw_context *);
+
/* brw_primitive_restart.c */
GLboolean
brw_handle_primitive_restart(struct gl_context *ctx,
diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c
b/src/mesa/drivers/dri/i965/brw_draw_upload.c
index 5d6b766..38b1087 100644
--- a/src/mesa/drivers/dri/i965/brw_draw_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c
@@ -604,12 +604,24 @@ brw_prepare_vertices(struct brw_context *brw)
brw-vb.nr_buffers = j;
}
+void
+brw_prepare_shader_draw_parameters(struct brw_context *brw)
+{
+ if (brw-draw.indexed) {
+ brw-draw.start_vertex_location += brw-ib.start_vertex_offset;
+ brw-draw.base_vertex_location += brw-vb.start_vertex_bias;
+ } else {
+ brw-draw.start_vertex_location += brw-vb.start_vertex_bias;
+ }
+}
+
static void brw_emit_vertices(struct brw_context *brw)
{
struct gl_context *ctx = brw-ctx;
GLuint i,
Re: [Mesa-dev] [PATCH 10/13] i965: Make gl_BaseVertex available in a buffer object.
Reviewed-by: Ian Romanick ian.d.roman...@intel.com
On 08/08/2014 12:31 AM, Kenneth Graunke wrote:
This will be used for GL_ARB_shader_draw_parameters, as well as fixing
gl_VertexID, which is supposed to include gl_BaseVertex's value.
For indirect draws, we simply point at the indirect buffer; for normal
draws, we upload the value via the upload buffer.
Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
src/mesa/drivers/dri/i965/brw_context.h | 7 +++
src/mesa/drivers/dri/i965/brw_draw.c| 14 ++
src/mesa/drivers/dri/i965/brw_draw_upload.c | 10 ++
3 files changed, 31 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/brw_context.h
b/src/mesa/drivers/dri/i965/brw_context.h
index eea7938..b5b5e41 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -1063,6 +1063,13 @@ struct brw_context
int start_vertex_location;
int base_vertex_location;
+
+ /**
+ * Buffer and offset used for GL_ARB_shader_draw_parameters
+ * (for now, only gl_BaseVertex).
+ */
+ drm_intel_bo *draw_params_bo;
+ uint32_t draw_params_offset;
} draw;
struct {
diff --git a/src/mesa/drivers/dri/i965/brw_draw.c
b/src/mesa/drivers/dri/i965/brw_draw.c
index d8e16a7..f0aaec7 100644
--- a/src/mesa/drivers/dri/i965/brw_draw.c
+++ b/src/mesa/drivers/dri/i965/brw_draw.c
@@ -465,6 +465,20 @@ static bool brw_try_draw_prims( struct gl_context *ctx,
brw-draw.start_vertex_location = prims[i].start;
brw-draw.base_vertex_location = prims[i].basevertex;
+ if (prims[i].is_indirect) {
+ /* Point draw_params_bo at the indirect buffer. */
+ brw-draw.draw_params_bo =
+intel_buffer_object(ctx-DrawIndirectBuffer)-buffer;
+ brw-draw.draw_params_offset =
+prims[i].indirect_offset + (prims[i].indexed ? 12 : 8);
+ } else {
+ /* Set draw_params_bo to NULL so brw_prepare_vertices knows it
+ * has to upload gl_BaseVertex and such if they're needed.
+ */
+ brw-draw.draw_params_bo = NULL;
+ brw-draw.draw_params_offset = 0;
+ }
+
if (brw-gen 6)
brw_set_prim(brw, prims[i]);
else
diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c
b/src/mesa/drivers/dri/i965/brw_draw_upload.c
index 38b1087..37a65bc 100644
--- a/src/mesa/drivers/dri/i965/brw_draw_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c
@@ -607,11 +607,21 @@ brw_prepare_vertices(struct brw_context *brw)
void
brw_prepare_shader_draw_parameters(struct brw_context *brw)
{
+ int *gl_basevertex_value;
if (brw-draw.indexed) {
brw-draw.start_vertex_location += brw-ib.start_vertex_offset;
brw-draw.base_vertex_location += brw-vb.start_vertex_bias;
+ gl_basevertex_value = brw-draw.base_vertex_location;
} else {
brw-draw.start_vertex_location += brw-vb.start_vertex_bias;
+ gl_basevertex_value = brw-draw.start_vertex_location;
+ }
+
+ /* For non-indirect draws, upload gl_BaseVertex. */
+ if (brw-vs.prog_data-uses_vertexid brw-draw.draw_params_bo == NULL)
{
+ intel_upload_data(brw, gl_basevertex_value, 4, 4,
+ brw-draw.draw_params_bo,
+brw-draw.draw_params_offset);
}
}
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 11/13] i965: Refactor Gen4-7 VERTEX_BUFFER_STATE emission into a helper.
Reviewed-by: Ian Romanick ian.d.roman...@intel.com
On 08/08/2014 12:31 AM, Kenneth Graunke wrote:
We'll need to emit another VERTEX_BUFFER_STATE for gl_BaseVertex;
pulling this into a helper function will save us from having to deal
with cross-generation differences in that code.
Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
src/mesa/drivers/dri/i965/brw_draw_upload.c | 77
++---
1 file changed, 47 insertions(+), 30 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c
b/src/mesa/drivers/dri/i965/brw_draw_upload.c
index 37a65bc..7c01d79 100644
--- a/src/mesa/drivers/dri/i965/brw_draw_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c
@@ -625,9 +625,52 @@ brw_prepare_shader_draw_parameters(struct brw_context
*brw)
}
}
-static void brw_emit_vertices(struct brw_context *brw)
+/**
+ * Emit a VERTEX_BUFFER_STATE entry (part of 3DSTATE_VERTEX_BUFFERS).
+ */
+static void
+emit_vertex_buffer_state(struct brw_context *brw,
+ unsigned buffer_nr,
+ drm_intel_bo *bo,
+ unsigned bo_ending_address,
+ unsigned bo_offset,
+ unsigned stride,
+ unsigned step_rate)
{
struct gl_context *ctx = brw-ctx;
+ uint32_t dw0;
+
+ if (brw-gen = 6) {
+ dw0 = (buffer_nr GEN6_VB0_INDEX_SHIFT) |
+(step_rate ? GEN6_VB0_ACCESS_INSTANCEDATA
+ : GEN6_VB0_ACCESS_VERTEXDATA);
+ } else {
+ dw0 = (buffer_nr BRW_VB0_INDEX_SHIFT) |
+(step_rate ? BRW_VB0_ACCESS_INSTANCEDATA
+ : BRW_VB0_ACCESS_VERTEXDATA);
+ }
+
+ if (brw-gen = 7)
+ dw0 |= GEN7_VB0_ADDRESS_MODIFYENABLE;
+
+ if (brw-gen == 7)
+ dw0 |= GEN7_MOCS_L3 16;
+
+ WARN_ONCE(stride = (brw-gen = 5 ? 2048 : 2047),
+ VBO stride %d too large, bad rendering may occur\n,
+ stride);
+ OUT_BATCH(dw0 | (stride BRW_VB0_PITCH_SHIFT));
+ OUT_RELOC(bo, I915_GEM_DOMAIN_VERTEX, 0, bo_offset);
+ if (brw-gen = 5) {
+ OUT_RELOC(bo, I915_GEM_DOMAIN_VERTEX, 0, bo_ending_address);
+ } else {
+ OUT_BATCH(0);
+ }
+ OUT_BATCH(step_rate);
+}
+
+static void brw_emit_vertices(struct brw_context *brw)
+{
GLuint i, nr_elements;
brw_prepare_vertices(brw);
@@ -680,36 +723,10 @@ static void brw_emit_vertices(struct brw_context *brw)
OUT_BATCH((_3DSTATE_VERTEX_BUFFERS 16) | (4*brw-vb.nr_buffers -
1));
for (i = 0; i brw-vb.nr_buffers; i++) {
struct brw_vertex_buffer *buffer = brw-vb.buffers[i];
- uint32_t dw0;
-
- if (brw-gen = 6) {
- dw0 = buffer-step_rate
- ? GEN6_VB0_ACCESS_INSTANCEDATA
- : GEN6_VB0_ACCESS_VERTEXDATA;
- dw0 |= i GEN6_VB0_INDEX_SHIFT;
- } else {
- dw0 = buffer-step_rate
- ? BRW_VB0_ACCESS_INSTANCEDATA
- : BRW_VB0_ACCESS_VERTEXDATA;
- dw0 |= i BRW_VB0_INDEX_SHIFT;
- }
+ emit_vertex_buffer_state(brw, i, buffer-bo, buffer-bo-size - 1,
+ buffer-offset, buffer-stride,
+ buffer-step_rate);
- if (brw-gen = 7)
- dw0 |= GEN7_VB0_ADDRESS_MODIFYENABLE;
-
- if (brw-gen == 7)
- dw0 |= GEN7_MOCS_L3 16;
-
- WARN_ONCE(buffer-stride = (brw-gen = 5 ? 2048 : 2047),
- VBO stride %d too large, bad rendering may occur\n,
- buffer-stride);
- OUT_BATCH(dw0 | (buffer-stride BRW_VB0_PITCH_SHIFT));
- OUT_RELOC(buffer-bo, I915_GEM_DOMAIN_VERTEX, 0, buffer-offset);
- if (brw-gen = 5) {
- OUT_RELOC(buffer-bo, I915_GEM_DOMAIN_VERTEX, 0, buffer-bo-size -
1);
- } else
- OUT_BATCH(0);
- OUT_BATCH(buffer-step_rate);
}
ADVANCE_BATCH();
}
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 06/13] mesa: Replace string comparisons with SYSTEM_VALUE enum checks.
This patch is Reviewed-by: Ian Romanick ian.d.roman...@intel.com But it raises an interesting question. If we lower gl_VertexID to gl_VertexIDMESA + gl_BaseVertex, are we going to report active attributes incorrectly through the API? On 08/08/2014 12:31 AM, Kenneth Graunke wrote: This is more efficient. Signed-off-by: Kenneth Graunke kenn...@whitecape.org --- src/mesa/main/shader_query.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) You can't see it in the diff, but this is in a switch statement on var-data.mode, in the ir_var_system_value case. diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp index 4267743..4871d09 100644 --- a/src/mesa/main/shader_query.cpp +++ b/src/mesa/main/shader_query.cpp @@ -92,8 +92,8 @@ is_active_attrib(const ir_variable *var) * are enumerated, including the special built-in inputs gl_VertexID * and gl_InstanceID. */ - return !strcmp(var-name, gl_VertexID) || - !strcmp(var-name, gl_InstanceID); + return var-data.location == SYSTEM_VALUE_VERTEX_ID || + var-data.location == SYSTEM_VALUE_INSTANCE_ID; default: return false; ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 12/13] i965: Expose gl_BaseVertex via a vertex attribute.
(For what it's worth.)
Reviewed-by: Ian Romanick ian.d.roman...@intel.com
On 08/08/2014 12:31 AM, Kenneth Graunke wrote:
Now that we have the data available, we need to expose it to the
shaders. We can reuse the same vertex element that we use for
gl_VertexID, but we need to back it by an actual vertex buffer.
A hardware restriction requires that vertex attributes coming from a
buffer (STORE_SRC) must come before any other types (i.e. STORE_0).
So, we have to make gl_BaseVertex be the .x component of the vertex
attribute. This means moving gl_VertexID to a different component.
I chose to move gl_VertexID and gl_InstanceID to the .z and .w
components, respectively, to make room for gl_BaseInstance in the .y
component (which would also come from a buffer, and therefore be
STORE_SRC).
Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
src/mesa/drivers/dri/i965/brw_draw_upload.c | 38 ++---
src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp | 7 ++--
src/mesa/drivers/dri/i965/gen8_draw_upload.c | 40
+++
3 files changed, 65 insertions(+), 20 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c
b/src/mesa/drivers/dri/i965/brw_draw_upload.c
index 7c01d79..d59ca8b 100644
--- a/src/mesa/drivers/dri/i965/brw_draw_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c
@@ -712,15 +712,18 @@ static void brw_emit_vertices(struct brw_context *brw)
/* Now emit VB and VEP state packets.
*/
- if (brw-vb.nr_buffers) {
+ unsigned nr_buffers =
+ brw-vb.nr_buffers + brw-vs.prog_data-uses_vertexid;
+
+ if (nr_buffers) {
if (brw-gen = 6) {
- assert(brw-vb.nr_buffers = 33);
+ assert(nr_buffers = 33);
} else {
- assert(brw-vb.nr_buffers = 17);
+ assert(nr_buffers = 17);
}
- BEGIN_BATCH(1 + 4*brw-vb.nr_buffers);
- OUT_BATCH((_3DSTATE_VERTEX_BUFFERS 16) | (4*brw-vb.nr_buffers -
1));
+ BEGIN_BATCH(1 + 4 * nr_buffers);
+ OUT_BATCH((_3DSTATE_VERTEX_BUFFERS 16) | (4 * nr_buffers - 1));
for (i = 0; i brw-vb.nr_buffers; i++) {
struct brw_vertex_buffer *buffer = brw-vb.buffers[i];
emit_vertex_buffer_state(brw, i, buffer-bo, buffer-bo-size - 1,
@@ -728,6 +731,15 @@ static void brw_emit_vertices(struct brw_context *brw)
buffer-step_rate);
}
+
+ if (brw-vs.prog_data-uses_vertexid) {
+ emit_vertex_buffer_state(brw, brw-vb.nr_buffers,
+ brw-draw.draw_params_bo,
+ brw-draw.draw_params_bo-size - 1,
+ brw-draw.draw_params_offset,
+ 0, /* stride */
+ 0); /* step rate */
+ }
ADVANCE_BATCH();
}
@@ -815,15 +827,19 @@ static void brw_emit_vertices(struct brw_context *brw)
if (brw-vs.prog_data-uses_vertexid) {
uint32_t dw0 = 0, dw1 = 0;
- dw1 = ((BRW_VE1_COMPONENT_STORE_VID BRW_VE1_COMPONENT_0_SHIFT) |
- (BRW_VE1_COMPONENT_STORE_IID BRW_VE1_COMPONENT_1_SHIFT) |
- (BRW_VE1_COMPONENT_STORE_0 BRW_VE1_COMPONENT_2_SHIFT) |
- (BRW_VE1_COMPONENT_STORE_0 BRW_VE1_COMPONENT_3_SHIFT));
+ dw1 = (BRW_VE1_COMPONENT_STORE_SRC BRW_VE1_COMPONENT_0_SHIFT) |
+(BRW_VE1_COMPONENT_STORE_0BRW_VE1_COMPONENT_1_SHIFT) |
+(BRW_VE1_COMPONENT_STORE_VID BRW_VE1_COMPONENT_2_SHIFT) |
+(BRW_VE1_COMPONENT_STORE_IID BRW_VE1_COMPONENT_3_SHIFT);
if (brw-gen = 6) {
- dw0 |= GEN6_VE0_VALID;
+ dw0 |= GEN6_VE0_VALID |
+brw-vb.nr_buffers GEN6_VE0_INDEX_SHIFT |
+BRW_SURFACEFORMAT_R32_UINT BRW_VE0_FORMAT_SHIFT;
} else {
- dw0 |= BRW_VE0_VALID;
+ dw0 |= BRW_VE0_VALID |
+brw-vb.nr_buffers BRW_VE0_INDEX_SHIFT |
+BRW_SURFACEFORMAT_R32_UINT BRW_VE0_FORMAT_SHIFT;
dw1 |= (i * 4) BRW_VE1_DST_OFFSET_SHIFT;
}
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp
b/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp
index 6a1c049..667ed68 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp
@@ -154,12 +154,15 @@ vec4_vs_visitor::make_reg_for_system_value(ir_variable
*ir)
vs_prog_data-uses_vertexid = true;
switch (ir-data.location) {
+ case SYSTEM_VALUE_BASE_VERTEX:
+ reg-writemask = WRITEMASK_X;
+ break;
case SYSTEM_VALUE_VERTEX_ID:
case SYSTEM_VALUE_VERTEX_ID_ZERO_BASE:
- reg-writemask = WRITEMASK_X;
+ reg-writemask = WRITEMASK_Z;
break;
case SYSTEM_VALUE_INSTANCE_ID:
- reg-writemask = WRITEMASK_Y;
+ reg-writemask = WRITEMASK_W;
break;
default:
unreachable(not reached);
diff --git
Re: [Mesa-dev] [PATCH] configure: check for dladdr via AC_CHECK_FUNC/AC_CHECK_LIB
No idea why this was a problem, but the new code matches the existing dlopen check. Reviewed-by: Matt Turner matts...@gmail.com ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/5] i965: Add support for ARB_copy_image
On Fri, Aug 8, 2014 at 4:05 AM, Neil Roberts n...@linux.intel.com wrote:
Jason Ekstrand ja...@jlekstrand.net writes:
+ if (src_mt == dst_mt src_level == dst_level src_z == dst_z) {
+ /* If we are on the same miptree, same level, and same slice, then
+ * intel_miptree_map won't let us map it twice. We have to do a
+ * single map in read-write mode.
+ */
+
+ map_x1 = MIN2(src_x, dst_x);
+ map_y1 = MIN2(src_y, dst_y);
+ map_x2 = MAX2(src_x, dst_x) + src_width;
+ map_y2 = MAX2(src_y, dst_y) + src_height;
+
+ intel_miptree_map(brw, src_mt, src_level, src_z,
+map_x1, map_y1, map_x2 - map_x1, map_y2 -
map_y1,
+GL_MAP_READ_BIT | GL_MAP_WRITE_BIT,
+(void **)mapped, src_stride);
+
+ dst_stride = src_stride;
+
+ /* Set the offsets here so we don't have to think about it later
*/
+ src_mapped = mapped + (src_y - map_y1) * src_stride +
+(src_x - map_x1) * cpp;
+ dst_mapped = mapped + (dst_y - map_y1) * dst_stride +
+(dst_x - map_x1) * cpp;
This needs to take into account the block size of the format or it will
offset to the wrong position. I guess that is quite important as this
code path will only be used for compressed formats.
Yeah, you're right. I'll get that patched up and update the tests to
exercise it.
The commit message still mentions the maximum stride which is no longer
valid.
Regards,
- Neil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] configure: check for dladdr via AC_CHECK_FUNC/AC_CHECK_LIB
The problem is that AC_CHECK_FUNCS would stick the LDFLAGS before the conftest.c arg while AC_CHECK_LIB sticks the -ldl after conftest.c. This apparently matters with newer gcc's. There's probably some correct autoconf way of dealing with it, but... this works :) On Fri, Aug 8, 2014 at 1:55 PM, Matt Turner matts...@gmail.com wrote: No idea why this was a problem, but the new code matches the existing dlopen check. Reviewed-by: Matt Turner matts...@gmail.com ___ 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 07/13] mesa: Fix glGetActiveAttribute for gl_VertexID when lowered.
On 08/08/2014 12:31 AM, Kenneth Graunke wrote:
The lower_vertex_id pass converts uses of the gl_VertexID system value
to the gl_BaseVertex and gl_VertexIDMESA system values. Since
gl_VertexID is no longer accessed, it would not be considered active.
Of course, it should be, since the shader uses gl_VertexID.
Right... hmm. Between this and your comments on patch 3, I think we're
going to have some additional challenges implementing
GL_ARB_shader_draw_parameters... all surrounding various abuse of
gl_BaseVertex. I have a couple ideas for handling that, but I don't
think it will require enough re-work to warrant delaying this code any
further.
Reviewed-by: Ian Romanick ian.d.roman...@intel.com
Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
src/mesa/main/shader_query.cpp | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
Avoids regressing Piglit's gl-get-active-attrib-returns-all-inputs when
enabling lowering later in the series.
diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
index 4871d09..c395a78 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -93,6 +93,7 @@ is_active_attrib(const ir_variable *var)
* and gl_InstanceID.
*/
return var-data.location == SYSTEM_VALUE_VERTEX_ID ||
+ var-data.location == SYSTEM_VALUE_VERTEX_ID_ZERO_BASE ||
var-data.location == SYSTEM_VALUE_INSTANCE_ID;
default:
@@ -128,12 +129,22 @@ _mesa_GetActiveAttrib(GLhandleARB program, GLuint
desired_index,
foreach_in_list(ir_instruction, node, ir) {
const ir_variable *const var = node-as_variable();
+ const char *var_name = var-name;
if (!is_active_attrib(var))
continue;
if (current_index == desired_index) {
- _mesa_copy_string(name, maxLength, length, var-name);
+ /* Since gl_VertexID may be lowered to gl_VertexIDMESA, we need to
+ * consider gl_VertexIDMESA as gl_VertexID for purposes of checking
+ * active attributes.
+ */
+ if (var-data.mode == ir_var_system_value
+ var-data.location == SYSTEM_VALUE_VERTEX_ID_ZERO_BASE) {
+var_name = gl_VertexID;
+ }
+
+ _mesa_copy_string(name, maxLength, length, var_name);
if (size)
*size = (var-type-is_array()) ? var-type-length : 1;
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 03/13] mesa: Add SYSTEM_VALUE_BASE_VERTEX
On 08/08/2014 12:37 AM, Kenneth Graunke wrote: On Friday, August 08, 2014 12:31:07 AM Kenneth Graunke wrote: From: Ian Romanick ian.d.roman...@intel.com This system value represents the basevertex value passed to glDrawElementsBaseVertex and related functions. Signed-off-by: Ian Romanick ian.d.roman...@intel.com --- src/mesa/main/mtypes.h | 15 ++- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 1 + 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index c603007..99037c6 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -2084,7 +2084,12 @@ typedef enum * gl_VertexID gets basevertex added in. This differs from DirectX where * SV_VertexID does \b not get basevertex added in. * -* \sa SYSTEM_VALUE_VERTEX_ID_ZERO_BASE +* \note +* If all system values are available, \c SYSTEM_VALUE_VERTEX_ID will be +* equal to \c SYSTEM_VALUE_VERTEX_ID_ZERO_BASE plus +* \c SYSTEM_VALUE_BASE_VERTEX. +* +* \sa SYSTEM_VALUE_VERTEX_ID_ZERO_BASE, SYSTEM_VALUE_BASE_VERTEX */ SYSTEM_VALUE_VERTEX_ID, @@ -2126,6 +2131,14 @@ typedef enum * \sa SYSTEM_VALUE_VERTEX_ID, SYSTEM_VALUE_BASE_VERTEX */ SYSTEM_VALUE_VERTEX_ID_ZERO_BASE, + + /** +* Value of \c basevertex passed to \c glDrawElementsBaseVertex and similar +* functions. +* +* \sa SYSTEM_VALUE_VERTEX_ID, SYSTEM_VALUE_VERTEX_ID_ZERO_BASE +*/ + SYSTEM_VALUE_BASE_VERTEX, /*@}*/ Ian, It occurred to me that we're sort of abusing this system value in the i965 patches later in this series - we're using it to store gl_BaseVertexARB, but also using it to store the first parameter for glDrawArrays. I think in the glDrawArrays case, gl_BaseVertexARB is supposed to be 0. Yeah, you're right on all counts. I sent some commentary on patch 7. Short version... we'll have to do some rework to support GL_ARB_shader_draw_parameters, but I think it's safe to leave this code as-is until we implement that other extension. I'm not sure what the right thing to do is. --Ken ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 82327] FAIL: glcpp/tests/glcpp-test-cr-lf
https://bugs.freedesktop.org/show_bug.cgi?id=82327 Ian Romanick i...@freedesktop.org changed: What|Removed |Added Assignee|mesa-dev@lists.freedesktop. |cwo...@cworth.org |org | --- Comment #1 from Ian Romanick i...@freedesktop.org --- Hm... we may want to disable some of these tests on non-Linux platforms. Since we're playing around with the line endings (and assuming they start in a certain state), there are a few things that could go wrong. Carl, what do you think? -- 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 4/5] i965: Add support for ARB_copy_image
This, together with the meta path, provides a complete implemetation of
ARB_copy_image.
v2: Add a fallback memcpy path for when the texture is too big for the
blitter
v3: Properly support copying between two places on the same texture in the
memcpy fallback
v4: Properly handle blit between the same two images in the fallback path
v5: Properly handle blit between the same two compressed images in the
fallback path
Signed-off-by: Jason Ekstrand jason.ekstr...@intel.com
---
src/mesa/drivers/dri/i965/Makefile.sources | 1 +
src/mesa/drivers/dri/i965/brw_context.c | 1 +
src/mesa/drivers/dri/i965/intel_copy_image.c | 267 +++
src/mesa/drivers/dri/i965/intel_extensions.c | 1 +
src/mesa/drivers/dri/i965/intel_tex.h| 2 +
5 files changed, 272 insertions(+)
create mode 100644 src/mesa/drivers/dri/i965/intel_copy_image.c
diff --git a/src/mesa/drivers/dri/i965/Makefile.sources
b/src/mesa/drivers/dri/i965/Makefile.sources
index ee28dd9..1e5d1c6 100644
--- a/src/mesa/drivers/dri/i965/Makefile.sources
+++ b/src/mesa/drivers/dri/i965/Makefile.sources
@@ -8,6 +8,7 @@ i965_FILES = \
intel_blit.c \
intel_buffer_objects.c \
intel_buffers.c \
+ intel_copy_image.c \
intel_debug.c \
intel_extensions.c \
intel_fbo.c \
diff --git a/src/mesa/drivers/dri/i965/brw_context.c
b/src/mesa/drivers/dri/i965/brw_context.c
index 52f2557..60a225e 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -245,6 +245,7 @@ brw_init_driver_functions(struct brw_context *brw,
intelInitTextureImageFuncs(functions);
intelInitTextureSubImageFuncs(functions);
intelInitTextureCopyImageFuncs(functions);
+ intelInitCopyImageFuncs(functions);
intelInitClearFuncs(functions);
intelInitBufferFuncs(functions);
intelInitPixelFuncs(functions);
diff --git a/src/mesa/drivers/dri/i965/intel_copy_image.c
b/src/mesa/drivers/dri/i965/intel_copy_image.c
new file mode 100644
index 000..068b76b
--- /dev/null
+++ b/src/mesa/drivers/dri/i965/intel_copy_image.c
@@ -0,0 +1,267 @@
+/*
+ * Mesa 3-D graphics library
+ *
+ * Copyright (C) 2014 Intel Corporation All Rights Reserved.
+ *
+ * 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 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:
+ *Jason Ekstrand jason.ekstr...@intel.com
+ */
+
+#include intel_tex.h
+#include intel_blit.h
+#include intel_mipmap_tree.h
+#include main/formats.h
+#include drivers/common/meta.h
+
+static bool
+copy_image_with_blitter(struct brw_context *brw,
+struct intel_mipmap_tree *src_mt, int src_level,
+int src_x, int src_y, int src_z,
+struct intel_mipmap_tree *dst_mt, int dst_level,
+int dst_x, int dst_y, int dst_z,
+int src_width, int src_height)
+{
+ GLuint bw, bh;
+ int cpp;
+
+ /* The blitter doesn't understand multisampling at all. */
+ if (src_mt-num_samples 0 || dst_mt-num_samples 0)
+ return false;
+
+ /* According to the Ivy Bridge PRM, Vol1 Part4, section 1.2.1.2 (Graphics
+* Data Size Limitations):
+*
+*The BLT engine is capable of transferring very large quantities of
+*graphics data. Any graphics data read from and written to the
+*destination is permitted to represent a number of pixels that
+*occupies up to 65,536 scan lines and up to 32,768 bytes per scan line
+*at the destination. The maximum number of pixels that may be
+*represented per scan line’s worth of graphics data depends on the
+*color depth.
+*
+* Furthermore, intelEmitCopyBlit (which is called below) uses a signed
+* 16-bit integer to represent buffer pitch, so it can only handle buffer
+* pitches 32k.
+*
+* As a result of these two limitations, we can only use the blitter to do
+* this copy when the miptree's pitch is
Re: [Mesa-dev] [PATCH 03/13] mesa: Add SYSTEM_VALUE_BASE_VERTEX
Am 08.08.2014 20:06, schrieb Ian Romanick: On 08/08/2014 12:37 AM, Kenneth Graunke wrote: On Friday, August 08, 2014 12:31:07 AM Kenneth Graunke wrote: From: Ian Romanick ian.d.roman...@intel.com This system value represents the basevertex value passed to glDrawElementsBaseVertex and related functions. Signed-off-by: Ian Romanick ian.d.roman...@intel.com --- src/mesa/main/mtypes.h | 15 ++- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 1 + 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index c603007..99037c6 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -2084,7 +2084,12 @@ typedef enum * gl_VertexID gets basevertex added in. This differs from DirectX where * SV_VertexID does \b not get basevertex added in. * -* \sa SYSTEM_VALUE_VERTEX_ID_ZERO_BASE +* \note +* If all system values are available, \c SYSTEM_VALUE_VERTEX_ID will be +* equal to \c SYSTEM_VALUE_VERTEX_ID_ZERO_BASE plus +* \c SYSTEM_VALUE_BASE_VERTEX. +* +* \sa SYSTEM_VALUE_VERTEX_ID_ZERO_BASE, SYSTEM_VALUE_BASE_VERTEX */ SYSTEM_VALUE_VERTEX_ID, @@ -2126,6 +2131,14 @@ typedef enum * \sa SYSTEM_VALUE_VERTEX_ID, SYSTEM_VALUE_BASE_VERTEX */ SYSTEM_VALUE_VERTEX_ID_ZERO_BASE, + + /** +* Value of \c basevertex passed to \c glDrawElementsBaseVertex and similar +* functions. +* +* \sa SYSTEM_VALUE_VERTEX_ID, SYSTEM_VALUE_VERTEX_ID_ZERO_BASE +*/ + SYSTEM_VALUE_BASE_VERTEX, /*@}*/ Ian, It occurred to me that we're sort of abusing this system value in the i965 patches later in this series - we're using it to store gl_BaseVertexARB, but also using it to store the first parameter for glDrawArrays. I think in the glDrawArrays case, gl_BaseVertexARB is supposed to be 0. Yeah, you're right on all counts. I sent some commentary on patch 7. Short version... we'll have to do some rework to support GL_ARB_shader_draw_parameters, but I think it's safe to leave this code as-is until we implement that other extension. So, are you saying the spec really meant the first parameter of Draw calls does not count as baseVertex? That behavior would look very inconsistent (and useless) to me. gl_VertexId has similar language floating around (gl_VertexID will have the base vertex applied to it) and that was resolved to include the first parameter of non-indexed draw too. Roland ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] draw: (trivial) use information about gs being present from variant key
On 08/08/2014 11:02 AM, srol...@vmware.com wrote: From: Roland Scheidegger srol...@vmware.com This is a purely cosmetic change. --- src/gallium/auxiliary/draw/draw_llvm.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/gallium/auxiliary/draw/draw_llvm.c b/src/gallium/auxiliary/draw/draw_llvm.c index 5a7bedb..3d9ddf2 100644 --- a/src/gallium/auxiliary/draw/draw_llvm.c +++ b/src/gallium/auxiliary/draw/draw_llvm.c @@ -1499,16 +1499,15 @@ draw_llvm_generate(struct draw_llvm *llvm, struct draw_llvm_variant *variant, LLVMValueRef fetch_max; struct lp_build_sampler_soa *sampler = 0; LLVMValueRef ret, clipmask_bool_ptr; - const struct draw_geometry_shader *gs = draw-gs.geometry_shader; struct draw_llvm_variant_key *key = variant-key; /* If geometry shader is present we need to skip both the viewport * transformation and clipping otherwise the inputs to the geometry * shader will be incorrect. */ - const boolean bypass_viewport = gs || key-bypass_viewport; - const boolean enable_cliptest = !gs (key-clip_xy || - key-clip_z || - key-clip_user); + const boolean bypass_viewport = key-has_gs || key-bypass_viewport; + const boolean enable_cliptest = !key-has_gs (key-clip_xy || +key-clip_z || +key-clip_user); LLVMValueRef variant_func; const unsigned pos = llvm-draw-vs.position_output; const unsigned cv = llvm-draw-vs.clipvertex_output; Reviewed-by: Brian Paul bri...@vmware.com ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/5] mesa: Add GL API support for ARB_copy_image
This adds the API entrypoint, error checking logic, and a driver hook for the ARB_copy_image extension. v2: Fix a typo in ARB_copy_image.xml and add it to the makefile v3: Put ARB_copy_image.xml in the right place alphebetically in the makefile and properly prefix the commit message v4: Fixed some line wrapping and added a check for null v5: Check for incomplete renderbuffers Signed-off-by: Jason Ekstrand jason.ekstr...@intel.com Reviewed-by: Juha-Pekka Heikkila juhapekka.heikk...@gmail.com Reviewed-by: Neil Roberts n...@linux.intel.com v6: Update dispatch_sanity for the addition of CopyImageSubData --- src/mapi/glapi/gen/ARB_copy_image.xml | 28 +++ src/mapi/glapi/gen/Makefile.am | 1 + src/mapi/glapi/gen/gl_API.xml | 2 +- src/mapi/glapi/gen/gl_genexec.py| 1 + src/mesa/Makefile.sources | 1 + src/mesa/main/copyimage.c | 356 src/mesa/main/copyimage.h | 49 + src/mesa/main/dd.h | 16 ++ src/mesa/main/extensions.c | 1 + src/mesa/main/mtypes.h | 1 + src/mesa/main/tests/dispatch_sanity.cpp | 2 +- src/mesa/main/textureview.c | 36 ++-- src/mesa/main/textureview.h | 4 + 13 files changed, 477 insertions(+), 21 deletions(-) create mode 100644 src/mapi/glapi/gen/ARB_copy_image.xml create mode 100644 src/mesa/main/copyimage.c create mode 100644 src/mesa/main/copyimage.h diff --git a/src/mapi/glapi/gen/ARB_copy_image.xml b/src/mapi/glapi/gen/ARB_copy_image.xml new file mode 100644 index 000..2fbd845 --- /dev/null +++ b/src/mapi/glapi/gen/ARB_copy_image.xml @@ -0,0 +1,28 @@ +?xml version=1.0? +!DOCTYPE OpenGLAPI SYSTEM gl_API.dtd + +OpenGLAPI + +category name=GL_ARB_copy_image number=123 + +function name=CopyImageSubData offset=assign +param name=srcName type=GLuint/ +param name=srcTarget type=GLenum/ +param name=srcLevel type=GLint/ +param name=srcX type=GLint/ +param name=srcY type=GLint/ +param name=srcZ type=GLint/ +param name=dstName type=GLuint/ +param name=dstTarget type=GLenum/ +param name=dstLevel type=GLint/ +param name=dstX type=GLint/ +param name=dstY type=GLint/ +param name=dstZ type=GLint/ +param name=srcWidth type=GLsizei/ +param name=srcHeight type=GLsizei/ +param name=srcDepth type=GLsizei/ +/function + +/category + +/OpenGLAPI diff --git a/src/mapi/glapi/gen/Makefile.am b/src/mapi/glapi/gen/Makefile.am index 212731f..645def4 100644 --- a/src/mapi/glapi/gen/Makefile.am +++ b/src/mapi/glapi/gen/Makefile.am @@ -117,6 +117,7 @@ API_XML = \ ARB_compressed_texture_pixel_storage.xml \ ARB_compute_shader.xml \ ARB_copy_buffer.xml \ + ARB_copy_image.xml \ ARB_debug_output.xml \ ARB_depth_buffer_float.xml \ ARB_depth_clamp.xml \ diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml index e011509..619717d 100644 --- a/src/mapi/glapi/gen/gl_API.xml +++ b/src/mapi/glapi/gen/gl_API.xml @@ -8302,7 +8302,7 @@ xi:include href=ARB_compute_shader.xml xmlns:xi=http://www.w3.org/2001/XInclude/ -!-- ARB extension #123 -- +xi:include href=ARB_copy_image.xml xmlns:xi=http://www.w3.org/2001/XInclude/ xi:include href=ARB_texture_view.xml xmlns:xi=http://www.w3.org/2001/XInclude/ diff --git a/src/mapi/glapi/gen/gl_genexec.py b/src/mapi/glapi/gen/gl_genexec.py index 4609193..d479e66 100644 --- a/src/mapi/glapi/gen/gl_genexec.py +++ b/src/mapi/glapi/gen/gl_genexec.py @@ -62,6 +62,7 @@ header = /** #include main/condrender.h #include main/context.h #include main/convolve.h +#include main/copyimage.h #include main/depth.h #include main/dlist.h #include main/drawpix.h diff --git a/src/mesa/Makefile.sources b/src/mesa/Makefile.sources index 45c53ca..d02c174 100644 --- a/src/mesa/Makefile.sources +++ b/src/mesa/Makefile.sources @@ -31,6 +31,7 @@ MAIN_FILES = \ $(SRCDIR)main/condrender.c \ $(SRCDIR)main/context.c \ $(SRCDIR)main/convolve.c \ + $(SRCDIR)main/copyimage.c \ $(SRCDIR)main/cpuinfo.c \ $(SRCDIR)main/debug.c \ $(SRCDIR)main/depth.c \ diff --git a/src/mesa/main/copyimage.c b/src/mesa/main/copyimage.c new file mode 100644 index 000..e1110dd --- /dev/null +++ b/src/mesa/main/copyimage.c @@ -0,0 +1,356 @@ +/* + * Mesa 3-D graphics library + * + * Copyright (C) 2014 Intel Corporation. All Rights Reserved. + * + * 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
Re: [Mesa-dev] [PATCH 1/2] draw: don't use clipvertex output if user plane clipping is disabled
On 08/08/2014 11:02 AM, srol...@vmware.com wrote:
From: Roland Scheidegger srol...@vmware.com
The non-llvm path made sure that both clip and pre_clip_pos point to the data
output by position, not clipvertex, if user based clipping is disabled.
However, the llvm path did not, which apparently led to failures if
gl_ClipVertex was written but user plane clipping not enabled (bug 80183).
Why I have no idea really, but just make it match the non-llvm behavior...
---
src/gallium/auxiliary/draw/draw_llvm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/gallium/auxiliary/draw/draw_llvm.c
b/src/gallium/auxiliary/draw/draw_llvm.c
index d29adfb..5a7bedb 100644
--- a/src/gallium/auxiliary/draw/draw_llvm.c
+++ b/src/gallium/auxiliary/draw/draw_llvm.c
@@ -1732,7 +1732,7 @@ draw_llvm_generate(struct draw_llvm *llvm, struct
draw_llvm_variant *variant,
if (pos != -1 cv != -1) {
/* store original positions in clip before further manipulation */
- store_clip(gallivm, vs_type, io, outputs, 0, cv);
+ store_clip(gallivm, vs_type, io, outputs, 0, key-clip_user ? cv :
pos);
store_clip(gallivm, vs_type, io, outputs, 1, pos);
/* do cliptest */
As long as you're there, maybe convert the 0/1 arguments to TRUE/FALSE.
Reviewed-by: Brian Paul bri...@vmware.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] configure: check for dladdr via AC_CHECK_FUNC/AC_CHECK_LIB
On 08/08/2014 18:58, Ilia Mirkin wrote: The problem is that AC_CHECK_FUNCS would stick the LDFLAGS before the conftest.c arg while AC_CHECK_LIB sticks the -ldl after conftest.c. This apparently matters with newer gcc's. There's probably some The key difference seems to be that lto was enabled, which I guess means we can't get away with listing objects in a random order :-) correct autoconf way of dealing with it, but... this works :) I think I have used LDFLAGS here where I should have used LIBS. So the more correct way is something like: diff --git a/configure.ac b/configure.ac index 96a67a3..bba64a0 100644 --- a/configure.ac +++ b/configure.ac @@ -535,9 +535,10 @@ AC_CHECK_FUNC([dlopen], [DEFINES=$DEFINES -DHAVE_DLOPEN], AC_SUBST([DLOPEN_LIBS]) dnl Check if that library also has dladdr -AC_CHECK_FUNC([dladdr], [DEFINES=$DEFINES -DHAVE_DLADDR], -[AC_CHECK_LIB([dl], [dladdr], - [DEFINES=$DEFINES -DHAVE_DLADDR])]) +save_LIBS=$LIBS +LIBS=$LIBS $DLOPEN_LIBS +AC_CHECK_FUNCS([dladdr]) +LIBS=$save_LIBS case $host_os in darwin*|mingw*) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/3] cl workdim v2
On Thu, 2014-08-07 at 16:02 +0300, Francisco Jerez wrote: Jan Vesely jan.ves...@rutgers.edu writes: This respin includes Francisco's approach of providing implicit in the arg vector passed from clover, and Tom's idea of appending implicit args after the kernel args. Hmmm... Maybe it would make sense to add some sort of versioning (e.g. as part of the target triple) to the binary interface between clover and the kernel instead, so we can handle this sort of non-backwards compatible changes and the compiler back-end and libclc have some way to find out whether some specific feature is available and e.g. some specific extension should be enabled. I assumed it's not safe to modify exec.input, so the input vector is copied before appending work dim. Why wouldn't it be safe? You just need to make sure they're appended before the compute state is created. I thought there might be a problem when called from multiple threads, but it looks like most of the vars are local to the current call anyway. I looked at the code a bit better, and need a bit of help with what the proffered approach would be. exec_context::bind() appends all kernel args to the input vector. If the implicit args are added before bind() it shifts all other args, which is not what we want. if the implicit args are appended after, they are not accounted for in shader-input_size (and not copied by the driver). my current code modifies exec_context::bind() to preserve the content of input before binding kernel args, and append the old content after the args are bound. I have also considered passing and implicit args vector to exec_context::bind to make the trick more visible. Turning workdim into a proper arg in _args does not work either, because it is not present in module args. any thoughts? thanks, jan Passes get-work-dim piglit on turks without any regression, I have not tested SI as I don't have the hw. jan Jan Vesely (3): gallium: Pass input data size to launch_grid clover: Add work dimension implicit param to input r600,radeonsi: Copy implicit args provided by clover src/gallium/drivers/ilo/ilo_gpgpu.c | 2 +- src/gallium/drivers/nouveau/nvc0/nvc0_compute.c | 2 +- src/gallium/drivers/nouveau/nvc0/nvc0_context.h | 4 +- src/gallium/drivers/nouveau/nvc0/nve4_compute.c | 2 +- src/gallium/drivers/r600/evergreen_compute.c | 14 +- src/gallium/drivers/r600/evergreen_compute.h | 1 - src/gallium/drivers/radeonsi/si_compute.c | 6 +- src/gallium/include/pipe/p_context.h | 2 +- src/gallium/state_trackers/clover/core/kernel.cpp | 162 -- src/gallium/tests/trivial/compute.c | 40 +++--- 10 files changed, 122 insertions(+), 113 deletions(-) -- 1.9.3 -- Jan Vesely jan.ves...@rutgers.edu signature.asc Description: This is a digitally signed message part ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] configure: check for dladdr via AC_CHECK_FUNC/AC_CHECK_LIB
On 08/08/14 20:09, Jon TURNEY wrote: On 08/08/2014 18:58, Ilia Mirkin wrote: The problem is that AC_CHECK_FUNCS would stick the LDFLAGS before the conftest.c arg while AC_CHECK_LIB sticks the -ldl after conftest.c. This apparently matters with newer gcc's. There's probably some The key difference seems to be that lto was enabled, which I guess means we can't get away with listing objects in a random order :-) correct autoconf way of dealing with it, but... this works :) I think I have used LDFLAGS here where I should have used LIBS. So the more correct way is something like: Ouch... this seems like a trivial typo which we could have been spotted during review. To make it even more inspiring I've pushed Pali's version :\ Feel free to revert and/or commit this patch. -Emil diff --git a/configure.ac b/configure.ac index 96a67a3..bba64a0 100644 --- a/configure.ac +++ b/configure.ac @@ -535,9 +535,10 @@ AC_CHECK_FUNC([dlopen], [DEFINES=$DEFINES -DHAVE_DLOPEN], AC_SUBST([DLOPEN_LIBS]) dnl Check if that library also has dladdr -AC_CHECK_FUNC([dladdr], [DEFINES=$DEFINES -DHAVE_DLADDR], -[AC_CHECK_LIB([dl], [dladdr], - [DEFINES=$DEFINES -DHAVE_DLADDR])]) +save_LIBS=$LIBS +LIBS=$LIBS $DLOPEN_LIBS +AC_CHECK_FUNCS([dladdr]) +LIBS=$save_LIBS case $host_os in darwin*|mingw*) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965: Use memcpy instead of an assignment when storing uniforms
The i965 driver uses a float pointer to point to the value of a uniform and
also as the destination within the batch buffer. However the same locations
can also be used to store values for integer uniforms. Previously the value
was being copied into the batch buffer with a regular assignment. This breaks
if the compiler does this by loading and saving through an x87 register
because the fst instruction tries to fix up invalid float values. That can
corrupt some specific integer values. This patch changes it to use a memcpy
instead so that it won't use a floating-point register.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81150
---
src/mesa/drivers/dri/i965/gen6_vs_state.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/gen6_vs_state.c
b/src/mesa/drivers/dri/i965/gen6_vs_state.c
index 905e123..cdbc083 100644
--- a/src/mesa/drivers/dri/i965/gen6_vs_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_vs_state.c
@@ -81,7 +81,13 @@ gen6_upload_push_constants(struct brw_context *brw,
* wouldn't be set for them.
*/
for (i = 0; i prog_data-nr_params; i++) {
- param[i] = *prog_data-param[i];
+ /* A memcpy is used here instead of a regular assignment because
+ * otherwise the value may end up being copied through a
+ * floating-point register. This will break if the x87 registers are
+ * used and we are storing an integer value here because the fst
+ * instruction tries to fix up invalid values and that would corrupt
+ * an integer value */
+ memcpy(param + i, prog_data-param[i], sizeof param[i]);
}
if (0) {
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 0/12] Add support for BPTC texture compression
Patches 1, 2, 3, and 5 though 12 are Reviewed-by: Ian Romanick ian.d.roman...@intel.com Admittedly, I wasn't exceptionally thorogh with patches 6 and 7. I'm not worried about 6 getting a lot of use, and I expect we'll revisit 7 in the not too distant future (per previous discussions about the compressor). On 08/06/2014 09:27 AM, Neil Roberts wrote: Here is a v2 of the BPTC texture compression series. The main difference is that instead of going via DXT3 for the UNORM formats it now always uses the custom naïve compressor for all formats. This doesn't give very good-looking results but it is fast and doesn't add any dependencies. There was some discussion about alternative approaches on the list here: http://lists.freedesktop.org/archives/mesa-dev/2014-July/064436.html I didn't manage to get any consensus on whether this approach is the right thing to do so I thought I would just post the patches and see what happens. The other changes are: • The patches are rebased on top of Jason Ekstrand's texstore changes. This required some modification to format_info.py. • Added a patch to make glGenerateMipmap work with the BPTC formats. • Added a patch to make the meta implementation of glGetTexImage work with the two floating-point formats. • Added the formats to some format query functions that were missed. (There are a lot of switches for formats spread around Mesa!) • Fixed setting the alpha component to 1.0 when fetching from the RGB half-float formats. • Fixed fetching the alpha component from sRGB formats. • Fixed the quantization step for the half-float compressor. • Fixed a typo causing a bug in the compressor for textures with a width that isn't a multiple of four. The patches are also available on Github here: https://github.com/bpeel/mesa/commits/wip/bptc There are piglit tests for BPTC in a branch here: https://github.com/bpeel/piglit/commits/wip/bptc - Neil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Use memcpy instead of an assignment when storing uniforms
On 08/08/2014 01:56 PM, Neil Roberts wrote:
The i965 driver uses a float pointer to point to the value of a uniform and
also as the destination within the batch buffer. However the same locations
can also be used to store values for integer uniforms. Previously the value
was being copied into the batch buffer with a regular assignment. This breaks
if the compiler does this by loading and saving through an x87 register
because the fst instruction tries to fix up invalid float values. That can
corrupt some specific integer values. This patch changes it to use a memcpy
instead so that it won't use a floating-point register.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81150
There was a lot of chatter about this on IRC. Glad it was resolved!
Reviewed-by: Ian Romanick ian.d.roman...@intel.com
If this code also exists on the 10.2 branch, this should be tagged
Cc: mesa-sta...@lists.freedesktop.org
---
src/mesa/drivers/dri/i965/gen6_vs_state.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/gen6_vs_state.c
b/src/mesa/drivers/dri/i965/gen6_vs_state.c
index 905e123..cdbc083 100644
--- a/src/mesa/drivers/dri/i965/gen6_vs_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_vs_state.c
@@ -81,7 +81,13 @@ gen6_upload_push_constants(struct brw_context *brw,
* wouldn't be set for them.
*/
for (i = 0; i prog_data-nr_params; i++) {
- param[i] = *prog_data-param[i];
+ /* A memcpy is used here instead of a regular assignment because
+ * otherwise the value may end up being copied through a
+ * floating-point register. This will break if the x87 registers are
+ * used and we are storing an integer value here because the fst
+ * instruction tries to fix up invalid values and that would corrupt
+ * an integer value */
+ memcpy(param + i, prog_data-param[i], sizeof param[i]);
}
if (0) {
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] mesa: Fix glDrawBuffer/glDrawBuffers logic in _mesa_drawbuffer.
I've come up with a simpler solution. Patch coming soon.
-Brian
On 07/25/2014 04:00 AM, Popov, Pavel E wrote:
Hi Brian,
Could you commit the updated patch if it's ok?
Regards,
Pavel
-Original Message-
From: Popov, Pavel E
Sent: Wednesday, July 23, 2014 4:58 PM
To: mesa-dev@lists.freedesktop.org
Cc: Popov, Pavel E
Subject: [PATCH v2] mesa: Fix glDrawBuffer/glDrawBuffers logic in
_mesa_drawbuffer.
Piglit test 'gl30basic' fails on Debug Mesa with the assert:
'main/buffers.c:520: _mesa_drawbuffers:
Assertion `__builtin_popcount(destMask[buf]) == 1' failed.'.
According to spec (OpenGL 4.0 specification, pages 254-255) we have a
different bits set for one buffer and for multiple buffers. For
glDrawBuffer we may have up to four bits set but for glDrawBuffers we
can only have one bit set.
The _mesa_drawbuffers is called with ctx-Const.MaxDrawBuffers and NULL
arguments when _mesa_update_framebuffer or _mesa_update_draw_buffers
is called. In this case glDrawBuffers is always used if MaxDrawBuffers 1.
But glDrawBuffer has to be used instead of glDrawBuffers if only
destMask[0] is set.
v2 (Brian Paul): Only 0th entry requires special validation for (m == 1).
Signed-off-by: Pavel Popov pavel.e.po...@intel.com
---
src/mesa/main/buffers.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/mesa/main/buffers.c b/src/mesa/main/buffers.c index
b13a7af..95a8722 100644
--- a/src/mesa/main/buffers.c
+++ b/src/mesa/main/buffers.c
@@ -480,6 +480,7 @@ _mesa_drawbuffers(struct gl_context *ctx, GLuint n, const
GLenum *buffers,
struct gl_framebuffer *fb = ctx-DrawBuffer;
GLbitfield mask[MAX_DRAW_BUFFERS];
GLuint buf;
+ GLuint m = n;
if (!destMask) {
/* compute destMask values now */ @@ -489,15 +490,17 @@
_mesa_drawbuffers(struct gl_context *ctx, GLuint n, const GLenum *buffers,
mask[output] = draw_buffer_enum_to_bitmask(ctx, buffers[output]);
ASSERT(mask[output] != BAD_MASK);
mask[output] = supportedMask;
+ if (mask[output] == 0)
+m--;
}
destMask = mask;
}
/*
-* If n==1, destMask[0] may have up to four bits set.
+* If m==1, destMask[0] may have up to four bits set.
* Otherwise, destMask[x] can only have one bit set.
*/
- if (n == 1) {
+ if (m == 1 destMask[0]) {
GLuint count = 0, destMask0 = destMask[0];
while (destMask0) {
GLint bufIndex = ffs(destMask0) - 1;
--
1.9.1
Closed Joint Stock Company Intel A/O
Registered legal address: Krylatsky Hills Business Park,
17 Krylatskaya Str., Bldg 4, Moscow 121614,
Russian Federation
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Support the allow_glsl_extension_directive_midshader option.
On 08/08/2014 01:06 AM, Kenneth Graunke wrote: This adds support for Marek's new driconf parameter, which avoids totally white rendering in Unigine Valley (which attempts to enable the GL_ARB_sample_shading extension in an illegal place). Signed-off-by: Kenneth Graunke kenn...@whitecape.org Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75664 Reviewed-by: Ian Romanick ian.d.roman...@intel.com --- src/mesa/drivers/dri/i965/brw_context.c | 3 +++ src/mesa/drivers/dri/i965/intel_screen.c | 1 + 2 files changed, 4 insertions(+) diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c index 52f2557..402d936 100644 --- a/src/mesa/drivers/dri/i965/brw_context.c +++ b/src/mesa/drivers/dri/i965/brw_context.c @@ -570,6 +570,9 @@ brw_process_driconf_options(struct brw_context *brw) ctx-Const.DisableGLSLLineContinuations = driQueryOptionb(options, disable_glsl_line_continuations); + + ctx-Const.AllowGLSLExtensionDirectiveMidShader = + driQueryOptionb(options, allow_glsl_extension_directive_midshader); } GLboolean diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c index 5ebf1d5..ea0fc58 100644 --- a/src/mesa/drivers/dri/i965/intel_screen.c +++ b/src/mesa/drivers/dri/i965/intel_screen.c @@ -84,6 +84,7 @@ DRI_CONF_BEGIN DRI_CONF_FORCE_GLSL_EXTENSIONS_WARN(false) DRI_CONF_DISABLE_GLSL_LINE_CONTINUATIONS(false) DRI_CONF_DISABLE_BLEND_FUNC_EXTENDED(false) + DRI_CONF_ALLOW_GLSL_EXTENSION_DIRECTIVE_MIDSHADER(false) DRI_CONF_OPT_BEGIN_B(shader_precompile, true) DRI_CONF_DESC(en, Perform code generation at shader link time.) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/8] st/mesa: use PRIu64 for printing 64-bit uints
---
src/mesa/state_tracker/st_cb_bufferobjects.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/mesa/state_tracker/st_cb_bufferobjects.c
b/src/mesa/state_tracker/st_cb_bufferobjects.c
index 3b4d28d..4143dff 100644
--- a/src/mesa/state_tracker/st_cb_bufferobjects.c
+++ b/src/mesa/state_tracker/st_cb_bufferobjects.c
@@ -31,6 +31,8 @@
*/
+#include inttypes.h /* for PRIu64 macro */
+
#include main/imports.h
#include main/mtypes.h
#include main/arrayobj.h
@@ -271,7 +273,8 @@ st_bufferobj_data(struct gl_context *ctx,
pipe_resource_reference( st_obj-buffer, NULL );
if (ST_DEBUG DEBUG_BUFFER) {
- debug_printf(Create buffer size %td bind 0x%x\n, size, bind);
+ debug_printf(Create buffer size % PRIu64 bind 0x%x\n,
+ (uint64_t) size, bind);
}
if (size != 0) {
--
1.7.10.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/8] mesa: use PRIu64 for printing 64-bit uints
Silences MinGW warnings:
warning: unknown conversion type character ‘l’ in format [-Wformat]
warning: too many arguments for format [-Wformat-extra-args]
---
src/mesa/main/bufferobj.c | 33 +
src/mesa/main/varray.c| 13 -
2 files changed, 25 insertions(+), 21 deletions(-)
diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
index 1dfcda3..d59e63c 100644
--- a/src/mesa/main/bufferobj.c
+++ b/src/mesa/main/bufferobj.c
@@ -31,6 +31,7 @@
*/
#include stdbool.h
+#include inttypes.h /* for PRIu64 macro */
#include glheader.h
#include enums.h
#include hash.h
@@ -2824,8 +2825,8 @@ bind_buffers_check_offset_and_size(struct gl_context *ctx,
* value in offsets is less than zero (per binding).
*/
_mesa_error(ctx, GL_INVALID_VALUE,
- glBindBuffersRange(offsets[%u]=%lld 0),
- index, (long long int) offsets[index]);
+ glBindBuffersRange(offsets[%u]=% PRIu64 0),
+ index, (uint64_t) offsets[index]);
return false;
}
@@ -2836,8 +2837,8 @@ bind_buffers_check_offset_and_size(struct gl_context *ctx,
* value in sizes is less than or equal to zero (per binding).
*/
_mesa_error(ctx, GL_INVALID_VALUE,
- glBindBuffersRange(sizes[%u]=%lld = 0),
- index, (long long int) sizes[index]);
+ glBindBuffersRange(sizes[%u]=% PRIu64 = 0),
+ index, (uint64_t) sizes[index]);
return false;
}
@@ -3032,11 +3033,11 @@ bind_uniform_buffers_range(struct gl_context *ctx,
GLuint first, GLsizei count,
*/
if (offsets[i] (ctx-Const.UniformBufferOffsetAlignment - 1)) {
_mesa_error(ctx, GL_INVALID_VALUE,
- glBindBuffersRange(offsets[%u]=%lld is misaligned;
- it must be a multiple of the value of
+ glBindBuffersRange(offsets[%u]=% PRIu64
+ is misaligned; it must be a multiple of the value of
GL_UNIFORM_BUFFER_OFFSET_ALIGNMENT=%u when
target=GL_UNIFORM_BUFFER),
- i, (long long int) offsets[i],
+ i, (uint64_t) offsets[i],
ctx-Const.UniformBufferOffsetAlignment);
continue;
}
@@ -3270,19 +3271,19 @@ bind_xfb_buffers_range(struct gl_context *ctx,
*/
if (offsets[i] 0x3) {
_mesa_error(ctx, GL_INVALID_VALUE,
- glBindBuffersRange(offsets[%u]=%lld is misaligned;
- it must be a multiple of 4 when
+ glBindBuffersRange(offsets[%u]=% PRIu64
+ is misaligned; it must be a multiple of 4 when
target=GL_TRANSFORM_FEEDBACK_BUFFER),
- i, (long long int) offsets[i]);
+ i, (uint64_t) offsets[i]);
continue;
}
if (sizes[i] 0x3) {
_mesa_error(ctx, GL_INVALID_VALUE,
- glBindBuffersRange(sizes[%u]=%lld is misaligned;
- it must be a multiple of 4 when
+ glBindBuffersRange(sizes[%u]=% PRIu64
+ is misaligned; it must be a multiple of 4 when
target=GL_TRANSFORM_FEEDBACK_BUFFER),
- i, (long long int) sizes[i]);
+ i, (uint64_t) sizes[i]);
continue;
}
@@ -3488,10 +3489,10 @@ bind_atomic_buffers_range(struct gl_context *ctx,
*/
if (offsets[i] (ATOMIC_COUNTER_SIZE - 1)) {
_mesa_error(ctx, GL_INVALID_VALUE,
- glBindBuffersRange(offsets[%u]=%lld is misaligned;
- it must be a multiple of %d when
+ glBindBuffersRange(offsets[%u]=% PRIu64
+ is misaligned; it must be a multiple of %d when
target=GL_ATOMIC_COUNTER_BUFFER),
- i, (long long int) offsets[i], ATOMIC_COUNTER_SIZE);
+ i, (uint64_t) offsets[i], ATOMIC_COUNTER_SIZE);
continue;
}
diff --git a/src/mesa/main/varray.c b/src/mesa/main/varray.c
index 1454449..d1032d3 100644
--- a/src/mesa/main/varray.c
+++ b/src/mesa/main/varray.c
@@ -24,6 +24,8 @@
*/
+#include inttypes.h /* for PRIu64 macro */
+
#include glheader.h
#include imports.h
#include bufferobj.h
@@ -1424,7 +1426,8 @@ _mesa_BindVertexBuffer(GLuint bindingIndex, GLuint
buffer, GLintptr offset,
*/
if (offset 0) {
_mesa_error(ctx, GL_INVALID_VALUE,
- glBindVertexBuffer(offset=%lld 0), (long long)offset);
+ glBindVertexBuffer(offset=% PRIu64 0),
+ (uint64_t) offset);
return;
}
@@ -1550,15 +1553,15 @@ _mesa_BindVertexBuffers(GLuint first, GLsizei count,
const GLuint *buffers,
*/
if (offsets[i] 0) {
[Mesa-dev] [PATCH 5/8] mesa: simplify/rename _mesa_init_program_struct()
No need to return a value. Remove unused ctx parameter. Remove
_mesa_ prefix since it's static.
---
src/mesa/program/program.c | 69 ++--
1 file changed, 35 insertions(+), 34 deletions(-)
diff --git a/src/mesa/program/program.c b/src/mesa/program/program.c
index aedce3e..9886b75 100644
--- a/src/mesa/program/program.c
+++ b/src/mesa/program/program.c
@@ -226,27 +226,24 @@ _mesa_find_line_column(const GLubyte *string, const
GLubyte *pos,
/**
- * Initialize a new vertex/fragment program object.
+ * Initialize a new gl_program object.
*/
-static struct gl_program *
-_mesa_init_program_struct( struct gl_context *ctx, struct gl_program *prog,
- GLenum target, GLuint id)
+static void
+init_program_struct(struct gl_program *prog, GLenum target, GLuint id)
{
- (void) ctx;
- if (prog) {
- GLuint i;
- memset(prog, 0, sizeof(*prog));
- prog-Id = id;
- prog-Target = target;
- prog-RefCount = 1;
- prog-Format = GL_PROGRAM_FORMAT_ASCII_ARB;
-
- /* default mapping from samplers to texture units */
- for (i = 0; i MAX_SAMPLERS; i++)
- prog-SamplerUnits[i] = i;
- }
+ GLuint i;
- return prog;
+ assert(prog);
+
+ memset(prog, 0, sizeof(*prog));
+ prog-Id = id;
+ prog-Target = target;
+ prog-RefCount = 1;
+ prog-Format = GL_PROGRAM_FORMAT_ASCII_ARB;
+
+ /* default mapping from samplers to texture units */
+ for (i = 0; i MAX_SAMPLERS; i++)
+ prog-SamplerUnits[i] = i;
}
@@ -257,10 +254,11 @@ struct gl_program *
_mesa_init_fragment_program( struct gl_context *ctx, struct
gl_fragment_program *prog,
GLenum target, GLuint id)
{
- if (prog)
- return _mesa_init_program_struct( ctx, prog-Base, target, id );
- else
- return NULL;
+ if (prog) {
+ init_program_struct(prog-Base, target, id);
+ return prog-Base;
+ }
+ return NULL;
}
@@ -271,10 +269,11 @@ struct gl_program *
_mesa_init_vertex_program( struct gl_context *ctx, struct gl_vertex_program
*prog,
GLenum target, GLuint id)
{
- if (prog)
- return _mesa_init_program_struct( ctx, prog-Base, target, id );
- else
- return NULL;
+ if (prog) {
+ init_program_struct(prog-Base, target, id);
+ return prog-Base;
+ }
+ return NULL;
}
@@ -286,10 +285,11 @@ _mesa_init_compute_program(struct gl_context *ctx,
struct gl_compute_program *prog, GLenum target,
GLuint id)
{
- if (prog)
- return _mesa_init_program_struct( ctx, prog-Base, target, id );
- else
- return NULL;
+ if (prog) {
+ init_program_struct(prog-Base, target, id);
+ return prog-Base;
+ }
+ return NULL;
}
@@ -300,10 +300,11 @@ struct gl_program *
_mesa_init_geometry_program( struct gl_context *ctx, struct
gl_geometry_program *prog,
GLenum target, GLuint id)
{
- if (prog)
- return _mesa_init_program_struct( ctx, prog-Base, target, id );
- else
- return NULL;
+ if (prog) {
+ init_program_struct(prog-Base, target, id);
+ return prog-Base;
+ }
+ return NULL;
}
--
1.7.10.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 6/8] mesa: whitespace, 80-column wrapping in program.c
---
src/mesa/program/program.c | 19 +++
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/src/mesa/program/program.c b/src/mesa/program/program.c
index 9886b75..ef5bf6b 100644
--- a/src/mesa/program/program.c
+++ b/src/mesa/program/program.c
@@ -251,8 +251,9 @@ init_program_struct(struct gl_program *prog, GLenum target,
GLuint id)
* Initialize a new fragment program object.
*/
struct gl_program *
-_mesa_init_fragment_program( struct gl_context *ctx, struct
gl_fragment_program *prog,
- GLenum target, GLuint id)
+_mesa_init_fragment_program(struct gl_context *ctx,
+struct gl_fragment_program *prog,
+GLenum target, GLuint id)
{
if (prog) {
init_program_struct(prog-Base, target, id);
@@ -266,8 +267,9 @@ _mesa_init_fragment_program( struct gl_context *ctx, struct
gl_fragment_program
* Initialize a new vertex program object.
*/
struct gl_program *
-_mesa_init_vertex_program( struct gl_context *ctx, struct gl_vertex_program
*prog,
- GLenum target, GLuint id)
+_mesa_init_vertex_program(struct gl_context *ctx,
+ struct gl_vertex_program *prog,
+ GLenum target, GLuint id)
{
if (prog) {
init_program_struct(prog-Base, target, id);
@@ -282,8 +284,8 @@ _mesa_init_vertex_program( struct gl_context *ctx, struct
gl_vertex_program *pro
*/
struct gl_program *
_mesa_init_compute_program(struct gl_context *ctx,
- struct gl_compute_program *prog, GLenum target,
- GLuint id)
+ struct gl_compute_program *prog,
+ GLenum target, GLuint id)
{
if (prog) {
init_program_struct(prog-Base, target, id);
@@ -297,8 +299,9 @@ _mesa_init_compute_program(struct gl_context *ctx,
* Initialize a new geometry program object.
*/
struct gl_program *
-_mesa_init_geometry_program( struct gl_context *ctx, struct
gl_geometry_program *prog,
- GLenum target, GLuint id)
+_mesa_init_geometry_program(struct gl_context *ctx,
+struct gl_geometry_program *prog,
+GLenum target, GLuint id)
{
if (prog) {
init_program_struct(prog-Base, target, id);
--
1.7.10.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/8] mesa: define and use ALL_TYPE_BITS in varray.c code
---
src/mesa/main/varray.c | 33 +
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/src/mesa/main/varray.c b/src/mesa/main/varray.c
index 0356858..1454449 100644
--- a/src/mesa/main/varray.c
+++ b/src/mesa/main/varray.c
@@ -46,21 +46,22 @@
/** Used to indicate which GL datatypes are accepted by each of the
* glVertex/Color/Attrib/EtcPointer() functions.
*/
-#define BOOL_BIT 0x1
-#define BYTE_BIT 0x2
-#define UNSIGNED_BYTE_BIT0x4
-#define SHORT_BIT0x8
-#define UNSIGNED_SHORT_BIT 0x10
-#define INT_BIT 0x20
-#define UNSIGNED_INT_BIT 0x40
-#define HALF_BIT 0x80
-#define FLOAT_BIT0x100
-#define DOUBLE_BIT 0x200
-#define FIXED_ES_BIT 0x400
-#define FIXED_GL_BIT 0x800
-#define UNSIGNED_INT_2_10_10_10_REV_BIT 0x1000
-#define INT_2_10_10_10_REV_BIT 0x2000
-#define UNSIGNED_INT_10F_11F_11F_REV_BIT 0x4000
+#define BOOL_BIT (1 0)
+#define BYTE_BIT (1 1)
+#define UNSIGNED_BYTE_BIT (1 2)
+#define SHORT_BIT (1 3)
+#define UNSIGNED_SHORT_BIT(1 4)
+#define INT_BIT (1 5)
+#define UNSIGNED_INT_BIT (1 6)
+#define HALF_BIT (1 7)
+#define FLOAT_BIT (1 8)
+#define DOUBLE_BIT(1 9)
+#define FIXED_ES_BIT (1 10)
+#define FIXED_GL_BIT (1 11)
+#define UNSIGNED_INT_2_10_10_10_REV_BIT (1 12)
+#define INT_2_10_10_10_REV_BIT(1 13)
+#define UNSIGNED_INT_10F_11F_11F_REV_BIT (1 14)
+#define ALL_TYPE_BITS((1 15) - 1)
/** Convert GL datatype enum into a type_BIT value seen above */
@@ -185,7 +186,7 @@ vertex_binding_divisor(struct gl_context *ctx, GLuint
bindingIndex,
static GLbitfield
get_legal_types_mask(const struct gl_context *ctx)
{
- GLbitfield legalTypesMask = ~0u; /* all */
+ GLbitfield legalTypesMask = ALL_TYPE_BITS;
if (_mesa_is_gles(ctx)) {
legalTypesMask = ~(FIXED_GL_BIT |
--
1.7.10.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 7/8] mesa: fix assertion in _mesa_drawbuffers()
Fixes failed assertion when _mesa_update_draw_buffers() was called
with GL_DRAW_BUFFER == GL_FRONT_AND_BACK. The piglit gl30basic hit
this.
Cc: 10.2 mesa-sta...@lists.freedesktop.org
---
src/mesa/main/buffers.c |5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/mesa/main/buffers.c b/src/mesa/main/buffers.c
index b13a7af..6b4fac9 100644
--- a/src/mesa/main/buffers.c
+++ b/src/mesa/main/buffers.c
@@ -494,10 +494,11 @@ _mesa_drawbuffers(struct gl_context *ctx, GLuint n, const
GLenum *buffers,
}
/*
-* If n==1, destMask[0] may have up to four bits set.
+* destMask[0] may have up to four bits set
+* (ex: glDrawBuffer(GL_FRONT_AND_BACK)).
* Otherwise, destMask[x] can only have one bit set.
*/
- if (n == 1) {
+ if (_mesa_bitcount(destMask[0]) 1) {
GLuint count = 0, destMask0 = destMask[0];
while (destMask0) {
GLint bufIndex = ffs(destMask0) - 1;
--
1.7.10.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 8/8] mesa: simplify _mesa_update_draw_buffers()
There's no need to copy the array of DrawBuffer enums to a temp array.
---
src/mesa/main/buffers.c |9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/src/mesa/main/buffers.c b/src/mesa/main/buffers.c
index 6b4fac9..140cf6e 100644
--- a/src/mesa/main/buffers.c
+++ b/src/mesa/main/buffers.c
@@ -567,16 +567,11 @@ _mesa_drawbuffers(struct gl_context *ctx, GLuint n, const
GLenum *buffers,
void
_mesa_update_draw_buffers(struct gl_context *ctx)
{
- GLenum buffers[MAX_DRAW_BUFFERS];
- GLuint i;
-
/* should be a window system FBO */
assert(_mesa_is_winsys_fbo(ctx-DrawBuffer));
- for (i = 0; i ctx-Const.MaxDrawBuffers; i++)
- buffers[i] = ctx-Color.DrawBuffer[i];
-
- _mesa_drawbuffers(ctx, ctx-Const.MaxDrawBuffers, buffers, NULL);
+ _mesa_drawbuffers(ctx, ctx-Const.MaxDrawBuffers,
+ ctx-Color.DrawBuffer, NULL);
}
--
1.7.10.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/8] mesa: add comment that GL_CLIP_DISTANCE0 == GL_CLIP_PLANE0 in enable.c
--- src/mesa/main/enable.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mesa/main/enable.c b/src/mesa/main/enable.c index 0f3bcf0..417548a 100644 --- a/src/mesa/main/enable.c +++ b/src/mesa/main/enable.c @@ -313,7 +313,7 @@ _mesa_set_enable(struct gl_context *ctx, GLenum cap, GLboolean state) } } break; - case GL_CLIP_DISTANCE0: + case GL_CLIP_DISTANCE0: /* aka GL_CLIP_PLANE0 */ case GL_CLIP_DISTANCE1: case GL_CLIP_DISTANCE2: case GL_CLIP_DISTANCE3: @@ -1202,7 +1202,7 @@ _mesa_IsEnabled( GLenum cap ) return ctx-Eval.AutoNormal; case GL_BLEND: return ctx-Color.BlendEnabled 1; /* return state for buffer[0] */ - case GL_CLIP_DISTANCE0: + case GL_CLIP_DISTANCE0: /* aka GL_CLIP_PLANE0 */ case GL_CLIP_DISTANCE1: case GL_CLIP_DISTANCE2: case GL_CLIP_DISTANCE3: -- 1.7.10.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Use memcpy instead of an assignment when storing uniforms
Am 08.08.2014 22:56, schrieb Neil Roberts:
The i965 driver uses a float pointer to point to the value of a uniform and
also as the destination within the batch buffer. However the same locations
can also be used to store values for integer uniforms. Previously the value
was being copied into the batch buffer with a regular assignment. This breaks
if the compiler does this by loading and saving through an x87 register
because the fst instruction tries to fix up invalid float values. That can
corrupt some specific integer values. This patch changes it to use a memcpy
instead so that it won't use a floating-point register.
Bugzilla:
https://urldefense.proofpoint.com/v1/url?u=https://bugs.freedesktop.org/show_bug.cgi?id%3D81150k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0Ar=F4msKE2WxRzA%2BwN%2B25muztFm5TSPwE8HKJfWfR2NgfY%3D%0Am=5ZyizVpb8bGGN4qANo378HB6GFoFoBABoDXdPAyIszE%3D%0As=107a04b60233e66abfb9e83a62b90d128d60d6c61eb41b1a08ec7ab537283903
---
src/mesa/drivers/dri/i965/gen6_vs_state.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/gen6_vs_state.c
b/src/mesa/drivers/dri/i965/gen6_vs_state.c
index 905e123..cdbc083 100644
--- a/src/mesa/drivers/dri/i965/gen6_vs_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_vs_state.c
@@ -81,7 +81,13 @@ gen6_upload_push_constants(struct brw_context *brw,
* wouldn't be set for them.
*/
for (i = 0; i prog_data-nr_params; i++) {
- param[i] = *prog_data-param[i];
+ /* A memcpy is used here instead of a regular assignment because
+ * otherwise the value may end up being copied through a
+ * floating-point register. This will break if the x87 registers are
+ * used and we are storing an integer value here because the fst
+ * instruction tries to fix up invalid values and that would corrupt
+ * an integer value */
+ memcpy(param + i, prog_data-param[i], sizeof param[i]);
}
if (0) {
Wow, crazy.
Maybe it would make sense to just use a void pointer and do a single
memcpy instead of looping through all params?
I wonder why this isn't hit elsewhere, I'm pretty sure there's other
places (not necessarily in i965 driver) which assign potential int data
through floats. Seems to me the compiler is pretty crazy to use fst
though to begin with...
Roland
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Mesa (master): mesa/formats: Add layout and swizzle information
Am 08.08.2014 09:46, schrieb Kenneth Graunke: On Thursday, August 07, 2014 10:41:36 PM Roland Scheidegger wrote: Am 07.08.2014 20:25, schrieb Jason Ekstrand: Michel, On Thu, Aug 7, 2014 at 12:04 AM, Michel Dänzer mic...@daenzer.net mailto:mic...@daenzer.net wrote: On 07.08.2014 02:02, Jason Ekstrand wrote: Michael, Close, but no cigar. :) I'm sorry about that. I must have read too quickly. :-/ Could you please point me at the failing tests. spec/!OpenGL 1.1/depthstencil-default_fb-drawpixels-FLOAT-and-USHORT spec/!OpenGL 1.1/draw-pixels spec/!OpenGL 1.1/stencil-drawpixels spec/!OpenGL 1.4/copy-pixels spec/ARB_depth_buffer_float/fbo-depthstencil-GL_DEPTH32F_STENCIL8-drawpixels-FLOAT-and-USHORT spec/ARB_depth_buffer_float/fbo-stencil-GL_DEPTH32F_STENCIL8-drawpixels spec/EXT_framebuffer_object/fbo-stencil-GL_STENCIL_INDEX1-drawpixels spec/EXT_framebuffer_object/fbo-stencil-GL_STENCIL_INDEX16-drawpixels spec/EXT_framebuffer_object/fbo-stencil-GL_STENCIL_INDEX14-drawpixels spec/EXT_framebuffer_object/fbo-stencil-GL_STENCIL_INDEX8-drawpixels spec/EXT_packed_depth_stencil/fbo-depthstencil-GL_DEPTH24_STENCIL8-drawpixels-FLOAT-and-USHORT spec/EXT_packed_depth_stencil/fbo-stencil-GL_DEPTH24_STENCIL8-drawpixels (The total number of regressions is around 20 because some of these are run for several numbers of samples) I don't have a radeon, but I can run with llvmpipe or dri swrast and try to find the bug that way. At least the draw-pixels test indeed regressed with llvmpipe as well. The draw pixels regression on llvmpipe is different. The changes I made to texture upload included a subtle change in the way we handle signed input data. In older GL versions there were two formulas, one which mapped [-128, 127] to [-1, 1] and one which mapped [-127, 127] to [-1, 1]. The former formula was used when uploading a non-snorm texture from signed integer data or when doing operations such as DrawPixels and ReadPixels. In GL 4.3, this first formula is going away and we will only have the later formula. (The later formula has the advantage of mapping 0 to 0.) If we think it's needed, I can add code to the swizzle_and_convert function to be able to handle the legacy formula in those cases where older GL versions say that it's needed. Yeah the two different formulas in older GL versions are quite a pity. I'm not sure if we really need to honor the old formula, but I guess if binary drivers do we might be required to as well. The gallium util code though never did. Maybe should just make the tests more lenient... Roland I have a pretty strong preference to ditch the old formula entirely: 1. OpenGL will silently promote you to a 4.2 context if you ask for less than that, because it's deemed backwards compatible (even though it strictly isn't, due to things like this). Implementing both sets of rules means that GL4-class hardware will use one formula, and GL3-class hardware will use the other...on the same driver. That seems weird. Yes that sounds a bit odd. Still could use the old formula on compatibility context but I guess if you can get the new formula automatically it is possibly not really needed. 2. OpenGL ES 3.x strictly uses the new formula. 3. The new formula matches DirectX, so it's probably what ported applications (or a virtualization environment) would expect. 4. Intel hardware cannot honor the older GL formula (we'd have to do the conversion manually, which seems like a waste). My preference would be to make tests accept either formula. Sounds ok to me but I don't really have a strong opinion on that. It would certainly simplify things. Roland ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 01/13] mesa: Document SYSTEM_VALUE_VERTEX_ID and SYSTEM_VALUE_INSTANCE_ID
On 08/08/2014 07:55 AM, Roland Scheidegger wrote:
The mesa parts of the series all look good to me.
So.. can we put your R-b on patches 1 through 7? :)
We definitely want something like that in gallium too (draw fails the
vertex id tests sort of on purpose right now because we needed d3d10
behavior).
Oh and sort of off-topic but since you're familiar with it do you know
why the gl_PrimitiveIn of the geometry shader isn't a system value
whereas all the vertex_id and friends are?
I have come vague recollection that Paul had a good reason for doing
that, but no recollection of what that reason was.
Roland
Am 08.08.2014 09:31, schrieb Kenneth Graunke:
From: Ian Romanick ian.d.roman...@intel.com
v2: Additions to the documentation for SYSTEM_VALUE_VERTEX_ID. Quote
the GL_ARB_shader_draw_parameters spec and mention DirectX SV_VertexID.
Signed-off-by: Ian Romanick ian.d.roman...@intel.com
Reviewed-by: Kenneth Graunke kenn...@whitecape.org
---
src/mesa/main/mtypes.h | 57
++
1 file changed, 57 insertions(+)
This series is available as the 'basevertex-v9' branch of ~kwg/mesa
(not ~idr/mesa). Ken tested this series against Piglit on Haswell and
Broadwell, but did not test earlier hardware, nor run the ES3 tests.
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index ff130da..207be0a 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2055,7 +2055,64 @@ typedef enum
* \name Vertex shader system values
*/
/*@{*/
+ /**
+* OpenGL-style vertex ID.
+*
+* Section 2.11.7 (Shader Execution), subsection Shader Inputs, of the
+* OpenGL 3.3 core profile spec says:
+*
+* gl_VertexID holds the integer index i implicitly passed by
+* DrawArrays or one of the other drawing commands defined in section
+* 2.8.3.
+*
+* Section 2.8.3 (Drawing Commands) of the same spec says:
+*
+* The commandsare equivalent to the commands with the same base
+* name (without the BaseVertex suffix), except that the ith element
+* transferred by the corresponding draw call will be taken from
+* element indices[i] + basevertex of each enabled array.
+*
+* Additionally, the overview in the GL_ARB_shader_draw_parameters spec
+* says:
+*
+* In unextended GL, vertex shaders have inputs named gl_VertexID
and
+* gl_InstanceID, which contain, respectively the index of the vertex
+* and instance. The value of gl_VertexID is the implicitly passed
+* index of the vertex being processed, which includes the value of
+* baseVertex, for those commands that accept it.
+*
+* gl_VertexID gets basevertex added in. This differs from DirectX where
+* SV_VertexID does \b not get basevertex added in.
+*/
SYSTEM_VALUE_VERTEX_ID,
+
+ /**
+* Instanced ID as supplied to gl_InstanceID
+*
+* Values assigned to gl_InstanceID always begin with zero, regardless of
+* the value of baseinstance.
+*
+* Section 11.1.3.9 (Shader Inputs) of the OpenGL 4.4 core profile spec
+* says:
+*
+* gl_InstanceID holds the integer instance number of the current
+* primitive in an instanced draw call (see section 10.5).
+*
+* Through a big chain of pseudocode, section 10.5 describes that
+* baseinstance is not counted by gl_InstanceID. In that section, notice
+*
+* If an enabled vertex attribute array is instanced (it has a
+* non-zero divisor as specified by VertexAttribDivisor), the element
+* index that is transferred to the GL, for all vertices, is given by
+*
+* floor(instance/divisor) + baseinstance
+*
+* If an array corresponding to an attribute required by a vertex
+* shader is not enabled, then the corresponding element is taken
from
+* the current attribute state (see section 10.2).
+*
+* Note that baseinstance is \b not included in the value of instance.
+*/
SYSTEM_VALUE_INSTANCE_ID,
/*@}*/
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965/cse: Don't eliminate instructions with side-effects
In particular, this caused problems where atomics operations were getting
eliminated.
Signed-off-by: Jason Ekstrand jason.ekstr...@intel.com
---
src/mesa/drivers/dri/i965/brw_fs_cse.cpp | 3 ++-
src/mesa/drivers/dri/i965/brw_vec4_cse.cpp | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
b/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
index 63d87f9..8cfc6c6 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
@@ -177,7 +177,8 @@ fs_visitor::opt_cse_local(bblock_t *block)
foreach_inst_in_block(fs_inst, inst, block) {
/* Skip some cases. */
if (is_expression(inst) !inst-is_partial_write()
- (inst-dst.file != HW_REG || inst-dst.is_null()))
+ (inst-dst.file != HW_REG || inst-dst.is_null())
+ !inst-has_side_effects())
{
bool found = false;
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
b/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
index 29d2e02..44651b4 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
@@ -135,7 +135,8 @@ vec4_visitor::opt_cse_local(bblock_t *block)
foreach_inst_in_block (vec4_instruction, inst, block) {
/* Skip some cases. */
if (is_expression(inst) !inst-predicate inst-mlen == 0
- (inst-dst.file != HW_REG || inst-dst.is_null()))
+ (inst-dst.file != HW_REG || inst-dst.is_null())
+ !inst-has_side_effects())
{
bool found = false;
--
2.0.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 03/13] mesa: Add SYSTEM_VALUE_BASE_VERTEX
On 08/08/2014 11:31 AM, Roland Scheidegger wrote: Am 08.08.2014 20:06, schrieb Ian Romanick: On 08/08/2014 12:37 AM, Kenneth Graunke wrote: On Friday, August 08, 2014 12:31:07 AM Kenneth Graunke wrote: From: Ian Romanick ian.d.roman...@intel.com This system value represents the basevertex value passed to glDrawElementsBaseVertex and related functions. Signed-off-by: Ian Romanick ian.d.roman...@intel.com --- src/mesa/main/mtypes.h | 15 ++- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 1 + 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index c603007..99037c6 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -2084,7 +2084,12 @@ typedef enum * gl_VertexID gets basevertex added in. This differs from DirectX where * SV_VertexID does \b not get basevertex added in. * -* \sa SYSTEM_VALUE_VERTEX_ID_ZERO_BASE +* \note +* If all system values are available, \c SYSTEM_VALUE_VERTEX_ID will be +* equal to \c SYSTEM_VALUE_VERTEX_ID_ZERO_BASE plus +* \c SYSTEM_VALUE_BASE_VERTEX. +* +* \sa SYSTEM_VALUE_VERTEX_ID_ZERO_BASE, SYSTEM_VALUE_BASE_VERTEX */ SYSTEM_VALUE_VERTEX_ID, @@ -2126,6 +2131,14 @@ typedef enum * \sa SYSTEM_VALUE_VERTEX_ID, SYSTEM_VALUE_BASE_VERTEX */ SYSTEM_VALUE_VERTEX_ID_ZERO_BASE, + + /** +* Value of \c basevertex passed to \c glDrawElementsBaseVertex and similar +* functions. +* +* \sa SYSTEM_VALUE_VERTEX_ID, SYSTEM_VALUE_VERTEX_ID_ZERO_BASE +*/ + SYSTEM_VALUE_BASE_VERTEX, /*@}*/ Ian, It occurred to me that we're sort of abusing this system value in the i965 patches later in this series - we're using it to store gl_BaseVertexARB, but also using it to store the first parameter for glDrawArrays. I think in the glDrawArrays case, gl_BaseVertexARB is supposed to be 0. Yeah, you're right on all counts. I sent some commentary on patch 7. Short version... we'll have to do some rework to support GL_ARB_shader_draw_parameters, but I think it's safe to leave this code as-is until we implement that other extension. So, are you saying the spec really meant the first parameter of Draw calls does not count as baseVertex? That behavior would look very inconsistent (and useless) to me. I think that is correct. The idea is that gl_BaseVertex can be used to recover the DX-style VertexID from the GL-style gl_VertexID when used with glDrawElementsBaseVertex and friends. Does DX have anything like the start element for glDrawArrays? What does VertexID get in that case? I'll have to look and see if there's a conformance test. :) That will be the real answer. gl_VertexId has similar language floating around (gl_VertexID will have the base vertex applied to it) and that was resolved to include the first parameter of non-indexed draw too. The language there is a bit different. It says, basically, gl_VertexID is that value that would have been passed to glArrayElement. Roland ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/8] mesa: add comment that GL_CLIP_DISTANCE0 == GL_CLIP_PLANE0 in enable.c
Assuming patch 7 fixes the problem Pavel was seeing (and I'm not even 100% what problem that was), the series is Reviewed-by: Ian Romanick ian.d.roman...@intel.com On 08/08/2014 02:20 PM, Brian Paul wrote: --- src/mesa/main/enable.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mesa/main/enable.c b/src/mesa/main/enable.c index 0f3bcf0..417548a 100644 --- a/src/mesa/main/enable.c +++ b/src/mesa/main/enable.c @@ -313,7 +313,7 @@ _mesa_set_enable(struct gl_context *ctx, GLenum cap, GLboolean state) } } break; - case GL_CLIP_DISTANCE0: + case GL_CLIP_DISTANCE0: /* aka GL_CLIP_PLANE0 */ case GL_CLIP_DISTANCE1: case GL_CLIP_DISTANCE2: case GL_CLIP_DISTANCE3: @@ -1202,7 +1202,7 @@ _mesa_IsEnabled( GLenum cap ) return ctx-Eval.AutoNormal; case GL_BLEND: return ctx-Color.BlendEnabled 1; /* return state for buffer[0] */ - case GL_CLIP_DISTANCE0: + case GL_CLIP_DISTANCE0: /* aka GL_CLIP_PLANE0 */ case GL_CLIP_DISTANCE1: case GL_CLIP_DISTANCE2: case GL_CLIP_DISTANCE3: ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 7/8] mesa: fix assertion in _mesa_drawbuffers()
Pavel,
Does this patch fix the problem you were trying to solve?
On 08/08/2014 02:20 PM, Brian Paul wrote:
Fixes failed assertion when _mesa_update_draw_buffers() was called
with GL_DRAW_BUFFER == GL_FRONT_AND_BACK. The piglit gl30basic hit
this.
Cc: 10.2 mesa-sta...@lists.freedesktop.org
---
src/mesa/main/buffers.c |5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/mesa/main/buffers.c b/src/mesa/main/buffers.c
index b13a7af..6b4fac9 100644
--- a/src/mesa/main/buffers.c
+++ b/src/mesa/main/buffers.c
@@ -494,10 +494,11 @@ _mesa_drawbuffers(struct gl_context *ctx, GLuint n,
const GLenum *buffers,
}
/*
-* If n==1, destMask[0] may have up to four bits set.
+* destMask[0] may have up to four bits set
+* (ex: glDrawBuffer(GL_FRONT_AND_BACK)).
* Otherwise, destMask[x] can only have one bit set.
*/
- if (n == 1) {
+ if (_mesa_bitcount(destMask[0]) 1) {
GLuint count = 0, destMask0 = destMask[0];
while (destMask0) {
GLint bufIndex = ffs(destMask0) - 1;
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Use memcpy instead of an assignment when storing uniforms
Roland Scheidegger srol...@vmware.com writes:
Am 08.08.2014 22:56, schrieb Neil Roberts:
The i965 driver uses a float pointer to point to the value of a uniform and
also as the destination within the batch buffer. However the same locations
can also be used to store values for integer uniforms. Previously the value
was being copied into the batch buffer with a regular assignment. This breaks
if the compiler does this by loading and saving through an x87 register
because the fst instruction tries to fix up invalid float values. That can
corrupt some specific integer values. This patch changes it to use a memcpy
instead so that it won't use a floating-point register.
Bugzilla:
https://urldefense.proofpoint.com/v1/url?u=https://bugs.freedesktop.org/show_bug.cgi?id%3D81150k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0Ar=F4msKE2WxRzA%2BwN%2B25muztFm5TSPwE8HKJfWfR2NgfY%3D%0Am=5ZyizVpb8bGGN4qANo378HB6GFoFoBABoDXdPAyIszE%3D%0As=107a04b60233e66abfb9e83a62b90d128d60d6c61eb41b1a08ec7ab537283903
---
src/mesa/drivers/dri/i965/gen6_vs_state.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/gen6_vs_state.c
b/src/mesa/drivers/dri/i965/gen6_vs_state.c
index 905e123..cdbc083 100644
--- a/src/mesa/drivers/dri/i965/gen6_vs_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_vs_state.c
@@ -81,7 +81,13 @@ gen6_upload_push_constants(struct brw_context *brw,
* wouldn't be set for them.
*/
for (i = 0; i prog_data-nr_params; i++) {
- param[i] = *prog_data-param[i];
+ /* A memcpy is used here instead of a regular assignment because
+ * otherwise the value may end up being copied through a
+ * floating-point register. This will break if the x87 registers
are
+ * used and we are storing an integer value here because the fst
+ * instruction tries to fix up invalid values and that would
corrupt
+ * an integer value */
+ memcpy(param + i, prog_data-param[i], sizeof param[i]);
}
Or just change the pointer type to uint32_t, right?
Wow, crazy.
Maybe it would make sense to just use a void pointer and do a single
memcpy instead of looping through all params?
I wonder why this isn't hit elsewhere, I'm pretty sure there's other
places (not necessarily in i965 driver) which assign potential int data
through floats. Seems to me the compiler is pretty crazy to use fst
though to begin with...
Well, the uniforms aren't in a single linear allocation. You need to
gather them from their separate storage locations.
pgp2jarUSx2iW.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965/fs: don't read from uninitialized memory while assigning registers
Signed-off-by: Connor Abbott connor.abb...@intel.com
---
src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp
b/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp
index 3f27364..2233621 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp
@@ -56,9 +56,9 @@ fs_visitor::assign_regs_trivial()
foreach_in_list(fs_inst, inst, instructions) {
assign_reg(hw_reg_mapping, inst-dst, reg_width);
- assign_reg(hw_reg_mapping, inst-src[0], reg_width);
- assign_reg(hw_reg_mapping, inst-src[1], reg_width);
- assign_reg(hw_reg_mapping, inst-src[2], reg_width);
+ for (i = 0; i inst-sources; i++) {
+ assign_reg(hw_reg_mapping, inst-src[i], reg_width);
+ }
}
if (this-grf_used = max_grf) {
@@ -518,9 +518,9 @@ fs_visitor::assign_regs(bool allow_spilling)
foreach_in_list(fs_inst, inst, instructions) {
assign_reg(hw_reg_mapping, inst-dst, reg_width);
- assign_reg(hw_reg_mapping, inst-src[0], reg_width);
- assign_reg(hw_reg_mapping, inst-src[1], reg_width);
- assign_reg(hw_reg_mapping, inst-src[2], reg_width);
+ for (int i = 0; i inst-sources; i++) {
+ assign_reg(hw_reg_mapping, inst-src[i], reg_width);
+ }
}
ralloc_free(g);
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/fs: don't read from uninitialized memory while assigning registers
Weird that I didn't notice this before. Thanks! Reviewed-by: Matt Turner matts...@gmail.com ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 03/13] mesa: Add SYSTEM_VALUE_BASE_VERTEX
Am 08.08.2014 23:47, schrieb Ian Romanick: On 08/08/2014 11:31 AM, Roland Scheidegger wrote: Am 08.08.2014 20:06, schrieb Ian Romanick: On 08/08/2014 12:37 AM, Kenneth Graunke wrote: On Friday, August 08, 2014 12:31:07 AM Kenneth Graunke wrote: From: Ian Romanick ian.d.roman...@intel.com This system value represents the basevertex value passed to glDrawElementsBaseVertex and related functions. Signed-off-by: Ian Romanick ian.d.roman...@intel.com --- src/mesa/main/mtypes.h | 15 ++- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 1 + 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index c603007..99037c6 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -2084,7 +2084,12 @@ typedef enum * gl_VertexID gets basevertex added in. This differs from DirectX where * SV_VertexID does \b not get basevertex added in. * -* \sa SYSTEM_VALUE_VERTEX_ID_ZERO_BASE +* \note +* If all system values are available, \c SYSTEM_VALUE_VERTEX_ID will be +* equal to \c SYSTEM_VALUE_VERTEX_ID_ZERO_BASE plus +* \c SYSTEM_VALUE_BASE_VERTEX. +* +* \sa SYSTEM_VALUE_VERTEX_ID_ZERO_BASE, SYSTEM_VALUE_BASE_VERTEX */ SYSTEM_VALUE_VERTEX_ID, @@ -2126,6 +2131,14 @@ typedef enum * \sa SYSTEM_VALUE_VERTEX_ID, SYSTEM_VALUE_BASE_VERTEX */ SYSTEM_VALUE_VERTEX_ID_ZERO_BASE, + + /** +* Value of \c basevertex passed to \c glDrawElementsBaseVertex and similar +* functions. +* +* \sa SYSTEM_VALUE_VERTEX_ID, SYSTEM_VALUE_VERTEX_ID_ZERO_BASE +*/ + SYSTEM_VALUE_BASE_VERTEX, /*@}*/ Ian, It occurred to me that we're sort of abusing this system value in the i965 patches later in this series - we're using it to store gl_BaseVertexARB, but also using it to store the first parameter for glDrawArrays. I think in the glDrawArrays case, gl_BaseVertexARB is supposed to be 0. Yeah, you're right on all counts. I sent some commentary on patch 7. Short version... we'll have to do some rework to support GL_ARB_shader_draw_parameters, but I think it's safe to leave this code as-is until we implement that other extension. So, are you saying the spec really meant the first parameter of Draw calls does not count as baseVertex? That behavior would look very inconsistent (and useless) to me. I think that is correct. The idea is that gl_BaseVertex can be used to recover the DX-style VertexID from the GL-style gl_VertexID when used with glDrawElementsBaseVertex and friends. Does DX have anything like the start element for glDrawArrays? What does VertexID get in that case? Yes. It is called StartVertexLocation (the corresponding parameter for indexed calls is called BaseVertexLocation). And neither one is included in the vertex id system value (as opposed to GL where gl_VertexID includes first or basevertex values), always starting from 0. I'll have to look and see if there's a conformance test. :) That will be the real answer. Well if that's true I'd say that's a bug in the spec. The ARB_shader_draw_parameters mentions other APIs have different opinion on whether to include basevertex or not - thus imho implying gl_baseVertex being useful to translate from the other API. Which just won't work if it can't do that for non-indexed draw calls. gl_VertexId has similar language floating around (gl_VertexID will have the base vertex applied to it) and that was resolved to include the first parameter of non-indexed draw too. The language there is a bit different. It says, basically, gl_VertexID is that value that would have been passed to glArrayElement. That is true, but really treating the gl_BaseVertex differently still does not make sense to me. In practice and ignoring the arrayelement cruft, in d3d10, the vertex id just effectively is the value before the basevertex/startvertex is applied (to the index used for looking up vertex data) whereas in GL it's after. Whatever you call these values they are effectively the same for indexed and non-indexed calls, so ignoring it for gl_BaseVertex only for non-indexed calls but honoring it for gl_VertexID in the same situation is quite inconsistent imho. Roland ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] RFC: glsl/glcpp: Allow for '#' characters to appear in shader body
Erik Faye-Lund kusmab...@gmail.com writes: It seems to me like this is there to support non-ASCII identifiers and strings, but GLSL doesn't support either. I'm not able to come up with a conclusion here. That's almost always the case with GLSL. The GLSL specification does have its own section for character set, etc. but weasels out of carefully specifying how #define works and just vaguely points to standard C++ behavior, (without saying what version of what C++ standard it might be referring to). So for nit-picky details like this, there's no possibility to come up with anything very definitive about how GLSL should work from the specification alone. -Carl -- carl.d.wo...@intel.com pgp6QuntftmuA.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glxinfo/wglinfo: query/print GL_ARB_texture_multisample limits
---
src/xdemos/glinfo_common.c |5 +
1 file changed, 5 insertions(+)
diff --git a/src/xdemos/glinfo_common.c b/src/xdemos/glinfo_common.c
index f536e98..d3acc19 100644
--- a/src/xdemos/glinfo_common.c
+++ b/src/xdemos/glinfo_common.c
@@ -573,6 +573,11 @@ print_limits(const char *extensions, const char
*oglstring, int version,
{ 1, GL_TEXTURE_BUFFER_OFFSET_ALIGNMENT,
GL_TEXTURE_BUFFER_OFFSET_ALIGNMENT, GL_ARB_texture_buffer_object },
{ 1, GL_MAX_TEXTURE_BUFFER_SIZE, GL_MAX_TEXTURE_BUFFER_SIZE,
GL_ARB_texture_buffer_object },
#endif
+#if defined (GL_ARB_texture_multisample)
+ { 1, GL_MAX_COLOR_TEXTURE_SAMPLES, GL_MAX_COLOR_TEXTURE_SAMPLES,
GL_ARB_texture_multisample },
+ { 1, GL_MAX_DEPTH_TEXTURE_SAMPLES, GL_MAX_DEPTH_TEXTURE_SAMPLES,
GL_ARB_texture_multisample },
+ { 1, GL_MAX_INTEGER_SAMPLES, GL_MAX_INTEGER_SAMPLES,
GL_ARB_texture_multisample },
+#endif
#if defined (GL_ARB_uniform_buffer_object)
{ 1, GL_MAX_VERTEX_UNIFORM_BLOCKS, GL_MAX_VERTEX_UNIFORM_BLOCKS,
GL_ARB_uniform_buffer_object },
{ 1, GL_MAX_FRAGMENT_UNIFORM_BLOCKS, GL_MAX_FRAGMENT_UNIFORM_BLOCKS,
GL_ARB_uniform_buffer_object },
--
1.7.10.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 82327] FAIL: glcpp/tests/glcpp-test-cr-lf
https://bugs.freedesktop.org/show_bug.cgi?id=82327 Matt Turner matts...@gmail.com changed: What|Removed |Added QA Contact||mesa-dev@lists.freedesktop. ||org CC|cwo...@cworth.org | -- You are receiving this mail because: You are the QA Contact for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965/fs: set virtual_grf_count in assign_regs()
This lets us call dump_instructions() after register allocation without
failing an assertion.
This interacts trivially with my previous patch.
Signed-off-by: Connor Abbott connor.abb...@intel.com
---
src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp | 4
1 file changed, 4 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp
b/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp
index 2233621..d626271 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp
@@ -64,6 +64,8 @@ fs_visitor::assign_regs_trivial()
if (this-grf_used = max_grf) {
fail(Ran out of regs on trivial allocator (%d/%d)\n,
this-grf_used, max_grf);
+ } else {
+ this-virtual_grf_count = this-grf_used;
}
}
@@ -523,6 +525,8 @@ fs_visitor::assign_regs(bool allow_spilling)
}
}
+ this-virtual_grf_count = this-grf_used;
+
ralloc_free(g);
return true;
--
1.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 01/13] mesa: Document SYSTEM_VALUE_VERTEX_ID and SYSTEM_VALUE_INSTANCE_ID
Am 08.08.2014 23:38, schrieb Ian Romanick:
On 08/08/2014 07:55 AM, Roland Scheidegger wrote:
The mesa parts of the series all look good to me.
So.. can we put your R-b on patches 1 through 7? :)
Actually I'm not really qualified for the glsl parts in 4,5 but
1-3, 6, 7 are
Reviewed-by: Roland Scheidegger srol...@vmware.com
We definitely want something like that in gallium too (draw fails the
vertex id tests sort of on purpose right now because we needed d3d10
behavior).
Oh and sort of off-topic but since you're familiar with it do you know
why the gl_PrimitiveIn of the geometry shader isn't a system value
whereas all the vertex_id and friends are?
I have come vague recollection that Paul had a good reason for doing
that, but no recollection of what that reason was.
Hmm. Still feels wrong to me :-).
Roland
Roland
Am 08.08.2014 09:31, schrieb Kenneth Graunke:
From: Ian Romanick ian.d.roman...@intel.com
v2: Additions to the documentation for SYSTEM_VALUE_VERTEX_ID. Quote
the GL_ARB_shader_draw_parameters spec and mention DirectX SV_VertexID.
Signed-off-by: Ian Romanick ian.d.roman...@intel.com
Reviewed-by: Kenneth Graunke kenn...@whitecape.org
---
src/mesa/main/mtypes.h | 57
++
1 file changed, 57 insertions(+)
This series is available as the 'basevertex-v9' branch of ~kwg/mesa
(not ~idr/mesa). Ken tested this series against Piglit on Haswell and
Broadwell, but did not test earlier hardware, nor run the ES3 tests.
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index ff130da..207be0a 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2055,7 +2055,64 @@ typedef enum
* \name Vertex shader system values
*/
/*@{*/
+ /**
+* OpenGL-style vertex ID.
+*
+* Section 2.11.7 (Shader Execution), subsection Shader Inputs, of the
+* OpenGL 3.3 core profile spec says:
+*
+* gl_VertexID holds the integer index i implicitly passed by
+* DrawArrays or one of the other drawing commands defined in
section
+* 2.8.3.
+*
+* Section 2.8.3 (Drawing Commands) of the same spec says:
+*
+* The commandsare equivalent to the commands with the same
base
+* name (without the BaseVertex suffix), except that the ith element
+* transferred by the corresponding draw call will be taken from
+* element indices[i] + basevertex of each enabled array.
+*
+* Additionally, the overview in the GL_ARB_shader_draw_parameters spec
+* says:
+*
+* In unextended GL, vertex shaders have inputs named gl_VertexID
and
+* gl_InstanceID, which contain, respectively the index of the
vertex
+* and instance. The value of gl_VertexID is the implicitly passed
+* index of the vertex being processed, which includes the value of
+* baseVertex, for those commands that accept it.
+*
+* gl_VertexID gets basevertex added in. This differs from DirectX
where
+* SV_VertexID does \b not get basevertex added in.
+*/
SYSTEM_VALUE_VERTEX_ID,
+
+ /**
+* Instanced ID as supplied to gl_InstanceID
+*
+* Values assigned to gl_InstanceID always begin with zero, regardless
of
+* the value of baseinstance.
+*
+* Section 11.1.3.9 (Shader Inputs) of the OpenGL 4.4 core profile spec
+* says:
+*
+* gl_InstanceID holds the integer instance number of the current
+* primitive in an instanced draw call (see section 10.5).
+*
+* Through a big chain of pseudocode, section 10.5 describes that
+* baseinstance is not counted by gl_InstanceID. In that section,
notice
+*
+* If an enabled vertex attribute array is instanced (it has a
+* non-zero divisor as specified by VertexAttribDivisor), the
element
+* index that is transferred to the GL, for all vertices, is given
by
+*
+* floor(instance/divisor) + baseinstance
+*
+* If an array corresponding to an attribute required by a vertex
+* shader is not enabled, then the corresponding element is taken
from
+* the current attribute state (see section 10.2).
+*
+* Note that baseinstance is \b not included in the value of instance.
+*/
SYSTEM_VALUE_INSTANCE_ID,
/*@}*/
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://urldefense.proofpoint.com/v1/url?u=http://lists.freedesktop.org/mailman/listinfo/mesa-devk=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0Ar=F4msKE2WxRzA%2BwN%2B25muztFm5TSPwE8HKJfWfR2NgfY%3D%0Am=c7mpo2cNxoz4%2B1pLJ8N5Bh2mWYO5UYb7Cgn66JTok3o%3D%0As=fcf1c30ffc6d4cf44a4573e3001f3f44f57b7becba1f3678074036428413468d
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
Re: [Mesa-dev] [PATCH] i965/fs: set virtual_grf_count in assign_regs()
Reviewed-by: Matt Turner matts...@gmail.com I'll commit both of these tonight. (Does the vec4 backend have the same problem?) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/cse: Don't eliminate instructions with side-effects
On Friday, August 08, 2014 02:42:27 PM Jason Ekstrand wrote:
In particular, this caused problems where atomics operations were getting
eliminated.
Signed-off-by: Jason Ekstrand jason.ekstr...@intel.com
---
src/mesa/drivers/dri/i965/brw_fs_cse.cpp | 3 ++-
src/mesa/drivers/dri/i965/brw_vec4_cse.cpp | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
b/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
index 63d87f9..8cfc6c6 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
@@ -177,7 +177,8 @@ fs_visitor::opt_cse_local(bblock_t *block)
foreach_inst_in_block(fs_inst, inst, block) {
/* Skip some cases. */
if (is_expression(inst) !inst-is_partial_write()
- (inst-dst.file != HW_REG || inst-dst.is_null()))
+ (inst-dst.file != HW_REG || inst-dst.is_null())
+ !inst-has_side_effects())
{
bool found = false;
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
b/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
index 29d2e02..44651b4 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
@@ -135,7 +135,8 @@ vec4_visitor::opt_cse_local(bblock_t *block)
foreach_inst_in_block (vec4_instruction, inst, block) {
/* Skip some cases. */
if (is_expression(inst) !inst-predicate inst-mlen == 0
- (inst-dst.file != HW_REG || inst-dst.is_null()))
+ (inst-dst.file != HW_REG || inst-dst.is_null())
+ !inst-has_side_effects())
{
bool found = false;
I was confused at first because operations with side-effects should never have
been part of the whitelist of opcodes to CSE. But Matt generalized it in
1d97212007ccae, by changing is_expression()'s default case to return
inst-is_send_from_grf().
I think a better patch would be to change that to:
default:
return inst-is_send_from_grf() !inst-has_side_effects();
It's also worth noting in your commit message that this is not actually fixing
a current bug, but rather preventing a regression once your patches that
convert atomics to send-from-GRFs land.
--Ken
signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/8] mesa: define and use ALL_TYPE_BITS in varray.c code
Am 08.08.2014 23:20, schrieb Brian Paul:
---
src/mesa/main/varray.c | 33 +
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/src/mesa/main/varray.c b/src/mesa/main/varray.c
index 0356858..1454449 100644
--- a/src/mesa/main/varray.c
+++ b/src/mesa/main/varray.c
@@ -46,21 +46,22 @@
/** Used to indicate which GL datatypes are accepted by each of the
* glVertex/Color/Attrib/EtcPointer() functions.
*/
-#define BOOL_BIT 0x1
-#define BYTE_BIT 0x2
-#define UNSIGNED_BYTE_BIT0x4
-#define SHORT_BIT0x8
-#define UNSIGNED_SHORT_BIT 0x10
-#define INT_BIT 0x20
-#define UNSIGNED_INT_BIT 0x40
-#define HALF_BIT 0x80
-#define FLOAT_BIT0x100
-#define DOUBLE_BIT 0x200
-#define FIXED_ES_BIT 0x400
-#define FIXED_GL_BIT 0x800
-#define UNSIGNED_INT_2_10_10_10_REV_BIT 0x1000
-#define INT_2_10_10_10_REV_BIT 0x2000
-#define UNSIGNED_INT_10F_11F_11F_REV_BIT 0x4000
+#define BOOL_BIT (1 0)
+#define BYTE_BIT (1 1)
+#define UNSIGNED_BYTE_BIT (1 2)
+#define SHORT_BIT (1 3)
+#define UNSIGNED_SHORT_BIT(1 4)
+#define INT_BIT (1 5)
+#define UNSIGNED_INT_BIT (1 6)
+#define HALF_BIT (1 7)
+#define FLOAT_BIT (1 8)
+#define DOUBLE_BIT(1 9)
+#define FIXED_ES_BIT (1 10)
+#define FIXED_GL_BIT (1 11)
+#define UNSIGNED_INT_2_10_10_10_REV_BIT (1 12)
+#define INT_2_10_10_10_REV_BIT(1 13)
+#define UNSIGNED_INT_10F_11F_11F_REV_BIT (1 14)
+#define ALL_TYPE_BITS((1 15) - 1)
/** Convert GL datatype enum into a type_BIT value seen above */
@@ -185,7 +186,7 @@ vertex_binding_divisor(struct gl_context *ctx, GLuint
bindingIndex,
static GLbitfield
get_legal_types_mask(const struct gl_context *ctx)
{
- GLbitfield legalTypesMask = ~0u; /* all */
+ GLbitfield legalTypesMask = ALL_TYPE_BITS;
if (_mesa_is_gles(ctx)) {
legalTypesMask = ~(FIXED_GL_BIT |
1/8, 2/8 are
Reviewed-by: Roland Scheidegger srol...@vmware.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/fs: set virtual_grf_count in assign_regs()
On Fri, Aug 8, 2014 at 4:55 PM, Matt Turner matts...@gmail.com wrote: Reviewed-by: Matt Turner matts...@gmail.com I'll commit both of these tonight. (Does the vec4 backend have the same problem?) AFAICT, it has the same problem that this patch fixes but not the last patch. I would fix send another patch, but I'm not as familiar with the vec4 backend. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Use memcpy instead of an assignment when storing uniforms
On Friday, August 08, 2014 02:56:53 PM Eric Anholt wrote:
Roland Scheidegger srol...@vmware.com writes:
Am 08.08.2014 22:56, schrieb Neil Roberts:
The i965 driver uses a float pointer to point to the value of a uniform and
also as the destination within the batch buffer. However the same locations
can also be used to store values for integer uniforms. Previously the value
was being copied into the batch buffer with a regular assignment. This
breaks
if the compiler does this by loading and saving through an x87 register
because the fst instruction tries to fix up invalid float values. That can
corrupt some specific integer values. This patch changes it to use a memcpy
instead so that it won't use a floating-point register.
Bugzilla:
https://urldefense.proofpoint.com/v1/url?u=https://bugs.freedesktop.org/show_bug.cgi?id%3D81150k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0Ar=F4msKE2WxRzA%2BwN%2B25muztFm5TSPwE8HKJfWfR2NgfY%3D%0Am=5ZyizVpb8bGGN4qANo378HB6GFoFoBABoDXdPAyIszE%3D%0As=107a04b60233e66abfb9e83a62b90d128d60d6c61eb41b1a08ec7ab537283903
---
src/mesa/drivers/dri/i965/gen6_vs_state.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/gen6_vs_state.c
b/src/mesa/drivers/dri/i965/gen6_vs_state.c
index 905e123..cdbc083 100644
--- a/src/mesa/drivers/dri/i965/gen6_vs_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_vs_state.c
@@ -81,7 +81,13 @@ gen6_upload_push_constants(struct brw_context *brw,
* wouldn't be set for them.
*/
for (i = 0; i prog_data-nr_params; i++) {
- param[i] = *prog_data-param[i];
+ /* A memcpy is used here instead of a regular assignment because
+ * otherwise the value may end up being copied through a
+ * floating-point register. This will break if the x87 registers
are
+ * used and we are storing an integer value here because the fst
+ * instruction tries to fix up invalid values and that would
corrupt
+ * an integer value */
+ memcpy(param + i, prog_data-param[i], sizeof param[i]);
}
Or just change the pointer type to uint32_t, right?
I think I'd prefer that.
--Ken
signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/cse: Don't eliminate instructions with side-effects
On Fri, Aug 8, 2014 at 4:58 PM, Kenneth Graunke kenn...@whitecape.org
wrote:
On Friday, August 08, 2014 02:42:27 PM Jason Ekstrand wrote:
In particular, this caused problems where atomics operations were getting
eliminated.
Signed-off-by: Jason Ekstrand jason.ekstr...@intel.com
---
src/mesa/drivers/dri/i965/brw_fs_cse.cpp | 3 ++-
src/mesa/drivers/dri/i965/brw_vec4_cse.cpp | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
b/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
index 63d87f9..8cfc6c6 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
@@ -177,7 +177,8 @@ fs_visitor::opt_cse_local(bblock_t *block)
foreach_inst_in_block(fs_inst, inst, block) {
/* Skip some cases. */
if (is_expression(inst) !inst-is_partial_write()
- (inst-dst.file != HW_REG || inst-dst.is_null()))
+ (inst-dst.file != HW_REG || inst-dst.is_null())
+ !inst-has_side_effects())
{
bool found = false;
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
b/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
index 29d2e02..44651b4 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
@@ -135,7 +135,8 @@ vec4_visitor::opt_cse_local(bblock_t *block)
foreach_inst_in_block (vec4_instruction, inst, block) {
/* Skip some cases. */
if (is_expression(inst) !inst-predicate inst-mlen == 0
- (inst-dst.file != HW_REG || inst-dst.is_null()))
+ (inst-dst.file != HW_REG || inst-dst.is_null())
+ !inst-has_side_effects())
{
bool found = false;
I was confused at first because operations with side-effects should never
have been part of the whitelist of opcodes to CSE. But Matt generalized it
in 1d97212007ccae, by changing is_expression()'s default case to return
inst-is_send_from_grf().
I think a better patch would be to change that to:
default:
return inst-is_send_from_grf() !inst-has_side_effects();
Right. Good point. I'll do it that way instead.
It's also worth noting in your commit message that this is not actually
fixing a current bug, but rather preventing a regression once your patches
that convert atomics to send-from-GRFs land.
Perhaps. Honestly, I'm not sure why CSE isn't causing problems now unless
it doesn't do CSE on message registers.
--Jason
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/cse: Don't eliminate instructions with side-effects
On Fri, Aug 8, 2014 at 4:58 PM, Kenneth Graunke kenn...@whitecape.org wrote:
On Friday, August 08, 2014 02:42:27 PM Jason Ekstrand wrote:
In particular, this caused problems where atomics operations were getting
eliminated.
Signed-off-by: Jason Ekstrand jason.ekstr...@intel.com
---
src/mesa/drivers/dri/i965/brw_fs_cse.cpp | 3 ++-
src/mesa/drivers/dri/i965/brw_vec4_cse.cpp | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
b/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
index 63d87f9..8cfc6c6 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
@@ -177,7 +177,8 @@ fs_visitor::opt_cse_local(bblock_t *block)
foreach_inst_in_block(fs_inst, inst, block) {
/* Skip some cases. */
if (is_expression(inst) !inst-is_partial_write()
- (inst-dst.file != HW_REG || inst-dst.is_null()))
+ (inst-dst.file != HW_REG || inst-dst.is_null())
+ !inst-has_side_effects())
{
bool found = false;
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
b/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
index 29d2e02..44651b4 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
@@ -135,7 +135,8 @@ vec4_visitor::opt_cse_local(bblock_t *block)
foreach_inst_in_block (vec4_instruction, inst, block) {
/* Skip some cases. */
if (is_expression(inst) !inst-predicate inst-mlen == 0
- (inst-dst.file != HW_REG || inst-dst.is_null()))
+ (inst-dst.file != HW_REG || inst-dst.is_null())
+ !inst-has_side_effects())
{
bool found = false;
I was confused at first because operations with side-effects should never
have been part of the whitelist of opcodes to CSE. But Matt generalized it
in 1d97212007ccae, by changing is_expression()'s default case to return
inst-is_send_from_grf().
I think a better patch would be to change that to:
default:
return inst-is_send_from_grf() !inst-has_side_effects();
Yeah, that seems fine assuming we never add a non-send-from-grf
instruction to the has_side_effect list. I think that's a safe
assumption, since the other instructions that have side effects are
things that e.g., implicitly write the accumulator, i.e., things we
can still track.
Either way works for me.
Reviewed-by: Matt Turner matts...@gmail.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/8] mesa: use PRIu64 for printing 64-bit uints
Am 08.08.2014 23:20, schrieb Brian Paul:
Silences MinGW warnings:
warning: unknown conversion type character ‘l’ in format [-Wformat]
warning: too many arguments for format [-Wformat-extra-args]
---
src/mesa/main/bufferobj.c | 33 +
src/mesa/main/varray.c| 13 -
2 files changed, 25 insertions(+), 21 deletions(-)
diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
index 1dfcda3..d59e63c 100644
--- a/src/mesa/main/bufferobj.c
+++ b/src/mesa/main/bufferobj.c
@@ -31,6 +31,7 @@
*/
#include stdbool.h
+#include inttypes.h /* for PRIu64 macro */
#include glheader.h
#include enums.h
#include hash.h
@@ -2824,8 +2825,8 @@ bind_buffers_check_offset_and_size(struct gl_context
*ctx,
* value in offsets is less than zero (per binding).
*/
_mesa_error(ctx, GL_INVALID_VALUE,
- glBindBuffersRange(offsets[%u]=%lld 0),
- index, (long long int) offsets[index]);
+ glBindBuffersRange(offsets[%u]=% PRIu64 0),
+ index, (uint64_t) offsets[index]);
Shouldn't that be printed with PRId64 instead (casting to int64_t)? Same
for all others (and same in 4/8).
Other than that, is there some recommendation wrt whitespace around that
ugly format specifier? Some places seem to use it, some don't. Either
way it's ugly dunno which one is worse. Would be nice though if we'd be
consistent imho.
Roland
return false;
}
@@ -2836,8 +2837,8 @@ bind_buffers_check_offset_and_size(struct gl_context
*ctx,
* value in sizes is less than or equal to zero (per binding).
*/
_mesa_error(ctx, GL_INVALID_VALUE,
- glBindBuffersRange(sizes[%u]=%lld = 0),
- index, (long long int) sizes[index]);
+ glBindBuffersRange(sizes[%u]=% PRIu64 = 0),
+ index, (uint64_t) sizes[index]);
return false;
}
@@ -3032,11 +3033,11 @@ bind_uniform_buffers_range(struct gl_context *ctx,
GLuint first, GLsizei count,
*/
if (offsets[i] (ctx-Const.UniformBufferOffsetAlignment - 1)) {
_mesa_error(ctx, GL_INVALID_VALUE,
- glBindBuffersRange(offsets[%u]=%lld is misaligned;
- it must be a multiple of the value of
+ glBindBuffersRange(offsets[%u]=% PRIu64
+ is misaligned; it must be a multiple of the value of
GL_UNIFORM_BUFFER_OFFSET_ALIGNMENT=%u when
target=GL_UNIFORM_BUFFER),
- i, (long long int) offsets[i],
+ i, (uint64_t) offsets[i],
ctx-Const.UniformBufferOffsetAlignment);
continue;
}
@@ -3270,19 +3271,19 @@ bind_xfb_buffers_range(struct gl_context *ctx,
*/
if (offsets[i] 0x3) {
_mesa_error(ctx, GL_INVALID_VALUE,
- glBindBuffersRange(offsets[%u]=%lld is misaligned;
- it must be a multiple of 4 when
+ glBindBuffersRange(offsets[%u]=% PRIu64
+ is misaligned; it must be a multiple of 4 when
target=GL_TRANSFORM_FEEDBACK_BUFFER),
- i, (long long int) offsets[i]);
+ i, (uint64_t) offsets[i]);
continue;
}
if (sizes[i] 0x3) {
_mesa_error(ctx, GL_INVALID_VALUE,
- glBindBuffersRange(sizes[%u]=%lld is misaligned;
- it must be a multiple of 4 when
+ glBindBuffersRange(sizes[%u]=% PRIu64
+ is misaligned; it must be a multiple of 4 when
target=GL_TRANSFORM_FEEDBACK_BUFFER),
- i, (long long int) sizes[i]);
+ i, (uint64_t) sizes[i]);
continue;
}
@@ -3488,10 +3489,10 @@ bind_atomic_buffers_range(struct gl_context *ctx,
*/
if (offsets[i] (ATOMIC_COUNTER_SIZE - 1)) {
_mesa_error(ctx, GL_INVALID_VALUE,
- glBindBuffersRange(offsets[%u]=%lld is misaligned;
- it must be a multiple of %d when
+ glBindBuffersRange(offsets[%u]=% PRIu64
+ is misaligned; it must be a multiple of %d when
target=GL_ATOMIC_COUNTER_BUFFER),
- i, (long long int) offsets[i], ATOMIC_COUNTER_SIZE);
+ i, (uint64_t) offsets[i], ATOMIC_COUNTER_SIZE);
continue;
}
diff --git a/src/mesa/main/varray.c b/src/mesa/main/varray.c
index 1454449..d1032d3 100644
--- a/src/mesa/main/varray.c
+++ b/src/mesa/main/varray.c
@@ -24,6 +24,8 @@
*/
+#include inttypes.h /* for PRIu64 macro */
+
#include glheader.h
#include imports.h
#include bufferobj.h
@@ -1424,7
Re: [Mesa-dev] [PATCH] i965/cse: Don't eliminate instructions with side-effects
On Friday, August 08, 2014 05:05:35 PM Jason Ekstrand wrote: On Fri, Aug 8, 2014 at 4:58 PM, Kenneth Graunke wrote: It's also worth noting in your commit message that this is not actually fixing a current bug, but rather preventing a regression once your patches that convert atomics to send-from-GRFs land. Perhaps. Honestly, I'm not sure why CSE isn't causing problems now unless it doesn't do CSE on message registers. --Jason CSE should work fine with message registers. It finds code like: OPER m3 src0 src1 ... OPER m4 src0 src1 and replaces it with: OPER tmp src0 src1 MOVm3 tmp ... MOVm4 tmp There are no magical side effects to message registers...they're just where you put data you want to send to some unit. As long as the right data ends up in those registers, it doesn't matter how it gets there. These days, it can also CSE certain SEND messages, but those are...pull constant loads and texturing (memory reads), pixel interpolator requests (also pure), and shader time adds (which are atomic writes, so it really shouldn't!) I guess we've been getting lucky with SHADER_OPCODE_SHADER_TIME_ADD since we never emit two shader time adds with the same sources. (INTEL_DEBUG=shader_time is just a debugging tool.) --Ken signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 6/8] mesa: whitespace, 80-column wrapping in program.c
Am 08.08.2014 23:20, schrieb Brian Paul:
---
src/mesa/program/program.c | 19 +++
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/src/mesa/program/program.c b/src/mesa/program/program.c
index 9886b75..ef5bf6b 100644
--- a/src/mesa/program/program.c
+++ b/src/mesa/program/program.c
@@ -251,8 +251,9 @@ init_program_struct(struct gl_program *prog, GLenum
target, GLuint id)
* Initialize a new fragment program object.
*/
struct gl_program *
-_mesa_init_fragment_program( struct gl_context *ctx, struct
gl_fragment_program *prog,
- GLenum target, GLuint id)
+_mesa_init_fragment_program(struct gl_context *ctx,
+struct gl_fragment_program *prog,
+GLenum target, GLuint id)
{
if (prog) {
init_program_struct(prog-Base, target, id);
@@ -266,8 +267,9 @@ _mesa_init_fragment_program( struct gl_context *ctx,
struct gl_fragment_program
* Initialize a new vertex program object.
*/
struct gl_program *
-_mesa_init_vertex_program( struct gl_context *ctx, struct gl_vertex_program
*prog,
- GLenum target, GLuint id)
+_mesa_init_vertex_program(struct gl_context *ctx,
+ struct gl_vertex_program *prog,
+ GLenum target, GLuint id)
{
if (prog) {
init_program_struct(prog-Base, target, id);
@@ -282,8 +284,8 @@ _mesa_init_vertex_program( struct gl_context *ctx, struct
gl_vertex_program *pro
*/
struct gl_program *
_mesa_init_compute_program(struct gl_context *ctx,
- struct gl_compute_program *prog, GLenum target,
- GLuint id)
+ struct gl_compute_program *prog,
+ GLenum target, GLuint id)
{
if (prog) {
init_program_struct(prog-Base, target, id);
@@ -297,8 +299,9 @@ _mesa_init_compute_program(struct gl_context *ctx,
* Initialize a new geometry program object.
*/
struct gl_program *
-_mesa_init_geometry_program( struct gl_context *ctx, struct
gl_geometry_program *prog,
- GLenum target, GLuint id)
+_mesa_init_geometry_program(struct gl_context *ctx,
+struct gl_geometry_program *prog,
+GLenum target, GLuint id)
{
if (prog) {
init_program_struct(prog-Base, target, id);
5/8 and 6/8 are
Reviewed-by: Roland Scheidegger srol...@vmware.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/8] mesa: use PRIu64 for printing 64-bit uints
Other than that, is there some recommendation wrt whitespace around that ugly format specifier? Some places seem to use it, some don't. Either way it's ugly dunno which one is worse. Would be nice though if we'd be consistent imho. I've always seen spaces between the quotes and the specifier. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
