Re: [Mesa-dev] [PATCH] i965: Rework Sandy Bridge HiZ and stencil layouts
On Mon, May 29, 2017 at 12:09:01PM -0700, Jason Ekstrand wrote: > Sandy Bridge does not technically support mipmapped depth/stencil. In > order to work around this, we allocate what are effectively completely > separate images for each miplevel, ensure that they are page-aligned, > and manually offset to them. Prior to layered rendering, this was a > simple matter of setting a large enough halign/valign. > > With the advent of layered rendering, however, things got more > complicated. Now, things weren't as simple as just handing a surface > off to the hardware. Any miplevel of a normally mipmapped surface can > be considered as just an array surface given the right qpitch. However, > the hardware gives us no capability to specify qpitch so this won't > work. Instead, the chosen solution was to use a new "all slices at each > LOD" layout which laid things out as a mipmap of arrays rather than an > array of mipmaps. This way you can easily offset to any of the > miplevels and each is a valid array. > > Unfortunately, the "all slices at each lod" concept missed one > fundamental thing about SNB HiZ and stencil hardware: It doesn't just > always act as if you're always working with a non-mipmapped surface, it > acts as if you're always working on a non-mipmapped surface of the same > size as LOD0. In other words, even though it may only write the > upper-left corner of each array slice, the qpitch for the array is for a > surface the size of LOD0 of the depth surface. This mistake causes us > to under-allocate HiZ and stencil in some cases and also to accidentally > allow different miplevels to overlap. Sadly, piglit test coverage > didn't quite catch this until I started making changes to the resolve > code that caused additional HiZ resolves in certain tests. > > This commit switches Sandy Bridge HiZ and stencil over to a new scheme > that lays out the non-zero miplevels horizontally below LOD0. This way > they can all have the same qpitch without interfering with each other. > Technically, the miplevels still overlap, but things are spaced out > enough that each page is only in the "written area" of one LOD. Hopefully, > this will get rid of at least some of the random SNB hangs. > > Cc: "17.0 17.1" > Cc: Topi Pohjolainen > Cc: Nanley Chery > Cc: Jordan Justen > Cc: Kenneth Graunke There is a few nits, but looks good. Thanks a lot for figuring out how it really works: Reviewed-by: Topi Pohjolainen > > --- > The series I sent out on Friday suffered from a GPU hang or two on Sandy > Bridge. It turns out that those hangs were caused by the hardware HiZ > resolving part of my batch buffer due to this under-allocation. > > Topi, I'm sorry but this will likely make hash of your earlier patch > series. Sadly, I don't think there's really anything else we can do. :-( > Also, given how tricky this is to get right, I concede that we may want to > add an ISL_DIM_LAYOUT_GEN6_HIZ_STENCIL layout to ISL. We could still do it > with the "array of surfaces" approach but I think these sorts of > calculations are best done in with the other surface calculation code. I > can help draft it if you'd like. > > src/mesa/drivers/dri/i965/brw_blorp.c | 11 ++- > src/mesa/drivers/dri/i965/brw_tex_layout.c| 100 > ++ > src/mesa/drivers/dri/i965/gen6_depth_state.c | 4 +- > src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 11 +-- > src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 37 +- > 5 files changed, 134 insertions(+), 29 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c > b/src/mesa/drivers/dri/i965/brw_blorp.c > index 6e860f0..cb9933b 100644 > --- a/src/mesa/drivers/dri/i965/brw_blorp.c > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c > @@ -123,7 +123,7 @@ apply_gen6_stencil_hiz_offset(struct isl_surf *surf, >uint32_t lod, >uint32_t *offset) > { > - assert(mt->array_layout == ALL_SLICES_AT_EACH_LOD); > + assert(mt->array_layout == GEN6_HIZ_STENCIL); > > if (mt->format == MESA_FORMAT_S_UINT8) { >/* Note: we can't compute the stencil offset using > @@ -183,12 +183,12 @@ blorp_surf_for_miptree(struct brw_context *brw, > }; > > if (brw->gen == 6 && mt->format == MESA_FORMAT_S_UINT8 && > - mt->array_layout == ALL_SLICES_AT_EACH_LOD) { > - /* Sandy bridge stencil and HiZ use this ALL_SLICES_AT_EACH_LOD hack in > + mt->array_layout == GEN6_HIZ_STENCIL) { > + /* Sandy bridge stencil and HiZ use this GEN6_HIZ_STENCIL hack in > * order to allow for layered rendering. The hack makes each LOD of > the > * stencil or HiZ buffer a single tightly packed array surface at some > * offset into the surface. Since ISL doesn't know how to deal with > the > - * crazy ALL_SLICES_AT_EACH_LOD layout and since we have to do a manual > + * crazy GEN6_HIZ_STENCIL layout and since we have to do a manual >
[Mesa-dev] [PATCH 4/4] st_glsl_to_tgsi: replace variables tracking list with a hash table
From: Dave Airlie This removes the linear search which is fail when number of variables goes up to 3 or so. --- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 44 +- 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp index 0e59aca..87c4b10 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp @@ -56,6 +56,7 @@ #include "st_nir.h" #include "st_shader_cache.h" +#include "util/hash_table.h" #include #define PROGRAM_ANY_CONST ((1 << PROGRAM_STATE_VAR) |\ @@ -310,7 +311,9 @@ public: const struct tgsi_opcode_info *info; }; -class variable_storage : public exec_node { +class variable_storage { + DECLARE_RZALLOC_CXX_OPERATORS(variable_storage) + public: variable_storage(ir_variable *var, gl_register_file file, int index, unsigned array_id = 0) @@ -488,7 +491,7 @@ public: st_src_reg result; /** List of variable_storage */ - exec_list variables; + struct hash_table *variables; /** List of immediate_storage */ exec_list immediates; @@ -1306,13 +1309,15 @@ glsl_to_tgsi_visitor::get_temp(const glsl_type *type) variable_storage * glsl_to_tgsi_visitor::find_variable_storage(ir_variable *var) { + struct hash_entry *entry = _mesa_hash_table_search(this->variables, + var); + variable_storage *storage; + if (!entry) + return NULL; - foreach_in_list(variable_storage, entry, &this->variables) { - if (entry->var == var) - return entry; - } + storage = (variable_storage *)entry->data; - return NULL; + return storage; } void @@ -1345,7 +1350,8 @@ glsl_to_tgsi_visitor::visit(ir_variable *ir) if (i == ir->get_num_state_slots()) { /* We'll set the index later. */ storage = new(mem_ctx) variable_storage(ir, PROGRAM_STATE_VAR, -1); - this->variables.push_tail(storage); + + _mesa_hash_table_insert(this->variables, ir, storage); dst = undef_dst; } else { @@ -1360,7 +1366,7 @@ glsl_to_tgsi_visitor::visit(ir_variable *ir) storage = new(mem_ctx) variable_storage(ir, dst.file, dst.index, dst.array_id); - this->variables.push_tail(storage); + _mesa_hash_table_insert(this->variables, ir, storage); } @@ -2595,7 +2601,7 @@ glsl_to_tgsi_visitor::visit(ir_dereference_variable *ir) case ir_var_uniform: entry = new(mem_ctx) variable_storage(var, PROGRAM_UNIFORM, var->data.param_index); - this->variables.push_tail(entry); + _mesa_hash_table_insert(this->variables, var, entry); break; case ir_var_shader_in: { /* The linker assigns locations for varyings and attributes, @@ -2642,7 +2648,7 @@ glsl_to_tgsi_visitor::visit(ir_dereference_variable *ir) decl->array_id); entry->component = component; - this->variables.push_tail(entry); + _mesa_hash_table_insert(this->variables, var, entry); break; } case ir_var_shader_out: { @@ -2700,7 +2706,8 @@ glsl_to_tgsi_visitor::visit(ir_dereference_variable *ir) } entry->component = component; - this->variables.push_tail(entry); + _mesa_hash_table_insert(this->variables, var, entry); + break; } case ir_var_system_value: @@ -2714,7 +2721,7 @@ glsl_to_tgsi_visitor::visit(ir_dereference_variable *ir) entry = new(mem_ctx) variable_storage(var, src.file, src.index, src.array_id); - this->variables.push_tail(entry); + _mesa_hash_table_insert(this->variables, var, entry); break; } @@ -4569,10 +4576,19 @@ glsl_to_tgsi_visitor::glsl_to_tgsi_visitor() have_fma = false; use_shared_memory = false; has_tex_txf_lz = false; + variables = NULL; +} + +static void var_destroy(struct hash_entry *entry) +{ + variable_storage *storage = (variable_storage *)entry->data; + + delete storage; } glsl_to_tgsi_visitor::~glsl_to_tgsi_visitor() { + _mesa_hash_table_destroy(variables, var_destroy); free(array_sizes); ralloc_free(mem_ctx); } @@ -6772,6 +6788,8 @@ get_mesa_program_tgsi(struct gl_context *ctx, v->has_tex_txf_lz = pscreen->get_param(pscreen, PIPE_CAP_TGSI_TEX_TXF_LZ); + v->variables = _mesa_hash_table_create(v->mem_ctx, _mesa_hash_pointer, + _mesa_key_pointer_equal); _mesa_generate_parameters_list_for_uniforms(shader_program, shader, prog->Parameters); -- 2.1.0 _
[Mesa-dev] [RFC] some glsl->tgsi optimisations
While looking at the fp64 emulation code, we were spending minutes in the glsl->tgsi passes as some tests were producing shaders with > 32000 temporaries. Now it might be possible to reduce these earlier with some GLSL passes, but this code is pretty bad as-is. This reduces one test execution time from 4m30 -> 5s. I've not regression tested these much, but someone interested might want to throw them at some slow compile situations to see if they help any. I'll give them a lot more testing before I consider them non RFC. Dave. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/4] st_glsl_to_tgsi: track range for writes in a if/else/endif blocks.
From: Dave Airlie This overhauls the copy prop and dead code passes to avoid major CPU overhead in some corner cases trigged by the fp64 patches --- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 122 + 1 file changed, 108 insertions(+), 14 deletions(-) diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp index e87c241..8835dc2 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp @@ -4895,6 +4895,78 @@ glsl_to_tgsi_visitor::get_last_temp_write(int *last_writes) * * which allows for dead code elimination on TEMP[1]'s writes. */ +#define DEFAULT_LEVELS 8 + +class per_level_info { + + struct per_level_range { + int32_t min_temp_idx; + int32_t max_temp_idx; + } *lvls; + + void *mem_ctx; + int num_alloced_levels; + int level; + int max_temps; +public: + + per_level_info(void *mem_ctx_in, int max_temps_in) { + num_alloced_levels = DEFAULT_LEVELS; + max_temps = max_temps_in; + mem_ctx = mem_ctx_in; + level = 0; + lvls = (struct per_level_range *)reralloc_array_size(mem_ctx, + NULL, + sizeof(struct per_level_range), + num_alloced_levels); + lvls[0].min_temp_idx = max_temps; + lvls[0].max_temp_idx = 0; + } + + ~per_level_info() { + ralloc_free(lvls); + } + + int get_level(void) { + return level; + } + + void push_level(void) { + level++; + if (level >= num_alloced_levels) { + num_alloced_levels += 4; + lvls = (struct per_level_range *)reralloc_array_size(mem_ctx, + (void *)lvls, + sizeof(struct per_level_range), + num_alloced_levels); + } + lvls[level].min_temp_idx = max_temps; + lvls[level].max_temp_idx = 0; + } + + void pop_level(void) { + if (lvls[level - 1].min_temp_idx > lvls[level].min_temp_idx) + lvls[level - 1].min_temp_idx = lvls[level].min_temp_idx; + if (lvls[level - 1].max_temp_idx < lvls[level].max_temp_idx) + lvls[level - 1].max_temp_idx = lvls[level].max_temp_idx; + level--; + } + + void get_level_range(int32_t *min, int32_t *max) + { + *min = lvls[level].min_temp_idx; + *max = lvls[level].max_temp_idx; + } + + void update_level_range(int32_t idx) + { + if (idx < lvls[level].min_temp_idx) + lvls[level].min_temp_idx = idx; + if ((idx + 1) > lvls[level].max_temp_idx) + lvls[level].max_temp_idx = idx + 1; + } +}; + void glsl_to_tgsi_visitor::copy_propagate(void) { @@ -4902,7 +4974,9 @@ glsl_to_tgsi_visitor::copy_propagate(void) glsl_to_tgsi_instruction *, this->next_temp * 4); int *acp_level = rzalloc_array(mem_ctx, int, this->next_temp * 4); - int level = 0; + class per_level_info lvl_info(mem_ctx, this->next_temp); + int min_lvl, max_lvl; + int level; foreach_in_list(glsl_to_tgsi_instruction, inst, &this->instructions) { assert(inst->dst[0].file != PROGRAM_TEMPORARY @@ -4926,13 +5000,12 @@ glsl_to_tgsi_visitor::copy_propagate(void) for (int i = 0; i < 4; i++) { int src_chan = GET_SWZ(inst->src[r].swizzle, i); glsl_to_tgsi_instruction *copy_chan = acp[acp_base + src_chan]; - if (!copy_chan) { good = false; break; } -assert(acp_level[acp_base + src_chan] <= level); +assert(acp_level[acp_base + src_chan] <= lvl_info.get_level()); if (!first) { first = copy_chan; @@ -4977,7 +5050,7 @@ glsl_to_tgsi_visitor::copy_propagate(void) case TGSI_OPCODE_IF: case TGSI_OPCODE_UIF: - ++level; + lvl_info.push_level(); break; case TGSI_OPCODE_ENDIF: @@ -4985,7 +5058,8 @@ glsl_to_tgsi_visitor::copy_propagate(void) /* Clear all channels written inside the block from the ACP, but * leaving those that were not touched. */ - for (int r = 0; r < this->next_temp; r++) { + lvl_info.get_level_range(&min_lvl, &max_lvl); + for (int r = min_lvl; r < max_lvl; r++) { for (int c = 0; c < 4; c++) { if (!acp[4 * r + c]) continue; @@ -4994,8 +5068,11 @@ glsl_to_tgsi_visitor::copy_propagate(void) acp[4 * r + c] = NULL; } } - if (inst->op == TGSI_OPCODE_ENDIF) ---level; + lvl_info.pop_level(); + + if (inst->op != TGSI_OPCODE_ENDIF) +
[Mesa-dev] [PATCH 1/4] st_glsl_to_tgsi: bump index back up to 32-bit
From: Dave Airlie with some of the fp64 emulation, we are seeing shaders coming in with > 32K temps, they go out with 40 or so used, but while doing register renumber we need to store a lot of them. So bump this fields back up to 32-bit. --- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp index 1eacbb1..e87c241 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp @@ -165,7 +165,7 @@ public: explicit st_src_reg(st_dst_reg reg); - int16_t index; /**< temporary index, VERT_ATTRIB_*, VARYING_SLOT_*, etc. */ + int32_t index; /**< temporary index, VERT_ATTRIB_*, VARYING_SLOT_*, etc. */ int16_t index2D; uint16_t swizzle; /**< SWIZZLE_XYZWONEZERO swizzles from Mesa. */ int negate:4; /**< NEGATE_XYZW mask from mesa */ @@ -239,7 +239,7 @@ public: explicit st_dst_reg(st_src_reg reg); - int16_t index; /**< temporary index, VERT_ATTRIB_*, VARYING_SLOT_*, etc. */ + int32_t index; /**< temporary index, VERT_ATTRIB_*, VARYING_SLOT_*, etc. */ int16_t index2D; gl_register_file file:5; /**< PROGRAM_* from Mesa */ unsigned writemask:4; /**< Bitfield of WRITEMASK_[XYZW] */ -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/4] st_glsl_to_tgsi: rewrite rename registers to use array fully.
From: Dave Airlie Instead of having to search the whole array, just use the whole thing and store a valid bit in there with the rename. Removes this from the profile on some of the fp64 tests --- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 51 +++--- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp index 8835dc2..0e59aca 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp @@ -388,7 +388,7 @@ find_array_type(struct inout_decl *decls, unsigned count, unsigned array_id) } struct rename_reg_pair { - int old_reg; + bool valid; int new_reg; }; @@ -556,7 +556,7 @@ public: void simplify_cmp(void); - void rename_temp_registers(int num_renames, struct rename_reg_pair *renames); + void rename_temp_registers(struct rename_reg_pair *renames); void get_first_temp_read(int *first_reads); void get_last_temp_read_first_temp_write(int *last_reads, int *first_writes); void get_last_temp_write(int *last_writes); @@ -4746,30 +4746,32 @@ glsl_to_tgsi_visitor::simplify_cmp(void) /* Replaces all references to a temporary register index with another index. */ void -glsl_to_tgsi_visitor::rename_temp_registers(int num_renames, struct rename_reg_pair *renames) +glsl_to_tgsi_visitor::rename_temp_registers(struct rename_reg_pair *renames) { foreach_in_list(glsl_to_tgsi_instruction, inst, &this->instructions) { unsigned j; - int k; for (j = 0; j < num_inst_src_regs(inst); j++) { - if (inst->src[j].file == PROGRAM_TEMPORARY) -for (k = 0; k < num_renames; k++) - if (inst->src[j].index == renames[k].old_reg) - inst->src[j].index = renames[k].new_reg; + if (inst->src[j].file == PROGRAM_TEMPORARY) { +int old_idx = inst->src[j].index; +if (renames[old_idx].valid) + inst->src[j].index = renames[old_idx].new_reg; + } } for (j = 0; j < inst->tex_offset_num_offset; j++) { - if (inst->tex_offsets[j].file == PROGRAM_TEMPORARY) -for (k = 0; k < num_renames; k++) - if (inst->tex_offsets[j].index == renames[k].old_reg) - inst->tex_offsets[j].index = renames[k].new_reg; + if (inst->tex_offsets[j].file == PROGRAM_TEMPORARY) { +int old_idx = inst->tex_offsets[j].index; +if (renames[old_idx].valid) + inst->tex_offsets[j].index = renames[old_idx].new_reg; + } } for (j = 0; j < num_inst_dst_regs(inst); j++) { - if (inst->dst[j].file == PROGRAM_TEMPORARY) - for (k = 0; k < num_renames; k++) -if (inst->dst[j].index == renames[k].old_reg) - inst->dst[j].index = renames[k].new_reg; + if (inst->dst[j].file == PROGRAM_TEMPORARY) { +int old_idx = inst->dst[j].index; +if (renames[old_idx].valid) + inst->dst[j].index = renames[old_idx].new_reg; + } } } } @@ -5402,7 +5404,6 @@ glsl_to_tgsi_visitor::merge_registers(void) int *first_writes = rzalloc_array(mem_ctx, int, this->next_temp); struct rename_reg_pair *renames = rzalloc_array(mem_ctx, struct rename_reg_pair, this->next_temp); int i, j; - int num_renames = 0; /* Read the indices of the last read and first write to each temp register * into an array so that we don't have to traverse the instruction list as @@ -5429,9 +5430,8 @@ glsl_to_tgsi_visitor::merge_registers(void) * as the register at index j. */ if (first_writes[i] <= first_writes[j] && last_reads[i] <= first_writes[j]) { -renames[num_renames].old_reg = j; -renames[num_renames].new_reg = i; -num_renames++; +renames[j].new_reg = i; +renames[j].valid = true; /* Update the first_writes and last_reads arrays with the new * values for the merged register index, and mark the newly unused @@ -5444,7 +5444,7 @@ glsl_to_tgsi_visitor::merge_registers(void) } } - rename_temp_registers(num_renames, renames); + rename_temp_registers(renames); ralloc_free(renames); ralloc_free(last_reads); ralloc_free(first_writes); @@ -5459,7 +5459,7 @@ glsl_to_tgsi_visitor::renumber_registers(void) int new_index = 0; int *first_reads = rzalloc_array(mem_ctx, int, this->next_temp); struct rename_reg_pair *renames = rzalloc_array(mem_ctx, struct rename_reg_pair, this->next_temp); - int num_renames = 0; + for (i = 0; i < this->next_temp; i++) { first_reads[i] = -1; } @@ -5468,14 +5468,13 @@ glsl_to_tgsi_visitor::renumber_registers(void) for (i = 0; i < this->next_temp; i++) { if (first_reads[i] < 0) continue; if (i != new_index) { - renames[num_rena
[Mesa-dev] [PATCH] i965: Fix alpha to one with dual color blending.
The BLEND_STATE documentation says that alpha to one must be disabled when dual color blending is enabled. However, it appears that it simply fails to override src1 alpha to one. We can work around this by leaving alpha to one enabled, but overriding SRC1_ALPHA to ONE and ONE_MINUS_SRC1_ALPHA to ZERO. This appears to be what the other driver does, and it looks like it works despite the documentation saying not to do it. Fixes spec/ext_framebuffer_multisample/alpha-to-one-dual-src-blend * Piglit tests. --- src/mesa/drivers/dri/i965/genX_state_upload.c | 57 +-- 1 file changed, 44 insertions(+), 13 deletions(-) diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c b/src/mesa/drivers/dri/i965/genX_state_upload.c index 76d2ea887b1..145173c2fc1 100644 --- a/src/mesa/drivers/dri/i965/genX_state_upload.c +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c @@ -2435,6 +2435,20 @@ static const struct brw_tracked_state genX(gs_state) = { /* -- */ +static GLenum +fix_dual_blend_alpha_to_one(GLenum function) +{ + switch (function) { + case GL_SRC1_ALPHA: + return GL_ONE; + + case GL_ONE_MINUS_SRC1_ALPHA: + return GL_ZERO; + } + + return function; +} + #define blend_factor(x) brw_translate_blend_factor(x) #define blend_eqn(x) brw_translate_blend_equation(x) @@ -2562,6 +2576,19 @@ genX(upload_blend_state)(struct brw_context *brw) dstA = brw_fix_xRGB_alpha(dstA); } +/* From the BLEND_STATE docs, DWord 0, Bit 29 (AlphaToOne Enable): + * "If Dual Source Blending is enabled, this bit must be disabled." + * + * We override SRC1_ALPHA to ONE and ONE_MINUS_SRC1_ALPHA to ZERO, + * and leave it enabled anyway. + */ +if (ctx->Color.Blend[i]._UsesDualSrc && blend.AlphaToOneEnable) { + srcRGB = fix_dual_blend_alpha_to_one(srcRGB); + srcA = fix_dual_blend_alpha_to_one(srcA); + dstRGB = fix_dual_blend_alpha_to_one(dstRGB); + dstA = fix_dual_blend_alpha_to_one(dstA); +} + entry.ColorBufferBlendEnable = true; entry.DestinationBlendFactor = blend_factor(dstRGB); entry.SourceBlendFactor = blend_factor(srcRGB); @@ -2600,16 +2627,6 @@ genX(upload_blend_state)(struct brw_context *brw) entry.WriteDisableBlue = !ctx->Color.ColorMask[i][2]; entry.WriteDisableAlpha = !ctx->Color.ColorMask[i][3]; - /* From the BLEND_STATE docs, DWord 0, Bit 29 (AlphaToOne Enable): - * "If Dual Source Blending is enabled, this bit must be disabled." - */ - WARN_ONCE(ctx->Color.Blend[i]._UsesDualSrc && - _mesa_is_multisample_enabled(ctx) && - ctx->Multisample.SampleAlphaToOne, - "HW workaround: disabling alpha to one with dual src " - "blending\n"); - if (ctx->Color.Blend[i]._UsesDualSrc) -blend.AlphaToOneEnable = false; #if GEN_GEN >= 8 GENX(BLEND_STATE_ENTRY_pack)(NULL, &blend_map[1 + i * 2], &entry); #else @@ -4049,11 +4066,15 @@ genX(upload_ps_blend)(struct brw_context *brw) /* BRW_NEW_FRAGMENT_PROGRAM | _NEW_BUFFERS | _NEW_COLOR */ pb.HasWriteableRT = brw_color_buffer_write_enabled(brw); + bool alpha_to_one = false; + if (!buffer0_is_integer) { /* _NEW_MULTISAMPLE */ - pb.AlphaToCoverageEnable = -_mesa_is_multisample_enabled(ctx) && -ctx->Multisample.SampleAlphaToCoverage; + + if (_mesa_is_multisample_enabled(ctx)) { +pb.AlphaToCoverageEnable = ctx->Multisample.SampleAlphaToCoverage; +alpha_to_one = ctx->Multisample.SampleAlphaToOne; + } pb.AlphaTestEnable = color->AlphaEnabled; } @@ -4098,6 +4119,16 @@ genX(upload_ps_blend)(struct brw_context *brw) dstA = brw_fix_xRGB_alpha(dstA); } + /* Alpha to One doesn't work with Dual Color Blending. Override + * SRC1_ALPHA to ONE and ONE_MINUS_SRC1_ALPHA to ZERO. + */ + if (alpha_to_one && color->Blend[0]._UsesDualSrc) { +srcRGB = fix_dual_blend_alpha_to_one(srcRGB); +srcA = fix_dual_blend_alpha_to_one(srcA); +dstRGB = fix_dual_blend_alpha_to_one(dstRGB); +dstA = fix_dual_blend_alpha_to_one(dstA); + } + pb.ColorBufferBlendEnable = true; pb.SourceAlphaBlendFactor = brw_translate_blend_factor(srcA); pb.DestinationAlphaBlendFactor = brw_translate_blend_factor(dstA); -- 2.13.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] etnaviv: Don't try to use the index buffer if size is zero
On 05/29/2017 02:47 PM, Lucas Stach wrote: > Hi Tomeu, > > Am Freitag, den 19.05.2017, 12:40 +0200 schrieb Tomeu Vizoso: >> If info->index_size is zero, info->index will point to uninitialized >> memory. >> >> Fatal signal 11 (SIGSEGV), code 2, fault addr 0xab5d07a3 in tid 20456 >> (surfaceflinger) >> >> Signed-off-by: Tomeu Vizoso >> Cc: etna...@lists.freedesktop.org >> Cc: Marek Olšák >> Fixes: 330d0607ed60 ("gallium: remove pipe_index_buffer and >> set_index_buffer") >> --- >> src/gallium/drivers/etnaviv/etnaviv_context.c | 36 >> +++ >> 1 file changed, 20 insertions(+), 16 deletions(-) >> >> diff --git a/src/gallium/drivers/etnaviv/etnaviv_context.c >> b/src/gallium/drivers/etnaviv/etnaviv_context.c >> index 306cb6fc498d..dcda4088bfd5 100644 >> --- a/src/gallium/drivers/etnaviv/etnaviv_context.c >> +++ b/src/gallium/drivers/etnaviv/etnaviv_context.c >> @@ -178,24 +178,28 @@ etna_draw_vbo(struct pipe_context *pctx, const struct >> pipe_draw_info *info) >> >> /* Upload a user index buffer. */ >> unsigned index_offset = 0; >> - struct pipe_resource *indexbuf = info->has_user_indices ? NULL : >> info->index.resource; >> - if (info->index_size && info->has_user_indices && >> - !util_upload_index_buffer(pctx, info, &indexbuf, &index_offset)) { >> - BUG("Index buffer upload failed."); >> - return; >> - } >> + struct pipe_resource *indexbuf = NULL; >> >> - if (info->index_size && indexbuf) { >> - ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR.bo = >> etna_resource(indexbuf)->bo; >> - ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR.offset = index_offset; >> - ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR.flags = ETNA_RELOC_READ; >> - ctx->index_buffer.FE_INDEX_STREAM_CONTROL = >> translate_index_size(info->index_size); >> - ctx->dirty |= ETNA_DIRTY_INDEX_BUFFER; >> - } >> + if (info->index_size) { >> + indexbuf = info->has_user_indices ? NULL : info->index.resource; >> + if (info->has_user_indices && >> + !util_upload_index_buffer(pctx, info, &indexbuf, &index_offset)) { >> + BUG("Index buffer upload failed."); >> + return; >> + } >> >> - if (info->index_size && !ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR.bo) >> { >> - BUG("Unsupported or no index buffer"); >> - return; >> + if (indexbuf) { > > This can be simplified: If this is a indexed draw (index_size != 0) then > we will have a indexbuf at that point in the code, as its either > provided or constructed via the util_upload_index_buffer() call. > > Mind if squash this simplification into your patch while pushing? Sounds good to me. Thanks, Tomeu > Regards, > Lucas > >> + ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR.bo = >> etna_resource(indexbuf)->bo; >> + ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR.offset = index_offset; >> + ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR.flags = >> ETNA_RELOC_READ; >> + ctx->index_buffer.FE_INDEX_STREAM_CONTROL = >> translate_index_size(info->index_size); >> + ctx->dirty |= ETNA_DIRTY_INDEX_BUFFER; >> + } >> + >> + if (!ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR.bo) { >> + BUG("Unsupported or no index buffer"); >> + return; >> + } >> } >> >> struct etna_shader_key key = {}; > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] mesa: remove _mesa from static function names
--- src/mesa/program/programopt.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/mesa/program/programopt.c b/src/mesa/program/programopt.c index 501acde..f560bce 100644 --- a/src/mesa/program/programopt.c +++ b/src/mesa/program/programopt.c @@ -39,21 +39,21 @@ #include "programopt.h" #include "prog_instruction.h" /** * This function inserts instructions for coordinate modelview * projection * into a vertex program. * May be used to implement the position_invariant option. */ static void -_mesa_insert_mvp_dp4_code(struct gl_context *ctx, struct gl_program *vprog) +insert_mvp_dp4_code(struct gl_context *ctx, struct gl_program *vprog) { struct prog_instruction *newInst; const GLuint origLen = vprog->arb.NumInstructions; const GLuint newLen = origLen + 4; GLuint i; /* * Setup state references for the modelview/projection matrix. * XXX we should check if these state vars are already declared. */ @@ -106,21 +106,21 @@ _mesa_insert_mvp_dp4_code(struct gl_context *ctx, struct gl_program *vprog) /* install new instructions */ vprog->arb.Instructions = newInst; vprog->arb.NumInstructions = newLen; vprog->info.inputs_read |= VERT_BIT_POS; vprog->info.outputs_written |= BITFIELD64_BIT(VARYING_SLOT_POS); } static void -_mesa_insert_mvp_mad_code(struct gl_context *ctx, struct gl_program *vprog) +insert_mvp_mad_code(struct gl_context *ctx, struct gl_program *vprog) { struct prog_instruction *newInst; const GLuint origLen = vprog->arb.NumInstructions; const GLuint newLen = origLen + 4; GLuint hposTemp; GLuint i; /* * Setup state references for the modelview/projection matrix. * XXX we should check if these state vars are already declared. @@ -210,23 +210,23 @@ _mesa_insert_mvp_mad_code(struct gl_context *ctx, struct gl_program *vprog) vprog->arb.NumInstructions = newLen; vprog->info.inputs_read |= VERT_BIT_POS; vprog->info.outputs_written |= BITFIELD64_BIT(VARYING_SLOT_POS); } void _mesa_insert_mvp_code(struct gl_context *ctx, struct gl_program *vprog) { if (ctx->Const.ShaderCompilerOptions[MESA_SHADER_VERTEX].OptimizeForAOS) - _mesa_insert_mvp_dp4_code( ctx, vprog ); + insert_mvp_dp4_code( ctx, vprog ); else - _mesa_insert_mvp_mad_code( ctx, vprog ); + insert_mvp_mad_code( ctx, vprog ); } /** * Append instructions to implement fog * -- 2.9.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] mesa/st: indentation tidy-up
--- src/mesa/state_tracker/st_mesa_to_tgsi.c | 65 1 file changed, 32 insertions(+), 33 deletions(-) diff --git a/src/mesa/state_tracker/st_mesa_to_tgsi.c b/src/mesa/state_tracker/st_mesa_to_tgsi.c index ce75cf7..2d12de2 100644 --- a/src/mesa/state_tracker/st_mesa_to_tgsi.c +++ b/src/mesa/state_tracker/st_mesa_to_tgsi.c @@ -931,56 +931,55 @@ st_translate_mesa_program( /* Declare address register. */ if (program->arb.NumAddressRegs > 0) { debug_assert( program->arb.NumAddressRegs == 1 ); t->address[0] = ureg_DECL_address( ureg ); } /* Declare misc input registers */ - { - GLbitfield sysInputs = program->info.system_values_read; - - for (i = 0; sysInputs; i++) { - if (sysInputs & (1 << i)) { -unsigned semName = _mesa_sysval_to_semantic(i); - -t->systemValues[i] = ureg_DECL_system_value(ureg, semName, 0); - -if (semName == TGSI_SEMANTIC_INSTANCEID || -semName == TGSI_SEMANTIC_VERTEXID) { - /* From Gallium perspective, these system values are always -* integer, and require native integer support. However, if -* native integer is supported on the vertex stage but not the -* pixel stage (e.g, i915g + draw), Mesa will generate IR that -* assumes these system values are floats. To resolve the -* inconsistency, we insert a U2F. -*/ - struct st_context *st = st_context(ctx); - struct pipe_screen *pscreen = st->pipe->screen; - assert(procType == PIPE_SHADER_VERTEX); - assert(pscreen->get_shader_param(pscreen, PIPE_SHADER_VERTEX, PIPE_SHADER_CAP_INTEGERS)); - (void) pscreen; /* silence non-debug build warnings */ - if (!ctx->Const.NativeIntegers) { - struct ureg_dst temp = ureg_DECL_local_temporary(t->ureg); - ureg_U2F( t->ureg, ureg_writemask(temp, TGSI_WRITEMASK_X), t->systemValues[i]); - t->systemValues[i] = ureg_scalar(ureg_src(temp), 0); - } + GLbitfield sysInputs = program->info.system_values_read; + for (i = 0; sysInputs; i++) { + if (sysInputs & (1 << i)) { + unsigned semName = _mesa_sysval_to_semantic(i); + + t->systemValues[i] = ureg_DECL_system_value(ureg, semName, 0); + + if (semName == TGSI_SEMANTIC_INSTANCEID || + semName == TGSI_SEMANTIC_VERTEXID) { +/* From Gallium perspective, these system values are always + * integer, and require native integer support. However, if + * native integer is supported on the vertex stage but not the + * pixel stage (e.g, i915g + draw), Mesa will generate IR that + * assumes these system values are floats. To resolve the + * inconsistency, we insert a U2F. + */ +struct st_context *st = st_context(ctx); +struct pipe_screen *pscreen = st->pipe->screen; +assert(procType == PIPE_SHADER_VERTEX); +assert(pscreen->get_shader_param(pscreen, PIPE_SHADER_VERTEX, + PIPE_SHADER_CAP_INTEGERS)); +(void) pscreen; /* silence non-debug build warnings */ +if (!ctx->Const.NativeIntegers) { + struct ureg_dst temp = ureg_DECL_local_temporary(t->ureg); + ureg_U2F(t->ureg, ureg_writemask(temp, TGSI_WRITEMASK_X), +t->systemValues[i]); + t->systemValues[i] = ureg_scalar(ureg_src(temp), 0); } + } -if (procType == PIPE_SHADER_FRAGMENT && -semName == TGSI_SEMANTIC_POSITION) - emit_wpos(st_context(ctx), t, program, ureg); + if (procType == PIPE_SHADER_FRAGMENT && + semName == TGSI_SEMANTIC_POSITION) +emit_wpos(st_context(ctx), t, program, ureg); -sysInputs &= ~(1 << i); - } + sysInputs &= ~(1 << i); } } if (program->arb.IndirectRegisterFiles & (1 << PROGRAM_TEMPORARY)) { /* If temps are accessed with indirect addressing, declare temporaries * in sequential order. Else, we declare them on demand elsewhere. */ for (i = 0; i < program->arb.NumTemporaries; i++) { /* XXX use TGSI_FILE_TEMPORARY_ARRAY when it's supported by ureg */ t->temps[i] = ureg_DECL_temporary( t->ureg ); -- 2.9.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 101214] xdriinfo and libglvnd Screen 0: not direct rendering capable
https://bugs.freedesktop.org/show_bug.cgi?id=101214 --- Comment #4 from LoneVVolf --- Created attachment 131570 --> https://bugs.freedesktop.org/attachment.cgi?id=131570&action=edit patch to give xdriinfo glvnd support created by hans de goede -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 101214] xdriinfo and libglvnd Screen 0: not direct rendering capable
https://bugs.freedesktop.org/show_bug.cgi?id=101214 --- Comment #3 from LoneVVolf --- xdriinfo build from git didn't change anything. I dug deeper, found that fedora core has put xdriinfo in their glx-utils package which is a sub pacakage of their mesa-demos package . downloaded the latest source-rpm from https://koji.fedoraproject.org/koji/buildinfo?buildID=868167 . after extracting the rpm i found xdriinfo-1.0.4-glvnd.patch (attached ). Applying that patch to xdriinfo from git solved the issue. Now to figure out how to continue and get that info to xdriinfo upstream. -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] util: make set's deleted_key_value declaration consistent with hash table one
This also silences following clang warnings: no previous extern declaration for non-static variable 'deleted_key' [-Werror,-Wmissing-variable-declarations] const void *deleted_key = &deleted_key_value; ^ no previous extern declaration for non-static variable 'deleted_key_value' [-Werror,-Wmissing-variable-declarations] uint32_t deleted_key_value; ^ --- src/util/set.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/set.c b/src/util/set.c index 99abefd063..3925066395 100644 --- a/src/util/set.c +++ b/src/util/set.c @@ -45,8 +45,8 @@ * free to avoid exponential performance degradation as the hash table fills */ -uint32_t deleted_key_value; -const void *deleted_key = &deleted_key_value; +static const uint32_t deleted_key_value; +static const void *deleted_key = &deleted_key_value; static const struct { uint32_t max_entries, size, rehash; -- 2.11.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 54971] __glXInitialize can initialize same display multiple times
https://bugs.freedesktop.org/show_bug.cgi?id=54971 Jordan Justen changed: What|Removed |Added Blocks||99831 Referenced Bugs: https://bugs.freedesktop.org/show_bug.cgi?id=99831 [Bug 99831] running qrenderdoc exits with "intel_do_flush_locked failed" -- You are receiving this mail because: You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 101214] xdriinfo and libglvnd Screen 0: not direct rendering capable
https://bugs.freedesktop.org/show_bug.cgi?id=101214 --- Comment #2 from LoneVVolf --- Can't find a commit with that hash, or do you mean https://cgit.freedesktop.org/mesa/mesa/commit/?id=84f764a7591715104b28c035c837ce9fd86157ad glxglvnddispatch: Add missing dispatch for GetDriverConfig Together with some fixes to xdriinfo this fixes xdriinfo not working with glvnd. That commit is present in my build, looking for the fixes to xdriinfo now. https://cgit.freedesktop.org/xorg/app/xdriinfo/log/ shows 3 commits after last release, going to build xdriinfo from git. -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 99116] Wine program showing only a blackscreen when using mesa
https://bugs.freedesktop.org/show_bug.cgi?id=99116 --- Comment #14 from JL --- (In reply to Józef Kucia from comment #13) > We've got a piglit test for this bug: > https://cgit.freedesktop.org/piglit/commit/ > ?id=eae8e3d1f2d33729fbad9a9433c1c91fd29dae2b "out": "Probe color at (0,0)\n Expected: 0 255 0\n Observed: 0 0 0\n", "result": "fail", -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Rework Sandy Bridge HiZ and stencil layouts
+chad On Mon, May 29, 2017 at 12:09 PM, Jason Ekstrand wrote: > Sandy Bridge does not technically support mipmapped depth/stencil. In > order to work around this, we allocate what are effectively completely > separate images for each miplevel, ensure that they are page-aligned, > and manually offset to them. Prior to layered rendering, this was a > simple matter of setting a large enough halign/valign. > > With the advent of layered rendering, however, things got more > complicated. Now, things weren't as simple as just handing a surface > off to the hardware. Any miplevel of a normally mipmapped surface can > be considered as just an array surface given the right qpitch. However, > the hardware gives us no capability to specify qpitch so this won't > work. Instead, the chosen solution was to use a new "all slices at each > LOD" layout which laid things out as a mipmap of arrays rather than an > array of mipmaps. This way you can easily offset to any of the > miplevels and each is a valid array. > > Unfortunately, the "all slices at each lod" concept missed one > fundamental thing about SNB HiZ and stencil hardware: It doesn't just > always act as if you're always working with a non-mipmapped surface, it > acts as if you're always working on a non-mipmapped surface of the same > size as LOD0. In other words, even though it may only write the > upper-left corner of each array slice, the qpitch for the array is for a > surface the size of LOD0 of the depth surface. This mistake causes us > to under-allocate HiZ and stencil in some cases and also to accidentally > allow different miplevels to overlap. Sadly, piglit test coverage > didn't quite catch this until I started making changes to the resolve > code that caused additional HiZ resolves in certain tests. > > This commit switches Sandy Bridge HiZ and stencil over to a new scheme > that lays out the non-zero miplevels horizontally below LOD0. This way > they can all have the same qpitch without interfering with each other. > Technically, the miplevels still overlap, but things are spaced out > enough that each page is only in the "written area" of one LOD. Hopefully, > this will get rid of at least some of the random SNB hangs. > > Cc: "17.0 17.1" > Cc: Topi Pohjolainen > Cc: Nanley Chery > Cc: Jordan Justen > Cc: Kenneth Graunke > > --- > The series I sent out on Friday suffered from a GPU hang or two on Sandy > Bridge. It turns out that those hangs were caused by the hardware HiZ > resolving part of my batch buffer due to this under-allocation. > > Topi, I'm sorry but this will likely make hash of your earlier patch > series. Sadly, I don't think there's really anything else we can do. :-( > Also, given how tricky this is to get right, I concede that we may want to > add an ISL_DIM_LAYOUT_GEN6_HIZ_STENCIL layout to ISL. We could still do > it > with the "array of surfaces" approach but I think these sorts of > calculations are best done in with the other surface calculation code. I > can help draft it if you'd like. > > src/mesa/drivers/dri/i965/brw_blorp.c | 11 ++- > src/mesa/drivers/dri/i965/brw_tex_layout.c| 100 > ++ > src/mesa/drivers/dri/i965/gen6_depth_state.c | 4 +- > src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 11 +-- > src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 37 +- > 5 files changed, 134 insertions(+), 29 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c > b/src/mesa/drivers/dri/i965/brw_blorp.c > index 6e860f0..cb9933b 100644 > --- a/src/mesa/drivers/dri/i965/brw_blorp.c > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c > @@ -123,7 +123,7 @@ apply_gen6_stencil_hiz_offset(struct isl_surf *surf, >uint32_t lod, >uint32_t *offset) > { > - assert(mt->array_layout == ALL_SLICES_AT_EACH_LOD); > + assert(mt->array_layout == GEN6_HIZ_STENCIL); > > if (mt->format == MESA_FORMAT_S_UINT8) { >/* Note: we can't compute the stencil offset using > @@ -183,12 +183,12 @@ blorp_surf_for_miptree(struct brw_context *brw, > }; > > if (brw->gen == 6 && mt->format == MESA_FORMAT_S_UINT8 && > - mt->array_layout == ALL_SLICES_AT_EACH_LOD) { > - /* Sandy bridge stencil and HiZ use this ALL_SLICES_AT_EACH_LOD > hack in > + mt->array_layout == GEN6_HIZ_STENCIL) { > + /* Sandy bridge stencil and HiZ use this GEN6_HIZ_STENCIL hack in > * order to allow for layered rendering. The hack makes each LOD > of the > * stencil or HiZ buffer a single tightly packed array surface at > some > * offset into the surface. Since ISL doesn't know how to deal > with the > - * crazy ALL_SLICES_AT_EACH_LOD layout and since we have to do a > manual > + * crazy GEN6_HIZ_STENCIL layout and since we have to do a manual > * offset of it anyway, we might as well do the offset here and > keep the > * hacks inside the i965 driver. >
[Mesa-dev] [PATCH] i965: Rework Sandy Bridge HiZ and stencil layouts
Sandy Bridge does not technically support mipmapped depth/stencil. In order to work around this, we allocate what are effectively completely separate images for each miplevel, ensure that they are page-aligned, and manually offset to them. Prior to layered rendering, this was a simple matter of setting a large enough halign/valign. With the advent of layered rendering, however, things got more complicated. Now, things weren't as simple as just handing a surface off to the hardware. Any miplevel of a normally mipmapped surface can be considered as just an array surface given the right qpitch. However, the hardware gives us no capability to specify qpitch so this won't work. Instead, the chosen solution was to use a new "all slices at each LOD" layout which laid things out as a mipmap of arrays rather than an array of mipmaps. This way you can easily offset to any of the miplevels and each is a valid array. Unfortunately, the "all slices at each lod" concept missed one fundamental thing about SNB HiZ and stencil hardware: It doesn't just always act as if you're always working with a non-mipmapped surface, it acts as if you're always working on a non-mipmapped surface of the same size as LOD0. In other words, even though it may only write the upper-left corner of each array slice, the qpitch for the array is for a surface the size of LOD0 of the depth surface. This mistake causes us to under-allocate HiZ and stencil in some cases and also to accidentally allow different miplevels to overlap. Sadly, piglit test coverage didn't quite catch this until I started making changes to the resolve code that caused additional HiZ resolves in certain tests. This commit switches Sandy Bridge HiZ and stencil over to a new scheme that lays out the non-zero miplevels horizontally below LOD0. This way they can all have the same qpitch without interfering with each other. Technically, the miplevels still overlap, but things are spaced out enough that each page is only in the "written area" of one LOD. Hopefully, this will get rid of at least some of the random SNB hangs. Cc: "17.0 17.1" Cc: Topi Pohjolainen Cc: Nanley Chery Cc: Jordan Justen Cc: Kenneth Graunke --- The series I sent out on Friday suffered from a GPU hang or two on Sandy Bridge. It turns out that those hangs were caused by the hardware HiZ resolving part of my batch buffer due to this under-allocation. Topi, I'm sorry but this will likely make hash of your earlier patch series. Sadly, I don't think there's really anything else we can do. :-( Also, given how tricky this is to get right, I concede that we may want to add an ISL_DIM_LAYOUT_GEN6_HIZ_STENCIL layout to ISL. We could still do it with the "array of surfaces" approach but I think these sorts of calculations are best done in with the other surface calculation code. I can help draft it if you'd like. src/mesa/drivers/dri/i965/brw_blorp.c | 11 ++- src/mesa/drivers/dri/i965/brw_tex_layout.c| 100 ++ src/mesa/drivers/dri/i965/gen6_depth_state.c | 4 +- src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 11 +-- src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 37 +- 5 files changed, 134 insertions(+), 29 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c b/src/mesa/drivers/dri/i965/brw_blorp.c index 6e860f0..cb9933b 100644 --- a/src/mesa/drivers/dri/i965/brw_blorp.c +++ b/src/mesa/drivers/dri/i965/brw_blorp.c @@ -123,7 +123,7 @@ apply_gen6_stencil_hiz_offset(struct isl_surf *surf, uint32_t lod, uint32_t *offset) { - assert(mt->array_layout == ALL_SLICES_AT_EACH_LOD); + assert(mt->array_layout == GEN6_HIZ_STENCIL); if (mt->format == MESA_FORMAT_S_UINT8) { /* Note: we can't compute the stencil offset using @@ -183,12 +183,12 @@ blorp_surf_for_miptree(struct brw_context *brw, }; if (brw->gen == 6 && mt->format == MESA_FORMAT_S_UINT8 && - mt->array_layout == ALL_SLICES_AT_EACH_LOD) { - /* Sandy bridge stencil and HiZ use this ALL_SLICES_AT_EACH_LOD hack in + mt->array_layout == GEN6_HIZ_STENCIL) { + /* Sandy bridge stencil and HiZ use this GEN6_HIZ_STENCIL hack in * order to allow for layered rendering. The hack makes each LOD of the * stencil or HiZ buffer a single tightly packed array surface at some * offset into the surface. Since ISL doesn't know how to deal with the - * crazy ALL_SLICES_AT_EACH_LOD layout and since we have to do a manual + * crazy GEN6_HIZ_STENCIL layout and since we have to do a manual * offset of it anyway, we might as well do the offset here and keep the * hacks inside the i965 driver. * @@ -241,8 +241,7 @@ blorp_surf_for_miptree(struct brw_context *brw, struct intel_mipmap_tree *hiz_mt = mt->hiz_buf->mt; if (hiz_mt) { -assert(brw->gen == 6 && - hiz_mt->array_layout == ALL_SL
[Mesa-dev] Mesa 17.0.7 release candidate
Hello list, The candidate for the Mesa 17.0.7 is now available. Currently we have: - 21 queued - 5 nominated (outstanding) - and 0 rejected patch(es) Note: this is the final anticipated release from the 17.0.x series. We have a few important outstanding patches which I'm planning to merge considering we have no regressions are reported by the Intel Jenkins instance. That aside, in the current queue we have: The mesa GLVND GLX library now handles glXGetDriverConfig, as used by driconf. For EGL the error checking within eglMakeCurrent has been relaxed. On the driver side: i965 and ANV have improved DXT1 and BC1 format handling. And the i965 vec4 backend has a few uniforms related fixes. The nouveau, etnaviv and vc4 drivers have seen minor improvements. Last but not least, we have several Wayland patches improving event queue handling. Take a look at section "Mesa stable queue" for more information. Testing reports/general approval Any testing reports (or general approval of the state of the branch) will be greatly appreciated. The plan is to have 17.0.7 this Wednesday (31th of May), around or shortly after 20:00 GMT. If you have any questions or suggestions - be that about the current patch queue or otherwise, please go ahead. Trivial merge conflicts --- commit 87d16afa6ff11e7c72870bdba4a514858b31f1ab Author: Nanley Chery i965/formats: Update the three-channel DXT1 mappings (cherry picked from commit 688ddb85c8c3357d8e1e9d360c74cd728b128d98) commit afbe5bf434fb059c5e06490f2e275445c60a Author: Daniel Stone vulkan/wsi/wayland: Use proxy wrappers for swapchain (cherry picked from commit 5034c615582add2be9309dc1d7383fb0daba6dd3) Cheers, Emil Mesa stable queue - Nominated (5) = Jason Ekstrand (2): 441cd7a i965/blorp: Do and end-of-pipe sync on both sides of fast-clear ops 0901d0b i965: Round copy size to the nearest block in intel_miptree_copy Emil Velikov (3): 3e8790b anv: automake: list shared libraries after the static ones 2b6ad89 radv: automake: list shared libraries after the static ones 6ef0fc4 egl/wayland: select the format based on the interface used Queued (21) === Chad Versace (1): egl: Partially revert 23c86c74, fix eglMakeCurrent Daniel Stone (7): vulkan: Fix Wayland uninitialised registry vulkan/wsi/wayland: Remove roundtrip when creating image vulkan/wsi/wayland: Use per-display event queue vulkan/wsi/wayland: Use proxy wrappers for swapchain egl/wayland: Ensure we get a back buffer egl/wayland: Don't open-code roundtrip egl/wayland: Use per-surface event queues Squashed with egl/wayland: verify event queue was allocated Emil Velikov (1): st/va: fix misplaced closing bracket Eric Anholt (2): renderonly: Initialize fields of struct winsys_handle. vc4: Don't allocate new BOs to avoid synchronization when they're shared. Hans de Goede (1): glxglvnddispatch: Add missing dispatch for GetDriverConfig Ilia Mirkin (1): nvc0/ir: SHLADD's middle source must be an immediate Lucas Stach (1): etnaviv: stop oversizing buffer resources Nanley Chery (2): anv/formats: Update the three-channel BC1 mappings i965/formats: Update the three-channel DXT1 mappings Pohjolainen, Topi (1): intel/isl/gen7: Use stencil vertical alignment of 8 instead of 4 Samuel Iglesias Gonsálvez (3): i965/vec4/gs: restore the uniform values which was overwritten by failed vec4_gs_visitor execution i965/vec4: fix swizzle and writemask when loading an uniform with constant offset i965/vec4: load dvec3/4 uniforms first in the push constant buffer Tom Stellard (1): gallivm: Make sure module has the correct data layout when pass manager runs Rejected (0) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] mesa: throw an INVALID_OPERATION error in get_texobj_by_name()
On 05/29/2017 08:10 PM, Emil Velikov wrote: Hi Samuel, Worth adding something vaguely like the following, since it's not immediately obvious? "get_texobj_by_name() can throw a GL_INVALID_ENUM, which gets overridden in the caller. Address that by updating by moving the GL_INVALID_OPERATION within the function itself." Hi Emil, Sure, no problem. :) -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] mesa: throw an INVALID_OPERATION error in get_texobj_by_name()
Hi Samuel, Worth adding something vaguely like the following, since it's not immediately obvious? "get_texobj_by_name() can throw a GL_INVALID_ENUM, which gets overridden in the caller. Address that by updating by moving the GL_INVALID_OPERATION within the function itself." -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 101199] nouveau_screen.c: undefined reference to `nouveau_drm_del'
https://bugs.freedesktop.org/show_bug.cgi?id=101199 --- Comment #6 from Emil Velikov --- I'd imagine that the system libdrm_nouveau is v2.6.65 or earlier and the issue happens only during make install. Is that correct? If so, it may be a libtool bug^feature and this workaround should work: $ export LD_LIBRARY_PATH=/path/to/libdrm/2.4.80/lib/:$LD_LIBRARY_PATH $ ./configure ... -- 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] [PATCH 3/3] anv: Port over CACHE_MODE_1 optimization fix enables from brw.
Looks good to me :) This series is: Reviewed-by: Plamena Manolova On Wed, May 24, 2017 at 8:56 AM, Kenneth Graunke wrote: > Ben and I haven't observed these to help anything, but they enable > hardware optimizations for particular cases. It's probably best to > enable them ahead of time, before we run into such a case. > --- > src/intel/vulkan/genX_state.c | 13 + > 1 file changed, 13 insertions(+) > > diff --git a/src/intel/vulkan/genX_state.c b/src/intel/vulkan/genX_state.c > index bf1217bbcdc..00c4105a825 100644 > --- a/src/intel/vulkan/genX_state.c > +++ b/src/intel/vulkan/genX_state.c > @@ -52,6 +52,19 @@ genX(init_device_state)(struct anv_device *device) >ps.PipelineSelection = _3D; > } > > +#if GEN_GEN >= 9 > + uint32_t cache_mode_1; > + anv_pack_struct(&cache_mode_1, GENX(CACHE_MODE_1), > + .PartialResolveDisableInVC = true, > + .PartialResolveDisableInVCMask = true, > + .FloatBlendOptimizationEnable = true, > + .FloatBlendOptimizationEnableMask = true); > + anv_batch_emit(&batch, GENX(MI_LOAD_REGISTER_IMM), lri) { > + lri.RegisterOffset = GENX(CACHE_MODE_1_num); > + lri.DataDWord = cache_mode_1; > + } > +#endif > + > anv_batch_emit(&batch, GENX(3DSTATE_AA_LINE_PARAMETERS), aa); > > anv_batch_emit(&batch, GENX(3DSTATE_DRAWING_RECTANGLE), rect) { > -- > 2.12.2 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] amd/common: add vcn dec ip info query for amdgpu version 3.17
On 26 May 2017 at 14:59, Marek Olšák wrote: > On Fri, May 26, 2017 at 3:07 PM, Emil Velikov > wrote: >> Hi Leo, >> >> On 26 May 2017 at 12:24, Leo Liu wrote: >>> Signed-off-by: Leo Liu >>> --- >>> src/amd/common/ac_gpu_info.c | 10 +- >>> 1 file changed, 9 insertions(+), 1 deletion(-) >>> >>> diff --git a/src/amd/common/ac_gpu_info.c b/src/amd/common/ac_gpu_info.c >>> index 56645c4..3dae2bd 100644 >>> --- a/src/amd/common/ac_gpu_info.c >>> +++ b/src/amd/common/ac_gpu_info.c >>> @@ -88,7 +88,7 @@ bool ac_query_gpu_info(int fd, amdgpu_device_handle dev, >>> { >>> struct amdgpu_buffer_size_alignments alignment_info = {}; >>> struct amdgpu_heap_info vram, vram_vis, gtt; >>> - struct drm_amdgpu_info_hw_ip dma = {}, compute = {}, uvd = {}, vce >>> = {}; >>> + struct drm_amdgpu_info_hw_ip dma = {}, compute = {}, uvd = {}, vce >>> = {}, vcn_dec = {}; >>> uint32_t vce_version = 0, vce_feature = 0, uvd_version = 0, >>> uvd_feature = 0; >>> uint32_t unused_feature; >>> int r, i, j; >>> @@ -157,6 +157,14 @@ bool ac_query_gpu_info(int fd, amdgpu_device_handle >>> dev, >>> return false; >>> } >>> >>> + if (info->drm_major == 3 && info->drm_minor >= 17) { >>> + r = amdgpu_query_hw_ip_info(dev, AMDGPU_HW_IP_VCN_DEC, 0, >>> &vcn_dec); >>> + if (r) { >>> + fprintf(stderr, "amdgpu: >>> amdgpu_query_hw_ip_info(vcn_dec) failed.\n"); >>> + return false; >> Drive-by question: >> >> What would happen if the hardware does not support the said engine - >> is the ioctl going to fail or it will succeed storing "unsupported >> engine" type of information in vcn_dec? >> I think the code needs to be addressed in either case. > > If the engine is unsupported, the kernel driver returns > drm_amdgpu_info_hw_ip::available_rings == 0. > Indeed. Pardon for the silly question :-\ -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v5 0/4] Disable glthread if libX11 isn't thread-safe
On 29 May 2017 at 15:45, Dieter Nützel wrote: > Hi Gregory, > > there isn't currently a copy of this on Mesa-Patchwork. > Can you please send one over there? > > And maybe an updated version of: > [PATCH v5 0/3] asynchronous pbo transfer with glthread > > Would be awesome. > The series is in master now, so it should be a bit easier ;-) Gregory, thanks again for the work. Please keep an eye open for reports. it's highly unlikely but still. -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 2/5] r600: remove custom and incomplete OpenCL code paths
The "ac" functions could also be forked and put into r600 if people want to preserve the OpenCL support. That would remove the dependency on "ac". Marek On Mon, May 29, 2017 at 3:46 PM, Emil Velikov wrote: > From: Emil Velikov > > The code hasn't bee en touched in a very long time, and was never > completed. > > It is conditionally built only when OpenCL is selected at built time, > and pulls the libamd_common static library, which in itself depends on > amdgpu. > > With later commit(s) we'll address the amdgpu dependency, but for now > drop this partial code. If anyone is interested in reviving and beating > it into shape they are welcome to git revert. > > v2: remove unused radeon_shader_binary_clean() call and struct > r600_pipe_compute::binary. Add ac_binary.h include in r600_pipe_common.h > > Cc: Nicolai Hähnle > Cc: Marek Olšák > Cc: Dave Airlie > Cc: Aaron Watry > Cc: Jan Vesely > Signed-off-by: Emil Velikov > --- > > Gents, > > Yes, this patch removes OpenCL for r600 and yes, ideally we'll get > people to finish it. > > At the same time the (r300/r600 only) build was broken for over a week > so I've opted for the simple solution as mentioned in [1]. > > Feel free to address properly, but until then lets merge something so > that people can actualy build the driver(s). > > [1] https://lists.freedesktop.org/archives/mesa-dev/2017-May/156429.html > > src/gallium/drivers/r600/Makefile.am | 5 -- > src/gallium/drivers/r600/evergreen_compute.c | 91 > -- > .../drivers/r600/evergreen_compute_internal.h | 8 -- > 3 files changed, 104 deletions(-) > > diff --git a/src/gallium/drivers/r600/Makefile.am > b/src/gallium/drivers/r600/Makefile.am > index 21762d838d0..127e7424b74 100644 > --- a/src/gallium/drivers/r600/Makefile.am > +++ b/src/gallium/drivers/r600/Makefile.am > @@ -25,11 +25,6 @@ AM_CFLAGS += \ > > endif > > -if HAVE_GALLIUM_COMPUTE > -AM_CFLAGS += \ > - -DHAVE_OPENCL > -endif > - > EXTRA_DIST = \ > sb/notes.markdown \ > sb/sb_bc_fmt_def.inc > diff --git a/src/gallium/drivers/r600/evergreen_compute.c > b/src/gallium/drivers/r600/evergreen_compute.c > index 37ef1058d3f..fec49f439dd 100644 > --- a/src/gallium/drivers/r600/evergreen_compute.c > +++ b/src/gallium/drivers/r600/evergreen_compute.c > @@ -26,7 +26,6 @@ > > #include > #include > -#include "ac_binary.h" > #include "pipe/p_defines.h" > #include "pipe/p_state.h" > #include "pipe/p_context.h" > @@ -172,65 +171,6 @@ static void evergreen_cs_set_constant_buffer(struct > r600_context *rctx, > rctx->b.b.set_constant_buffer(&rctx->b.b, PIPE_SHADER_COMPUTE, > cb_index, &cb); > } > > -/* We need to define these R600 registers here, because we can't include > - * evergreend.h and r600d.h. > - */ > -#define R_028868_SQ_PGM_RESOURCES_VS 0x028868 > -#define R_028850_SQ_PGM_RESOURCES_PS 0x028850 > - > -#ifdef HAVE_OPENCL > - > -static void r600_shader_binary_read_config(const struct ac_shader_binary > *binary, > - struct r600_bytecode *bc, > - uint64_t symbol_offset, > - boolean *use_kill) > -{ > - unsigned i; > - const unsigned char *config = > - ac_shader_binary_config_start(binary, symbol_offset); > - > - for (i = 0; i < binary->config_size_per_symbol; i+= 8) { > - unsigned reg = > - util_le32_to_cpu(*(uint32_t*)(config + i)); > - unsigned value = > - util_le32_to_cpu(*(uint32_t*)(config + i + 4)); > - switch (reg) { > - /* R600 / R700 */ > - case R_028850_SQ_PGM_RESOURCES_PS: > - case R_028868_SQ_PGM_RESOURCES_VS: > - /* Evergreen / Northern Islands */ > - case R_028844_SQ_PGM_RESOURCES_PS: > - case R_028860_SQ_PGM_RESOURCES_VS: > - case R_0288D4_SQ_PGM_RESOURCES_LS: > - bc->ngpr = MAX2(bc->ngpr, G_028844_NUM_GPRS(value)); > - bc->nstack = MAX2(bc->nstack, > G_028844_STACK_SIZE(value)); > - break; > - case R_02880C_DB_SHADER_CONTROL: > - *use_kill = G_02880C_KILL_ENABLE(value); > - break; > - case R_0288E8_SQ_LDS_ALLOC: > - bc->nlds_dw = value; > - break; > - } > - } > -} > - > -static unsigned r600_create_shader(struct r600_bytecode *bc, > - const struct ac_shader_binary *binary, > - boolean *use_kill) > - > -{ > - assert(binary->code_size % 4 == 0); > - bc->bytecode = CALLOC(1, binary->code_size); > - memcpy(bc->bytecode, binary->code, binary->code_size); > - bc->ndw = binary->code_size / 4; > - >
Re: [Mesa-dev] [PATCH 0/2] egl/android: A few trivial cleanups
For the series Reviewed-by: Emil Velikov -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] android: radeon(s): fix libdrm_amdgpu shared dependencies
On 27 May 2017 at 10:59, Mauro Rossi wrote: > > > 2017-05-22 1:34 GMT+02:00 Mauro Rossi : >> >> >> >> 2017-05-21 18:27 GMT+02:00 Emil Velikov : >>> >>> Hi Mauro, >>> >>> There is a similar issue when building with autotools. There's a few >>> ways to address this so let's see what the devs prefer. >>> >>> Another temporary workaround is to build radeonsi alongside the other >>> radeon drivers. >>> >>> -Emil >> >> >> Just FYI, I am already building radeonsi (target libmesa_pipe_radeonsi) >> Mauro > > > ...continuing the sentence so building radeonsi (even if not working because > llvm 3.8 in nougat) > is not a workaround the r% drivers building errors. > > After commit 44b29dd "amd/common: add missing libdrm include path", > is there an alternative proposed solution for android building errors that > compares with submitted patch? > The patch mentioned is incomplete/wrong. Can you try "rev 3" of the following series https://patchwork.freedesktop.org/series/24960/ -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] amd/common: add vcn dec ip info query for amdgpu version 3.17
Reviewed-by: Marek Olšák Marek On Fri, May 26, 2017 at 1:24 PM, Leo Liu wrote: > Signed-off-by: Leo Liu > --- > src/amd/common/ac_gpu_info.c | 10 +- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/src/amd/common/ac_gpu_info.c b/src/amd/common/ac_gpu_info.c > index 56645c4..3dae2bd 100644 > --- a/src/amd/common/ac_gpu_info.c > +++ b/src/amd/common/ac_gpu_info.c > @@ -88,7 +88,7 @@ bool ac_query_gpu_info(int fd, amdgpu_device_handle dev, > { > struct amdgpu_buffer_size_alignments alignment_info = {}; > struct amdgpu_heap_info vram, vram_vis, gtt; > - struct drm_amdgpu_info_hw_ip dma = {}, compute = {}, uvd = {}, vce = > {}; > + struct drm_amdgpu_info_hw_ip dma = {}, compute = {}, uvd = {}, vce = > {}, vcn_dec = {}; > uint32_t vce_version = 0, vce_feature = 0, uvd_version = 0, > uvd_feature = 0; > uint32_t unused_feature; > int r, i, j; > @@ -157,6 +157,14 @@ bool ac_query_gpu_info(int fd, amdgpu_device_handle dev, > return false; > } > > + if (info->drm_major == 3 && info->drm_minor >= 17) { > + r = amdgpu_query_hw_ip_info(dev, AMDGPU_HW_IP_VCN_DEC, 0, > &vcn_dec); > + if (r) { > + fprintf(stderr, "amdgpu: > amdgpu_query_hw_ip_info(vcn_dec) failed.\n"); > + return false; > + } > + } > + > r = amdgpu_query_firmware_version(dev, AMDGPU_INFO_FW_GFX_ME, 0, 0, > &info->me_fw_version, > &unused_feature); > if (r) { > -- > 2.9.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] [Bug 101214] xdriinfo and libglvnd Screen 0: not direct rendering capable
https://bugs.freedesktop.org/show_bug.cgi?id=101214 --- Comment #1 from Emil Velikov --- Can you double-check that your build has the following commit c93f8f0cb3e8b655b0b39f2081bc5ad374abc79d. AFAICT it's the same one used by the Fedora folks, with a trivial cleanup. -- 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] [PATCH 5/9] i965: Move an 'i' declaration into its 'for' loop
On 29 May 2017 at 07:56, Martin Peres wrote: > On 27/05/17 01:15, Chad Versace wrote: >> In intel_update_dri2_buffers(). >> Trivial cleanup. >> --- >> src/mesa/drivers/dri/i965/brw_context.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/src/mesa/drivers/dri/i965/brw_context.c >> b/src/mesa/drivers/dri/i965/brw_context.c >> index adae921e57..825912b7b5 100644 >> --- a/src/mesa/drivers/dri/i965/brw_context.c >> +++ b/src/mesa/drivers/dri/i965/brw_context.c >> @@ -1410,7 +1410,7 @@ intel_update_dri2_buffers(struct brw_context *brw, >> __DRIdrawable *drawable) >> struct gl_framebuffer *fb = drawable->driverPrivate; >> struct intel_renderbuffer *rb; >> __DRIbuffer *buffers = NULL; >> - int i, count; >> + int count; >> const char *region_name; >> >> /* Set this up front, so that in case our buffers get invalidated >> @@ -1426,7 +1426,7 @@ intel_update_dri2_buffers(struct brw_context *brw, >> __DRIdrawable *drawable) >> if (buffers == NULL) >>return; >> >> - for (i = 0; i < count; i++) { >> + for (int i = 0; i < count; i++) { >> switch (buffers[i].attachment) { >> case __DRI_BUFFER_FRONT_LEFT: >> rb = intel_get_renderbuffer(fb, BUFFER_FRONT_LEFT); >> > > Same comment as in patch 2, this requires C99. Do we compile mesa in c99 > nowadays? Very short answer - yes. A slightly longer one is available in configure.ac - look for MSVC2013_COMPAT_CFLAGS -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] configure.ac: Add --with-wayland-scanner-path
On 29 May 2017 at 14:05, Jussi Kukkonen wrote: > On 29 May 2017 at 15:20, Emil Velikov wrote: >> >> On 26 May 2017 at 14:55, Jussi Kukkonen wrote: >> > On 26 May 2017 at 14:32, Emil Velikov wrote: >> >> >> >> On 26 May 2017 at 08:52, Jussi Kukkonen >> >> wrote: >> >> > >> >> > >> >> > On 24 May 2017 at 16:39, Emil Velikov >> >> > wrote: >> >> AFAICT there are a couple of alternative solutions - have you >> >> considered/tried any of them? >> >> >> >> a) w/o a wrapper script >> >> $ export PKG_CONFIG_PATH= // you need this if using the system >> >> pkgo-config. if the local/native one is used, this should be optional >> >> $ export >> >> >> >> PKG_CONFIG_LIBDIR=$path_to_non_system_host_pkgconfig:${SYSROOT}/usr/{lib,share}/pkgconfig >> >> $ export PKG_CONFIG_SYSROOT_DIR=${SYSROOT} >> > >> > We're using a natively built pkg-config which sets all the pkg-config >> > variables to what I believe are the correct values: the problem is that >> > none >> > of those variable is for the _native_ sysroot location so they don't >> > help in >> > this case. There is now way to say in a .pc file that this path should >> > be >> > prefixed with something like "${pc_native_sysroot_dir}" if it's >> > defined. >> > >> >> b) with a wrapper script - see [1]. >> >> I think that the "export PKG_CONFIG_DIR=" is a typo (should be ..PATH >> >> instead) and is not strictly required - feel free to check either way. >> >> Note that the exec at the end might need to be amended to >> >> /path/to/$(CHOST)-pkg-config. >> > >> > We don't provide a target wrapper -- I believe because it provides no >> > value >> > at all on top of the setup we have (the pkg-config that autotools finds >> > has >> > all the environment variables set correctly. It is essentially >> > $(CHOST)-pkg-config already). >> > >> > If I've missed something, I'd be happy to hear that. At the moment I >> > think >> > pkg-config just does not help in this case. >> > >> I'm confused a bit - did you try the above suggestions? If so can you >> share which one and how it fails? > > > I'm sorry but I do not see what I could test that could help: I mentioned > that the pkg-config that gets used by PKG_CHECK_MODULES() is essentially a > wrapped one: In more detail the build tool sets these variables: > Giving it a try won't hurt, right ;-) But on a more serious note: [1] Like any project in the wild Yocto might have bugs, please try w/o it. [2] A simple test [w/o Yocto] with my earlier suggestion seems to work fine Thanks Emil [1] From a quick look Yocto seems to be doing things a bit strange, wrong even? This is the first time I'm looking through it, so I may be wrong. Some examples, with the first and foremost being the prime suspect why things don't work as expected. * Yocto uses PKG_CONFIG_DIR. The variable is not a thing used by pkg-config (or pkgconf). Yes sometimes it's used only to be fed into LIBDIR/PATH (meta/conf/bitbake.conf), but it does not seem consistent. IMHO a good first step would be is to drop or rename it to PATH. * The native pkg-config has correct PATH/LIBDIR burned into the binary * A pkg-config-native wrapper also sets the PATH/LIBDIR variables - those are the default already stored within the binary - SYSROOT_DIR is explicitly discarded * Any PKG_CHECK_MODULES(.*) calls are discarded(??) - see wayland_1.11.0.bb [2] The following seems to work based on a quick test. -- Layout /usr/bin/{pkg-config,wayland-scanner} /usr/lib/pkgconfig/wayland-scanner.pc /native/usr/bin/{pkg-config,wayland-scanner} /native/usr/lib/pkgconfig/wayland-scanner.pc /target/usr/bin/{pkg-config,wayland-scanner} // just an example, one or both can be missing /target/usr/lib/pkgconfig/wayland-scanner.pc -- cat $(CHOST)-pkg-config #!/bin/sh export SYSROOT=/target/ export PKG_CONFIG_LIBDIR=/native/usr/lib/pkgconfig/:${SYSROOT}/usr/lib/pkgconfig export PKG_CONFIG_SYSROOT_DIR=${SYSROOT} /native/usr/bin/pkg-config "$@" ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 101211] Mesa swrast issue with visualization on BE PPC PPC64
https://bugs.freedesktop.org/show_bug.cgi?id=101211 --- Comment #5 from intermedi...@hotmail.com --- Investigating better the issue is present if LLVM. note llvm is needed for radeonsi egl. usually all distro have build the mesa in llvm. I been forced the build without llvm and without radeonsi dri. and mesa visualization now is ok. but is not a right way to fixing. -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] r600: Regarding "Failed to build shader (translation from TGSI) #99349
Hello all, Hardware; Radeon 6850HD, Mesa: mesa 17.0.1 and git (sha 531887), llvm: 4.0.0 Playing a bit around with the Unreal Editor I was confronted with the same error message reported in #99349, i.e. "Failed to build shader (translation from TGSI). After some digging though the code I found that the TGSI code [1] of the offending shader reserves 151 temporaries so that the available 128 GPRs are already allocated right from the start, and when the operation "MUL TEMP[11], CONST[26], CONST[23]" is translated to byte code, both constants are read from the cfile region, because tgsi_split_constant could not move one constant to a proper GPR. As one can see in the TGSI dump [1], the shader does not really use 151 temporaries, only 40 are actually also addresses as source, to all the other temps values are just written once (assuming the the TGSI notation is OP DEST, SRC0, SRC1 ...). My questions are now: Does the GSLS-TGSI stage of the compilation do any optimizations? Specifically, should the unused temporaries be eliminated in that step and that I get this TGSI-dump is actually a bug in this compilation stage? (In the Gallium3D wikipedia article [2] it is written that there is a TGSI optimization stage.) As far as I understand there is a optimization pass done after the TGSI translation, but because of the nature of the problem the shader is rejected before. Would it make sense to implement a patch that would work around this problem by reserving some GORs to move constants to (and the temporary that is now ctx.temp_reg), and then test the number of allocated registers only after the byte code optimization? I partially implemented something like this [3] when I tried to find the source of the bug, so I could clean that up and propose a patch, so far the graphical output is clobbered though. many thank, Gert [1] https://bugs.freedesktop.org/attachment.cgi?id=131567 (12kb, xz compressed) [2] https://en.wikipedia.org/wiki/Gallium3D#Tungsten_Graphics_Shader_In frastructure [3] https://github.com/gerddie/mesa ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v5 0/4] Disable glthread if libX11 isn't thread-safe
Hi Gregory, there isn't currently a copy of this on Mesa-Patchwork. Can you please send one over there? And maybe an updated version of: [PATCH v5 0/3] asynchronous pbo transfer with glthread Would be awesome. Dieter Am 29.05.2017 13:18, schrieb Gregory Hainaut: Hello Mesa developers, Following the discussion from https://lists.freedesktop.org/archives/mesa-dev/2017-April/153137.html A check was added to ensure that X11 display can be locked. It should be enough to ensure thread safety between X11 and glthread. I also did the check on DRI3 as I'm not 100% sure that it is really thread safe. v2: based on Nicolai/Matt reviews Add a check on DRI extension version Use C comments :) v3: based on Emil reviews Split the initial first patch into 3 sub patches dri extension / glx / egl Improve error message Improve code readability Just include libX11 on EGL protected by ifdef v4: based on Eric feedback, I marked DRI3 as always thread safe v5: Fix the null pointer check on patch 4. I added Daniel comment on patch 3 but I'm not sure I got it right. Thanks you for all the review comments. Best regards, Gregory Hainaut (4): dri: Extend __DRIbackgroundCallableExtensionRec to include a callback that checks for thread safety glx: implement __DRIbackgroundCallableExtension.isThreadSafe egl: implement __DRIbackgroundCallableExtension.isThreadSafe glthread/gallium: require safe_glthread to start glthread include/GL/internal/dri_interface.h | 13 +++ src/egl/drivers/dri2/egl_dri2.c | 34 +++- src/gallium/state_trackers/dri/dri_context.c | 21 + src/glx/dri2_glx.c | 15 +++- src/glx/dri3_glx.c | 12 +- 5 files changed, 88 insertions(+), 7 deletions(-) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 4/5] ac: remove amdgpu.h dependency
From: Emil Velikov Add a couple of forward declarations and drop the amdgpu.h requirement. With this we can build the r300 and r600 drivers without the need for amdgpu. v2: - Add amdgpu.h include in the C file (Marek) - Add a comment about pre C11 typedef redeclaration warning (Eric) Cc: Nicolai Hähnle Cc: Marek Olšák Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101189 Signed-off-by: Emil Velikov --- src/amd/common/ac_gpu_info.c | 2 ++ src/amd/common/ac_gpu_info.h | 6 -- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/amd/common/ac_gpu_info.c b/src/amd/common/ac_gpu_info.c index 56645c48632..26868855148 100644 --- a/src/amd/common/ac_gpu_info.c +++ b/src/amd/common/ac_gpu_info.c @@ -34,6 +34,8 @@ #include #include +#include + #define CIK_TILE_MODE_COLOR_2D 14 #define CIK__GB_TILE_MODE__PIPE_CONFIG(x)(((x) >> 6) & 0x1f) diff --git a/src/amd/common/ac_gpu_info.h b/src/amd/common/ac_gpu_info.h index 3f7ade1a0bb..a72ab58f9a3 100644 --- a/src/amd/common/ac_gpu_info.h +++ b/src/amd/common/ac_gpu_info.h @@ -28,12 +28,14 @@ #include "amd_family.h" -#include - #ifdef __cplusplus extern "C" { #endif +/* Prior to C11 the following may trigger a typedef redeclaration warning */ +typedef void * amdgpu_device_handle; +struct amdgpu_gpu_info; + struct radeon_info { /* PCI info: domain:bus:dev:func */ uint32_tpci_domain; -- 2.12.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/5] ac: remove amdgpu.h dependency
On 26 May 2017 at 17:14, Eric Engestrom wrote: > On Friday, 2017-05-26 16:32:52 +0100, Emil Velikov wrote: >> From: Emil Velikov >> >> Add a couple of forward declarations and drop the amdgpu.h requirement. >> >> With this we can build the r300 and r600 drivers without the need for >> amdgpu. >> >> Cc: Nicolai Hähnle >> Cc: Marek Olšák >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101189 >> Signed-off-by: Emil Velikov >> --- >> Note that I have not extensively tested the series. Will do so in ~two hours. >> >> src/amd/common/ac_gpu_info.h | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/src/amd/common/ac_gpu_info.h b/src/amd/common/ac_gpu_info.h >> index 3f7ade1a0bb..dabd3b003e1 100644 >> --- a/src/amd/common/ac_gpu_info.h >> +++ b/src/amd/common/ac_gpu_info.h >> @@ -28,12 +28,13 @@ >> >> #include "amd_family.h" >> >> -#include >> - >> #ifdef __cplusplus >> extern "C" { >> #endif >> >> +typedef void * amdgpu_device_handle; > > Just FYI: this will cause a 'typedef redefinition' warning if amdgpu.h > gets included by something that also includes ac_gpu_info.h. > (This was made legal in C11, but isn't in C99) > Indeed it would and there's 3 ways to address it :-) Since nobody has stated their preference I'd add a comment and leave it as-is. -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 2/5] r600: remove custom and incomplete OpenCL code paths
From: Emil Velikov The code hasn't bee en touched in a very long time, and was never completed. It is conditionally built only when OpenCL is selected at built time, and pulls the libamd_common static library, which in itself depends on amdgpu. With later commit(s) we'll address the amdgpu dependency, but for now drop this partial code. If anyone is interested in reviving and beating it into shape they are welcome to git revert. v2: remove unused radeon_shader_binary_clean() call and struct r600_pipe_compute::binary. Add ac_binary.h include in r600_pipe_common.h Cc: Nicolai Hähnle Cc: Marek Olšák Cc: Dave Airlie Cc: Aaron Watry Cc: Jan Vesely Signed-off-by: Emil Velikov --- Gents, Yes, this patch removes OpenCL for r600 and yes, ideally we'll get people to finish it. At the same time the (r300/r600 only) build was broken for over a week so I've opted for the simple solution as mentioned in [1]. Feel free to address properly, but until then lets merge something so that people can actualy build the driver(s). [1] https://lists.freedesktop.org/archives/mesa-dev/2017-May/156429.html src/gallium/drivers/r600/Makefile.am | 5 -- src/gallium/drivers/r600/evergreen_compute.c | 91 -- .../drivers/r600/evergreen_compute_internal.h | 8 -- 3 files changed, 104 deletions(-) diff --git a/src/gallium/drivers/r600/Makefile.am b/src/gallium/drivers/r600/Makefile.am index 21762d838d0..127e7424b74 100644 --- a/src/gallium/drivers/r600/Makefile.am +++ b/src/gallium/drivers/r600/Makefile.am @@ -25,11 +25,6 @@ AM_CFLAGS += \ endif -if HAVE_GALLIUM_COMPUTE -AM_CFLAGS += \ - -DHAVE_OPENCL -endif - EXTRA_DIST = \ sb/notes.markdown \ sb/sb_bc_fmt_def.inc diff --git a/src/gallium/drivers/r600/evergreen_compute.c b/src/gallium/drivers/r600/evergreen_compute.c index 37ef1058d3f..fec49f439dd 100644 --- a/src/gallium/drivers/r600/evergreen_compute.c +++ b/src/gallium/drivers/r600/evergreen_compute.c @@ -26,7 +26,6 @@ #include #include -#include "ac_binary.h" #include "pipe/p_defines.h" #include "pipe/p_state.h" #include "pipe/p_context.h" @@ -172,65 +171,6 @@ static void evergreen_cs_set_constant_buffer(struct r600_context *rctx, rctx->b.b.set_constant_buffer(&rctx->b.b, PIPE_SHADER_COMPUTE, cb_index, &cb); } -/* We need to define these R600 registers here, because we can't include - * evergreend.h and r600d.h. - */ -#define R_028868_SQ_PGM_RESOURCES_VS 0x028868 -#define R_028850_SQ_PGM_RESOURCES_PS 0x028850 - -#ifdef HAVE_OPENCL - -static void r600_shader_binary_read_config(const struct ac_shader_binary *binary, - struct r600_bytecode *bc, - uint64_t symbol_offset, - boolean *use_kill) -{ - unsigned i; - const unsigned char *config = - ac_shader_binary_config_start(binary, symbol_offset); - - for (i = 0; i < binary->config_size_per_symbol; i+= 8) { - unsigned reg = - util_le32_to_cpu(*(uint32_t*)(config + i)); - unsigned value = - util_le32_to_cpu(*(uint32_t*)(config + i + 4)); - switch (reg) { - /* R600 / R700 */ - case R_028850_SQ_PGM_RESOURCES_PS: - case R_028868_SQ_PGM_RESOURCES_VS: - /* Evergreen / Northern Islands */ - case R_028844_SQ_PGM_RESOURCES_PS: - case R_028860_SQ_PGM_RESOURCES_VS: - case R_0288D4_SQ_PGM_RESOURCES_LS: - bc->ngpr = MAX2(bc->ngpr, G_028844_NUM_GPRS(value)); - bc->nstack = MAX2(bc->nstack, G_028844_STACK_SIZE(value)); - break; - case R_02880C_DB_SHADER_CONTROL: - *use_kill = G_02880C_KILL_ENABLE(value); - break; - case R_0288E8_SQ_LDS_ALLOC: - bc->nlds_dw = value; - break; - } - } -} - -static unsigned r600_create_shader(struct r600_bytecode *bc, - const struct ac_shader_binary *binary, - boolean *use_kill) - -{ - assert(binary->code_size % 4 == 0); - bc->bytecode = CALLOC(1, binary->code_size); - memcpy(bc->bytecode, binary->code, binary->code_size); - bc->ndw = binary->code_size / 4; - - r600_shader_binary_read_config(binary, bc, 0, use_kill); - return 0; -} - -#endif - static void r600_destroy_shader(struct r600_bytecode *bc) { FREE(bc->bytecode); @@ -241,27 +181,6 @@ static void *evergreen_create_compute_state(struct pipe_context *ctx, { struct r600_context *rctx = (struct r600_context *)ctx; struct r600_pipe_compute *shader = CALLOC_STRUCT(r600_pipe_compute); -#ifdef HAVE_OPENCL - cons
Re: [Mesa-dev] [PATCH] configure.ac: Add --with-wayland-scanner-path
On 26 May 2017 at 17:08, Daniel Stone wrote: > Hi Jussi, > > On 26 May 2017 at 14:55, Jussi Kukkonen wrote: > > On 26 May 2017 at 14:32, Emil Velikov wrote: > >> b) with a wrapper script - see [1]. > >> I think that the "export PKG_CONFIG_DIR=" is a typo (should be ..PATH > >> instead) and is not strictly required - feel free to check either way. > >> Note that the exec at the end might need to be amended to > >> /path/to/$(CHOST)-pkg-config. > > > > We don't provide a target wrapper -- I believe because it provides no > value > > at all on top of the setup we have (the pkg-config that autotools finds > has > > all the environment variables set correctly. It is essentially > > $(CHOST)-pkg-config already). > > > > If I've missed something, I'd be happy to hear that. At the moment I > think > > pkg-config just does not help in this case. > > Thinking out loud, how about searching for a separate pkg-config file > (wayland-scanner-native.pc as a strawman) first, then falling back to > wayland-scanner.pc? The BitBake recipes could then install that > somewhere in $PKG_CONFIG_PATH as a special case in the install target. > I think I just understood what you meant: That a modified "wayland-scanner-native.pc" is installed as wayland-scanner.pc but only in the sysroot case: the real target wayland-scanner.pc would not be modified. This sounds like it's worth trying, thanks. Jussi ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] configure.ac: Add --with-wayland-scanner-path
On 29 May 2017 at 15:20, Emil Velikov wrote: > > On 26 May 2017 at 14:55, Jussi Kukkonen wrote: > > On 26 May 2017 at 14:32, Emil Velikov wrote: > >> > >> On 26 May 2017 at 08:52, Jussi Kukkonen wrote: > >> > > >> > > >> > On 24 May 2017 at 16:39, Emil Velikov wrote: > >> AFAICT there are a couple of alternative solutions - have you > >> considered/tried any of them? > >> > >> a) w/o a wrapper script > >> $ export PKG_CONFIG_PATH= // you need this if using the system > >> pkgo-config. if the local/native one is used, this should be optional > >> $ export > >> PKG_CONFIG_LIBDIR=$path_to_non_system_host_pkgconfig:${SYSROOT}/usr/{lib,share}/pkgconfig > >> $ export PKG_CONFIG_SYSROOT_DIR=${SYSROOT} > > > > We're using a natively built pkg-config which sets all the pkg-config > > variables to what I believe are the correct values: the problem is that none > > of those variable is for the _native_ sysroot location so they don't help in > > this case. There is now way to say in a .pc file that this path should be > > prefixed with something like "${pc_native_sysroot_dir}" if it's defined. > > > >> b) with a wrapper script - see [1]. > >> I think that the "export PKG_CONFIG_DIR=" is a typo (should be ..PATH > >> instead) and is not strictly required - feel free to check either way. > >> Note that the exec at the end might need to be amended to > >> /path/to/$(CHOST)-pkg-config. > > > > We don't provide a target wrapper -- I believe because it provides no value > > at all on top of the setup we have (the pkg-config that autotools finds has > > all the environment variables set correctly. It is essentially > > $(CHOST)-pkg-config already). > > > > If I've missed something, I'd be happy to hear that. At the moment I think > > pkg-config just does not help in this case. > > > I'm confused a bit - did you try the above suggestions? If so can you > share which one and how it fails? I'm sorry but I do not see what I could test that could help: I mentioned that the pkg-config that gets used by PKG_CHECK_MODULES() is essentially a wrapped one: In more detail the build tool sets these variables: export PKG_CONFIG_DIR = "${STAGING_DIR_HOST}${libdir}/pkgconfig" export PKG_CONFIG_PATH = "${PKG_CONFIG_DIR}:${STAGING_DATADIR}/pkgconfig" export PKG_CONFIG_LIBDIR = "${PKG_CONFIG_DIR}" export PKG_CONFIG_SYSROOT_DIR = "${STAGING_DIR_HOST}" export PKG_CONFIG_DISABLE_UNINSTALLED = "yes" export PKG_CONFIG_SYSTEM_LIBRARY_PATH = "${base_libdir}:${libdir}" export PKG_CONFIG_SYSTEM_INCLUDE_PATH = "${includedir}" When we're building target software STAGING_DIR_HOST is the target sysroot. This setup works fine for everything except the case where pkg-config is used to locate tools that need to be run during the build (like wayland_scanner variable): we wouldn't want to use tools from the target sysroot. When e.g. target mesa is being built a native sysroot already exists and it contains a wayland-scanner we should run. but there is no way to tell pkg-config a) native sysroot path (via a env variable) b) that variable wayland_scanner specifically should be prefixed with native sysroot if present (similar to ${pc_sysrootdir} but for native sysroot) Jussi ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] etnaviv: Don't try to use the index buffer if size is zero
Hi Tomeu, Am Freitag, den 19.05.2017, 12:40 +0200 schrieb Tomeu Vizoso: > If info->index_size is zero, info->index will point to uninitialized > memory. > > Fatal signal 11 (SIGSEGV), code 2, fault addr 0xab5d07a3 in tid 20456 > (surfaceflinger) > > Signed-off-by: Tomeu Vizoso > Cc: etna...@lists.freedesktop.org > Cc: Marek Olšák > Fixes: 330d0607ed60 ("gallium: remove pipe_index_buffer and set_index_buffer") > --- > src/gallium/drivers/etnaviv/etnaviv_context.c | 36 > +++ > 1 file changed, 20 insertions(+), 16 deletions(-) > > diff --git a/src/gallium/drivers/etnaviv/etnaviv_context.c > b/src/gallium/drivers/etnaviv/etnaviv_context.c > index 306cb6fc498d..dcda4088bfd5 100644 > --- a/src/gallium/drivers/etnaviv/etnaviv_context.c > +++ b/src/gallium/drivers/etnaviv/etnaviv_context.c > @@ -178,24 +178,28 @@ etna_draw_vbo(struct pipe_context *pctx, const struct > pipe_draw_info *info) > > /* Upload a user index buffer. */ > unsigned index_offset = 0; > - struct pipe_resource *indexbuf = info->has_user_indices ? NULL : > info->index.resource; > - if (info->index_size && info->has_user_indices && > - !util_upload_index_buffer(pctx, info, &indexbuf, &index_offset)) { > - BUG("Index buffer upload failed."); > - return; > - } > + struct pipe_resource *indexbuf = NULL; > > - if (info->index_size && indexbuf) { > - ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR.bo = > etna_resource(indexbuf)->bo; > - ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR.offset = index_offset; > - ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR.flags = ETNA_RELOC_READ; > - ctx->index_buffer.FE_INDEX_STREAM_CONTROL = > translate_index_size(info->index_size); > - ctx->dirty |= ETNA_DIRTY_INDEX_BUFFER; > - } > + if (info->index_size) { > + indexbuf = info->has_user_indices ? NULL : info->index.resource; > + if (info->has_user_indices && > + !util_upload_index_buffer(pctx, info, &indexbuf, &index_offset)) { > + BUG("Index buffer upload failed."); > + return; > + } > > - if (info->index_size && !ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR.bo) { > - BUG("Unsupported or no index buffer"); > - return; > + if (indexbuf) { This can be simplified: If this is a indexed draw (index_size != 0) then we will have a indexbuf at that point in the code, as its either provided or constructed via the util_upload_index_buffer() call. Mind if squash this simplification into your patch while pushing? Regards, Lucas > + ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR.bo = > etna_resource(indexbuf)->bo; > + ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR.offset = index_offset; > + ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR.flags = ETNA_RELOC_READ; > + ctx->index_buffer.FE_INDEX_STREAM_CONTROL = > translate_index_size(info->index_size); > + ctx->dirty |= ETNA_DIRTY_INDEX_BUFFER; > + } > + > + if (!ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR.bo) { > + BUG("Unsupported or no index buffer"); > + return; > + } > } > > struct etna_shader_key key = {}; ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] svga: fix git_sha1.h include path in Android.mk
On 26 May 2017 at 16:15, Mauro Rossi wrote: > Fixes the following building error: > > external/mesa/src/gallium/drivers/svga/svga_screen.c:26:10: > fatal error: 'git_sha1.h' file not found > ^ > 1 error generated. Mauro please add Fixes: 1ce3a2723f9 ("svga: Add the ability to log messages to vmware.log on the host.") > --- > src/gallium/drivers/svga/Android.mk | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/src/gallium/drivers/svga/Android.mk > b/src/gallium/drivers/svga/Android.mk > index c50743d509..d19bd59bfe 100644 > --- a/src/gallium/drivers/svga/Android.mk > +++ b/src/gallium/drivers/svga/Android.mk > @@ -30,7 +30,9 @@ include $(CLEAR_VARS) > > LOCAL_SRC_FILES := $(C_SOURCES) > > -LOCAL_C_INCLUDES := $(LOCAL_PATH)/include > +LOCAL_C_INCLUDES := \ > + $(LOCAL_PATH)/include \ > + $(call > generated-sources-dir-for,STATIC_LIBRARIES,libmesa_dricore,,)/main > Haven't looked too closely on the discussion, so pardon if it's mentioned already. Have you considered doing a "dummy" library analogous to libmesa_genxml, This one one doesn't need to preemptively build libmesa_dricore. -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] configure.ac: Add --with-wayland-scanner-path
On 26 May 2017 at 14:55, Jussi Kukkonen wrote: > On 26 May 2017 at 14:32, Emil Velikov wrote: >> >> On 26 May 2017 at 08:52, Jussi Kukkonen wrote: >> > >> > >> > On 24 May 2017 at 16:39, Emil Velikov wrote: >> >> >> >> Hi Jussi, >> >> >> >> On 23 May 2017 at 09:13, Jussi Kukkonen >> >> wrote: >> >> > Modify wayland-scanner lookup: Use the path given by pkg-config >> >> > but offer an option to override the path with >> >> > "--with-wayland-scanner-path=PATH". The latter is useful for >> >> > cross-compile situations. >> >> > >> >> > AC_PATH_PROG is no longer used (if the scanner is installed it should >> >> > get found by pkg-config). AC_SUBST is added so the output variable is >> >> > created when only the configure option is used. >> >> > --- >> >> > >> >> > My goal is to standardize wayland-scanner usage in a way that does >> >> > not >> >> > require patching when cross-compiling in Yocto (the detailed issue is >> >> > that in Yocto pkg-config will return a "wayland_scanner" variable but >> >> > that will contain a _target path_ when we would like to use a native >> >> > sysroot path instead). >> >> > >> >> > I've sent a similar patch to weston and intend to fix other projects >> >> > if these two patches are well received. >> >> > >> >> I might have misread something, but on a quick look the patch does not >> >> look quite right. Stepping aside for a moment, >> >> >> >> Can you explain clearly what's happening/wrong in the whole scenario? >> >> - Yocto does has A stage where it does X. >> >> - Then it proceeds to B... at which point $file >> >> foo/wayland-scanner.pc gets picked >> >> - That results in an error due to variable containing $bar, due to >> >> the $step above >> > >> > Hi Emil, >> > >> > I'm hoping this is a coherent enough explanation of the issue... >> > >> > When yocto cross-compiles mesa for target, we already have >> > a) All dependency headers and libraries in a target sysroot >> > b) Native wayland-scanner in a native sysroot >> > >> > The problem as I see it is that there is no way to express the _native_ >> > sysroot in pkg-config: PKG_CONFIG_SYSROOT_DIR and pc_sysroot_dir >> > logically >> > refer to the target sysroot. So when mesa configure does "pkg-config >> > --variable=wayland_scanner wayland-scanner" it gets a reasonable >> > "/usr/bin/wayland-scanner" response: this would be the correct path to >> > wayland-scanner on _target_. In the best case this fails during compile >> > when >> > the scanner isn't found but unfortunately /usr/bin/wayland-scanner is >> > often >> > also a valid path to _host_ wayland-scanner which might be from a >> > completely >> > different and unrelated wayland version... >> > >> Barring an important s/pkg-config/$PKG_CONFIG/ I think I agree here. >> >> AFAICT there are a couple of alternative solutions - have you >> considered/tried any of them? >> >> a) w/o a wrapper script >> $ export PKG_CONFIG_PATH= // you need this if using the system >> pkgo-config. if the local/native one is used, this should be optional >> $ export >> PKG_CONFIG_LIBDIR=$path_to_non_system_host_pkgconfig:${SYSROOT}/usr/{lib,share}/pkgconfig >> $ export PKG_CONFIG_SYSROOT_DIR=${SYSROOT} > > We're using a natively built pkg-config which sets all the pkg-config > variables to what I believe are the correct values: the problem is that none > of those variable is for the _native_ sysroot location so they don't help in > this case. There is now way to say in a .pc file that this path should be > prefixed with something like "${pc_native_sysroot_dir}" if it's defined. > >> b) with a wrapper script - see [1]. >> I think that the "export PKG_CONFIG_DIR=" is a typo (should be ..PATH >> instead) and is not strictly required - feel free to check either way. >> Note that the exec at the end might need to be amended to >> /path/to/$(CHOST)-pkg-config. > > We don't provide a target wrapper -- I believe because it provides no value > at all on top of the setup we have (the pkg-config that autotools finds has > all the environment variables set correctly. It is essentially > $(CHOST)-pkg-config already). > > If I've missed something, I'd be happy to hear that. At the moment I think > pkg-config just does not help in this case. > I'm confused a bit - did you try the above suggestions? If so can you share which one and how it fails? I do see your interest in having a native sysroot dir, although I am a bit suspicious that you need it in the first place. -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] mesa: add new 'name' parameter to get_texobj_by_name()
To display better function names when INVALID_OPERATION is returned. Requested by Timothy. Signed-off-by: Samuel Pitoiset --- src/mesa/main/texparam.c | 25 - 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/mesa/main/texparam.c b/src/mesa/main/texparam.c index 0156bbd275..5cdcb79ec3 100644 --- a/src/mesa/main/texparam.c +++ b/src/mesa/main/texparam.c @@ -153,7 +153,7 @@ get_texobj_by_target(struct gl_context *ctx, GLenum target, GLboolean get) * Only the glGetTexLevelParameter() functions accept proxy targets. */ static struct gl_texture_object * -get_texobj_by_name(struct gl_context *ctx, GLuint texture, GLboolean get) +get_texobj_by_name(struct gl_context *ctx, GLuint texture, const char *name) { struct gl_texture_object *texObj; @@ -179,8 +179,7 @@ get_texobj_by_name(struct gl_context *ctx, GLuint texture, GLboolean get) case GL_TEXTURE_RECTANGLE: return texObj; default: - _mesa_error(ctx, GL_INVALID_ENUM, - "gl%sTextureParameter(target)", get ? "Get" : ""); + _mesa_error(ctx, GL_INVALID_ENUM, "%s(target)", name); return NULL; } @@ -,7 +1110,7 @@ _mesa_TextureParameterfv(GLuint texture, GLenum pname, const GLfloat *params) struct gl_texture_object *texObj; GET_CURRENT_CONTEXT(ctx); - texObj = get_texobj_by_name(ctx, texture, GL_FALSE); + texObj = get_texobj_by_name(ctx, texture, "glTextureParameterfv"); if (!texObj) { /* User passed a non-generated name. */ _mesa_error(ctx, GL_INVALID_OPERATION, "glTextureParameterfv(texture)"); @@ -1127,7 +1126,7 @@ _mesa_TextureParameterf(GLuint texture, GLenum pname, GLfloat param) struct gl_texture_object *texObj; GET_CURRENT_CONTEXT(ctx); - texObj = get_texobj_by_name(ctx, texture, GL_FALSE); + texObj = get_texobj_by_name(ctx, texture, "glTextureParameterf"); if (!texObj) { /* User passed a non-generated name. */ _mesa_error(ctx, GL_INVALID_OPERATION, "glTextureParameterf(texture)"); @@ -1143,7 +1142,7 @@ _mesa_TextureParameteri(GLuint texture, GLenum pname, GLint param) struct gl_texture_object *texObj; GET_CURRENT_CONTEXT(ctx); - texObj = get_texobj_by_name(ctx, texture, GL_FALSE); + texObj = get_texobj_by_name(ctx, texture, "glTextureParameteri"); if (!texObj) { /* User passed a non-generated name. */ _mesa_error(ctx, GL_INVALID_OPERATION, "glTextureParameteri(texture)"); @@ -1160,7 +1159,7 @@ _mesa_TextureParameteriv(GLuint texture, GLenum pname, struct gl_texture_object *texObj; GET_CURRENT_CONTEXT(ctx); - texObj = get_texobj_by_name(ctx, texture, GL_FALSE); + texObj = get_texobj_by_name(ctx, texture, "glTextureParameteriv"); if (!texObj) { /* User passed a non-generated name. */ _mesa_error(ctx, GL_INVALID_OPERATION, "glTextureParameteriv(texture)"); @@ -1177,7 +1176,7 @@ _mesa_TextureParameterIiv(GLuint texture, GLenum pname, const GLint *params) struct gl_texture_object *texObj; GET_CURRENT_CONTEXT(ctx); - texObj = get_texobj_by_name(ctx, texture, GL_FALSE); + texObj = get_texobj_by_name(ctx, texture, "glTextureParameterIiv"); if (!texObj) { /* User passed a non-generated name. */ _mesa_error(ctx, GL_INVALID_OPERATION, @@ -1194,7 +1193,7 @@ _mesa_TextureParameterIuiv(GLuint texture, GLenum pname, const GLuint *params) struct gl_texture_object *texObj; GET_CURRENT_CONTEXT(ctx); - texObj = get_texobj_by_name(ctx, texture, GL_FALSE); + texObj = get_texobj_by_name(ctx, texture, "glTextureParameterIuiv"); if (!texObj) { /* User passed a non-generated name. */ _mesa_error(ctx, GL_INVALID_OPERATION, @@ -2337,7 +2336,7 @@ _mesa_GetTextureParameterfv(GLuint texture, GLenum pname, GLfloat *params) struct gl_texture_object *obj; GET_CURRENT_CONTEXT(ctx); - obj = get_texobj_by_name(ctx, texture, GL_TRUE); + obj = get_texobj_by_name(ctx, texture, "glGetTextureParameterfv"); if (!obj) { /* User passed a non-generated name. */ _mesa_error(ctx, GL_INVALID_OPERATION, @@ -2354,7 +2353,7 @@ _mesa_GetTextureParameteriv(GLuint texture, GLenum pname, GLint *params) struct gl_texture_object *obj; GET_CURRENT_CONTEXT(ctx); - obj = get_texobj_by_name(ctx, texture, GL_TRUE); + obj = get_texobj_by_name(ctx, texture, "glGetTextureParameteriv"); if (!obj) { /* User passed a non-generated name. */ _mesa_error(ctx, GL_INVALID_OPERATION, @@ -2371,7 +2370,7 @@ _mesa_GetTextureParameterIiv(GLuint texture, GLenum pname, GLint *params) struct gl_texture_object *texObj; GET_CURRENT_CONTEXT(ctx); - texObj = get_texobj_by_name(ctx, texture, GL_TRUE); + texObj = get_texobj_by_name(ctx, texture, "glGetTextureParameterIiv"); if (!texObj) { /* User passed a non-generated name. */ _mesa_error(ctx, GL_INVALID_OPERATION, @@ -2389,7 +2388,7 @@ _mesa_GetTextureParameterIuiv(GLuint texture, GLenum
[Mesa-dev] [PATCH 2/2] mesa: throw an INVALID_OPERATION error in get_texobj_by_name()
Signed-off-by: Samuel Pitoiset --- src/mesa/main/texparam.c | 61 +--- 1 file changed, 11 insertions(+), 50 deletions(-) diff --git a/src/mesa/main/texparam.c b/src/mesa/main/texparam.c index 5cdcb79ec3..c75adc6417 100644 --- a/src/mesa/main/texparam.c +++ b/src/mesa/main/texparam.c @@ -159,10 +159,7 @@ get_texobj_by_name(struct gl_context *ctx, GLuint texture, const char *name) texObj = _mesa_lookup_texture(ctx, texture); if (!texObj) { - /* - * User passed a non-generated name. - * Throw the error in the caller. - */ + _mesa_error(ctx, GL_INVALID_OPERATION, "%s(texture)", name); return NULL; } @@ -,11 +1108,8 @@ _mesa_TextureParameterfv(GLuint texture, GLenum pname, const GLfloat *params) GET_CURRENT_CONTEXT(ctx); texObj = get_texobj_by_name(ctx, texture, "glTextureParameterfv"); - if (!texObj) { - /* User passed a non-generated name. */ - _mesa_error(ctx, GL_INVALID_OPERATION, "glTextureParameterfv(texture)"); + if (!texObj) return; - } _mesa_texture_parameterfv(ctx, texObj, pname, params, true); } @@ -1127,11 +1121,8 @@ _mesa_TextureParameterf(GLuint texture, GLenum pname, GLfloat param) GET_CURRENT_CONTEXT(ctx); texObj = get_texobj_by_name(ctx, texture, "glTextureParameterf"); - if (!texObj) { - /* User passed a non-generated name. */ - _mesa_error(ctx, GL_INVALID_OPERATION, "glTextureParameterf(texture)"); + if (!texObj) return; - } _mesa_texture_parameterf(ctx, texObj, pname, param, true); } @@ -1143,11 +1134,8 @@ _mesa_TextureParameteri(GLuint texture, GLenum pname, GLint param) GET_CURRENT_CONTEXT(ctx); texObj = get_texobj_by_name(ctx, texture, "glTextureParameteri"); - if (!texObj) { - /* User passed a non-generated name. */ - _mesa_error(ctx, GL_INVALID_OPERATION, "glTextureParameteri(texture)"); + if (!texObj) return; - } _mesa_texture_parameteri(ctx, texObj, pname, param, true); } @@ -1160,11 +1148,8 @@ _mesa_TextureParameteriv(GLuint texture, GLenum pname, GET_CURRENT_CONTEXT(ctx); texObj = get_texobj_by_name(ctx, texture, "glTextureParameteriv"); - if (!texObj) { - /* User passed a non-generated name. */ - _mesa_error(ctx, GL_INVALID_OPERATION, "glTextureParameteriv(texture)"); + if (!texObj) return; - } _mesa_texture_parameteriv(ctx, texObj, pname, params, true); } @@ -1177,12 +1162,8 @@ _mesa_TextureParameterIiv(GLuint texture, GLenum pname, const GLint *params) GET_CURRENT_CONTEXT(ctx); texObj = get_texobj_by_name(ctx, texture, "glTextureParameterIiv"); - if (!texObj) { - /* User passed a non-generated name. */ - _mesa_error(ctx, GL_INVALID_OPERATION, - "glTextureParameterIiv(texture)"); + if (!texObj) return; - } _mesa_texture_parameterIiv(ctx, texObj, pname, params, true); } @@ -1194,12 +1175,8 @@ _mesa_TextureParameterIuiv(GLuint texture, GLenum pname, const GLuint *params) GET_CURRENT_CONTEXT(ctx); texObj = get_texobj_by_name(ctx, texture, "glTextureParameterIuiv"); - if (!texObj) { - /* User passed a non-generated name. */ - _mesa_error(ctx, GL_INVALID_OPERATION, - "glTextureParameterIuiv(texture)"); + if (!texObj) return; - } _mesa_texture_parameterIuiv(ctx, texObj, pname, params, true); } @@ -2337,12 +2314,8 @@ _mesa_GetTextureParameterfv(GLuint texture, GLenum pname, GLfloat *params) GET_CURRENT_CONTEXT(ctx); obj = get_texobj_by_name(ctx, texture, "glGetTextureParameterfv"); - if (!obj) { - /* User passed a non-generated name. */ - _mesa_error(ctx, GL_INVALID_OPERATION, - "glGetTextureParameterfv(texture)"); + if (!obj) return; - } get_tex_parameterfv(ctx, obj, pname, params, true); } @@ -2354,12 +2327,8 @@ _mesa_GetTextureParameteriv(GLuint texture, GLenum pname, GLint *params) GET_CURRENT_CONTEXT(ctx); obj = get_texobj_by_name(ctx, texture, "glGetTextureParameteriv"); - if (!obj) { - /* User passed a non-generated name. */ - _mesa_error(ctx, GL_INVALID_OPERATION, - "glGetTextureParameteriv(texture)"); + if (!obj) return; - } get_tex_parameteriv(ctx, obj, pname, params, true); } @@ -2371,12 +2340,8 @@ _mesa_GetTextureParameterIiv(GLuint texture, GLenum pname, GLint *params) GET_CURRENT_CONTEXT(ctx); texObj = get_texobj_by_name(ctx, texture, "glGetTextureParameterIiv"); - if (!texObj) { - /* User passed a non-generated name. */ - _mesa_error(ctx, GL_INVALID_OPERATION, - "glGetTextureParameterIiv(texture)"); + if (!texObj) return; - } get_tex_parameterIiv(ctx, texObj, pname, params, true); } @@ -2389,12 +2354,8 @@ _mesa_GetTextureParameterIuiv(GLuint texture, GLenum pname, GLuint *params) GET_CURRENT_CONTEXT(ctx); texObj = ge
Re: [Mesa-dev] [PATCH] configure.ac: Add --with-wayland-scanner-path
On 26 May 2017 at 17:08, Daniel Stone wrote: > Hi Jussi, > > On 26 May 2017 at 14:55, Jussi Kukkonen wrote: > > On 26 May 2017 at 14:32, Emil Velikov wrote: > >> b) with a wrapper script - see [1]. > >> I think that the "export PKG_CONFIG_DIR=" is a typo (should be ..PATH > >> instead) and is not strictly required - feel free to check either way. > >> Note that the exec at the end might need to be amended to > >> /path/to/$(CHOST)-pkg-config. > > > > We don't provide a target wrapper -- I believe because it provides no > value > > at all on top of the setup we have (the pkg-config that autotools finds > has > > all the environment variables set correctly. It is essentially > > $(CHOST)-pkg-config already). > > > > If I've missed something, I'd be happy to hear that. At the moment I > think > > pkg-config just does not help in this case. > > Thinking out loud, how about searching for a separate pkg-config file > (wayland-scanner-native.pc as a strawman) first, then falling back to > wayland-scanner.pc? The BitBake recipes could then install that > somewhere in $PKG_CONFIG_PATH as a special case in the install target. > Maybe I don't understand but that doesn't sound any easier: wayland would still need to be modified (to install an additional pc-file) and all the wayland-scanner users would need to be modified to look for that new pc file (unless you meant pkg-config would be modified to always look for "-native.pc"). I don't really mind if our case requires a configure option in every wayland-scanner using project, I'd just like to avoid patching build systems. Jussi ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 99116] Wine program showing only a blackscreen when using mesa
https://bugs.freedesktop.org/show_bug.cgi?id=99116 --- Comment #13 from Józef Kucia --- We've got a piglit test for this bug: https://cgit.freedesktop.org/piglit/commit/?id=eae8e3d1f2d33729fbad9a9433c1c91fd29dae2b -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v5 0/4] Disable glthread if libX11 isn't thread-safe
Hello Mesa developers, Following the discussion from https://lists.freedesktop.org/archives/mesa-dev/2017-April/153137.html A check was added to ensure that X11 display can be locked. It should be enough to ensure thread safety between X11 and glthread. I also did the check on DRI3 as I'm not 100% sure that it is really thread safe. v2: based on Nicolai/Matt reviews Add a check on DRI extension version Use C comments :) v3: based on Emil reviews Split the initial first patch into 3 sub patches dri extension / glx / egl Improve error message Improve code readability Just include libX11 on EGL protected by ifdef v4: based on Eric feedback, I marked DRI3 as always thread safe v5: Fix the null pointer check on patch 4. I added Daniel comment on patch 3 but I'm not sure I got it right. Thanks you for all the review comments. Best regards, Gregory Hainaut (4): dri: Extend __DRIbackgroundCallableExtensionRec to include a callback that checks for thread safety glx: implement __DRIbackgroundCallableExtension.isThreadSafe egl: implement __DRIbackgroundCallableExtension.isThreadSafe glthread/gallium: require safe_glthread to start glthread include/GL/internal/dri_interface.h | 13 +++ src/egl/drivers/dri2/egl_dri2.c | 34 +++- src/gallium/state_trackers/dri/dri_context.c | 21 + src/glx/dri2_glx.c | 15 +++- src/glx/dri3_glx.c | 12 +- 5 files changed, 88 insertions(+), 7 deletions(-) -- 2.11.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v5 4/4] glthread/gallium: require safe_glthread to start glthread
Print an error message for the user if the requirement isn't met, or we're not thread safe. v2: based on Nicolai feedbacks Check the DRI extension version v3: based on Emil feedbacks improve commit and error messages. use backgroundCallable variable to improve readability v5: based on Emil feedbacks Properly check the function pointer Signed-off-by: Gregory Hainaut --- src/gallium/state_trackers/dri/dri_context.c | 21 + 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/gallium/state_trackers/dri/dri_context.c b/src/gallium/state_trackers/dri/dri_context.c index 92d79849c4..ec555e44d7 100644 --- a/src/gallium/state_trackers/dri/dri_context.c +++ b/src/gallium/state_trackers/dri/dri_context.c @@ -51,20 +51,22 @@ dri_create_context(gl_api api, const struct gl_config * visual, { __DRIscreen *sPriv = cPriv->driScreenPriv; struct dri_screen *screen = dri_screen(sPriv); struct st_api *stapi = screen->st_api; struct dri_context *ctx = NULL; struct st_context_iface *st_share = NULL; struct st_context_attribs attribs; enum st_context_error ctx_err = 0; unsigned allowed_flags = __DRI_CTX_FLAG_DEBUG | __DRI_CTX_FLAG_FORWARD_COMPATIBLE; + const __DRIbackgroundCallableExtension *backgroundCallable = + screen->sPriv->dri2.backgroundCallable; if (screen->has_reset_status_query) allowed_flags |= __DRI_CTX_FLAG_ROBUST_BUFFER_ACCESS; if (flags & ~allowed_flags) { *error = __DRI_CTX_ERROR_UNKNOWN_FLAG; goto fail; } if (!screen->has_reset_status_query && notify_reset) { @@ -151,24 +153,35 @@ dri_create_context(gl_api api, const struct gl_config * visual, ctx->st->st_manager_private = (void *) ctx; ctx->stapi = stapi; if (ctx->st->cso_context) { ctx->pp = pp_init(ctx->st->pipe, screen->pp_enabled, ctx->st->cso_context); ctx->hud = hud_create(ctx->st->pipe, ctx->st->cso_context); } /* Do this last. */ if (ctx->st->start_thread && - /* the driver loader must implement this */ - screen->sPriv->dri2.backgroundCallable && - driQueryOptionb(&screen->optionCache, "mesa_glthread")) - ctx->st->start_thread(ctx->st); + driQueryOptionb(&screen->optionCache, "mesa_glthread")) { + + if (backgroundCallable && backgroundCallable->base.version >= 2 && +backgroundCallable->isThreadSafe) { + + if (backgroundCallable->isThreadSafe(cPriv->loaderPrivate)) +ctx->st->start_thread(ctx->st); + else +fprintf(stderr, "dri_create_context: glthread isn't thread safe " + "- missing call XInitThreads\n"); + } else { + fprintf(stderr, "dri_create_context: requested glthread but driver " + "is missing backgroundCallable V2 extension\n"); + } + } *error = __DRI_CTX_ERROR_SUCCESS; return GL_TRUE; fail: if (ctx && ctx->st) ctx->st->destroy(ctx->st); free(ctx); return GL_FALSE; -- 2.11.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v5 3/4] egl: implement __DRIbackgroundCallableExtension.isThreadSafe
v2: bump version v3: Add code comment s/IsGlThread/IsThread/ (and variation) Include X11/Xlibint.h protected by ifdef v5: based on Daniel feedback Move non X11 code outside of X11 define Always return true for Wayland Signed-off-by: Gregory Hainaut --- src/egl/drivers/dri2/egl_dri2.c | 34 +- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c index 0be7132ac5..8891771e3f 100644 --- a/src/egl/drivers/dri2/egl_dri2.c +++ b/src/egl/drivers/dri2/egl_dri2.c @@ -48,20 +48,24 @@ #include #include "GL/mesa_glinterop.h" #include #include #ifdef HAVE_WAYLAND_PLATFORM #include "wayland-drm.h" #include "wayland-drm-client-protocol.h" #endif +#ifdef HAVE_X11_PLATFORM +#include "X11/Xlibint.h" +#endif + #include "egl_dri2.h" #include "loader/loader.h" #include "util/u_atomic.h" /* The kernel header drm_fourcc.h defines the DRM formats below. We duplicate * some of the definitions here so that building Mesa won't bleeding-edge * kernel headers. */ #ifndef DRM_FORMAT_R8 #define DRM_FORMAT_R8fourcc_code('R', '8', ' ', ' ') /* [7:0] R */ @@ -85,24 +89,52 @@ static void dri_set_background_context(void *loaderPrivate) { _EGLContext *ctx = _eglGetCurrentContext(); _EGLThreadInfo *t = _eglGetCurrentThread(); _eglBindContextToThread(ctx, t); } +static GLboolean +dri_is_thread_safe(void *loaderPrivate) +{ + struct dri2_egl_surface *dri2_surf = loaderPrivate; + _EGLDisplay *display = dri2_surf->base.Resource.Display; + +#ifdef HAVE_X11_PLATFORM + Display *xdpy = (Display*)display->PlatformDisplay; + + /* Check Xlib is running in thread safe mode when running on EGL/X11-xlib +* platform +* +* 'lock_fns' is the XLockDisplay function pointer of the X11 display 'dpy'. +* It wll be NULL if XInitThreads wasn't called. +*/ + if (display->Platform == _EGL_PLATFORM_X11 && xdpy && !xdpy->lock_fns) + return false; +#endif + +#ifdef HAVE_WAYLAND_PLATFORM + if (display->Platform == _EGL_PLATFORM_WAYLAND) + return true; +#endif + + return true; +} + const __DRIbackgroundCallableExtension background_callable_extension = { - .base = { __DRI_BACKGROUND_CALLABLE, 1 }, + .base = { __DRI_BACKGROUND_CALLABLE, 2 }, .setBackgroundContext = dri_set_background_context, + .isThreadSafe = dri_is_thread_safe, }; const __DRIuseInvalidateExtension use_invalidate = { .base = { __DRI_USE_INVALIDATE, 1 } }; static const EGLint dri2_to_egl_attribute_map[__DRI_ATTRIB_MAX] = { [__DRI_ATTRIB_BUFFER_SIZE ] = EGL_BUFFER_SIZE, [__DRI_ATTRIB_LEVEL] = EGL_LEVEL, [__DRI_ATTRIB_RED_SIZE] = EGL_RED_SIZE, -- 2.11.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v5 2/4] glx: implement __DRIbackgroundCallableExtension.isThreadSafe
v2: bump version v3: Add code comment s/IsGlThread/IsThread/ (and variation) v4: DRI3 doesn't hit X through GL call so it is always safe Signed-off-by: Gregory Hainaut --- src/glx/dri2_glx.c | 15 ++- src/glx/dri3_glx.c | 12 +++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c index 145f44d6e8..4f163688f2 100644 --- a/src/glx/dri2_glx.c +++ b/src/glx/dri2_glx.c @@ -946,20 +946,32 @@ dri2GetSwapInterval(__GLXDRIdrawable *pdraw) return priv->swap_interval; } static void driSetBackgroundContext(void *loaderPrivate) { struct dri2_context *pcp = (struct dri2_context *) loaderPrivate; __glXSetCurrentContext(&pcp->base); } +static GLboolean +driIsThreadSafe(void *loaderPrivate) +{ + struct dri2_context *pcp = (struct dri2_context *) loaderPrivate; + /* Check Xlib is running in thread safe mode +* +* 'lock_fns' is the XLockDisplay function pointer of the X11 display 'dpy'. +* It wll be NULL if XInitThreads wasn't called. +*/ + return pcp->base.psc->dpy->lock_fns != NULL; +} + static const __DRIdri2LoaderExtension dri2LoaderExtension = { .base = { __DRI_DRI2_LOADER, 3 }, .getBuffers = dri2GetBuffers, .flushFrontBuffer= dri2FlushFrontBuffer, .getBuffersWithFormat= dri2GetBuffersWithFormat, }; static const __DRIdri2LoaderExtension dri2LoaderExtension_old = { .base = { __DRI_DRI2_LOADER, 3 }, @@ -967,23 +979,24 @@ static const __DRIdri2LoaderExtension dri2LoaderExtension_old = { .getBuffers = dri2GetBuffers, .flushFrontBuffer= dri2FlushFrontBuffer, .getBuffersWithFormat= NULL, }; static const __DRIuseInvalidateExtension dri2UseInvalidate = { .base = { __DRI_USE_INVALIDATE, 1 } }; static const __DRIbackgroundCallableExtension driBackgroundCallable = { - .base = { __DRI_BACKGROUND_CALLABLE, 1 }, + .base = { __DRI_BACKGROUND_CALLABLE, 2 }, .setBackgroundContext= driSetBackgroundContext, + .isThreadSafe= driIsThreadSafe, }; _X_HIDDEN void dri2InvalidateBuffers(Display *dpy, XID drawable) { __GLXDRIdrawable *pdraw = dri2GetGlxDrawableFromXDrawableId(dpy, drawable); struct dri2_screen *psc; struct dri2_drawable *pdp = (struct dri2_drawable *) pdraw; diff --git a/src/glx/dri3_glx.c b/src/glx/dri3_glx.c index e1dc5aa4a8..d07968e3c5 100644 --- a/src/glx/dri3_glx.c +++ b/src/glx/dri3_glx.c @@ -496,37 +496,47 @@ dri3_flush_front_buffer(__DRIdrawable *driDrawable, void *loaderPrivate) loader_dri3_wait_gl(draw); } static void dri_set_background_context(void *loaderPrivate) { struct dri3_context *pcp = (struct dri3_context *)loaderPrivate; __glXSetCurrentContext(&pcp->base); } +static GLboolean +dri_is_thread_safe(void *loaderPrivate) +{ + /* Unlike DRI2, DRI3 doesn't call GetBuffers/GetBuffersWithFormat +* during draw so we're safe here. +*/ + return true; +} + /* The image loader extension record for DRI3 */ static const __DRIimageLoaderExtension imageLoaderExtension = { .base = { __DRI_IMAGE_LOADER, 1 }, .getBuffers = loader_dri3_get_buffers, .flushFrontBuffer= dri3_flush_front_buffer, }; const __DRIuseInvalidateExtension dri3UseInvalidate = { .base = { __DRI_USE_INVALIDATE, 1 } }; static const __DRIbackgroundCallableExtension driBackgroundCallable = { - .base = { __DRI_BACKGROUND_CALLABLE, 1 }, + .base = { __DRI_BACKGROUND_CALLABLE, 2 }, .setBackgroundContext = dri_set_background_context, + .isThreadSafe = dri_is_thread_safe, }; static const __DRIextension *loader_extensions[] = { &imageLoaderExtension.base, &dri3UseInvalidate.base, &driBackgroundCallable.base, NULL }; /** dri3_swap_buffers -- 2.11.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v5 1/4] dri: Extend __DRIbackgroundCallableExtensionRec to include a callback that checks for thread safety
DRI-drivers could call Xlib functions, for example to allocate a new back buffer. When glthread is enabled, the driver runs mostly on a separate thread. Therefore we need to guarantee the thread safety between libX11 calls from the applications (not aware of the extra thread) and the ones from the driver. See discussion thread: https://lists.freedesktop.org/archives/mesa-dev/2017-April/152547.html Fortunately, Xlib allows to lock display to ensure thread safety but XInitThreads must be called first by the application to initialize the lock function pointer. This patch will allow to check XInitThreads was called to allow glthread on GLX or EGL platform. Note: a tentative was done to port libX11 code to XCB but it didn't solve fully thread safety. See discussion thread: https://lists.freedesktop.org/archives/mesa-dev/2017-April/153137.html Note: Nvidia forces the driver to call XInitThreads. Quoting their manpage: "The NVIDIA OpenGL driver will automatically attempt to enable Xlib thread-safe mode if needed. However, it might not be possible in some situations, such as when the NVIDIA OpenGL driver library is dynamically loaded after Xlib has been loaded and initialized. If that is the case, threaded optimizations will stay disabled unless the application is modified to call XInitThreads() before initializing Xlib or to link directly against the NVIDIA OpenGL driver library. Alternatively, using the LD_PRELOAD environment variable to include the NVIDIA OpenGL driver library should also achieve the desired result." v2: based on Nicolai and Matt feedback Use C style comment v3: based on Emil feedback split the patch in 3 s/isGlThreadSafe/isThreadSafe/ v5: based on Marek comment Add a comment that isThreadSafe is supported by extension v2 Signed-off-by: Gregory Hainaut --- include/GL/internal/dri_interface.h | 13 + 1 file changed, 13 insertions(+) diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h index c83056aa70..ffe99499fc 100644 --- a/include/GL/internal/dri_interface.h +++ b/include/GL/internal/dri_interface.h @@ -1714,13 +1714,26 @@ struct __DRIbackgroundCallableExtensionRec { * non-background thread (i.e. a thread that has already been bound to a * context using __DRIcoreExtensionRec::bindContext()); when this happens, * the \c loaderPrivate pointer must be equal to the pointer that was * passed to the driver when the currently bound context was created. * * This call should execute quickly enough that the driver can call it with * impunity whenever a background thread starts performing drawing * operations (e.g. it should just set a thread-local variable). */ void (*setBackgroundContext)(void *loaderPrivate); + + /** +* Indicate that it is multithread safe to use glthread. For GLX/EGL +* platforms using Xlib, that involves calling XInitThreads, before +* opening an X display. +* +* Note: only supported if extension version is at least 2. +* +* \param loaderPrivate is the value that was passed to to the driver when +* the context was created. This can be used by the loader to identify +* which context any callbacks are associated with. +*/ + GLboolean (*isThreadSafe)(void *loaderPrivate); }; #endif -- 2.11.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 101211] Mesa swrast issue with visualization on BE PPC PPC64
https://bugs.freedesktop.org/show_bug.cgi?id=101211 --- Comment #4 from intermedi...@hotmail.com --- Created attachment 131566 --> https://bugs.freedesktop.org/attachment.cgi?id=131566&action=edit flipped down dosbox as you can see flipped down dosbox -- 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] [PATCH 2/9] i965/dri: Combine declaration and assignment in intelCreateBuffer
On 05/29/2017 09:55 AM, Martin Peres wrote: On 27/05/17 01:15, Chad Versace wrote: Trivial cleanup. --- src/mesa/drivers/dri/i965/intel_screen.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c index 5de6f18b84..24123e7986 100644 --- a/src/mesa/drivers/dri/i965/intel_screen.c +++ b/src/mesa/drivers/dri/i965/intel_screen.c @@ -1169,12 +1169,11 @@ intelCreateBuffer(__DRIscreen *dri_screen, mesa_format rgbFormat; unsigned num_samples = intel_quantize_num_samples(screen, mesaVis->samples); - struct gl_framebuffer *fb; if (isPixmap) return false; - fb = CALLOC_STRUCT(gl_framebuffer); + struct gl_framebuffer *fb = CALLOC_STRUCT(gl_framebuffer); if (!fb) return false; Do we compile mesa with c99 nowadays? Did I miss anything? Yes, AFAICT has been in use some years already :) // Tapani ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] drirc: set force_glsl_version for Alchemist's Awakening
Yay! My request has been a win, game is fixed now. This patch is no longer needed. On 05/24/2017 09:05 PM, Marek Olšák wrote: Reviewed-by: Marek Olšák Marek On Wed, May 24, 2017 at 1:01 AM, Samuel Pitoiset wrote: A bunch of shaders are missing a version directive. This fixes the following compilation error and allows the game to launch. 0:43(28): error: cannot initialize uniform typeColor in GLSL 1.10 (GLSL 1.20 required) Signed-off-by: Samuel Pitoiset --- src/mesa/drivers/dri/common/drirc | 4 1 file changed, 4 insertions(+) diff --git a/src/mesa/drivers/dri/common/drirc b/src/mesa/drivers/dri/common/drirc index d820462fad0..111b53646e9 100644 --- a/src/mesa/drivers/dri/common/drirc +++ b/src/mesa/drivers/dri/common/drirc @@ -151,5 +151,9 @@ TODO: document the other workarounds. + + + + -- 2.13.0 ___ 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