[Mesa-dev] radeonfb: BAR 0: can't reserve [mem 0x98000000-0x9fffffff pref]
Hi, Before reporting a possible invalid bug report. Does anyone knows why radeaonfb is not configured the same way radeon is ? For instance on a PowerPC machine, when Open Firmware Frame Buffer is used (OFfb), I cannot `modprobe radeonfb` (but I can load `radeon`). It fails with: [ 96.551486] radeonfb :00:10.0: enabling device (0006 -> 0007) [ 96.551526] radeonfb :00:10.0: BAR 0: can't reserve [mem 0x9800-0x9fff pref] [ 96.551531] radeonfb (:00:10.0): cannot request region 0. [ 96.551545] radeonfb: probe of :00:10.0 failed with error -16 It seems (to me) that it should be possible to add something like this to `radeonfb`: https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/radeon/radeon_drv.c#L353 Is the above correct ? ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: geom shader max_vertices layout must match.
On 03/06/16 23:04, Dave Airlie wrote: > On 4 June 2016 at 03:39, Alejandro Piñeiro wrote: >> On 03/06/16 02:46, Dave Airlie wrote: >>> From: Dave Airlie >>> >>> "all geometry shader output vertex count declarations in a >>> program must declare the same count." >> This spec quote lacks context. As far as I see it comes from GLSL 4.5 >> spec, "4.4.2.3 Geometry Outputs". >> >>> Fixes: >>> GL45-CTS.geometry_shader.output.conflicted_output_vertices_max >> The patch looks good. But testing on Skylake and Broadwell, current >> master is passing this test without this patch. Did you mean a different >> test, or you made the test with a different hw? > It might be a regression since, > aaa69c79cd584db4d9c6ea7794e93d29f3d54572 > > Can you confirm you are running on master, Yes. And just in case, I updated today, and also tried at that commit point, and just before. In all cases the test is passing on Skylake. > some of the CTS results > I've seen suggest you might not have all the changes. BR ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: use enum glsl_interface_packing in more places. (v2)
With the const's gone on the return types of the new helpers, this is Reviewed-by: Ilia Mirkin On Mon, Jun 6, 2016 at 12:21 AM, Dave Airlie wrote: > From: Dave Airlie > > Although the glsl_types.h stores this in a bitfield, > we should hide that from everyone else. Hide the cast > in an accessor method and use the enum everywhere. > > This makes things a bit nicer in gdb, and improves type > safety. > > v2: fix a few pieces of interface I missed that caused some > piglit regressions. > > Signed-off-by: Dave Airlie > --- > src/compiler/glsl/ir.h | 4 > .../glsl/link_uniform_block_active_visitor.cpp | 6 ++ > src/compiler/glsl/link_uniform_blocks.cpp | 6 +++--- > src/compiler/glsl/link_uniforms.cpp| 22 > +++--- > src/compiler/glsl/linker.h | 8 > src/compiler/glsl/lower_buffer_access.cpp | 2 +- > src/compiler/glsl/lower_buffer_access.h| 2 +- > src/compiler/glsl/lower_shared_reference.cpp | 6 +++--- > src/compiler/glsl/lower_ubo_reference.cpp | 16 > src/compiler/glsl/opt_dead_code.cpp| 2 +- > src/compiler/glsl_types.h | 8 > 11 files changed, 46 insertions(+), 36 deletions(-) > > diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h > index 93716c4..b4c7ba2 100644 > --- a/src/compiler/glsl/ir.h > +++ b/src/compiler/glsl/ir.h > @@ -537,6 +537,10 @@ public: >return this->interface_type; > } > > + const enum glsl_interface_packing get_interface_type_packing() const > + { > + return this->interface_type->get_interface_packing(); > + } > /** > * Get the max_ifc_array_access pointer > * > diff --git a/src/compiler/glsl/link_uniform_block_active_visitor.cpp > b/src/compiler/glsl/link_uniform_block_active_visitor.cpp > index 54fea70..df8b221 100644 > --- a/src/compiler/glsl/link_uniform_block_active_visitor.cpp > +++ b/src/compiler/glsl/link_uniform_block_active_visitor.cpp > @@ -167,8 +167,7 @@ link_uniform_block_active_visitor::visit(ir_variable *var) > * also considered active, even if no member of the block is > * referenced." > */ > - if (var->get_interface_type()->interface_packing == > - GLSL_INTERFACE_PACKING_PACKED) > + if (var->get_interface_type_packing() == GLSL_INTERFACE_PACKING_PACKED) >return visit_continue; > > /* Process the block. Bail if there was an error. > @@ -258,8 +257,7 @@ > link_uniform_block_active_visitor::visit_enter(ir_dereference_array *ir) > * std140 layout qualifier, all its instances have been already marked > * as used in link_uniform_block_active_visitor::visit(ir_variable *). > */ > - if (var->get_interface_type()->interface_packing == > - GLSL_INTERFACE_PACKING_PACKED) { > + if (var->get_interface_type_packing() == GLSL_INTERFACE_PACKING_PACKED) { >b->var = var; >process_arrays(this->mem_ctx, ir, b); > } > diff --git a/src/compiler/glsl/link_uniform_blocks.cpp > b/src/compiler/glsl/link_uniform_blocks.cpp > index 3c2d13c..b816dab 100644 > --- a/src/compiler/glsl/link_uniform_blocks.cpp > +++ b/src/compiler/glsl/link_uniform_blocks.cpp > @@ -70,7 +70,7 @@ private: > } > > virtual void enter_record(const glsl_type *type, const char *, > - bool row_major, const unsigned packing) { > + bool row_major, const enum > glsl_interface_packing packing) { >assert(type->is_record()); >if (packing == GLSL_INTERFACE_PACKING_STD430) > this->offset = glsl_align( > @@ -81,7 +81,7 @@ private: > } > > virtual void leave_record(const glsl_type *type, const char *, > - bool row_major, const unsigned packing) { > + bool row_major, const enum > glsl_interface_packing packing) { >assert(type->is_record()); > >/* If this is the last field of a structure, apply rule #9. The > @@ -106,7 +106,7 @@ private: > > virtual void visit_field(const glsl_type *type, const char *name, > bool row_major, const glsl_type *, > -const unsigned packing, > +const enum glsl_interface_packing packing, > bool last_field) > { >assert(this->index < this->num_variables); > diff --git a/src/compiler/glsl/link_uniforms.cpp > b/src/compiler/glsl/link_uniforms.cpp > index ff2989f..3a2ac4d 100644 > --- a/src/compiler/glsl/link_uniforms.cpp > +++ b/src/compiler/glsl/link_uniforms.cpp > @@ -65,7 +65,7 @@ program_resource_visitor::process(const glsl_type *type, > const char *name) > > unsigned record_array_count = 1; > char *name_copy = ralloc_strdup(NULL, name); > - unsigned packing = type->interface_packing; > + enum glsl_interface_packing packing = type
[Mesa-dev] [PATCH] glsl: use enum glsl_interface_packing in more places. (v2)
From: Dave Airlie Although the glsl_types.h stores this in a bitfield, we should hide that from everyone else. Hide the cast in an accessor method and use the enum everywhere. This makes things a bit nicer in gdb, and improves type safety. v2: fix a few pieces of interface I missed that caused some piglit regressions. Signed-off-by: Dave Airlie --- src/compiler/glsl/ir.h | 4 .../glsl/link_uniform_block_active_visitor.cpp | 6 ++ src/compiler/glsl/link_uniform_blocks.cpp | 6 +++--- src/compiler/glsl/link_uniforms.cpp| 22 +++--- src/compiler/glsl/linker.h | 8 src/compiler/glsl/lower_buffer_access.cpp | 2 +- src/compiler/glsl/lower_buffer_access.h| 2 +- src/compiler/glsl/lower_shared_reference.cpp | 6 +++--- src/compiler/glsl/lower_ubo_reference.cpp | 16 src/compiler/glsl/opt_dead_code.cpp| 2 +- src/compiler/glsl_types.h | 8 11 files changed, 46 insertions(+), 36 deletions(-) diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h index 93716c4..b4c7ba2 100644 --- a/src/compiler/glsl/ir.h +++ b/src/compiler/glsl/ir.h @@ -537,6 +537,10 @@ public: return this->interface_type; } + const enum glsl_interface_packing get_interface_type_packing() const + { + return this->interface_type->get_interface_packing(); + } /** * Get the max_ifc_array_access pointer * diff --git a/src/compiler/glsl/link_uniform_block_active_visitor.cpp b/src/compiler/glsl/link_uniform_block_active_visitor.cpp index 54fea70..df8b221 100644 --- a/src/compiler/glsl/link_uniform_block_active_visitor.cpp +++ b/src/compiler/glsl/link_uniform_block_active_visitor.cpp @@ -167,8 +167,7 @@ link_uniform_block_active_visitor::visit(ir_variable *var) * also considered active, even if no member of the block is * referenced." */ - if (var->get_interface_type()->interface_packing == - GLSL_INTERFACE_PACKING_PACKED) + if (var->get_interface_type_packing() == GLSL_INTERFACE_PACKING_PACKED) return visit_continue; /* Process the block. Bail if there was an error. @@ -258,8 +257,7 @@ link_uniform_block_active_visitor::visit_enter(ir_dereference_array *ir) * std140 layout qualifier, all its instances have been already marked * as used in link_uniform_block_active_visitor::visit(ir_variable *). */ - if (var->get_interface_type()->interface_packing == - GLSL_INTERFACE_PACKING_PACKED) { + if (var->get_interface_type_packing() == GLSL_INTERFACE_PACKING_PACKED) { b->var = var; process_arrays(this->mem_ctx, ir, b); } diff --git a/src/compiler/glsl/link_uniform_blocks.cpp b/src/compiler/glsl/link_uniform_blocks.cpp index 3c2d13c..b816dab 100644 --- a/src/compiler/glsl/link_uniform_blocks.cpp +++ b/src/compiler/glsl/link_uniform_blocks.cpp @@ -70,7 +70,7 @@ private: } virtual void enter_record(const glsl_type *type, const char *, - bool row_major, const unsigned packing) { + bool row_major, const enum glsl_interface_packing packing) { assert(type->is_record()); if (packing == GLSL_INTERFACE_PACKING_STD430) this->offset = glsl_align( @@ -81,7 +81,7 @@ private: } virtual void leave_record(const glsl_type *type, const char *, - bool row_major, const unsigned packing) { + bool row_major, const enum glsl_interface_packing packing) { assert(type->is_record()); /* If this is the last field of a structure, apply rule #9. The @@ -106,7 +106,7 @@ private: virtual void visit_field(const glsl_type *type, const char *name, bool row_major, const glsl_type *, -const unsigned packing, +const enum glsl_interface_packing packing, bool last_field) { assert(this->index < this->num_variables); diff --git a/src/compiler/glsl/link_uniforms.cpp b/src/compiler/glsl/link_uniforms.cpp index ff2989f..3a2ac4d 100644 --- a/src/compiler/glsl/link_uniforms.cpp +++ b/src/compiler/glsl/link_uniforms.cpp @@ -65,7 +65,7 @@ program_resource_visitor::process(const glsl_type *type, const char *name) unsigned record_array_count = 1; char *name_copy = ralloc_strdup(NULL, name); - unsigned packing = type->interface_packing; + enum glsl_interface_packing packing = type->get_interface_packing(); recursion(type, &name_copy, strlen(name), false, NULL, packing, false, record_array_count, NULL); @@ -79,9 +79,9 @@ program_resource_visitor::process(ir_variable *var) const bool row_major = var->data.matrix_layout == GLSL_MATRIX_LAYOUT_ROW_MAJOR; - const unsigned packing = var->get_interface_type()
Re: [Mesa-dev] [PATCH v2] i965: Fix point size with tessellation/geometry shaders in GLES.
On Friday, June 3, 2016 4:40:46 PM PDT Ilia Mirkin wrote: > On Fri, Jun 3, 2016 at 4:08 AM, Kenneth Graunke wrote: > > Our previous code worked for desktop GL, and ES without geometry or > > tessellation shaders. But those features require fancier point size > > handling. Fortunately, we can use one rule for all APIs. > > > > Fixes a number of dEQP tests with EXT_tessellation_shader enabled: > > dEQP-GLES31.functional.tessellation_geometry_interaction.point_size.* > > > > Cc: Ilia Mirkin > > Signed-off-by: Kenneth Graunke > > --- > > src/mesa/drivers/dri/i965/brw_state.h | 49 > > +++ > > src/mesa/drivers/dri/i965/gen6_sf_state.c | 5 ++-- > > src/mesa/drivers/dri/i965/gen7_sf_state.c | 7 +++-- > > src/mesa/drivers/dri/i965/gen8_sf_state.c | 7 +++-- > > 4 files changed, 59 insertions(+), 9 deletions(-) > > > > Hey Ilia, > > > > Thanks for helping me figure out these rules on IRC yesterday. > > I think this version should work... > > > > --Ken > > > > diff --git a/src/mesa/drivers/dri/i965/brw_state.h > > b/src/mesa/drivers/dri/i965/brw_state.h > > index 0a4c21f..be7f6ce 100644 > > --- a/src/mesa/drivers/dri/i965/brw_state.h > > +++ b/src/mesa/drivers/dri/i965/brw_state.h > > @@ -473,6 +473,55 @@ is_drawing_lines(const struct brw_context *brw) > > return false; > > } > > > > +static inline bool > > +use_state_point_size(const struct brw_context *brw) > > +{ > > + const struct gl_context *ctx = &brw->ctx; > > + > > + /* Section 14.4 (Points) of the OpenGL 4.5 specification says: > > +* > > +*"If program point size mode is enabled, the derived point size is > > +* taken from the (potentially clipped) shader built-in gl_PointSize > > +* written by: > > +* > > +** the geometry shader, if active; > > +** the tessellation evaluation shader, if active and no > > +* geometry shader is active; > > +** the vertex shader, otherwise > > +* > > +*and clamped to the implementation-dependent point size range. If > > +*the value written to gl_PointSize is less than or equal to zero, > > +*or if no value was written to gl_PointSize, results are undefined. > > +*If program point size mode is disabled, the derived point size is > > +*specified with the command > > +* > > +* void PointSize(float size); > > +* > > +*size specifies the requested size of a point. The default value > > +*is 1.0." > > +* > > +* The rules for GLES come from the ES 3.2, OES_geometry_point_size, and > > +* OES_tessellation_point_size specifications. To summarize: if the > > last > > +* stage before rasterization is a GS or TES, then use gl_PointSize from > > +* the shader if written. Otherwise, use 1.0. If the last stage is a > > +* vertex shader, use gl_PointSize, or it is undefined. > > +* > > +* We can combine these rules into a single condition for both APIs. > > +* Using the state point size when the last shader stage doesn't write > > +* gl_PointSize satisfies GL's requirements, as it's undefined. Because > > +* ES doesn't have a PointSize() command, the state point size will > > +* remain 1.0, satisfying the ES default value in the GS/TES case, and > > +* the VS case (1.0 works for "undefined"). Mesa sets the program point > > +* mode flag to always-enabled in ES, so we can safely check that, and > > +* it'll be ignored for ES. > > +* > > +* _NEW_PROGRAM | _NEW_POINT > > +* BRW_NEW_VUE_MAP_GEOM_OUT > > +*/ > > + return (!ctx->VertexProgram.PointSizeEnabled && > > !ctx->Point._Attenuated) || > > + (brw->vue_map_geom_out.slots_valid & VARYING_BIT_PSIZ) == 0; > > I believe that this is "overly" nice to buggy programs, but not > incorrect. The cases where this has to return true: > > (a) point size is explicitly disabled > (b) ES2 context, and point size isn't emitted in TES/GS > > And it has to return false when point size is enabled && "last stage" > emits a point size. In all other cases, you can do whatever. I believe > you're choosing to be nice and use the fixed function point size if a > point size isn't being emitted, which is fine but unnecessary. Right. I decided to use the API-specified point size in a few undefined cases because I figured it would actually take more code to be meaner. > Dunno what the _Attenuated thing is, but it was like that before too, > so I assume you know what you're doing. Apparently there's a GL_DISTANCE_ATTENUATION_EXT point parameter, where you can set three values, and it varies the point size based on the distance. We emulate this in ffvertex_prog.c. > In principle this makes sense to me, but I don't know what > vue_map_geom_out actually is, so I'm going to leave the actual review > to someone who knows the driver better. > > -ilia vue_map_geom_out is just a data structure representing the o
Re: [Mesa-dev] [PATCH 0/5] GL_ARB_shader_group_vote support
On 6 June 2016 at 11:47, Ilia Mirkin wrote: > ping > > On Sun, May 29, 2016 at 2:01 PM, Ilia Mirkin wrote: >> Turns out Matt already did a bunch of this work last year, but since I >> only found out about that after I wrote my patches, I decided to keep >> mine in place (I like the "vote" naming better TBH). This passes the >> very simple piglit tests I just sent out to the list. For the first 4, Reviewed-by: Dave Airlie ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] i965: Fix point size with tessellation/geometry shaders in GLES.
Hi Kenneth, Am 03.06.2016 um 10:08 schrieb Kenneth Graunke: > Our previous code worked for desktop GL, and ES without geometry or > tessellation shaders. But those features require fancier point size > handling. Fortunately, we can use one rule for all APIs. > > Fixes a number of dEQP tests with EXT_tessellation_shader enabled: > dEQP-GLES31.functional.tessellation_geometry_interaction.point_size.* > > Cc: Ilia Mirkin > Signed-off-by: Kenneth Graunke > --- > src/mesa/drivers/dri/i965/brw_state.h | 49 > +++ > src/mesa/drivers/dri/i965/gen6_sf_state.c | 5 ++-- > src/mesa/drivers/dri/i965/gen7_sf_state.c | 7 +++-- > src/mesa/drivers/dri/i965/gen8_sf_state.c | 7 +++-- > 4 files changed, 59 insertions(+), 9 deletions(-) > > Hey Ilia, > > Thanks for helping me figure out these rules on IRC yesterday. > I think this version should work... > > --Ken > > diff --git a/src/mesa/drivers/dri/i965/brw_state.h > b/src/mesa/drivers/dri/i965/brw_state.h > index 0a4c21f..be7f6ce 100644 > --- a/src/mesa/drivers/dri/i965/brw_state.h > +++ b/src/mesa/drivers/dri/i965/brw_state.h > @@ -473,6 +473,55 @@ is_drawing_lines(const struct brw_context *brw) > return false; > } > > +static inline bool > +use_state_point_size(const struct brw_context *brw) > +{ > + const struct gl_context *ctx = &brw->ctx; > + > + /* Section 14.4 (Points) of the OpenGL 4.5 specification says: > +* > +*"If program point size mode is enabled, the derived point size is > +* taken from the (potentially clipped) shader built-in gl_PointSize > +* written by: > +* > +** the geometry shader, if active; > +** the tessellation evaluation shader, if active and no > +* geometry shader is active; > +** the vertex shader, otherwise > +* > +*and clamped to the implementation-dependent point size range. If > +*the value written to gl_PointSize is less than or equal to zero, > +*or if no value was written to gl_PointSize, results are undefined. > +*If program point size mode is disabled, the derived point size is > +*specified with the command > +* > +* void PointSize(float size); > +* > +*size specifies the requested size of a point. The default value > +*is 1.0." > +* > +* The rules for GLES come from the ES 3.2, OES_geometry_point_size, and > +* OES_tessellation_point_size specifications. To summarize: if the last > +* stage before rasterization is a GS or TES, then use gl_PointSize from > +* the shader if written. Otherwise, use 1.0. If the last stage is a > +* vertex shader, use gl_PointSize, or it is undefined. > +* > +* We can combine these rules into a single condition for both APIs. > +* Using the state point size when the last shader stage doesn't write > +* gl_PointSize satisfies GL's requirements, as it's undefined. Because > +* ES doesn't have a PointSize() command, the state point size will > +* remain 1.0, satisfying the ES default value in the GS/TES case, and > +* the VS case (1.0 works for "undefined"). Mesa sets the program point > +* mode flag to always-enabled in ES, so we can safely check that, and > +* it'll be ignored for ES. > +* > +* _NEW_PROGRAM | _NEW_POINT > +* BRW_NEW_VUE_MAP_GEOM_OUT > +*/ > + return (!ctx->VertexProgram.PointSizeEnabled && !ctx->Point._Attenuated) > || > + (brw->vue_map_geom_out.slots_valid & VARYING_BIT_PSIZ) == 0; That last condition looks decidedly weird. Are those brackets correct? --Michael > +} > + > > #ifdef __cplusplus > } > diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c > b/src/mesa/drivers/dri/i965/gen6_sf_state.c > index 0538ab7..94731e0 100644 > --- a/src/mesa/drivers/dri/i965/gen6_sf_state.c > +++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c > @@ -373,9 +373,8 @@ upload_sf_state(struct brw_context *brw) > if (multisampled_fbo && ctx->Multisample.Enabled) >dw3 |= GEN6_SF_MSRAST_ON_PATTERN; > > - /* _NEW_PROGRAM | _NEW_POINT */ > - if (!(ctx->VertexProgram.PointSizeEnabled || > - ctx->Point._Attenuated)) > + /* _NEW_PROGRAM | _NEW_POINT, BRW_NEW_VUE_MAP_GEOM_OUT */ > + if (use_state_point_size(brw)) >dw4 |= GEN6_SF_USE_STATE_POINT_WIDTH; > > /* _NEW_POINT - Clamp to ARB_point_parameters user limits */ > diff --git a/src/mesa/drivers/dri/i965/gen7_sf_state.c > b/src/mesa/drivers/dri/i965/gen7_sf_state.c > index d3a658c..8d49e24 100644 > --- a/src/mesa/drivers/dri/i965/gen7_sf_state.c > +++ b/src/mesa/drivers/dri/i965/gen7_sf_state.c > @@ -213,8 +213,8 @@ upload_sf_state(struct brw_context *brw) > > dw3 = GEN6_SF_LINE_AA_MODE_TRUE; > > - /* _NEW_PROGRAM | _NEW_POINT */ > - if (!(ctx->VertexProgram.PointSizeEnabled || ctx->Point._Attenuated)) > + /* _NEW_PROGRAM | _NEW_POINT, BRW_NEW_VUE_MAP_GEOM_OUT */ > + if (use_state_point_size(b
Re: [Mesa-dev] [PATCH 0/5] GL_ARB_shader_group_vote support
ping On Sun, May 29, 2016 at 2:01 PM, Ilia Mirkin wrote: > Turns out Matt already did a bunch of this work last year, but since I > only found out about that after I wrote my patches, I decided to keep > mine in place (I like the "vote" naming better TBH). This passes the > very simple piglit tests I just sent out to the list. > > Ilia Mirkin (5): > mesa: hook up core bits of GL_ARB_shader_group_vote > gallium: add VOTE_* opcodes to implement GL_ARB_shader_group_vote > gallium: add PIPE_CAP_TGSI_VOTE for when the VOTE ops are allowed > st/mesa: expose GL_ARB_shader_group_vote when supported by backend > nvc0: add support for VOTE tgsi opcodes > > src/compiler/glsl/builtin_functions.cpp| 22 ++ > src/compiler/glsl/glcpp/glcpp-parse.y | 3 +++ > src/compiler/glsl/glsl_parser_extras.cpp | 1 + > src/compiler/glsl/glsl_parser_extras.h | 2 ++ > src/compiler/glsl/ir.cpp | 6 + > src/compiler/glsl/ir.h | 9 +++- > src/compiler/glsl/ir_validate.cpp | 8 +++ > src/gallium/auxiliary/tgsi/tgsi_info.c | 3 +++ > src/gallium/docs/source/screen.rst | 1 + > src/gallium/docs/source/tgsi.rst | 17 ++ > src/gallium/drivers/freedreno/freedreno_screen.c | 1 + > src/gallium/drivers/i915/i915_screen.c | 1 + > src/gallium/drivers/ilo/ilo_screen.c | 1 + > src/gallium/drivers/llvmpipe/lp_screen.c | 1 + > .../drivers/nouveau/codegen/nv50_ir_emit_gk110.cpp | 23 ++ > .../drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp | 27 > +- > .../drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp | 23 ++ > .../drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 21 + > src/gallium/drivers/nouveau/nv30/nv30_screen.c | 1 + > src/gallium/drivers/nouveau/nv50/nv50_screen.c | 1 + > src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 4 ++-- > src/gallium/drivers/r300/r300_screen.c | 1 + > src/gallium/drivers/r600/r600_pipe.c | 1 + > src/gallium/drivers/radeonsi/si_pipe.c | 1 + > src/gallium/drivers/softpipe/sp_screen.c | 1 + > src/gallium/drivers/svga/svga_screen.c | 1 + > src/gallium/drivers/swr/swr_screen.cpp | 1 + > src/gallium/drivers/vc4/vc4_screen.c | 1 + > src/gallium/drivers/virgl/virgl_screen.c | 1 + > src/gallium/include/pipe/p_defines.h | 1 + > src/gallium/include/pipe/p_shader_tokens.h | 7 +- > .../dri/i965/brw_fs_channel_expressions.cpp| 5 > src/mesa/main/extensions_table.h | 1 + > src/mesa/main/mtypes.h | 1 + > src/mesa/program/ir_to_mesa.cpp| 3 +++ > src/mesa/state_tracker/st_extensions.c | 1 + > src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 10 > 37 files changed, 190 insertions(+), 23 deletions(-) > > -- > 2.7.3 > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: for anonymous struct matching use without_array() (v3)
On Sun, Jun 5, 2016 at 9:30 PM, Dave Airlie wrote: > From: Dave Airlie > > With tessellation shaders we can have cases where we have > arrays of anon structs, so make sure we match using without_array(). > > Fixes: > GL45-CTS.tessellation_shader.tessellation_control_to_tessellation_evaluation.gl_in > > v2: > test lengths match as well (Ilia) > v3: > descend array lengths to check for matches as well (Ilia) > > Signed-off-by: Dave Airlie This looks good, but I have some petty suggestions which you're free to incorporate, or not. > --- > src/compiler/glsl/link_varyings.cpp | 29 ++--- > 1 file changed, 26 insertions(+), 3 deletions(-) > > diff --git a/src/compiler/glsl/link_varyings.cpp > b/src/compiler/glsl/link_varyings.cpp > index a286e77..1392095 100644 > --- a/src/compiler/glsl/link_varyings.cpp > +++ b/src/compiler/glsl/link_varyings.cpp > @@ -182,6 +182,31 @@ process_xfb_layout_qualifiers(void *mem_ctx, const > gl_shader *sh, > return has_xfb_qualifiers; > } > > +static bool > +anonymous_struct_type_matches(const glsl_type *output_type, > + const glsl_type *to_match) > +{ > +const glsl_type *output_tmp = output_type; > +const glsl_type *to_match_tmp = to_match; I'd personally not even have the _tmp vars, just use the args. > + > +while (output_tmp) { > +if (output_tmp->is_array() && to_match_tmp->is_array()) { You could just do while (output_tmp->is_array() && .. ) { I don't think output_tmp == null can ever happen. > +/* if the lengths at each level don't match fail. */ > +if (output_tmp->length != to_match_tmp->length) > +return false; > +output_tmp = output_tmp->fields.array; > +to_match_tmp = to_match_tmp->fields.array; > +} else { And then this would be outside of the while loop. IMHO it closer matches the flow of the code. Your call. Either way, Reviewed-by: Ilia Mirkin > +if (output_tmp->is_array() || to_match_tmp->is_array()) > +return false; > +return output_tmp->is_anonymous() && > + to_match_tmp->is_anonymous() && > + to_match_tmp->record_compare(output_tmp); > +} > +} > +return false; > +} > + > /** > * Validate the types and qualifiers of an output from one stage against the > * matching input to another stage. > @@ -226,9 +251,7 @@ cross_validate_types_and_qualifiers(struct > gl_shader_program *prog, > * fragment language." > */ >if (!output->type->is_array() || !is_gl_identifier(output->name)) { > - bool anon_matches = output->type->is_anonymous() && > -type_to_match->is_anonymous() && > -type_to_match->record_compare(output->type); > + bool anon_matches = anonymous_struct_type_matches(output->type, > type_to_match); > > if (!anon_matches) { > linker_error(prog, > -- > 2.7.4 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl: for anonymous struct matching use without_array() (v3)
From: Dave Airlie With tessellation shaders we can have cases where we have arrays of anon structs, so make sure we match using without_array(). Fixes: GL45-CTS.tessellation_shader.tessellation_control_to_tessellation_evaluation.gl_in v2: test lengths match as well (Ilia) v3: descend array lengths to check for matches as well (Ilia) Signed-off-by: Dave Airlie --- src/compiler/glsl/link_varyings.cpp | 29 ++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp index a286e77..1392095 100644 --- a/src/compiler/glsl/link_varyings.cpp +++ b/src/compiler/glsl/link_varyings.cpp @@ -182,6 +182,31 @@ process_xfb_layout_qualifiers(void *mem_ctx, const gl_shader *sh, return has_xfb_qualifiers; } +static bool +anonymous_struct_type_matches(const glsl_type *output_type, + const glsl_type *to_match) +{ +const glsl_type *output_tmp = output_type; +const glsl_type *to_match_tmp = to_match; + +while (output_tmp) { +if (output_tmp->is_array() && to_match_tmp->is_array()) { +/* if the lengths at each level don't match fail. */ +if (output_tmp->length != to_match_tmp->length) +return false; +output_tmp = output_tmp->fields.array; +to_match_tmp = to_match_tmp->fields.array; +} else { +if (output_tmp->is_array() || to_match_tmp->is_array()) +return false; +return output_tmp->is_anonymous() && + to_match_tmp->is_anonymous() && + to_match_tmp->record_compare(output_tmp); +} +} +return false; +} + /** * Validate the types and qualifiers of an output from one stage against the * matching input to another stage. @@ -226,9 +251,7 @@ cross_validate_types_and_qualifiers(struct gl_shader_program *prog, * fragment language." */ if (!output->type->is_array() || !is_gl_identifier(output->name)) { - bool anon_matches = output->type->is_anonymous() && -type_to_match->is_anonymous() && -type_to_match->record_compare(output->type); + bool anon_matches = anonymous_struct_type_matches(output->type, type_to_match); if (!anon_matches) { linker_error(prog, -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl: for anonymous struct matching use without_array() (v2)
From: Dave Airlie With tessellation shaders we can have cases where we have arrays of anon structs, so make sure we match using without_array(). Fixes: GL45-CTS.tessellation_shader.tessellation_control_to_tessellation_evaluation.gl_in v2: test lengths match as well (Ilia) Signed-off-by: Dave Airlie --- src/compiler/glsl/link_varyings.cpp | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp index a286e77..2252cad 100644 --- a/src/compiler/glsl/link_varyings.cpp +++ b/src/compiler/glsl/link_varyings.cpp @@ -226,9 +226,12 @@ cross_validate_types_and_qualifiers(struct gl_shader_program *prog, * fragment language." */ if (!output->type->is_array() || !is_gl_identifier(output->name)) { - bool anon_matches = output->type->is_anonymous() && -type_to_match->is_anonymous() && -type_to_match->record_compare(output->type); + const struct glsl_type *to_match_wa = type_to_match->without_array(); + const struct glsl_type *out_type_wa = output->type->without_array(); + bool anon_matches = out_type_wa->is_anonymous() && +to_match_wa->is_anonymous() && +to_match_wa->record_compare(out_type_wa) && +(type_to_match->array_size() == output->type->array_size()); if (!anon_matches) { linker_error(prog, -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl: for anonymous struct matching use without_array()
From: Dave Airlie With tessellation shaders we can have cases where we have arrays of anon structs, so make sure we match using without_array(). Fixes: GL45-CTS.tessellation_shader.tessellation_control_to_tessellation_evaluation.gl_in Signed-off-by: Dave Airlie --- src/compiler/glsl/link_varyings.cpp | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp index a286e77..395fef4 100644 --- a/src/compiler/glsl/link_varyings.cpp +++ b/src/compiler/glsl/link_varyings.cpp @@ -226,9 +226,11 @@ cross_validate_types_and_qualifiers(struct gl_shader_program *prog, * fragment language." */ if (!output->type->is_array() || !is_gl_identifier(output->name)) { - bool anon_matches = output->type->is_anonymous() && -type_to_match->is_anonymous() && -type_to_match->record_compare(output->type); + const struct glsl_type *to_match_wa = type_to_match->without_array(); + const struct glsl_type *out_type_wa = output->type->without_array(); + bool anon_matches = out_type_wa->is_anonymous() && +to_match_wa->is_anonymous() && +to_match_wa->record_compare(out_type_wa); if (!anon_matches) { linker_error(prog, -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] anv/pipeline: Add support for caching the push constant map
On Sat, Jun 4, 2016 at 3:39 PM, Jason Ekstrand wrote: > Signed-off-by: Jason Ekstrand > Cc: Kristian Høgsberg Kristensen Thanks for adding this, looks good to me. The copying of prog_data in anv_pipeline_cache_load() is a little fiddly, but I got nothing better... Reviewed-by: Kristian Høgsberg > --- > src/intel/vulkan/anv_pipeline_cache.c | 33 + > 1 file changed, 29 insertions(+), 4 deletions(-) > > diff --git a/src/intel/vulkan/anv_pipeline_cache.c > b/src/intel/vulkan/anv_pipeline_cache.c > index 62dbe3e..f75c423 100644 > --- a/src/intel/vulkan/anv_pipeline_cache.c > +++ b/src/intel/vulkan/anv_pipeline_cache.c > @@ -89,11 +89,15 @@ entry_size(struct cache_entry *entry) > * doesn't include the alignment padding bytes. > */ > > + struct brw_stage_prog_data *prog_data = (void *)entry->prog_data; > + const uint32_t param_size = > + prog_data->nr_params * sizeof(*prog_data->param); > + > const uint32_t map_size = >entry->surface_count * sizeof(struct anv_pipeline_binding) + >entry->sampler_count * sizeof(struct anv_pipeline_binding); > > - return sizeof(*entry) + entry->prog_data_size + map_size; > + return sizeof(*entry) + entry->prog_data_size + param_size + map_size; > } > > void > @@ -141,6 +145,7 @@ anv_pipeline_cache_search_unlocked(struct > anv_pipeline_cache *cache, > void *p = entry->prog_data; > *prog_data = p; > p += entry->prog_data_size; > +p += (*prog_data)->nr_params * sizeof(*(*prog_data)->param); > map->surface_count = entry->surface_count; > map->sampler_count = entry->sampler_count; > map->image_count = entry->image_count; > @@ -267,12 +272,18 @@ anv_pipeline_cache_upload_kernel(struct > anv_pipeline_cache *cache, > > struct cache_entry *entry; > > + assert((*prog_data)->nr_pull_params == 0); > + assert((*prog_data)->nr_image_params == 0); > + > + const uint32_t param_size = > + (*prog_data)->nr_params * sizeof(*(*prog_data)->param); > + > const uint32_t map_size = >map->surface_count * sizeof(struct anv_pipeline_binding) + >map->sampler_count * sizeof(struct anv_pipeline_binding); > > const uint32_t preamble_size = > - align_u32(sizeof(*entry) + prog_data_size + map_size, 64); > + align_u32(sizeof(*entry) + prog_data_size + param_size + map_size, 64); > > const uint32_t size = preamble_size + kernel_size; > > @@ -291,6 +302,10 @@ anv_pipeline_cache_upload_kernel(struct > anv_pipeline_cache *cache, > memcpy(p, *prog_data, prog_data_size); > p += prog_data_size; > > + memcpy(p, (*prog_data)->param, param_size); > + ((struct brw_stage_prog_data *)entry->prog_data)->param = p; > + p += param_size; > + > memcpy(p, map->surface_to_descriptor, >map->surface_count * sizeof(struct anv_pipeline_binding)); > map->surface_to_descriptor = p; > @@ -358,9 +373,17 @@ anv_pipeline_cache_load(struct anv_pipeline_cache *cache, >struct cache_entry *entry = p; > >void *data = entry->prog_data; > - const struct brw_stage_prog_data *prog_data = data; > + > + /* Make a copy of prog_data so that it's mutable */ > + uint8_t prog_data_tmp[512]; > + assert(entry->prog_data_size <= sizeof(prog_data_tmp)); > + memcpy(prog_data_tmp, data, entry->prog_data_size); > + struct brw_stage_prog_data *prog_data = (void *)prog_data_tmp; >data += entry->prog_data_size; > > + prog_data->param = data; > + data += prog_data->nr_params * sizeof(*prog_data->param); > + >struct anv_pipeline_binding *surface_to_descriptor = data; >data += entry->surface_count * sizeof(struct anv_pipeline_binding); >struct anv_pipeline_binding *sampler_to_descriptor = data; > @@ -375,9 +398,11 @@ anv_pipeline_cache_load(struct anv_pipeline_cache *cache, > .sampler_to_descriptor = sampler_to_descriptor >}; > > + const struct brw_stage_prog_data *const_prog_data = prog_data; > + >anv_pipeline_cache_upload_kernel(cache, entry->sha1, > kernel, entry->kernel_size, > - &prog_data, > + &const_prog_data, > entry->prog_data_size, &map); >p = kernel + entry->kernel_size; > } > -- > 2.5.0.400.gff86faf > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v4] clover: Update OpenCL version string to match OpenGL
Vedran Miletić writes: > Change MESA into Mesa in CL_PLATFORM_VERSION and CL_DEVICE_VERSION. For > both, always append git version suffix from git_sha1.h. > > v4: dropped #ifdef guards. > > It would be very nice if this could be considered for 12.0 release. > --- > src/gallium/state_trackers/clover/api/device.cpp | 5 - > src/gallium/state_trackers/clover/api/platform.cpp | 5 - > 2 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/src/gallium/state_trackers/clover/api/device.cpp > b/src/gallium/state_trackers/clover/api/device.cpp > index bc93f91..0d3de45 100644 > --- a/src/gallium/state_trackers/clover/api/device.cpp > +++ b/src/gallium/state_trackers/clover/api/device.cpp > @@ -23,6 +23,7 @@ > #include "api/util.hpp" > #include "core/platform.hpp" > #include "core/device.hpp" > +#include "git_sha1.h" > > using namespace clover; > > @@ -300,7 +301,9 @@ clGetDeviceInfo(cl_device_id d_dev, cl_device_info param, >break; > > case CL_DEVICE_VERSION: > - buf.as_string() = "OpenCL 1.1 MESA " PACKAGE_VERSION; > + buf.as_string() = "OpenCL 1.1 Mesa " PACKAGE_VERSION > +" (" MESA_GIT_SHA1 ")" > +; No need to put the semicolon in a separate line. >break; > > case CL_DEVICE_EXTENSIONS: > diff --git a/src/gallium/state_trackers/clover/api/platform.cpp > b/src/gallium/state_trackers/clover/api/platform.cpp > index cf71593..1784fbc 100644 > --- a/src/gallium/state_trackers/clover/api/platform.cpp > +++ b/src/gallium/state_trackers/clover/api/platform.cpp > @@ -22,6 +22,7 @@ > > #include "api/util.hpp" > #include "core/platform.hpp" > +#include "git_sha1.h" > > using namespace clover; > > @@ -57,7 +58,9 @@ clover::GetPlatformInfo(cl_platform_id d_platform, > cl_platform_info param, >break; > > case CL_PLATFORM_VERSION: > - buf.as_string() = "OpenCL 1.1 MESA " PACKAGE_VERSION; > + buf.as_string() = "OpenCL 1.1 Mesa " PACKAGE_VERSION > +" (" MESA_GIT_SHA1 ")" > +; Same here. With my nitpicks addressed: Reviewed-by: Francisco Jerez >break; > > case CL_PLATFORM_NAME: > -- > 2.7.4 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v1.1 2/2] clover: fix getting scalar args api size
Vedran Miletić writes: > On 06/04/2016 04:27 AM, Francisco Jerez wrote: >> Serge Martin writes: >> >>> This fix getting the size of a struct arg. vec3 types still work ok. >>> Only buit-in args need to have power of two alignment, getTypeAllocSize >>> reports the correct size. >>> >> Is there any guarantee that the alloc size of the type will match what >> the CL API expects? (which is the only thing arg_api_size is used for >> IIRC) Isn't it fully dependent on the data layout which is pretty much >> up to the target? >> > > It is dependent on the target. The issue is that right now we get the > struct size wrongly, as we round it here to the next power of two, store > that in scalar arg object, and later compare that rounded value to > sizeof(struct_arg_type) which user has passed via the API. > I know, but using the target data layout-dependent alloc size of the type as API-visible size seems pretty bogus too -- Unless we set as requirement on all targets that their data alignment rules should match the CL API's exactly, which I don't think is the case right now but doesn't sound like a completely crazy idea to me. > Regards, > Vedran > > -- > Vedran Miletić > vedran.miletic.net signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] clover: Pass unquoted compiler arguments to Clang
Vedran Miletić writes: > On 06/04/2016 10:13 AM, Francisco Jerez wrote: >> Vedran Miletić writes: >> >>> OpenCL apps can quote arguments they pass to the OpenCL compiler, most >>> commonly include paths containing spaces. If the OpenCL compiler was >>> called via a shell, the shell would remove (single or double) quotes >>> before passing the argument to the compiler. Since we call Clang as a >>> library, we have to remove quotes before passing the argument. >> >> Sigh, it's a shame that the OpenCL spec doesn't specify what format the >> option string should have, whether quotes have any special >> interpretation at all, whether escape sequences are supported, etc. >> Might be worth reporting the problem at Khronos -- Or have you found any >> evidence in the spec that the parsing logic you implement below is the >> correct one? >> > > Aside from working just like NVIDIA and AMD proprietary stacks, no. > > However, what we do right now is most certainly broken. Consider the > following argument > > -I"/foo bar/baz" > > which is going to be sent to Clang as two arguments broken over space, > while it should be one. But the OpenCL spec doesn't impose any explicit requirements on the formatting of the option string (unless I'm missing something), so strictly speaking both interpretations are equally broken. It wasn't my intention to argue against implementing the same behaviour as the proprietary CL stacks as temporary solution, but the fact that this is unspecified is certainly an OpenCL spec bug. > We should split arguments correctly, and whether or not we remove > quotes in the process or we expect Clang to handle them is the easier > part of the job. > > Regards, > Vedran > > -- > Vedran Miletić > vedran.miletic.net signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [AppVeyor] mesa master #1517 completed
Build mesa 1517 completed Commit f657a59d98 by Kenneth Graunke on 6/5/2016 11:31 PM: mesa: Try to unbreak the MSVC build.\n\nPATH_MAX is apparently not a thing on Windows. Borrow the hack from\npipe_loader.c to try and make this work.\n\nSigned-off-by: Kenneth Graunke Configure your notification preferences ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v4] clover: Update OpenCL version string to match OpenGL
On 06/06/2016 12:44 AM, Vedran Miletić wrote: Change MESA into Mesa in CL_PLATFORM_VERSION and CL_DEVICE_VERSION. For both, always append git version suffix from git_sha1.h. v4: dropped #ifdef guards. It would be very nice if this could be considered for 12.0 release. --- src/gallium/state_trackers/clover/api/device.cpp | 5 - src/gallium/state_trackers/clover/api/platform.cpp | 5 - 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/gallium/state_trackers/clover/api/device.cpp b/src/gallium/state_trackers/clover/api/device.cpp index bc93f91..0d3de45 100644 --- a/src/gallium/state_trackers/clover/api/device.cpp +++ b/src/gallium/state_trackers/clover/api/device.cpp @@ -23,6 +23,7 @@ #include "api/util.hpp" #include "core/platform.hpp" #include "core/device.hpp" +#include "git_sha1.h" using namespace clover; @@ -300,7 +301,9 @@ clGetDeviceInfo(cl_device_id d_dev, cl_device_info param, break; case CL_DEVICE_VERSION: - buf.as_string() = "OpenCL 1.1 MESA " PACKAGE_VERSION; + buf.as_string() = "OpenCL 1.1 Mesa " PACKAGE_VERSION +" (" MESA_GIT_SHA1 ")" +; break; case CL_DEVICE_EXTENSIONS: diff --git a/src/gallium/state_trackers/clover/api/platform.cpp b/src/gallium/state_trackers/clover/api/platform.cpp index cf71593..1784fbc 100644 --- a/src/gallium/state_trackers/clover/api/platform.cpp +++ b/src/gallium/state_trackers/clover/api/platform.cpp @@ -22,6 +22,7 @@ #include "api/util.hpp" #include "core/platform.hpp" +#include "git_sha1.h" using namespace clover; @@ -57,7 +58,9 @@ clover::GetPlatformInfo(cl_platform_id d_platform, cl_platform_info param, break; case CL_PLATFORM_VERSION: - buf.as_string() = "OpenCL 1.1 MESA " PACKAGE_VERSION; + buf.as_string() = "OpenCL 1.1 Mesa " PACKAGE_VERSION +" (" MESA_GIT_SHA1 ")" +; break; case CL_PLATFORM_NAME: CC'ing mesa-stable as well. -- Vedran Miletić vedran.miletic.net ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function
On Sun, Jun 5, 2016 at 6:30 PM, Emil Velikov wrote: > On 5 June 2016 at 23:22, Ilia Mirkin wrote: >> You're arguing about clarity with the ~2 active developers/reviewers >> who understand the code. > What I'm saying is that with contradicting commit messages like this > one there'll be little others who will understand it :-\ If it were that easy to get people to be interested in development, I'd write a novel for every commit. -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v1.1 2/2] clover: fix getting scalar args api size
On 06/04/2016 04:27 AM, Francisco Jerez wrote: Serge Martin writes: This fix getting the size of a struct arg. vec3 types still work ok. Only buit-in args need to have power of two alignment, getTypeAllocSize reports the correct size. Is there any guarantee that the alloc size of the type will match what the CL API expects? (which is the only thing arg_api_size is used for IIRC) Isn't it fully dependent on the data layout which is pretty much up to the target? It is dependent on the target. The issue is that right now we get the struct size wrongly, as we round it here to the next power of two, store that in scalar arg object, and later compare that rounded value to sizeof(struct_arg_type) which user has passed via the API. Regards, Vedran -- Vedran Miletić vedran.miletic.net ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] nv50: reinstate dedicated constbuf push path
This was disabled due to occasionally incorrect behavior when trying to upload data. It later became apparent that nvc0 also had a similar but slightly different issue, which was resolved in commit e50c01d5. This takes the same logic as nvc0 and applies it to nv50 (which has somewhat different interfaces). Unfortunately I did not note down precisely what was broken with UBOs when removing the support from nv50, but I've tested a bunch of local traces, and none of them appear to regress. This should hopefully improve performance when UBOs are used, but this was not directly verified. Signed-off-by: Ilia Mirkin --- src/gallium/drivers/nouveau/nv50/nv50_context.c| 7 --- src/gallium/drivers/nouveau/nv50/nv50_context.h| 3 +- .../drivers/nouveau/nv50/nv50_shader_state.c | 1 + src/gallium/drivers/nouveau/nv50/nv50_state.c | 5 +- src/gallium/drivers/nouveau/nv50/nv50_transfer.c | 63 +++--- 5 files changed, 50 insertions(+), 29 deletions(-) diff --git a/src/gallium/drivers/nouveau/nv50/nv50_context.c b/src/gallium/drivers/nouveau/nv50/nv50_context.c index 8a148fc..84b5e3f 100644 --- a/src/gallium/drivers/nouveau/nv50/nv50_context.c +++ b/src/gallium/drivers/nouveau/nv50/nv50_context.c @@ -318,14 +318,7 @@ nv50_create(struct pipe_screen *pscreen, void *priv, unsigned ctxflags) nv50->base.screen= &screen->base; nv50->base.copy_data = nv50_m2mf_copy_linear; nv50->base.push_data = nv50_sifc_linear_u8; - /* FIXME: Make it possible to use this again. The problem is that there is -* some clever logic in the card that allows for multiple renders to happen -* when there are only constbuf changes. However that relies on the -* constbuf updates happening to the right constbuf slots. Currently -* implementation just makes it go through a separate slot which doesn't -* properly update the right constbuf data. nv50->base.push_cb = nv50_cb_push; -*/ nv50->screen = screen; pipe->screen = pscreen; diff --git a/src/gallium/drivers/nouveau/nv50/nv50_context.h b/src/gallium/drivers/nouveau/nv50/nv50_context.h index b7963a4..0582e24 100644 --- a/src/gallium/drivers/nouveau/nv50/nv50_context.h +++ b/src/gallium/drivers/nouveau/nv50/nv50_context.h @@ -286,8 +286,7 @@ nv50_m2mf_copy_linear(struct nouveau_context *pipe, unsigned size); void nv50_cb_push(struct nouveau_context *nv, - struct nouveau_bo *bo, unsigned domain, - unsigned base, unsigned size, + struct nv04_resource *res, unsigned offset, unsigned words, const uint32_t *data); /* nv50_vbo.c */ diff --git a/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c b/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c index f838d15..2326394 100644 --- a/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c +++ b/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c @@ -99,6 +99,7 @@ nv50_constbufs_validate(struct nv50_context *nv50) BCTX_REFN(nv50->bufctx_3d, 3D_CB(s, i), res, RD); nv50->cb_dirty = 1; /* Force cache flush for UBO. */ + res->cb_bindings[s] |= 1 << i; } else { BEGIN_NV04(push, NV50_3D(SET_PROGRAM_CB), 1); PUSH_DATA (push, (i << 8) | p | 0); diff --git a/src/gallium/drivers/nouveau/nv50/nv50_state.c b/src/gallium/drivers/nouveau/nv50/nv50_state.c index 86e74d6..3231aba 100644 --- a/src/gallium/drivers/nouveau/nv50/nv50_state.c +++ b/src/gallium/drivers/nouveau/nv50/nv50_state.c @@ -856,9 +856,10 @@ nv50_set_constant_buffer(struct pipe_context *pipe, uint shader, uint index, if (nv50->constbuf[s][i].user) nv50->constbuf[s][i].u.buf = NULL; else - if (nv50->constbuf[s][i].u.buf) + if (nv50->constbuf[s][i].u.buf) { nouveau_bufctx_reset(nv50->bufctx_3d, NV50_BIND_3D_CB(s, i)); - + nv04_resource(nv50->constbuf[s][i].u.buf)->cb_bindings[s] &= ~(1 << i); + } pipe_resource_reference(&nv50->constbuf[s][i].u.buf, res); nv50->constbuf[s][i].user = (cb && cb->user_buffer) ? true : false; diff --git a/src/gallium/drivers/nouveau/nv50/nv50_transfer.c b/src/gallium/drivers/nouveau/nv50/nv50_transfer.c index f5c7c57..c751ade 100644 --- a/src/gallium/drivers/nouveau/nv50/nv50_transfer.c +++ b/src/gallium/drivers/nouveau/nv50/nv50_transfer.c @@ -377,32 +377,24 @@ nv50_miptree_transfer_unmap(struct pipe_context *pctx, FREE(tx); } -void -nv50_cb_push(struct nouveau_context *nv, - struct nouveau_bo *bo, unsigned domain, - unsigned base, unsigned size, - unsigned offset, unsigned words, const uint32_t *data) +static void +nv50_cb_bo_push(struct nouveau_context *nv, +struct nouveau_bo *bo, unsigned domain, +unsigned bufid, +unsigned offset, unsigned words, +const uint32_t *data) { struct nouveau_pushbuf *push = nv->pushbuf; - struct nouveau_bufctx *bctx = nv50_
Re: [Mesa-dev] [PATCH 1/2] clover: assert struct argument is compiled usably
On 06/04/2016 04:18 AM, Francisco Jerez wrote: Serge Martin writes: From: Vedran Miletić Make sure that a struct argument did not get compiled into a pointer type with the byval attribute. If we try to handle the pointer with byval, we end up with the pointer size instead of the struct size. Ugh, is that a bug in the code below? How are byval pointers supposed to be handled here? Exactly as if the argument wasn't a pointer at all by providing a copy of the pointed-to object as-is in the kernel input buffer? In that case wouldn't the code below need to pass the correct size of the pointed-to object as target/api size rather than the size of the pointer? Yes, byval+pointer should be handled as there is no pointer at all. I have tried passing the correct size, but IIRC LLVM AMDGPU backend does not generate correct asm for byval+pointer variant. The simple solution is to fail with an assert here unless Clang generates code both Clover and the backend can handle. Regards, Vedran -- Vedran Miletić vedran.miletic.net ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] st/mesa: check shader image format support before using PBO download
ARB_shader_image_load_store only requires a very fixed list of formats to be supported, while textures may be in all kinds of formats, like BGRA which are presently not supported on at least Kepler. Signed-off-by: Ilia Mirkin --- src/mesa/state_tracker/st_cb_readpixels.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/src/mesa/state_tracker/st_cb_readpixels.c b/src/mesa/state_tracker/st_cb_readpixels.c index 6df3a39..09450c9 100644 --- a/src/mesa/state_tracker/st_cb_readpixels.c +++ b/src/mesa/state_tracker/st_cb_readpixels.c @@ -79,6 +79,7 @@ try_pbo_readpixels(struct st_context *st, struct st_renderbuffer *strb, const struct gl_pixelstore_attrib *pack, void *pixels) { struct pipe_context *pipe = st->pipe; + struct pipe_screen *screen = pipe->screen; struct cso_context *cso = st->cso_context; struct pipe_surface *surface = strb->surface; struct pipe_resource *texture = strb->texture; @@ -91,6 +92,11 @@ try_pbo_readpixels(struct st_context *st, struct st_renderbuffer *strb, if (texture->nr_samples > 1) return false; + if (!screen->is_format_supported(screen, dst_format, PIPE_TEXTURE_2D, +texture->nr_samples, +PIPE_BIND_SHADER_IMAGE)) + return false; + desc = util_format_description(dst_format); /* Compute PBO addresses */ -- 2.7.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] clover: Pass unquoted compiler arguments to Clang
On 06/04/2016 10:13 AM, Francisco Jerez wrote: Vedran Miletić writes: OpenCL apps can quote arguments they pass to the OpenCL compiler, most commonly include paths containing spaces. If the OpenCL compiler was called via a shell, the shell would remove (single or double) quotes before passing the argument to the compiler. Since we call Clang as a library, we have to remove quotes before passing the argument. Sigh, it's a shame that the OpenCL spec doesn't specify what format the option string should have, whether quotes have any special interpretation at all, whether escape sequences are supported, etc. Might be worth reporting the problem at Khronos -- Or have you found any evidence in the spec that the parsing logic you implement below is the correct one? Aside from working just like NVIDIA and AMD proprietary stacks, no. However, what we do right now is most certainly broken. Consider the following argument -I"/foo bar/baz" which is going to be sent to Clang as two arguments broken over space, while it should be one. We should split arguments correctly, and whether or not we remove quotes in the process or we expect Clang to handle them is the easier part of the job. Regards, Vedran -- Vedran Miletić vedran.miletic.net ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v4] clover: Update OpenCL version string to match OpenGL
Change MESA into Mesa in CL_PLATFORM_VERSION and CL_DEVICE_VERSION. For both, always append git version suffix from git_sha1.h. v4: dropped #ifdef guards. It would be very nice if this could be considered for 12.0 release. --- src/gallium/state_trackers/clover/api/device.cpp | 5 - src/gallium/state_trackers/clover/api/platform.cpp | 5 - 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/gallium/state_trackers/clover/api/device.cpp b/src/gallium/state_trackers/clover/api/device.cpp index bc93f91..0d3de45 100644 --- a/src/gallium/state_trackers/clover/api/device.cpp +++ b/src/gallium/state_trackers/clover/api/device.cpp @@ -23,6 +23,7 @@ #include "api/util.hpp" #include "core/platform.hpp" #include "core/device.hpp" +#include "git_sha1.h" using namespace clover; @@ -300,7 +301,9 @@ clGetDeviceInfo(cl_device_id d_dev, cl_device_info param, break; case CL_DEVICE_VERSION: - buf.as_string() = "OpenCL 1.1 MESA " PACKAGE_VERSION; + buf.as_string() = "OpenCL 1.1 Mesa " PACKAGE_VERSION +" (" MESA_GIT_SHA1 ")" +; break; case CL_DEVICE_EXTENSIONS: diff --git a/src/gallium/state_trackers/clover/api/platform.cpp b/src/gallium/state_trackers/clover/api/platform.cpp index cf71593..1784fbc 100644 --- a/src/gallium/state_trackers/clover/api/platform.cpp +++ b/src/gallium/state_trackers/clover/api/platform.cpp @@ -22,6 +22,7 @@ #include "api/util.hpp" #include "core/platform.hpp" +#include "git_sha1.h" using namespace clover; @@ -57,7 +58,9 @@ clover::GetPlatformInfo(cl_platform_id d_platform, cl_platform_info param, break; case CL_PLATFORM_VERSION: - buf.as_string() = "OpenCL 1.1 MESA " PACKAGE_VERSION; + buf.as_string() = "OpenCL 1.1 Mesa " PACKAGE_VERSION +" (" MESA_GIT_SHA1 ")" +; break; case CL_PLATFORM_NAME: -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function
On 06/06/2016 12:30 AM, Emil Velikov wrote: On 5 June 2016 at 23:22, Ilia Mirkin wrote: On Sun, Jun 5, 2016 at 6:08 PM, Emil Velikov wrote: On 5 June 2016 at 23:00, Samuel Pitoiset wrote: On 06/05/2016 11:50 PM, Emil Velikov wrote: On 5 June 2016 at 22:36, Ilia Mirkin wrote: On Sun, Jun 5, 2016 at 5:34 PM, Emil Velikov wrote: On 5 June 2016 at 22:17, Ilia Mirkin wrote: On Sun, Jun 5, 2016 at 5:16 PM, Emil Velikov wrote: On 5 June 2016 at 22:13, Emil Velikov wrote: On 5 June 2016 at 17:56, Samuel Pitoiset wrote: We should not call nouveau_bufctx_reset() inside a validate function. This seems to contradict the changes introduced in nvc0_compute.c. Worth explaining a bit better the dos and don'ts ? As this is already in master, can you please provide a more elaborate/correct summary for -stable ? I think it's fine as is. Do: reset bufctx when setting dirty bit Don't: reset bufctx in validate logic, since it's "too late" by then. (Not strictly wrong, but just should do it earlier.) So nvc0_compute_*validate*_surfaces is not validate logic ? Err... what a confusing name it has ;-) It validates compute. And it invalidates (and clears) the 3d bin. So one can reset_bufctx(3d) from the compute validate and vice-versa. While doing reset_bufctx(foo) from foo validate is a bad idea ? Shouldn't one just say so in the commit message ? Because the common practice is to clear foo bins at the same place where the dirty_3d |= foo is updated, this makes sense. :) Yet the commit message does not say that, right ? It says "We should not call nouveau_bufctx_reset() inside a validate function.", while the patch does the complete opposite - it adds a call to nouveau_bufctx_reset() inside a validate function. All I'm asking is for the commit message to reflect the code change or vice-versa. I hope I'm not being unreasonable ? The commit message also doesn't explain what bufctx's are, what a validation function is, and the overall structure of the nouveau code and how it uses those bufctx's. You're arguing about clarity with the ~2 active developers/reviewers who understand the code. What I'm saying is that with contradicting commit messages like this one there'll be little others who will understand it :-\ I understand that this might be unclear to you, but I know I'm not about to explain everything in commit messages all the time -- too much effort for zero benefit. Case in point - had this commit said "nvc0: fix validation logic" and left it at that, we wouldn't be having this discussion. But there was a bit of an explanation, that was perhaps not infinitely precise, and now there's a long discussion about how it's unclear. Looking from another angle - is false information better than no information ? All I was asking is to correct the commit message. I wasn't asking that one should explain reasoning behind it or anything deeper into the stack, which I believe was understood. Seems like that is too much to ask so I won't bother you guys any more. I'll make an effort the next time. At least, try to not introduce contradictions between the code and the commit message. Thanks Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function
On 5 June 2016 at 23:22, Ilia Mirkin wrote: > On Sun, Jun 5, 2016 at 6:08 PM, Emil Velikov wrote: >> On 5 June 2016 at 23:00, Samuel Pitoiset wrote: >>> >>> >>> On 06/05/2016 11:50 PM, Emil Velikov wrote: On 5 June 2016 at 22:36, Ilia Mirkin wrote: > > On Sun, Jun 5, 2016 at 5:34 PM, Emil Velikov > wrote: >> >> On 5 June 2016 at 22:17, Ilia Mirkin wrote: >>> >>> On Sun, Jun 5, 2016 at 5:16 PM, Emil Velikov >>> wrote: On 5 June 2016 at 22:13, Emil Velikov wrote: > > On 5 June 2016 at 17:56, Samuel Pitoiset > wrote: >> >> We should not call nouveau_bufctx_reset() inside a validate >> function. > > This seems to contradict the changes introduced in nvc0_compute.c. > Worth explaining a bit better the dos and don'ts ? > As this is already in master, can you please provide a more elaborate/correct summary for -stable ? >>> >>> >>> I think it's fine as is. >>> >>> Do: reset bufctx when setting dirty bit >>> Don't: reset bufctx in validate logic, since it's "too late" by then. >>> (Not strictly wrong, but just should do it earlier.) >> >> >> So nvc0_compute_*validate*_surfaces is not validate logic ? Err... >> what a confusing name it has ;-) > > > It validates compute. And it invalidates (and clears) the 3d bin. > So one can reset_bufctx(3d) from the compute validate and vice-versa. While doing reset_bufctx(foo) from foo validate is a bad idea ? Shouldn't one just say so in the commit message ? >>> >>> >>> Because the common practice is to clear foo bins at the same place where the >>> dirty_3d |= foo is updated, this makes sense. :) >>> >> Yet the commit message does not say that, right ? It says "We should >> not call nouveau_bufctx_reset() inside a validate function.", while >> the patch does the complete opposite - it adds a call to >> nouveau_bufctx_reset() inside a validate function. >> >> All I'm asking is for the commit message to reflect the code change or >> vice-versa. I hope I'm not being unreasonable ? > > The commit message also doesn't explain what bufctx's are, what a > validation function is, and the overall structure of the nouveau code > and how it uses those bufctx's. > > You're arguing about clarity with the ~2 active developers/reviewers > who understand the code. What I'm saying is that with contradicting commit messages like this one there'll be little others who will understand it :-\ > I understand that this might be unclear to > you, but I know I'm not about to explain everything in commit messages > all the time -- too much effort for zero benefit. Case in point - had > this commit said "nvc0: fix validation logic" and left it at that, we > wouldn't be having this discussion. But there was a bit of an > explanation, that was perhaps not infinitely precise, and now there's > a long discussion about how it's unclear. > Looking from another angle - is false information better than no information ? All I was asking is to correct the commit message. I wasn't asking that one should explain reasoning behind it or anything deeper into the stack, which I believe was understood. Seems like that is too much to ask so I won't bother you guys any more. Thanks Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] radeonsi: Save and restore entire CE RAM.
This fixes a problem with the CE preamble and restoring only stuff in the preamble when needed. To illustrate suppose we have two graphics IB's 1 and 2, which are submitted in that order. Furthermore suppose IB 1 does not use CE ram, but IB 2 does, and we have a context switch at the start of IB 1, but not between IB 1 and IB 2. The old code put the CE RAM loads in the preamble of IB 2. As the preamble of IB 1 does not have the loads and the preamble of IB 2 does not get executed, the old values are not load into CE RAM. Fix this by always restoring the entire CE RAM. Signed-off-by: Bas Nieuwenhuizen Cc: "12.0" --- src/gallium/drivers/radeonsi/si_descriptors.c | 56 +-- src/gallium/drivers/radeonsi/si_hw_context.c | 4 ++ src/gallium/drivers/radeonsi/si_pipe.c| 7 src/gallium/drivers/radeonsi/si_pipe.h| 1 + src/gallium/drivers/radeonsi/si_state.h | 7 ++-- 5 files changed, 43 insertions(+), 32 deletions(-) diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c b/src/gallium/drivers/radeonsi/si_descriptors.c index baddc5f..5a1a344 100644 --- a/src/gallium/drivers/radeonsi/si_descriptors.c +++ b/src/gallium/drivers/radeonsi/si_descriptors.c @@ -160,30 +160,39 @@ static bool si_ce_upload(struct si_context *sctx, unsigned ce_offset, unsigned s return true; } -static void si_reinitialize_ce_ram(struct si_context *sctx, -struct si_descriptors *desc) +void si_ce_save_ram(struct si_context *sctx) { - if (desc->buffer) { - struct r600_resource *buffer = (struct r600_resource*)desc->buffer; - unsigned list_size = desc->num_elements * desc->element_dw_size * 4; - uint64_t va = buffer->gpu_address + desc->buffer_offset; - struct radeon_winsys_cs *ib = sctx->ce_preamble_ib; + if (!sctx->ce_ib) + return; - if (!ib) - ib = sctx->ce_ib; + radeon_emit(sctx->ce_ib, PKT3(PKT3_DUMP_CONST_RAM, 3, 0)); + radeon_emit(sctx->ce_ib, 0); + radeon_emit(sctx->ce_ib, SI_CE_RAM_SIZE / 4); + radeon_emit(sctx->ce_ib, sctx->ce_ram_data->gpu_address); + radeon_emit(sctx->ce_ib, sctx->ce_ram_data->gpu_address >> 32); - list_size = align(list_size, 32); + radeon_add_to_buffer_list(&sctx->b, &sctx->b.gfx, sctx->ce_ram_data, + RADEON_USAGE_WRITE, RADEON_PRIO_DESCRIPTORS); +} - radeon_emit(ib, PKT3(PKT3_LOAD_CONST_RAM, 3, 0)); - radeon_emit(ib, va); - radeon_emit(ib, va >> 32); - radeon_emit(ib, list_size / 4); - radeon_emit(ib, desc->ce_offset); +void si_ce_restore_ram(struct si_context *sctx) +{ + struct radeon_winsys_cs *ib = sctx->ce_preamble_ib; - radeon_add_to_buffer_list(&sctx->b, &sctx->b.gfx, desc->buffer, - RADEON_USAGE_READ, RADEON_PRIO_DESCRIPTORS); - } - desc->ce_ram_dirty = false; + if (!ib) + ib = sctx->ce_ib; + + if (!ib) + return; + + radeon_emit(ib, PKT3(PKT3_LOAD_CONST_RAM, 3, 0)); + radeon_emit(ib, sctx->ce_ram_data->gpu_address); + radeon_emit(ib, sctx->ce_ram_data->gpu_address >> 32); + radeon_emit(ib, SI_CE_RAM_SIZE / 4); + radeon_emit(ib, 0); + + radeon_add_to_buffer_list(&sctx->b, &sctx->b.gfx, sctx->ce_ram_data, + RADEON_USAGE_READ, RADEON_PRIO_DESCRIPTORS); } void si_ce_enable_loads(struct radeon_winsys_cs *ib) @@ -206,9 +215,6 @@ static bool si_upload_descriptors(struct si_context *sctx, if (sctx->ce_ib) { uint32_t const* list = (uint32_t const*)desc->list; - if (desc->ce_ram_dirty) - si_reinitialize_ce_ram(sctx, desc); - while(desc->dirty_mask) { int begin, count; u_bit_scan_consecutive_range(&desc->dirty_mask, &begin, @@ -287,8 +293,6 @@ static void si_sampler_views_begin_new_cs(struct si_context *sctx, RADEON_USAGE_READ); } - views->desc.ce_ram_dirty = true; - if (!views->desc.buffer) return; radeon_add_to_buffer_list(&sctx->b, &sctx->b.gfx, views->desc.buffer, @@ -489,8 +493,6 @@ si_image_views_begin_new_cs(struct si_context *sctx, struct si_images_info *imag RADEON_USAGE_READWRITE); } - images->desc.ce_ram_dirty = true; - if (images->desc.buffer) { radeon_add_to_buffer_list(&sctx->b, &sctx->b.gfx, images->desc.buffer, @@ -737,8 +739,6 @@ static void si_buffer_resources_begin_new_cs(struct si_context *sctx, buffers->shader_usage, buffers->priority); } - buffers->desc.ce_ra
Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function
On Sun, Jun 5, 2016 at 6:08 PM, Emil Velikov wrote: > On 5 June 2016 at 23:00, Samuel Pitoiset wrote: >> >> >> On 06/05/2016 11:50 PM, Emil Velikov wrote: >>> >>> On 5 June 2016 at 22:36, Ilia Mirkin wrote: On Sun, Jun 5, 2016 at 5:34 PM, Emil Velikov wrote: > > On 5 June 2016 at 22:17, Ilia Mirkin wrote: >> >> On Sun, Jun 5, 2016 at 5:16 PM, Emil Velikov >> wrote: >>> >>> On 5 June 2016 at 22:13, Emil Velikov >>> wrote: On 5 June 2016 at 17:56, Samuel Pitoiset wrote: > > We should not call nouveau_bufctx_reset() inside a validate > function. This seems to contradict the changes introduced in nvc0_compute.c. Worth explaining a bit better the dos and don'ts ? >>> As this is already in master, can you please provide a more >>> elaborate/correct summary for -stable ? >> >> >> I think it's fine as is. >> >> Do: reset bufctx when setting dirty bit >> Don't: reset bufctx in validate logic, since it's "too late" by then. >> (Not strictly wrong, but just should do it earlier.) > > > So nvc0_compute_*validate*_surfaces is not validate logic ? Err... > what a confusing name it has ;-) It validates compute. And it invalidates (and clears) the 3d bin. >>> So one can reset_bufctx(3d) from the compute validate and vice-versa. >>> While doing reset_bufctx(foo) from foo validate is a bad idea ? >>> Shouldn't one just say so in the commit message ? >> >> >> Because the common practice is to clear foo bins at the same place where the >> dirty_3d |= foo is updated, this makes sense. :) >> > Yet the commit message does not say that, right ? It says "We should > not call nouveau_bufctx_reset() inside a validate function.", while > the patch does the complete opposite - it adds a call to > nouveau_bufctx_reset() inside a validate function. > > All I'm asking is for the commit message to reflect the code change or > vice-versa. I hope I'm not being unreasonable ? The commit message also doesn't explain what bufctx's are, what a validation function is, and the overall structure of the nouveau code and how it uses those bufctx's. You're arguing about clarity with the ~2 active developers/reviewers who understand the code. I understand that this might be unclear to you, but I know I'm not about to explain everything in commit messages all the time -- too much effort for zero benefit. Case in point - had this commit said "nvc0: fix validation logic" and left it at that, we wouldn't be having this discussion. But there was a bit of an explanation, that was perhaps not infinitely precise, and now there's a long discussion about how it's unclear. -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function
On 06/06/2016 12:08 AM, Emil Velikov wrote: On 5 June 2016 at 23:00, Samuel Pitoiset wrote: On 06/05/2016 11:50 PM, Emil Velikov wrote: On 5 June 2016 at 22:36, Ilia Mirkin wrote: On Sun, Jun 5, 2016 at 5:34 PM, Emil Velikov wrote: On 5 June 2016 at 22:17, Ilia Mirkin wrote: On Sun, Jun 5, 2016 at 5:16 PM, Emil Velikov wrote: On 5 June 2016 at 22:13, Emil Velikov wrote: On 5 June 2016 at 17:56, Samuel Pitoiset wrote: We should not call nouveau_bufctx_reset() inside a validate function. This seems to contradict the changes introduced in nvc0_compute.c. Worth explaining a bit better the dos and don'ts ? As this is already in master, can you please provide a more elaborate/correct summary for -stable ? I think it's fine as is. Do: reset bufctx when setting dirty bit Don't: reset bufctx in validate logic, since it's "too late" by then. (Not strictly wrong, but just should do it earlier.) So nvc0_compute_*validate*_surfaces is not validate logic ? Err... what a confusing name it has ;-) It validates compute. And it invalidates (and clears) the 3d bin. So one can reset_bufctx(3d) from the compute validate and vice-versa. While doing reset_bufctx(foo) from foo validate is a bad idea ? Shouldn't one just say so in the commit message ? Because the common practice is to clear foo bins at the same place where the dirty_3d |= foo is updated, this makes sense. :) Yet the commit message does not say that, right ? It says "We should not call nouveau_bufctx_reset() inside a validate function.", while the patch does the complete opposite - it adds a call to nouveau_bufctx_reset() inside a validate function. All I'm asking is for the commit message to reflect the code change or vice-versa. I hope I'm not being unreasonable ? You are reasonable. I assume something like "nvc0: clear out surfaces bins at the right place" (and explain why in the description) is more close to what the code does. Thanks Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function
On 5 June 2016 at 23:00, Samuel Pitoiset wrote: > > > On 06/05/2016 11:50 PM, Emil Velikov wrote: >> >> On 5 June 2016 at 22:36, Ilia Mirkin wrote: >>> >>> On Sun, Jun 5, 2016 at 5:34 PM, Emil Velikov >>> wrote: On 5 June 2016 at 22:17, Ilia Mirkin wrote: > > On Sun, Jun 5, 2016 at 5:16 PM, Emil Velikov > wrote: >> >> On 5 June 2016 at 22:13, Emil Velikov >> wrote: >>> >>> On 5 June 2016 at 17:56, Samuel Pitoiset >>> wrote: We should not call nouveau_bufctx_reset() inside a validate function. >>> >>> This seems to contradict the changes introduced in nvc0_compute.c. >>> Worth explaining a bit better the dos and don'ts ? >>> >> As this is already in master, can you please provide a more >> elaborate/correct summary for -stable ? > > > I think it's fine as is. > > Do: reset bufctx when setting dirty bit > Don't: reset bufctx in validate logic, since it's "too late" by then. > (Not strictly wrong, but just should do it earlier.) So nvc0_compute_*validate*_surfaces is not validate logic ? Err... what a confusing name it has ;-) >>> >>> >>> It validates compute. And it invalidates (and clears) the 3d bin. >>> >> So one can reset_bufctx(3d) from the compute validate and vice-versa. >> While doing reset_bufctx(foo) from foo validate is a bad idea ? >> Shouldn't one just say so in the commit message ? > > > Because the common practice is to clear foo bins at the same place where the > dirty_3d |= foo is updated, this makes sense. :) > Yet the commit message does not say that, right ? It says "We should not call nouveau_bufctx_reset() inside a validate function.", while the patch does the complete opposite - it adds a call to nouveau_bufctx_reset() inside a validate function. All I'm asking is for the commit message to reflect the code change or vice-versa. I hope I'm not being unreasonable ? Thanks Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function
On 06/05/2016 11:50 PM, Emil Velikov wrote: On 5 June 2016 at 22:36, Ilia Mirkin wrote: On Sun, Jun 5, 2016 at 5:34 PM, Emil Velikov wrote: On 5 June 2016 at 22:17, Ilia Mirkin wrote: On Sun, Jun 5, 2016 at 5:16 PM, Emil Velikov wrote: On 5 June 2016 at 22:13, Emil Velikov wrote: On 5 June 2016 at 17:56, Samuel Pitoiset wrote: We should not call nouveau_bufctx_reset() inside a validate function. This seems to contradict the changes introduced in nvc0_compute.c. Worth explaining a bit better the dos and don'ts ? As this is already in master, can you please provide a more elaborate/correct summary for -stable ? I think it's fine as is. Do: reset bufctx when setting dirty bit Don't: reset bufctx in validate logic, since it's "too late" by then. (Not strictly wrong, but just should do it earlier.) So nvc0_compute_*validate*_surfaces is not validate logic ? Err... what a confusing name it has ;-) It validates compute. And it invalidates (and clears) the 3d bin. So one can reset_bufctx(3d) from the compute validate and vice-versa. While doing reset_bufctx(foo) from foo validate is a bad idea ? Shouldn't one just say so in the commit message ? Because the common practice is to clear foo bins at the same place where the dirty_3d |= foo is updated, this makes sense. :) -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function
On 5 June 2016 at 22:36, Ilia Mirkin wrote: > On Sun, Jun 5, 2016 at 5:34 PM, Emil Velikov wrote: >> On 5 June 2016 at 22:17, Ilia Mirkin wrote: >>> On Sun, Jun 5, 2016 at 5:16 PM, Emil Velikov >>> wrote: On 5 June 2016 at 22:13, Emil Velikov wrote: > On 5 June 2016 at 17:56, Samuel Pitoiset > wrote: >> We should not call nouveau_bufctx_reset() inside a validate function. > This seems to contradict the changes introduced in nvc0_compute.c. > Worth explaining a bit better the dos and don'ts ? > As this is already in master, can you please provide a more elaborate/correct summary for -stable ? >>> >>> I think it's fine as is. >>> >>> Do: reset bufctx when setting dirty bit >>> Don't: reset bufctx in validate logic, since it's "too late" by then. >>> (Not strictly wrong, but just should do it earlier.) >> >> So nvc0_compute_*validate*_surfaces is not validate logic ? Err... >> what a confusing name it has ;-) > > It validates compute. And it invalidates (and clears) the 3d bin. > So one can reset_bufctx(3d) from the compute validate and vice-versa. While doing reset_bufctx(foo) from foo validate is a bad idea ? Shouldn't one just say so in the commit message ? -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function
On Sun, Jun 5, 2016 at 5:34 PM, Emil Velikov wrote: > On 5 June 2016 at 22:17, Ilia Mirkin wrote: >> On Sun, Jun 5, 2016 at 5:16 PM, Emil Velikov >> wrote: >>> On 5 June 2016 at 22:13, Emil Velikov wrote: On 5 June 2016 at 17:56, Samuel Pitoiset wrote: > We should not call nouveau_bufctx_reset() inside a validate function. This seems to contradict the changes introduced in nvc0_compute.c. Worth explaining a bit better the dos and don'ts ? >>> As this is already in master, can you please provide a more >>> elaborate/correct summary for -stable ? >> >> I think it's fine as is. >> >> Do: reset bufctx when setting dirty bit >> Don't: reset bufctx in validate logic, since it's "too late" by then. >> (Not strictly wrong, but just should do it earlier.) > > So nvc0_compute_*validate*_surfaces is not validate logic ? Err... > what a confusing name it has ;-) It validates compute. And it invalidates (and clears) the 3d bin. -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function
On 5 June 2016 at 22:17, Ilia Mirkin wrote: > On Sun, Jun 5, 2016 at 5:16 PM, Emil Velikov wrote: >> On 5 June 2016 at 22:13, Emil Velikov wrote: >>> On 5 June 2016 at 17:56, Samuel Pitoiset wrote: We should not call nouveau_bufctx_reset() inside a validate function. >>> This seems to contradict the changes introduced in nvc0_compute.c. >>> Worth explaining a bit better the dos and don'ts ? >>> >> As this is already in master, can you please provide a more >> elaborate/correct summary for -stable ? > > I think it's fine as is. > > Do: reset bufctx when setting dirty bit > Don't: reset bufctx in validate logic, since it's "too late" by then. > (Not strictly wrong, but just should do it earlier.) So nvc0_compute_*validate*_surfaces is not validate logic ? Err... what a confusing name it has ;-) Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function
On 06/05/2016 11:17 PM, Ilia Mirkin wrote: On Sun, Jun 5, 2016 at 5:16 PM, Emil Velikov wrote: On 5 June 2016 at 22:13, Emil Velikov wrote: On 5 June 2016 at 17:56, Samuel Pitoiset wrote: We should not call nouveau_bufctx_reset() inside a validate function. This seems to contradict the changes introduced in nvc0_compute.c. Worth explaining a bit better the dos and don'ts ? As this is already in master, can you please provide a more elaborate/correct summary for -stable ? I think it's fine as is. Do: reset bufctx when setting dirty bit Don't: reset bufctx in validate logic, since it's "too late" by then. (Not strictly wrong, but just should do it earlier.) And usually, we clear the bins (ie. nouveau_bufctx_reset()) when the dirty flag is updated. This doesn't change anything, it's just wrong to do it inside the validate function. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function
On Sun, Jun 5, 2016 at 5:16 PM, Emil Velikov wrote: > On 5 June 2016 at 22:13, Emil Velikov wrote: >> On 5 June 2016 at 17:56, Samuel Pitoiset wrote: >>> We should not call nouveau_bufctx_reset() inside a validate function. >> This seems to contradict the changes introduced in nvc0_compute.c. >> Worth explaining a bit better the dos and don'ts ? >> > As this is already in master, can you please provide a more > elaborate/correct summary for -stable ? I think it's fine as is. Do: reset bufctx when setting dirty bit Don't: reset bufctx in validate logic, since it's "too late" by then. (Not strictly wrong, but just should do it earlier.) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function
On 5 June 2016 at 22:13, Emil Velikov wrote: > On 5 June 2016 at 17:56, Samuel Pitoiset wrote: >> We should not call nouveau_bufctx_reset() inside a validate function. > This seems to contradict the changes introduced in nvc0_compute.c. > Worth explaining a bit better the dos and don'ts ? > As this is already in master, can you please provide a more elaborate/correct summary for -stable ? Thanks Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function
On 5 June 2016 at 17:56, Samuel Pitoiset wrote: > We should not call nouveau_bufctx_reset() inside a validate function. This seems to contradict the changes introduced in nvc0_compute.c. Worth explaining a bit better the dos and don'ts ? -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl/ast: don't crash when func_name is NULL
From: Dave Airlie This fixes a crash in GL43-CTS.shader_subroutine.subroutines_not_allowed_as_variables_constructors_and_argument_or_return_types If we can't find the func_name in one of these paths, we have emitted an earlier error so just return here. Signed-off-by: Dave Airlie --- src/compiler/glsl/ast_function.cpp | 4 1 file changed, 4 insertions(+) diff --git a/src/compiler/glsl/ast_function.cpp b/src/compiler/glsl/ast_function.cpp index a97e6c9..f74394f 100644 --- a/src/compiler/glsl/ast_function.cpp +++ b/src/compiler/glsl/ast_function.cpp @@ -2078,6 +2078,10 @@ ast_function_expression::hir(exec_list *instructions, func_name = id->primary_expression.identifier; } + /* an error was emitted earlier */ + if (!func_name) + return ir_rvalue::error_value(ctx); + ir_function_signature *sig = match_function_by_name(func_name, &actual_parameters, state); -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [AppVeyor] mesa master #1516 failed
Build mesa 1516 failed Commit c417c0c9c3 by Kenneth Graunke on 9/7/2014 3:26 AM: mesa: Add MESA_SHADER_CAPTURE_PATH for writing .shader_test files.\n\nThis writes linked shader programs to .shader_test files to\n$MESA_SHADER_CAPTURE_PATH in the format used by shader-db\n(http://cgit.freedesktop.org/mesa/shader-db).\n\nIt supports both GLSL shaders and ARB programs. All stages that\nare linked together are written in a single .shader_test file.\n\nThis eliminates the need for shader-db's split-to-files.py, as Mesa\nproduces the desired format directly. It's much more reliable than\nparsing stdout/stderr, as those may contain extraneous messages, or\nsimply be closed by the application and unavailable.\n\nWe have many similar features already, but this is a bit different:\n- MESA_GLSL=dump writes to stdout, not files.\n- MESA_GLSL=log writes each stage to separate files (rather than\n all linked shaders in one file), at draw time (not link time),\n with uniform data and state flag info.\n- Tapani's shader replacement mechanism (MESA_SHADER_DUMP_PATH and\n MESA_SHADER_READ_PATH) also uses separate files per shader stage,\n but allows reading in files to replace an app's shader code.\n\nv2: Dump ARB programs too, not just GLSL.\nv3: Don't dump bogus 0.shader_test file.\nv4: Add "GL_ARB_separate_shader_objects" to the [require] block.\nv5: Print "GLSL 4.00" instead of "GLSL 4.0" in the [require] block.\nv6: Don't hardcode /tmp/mesa.\nv7: Fix memoization of getenv().\nv8: Also print "SSO ENABLED" (suggested by Timothy).\nv9: Also handle ES shaders (suggested by Ilia).\nv10: Guard against MESA_SHADER_CAPTURE_PATH being too long; add\n _mesa_warning calls on error handling (suggested by Ben).\nv11: Fix crash when variable is unset introduced in v10.\n\nSigned-off-by: Kenneth Graunke \nReviewed-by: Matt Turner Configure your notification preferences ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl: handle ast_aggregate in has_sequence_subexpression. (v2)
From: Dave Airlie GL43-CTS.compute_shader.work-group-size does uniform uint g_uniform[gl_WorkGroupSize.z + 20] = { 1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24 }; The initializer triggers the GLSL 4.30/GLES3 tests for constant sequence subexpressions, so it doesn't happen unless you are using those, so just return false as this path is now reachable. v2: update commit msg with diagnosis Signed-off-by: Dave Airlie --- src/compiler/glsl/ast_to_hir.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp index 400d3c4..b7192b2 100644 --- a/src/compiler/glsl/ast_to_hir.cpp +++ b/src/compiler/glsl/ast_to_hir.cpp @@ -2100,7 +2100,7 @@ ast_expression::has_sequence_subexpression() const return false; case ast_aggregate: - unreachable("ast_aggregate: Should never get here."); + return false; case ast_function_call: unreachable("should be handled by ast_function_expression::hir"); -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa/readpix: clamp SNORM properly (v2)
On 6 June 2016 at 05:40, Dave Airlie wrote: > From: Dave Airlie > > The clamping code always clamps to 0..1, which for SNORM is > incorrect. This adds a bit to say that clamping is for > an snorm and to use the correct range. > > This fixes a number of SNORM cases in: > GL33-CTS.texture_size_promotion.functional actually ignore this, it's a test bug I think now that I've reread the spec. Dave. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/3] radeonsi: use hw MSAA resolve for layered resolves
Please ignore this patch. It doesn't work. Marek On Sun, Jun 5, 2016 at 5:07 PM, Marek Olšák wrote: > From: Marek Olšák > > --- > src/gallium/drivers/radeonsi/si_blit.c | 34 > +- > 1 file changed, 21 insertions(+), 13 deletions(-) > > diff --git a/src/gallium/drivers/radeonsi/si_blit.c > b/src/gallium/drivers/radeonsi/si_blit.c > index 1c2c538..80844ec 100644 > --- a/src/gallium/drivers/radeonsi/si_blit.c > +++ b/src/gallium/drivers/radeonsi/si_blit.c > @@ -859,13 +859,13 @@ static bool do_hardware_msaa_resolve(struct > pipe_context *ctx, > unsigned sample_mask = ~0; > struct pipe_resource *tmp, templ; > struct pipe_blit_info blit; > + unsigned i; > > - /* Check basic requirements for hw resolve. */ > + /* Check basic requirements for MSAA resolve. */ > if (!(info->src.resource->nr_samples > 1 && > info->dst.resource->nr_samples <= 1 && > !util_format_is_pure_integer(format) && > - !util_format_is_depth_or_stencil(format) && > - util_max_layer(info->src.resource, 0) == 0)) > + !util_format_is_depth_or_stencil(format))) > return false; > > /* Hardware MSAA resolve doesn't work if SPI format = NORM16_ABGR and > @@ -878,6 +878,7 @@ static bool do_hardware_msaa_resolve(struct pipe_context > *ctx, > > /* Check the remaining requirements for hw resolve. */ > if (util_max_layer(info->dst.resource, info->dst.level) == 0 && > + util_max_layer(info->src.resource, 0) == 0 && > !info->scissor_enable && > (info->mask & PIPE_MASK_RGBA) == PIPE_MASK_RGBA && > > util_is_format_compatible(util_format_description(info->src.format), > @@ -914,12 +915,12 @@ static bool do_hardware_msaa_resolve(struct > pipe_context *ctx, > * a temporary texture and blit. > */ > memset(&templ, 0, sizeof(templ)); > - templ.target = PIPE_TEXTURE_2D; > + templ.target = info->src.resource->target; > templ.format = info->src.resource->format; > templ.width0 = info->src.resource->width0; > templ.height0 = info->src.resource->height0; > templ.depth0 = 1; > - templ.array_size = 1; > + templ.array_size = info->src.resource->array_size; > templ.usage = PIPE_USAGE_DEFAULT; > templ.flags = R600_RESOURCE_FLAG_FORCE_TILING; > > @@ -927,14 +928,21 @@ static bool do_hardware_msaa_resolve(struct > pipe_context *ctx, > if (!tmp) > return false; > > - /* resolve */ > - si_blitter_begin(ctx, SI_COLOR_RESOLVE | > -(info->render_condition_enable ? 0 : > SI_DISABLE_RENDER_COND)); > - util_blitter_custom_resolve_color(sctx->blitter, tmp, 0, 0, > - info->src.resource, info->src.box.z, > - sample_mask, > sctx->custom_blend_resolve, > - format); > - si_blitter_end(ctx); > + /* resolve all layers that need to be resolved */ > + assert(info->src.box.depth > 0); > + for (i = 0; i < info->src.box.depth; i++) { > + si_blitter_begin(ctx, SI_COLOR_RESOLVE | > +(info->render_condition_enable ? > + 0 : SI_DISABLE_RENDER_COND)); > + util_blitter_custom_resolve_color(sctx->blitter, tmp, 0, > + info->src.box.z + i, > + info->src.resource, > + info->src.box.z + i, > + sample_mask, > + sctx->custom_blend_resolve, > + format); > + si_blitter_end(ctx); > + } > > /* blit */ > blit = *info; > -- > 2.7.4 > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] mesa/readpix: clamp SNORM properly (v2)
From: Dave Airlie The clamping code always clamps to 0..1, which for SNORM is incorrect. This adds a bit to say that clamping is for an snorm and to use the correct range. This fixes a number of SNORM cases in: GL33-CTS.texture_size_promotion.functional v2: handle the case where the format is an array format. don't test for snorm in readpixels, readpixels always does [0..1] according to spec. Signed-off-by: Dave Airlie --- src/mesa/main/pack.c | 4 +++- src/mesa/main/pixeltransfer.c | 10 ++ src/mesa/main/pixeltransfer.h | 1 + src/mesa/main/readpix.c | 4 +++- src/mesa/main/texgetimage.c | 4 +++- src/mesa/main/texstore.c | 10 +- src/mesa/swrast/s_copypix.c | 4 +++- src/mesa/swrast/s_drawpix.c | 2 +- 8 files changed, 29 insertions(+), 10 deletions(-) diff --git a/src/mesa/main/pack.c b/src/mesa/main/pack.c index 89faf51..f810929 100644 --- a/src/mesa/main/pack.c +++ b/src/mesa/main/pack.c @@ -1607,7 +1607,9 @@ _mesa_unpack_color_index_to_rgba_float(struct gl_context *ctx, GLuint dims, * with color indexes. */ transferOps &= ~(IMAGE_SCALE_BIAS_BIT | IMAGE_MAP_COLOR_BIT); - _mesa_apply_rgba_transfer_ops(ctx, transferOps, count, (float (*)[4])dstPtr); + _mesa_apply_rgba_transfer_ops(ctx, transferOps, +false, +count, (float (*)[4])dstPtr); dstPtr += srcHeight * srcWidth * 4; } diff --git a/src/mesa/main/pixeltransfer.c b/src/mesa/main/pixeltransfer.c index 22eac00..93ab8ad 100644 --- a/src/mesa/main/pixeltransfer.c +++ b/src/mesa/main/pixeltransfer.c @@ -162,6 +162,7 @@ _mesa_scale_and_bias_depth_uint(const struct gl_context *ctx, GLuint n, */ void _mesa_apply_rgba_transfer_ops(struct gl_context *ctx, GLbitfield transferOps, + bool clamp_snorm, GLuint n, GLfloat rgba[][4]) { /* scale & bias */ @@ -180,11 +181,12 @@ _mesa_apply_rgba_transfer_ops(struct gl_context *ctx, GLbitfield transferOps, /* clamping to [0,1] */ if (transferOps & IMAGE_CLAMP_BIT) { GLuint i; + GLfloat lower_val = clamp_snorm ? -1.0F : 0.0F; for (i = 0; i < n; i++) { - rgba[i][RCOMP] = CLAMP(rgba[i][RCOMP], 0.0F, 1.0F); - rgba[i][GCOMP] = CLAMP(rgba[i][GCOMP], 0.0F, 1.0F); - rgba[i][BCOMP] = CLAMP(rgba[i][BCOMP], 0.0F, 1.0F); - rgba[i][ACOMP] = CLAMP(rgba[i][ACOMP], 0.0F, 1.0F); + rgba[i][RCOMP] = CLAMP(rgba[i][RCOMP], lower_val, 1.0F); + rgba[i][GCOMP] = CLAMP(rgba[i][GCOMP], lower_val, 1.0F); + rgba[i][BCOMP] = CLAMP(rgba[i][BCOMP], lower_val, 1.0F); + rgba[i][ACOMP] = CLAMP(rgba[i][ACOMP], lower_val, 1.0F); } } } diff --git a/src/mesa/main/pixeltransfer.h b/src/mesa/main/pixeltransfer.h index b0a301f..e67ab53 100644 --- a/src/mesa/main/pixeltransfer.h +++ b/src/mesa/main/pixeltransfer.h @@ -56,6 +56,7 @@ _mesa_scale_and_bias_depth_uint(const struct gl_context *ctx, GLuint n, extern void _mesa_apply_rgba_transfer_ops(struct gl_context *ctx, GLbitfield transferOps, + bool clamp_snorm, GLuint n, GLfloat rgba[][4]); extern void diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c index 1cb06c7..9c17a10 100644 --- a/src/mesa/main/readpix.c +++ b/src/mesa/main/readpix.c @@ -544,7 +544,9 @@ read_rgba_pixels( struct gl_context *ctx, /* Handle transfer ops if necessary */ if (transferOps) - _mesa_apply_rgba_transfer_ops(ctx, transferOps, width * height, rgba); + _mesa_apply_rgba_transfer_ops(ctx, transferOps, + false, + width * height, rgba); /* If we had to rebase, we have already taken care of that */ needs_rebase = false; diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c index fc3cc6b..c501f9e 100644 --- a/src/mesa/main/texgetimage.c +++ b/src/mesa/main/texgetimage.c @@ -518,7 +518,9 @@ get_tex_rgba_uncompressed(struct gl_context *ctx, GLuint dimensions, needsRebase ? rebaseSwizzle : NULL); /* Handle transfer ops now */ - _mesa_apply_rgba_transfer_ops(ctx, transferOps, width * height, rgba); + _mesa_apply_rgba_transfer_ops(ctx, transferOps, + _mesa_get_format_datatype(texFormat) == GL_SIGNED_NORMALIZED, + width * height, rgba); /* If we had to rebase, we have already handled that */ needsRebase = false; diff --git a/src/mesa/main/texstore.c b/src/mesa/main/texstore.c index b54e033..bed416a 100644 --- a/src/mesa/main/texstore.c +++ b/src/mesa/main/texstore.c @@ -760,6 +760,7 @@ texstore_rgba(TEXSTORE_PARAMS) _mesa_texstore_needs_transfer_ops(ctx, baseInternalFormat, dstFormat)) { /* Allocate RGBA float image
Re: [Mesa-dev] [PATCH] st/mesa: change SQRT lowering to fix the game Risen
On Sun, Jun 5, 2016 at 6:34 PM, Axel Davy wrote: > if abs is not inserted in the source glsl, that's a wine bug, not a mesa > bug, > Thus I don't think doing this is a good idea. The old code used CMP, this code uses ABS instead. ABS is faster. I don't see a problem with that considering the behavior is "undefined" in GLSL. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 94512] X segfaults with glx-tls enabled in a x32 environment
https://bugs.freedesktop.org/show_bug.cgi?id=94512 --- Comment #6 from Emil Velikov --- Based of the backtrace there is no information if mesa/libglapi was build with or w/o glx-tls. Following an IRC conversation it seems that the library is built w/o glx-tls as i is missing the _glapi_tls_Context and _glapi_tls_Dispatch symbols. Perhaps they are different builds ? Either way it's good to attach the output of $nm -CD --defined-only /usr/libx32/libglapi.so in this particular case. Also please that the correct libraries are being picked up. This can be done via something like: $LD_DEBUG=libs foo (startx) 2>ld_debug.log Obviously none of these excludes that there is an actual issue with x32 and glx-tls and/or that mesa silently ignores the flag on said platform ;-) -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Patchwork review process (efficiency) questions
On 2016.06.03 18:49, ⚛ wrote: > On Fri, Jun 3, 2016 at 2:08 PM, Nicolai Hähnle wrote: >> On 03.06.2016 13:12, wrote: >>> >>> Situation: Looking at the content displayed by the web browser for URL >>> http://patchwork.freedesktop.org/project/mesa/series and sub-pages >>> accessible via the links. >> >> >> Patchwork isn't really central to most people's workflow as far as I >> understand it. > > Ok. But how does a developer know (better: gets notified) when a patch > has been accepted and added to mainline mesa-git? Viewing all git log > messages every day because - just in case - the patch _might_ have > been added to mesa-git seems quite inefficient to me. I only ever sent one (1) patch to mesa "out-of-the-blue", so I believe I can speak from a "newcomer" POV here. I did not know about this patchwork thing until You mentioned it here. I just sent my patch to mesa-dev and waited. It did get some comments right away, but did not get reviewed for quite a while. That prompted me to simply ask on IRC about it. I got promptly informed that the person who can review it was tad busy at the time, so he might take a some time to do it. When it did get reviewed and committed, I knew instantly. By automation, as You suggest. By adding a simple filter rule in my mailing client to alert me when something gets pushed to mesa-commit mailing list containing my name. As I like to say it in such cases "wow, that was hard" \_(:_/ So, as a by definition and "outsider"/"newcomer" contributor I can say I encountered zero problems with the process. >> Most of your questions' answers naturally fall out from that. > > Not in my world. > >> This mailing list is what's important. > > In my opinion, now isn't the 1990-ties. (This is just my personal > opinion, you do not need to agree with it.) > >>> - What is the influence of the default ordering (URL suffix >>> "?ordering=-last_updated") on the behavior of reviewers? >> >> Probably zero, because I doubt people go via Patchwork. > > In that case, the process might be even less efficient than I though. > >>> - What about those patches on the 10th page from previous year? Why >>> are they in the list? >> >> Nothing. >> >>> - Do patch submitters regularly clean up outdated patches? >> >> No. >> >>> - Does a patch submitter receive a notification email when he/she >>> forgets about a patch over time? >> >> No. >> >>> It seems to me that the current review process isn't as efficient as it >>> can be. >> >> To be blunt: that may be the case, but even if so, it's extremely unlikely >> bordering on the impossible that comments from the sideline from somebody >> who hasn't got an experience of contributing could ever be helpful. > > Outsiders and enemies. That is so ... cool. Again, I was and still am very much an outsider, I should say. But a bit of sincere modesty goes a long way. And it did for me. Please do note that I am just sharing my experiences and providing and opinion of a "newcomer"/"outsider" here. And my opinion is that it is not broken. And what is not broken does not need any fixing. I do not know how You are, Mr./Ms./Mrs. Atom-icon. Since that means You could be anything from a lowly troll to a wizard in disguise dispensing wisdom, I shall not dare to question Your credibility on the matter, but I must point out that this makes it tad complicated to take You as someone serious instead of someone who is just bored and has nothing better to do. I believe You should realize that. Lastly, I agree that some points You made do make a lot sense "on paper". Sadly, that does not mean they always end up being good in practice. Since I do plan on contributing to mesa again in the future, I am quite eager to see how this turns out in the end, so I shall be watching closely. Oh boy! :] Those are my 2¢ I Hope the currency of the said 2¢ is strong, making it a valuable 2¢, if You catch my drift... ;] - Gediminas Jakutis LDK Varčiai www.varciai.lt signature.asc Description: OpenPGP digital signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nvc0: do not clear surfaces bins in the validate function
Reviewed-by: Ilia Mirkin On Sun, Jun 5, 2016 at 12:56 PM, Samuel Pitoiset wrote: > We should not call nouveau_bufctx_reset() inside a validate function. > This only affects Fermi where images are aliased between 3D and CP. > > Signed-off-by: Samuel Pitoiset > Cc: "12.0" > --- > src/gallium/drivers/nouveau/nvc0/nvc0_compute.c | 1 + > src/gallium/drivers/nouveau/nvc0/nvc0_tex.c | 6 +- > 2 files changed, 2 insertions(+), 5 deletions(-) > > diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c > b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c > index 21c8b2e..59bbe1e 100644 > --- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c > +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c > @@ -324,6 +324,7 @@ nvc0_compute_validate_surfaces(struct nvc0_context *nvc0) > nvc0_validate_suf(nvc0, 5); > > /* Invalidate all FRAGMENT images because they are aliased with COMPUTE. > */ > + nouveau_bufctx_reset(nvc0->bufctx_3d, NVC0_BIND_3D_SUF); > nvc0->dirty_3d |= NVC0_NEW_3D_SURFACES; > nvc0->images_dirty[4] |= nvc0->images_valid[4]; > } > diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c > b/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c > index b54de0f..1a5d8ec 100644 > --- a/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c > +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c > @@ -994,11 +994,6 @@ nvc0_validate_suf(struct nvc0_context *nvc0, int s) > struct nouveau_pushbuf *push = nvc0->base.pushbuf; > struct nvc0_screen *screen = nvc0->screen; > > - if (s == 5) > - nouveau_bufctx_reset(nvc0->bufctx_cp, NVC0_BIND_CP_SUF); > - else > - nouveau_bufctx_reset(nvc0->bufctx_3d, NVC0_BIND_3D_SUF); > - > for (int i = 0; i < NVC0_MAX_IMAGES; ++i) { >struct pipe_image_view *view = &nvc0->images[s][i]; >int width, height, depth; > @@ -1099,6 +1094,7 @@ nvc0_update_surface_bindings(struct nvc0_context *nvc0) > nvc0_validate_suf(nvc0, 4); > > /* Invalidate all COMPUTE images because they are aliased with FRAGMENT. > */ > + nouveau_bufctx_reset(nvc0->bufctx_cp, NVC0_BIND_CP_SUF); > nvc0->dirty_cp |= NVC0_NEW_CP_SURFACES; > nvc0->images_dirty[5] |= nvc0->images_valid[5]; > } > -- > 2.8.3 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] nvc0: do not clear surfaces bins in the validate function
We should not call nouveau_bufctx_reset() inside a validate function. This only affects Fermi where images are aliased between 3D and CP. Signed-off-by: Samuel Pitoiset Cc: "12.0" --- src/gallium/drivers/nouveau/nvc0/nvc0_compute.c | 1 + src/gallium/drivers/nouveau/nvc0/nvc0_tex.c | 6 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c index 21c8b2e..59bbe1e 100644 --- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c @@ -324,6 +324,7 @@ nvc0_compute_validate_surfaces(struct nvc0_context *nvc0) nvc0_validate_suf(nvc0, 5); /* Invalidate all FRAGMENT images because they are aliased with COMPUTE. */ + nouveau_bufctx_reset(nvc0->bufctx_3d, NVC0_BIND_3D_SUF); nvc0->dirty_3d |= NVC0_NEW_3D_SURFACES; nvc0->images_dirty[4] |= nvc0->images_valid[4]; } diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c b/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c index b54de0f..1a5d8ec 100644 --- a/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c @@ -994,11 +994,6 @@ nvc0_validate_suf(struct nvc0_context *nvc0, int s) struct nouveau_pushbuf *push = nvc0->base.pushbuf; struct nvc0_screen *screen = nvc0->screen; - if (s == 5) - nouveau_bufctx_reset(nvc0->bufctx_cp, NVC0_BIND_CP_SUF); - else - nouveau_bufctx_reset(nvc0->bufctx_3d, NVC0_BIND_3D_SUF); - for (int i = 0; i < NVC0_MAX_IMAGES; ++i) { struct pipe_image_view *view = &nvc0->images[s][i]; int width, height, depth; @@ -1099,6 +1094,7 @@ nvc0_update_surface_bindings(struct nvc0_context *nvc0) nvc0_validate_suf(nvc0, 4); /* Invalidate all COMPUTE images because they are aliased with FRAGMENT. */ + nouveau_bufctx_reset(nvc0->bufctx_cp, NVC0_BIND_CP_SUF); nvc0->dirty_cp |= NVC0_NEW_CP_SURFACES; nvc0->images_dirty[5] |= nvc0->images_valid[5]; } -- 2.8.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] nvc0: re-validate images after launching a grid on Fermi
On Sun, Jun 5, 2016 at 12:28 PM, Samuel Pitoiset wrote: > Images invalidation is a bit weird on Fermi and there is already a hack > which forces invalidating all images when launching a computer shader > to help in fixing 3D<->CP interaction. > > However, we need to re-validate images for compute because > nvc0_compute_invalidate_surfaces() will destroy the previous binding. > This is not really good for performance purposes but this might be > improved later. > > This fixes the following piglits: > - spec/arb_compute_shader/execution/basic-uniform-access > - spec/arb_compute_shader/execution/mutiple-texture-reading > - spec/arb_compute_shader/execution/multiple-workgroups > - spec/glsl-4.30/execution/built-in-functions/cs-* (207 tests) > > v2: reset CP_SUF bins > > Signed-off-by: Samuel Pitoiset > Cc: "12.0" Reviewed-by: Ilia Mirkin > --- > src/gallium/drivers/nouveau/nvc0/nvc0_compute.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c > b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c > index 8c88225..21c8b2e 100644 > --- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c > +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c > @@ -458,4 +458,7 @@ nvc0_launch_grid(struct pipe_context *pipe, const struct > pipe_grid_info *info) > > /* TODO: Not sure if this is really necessary. */ > nvc0_compute_invalidate_surfaces(nvc0, 5); > + nouveau_bufctx_reset(nvc0->bufctx_cp, NVC0_BIND_CP_SUF); > + nvc0->dirty_cp |= NVC0_NEW_CP_SURFACES; > + nvc0->images_dirty[5] |= nvc0->images_valid[5]; > } > -- > 2.8.3 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/mesa: change SQRT lowering to fix the game Risen
if abs is not inserted in the source glsl, that's a wine bug, not a mesa bug, Thus I don't think doing this is a good idea. On 05/06/2016 17:17, Ilia Mirkin wrote: On Mon, May 30, 2016 at 7:19 PM, Marek Olšák wrote: From: Marek Olšák Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94627 (against nouveau) --- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 16 +--- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp index aa443a5..0f5ee02 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp @@ -1901,13 +1901,15 @@ glsl_to_tgsi_visitor::visit_expression(ir_expression* ir, st_src_reg *op) if (have_sqrt) { emit_scalar(ir, TGSI_OPCODE_SQRT, result_dst, op[0]); } else { - /* sqrt(x) = x * rsq(x). */ - emit_scalar(ir, TGSI_OPCODE_RSQ, result_dst, op[0]); - emit_asm(ir, TGSI_OPCODE_MUL, result_dst, result_src, op[0]); - /* For incoming channels <= 0, set the result to 0. */ - op[0].negate = ~op[0].negate; - emit_asm(ir, TGSI_OPCODE_CMP, result_dst, - op[0], result_src, st_src_reg_for_float(0.0)); + /* This is the only instruction sequence that makes the game "Risen" + * render correctly. ABS is not required for the game, but since GLSL + * declares negative values as "undefined", allowing us to do whatever + * we want, I choose to use ABS to match DX9 and pre-GLSL RSQ + * behavior. + */ + emit_scalar(ir, TGSI_OPCODE_ABS, result_dst, op[0]); + emit_scalar(ir, TGSI_OPCODE_RSQ, result_dst, result_src); + emit_scalar(ir, TGSI_OPCODE_RCP, result_dst, result_src); I spent a bunch of time trying to come up with alternatives that still had the desired behavior for 0 and Infinity (since RCP sucks in terms of precision). At the end of the day, it depends on what the hw does with RSQ(0) and RCP(0). For nv50/nvc0, this sequence did have the desired behavior - perhaps that holds for all hardware? Either way, I'm fairly ambivalent about this - nv50/nvc0 now claim to have SQRT support (and perform the lowering in the backend), and I doubt much of anything would care on nv30. I'd just as soon leave the abs out of it, but I don't strongly care. Acked-by: Ilia Mirkin } break; case ir_unop_rsq: -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2] nvc0: re-validate images after launching a grid on Fermi
Images invalidation is a bit weird on Fermi and there is already a hack which forces invalidating all images when launching a computer shader to help in fixing 3D<->CP interaction. However, we need to re-validate images for compute because nvc0_compute_invalidate_surfaces() will destroy the previous binding. This is not really good for performance purposes but this might be improved later. This fixes the following piglits: - spec/arb_compute_shader/execution/basic-uniform-access - spec/arb_compute_shader/execution/mutiple-texture-reading - spec/arb_compute_shader/execution/multiple-workgroups - spec/glsl-4.30/execution/built-in-functions/cs-* (207 tests) v2: reset CP_SUF bins Signed-off-by: Samuel Pitoiset Cc: "12.0" --- src/gallium/drivers/nouveau/nvc0/nvc0_compute.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c index 8c88225..21c8b2e 100644 --- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c @@ -458,4 +458,7 @@ nvc0_launch_grid(struct pipe_context *pipe, const struct pipe_grid_info *info) /* TODO: Not sure if this is really necessary. */ nvc0_compute_invalidate_surfaces(nvc0, 5); + nouveau_bufctx_reset(nvc0->bufctx_cp, NVC0_BIND_CP_SUF); + nvc0->dirty_cp |= NVC0_NEW_CP_SURFACES; + nvc0->images_dirty[5] |= nvc0->images_valid[5]; } -- 2.8.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nvc0: re-validate images after launching a grid on Fermi
On Sun, Jun 5, 2016 at 12:13 PM, Samuel Pitoiset wrote: > Images invalidation is a bit weird on Fermi and there is already a hack > which forces invalidating all images when launching a computer shader > to help in fixing 3D<->CP interaction. > > However, we need to re-validate images for compute because > nvc0_compute_invalidate_surfaces() will destroy the previous binding. > This is not really good for performance purposes but this might be > improved later. > > This fixes the following piglits: > - spec/arb_compute_shader/execution/basic-uniform-access > - spec/arb_compute_shader/execution/mutiple-texture-reading > - spec/arb_compute_shader/execution/multiple-workgroups > - spec/glsl-4.30/execution/built-in-functions/cs-* (207 tests) > > Signed-off-by: Samuel Pitoiset > Cc: "12.0" > --- > src/gallium/drivers/nouveau/nvc0/nvc0_compute.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c > b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c > index 8c88225..baeeca5 100644 > --- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c > +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c > @@ -458,4 +458,6 @@ nvc0_launch_grid(struct pipe_context *pipe, const struct > pipe_grid_info *info) > > /* TODO: Not sure if this is really necessary. */ > nvc0_compute_invalidate_surfaces(nvc0, 5); > + nvc0->dirty_cp |= NVC0_NEW_CP_SURFACES; > + nvc0->images_dirty[5] |= nvc0->images_valid[5]; Don't forget to clear out the bufctx. Also (in another patch), do a pass over all your invalidation hacks - any time you do nvc0->dirty |= foo, you should have an associated nouveau_bufctx_reset(). Otherwise things can start piling up in there. (Since validation only adds to the buckets.) -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] nvc0: re-validate images after launching a grid on Fermi
Images invalidation is a bit weird on Fermi and there is already a hack which forces invalidating all images when launching a computer shader to help in fixing 3D<->CP interaction. However, we need to re-validate images for compute because nvc0_compute_invalidate_surfaces() will destroy the previous binding. This is not really good for performance purposes but this might be improved later. This fixes the following piglits: - spec/arb_compute_shader/execution/basic-uniform-access - spec/arb_compute_shader/execution/mutiple-texture-reading - spec/arb_compute_shader/execution/multiple-workgroups - spec/glsl-4.30/execution/built-in-functions/cs-* (207 tests) Signed-off-by: Samuel Pitoiset Cc: "12.0" --- src/gallium/drivers/nouveau/nvc0/nvc0_compute.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c index 8c88225..baeeca5 100644 --- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c @@ -458,4 +458,6 @@ nvc0_launch_grid(struct pipe_context *pipe, const struct pipe_grid_info *info) /* TODO: Not sure if this is really necessary. */ nvc0_compute_invalidate_surfaces(nvc0, 5); + nvc0->dirty_cp |= NVC0_NEW_CP_SURFACES; + nvc0->images_dirty[5] |= nvc0->images_valid[5]; } -- 2.8.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/mesa: change SQRT lowering to fix the game Risen
On Mon, May 30, 2016 at 7:19 PM, Marek Olšák wrote: > From: Marek Olšák > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94627 > (against nouveau) > --- > src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 16 +--- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp > b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp > index aa443a5..0f5ee02 100644 > --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp > +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp > @@ -1901,13 +1901,15 @@ glsl_to_tgsi_visitor::visit_expression(ir_expression* > ir, st_src_reg *op) >if (have_sqrt) { > emit_scalar(ir, TGSI_OPCODE_SQRT, result_dst, op[0]); >} else { > - /* sqrt(x) = x * rsq(x). */ > - emit_scalar(ir, TGSI_OPCODE_RSQ, result_dst, op[0]); > - emit_asm(ir, TGSI_OPCODE_MUL, result_dst, result_src, op[0]); > - /* For incoming channels <= 0, set the result to 0. */ > - op[0].negate = ~op[0].negate; > - emit_asm(ir, TGSI_OPCODE_CMP, result_dst, > - op[0], result_src, st_src_reg_for_float(0.0)); > + /* This is the only instruction sequence that makes the game "Risen" > + * render correctly. ABS is not required for the game, but since > GLSL > + * declares negative values as "undefined", allowing us to do > whatever > + * we want, I choose to use ABS to match DX9 and pre-GLSL RSQ > + * behavior. > + */ > + emit_scalar(ir, TGSI_OPCODE_ABS, result_dst, op[0]); > + emit_scalar(ir, TGSI_OPCODE_RSQ, result_dst, result_src); > + emit_scalar(ir, TGSI_OPCODE_RCP, result_dst, result_src); I spent a bunch of time trying to come up with alternatives that still had the desired behavior for 0 and Infinity (since RCP sucks in terms of precision). At the end of the day, it depends on what the hw does with RSQ(0) and RCP(0). For nv50/nvc0, this sequence did have the desired behavior - perhaps that holds for all hardware? Either way, I'm fairly ambivalent about this - nv50/nvc0 now claim to have SQRT support (and perform the lowering in the backend), and I doubt much of anything would care on nv30. I'd just as soon leave the abs out of it, but I don't strongly care. Acked-by: Ilia Mirkin >} >break; > case ir_unop_rsq: > -- > 2.7.4 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/3] radeonsi: use hw MSAA resolve for non-trivial resolves
From: Marek Olšák This improves MSAA resolve performance. --- src/gallium/drivers/radeonsi/si_blit.c | 62 +- 1 file changed, 53 insertions(+), 9 deletions(-) diff --git a/src/gallium/drivers/radeonsi/si_blit.c b/src/gallium/drivers/radeonsi/si_blit.c index 3748a59..1c2c538 100644 --- a/src/gallium/drivers/radeonsi/si_blit.c +++ b/src/gallium/drivers/radeonsi/si_blit.c @@ -857,6 +857,16 @@ static bool do_hardware_msaa_resolve(struct pipe_context *ctx, unsigned dst_height = u_minify(info->dst.resource->height0, info->dst.level); enum pipe_format format = info->dst.format; unsigned sample_mask = ~0; + struct pipe_resource *tmp, templ; + struct pipe_blit_info blit; + + /* Check basic requirements for hw resolve. */ + if (!(info->src.resource->nr_samples > 1 && + info->dst.resource->nr_samples <= 1 && + !util_format_is_pure_integer(format) && + !util_format_is_depth_or_stencil(format) && + util_max_layer(info->src.resource, 0) == 0)) + return false; /* Hardware MSAA resolve doesn't work if SPI format = NORM16_ABGR and * the format is R16G16. Use R16A16, which does work. @@ -866,16 +876,12 @@ static bool do_hardware_msaa_resolve(struct pipe_context *ctx, if (format == PIPE_FORMAT_R16G16_SNORM) format = PIPE_FORMAT_R16A16_SNORM; - if (info->src.resource->nr_samples > 1 && - info->dst.resource->nr_samples <= 1 && - util_max_layer(info->src.resource, 0) == 0 && - util_max_layer(info->dst.resource, info->dst.level) == 0 && - util_is_format_compatible(util_format_description(info->src.format), - util_format_description(info->dst.format)) && - !util_format_is_pure_integer(format) && - !util_format_is_depth_or_stencil(format) && + /* Check the remaining requirements for hw resolve. */ + if (util_max_layer(info->dst.resource, info->dst.level) == 0 && !info->scissor_enable && (info->mask & PIPE_MASK_RGBA) == PIPE_MASK_RGBA && + util_is_format_compatible(util_format_description(info->src.format), + util_format_description(info->dst.format)) && dst_width == info->src.resource->width0 && dst_height == info->src.resource->height0 && info->dst.box.x == 0 && @@ -903,7 +909,45 @@ static bool do_hardware_msaa_resolve(struct pipe_context *ctx, si_blitter_end(ctx); return true; } - return false; + + /* Shader-based resolve is VERY SLOW. Instead, resolve into +* a temporary texture and blit. +*/ + memset(&templ, 0, sizeof(templ)); + templ.target = PIPE_TEXTURE_2D; + templ.format = info->src.resource->format; + templ.width0 = info->src.resource->width0; + templ.height0 = info->src.resource->height0; + templ.depth0 = 1; + templ.array_size = 1; + templ.usage = PIPE_USAGE_DEFAULT; + templ.flags = R600_RESOURCE_FLAG_FORCE_TILING; + + tmp = ctx->screen->resource_create(ctx->screen, &templ); + if (!tmp) + return false; + + /* resolve */ + si_blitter_begin(ctx, SI_COLOR_RESOLVE | +(info->render_condition_enable ? 0 : SI_DISABLE_RENDER_COND)); + util_blitter_custom_resolve_color(sctx->blitter, tmp, 0, 0, + info->src.resource, info->src.box.z, + sample_mask, sctx->custom_blend_resolve, + format); + si_blitter_end(ctx); + + /* blit */ + blit = *info; + blit.src.resource = tmp; + blit.src.box.z = 0; + + si_blitter_begin(ctx, SI_BLIT | +(info->render_condition_enable ? 0 : SI_DISABLE_RENDER_COND)); + util_blitter_blit(sctx->blitter, &blit); + si_blitter_end(ctx); + + pipe_resource_reference(&tmp, NULL); + return true; } static void si_blit(struct pipe_context *ctx, -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/3] r600g: use hw MSAA resolve for non-trivial resolves
From: Marek Olšák This improves MSAA resolve performance. --- src/gallium/drivers/r600/r600_blit.c | 60 +++- 1 file changed, 52 insertions(+), 8 deletions(-) diff --git a/src/gallium/drivers/r600/r600_blit.c b/src/gallium/drivers/r600/r600_blit.c index c9d7823..6277fa3 100644 --- a/src/gallium/drivers/r600/r600_blit.c +++ b/src/gallium/drivers/r600/r600_blit.c @@ -761,15 +761,21 @@ static bool do_hardware_msaa_resolve(struct pipe_context *ctx, unsigned sample_mask = rctx->b.chip_class == CAYMAN ? ~0 : ((1ull << MAX2(1, info->src.resource->nr_samples)) - 1); - - if (info->src.resource->nr_samples > 1 && - info->dst.resource->nr_samples <= 1 && - util_max_layer(info->src.resource, 0) == 0 && - util_max_layer(info->dst.resource, info->dst.level) == 0 && + struct pipe_resource *tmp, templ; + struct pipe_blit_info blit; + + /* Check basic requirements for hw resolve. */ + if (!(info->src.resource->nr_samples > 1 && + info->dst.resource->nr_samples <= 1 && + !util_format_is_pure_integer(format) && + !util_format_is_depth_or_stencil(format) && + util_max_layer(info->src.resource, 0) == 0)) + return false; + + /* Check the remaining requirements for hw resolve. */ + if (util_max_layer(info->dst.resource, info->dst.level) == 0 && util_is_format_compatible(util_format_description(info->src.format), util_format_description(info->dst.format)) && - !util_format_is_pure_integer(format) && - !util_format_is_depth_or_stencil(format) && !info->scissor_enable && (info->mask & PIPE_MASK_RGBA) == PIPE_MASK_RGBA && dst_width == info->src.resource->width0 && @@ -797,7 +803,45 @@ static bool do_hardware_msaa_resolve(struct pipe_context *ctx, r600_blitter_end(ctx); return true; } - return false; + + /* Shader-based resolve is VERY SLOW. Instead, resolve into +* a temporary texture and blit. +*/ + memset(&templ, 0, sizeof(templ)); + templ.target = PIPE_TEXTURE_2D; + templ.format = info->src.resource->format; + templ.width0 = info->src.resource->width0; + templ.height0 = info->src.resource->height0; + templ.depth0 = 1; + templ.array_size = 1; + templ.usage = PIPE_USAGE_DEFAULT; + templ.flags = R600_RESOURCE_FLAG_FORCE_TILING; + + tmp = ctx->screen->resource_create(ctx->screen, &templ); + if (!tmp) + return false; + + /* resolve */ + r600_blitter_begin(ctx, R600_COLOR_RESOLVE | + (info->render_condition_enable ? 0 : R600_DISABLE_RENDER_COND)); + util_blitter_custom_resolve_color(rctx->blitter, tmp, 0, 0, + info->src.resource, info->src.box.z, + sample_mask, rctx->custom_blend_resolve, + format); + r600_blitter_end(ctx); + + /* blit */ + blit = *info; + blit.src.resource = tmp; + blit.src.box.z = 0; + + r600_blitter_begin(ctx, R600_BLIT | + (info->render_condition_enable ? 0 : R600_DISABLE_RENDER_COND)); + util_blitter_blit(rctx->blitter, &blit); + r600_blitter_end(ctx); + + pipe_resource_reference(&tmp, NULL); + return true; } static void r600_blit(struct pipe_context *ctx, -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 0/3] R600-GCN: Use hw MSAA resolve for non-trivial resolves
Hi, The shader-based resolve is slow. With this series, one scene with 8xMSAA in HL2: Lost Coast goes from 8-9 FPS to 32 FPS on Evergreen & Wine/Nine. Please review. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/3] radeonsi: use hw MSAA resolve for layered resolves
From: Marek Olšák --- src/gallium/drivers/radeonsi/si_blit.c | 34 +- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/src/gallium/drivers/radeonsi/si_blit.c b/src/gallium/drivers/radeonsi/si_blit.c index 1c2c538..80844ec 100644 --- a/src/gallium/drivers/radeonsi/si_blit.c +++ b/src/gallium/drivers/radeonsi/si_blit.c @@ -859,13 +859,13 @@ static bool do_hardware_msaa_resolve(struct pipe_context *ctx, unsigned sample_mask = ~0; struct pipe_resource *tmp, templ; struct pipe_blit_info blit; + unsigned i; - /* Check basic requirements for hw resolve. */ + /* Check basic requirements for MSAA resolve. */ if (!(info->src.resource->nr_samples > 1 && info->dst.resource->nr_samples <= 1 && !util_format_is_pure_integer(format) && - !util_format_is_depth_or_stencil(format) && - util_max_layer(info->src.resource, 0) == 0)) + !util_format_is_depth_or_stencil(format))) return false; /* Hardware MSAA resolve doesn't work if SPI format = NORM16_ABGR and @@ -878,6 +878,7 @@ static bool do_hardware_msaa_resolve(struct pipe_context *ctx, /* Check the remaining requirements for hw resolve. */ if (util_max_layer(info->dst.resource, info->dst.level) == 0 && + util_max_layer(info->src.resource, 0) == 0 && !info->scissor_enable && (info->mask & PIPE_MASK_RGBA) == PIPE_MASK_RGBA && util_is_format_compatible(util_format_description(info->src.format), @@ -914,12 +915,12 @@ static bool do_hardware_msaa_resolve(struct pipe_context *ctx, * a temporary texture and blit. */ memset(&templ, 0, sizeof(templ)); - templ.target = PIPE_TEXTURE_2D; + templ.target = info->src.resource->target; templ.format = info->src.resource->format; templ.width0 = info->src.resource->width0; templ.height0 = info->src.resource->height0; templ.depth0 = 1; - templ.array_size = 1; + templ.array_size = info->src.resource->array_size; templ.usage = PIPE_USAGE_DEFAULT; templ.flags = R600_RESOURCE_FLAG_FORCE_TILING; @@ -927,14 +928,21 @@ static bool do_hardware_msaa_resolve(struct pipe_context *ctx, if (!tmp) return false; - /* resolve */ - si_blitter_begin(ctx, SI_COLOR_RESOLVE | -(info->render_condition_enable ? 0 : SI_DISABLE_RENDER_COND)); - util_blitter_custom_resolve_color(sctx->blitter, tmp, 0, 0, - info->src.resource, info->src.box.z, - sample_mask, sctx->custom_blend_resolve, - format); - si_blitter_end(ctx); + /* resolve all layers that need to be resolved */ + assert(info->src.box.depth > 0); + for (i = 0; i < info->src.box.depth; i++) { + si_blitter_begin(ctx, SI_COLOR_RESOLVE | +(info->render_condition_enable ? + 0 : SI_DISABLE_RENDER_COND)); + util_blitter_custom_resolve_color(sctx->blitter, tmp, 0, + info->src.box.z + i, + info->src.resource, + info->src.box.z + i, + sample_mask, + sctx->custom_blend_resolve, + format); + si_blitter_end(ctx); + } /* blit */ blit = *info; -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/mesa: change SQRT lowering to fix the game Risen
Ping. This is a more accurate version than RSQ+MUL+CMP. Marek On Tue, May 31, 2016 at 1:19 AM, Marek Olšák wrote: > From: Marek Olšák > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94627 > (against nouveau) > --- > src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 16 +--- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp > b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp > index aa443a5..0f5ee02 100644 > --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp > +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp > @@ -1901,13 +1901,15 @@ glsl_to_tgsi_visitor::visit_expression(ir_expression* > ir, st_src_reg *op) >if (have_sqrt) { > emit_scalar(ir, TGSI_OPCODE_SQRT, result_dst, op[0]); >} else { > - /* sqrt(x) = x * rsq(x). */ > - emit_scalar(ir, TGSI_OPCODE_RSQ, result_dst, op[0]); > - emit_asm(ir, TGSI_OPCODE_MUL, result_dst, result_src, op[0]); > - /* For incoming channels <= 0, set the result to 0. */ > - op[0].negate = ~op[0].negate; > - emit_asm(ir, TGSI_OPCODE_CMP, result_dst, > - op[0], result_src, st_src_reg_for_float(0.0)); > + /* This is the only instruction sequence that makes the game "Risen" > + * render correctly. ABS is not required for the game, but since > GLSL > + * declares negative values as "undefined", allowing us to do > whatever > + * we want, I choose to use ABS to match DX9 and pre-GLSL RSQ > + * behavior. > + */ > + emit_scalar(ir, TGSI_OPCODE_ABS, result_dst, op[0]); > + emit_scalar(ir, TGSI_OPCODE_RSQ, result_dst, result_src); > + emit_scalar(ir, TGSI_OPCODE_RCP, result_dst, result_src); >} >break; > case ir_unop_rsq: > -- > 2.7.4 > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev