[Mesa-dev] [PATCH] i965/fs: Use WE_all for gl_SampleID header register munging.
This code should execute without regard to the currently executing channels. Asking for gl_SampleID inside control flow might break in strange ways. It appears to break even at the top of the program in SIMD16 mode occasionally as well. Signed-off-by: Kenneth Graunke Cc: mesa-sta...@lists.freedesktop.org --- src/mesa/drivers/dri/i965/brw_fs.cpp | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp index 53bd91b..1a5d987 100644 --- a/src/mesa/drivers/dri/i965/brw_fs.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp @@ -1305,12 +1305,16 @@ fs_visitor::emit_sampleid_setup(ir_variable *ir) * and then reading from it using vstride=1, width=4, hstride=0. * These computations hold good for 4x multisampling as well. */ - emit(BRW_OPCODE_AND, t1, - fs_reg(retype(brw_vec1_grf(0, 0), BRW_REGISTER_TYPE_UD)), - fs_reg(0xc0)); - emit(BRW_OPCODE_SHR, t1, t1, fs_reg(5)); + fs_inst *inst; + inst = emit(BRW_OPCODE_AND, t1, + fs_reg(retype(brw_vec1_grf(0, 0), BRW_REGISTER_TYPE_UD)), + fs_reg(0xc0)); + inst->force_writemask_all = true; + inst = emit(BRW_OPCODE_SHR, t1, t1, fs_reg(5)); + inst->force_writemask_all = true; /* This works for both SIMD8 and SIMD16 */ - emit(MOV(t2, brw_imm_v(0x3210))); + inst = emit(MOV(t2, brw_imm_v(0x3210))); + inst->force_writemask_all = true; /* This special instruction takes care of setting vstride=1, * width=4, hstride=0 of t2 during an ADD instruction. */ -- 2.0.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] util: Add util_memcpy_cpu_to_le()
First of all, I think this should be two patches, one which adds the helper and one which makes radeonsi use it. On 16.07.2014 01:19, Tom Stellard wrote: > diff --git a/src/gallium/auxiliary/util/u_math.h > b/src/gallium/auxiliary/util/u_math.h > index b9ed197..cd3cf04 100644 > --- a/src/gallium/auxiliary/util/u_math.h > +++ b/src/gallium/auxiliary/util/u_math.h > @@ -812,6 +812,28 @@ util_bswap16(uint16_t n) >(n << 8); > } > > +static INLINE void* > +util_memcpy_cpu_to_le(void *dest, void *src, size_t n) This should indicate the size of the words being byte-swapped, e.g. util_memcpy_cpu_to_le32(). > +{ > +#ifdef PIPE_ARCH_BIG_ENDIAN > + size_t i, e; > + for (i = 0, e = n % 8; i < e; i++) { > + char *d = (char*)dest; > + char *s = (char*)src; > + d[i] = s[e - i - 1]; > + } > + dest += i; > + n -= i; This doesn't make sense. It's a caller error if n is not a multiple of the size of the words being being byte-swapped, so maybe just: assert((n % 3) == 0); > + for (i = 0, e = n / 8; i < e; i++) { > + uint64_t *d = (uint64_t*)dest; > + uint64_t *s = (uint64_t*)src; > + d[i] = util_bswap64(s[e - i - 1]); > + } This looks wrong as well. I think it should be: for (i = 0; i < n; i++) { uint32_t *d = (uint32_t*)dest; uint32_t *s = (uint32_t*)src; d[i] = util_bswap32(s[i]); } -- Earthling Michel Dänzer| http://www.amd.com Libre software enthusiast |Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 80615] Files in bellagio directory [omx tracker] don't respect installation folder
https://bugs.freedesktop.org/show_bug.cgi?id=80615 Alexandre Demers changed: What|Removed |Added CC||deathsim...@vodafone.de, ||emil.l.veli...@gmail.com -- You are receiving this mail because: You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 02/26] glsl: Rebuild the symbol table without unreachable symbols
On Monday, July 14, 2014 03:48:34 PM Ian Romanick wrote: > From: Ian Romanick > > Previously we had to keep unreachable global symbols in the symbol table > because the symbol table is used during linking. Having the symbol > table retain pointers to freed memory... what could possibly go wrong? > At the same time, this meant that we kept live references to tons of > memory that was no longer needed. > > New strategy: destroy the old symbol table, and make a new one from the > reachable symbols. > > Valgrind massif results for a trimmed apitrace of dota2: > > ntime(i) total(B) useful-heap(B) extra- heap(B)stacks(B) > Before (32-bit): 59 40,642,425,451 76,337,968 69,720,886 6,617,0820 > After (32-bit): 46 40,661,487,174 75,116,800 68,854,065 6,262,7350 > > Before (64-bit): 79 37,179,441,771 106,986,512 98,112,095 8,874,4170 > After (64-bit): 64 37,200,329,700 104,872,672 96,514,546 8,358,1260 > > A real savings of 846KiB on 32-bit and 1.5MiB on 64-bit. > > Signed-off-by: Ian Romanick > Cc: Kenneth Graunke > --- > src/glsl/glsl_parser_extras.cpp | 37 - > 1 file changed, 36 insertions(+), 1 deletion(-) > > diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp > index b327c2b..2962407 100644 > --- a/src/glsl/glsl_parser_extras.cpp > +++ b/src/glsl/glsl_parser_extras.cpp > @@ -1485,7 +1485,7 @@ _mesa_glsl_compile_shader(struct gl_context *ctx, struct gl_shader *shader, > if (shader->InfoLog) >ralloc_free(shader->InfoLog); > > - shader->symbols = state->symbols; > + shader->symbols = new(shader->ir) glsl_symbol_table; > shader->CompileStatus = !state->error; > shader->InfoLog = state->info_log; > shader->Version = state->language_version; > @@ -1498,6 +1498,41 @@ _mesa_glsl_compile_shader(struct gl_context *ctx, struct gl_shader *shader, > /* Retain any live IR, but trash the rest. */ > reparent_ir(shader->ir, shader->ir); > > + /* Destroy the symbol table. Create a new symbol table that contains only > +* the variables and functions that still exist in the IR. The symbol > +* table will be used later during linking. > +* > +* There must NOT be any freed objects still referenced by the symbol > +* table. That could cause the linker to dereference freed memory. > +* > +* We don't have to worry about types or interface-types here because those > +* are fly-weights that are looked up by glsl_type. > +*/ > + foreach_in_list (ir_instruction, ir, shader->ir) { > + switch (ir->ir_type) { > + case ir_type_function: { > + /* If the function is a built-in that is partially overridden in the > + * shader, the ir_function stored in the symbol table may not be the > + * same as the one that appears in the shader. The one in the shader > + * will only include definitions from inside the shader. We need the > + * one from the symbol table because it will include built-in > + * defintions and definitions from the shader. > + */ > + ir_function *const def = (ir_function *) ir; > + ir_function *const decl = state->symbols->get_function(def->name); Huh. This doesn't match my understanding of the code. I believe there should only be one ir_function with a given name, and it should be both in the shader's instruction stream and in the symbol table. That single ir_function should contain any definitions and prototypes created in the shader's GLSL code, and also prototypes (but not definitions) for any built-in signatures called. If you look at match_function_by_name(), it always uses the existing function, if there is one, or creates a new one and adds it in both places...walking through a couple scenarios, I haven't seen what's going wrong. I suppose I should try the obvious shader->symobls->add_function((ir_function *) ir); and see what breaks. It sure doesn't seem like anything should. I'll try to look into it soon. > + shader->symbols->add_function(decl); > + break; > + } > + case ir_type_variable: > + shader->symbols->add_variable((ir_variable *) ir); > + break; > + default: > + break; > + } > + } > + > + delete state->symbols; > ralloc_free(state); > } signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 81139] Rendering sometimes halts in waiting for back buffers with dri3 & xwayland
https://bugs.freedesktop.org/show_bug.cgi?id=81139 Boyan Ding changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |NOTOURBUG -- You are receiving this mail because: You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 10/14] i965: Add function to indicate when sampling with hiz is supported
Currently it indicates that this is never supported, but soon it will be supported for gen8+. Signed-off-by: Jordan Justen --- src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 10 ++ src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 4 2 files changed, 14 insertions(+) diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c index 14c5381..880e41d 100644 --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c @@ -1643,6 +1643,16 @@ intel_miptree_alloc_hiz(struct brw_context *brw, } /** + * Can the miptree sample using the hiz buffer? + */ +bool +intel_miptree_sample_with_hiz(struct brw_context *brw, + struct intel_mipmap_tree *mt) +{ + return false; +} + +/** * Does the miptree slice have hiz enabled? */ bool diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h index d0cf65f..e4e5b66 100644 --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h @@ -717,6 +717,10 @@ void intel_hiz_exec(struct brw_context *brw, struct intel_mipmap_tree *mt, unsigned int level, unsigned int layer, enum gen6_hiz_op op); +bool +intel_miptree_sample_with_hiz(struct brw_context *brw, + struct intel_mipmap_tree *mt); + #ifdef __cplusplus } #endif -- 2.0.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 05/14] i965/gen8: Don't allocate hiz miptree structure
We now skip allocating a hiz miptree for gen8. Instead, we calculate the required hiz buffer parameters and allocate a bo directly. Signed-off-by: Jordan Justen --- src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 91 +++ 1 file changed, 91 insertions(+) diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c index 7e8bec8..08a4ebe 100644 --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c @@ -1460,6 +1460,95 @@ intel_gen7_hiz_buf_create(struct brw_context *brw, } +/** + * Helper for intel_miptree_alloc_hiz() that determines the required hiz + * buffer dimensions and allocates a bo for the hiz buffer. + */ +static struct intel_miptree_aux_buffer * +intel_gen8_hiz_buf_create(struct brw_context *brw, + struct intel_mipmap_tree *mt) +{ + unsigned z_width = mt->logical_width0; + unsigned z_height = mt->logical_height0; + const unsigned z_depth = mt->logical_depth0; + unsigned hz_width, hz_height; + struct intel_miptree_aux_buffer *buf = calloc(sizeof(*buf), 1); + + if (!buf) + return NULL; + + /* Gen7 PRM Volume 2, Part 1, 11.5.3 "Hierarchical Depth Buffer" documents +* adjustments required for Z_Height and Z_Width based on multisampling. +*/ + switch(mt->num_samples) { + case 0: + case 1: + break; + case 2: + case 4: + z_width *= 2; + z_height *= 2; + break; + case 8: + z_width *= 4; + z_height *= 2; + break; + default: + assert(!"Unsupported sample count!"); + } + + const unsigned vertical_align = 8; /* 'j' in the docs */ + const unsigned H0 = z_height; + const unsigned h0 = ALIGN(H0, vertical_align); + const unsigned h1 = ALIGN(minify(H0, 1), vertical_align); + const unsigned Z0 = z_depth; + + /* HZ_Width (bytes) = ceiling(Z_Width / 16) * 16 */ + hz_width = ALIGN(z_width, 16); + + unsigned H_i = H0; + unsigned Z_i = Z0; + unsigned sum_h_i = 0; + unsigned hz_height_3d_sum = 0; + for (int level = mt->first_level; level <= mt->last_level; ++level) { + unsigned i = level - mt->first_level; + unsigned h_i = ALIGN(H_i, vertical_align); + /* sum(i=2 to m; h_i) */ + if (i >= 2) { + sum_h_i += h_i; + } + /* sum(i=0 to m; h_i * max(1, floor(Z_Depth/2**i))) */ + hz_height_3d_sum += h_i * Z_i; + H_i = minify(H_i, 1); + Z_i = minify(Z_i, 1); + } + /* HZ_QPitch = h0 + max(h1, sum(i=2 to m; h_i)) */ + buf->qpitch = h0 + MAX2(h1, sum_h_i); + + if (mt->target == GL_TEXTURE_3D) { + /* (1/2) * sum(i=0 to m; h_i * max(1, floor(Z_Depth/2**i))) */ + hz_height = CEILING(hz_height_3d_sum, 2); + } else { + /* HZ_Height (rows) = ceiling( (HZ_QPitch/2)/8) *8 * Z_Depth */ + hz_height = (ALIGN(buf->qpitch, 8) / 2) * Z0; + if (mt->target == GL_TEXTURE_CUBE_MAP_ARRAY || + mt->target == GL_TEXTURE_CUBE_MAP) { + hz_height *= 6; + } + } + + unsigned long pitch; + uint32_t tiling = I915_TILING_Y; + buf->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "hiz", + hz_width, hz_height, 1, + &tiling, &pitch, + BO_ALLOC_FOR_RENDER); + buf->pitch = pitch; + + return buf; +} + + static struct intel_miptree_aux_buffer * intel_hiz_miptree_buf_create(struct brw_context *brw, struct intel_mipmap_tree *mt) @@ -1501,6 +1590,8 @@ intel_miptree_alloc_hiz(struct brw_context *brw, if (brw->gen == 7) { mt->hiz_buf = intel_gen7_hiz_buf_create(brw, mt); + } else if (brw->gen >= 8) { + mt->hiz_buf = intel_gen8_hiz_buf_create(brw, mt); } else { mt->hiz_buf = intel_hiz_miptree_buf_create(brw, mt); } -- 2.0.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 11/14] i965: Support sampling with hiz during rendering
For gen8, we can sample from depth while using the hiz buffer. This allows us to sample depth without resolving from hiz to the depth texture. To do this we must resolve to hiz before drawing so we can use the hiz buffer to sample while rendering. Hopefully the hiz buffer will already be resolved in most cases because it was previously rendered, meaning the hiz resolve is a no-op. Note that this is still controlled by the intel_miptree_sample_with_hiz function, and we will enable hiz sampling for gen8 in a separate patch. Signed-off-by: Jordan Justen --- src/mesa/drivers/dri/i965/brw_draw.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/mesa/drivers/dri/i965/brw_draw.c b/src/mesa/drivers/dri/i965/brw_draw.c index ac21656..3c63b8a 100644 --- a/src/mesa/drivers/dri/i965/brw_draw.c +++ b/src/mesa/drivers/dri/i965/brw_draw.c @@ -321,7 +321,10 @@ brw_predraw_resolve_buffers(struct brw_context *brw) tex_obj = intel_texture_object(ctx->Texture.Unit[i]._Current); if (!tex_obj || !tex_obj->mt) continue; - intel_miptree_all_slices_resolve_depth(brw, tex_obj->mt); + if (intel_miptree_sample_with_hiz(brw, tex_obj->mt)) + intel_miptree_all_slices_resolve_hiz(brw, tex_obj->mt); + else + intel_miptree_all_slices_resolve_depth(brw, tex_obj->mt); intel_miptree_resolve_color(brw, tex_obj->mt); brw_render_cache_set_check_flush(brw, tex_obj->mt->bo); } -- 2.0.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 09/14] i965/gen8: Use aux buf qpitch for Auxiliary Buffer (MCS)
For hiz, the qpitch may be different than the main miptree. In "i965: Wrap MCS miptree in intel_miptree_aux_buffer" we set aux_buf->qpitch to mt->qpitch, so for MCS, this should be a no-op. Signed-off-by: Jordan Justen --- src/mesa/drivers/dri/i965/gen8_surface_state.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c b/src/mesa/drivers/dri/i965/gen8_surface_state.c index 87f0d49..3d97232 100644 --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c @@ -203,7 +203,7 @@ gen8_update_texture_surface(struct gl_context *ctx, (intelObj->_MaxLevel - tObj->BaseLevel); /* mip count */ if (aux_buf) { - surf[6] = SET_FIELD(mt->qpitch / 4, GEN8_SURFACE_AUX_QPITCH) | + surf[6] = SET_FIELD(aux_buf->qpitch / 4, GEN8_SURFACE_AUX_QPITCH) | SET_FIELD((aux_buf->pitch / 128) - 1, GEN8_SURFACE_AUX_PITCH) | aux_mode; } else { @@ -394,7 +394,7 @@ gen8_update_renderbuffer_surface(struct brw_context *brw, surf[5] = irb->mt_level - irb->mt->first_level; if (aux_buf) { - surf[6] = SET_FIELD(mt->qpitch / 4, GEN8_SURFACE_AUX_QPITCH) | + surf[6] = SET_FIELD(aux_buf->qpitch / 4, GEN8_SURFACE_AUX_QPITCH) | SET_FIELD((aux_buf->pitch / 128) - 1, GEN8_SURFACE_AUX_PITCH) | aux_mode; } else { -- 2.0.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 14/14] i965/gen8: Allow sampling with hiz when supported
For gen8+ this will indicate when we should allow hiz based sampling during rendering. Signed-off-by: Jordan Justen --- src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c index 880e41d..756d122 100644 --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c @@ -1649,7 +1649,17 @@ bool intel_miptree_sample_with_hiz(struct brw_context *brw, struct intel_mipmap_tree *mt) { - return false; + /* Sampling using the hiz buffer requires gen8. +* +* If compressed multisampling is enabled, then we use it for the auxiliary +* buffer instead. +* +* We can't enable hiz auxiliary buffer support for 3D textures. +*/ + return (brw->gen >= 8 && + mt->hiz_buf && + !mt->mcs_buf && + mt->target != GL_TEXTURE_3D); } /** -- 2.0.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 08/14] i965/gen8: Use intel_miptree_aux_buffer for auxiliary buffer
This will allow us to treat HiZ and MCS the same when using them as auxiliary surface buffers. Signed-off-by: Jordan Justen --- src/mesa/drivers/dri/i965/gen8_surface_state.c | 28 +- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c b/src/mesa/drivers/dri/i965/gen8_surface_state.c index c9b0842..87f0d49 100644 --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c @@ -132,7 +132,7 @@ gen8_update_texture_surface(struct gl_context *ctx, struct intel_mipmap_tree *mt = intelObj->mt; struct gl_texture_image *firstImage = tObj->Image[0][tObj->BaseLevel]; struct gl_sampler_object *sampler = _mesa_get_samplerobj(ctx, unit); - struct intel_mipmap_tree *aux_mt = NULL; + struct intel_miptree_aux_buffer *aux_buf = NULL; uint32_t aux_mode = 0; mesa_format format = intelObj->_Format; @@ -156,7 +156,7 @@ gen8_update_texture_surface(struct gl_context *ctx, } if (mt->mcs_buf) { - aux_mt = mt->mcs_buf->mt; + aux_buf = mt->mcs_buf; aux_mode = GEN8_SURFACE_AUX_MODE_MCS; } @@ -202,9 +202,9 @@ gen8_update_texture_surface(struct gl_context *ctx, GEN7_SURFACE_MIN_LOD) | (intelObj->_MaxLevel - tObj->BaseLevel); /* mip count */ - if (aux_mt) { + if (aux_buf) { surf[6] = SET_FIELD(mt->qpitch / 4, GEN8_SURFACE_AUX_QPITCH) | -SET_FIELD((aux_mt->pitch / 128) - 1, GEN8_SURFACE_AUX_PITCH) | +SET_FIELD((aux_buf->pitch / 128) - 1, GEN8_SURFACE_AUX_PITCH) | aux_mode; } else { surf[6] = 0; @@ -230,10 +230,10 @@ gen8_update_texture_surface(struct gl_context *ctx, *((uint64_t *) &surf[8]) = mt->bo->offset64 + mt->offset; /* reloc */ - if (aux_mt) { - *((uint64_t *) &surf[10]) = aux_mt->bo->offset64; + if (aux_buf) { + *((uint64_t *) &surf[10]) = aux_buf->bo->offset64; drm_intel_bo_emit_reloc(brw->batch.bo, *surf_offset + 10 * 4, - aux_mt->bo, 0, + aux_buf->bo, 0, I915_GEM_DOMAIN_SAMPLER, 0); } else { surf[10] = 0; @@ -308,7 +308,7 @@ gen8_update_renderbuffer_surface(struct brw_context *brw, struct gl_context *ctx = &brw->ctx; struct intel_renderbuffer *irb = intel_renderbuffer(rb); struct intel_mipmap_tree *mt = irb->mt; - struct intel_mipmap_tree *aux_mt = NULL; + struct intel_miptree_aux_buffer *aux_buf = NULL; uint32_t aux_mode = 0; unsigned width = mt->logical_width0; unsigned height = mt->logical_height0; @@ -363,7 +363,7 @@ gen8_update_renderbuffer_surface(struct brw_context *brw, } if (mt->mcs_buf) { - aux_mt = mt->mcs_buf->mt; + aux_buf = mt->mcs_buf; aux_mode = GEN8_SURFACE_AUX_MODE_MCS; } @@ -393,9 +393,9 @@ gen8_update_renderbuffer_surface(struct brw_context *brw, surf[5] = irb->mt_level - irb->mt->first_level; - if (aux_mt) { + if (aux_buf) { surf[6] = SET_FIELD(mt->qpitch / 4, GEN8_SURFACE_AUX_QPITCH) | -SET_FIELD((aux_mt->pitch / 128) - 1, GEN8_SURFACE_AUX_PITCH) | +SET_FIELD((aux_buf->pitch / 128) - 1, GEN8_SURFACE_AUX_PITCH) | aux_mode; } else { surf[6] = 0; @@ -409,11 +409,11 @@ gen8_update_renderbuffer_surface(struct brw_context *brw, *((uint64_t *) &surf[8]) = mt->bo->offset64; /* reloc */ - if (aux_mt) { - *((uint64_t *) &surf[10]) = aux_mt->bo->offset64; + if (aux_buf) { + *((uint64_t *) &surf[10]) = aux_buf->bo->offset64; drm_intel_bo_emit_reloc(brw->batch.bo, brw->wm.base.surf_offset[surf_index] + 10 * 4, - aux_mt->bo, 0, + aux_buf->bo, 0, I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER); } else { surf[10] = 0; -- 2.0.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 02/14] i965/gen7: Don't rely directly on the hiz miptree structure
We are still allocating a miptree for hiz, but we only use fields from intel_miptree_aux_buffer. This will allow us to switch over to not allocating a miptree. Signed-off-by: Jordan Justen Reviewed-by: Topi Pohjolainen --- src/mesa/drivers/dri/i965/gen7_blorp.cpp| 6 +++--- src/mesa/drivers/dri/i965/gen7_misc_state.c | 7 --- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp b/src/mesa/drivers/dri/i965/gen7_blorp.cpp index d0d623d..ebfe6d1 100644 --- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp +++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp @@ -752,13 +752,13 @@ gen7_blorp_emit_depth_stencil_config(struct brw_context *brw, /* 3DSTATE_HIER_DEPTH_BUFFER */ { - struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_buf->mt; + struct intel_miptree_aux_buffer *hiz_buf = params->depth.mt->hiz_buf; BEGIN_BATCH(3); OUT_BATCH((GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16) | (3 - 2)); OUT_BATCH((mocs << 25) | -(hiz_mt->pitch - 1)); - OUT_RELOC(hiz_mt->bo, +(hiz_buf->pitch - 1)); + OUT_RELOC(hiz_buf->bo, I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, 0); ADVANCE_BATCH(); diff --git a/src/mesa/drivers/dri/i965/gen7_misc_state.c b/src/mesa/drivers/dri/i965/gen7_misc_state.c index 6c6a79b..8f8c33e 100644 --- a/src/mesa/drivers/dri/i965/gen7_misc_state.c +++ b/src/mesa/drivers/dri/i965/gen7_misc_state.c @@ -145,12 +145,13 @@ gen7_emit_depth_stencil_hiz(struct brw_context *brw, OUT_BATCH(0); ADVANCE_BATCH(); } else { - struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_buf->mt; + struct intel_miptree_aux_buffer *hiz_buf = depth_mt->hiz_buf; + BEGIN_BATCH(3); OUT_BATCH(GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16 | (3 - 2)); OUT_BATCH((mocs << 25) | -(hiz_mt->pitch - 1)); - OUT_RELOC(hiz_mt->bo, +(hiz_buf->pitch - 1)); + OUT_RELOC(hiz_buf->bo, I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, 0); -- 2.0.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 07/14] i965: Wrap MCS miptree in intel_miptree_aux_buffer
This will allow us to treat HiZ and MCS the same when using as an auxiliary surface buffer. Signed-off-by: Jordan Justen --- src/mesa/drivers/dri/i965/brw_blorp_clear.cpp | 2 +- src/mesa/drivers/dri/i965/gen7_blorp.cpp | 4 +- src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 8 +- src/mesa/drivers/dri/i965/gen8_surface_state.c| 8 +- src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 96 ++- src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 8 +- 6 files changed, 75 insertions(+), 51 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp index df34c72..d4c3c3d 100644 --- a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp +++ b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp @@ -526,7 +526,7 @@ do_single_blorp_clear(struct brw_context *brw, struct gl_framebuffer *fb, /* If the MCS buffer hasn't been allocated yet, we need to allocate * it now. */ - if (!irb->mt->mcs_mt) { + if (!irb->mt->mcs_buf) { if (!intel_miptree_alloc_non_msrt_mcs(brw, irb->mt)) { /* MCS allocation failed--probably this will only happen in * out-of-memory conditions. But in any case, try to recover diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp b/src/mesa/drivers/dri/i965/gen7_blorp.cpp index ebfe6d1..dd87c7f 100644 --- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp +++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp @@ -196,8 +196,8 @@ gen7_blorp_emit_surface_state(struct brw_context *brw, surf[3] = pitch_bytes - 1; surf[4] = gen7_surface_msaa_bits(surface->num_samples, surface->msaa_layout); - if (surface->mt->mcs_mt) { - gen7_set_surface_mcs_info(brw, surf, wm_surf_offset, surface->mt->mcs_mt, + if (surface->mt->mcs_buf) { + gen7_set_surface_mcs_info(brw, surf, wm_surf_offset, surface->mt->mcs_buf->mt, is_render_target); } diff --git a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c index 01120af..dabc3b1 100644 --- a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c +++ b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c @@ -359,9 +359,9 @@ gen7_update_texture_surface(struct gl_context *ctx, SET_FIELD(brw_swizzle_to_scs(GET_SWZ(swizzle, 3), need_scs_green_to_blue), GEN7_SURFACE_SCS_A); } - if (mt->mcs_mt) { + if (mt->mcs_buf) { gen7_set_surface_mcs_info(brw, surf, *surf_offset, -mt->mcs_mt, false /* is RT */); +mt->mcs_buf->mt, false /* is RT */); } /* Emit relocation to surface contents */ @@ -538,9 +538,9 @@ gen7_update_renderbuffer_surface(struct brw_context *brw, min_array_element << GEN7_SURFACE_MIN_ARRAY_ELEMENT_SHIFT | (depth - 1) << GEN7_SURFACE_RENDER_TARGET_VIEW_EXTENT_SHIFT; - if (irb->mt->mcs_mt) { + if (irb->mt->mcs_buf) { gen7_set_surface_mcs_info(brw, surf, brw->wm.base.surf_offset[surf_index], -irb->mt->mcs_mt, true /* is RT */); +irb->mt->mcs_buf->mt, true /* is RT */); } surf[7] = irb->mt->fast_clear_color_value; diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c b/src/mesa/drivers/dri/i965/gen8_surface_state.c index 40eb2ea..c9b0842 100644 --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c @@ -155,8 +155,8 @@ gen8_update_texture_surface(struct gl_context *ctx, pitch = mt->pitch; } - if (mt->mcs_mt) { - aux_mt = mt->mcs_mt; + if (mt->mcs_buf) { + aux_mt = mt->mcs_buf->mt; aux_mode = GEN8_SURFACE_AUX_MODE_MCS; } @@ -362,8 +362,8 @@ gen8_update_renderbuffer_surface(struct brw_context *brw, __FUNCTION__, _mesa_get_format_name(rb_format)); } - if (mt->mcs_mt) { - aux_mt = mt->mcs_mt; + if (mt->mcs_buf) { + aux_mt = mt->mcs_buf->mt; aux_mode = GEN8_SURFACE_AUX_MODE_MCS; } diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c index b804a5c..14c5381 100644 --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c @@ -829,7 +829,10 @@ intel_miptree_release(struct intel_mipmap_tree **mt) drm_intel_bo_unreference((*mt)->hiz_buf->bo); free((*mt)->hiz_buf); } - intel_miptree_release(&(*mt)->mcs_mt); + if ((*mt)->mcs_buf) { + intel_miptree_release(&(*mt)->mcs_buf->mt); + free((*mt)->mcs_buf); + } intel_resolve_map_clear(&(*mt)->hiz_map); for (i = 0; i < MAX_TEXTURE_LEVELS; i++) { @@ -1234,13 +1237,52 @@ intel_miptree_copy_teximage(struct brw_context *brw, intel_obj->needs_validate = true; } +static struct intel_miptree_aux_buffer * +intel_mcs_miptree_buf_create(struct brw_co
[Mesa-dev] [PATCH 12/14] i965/gen8: Initialize aux_mode to GEN8_SURFACE_AUX_MODE_NONE
GEN8_SURFACE_AUX_MODE_NONE is 0, so this is a no-op. Yet, this also makes it clear that we can compare aux_mode to the other GEN8_SURFACE_AUX_MODE_ values. We will want to compare to GEN8_SURFACE_AUX_MODE_HIZ. Signed-off-by: Jordan Justen --- src/mesa/drivers/dri/i965/gen8_surface_state.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c b/src/mesa/drivers/dri/i965/gen8_surface_state.c index 3d97232..4818fca 100644 --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c @@ -133,7 +133,7 @@ gen8_update_texture_surface(struct gl_context *ctx, struct gl_texture_image *firstImage = tObj->Image[0][tObj->BaseLevel]; struct gl_sampler_object *sampler = _mesa_get_samplerobj(ctx, unit); struct intel_miptree_aux_buffer *aux_buf = NULL; - uint32_t aux_mode = 0; + uint32_t aux_mode = GEN8_SURFACE_AUX_MODE_NONE; mesa_format format = intelObj->_Format; if (tObj->Target == GL_TEXTURE_BUFFER) { @@ -309,7 +309,7 @@ gen8_update_renderbuffer_surface(struct brw_context *brw, struct intel_renderbuffer *irb = intel_renderbuffer(rb); struct intel_mipmap_tree *mt = irb->mt; struct intel_miptree_aux_buffer *aux_buf = NULL; - uint32_t aux_mode = 0; + uint32_t aux_mode = GEN8_SURFACE_AUX_MODE_NONE; unsigned width = mt->logical_width0; unsigned height = mt->logical_height0; unsigned pitch = mt->pitch; -- 2.0.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 01/14] i965/hiz: Start to separate miptree out from hiz buffers
Today we allocate a miptree's for the hiz buffer. We needed this in the past because we would point the hardware at offsets of the hiz buffer. Since the hiz format is not documented, this is not a good idea. Since moving to support layered rendering on Gen7+, we no longer point at an offset into the buffer on Gen7+. Therefore, to support hiz on Gen7+, we don't need a full miptree structure allocated. This patch starts to create a new auxiliary buffer structure (intel_miptree_aux_buffer) that can be a more simplistic miptree side-band buffer associated with a miptree. (For example, to serve the needs of the hiz buffer.) Signed-off-by: Jordan Justen Reviewed-by: Topi Pohjolainen --- src/mesa/drivers/dri/i965/brw_misc_state.c| 4 +- src/mesa/drivers/dri/i965/gen6_blorp.cpp | 2 +- src/mesa/drivers/dri/i965/gen7_blorp.cpp | 2 +- src/mesa/drivers/dri/i965/gen7_misc_state.c | 2 +- src/mesa/drivers/dri/i965/gen8_depth_state.c | 6 +-- src/mesa/drivers/dri/i965/intel_fbo.c | 4 +- src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 55 +++ src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 29 -- 8 files changed, 76 insertions(+), 28 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c b/src/mesa/drivers/dri/i965/brw_misc_state.c index 76e22bd..af900cd 100644 --- a/src/mesa/drivers/dri/i965/brw_misc_state.c +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c @@ -182,7 +182,7 @@ brw_get_depthstencil_tile_masks(struct intel_mipmap_tree *depth_mt, if (intel_miptree_level_has_hiz(depth_mt, depth_level)) { uint32_t hiz_tile_mask_x, hiz_tile_mask_y; - intel_miptree_get_tile_masks(depth_mt->hiz_mt, + intel_miptree_get_tile_masks(depth_mt->hiz_buf->mt, &hiz_tile_mask_x, &hiz_tile_mask_y, false); @@ -643,7 +643,7 @@ brw_emit_depth_stencil_hiz(struct brw_context *brw, /* Emit hiz buffer. */ if (hiz) { - struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_mt; + struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_buf->mt; BEGIN_BATCH(3); OUT_BATCH((_3DSTATE_HIER_DEPTH_BUFFER << 16) | (3 - 2)); OUT_BATCH(hiz_mt->pitch - 1); diff --git a/src/mesa/drivers/dri/i965/gen6_blorp.cpp b/src/mesa/drivers/dri/i965/gen6_blorp.cpp index eb865b9..282c7b2 100644 --- a/src/mesa/drivers/dri/i965/gen6_blorp.cpp +++ b/src/mesa/drivers/dri/i965/gen6_blorp.cpp @@ -855,7 +855,7 @@ gen6_blorp_emit_depth_stencil_config(struct brw_context *brw, /* 3DSTATE_HIER_DEPTH_BUFFER */ { - struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_mt; + struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_buf->mt; uint32_t hiz_offset = intel_miptree_get_aligned_offset(hiz_mt, draw_x & ~tile_mask_x, diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp b/src/mesa/drivers/dri/i965/gen7_blorp.cpp index 0ad570b..d0d623d 100644 --- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp +++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp @@ -752,7 +752,7 @@ gen7_blorp_emit_depth_stencil_config(struct brw_context *brw, /* 3DSTATE_HIER_DEPTH_BUFFER */ { - struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_mt; + struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_buf->mt; BEGIN_BATCH(3); OUT_BATCH((GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16) | (3 - 2)); diff --git a/src/mesa/drivers/dri/i965/gen7_misc_state.c b/src/mesa/drivers/dri/i965/gen7_misc_state.c index 22911bf..6c6a79b 100644 --- a/src/mesa/drivers/dri/i965/gen7_misc_state.c +++ b/src/mesa/drivers/dri/i965/gen7_misc_state.c @@ -145,7 +145,7 @@ gen7_emit_depth_stencil_hiz(struct brw_context *brw, OUT_BATCH(0); ADVANCE_BATCH(); } else { - struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_mt; + struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_buf->mt; BEGIN_BATCH(3); OUT_BATCH(GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16 | (3 - 2)); OUT_BATCH((mocs << 25) | diff --git a/src/mesa/drivers/dri/i965/gen8_depth_state.c b/src/mesa/drivers/dri/i965/gen8_depth_state.c index 7c3bfe0..b6f373d 100644 --- a/src/mesa/drivers/dri/i965/gen8_depth_state.c +++ b/src/mesa/drivers/dri/i965/gen8_depth_state.c @@ -89,10 +89,10 @@ emit_depth_packets(struct brw_context *brw, } else { BEGIN_BATCH(5); OUT_BATCH(GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16 | (5 - 2)); - OUT_BATCH((depth_mt->hiz_mt->pitch - 1) | BDW_MOCS_WB << 25); - OUT_RELOC64(depth_mt->hiz_mt->bo, + OUT_BATCH((depth_mt->hiz_buf->mt->pitch - 1) | BDW_MOCS_WB << 25); + OUT_RELOC64(depth_mt->hiz_buf->mt->bo, I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, 0); - OUT_BATCH(depth_mt->hiz_mt->qpitch >> 2); + OUT_BATCH(depth_mt->hiz_buf->mt->qpitch >> 2); ADVANCE_BATCH(); } diff --git a/src/mesa/drivers/dri/i96
[Mesa-dev] [PATCH 04/14] i965/gen7: Don't allocate hiz miptree structure
We now skip allocating a hiz miptree for gen7. Instead, we calculate the required hiz buffer parameters and allocate a bo directly. Signed-off-by: Jordan Justen --- src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 95 ++- 1 file changed, 93 insertions(+), 2 deletions(-) diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c index 8719c29..7e8bec8 100644 --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c @@ -823,7 +823,10 @@ intel_miptree_release(struct intel_mipmap_tree **mt) drm_intel_bo_unreference((*mt)->bo); intel_miptree_release(&(*mt)->stencil_mt); if ((*mt)->hiz_buf) { - intel_miptree_release(&(*mt)->hiz_buf->mt); + if ((*mt)->hiz_buf->mt) +intel_miptree_release(&(*mt)->hiz_buf->mt); + else +drm_intel_bo_unreference((*mt)->hiz_buf->bo); free((*mt)->hiz_buf); } intel_miptree_release(&(*mt)->mcs_mt); @@ -1374,6 +1377,89 @@ intel_miptree_level_enable_hiz(struct brw_context *brw, } +/** + * Helper for intel_miptree_alloc_hiz() that determines the required hiz + * buffer dimensions and allocates a bo for the hiz buffer. + */ +static struct intel_miptree_aux_buffer * +intel_gen7_hiz_buf_create(struct brw_context *brw, + struct intel_mipmap_tree *mt) +{ + unsigned z_width = mt->logical_width0; + unsigned z_height = mt->logical_height0; + const unsigned z_depth = mt->logical_depth0; + unsigned hz_width, hz_height, qpitch; + struct intel_miptree_aux_buffer *buf = calloc(sizeof(*buf), 1); + + if (!buf) + return NULL; + + /* Gen7 PRM Volume 2, Part 1, 11.5.3 "Hierarchical Depth Buffer" documents +* adjustments required for Z_Height and Z_Width based on multisampling. +*/ + switch(mt->num_samples) { + case 0: + case 1: + break; + case 2: + case 4: + z_width *= 2; + z_height *= 2; + break; + case 8: + z_width *= 4; + z_height *= 2; + break; + default: + assert(!"Unsupported sample count!"); + } + + const unsigned vertical_align = 8; /* 'j' in the docs */ + const unsigned H0 = z_height; + const unsigned h0 = ALIGN(H0, vertical_align); + const unsigned h1 = ALIGN(minify(H0, 1), vertical_align); + const unsigned Z0 = z_depth; + + /* HZ_Width (bytes) = ceiling(Z_Width / 16) * 16 */ + hz_width = ALIGN(z_width, 16); + + if (mt->target == GL_TEXTURE_3D) { + unsigned H_i = H0; + unsigned Z_i = Z0; + hz_height = 0; + for (int level = mt->first_level; level <= mt->last_level; ++level) { + unsigned h_i = ALIGN(H_i, vertical_align); + /* sum(i=0 to m; h_i * max(1, floor(Z_Depth/2**i))) */ + hz_height += h_i * Z_i; + H_i = minify(H_i, 1); + Z_i = minify(Z_i, 1); + } + /* HZ_Height = + *(1/2) * sum(i=0 to m; h_i * max(1, floor(Z_Depth/2**i))) + */ + hz_height = CEILING(hz_height, 2); + } else { + qpitch = h0 + h1 + (12 * vertical_align); + /* HZ_Height (rows) = Ceiling ( ( Q_pitch * Z_depth/2) /8 ) * 8 */ + hz_height = (ALIGN(qpitch, 8) / 2) * Z0; + if (mt->target == GL_TEXTURE_CUBE_MAP_ARRAY || + mt->target == GL_TEXTURE_CUBE_MAP) { + hz_height *= 6; + } + } + + unsigned long pitch; + uint32_t tiling = I915_TILING_Y; + buf->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "hiz", + hz_width, hz_height, 1, + &tiling, &pitch, + BO_ALLOC_FOR_RENDER); + buf->pitch = pitch; + + return buf; +} + + static struct intel_miptree_aux_buffer * intel_hiz_miptree_buf_create(struct brw_context *brw, struct intel_mipmap_tree *mt) @@ -1412,7 +1498,12 @@ intel_miptree_alloc_hiz(struct brw_context *brw, struct intel_mipmap_tree *mt) { assert(mt->hiz_buf == NULL); - mt->hiz_buf = intel_hiz_miptree_buf_create(brw, mt); + + if (brw->gen == 7) { + mt->hiz_buf = intel_gen7_hiz_buf_create(brw, mt); + } else { + mt->hiz_buf = intel_hiz_miptree_buf_create(brw, mt); + } if (!mt->hiz_buf) return false; -- 2.0.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 00/14] i965/gen7+ HiZ cleanup + gen8 hiz improvements
1. No longer allocate a miptree structure for hiz on gen7+. This was previously sent out in the series: "i965 gen7+ hiz buffer change; always enable gen8 hiz" 2. Always enable HiZ for depth on gen8+. (Also part of the previous series.) 3. Enable HiZ Auxiliary Buffer support on gen8. This support allows Broadwell to sample from depth using the hiz buffer, and thereby removing the need to resolve depth to/from the hiz buffer in many cases. With regards to the content in both this and the "i965 gen7+ hiz buffer change; always enable gen8 hiz" series, there were these changes: 1. Calculating hiz buffer parameters was separated out into a gen7 and a gen8 version. 2. Some variables were renamed in the functions that allocate the hiz buffer. 3. Some comments were added to show the code from the documents compared to the code doing the calculation. No piglit regressions were seen on gen7 or gen8. This series is available in the bdw-aux-hiz branch of: git://people.freedesktop.org/~jljusten/mesa Jordan Justen (13): i965/hiz: Start to separate miptree out from hiz buffers i965/gen7: Don't rely directly on the hiz miptree structure i965/gen8: Don't rely directly on the hiz miptree structure i965/gen7: Don't allocate hiz miptree structure i965/gen8: Don't allocate hiz miptree structure i965/gen8: Enable hiz for all depth levels i965: Wrap MCS miptree in intel_miptree_aux_buffer i965/gen8: Use intel_miptree_aux_buffer for auxiliary buffer i965/gen8: Use aux buf qpitch for Auxiliary Buffer (MCS) i965: Add function to indicate when sampling with hiz is supported i965: Support sampling with hiz during rendering i965/gen8: Initialize aux_mode to GEN8_SURFACE_AUX_MODE_NONE i965/gen8: Allow sampling with hiz when supported Kenneth Graunke (1): i965/gen8: Add HiZ auxiliary buffer support src/mesa/drivers/dri/i965/brw_blorp_clear.cpp | 2 +- src/mesa/drivers/dri/i965/brw_draw.c | 5 +- src/mesa/drivers/dri/i965/brw_misc_state.c| 4 +- src/mesa/drivers/dri/i965/gen6_blorp.cpp | 2 +- src/mesa/drivers/dri/i965/gen7_blorp.cpp | 10 +- src/mesa/drivers/dri/i965/gen7_misc_state.c | 7 +- src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 8 +- src/mesa/drivers/dri/i965/gen8_depth_state.c | 6 +- src/mesa/drivers/dri/i965/gen8_surface_state.c| 45 +-- src/mesa/drivers/dri/i965/intel_fbo.c | 4 +- src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 357 ++ src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 41 ++- 12 files changed, 388 insertions(+), 103 deletions(-) -- 2.0.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 06/14] i965/gen8: Enable hiz for all depth levels
After modifying the hiz buffer allocation and qpitch calculation, hiz appears to work in all cases on gen8. Signed-off-by: Jordan Justen --- src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c index 08a4ebe..b804a5c 100644 --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c @@ -1355,7 +1355,7 @@ intel_miptree_level_enable_hiz(struct brw_context *brw, { assert(mt->hiz_buf); - if (brw->gen >= 8 || brw->is_haswell) { + if (brw->is_haswell) { uint32_t width = minify(mt->physical_width0, level); uint32_t height = minify(mt->physical_height0, level); -- 2.0.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 13/14] i965/gen8: Add HiZ auxiliary buffer support
From: Kenneth Graunke Signed-off-by: Kenneth Graunke [jordan.l.jus...@intel.com: convert from aux_mt to aux_buf] Signed-off-by: Jordan Justen --- src/mesa/drivers/dri/i965/gen8_surface_state.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c b/src/mesa/drivers/dri/i965/gen8_surface_state.c index 4818fca..45ff904 100644 --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c @@ -158,6 +158,9 @@ gen8_update_texture_surface(struct gl_context *ctx, if (mt->mcs_buf) { aux_buf = mt->mcs_buf; aux_mode = GEN8_SURFACE_AUX_MODE_MCS; + } else if (intel_miptree_sample_with_hiz(brw, mt)) { + aux_buf = mt->hiz_buf; + aux_mode = GEN8_SURFACE_AUX_MODE_HIZ; } /* If this is a view with restricted NumLayers, then our effective depth @@ -239,7 +242,7 @@ gen8_update_texture_surface(struct gl_context *ctx, surf[10] = 0; surf[11] = 0; } - surf[12] = 0; + surf[12] = (aux_mode == GEN8_SURFACE_AUX_MODE_HIZ) ? mt->depth_clear_value : 0; /* Emit relocation to surface contents */ drm_intel_bo_emit_reloc(brw->batch.bo, -- 2.0.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 03/14] i965/gen8: Don't rely directly on the hiz miptree structure
We are still allocating a miptree for hiz, but we only use fields from intel_miptree_aux_buffer. This will allow us to switch over to not allocating a miptree. Signed-off-by: Jordan Justen Reviewed-by: Topi Pohjolainen --- src/mesa/drivers/dri/i965/gen8_depth_state.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mesa/drivers/dri/i965/gen8_depth_state.c b/src/mesa/drivers/dri/i965/gen8_depth_state.c index b6f373d..8774595 100644 --- a/src/mesa/drivers/dri/i965/gen8_depth_state.c +++ b/src/mesa/drivers/dri/i965/gen8_depth_state.c @@ -89,10 +89,10 @@ emit_depth_packets(struct brw_context *brw, } else { BEGIN_BATCH(5); OUT_BATCH(GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16 | (5 - 2)); - OUT_BATCH((depth_mt->hiz_buf->mt->pitch - 1) | BDW_MOCS_WB << 25); - OUT_RELOC64(depth_mt->hiz_buf->mt->bo, + OUT_BATCH((depth_mt->hiz_buf->pitch - 1) | BDW_MOCS_WB << 25); + OUT_RELOC64(depth_mt->hiz_buf->bo, I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, 0); - OUT_BATCH(depth_mt->hiz_buf->mt->qpitch >> 2); + OUT_BATCH(depth_mt->hiz_buf->qpitch >> 2); ADVANCE_BATCH(); } -- 2.0.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 81174] Gallium: GL_LINE_LOOP broken with more than 512 points
https://bugs.freedesktop.org/show_bug.cgi?id=81174 --- Comment #14 from Roland Scheidegger --- In my testing this seems to be *both* a core (vbo) and a draw problem. With my piglit test the first error (without redrawing) appears with 4097 vertices - despite that the vbo code can handle 5460 (per vertex size is just 12 bytes) without splitting. -- You are receiving this mail because: You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 80183] [llvmpipe] triangles with vertices that map to raster positions > viewport width/height are not displayed
https://bugs.freedesktop.org/show_bug.cgi?id=80183 Roland Scheidegger changed: What|Removed |Added Component|Mesa core |Other --- Comment #6 from Roland Scheidegger --- Oops wrong bug. Disregard last comment... -- You are receiving this mail because: You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 80183] [llvmpipe] triangles with vertices that map to raster positions > viewport width/height are not displayed
https://bugs.freedesktop.org/show_bug.cgi?id=80183 Roland Scheidegger changed: What|Removed |Added Component|Other |Mesa core --- Comment #5 from Roland Scheidegger --- Reassigning to mesa core. Though in my testing this seems to be *both* a core and a draw problem. With my piglit test the first error (without redrawing) appears with 4097 vertices - despite that the vbo code can handle 5460 (per vertex size is just 12 bytes) without splitting. -- You are receiving this mail because: You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] glsl: Make the tree rebalancer bail on matrix operands.
Series is Reviewed-by: Matt Turner ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] Enabling ARB_gpu_shader5 in mesa/st
Hello, The mesa/main support for ARB_gs5 is nearly ready, and I have the various mesa/st patches all sent out. The last bit is the dynamic sampler support which I think will Just Work in mesa/st once glsl lets it through. Most extensions are enabled on the basis of a PIPE_CAP_*, either it being enabled, or being > some limit. However ARB_gs5 is an extension composed of like 10 sub-features. I could go through and add caps for each of them, but that seems really excessive. In a future universe, we might rely on PIPE_CAP_GLSL_FEATURE_LEVEL >= 400, but since fp64/tess support aren't there yet (and may not be implemented by all drivers simultaneously when they are), I feel a little weird about using that as the "defining" metric. But it's not so bad -- fp64 could require some sort of DOUBLE shader cap, and tess could check for max instructions with the relevant PIPE_SHADER_*. What's the preferred way of doing this? Cheers, -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/3] glsl: Make the tree rebalancer bail on matrix operands.
It doesn't handle things like (vector * matrix) correctly, and apparently Matt's intention was to bail. Fixes shader compilation in Natural Selection 2. Signed-off-by: Kenneth Graunke --- src/glsl/opt_rebalance_tree.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/glsl/opt_rebalance_tree.cpp b/src/glsl/opt_rebalance_tree.cpp index daabdc9..7ef0d2d 100644 --- a/src/glsl/opt_rebalance_tree.cpp +++ b/src/glsl/opt_rebalance_tree.cpp @@ -217,7 +217,9 @@ is_reduction(ir_instruction *ir, void *data) * constant fold once split up. Handling matrices will need some more * work. */ - if (expr->type->is_matrix()) { + if (expr->type->is_matrix() || + expr->operands[0]->type->is_matrix() || + (expr->operands[1] && expr->operands[1]->type->is_matrix())) { ird->is_reduction = false; return; } -- 2.0.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/3] glsl: Guard against error_type in the tree rebalancer.
This helped me track down the bug fixed in the previous commit. Signed-off-by: Kenneth Graunke --- src/glsl/opt_rebalance_tree.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/glsl/opt_rebalance_tree.cpp b/src/glsl/opt_rebalance_tree.cpp index 7ef0d2d..f82b16b 100644 --- a/src/glsl/opt_rebalance_tree.cpp +++ b/src/glsl/opt_rebalance_tree.cpp @@ -271,11 +271,13 @@ update_types(ir_instruction *ir, void *) if (!expr) return; - expr->type = + const glsl_type *const new_type = glsl_type::get_instance(expr->type->base_type, MAX2(expr->operands[0]->type->components(), expr->operands[1]->type->components()), 1); + assert(new_type != glsl_type::error_type); + expr->type = new_type; } void -- 2.0.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/3] glsl: Make the tree rebalancer use vector_elements, not components().
components() includes matrix columns, so if this code encountered a matrix, it would ask for something like a vec9 or vec16. This is clearly not what you want. Earlier code now prevents this from seeing matrices, but we should still use vector_elements, for clarity. Signed-off-by: Kenneth Graunke --- src/glsl/opt_rebalance_tree.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/glsl/opt_rebalance_tree.cpp b/src/glsl/opt_rebalance_tree.cpp index f82b16b..095f2d7 100644 --- a/src/glsl/opt_rebalance_tree.cpp +++ b/src/glsl/opt_rebalance_tree.cpp @@ -273,8 +273,8 @@ update_types(ir_instruction *ir, void *) const glsl_type *const new_type = glsl_type::get_instance(expr->type->base_type, - MAX2(expr->operands[0]->type->components(), - expr->operands[1]->type->components()), + MAX2(expr->operands[0]->type->vector_elements, + expr->operands[1]->type->vector_elements), 1); assert(new_type != glsl_type::error_type); expr->type = new_type; -- 2.0.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 00/10] [RFC] Probably useless algebraic optimizations
So, a little update on these patches. I've written some shaders for hitting each specific case in the patch-series. This shows that: Patch 1 (X - X) == 0,and Patch 9 (A - neg(B)) == A + B have no effect at all. The rest of the patches do indeed have a positive effect on the special-case shader. If anyone wants to have a look at the shaders then let me know. I could always put them in a dropbox-folder, or github, or something. The report from shader-db (sorted by patch-number): Patch 2: helped: shaders/mine/a_or_nota.shader_test fs16: 16 -> 5 (-68.75%) helped: shaders/mine/a_or_nota.shader_test fs8: 16 -> 5 (-68.75%) helped: shaders/mine/a_or_nota.shader_test vs:11 -> 5 (-54.55%) Patch 3: helped: shaders/mine/a_and_nota.shader_test fs16: 16 -> 5 (-68.75%) helped: shaders/mine/a_and_nota.shader_test fs8: 16 -> 5 (-68.75%) helped: shaders/mine/a_and_nota.shader_test vs: 11 -> 5 (-54.55%) Patch 4: helped: shaders/mine/or_and.shader_test fs16: 16 -> 14 (-12.50%) helped: shaders/mine/or_and.shader_test fs8: 16 -> 14 (-12.50%) helped: shaders/mine/or_and.shader_test vs: 11 -> 10 (-9.09%) Patch 5: helped: shaders/mine/minOver.shader_test fs16:8 -> 5 (-37.50%) helped: shaders/mine/minOver.shader_test fs8: 8 -> 5 (-37.50%) helped: shaders/mine/minOver.shader_test vs: 6 -> 5 (-16.67%) helped: shaders/mine/minUnder.shader_test fs16: 8 -> 5 (-37.50%) helped: shaders/mine/minUnder.shader_test fs8:8 -> 5 (-37.50%) helped: shaders/mine/minUnder.shader_test vs: 6 -> 5 (-16.67%) Patch 6 helped: shaders/mine/maxOver.shader_test fs16:8 -> 5 (-37.50%) helped: shaders/mine/maxOver.shader_test fs8: 8 -> 5 (-37.50%) helped: shaders/mine/maxOver.shader_test vs: 6 -> 5 (-16.67%) helped: shaders/mine/maxUnder.shader_test fs16: 8 -> 5 (-37.50%) helped: shaders/mine/maxUnder.shader_test fs8:8 -> 5 (-37.50%) helped: shaders/mine/maxUnder.shader_test vs: 6 -> 5 (-16.67%) Patch 7: helped: shaders/mine/loglog.shader_test fs16: 17 -> 11 (-35.29%) helped: shaders/mine/loglog.shader_test fs8: 17 -> 11 (-35.29%) helped: shaders/mine/loglog.shader_test vs: 7 -> 6 (-14.29%) Patch 8: helped: shaders/mine/expexp.shader_test fs16: 17 -> 11 (-35.29%) helped: shaders/mine/expexp.shader_test fs8: 17 -> 11 (-35.29%) helped: shaders/mine/expexp.shader_test vs: 7 -> 6 (-14.29%) Patch 10: helped: shaders/mine/pow0x.shader_test fs16: 8 -> 5 (-37.50%) helped: shaders/mine/pow0x.shader_test fs8: 8 -> 5 (-37.50%) helped: shaders/mine/pow0x.shader_test vs:6 -> 5 (-16.67%) helped: shaders/mine/powx-1.shader_test fs16: 11 -> 5 (-54.55%) helped: shaders/mine/powx-1.shader_test fs8: 11 -> 5 (-54.55%) helped: shaders/mine/powx-1.shader_test vs: 7 -> 5 (-28.57%) helped: shaders/mine/powx0.shader_test fs16: 8 -> 5 (-37.50%) helped: shaders/mine/powx0.shader_test fs8: 8 -> 5 (-37.50%) helped: shaders/mine/powx0.shader_test vs:6 -> 5 (-16.67%) 2014-07-15 0:22 GMT+02:00 : > From: Thomas Helland > > When writing that A || (A && B) patch some > days ago I also wrote some other patches > that have no impact on my collection of shaders. > (shader-db + Some TF2 and Portal-shaders). > No reduction in instruction count, and no > significant increase in compilation time. > > I decided to put them up here anyway, as > with your collection of shaders maybe YMMV. > > These are mostly RFC-quality, and not all are > as complete and nicely formatted as they could be. > Possibly some are also implemented incorrectly. > (I'm still trying to get a good understanding of > the buildup of the ir, the visitors, etc) > > Feel free to do with these patches as you please; > Ignore, test, review, flame, make cookies... > > Thomas Helland (10): > glsl: Optimize X - X -> 0 > glsl: Optimize !A || A == 1 > glsl: Optimize !A && A == 0 > glsl: Optimize (A || B) && A == A > glsl: Optimize min(-8, sin(x)) == -8 and similar > glsl: Optimize max(8, sin(x)) == 8 and similar > glsl: Optimize log(x) + log(y) == log(x*y) > glsl: Optimize exp(x)*exp(y) == exp(x+y) > glsl: Optimize A - neg(B) == A + B and neg(A) - B == neg(A + B) > glsl: Optimize some more pow() special cases > > src/glsl/opt_algebraic.cpp | 152 > + > 1 file changed, 152 insertions(+) > > -- > 2.0.0 > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 09/10] glsl: Optimize A - neg(B) == A + B
I agree, agree, and agree. We do indeed have a neg( neg() ) pass. Writing a shader similar to the one you suggest gives clean A + B with current master due to this, so this patch will do no good. 2014-07-15 19:05 GMT+02:00 Ian Romanick : > I haven't really looked at the other patches yet, but the subject of > this one caught my eye. > > I'm not sure this is useful. I believe we lower A-B to A+neg(B) for all > architectures. I'm pretty sure we also already have a pass that > converts A+neg(neg(B)) to A+B. Did you write any shaders to try to > tickle this code? What did something like the following produce before > and after this change? > > uniform vec4 a; > uniform vec4 b; > void main() > { > gl_Position = a - (-b); > } > > For the neg(A)-B case, I think the existing code is better for most > architectures. You end up with a single > > ADD r2, -r1, -r0 > > instruction currently. With this change you would potentially need an > extra move to apply the negation. I believe at least the i965 backend > has a pass to push those negations "down" the tree... effectively > undoing what your pass does. :) > > On 07/14/2014 03:22 PM, thomashellan...@gmail.com wrote: > > From: Thomas Helland > > > > ...and neg(A) - B == neg(A + B) > > > > Signed-off-by: Thomas Helland > > --- > > src/glsl/opt_algebraic.cpp | 6 ++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/src/glsl/opt_algebraic.cpp b/src/glsl/opt_algebraic.cpp > > index 2361d0f..fba9de6 100644 > > --- a/src/glsl/opt_algebraic.cpp > > +++ b/src/glsl/opt_algebraic.cpp > > @@ -454,6 +454,12 @@ > ir_algebraic_visitor::handle_expression(ir_expression *ir) > >/* X - X == 0 */ > >if (ir->operands[0]->equals(ir->operands[1])) > > return ir_constant::zero(ir, ir->type); > > + /* A - neg(B) = A + B */ > > + if (op_expr[1] && op_expr[1]->operation == ir_unop_neg) > > + return add(ir->operands[0], op_expr[1]->operands[0]); > > + /* neg(A) - B = neg(A + B) */ > > + if (op_expr[0] && op_expr[0]->operation == ir_unop_neg) > > + return neg(add(op_expr[0]->operands[0], ir->operands[1])); > >break; > > > > case ir_binop_mul: > > > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] A proposal for new testing requirements for stable releases
Michel Dänzer writes: >> configurations are. All I need is something along the lines of: >> >> "The radeon team is OK with releasing commit " ... [snip of the rest of my proposal] > This sounds good, but... Good! Thanks for reading this and giving me some feedback. >> And in order to put some teeth into this scheme: >> >> I propose that on the day of the release, the release manager >> drop all driver-specific patches for any driver for which the >> driver team has not provided an affirmative testing report. > > ... this seems a bit harsh. > > A lot of backported fixes are specific to one driver and have zero > impact on anything else. Surely it should be up to the discretion of the > driver maintainers whether or not such changes should be backported. Yes. I do want to give the driver maintainers a lot of discretion. They are already the people who are doing the proposal of which changes should be backported. Quite frankly, my real concern with all of this is not that the driver maintainers will propose something bad, but that I will inadvertently botch something while cherry-picking or merging a conflict, etc. that I won't be able to notice in my touch testing. > OTOH, who's supposed to give that sort of OK for changes to say st/mesa > or Mesa core? The former may affect all Gallium based drivers, the > latter basically anything. Previously, for each stable release, I was doing piglit testing against the hardware driver for my personal laptop. My hope was that that would give at least some coverage for any mesa-core changes. With the current release cycle, I've expanded that testing to also include piglit runs against swrast (in both dri and gallium flavors). I hope that that will give us sufficient confidence in non-driver-specific changes. With this additional testing, it's sufficiently time-intensive that I don't plan to repeat it in a single cycle, (I'm performing that testing before pushing out a proposed branch like I did yesterday). That's why my email yesterday said I could still accept driver-specific changes, (but any core changes will have to wait for the next release). > It's unlikely that any individual or organization can test all > affected setups in advance. I agree. And I won't require that. All I want is what I said above: "We're OK with releasing commit " From a reasonably representative of each driver team. I'm not going to ask for documentation to backup that statement, (no piglit reports or anything else). So every driver team can come up with their own criteria for comfort level. If you're happy to continue to just defer to me (or any other stable-release manager), then all I'm asking for is one quick email each release to give your OK. If driver teams want to do some actual testing, then this new process gives them time to do that testing and a way to communicate those results prior to release. And I think that's the most-important change here. I'm committing to providing a 3-day window in which testing can be performed prior to each stable release. > We at AMD are working towards having such regular testing, but we're not > quite there yet, nor do we have an exact schedule yet for when we will > be. It's great that you're working toward it. I think that's a valuable goal that every driver team should work toward. But my proposal was not intended to make that a requirement. You can continue to get your commits in with no additional testing on your part, by just affirming for each release that you're OK with what the stable-branch maintainer has put together. When I phrase things that way, does it seem more reasonable to you? And if this feels like a more bureaucratic means for doing nothing effectually different than what we did before, please humor me. I think it can help reduce some psychological pressure for the release maintainer. (Maybe I'm just a wimp, but it's been tough at times to trust myself to resolve conflicts in code that I know I don't have any way to test. I do ask for help if the conflict looks really messy. But I don't like to bother people for things that look trivial. And even the trivial, manual conflict resolution can give me misgivings that I might be breaking the release.) -Carl pgpN7MeBSwt6B.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: Don't declare variables in for-loop declaration.
On 07/15/2014 01:14 PM, Matt Turner wrote: Reported-by: Brian Paul --- src/glsl/list.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/glsl/list.h b/src/glsl/list.h index 3cc48ce..3ee6cda 100644 --- a/src/glsl/list.h +++ b/src/glsl/list.h @@ -411,9 +411,9 @@ static inline unsigned exec_list_length(const struct exec_list *list) { unsigned size = 0; + struct exec_node *node; - for (struct exec_node *node = list->head; node->next != NULL; - node = node->next) { + for (node = list->head; node->next != NULL; node = node->next) { size++; } Thanks, that's just what I was testing here. Reviewed-by: Brian Paul ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl: Don't declare variables in for-loop declaration.
Reported-by: Brian Paul --- src/glsl/list.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/glsl/list.h b/src/glsl/list.h index 3cc48ce..3ee6cda 100644 --- a/src/glsl/list.h +++ b/src/glsl/list.h @@ -411,9 +411,9 @@ static inline unsigned exec_list_length(const struct exec_list *list) { unsigned size = 0; + struct exec_node *node; - for (struct exec_node *node = list->head; node->next != NULL; - node = node->next) { + for (node = list->head; node->next != NULL; node = node->next) { size++; } -- 1.8.5.5 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC PATCH 14/16] i965/gen6: Stencil/hiz needs an offset for LOD > 0
On Mon, Jun 2, 2014 at 3:00 AM, Pohjolainen, Topi wrote: > On Thu, May 29, 2014 at 01:53:53PM -0700, Jordan Justen wrote: >> Since gen6 stencil only supports LOD0, we need to program an offset to >> the LOD when emitting the stencil/hiz. >> >> Signed-off-by: Jordan Justen >> --- >> src/mesa/drivers/dri/i965/gen6_blorp.cpp | 10 - >> src/mesa/drivers/dri/i965/gen6_depth_state.c | 32 >> ++-- >> 2 files changed, 39 insertions(+), 3 deletions(-) >> >> diff --git a/src/mesa/drivers/dri/i965/gen6_blorp.cpp >> b/src/mesa/drivers/dri/i965/gen6_blorp.cpp >> index 7be8ccd..920f435 100644 >> --- a/src/mesa/drivers/dri/i965/gen6_blorp.cpp >> +++ b/src/mesa/drivers/dri/i965/gen6_blorp.cpp >> @@ -872,13 +872,21 @@ gen6_blorp_emit_depth_stencil_config(struct >> brw_context *brw, >> /* 3DSTATE_HIER_DEPTH_BUFFER */ >> { >>struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_mt; >> + uint32_t offset = 0; >> + >> + if (hiz_mt->array_spacing_lod0) { >> + offset = intel_miptree_get_aligned_offset(hiz_mt, >> + >> hiz_mt->level[lod].level_x, >> + >> hiz_mt->level[lod].level_y, > > These lines are longer than 80 columns. > >> + false); >> + } >> >>BEGIN_BATCH(3); >>OUT_BATCH((_3DSTATE_HIER_DEPTH_BUFFER << 16) | (3 - 2)); >>OUT_BATCH(hiz_mt->pitch - 1); >>OUT_RELOC(hiz_mt->bo, >> I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, >> -0); >> +offset); >>ADVANCE_BATCH(); >> } >> >> diff --git a/src/mesa/drivers/dri/i965/gen6_depth_state.c >> b/src/mesa/drivers/dri/i965/gen6_depth_state.c >> index 3ef2fab..8fcc0a8 100644 >> --- a/src/mesa/drivers/dri/i965/gen6_depth_state.c >> +++ b/src/mesa/drivers/dri/i965/gen6_depth_state.c >> @@ -271,12 +271,21 @@ gen6_emit_depth_stencil_hiz(struct brw_context *brw, >>/* Emit hiz buffer. */ >>if (hiz) { >> struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_mt; >> + uint32_t offset = 0; >> + >> + if (hiz_mt->array_spacing_lod0) { >> +offset = intel_miptree_get_aligned_offset(hiz_mt, >> + >> hiz_mt->level[lod].level_x, >> + >> hiz_mt->level[lod].level_y, > > Same here. > >> + false); >> + } >> + >>BEGIN_BATCH(3); >>OUT_BATCH((_3DSTATE_HIER_DEPTH_BUFFER << 16) | (3 - 2)); >>OUT_BATCH(hiz_mt->pitch - 1); >>OUT_RELOC(hiz_mt->bo, >> I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, >> -0); >> +offset); >>ADVANCE_BATCH(); >>} else { >>BEGIN_BATCH(3); >> @@ -288,6 +297,25 @@ gen6_emit_depth_stencil_hiz(struct brw_context *brw, >> >>/* Emit stencil buffer. */ >>if (separate_stencil) { >> + uint32_t offset = 0; >> + >> + if (stencil_mt->array_spacing_lod0) { >> +if (stencil_mt->format == MESA_FORMAT_S_UINT8) { >> + /* Note: we can't compute the stencil offset using >> +* intel_region_get_aligned_offset(), because stencil_region >> claims > > This line is longer than 80 columns. > >> +* that the region is untiled even though it's W tiled. >> +*/ >> + offset = >> + stencil_mt->level[lod].level_y * stencil_mt->pitch + >> + stencil_mt->level[lod].level_x * 64; > > Why is the x-coordinate multiplied? And I don't think the result is aligned > to tile boundary. Hmm, I got this from brw_misc_state.c. So far, stencil breaks if I try anything other than this code. :( -Jordan >> +} else { >> + offset = intel_miptree_get_aligned_offset(stencil_mt, >> + >> stencil_mt->level[lod].level_x, >> + >> stencil_mt->level[lod].level_y, > > These lines are also longer than 80 columns. > >> + false); >> +} >> + } >> + >>BEGIN_BATCH(3); >>OUT_BATCH((_3DSTATE_STENCIL_BUFFER << 16) | (3 - 2)); >> /* The stencil buffer has quirky pitch requirements. From Vol 2a, >> @@ -298,7 +326,7 @@ gen6_emit_depth_stencil_hiz(struct brw_context *brw, >>OUT_BATCH(2 * stencil_mt->pitch - 1); >>OUT_RELOC(stencil_mt->bo, >> I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, >> -0); >> +offset); >>ADVANCE_BATCH(); >>} else { >>BEGIN_BATCH(3); >> -- >> 2.0.0.rc4 >> >> ___ >> mesa-dev mailing
Re: [Mesa-dev] [PATCH 0/3] various exec_list things
On Tue, Jul 8, 2014 at 12:20 PM, Connor Abbott wrote: > This series adds a couple things I need to exec_list for my work, and > does some cleanups made possible. Only compile tested on i965. > > Connor Abbott (3): > exec_list: add a prepend function > exec_list: add a function to count the size of a list > exec_list: make various places use the new get_size() method I fixed up a few small things and renamed get_size -> length and pushed these. Thanks! ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/4] i965: Silence many unused parameter warnings
On Tue, Jul 15, 2014 at 10:56 AM, Ian Romanick wrote: > From: Ian Romanick > > brw_inst.h: In function 'brw_inst_set_src1_vstride': > brw_inst.h:118:76: warning: unused parameter 'brw' [-Wunused-parameter] > > Signed-off-by: Ian Romanick > --- > src/mesa/drivers/dri/i965/brw_inst.h | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/src/mesa/drivers/dri/i965/brw_inst.h > b/src/mesa/drivers/dri/i965/brw_inst.h > index e880c9f..719ac8e 100644 > --- a/src/mesa/drivers/dri/i965/brw_inst.h > +++ b/src/mesa/drivers/dri/i965/brw_inst.h > @@ -56,6 +56,7 @@ brw_inst_set_##name(const struct brw_context *brw, > \ > brw_inst *inst, uint64_t v) \ > { \ > assert(assertions);\ > + (void) brw;\ > brw_inst_set_bits(inst, high, low, v); \ > } \ > static inline uint64_t\ > @@ -63,6 +64,7 @@ brw_inst_##name(const struct brw_context *brw, > \ > brw_inst *inst) \ > { \ > assert(assertions);\ > + (void) brw;\ > return brw_inst_bits(inst, high, low); \ > } > > @@ -306,12 +308,14 @@ brw_inst_set_##name(const struct brw_context *brw, > brw_inst *inst, int16_t v) \ > assert(assertions); > \ > assert(v <= (1 << 16) - 1); > \ > assert(v > -(1 << 16)); > \ > + (void) brw; > \ > brw_inst_set_bits(inst, high, low, (uint16_t) v); > \ > } > \ > static inline int16_t > \ > brw_inst_##name(const struct brw_context *brw, brw_inst *inst) > \ > { > \ > assert(assertions); > \ > + (void) brw; > \ > return brw_inst_bits(inst, high, low); > \ > } > > @@ -544,12 +548,14 @@ F(pi_message_data, MD(7), MD(0)) > static inline int > brw_inst_imm_d(const struct brw_context *brw, brw_inst *insn) > { > + (void) brw; > return brw_inst_bits(insn, 127, 96); > } > > static inline unsigned > brw_inst_imm_ud(const struct brw_context *brw, brw_inst *insn) > { > + (void) brw; > return brw_inst_bits(insn, 127, 96); > } > > @@ -557,6 +563,7 @@ static inline float > brw_inst_imm_f(const struct brw_context *brw, brw_inst *insn) > { > fi_type ft; > + (void) brw; > ft.u = brw_inst_bits(insn, 127, 96); > return ft.f; > } > @@ -565,6 +572,7 @@ static inline void > brw_inst_set_imm_d(const struct brw_context *brw, > brw_inst *insn, int value) > { > + (void) brw; > return brw_inst_set_bits(insn, 127, 96, value); > } > > @@ -572,6 +580,7 @@ static inline void > brw_inst_set_imm_ud(const struct brw_context *brw, > brw_inst *insn, unsigned value) > { > + (void) brw; > return brw_inst_set_bits(insn, 127, 96, value); > } > > @@ -580,6 +589,7 @@ brw_inst_set_imm_f(const struct brw_context *brw, > brw_inst *insn, float value) > { > fi_type ft; > + (void) brw; > ft.f = value; > brw_inst_set_bits(insn, 127, 96, ft.u); > } > -- Series is Reviewed-by: Matt Turner You could just drop the brw from the function declaration, and avoid the (void)brw. That might be nicer. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 5/6] glsl: Plumb through UBO array indexing expressions
On Sunday, July 13, 2014 01:52:00 PM Chris Forbes wrote: > Signed-off-by: Chris Forbes > --- > src/glsl/lower_ubo_reference.cpp | 50 > 1 file changed, 30 insertions(+), 20 deletions(-) > > diff --git a/src/glsl/lower_ubo_reference.cpp b/src/glsl/lower_ubo_reference.cpp > index c49ae34..80edd93 100644 > --- a/src/glsl/lower_ubo_reference.cpp > +++ b/src/glsl/lower_ubo_reference.cpp > @@ -69,9 +69,10 @@ public: > * \c UniformBlocks array. > */ > static const char * > -interface_field_name(void *mem_ctx, char *base_name, ir_dereference *d) > +interface_field_name(void *mem_ctx, char *base_name, ir_dereference *d, ir_rvalue **array_index) Maybe call this nonconst_block_index or nonconst_array_index or such? It doesn't seem to be set in the constant-indexed case, and the caller uses whether it was set to determine whether there was a non-constant array index. Whatever you think is best. This series looks good to me: Reviewed-by: Kenneth Graunke However, you might ping Ian - I know he's been working on a bunch of bug fixes in the lower_ubo_reference code. He's also more familiar with this code than I am. signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Fwd: [PATCH 2/6] glsl: Mark entire UBO array active if indexed with non-constant.
On Tuesday, July 15, 2014 10:28:07 PM Chris Forbes wrote: > Accidentally replied privately only, sorry. > > -- Forwarded message -- > From: Chris Forbes > Date: Tue, Jul 15, 2014 at 10:27 PM > Subject: Re: [Mesa-dev] [PATCH 2/6] glsl: Mark entire UBO array active > if indexed with non-constant. > To: Ilia Mirkin > > > On Tue, Jul 15, 2014 at 10:20 PM, Ilia Mirkin wrote: > > On Sat, Jul 12, 2014 at 9:51 PM, Chris Forbes wrote: > >> Without doing a lot more work, we have no idea which indices may > >> be used at runtime, so just mark them all. > >> > >> Signed-off-by: Chris Forbes > >> --- > >> src/glsl/link_uniform_block_active_visitor.cpp | 51 -- > >> 1 file changed, 32 insertions(+), 19 deletions(-) > >> > >> diff --git a/src/glsl/link_uniform_block_active_visitor.cpp b/src/glsl/link_uniform_block_active_visitor.cpp > >> index d19ce20..4bf7349 100644 > >> --- a/src/glsl/link_uniform_block_active_visitor.cpp > >> +++ b/src/glsl/link_uniform_block_active_visitor.cpp > >> @@ -109,32 +109,45 @@ link_uniform_block_active_visitor::visit_enter(ir_dereference_array *ir) > >> assert((b->num_array_elements == 0) == (b->array_elements == NULL)); > >> assert(b->type != NULL); > >> > >> - /* Determine whether or not this array index has already been added to the > >> -* list of active array indices. At this point all constant folding must > >> -* have occured, and the array index must be a constant. > >> -*/ > >> ir_constant *c = ir->array_index->as_constant(); > >> - assert(c != NULL); > >> > >> - const unsigned idx = c->get_uint_component(0); > >> + if (c) { > >> + /* Index is a constant, so mark just that element used, if not already */ > >> + const unsigned idx = c->get_uint_component(0); > >> > >> - unsigned i; > >> - for (i = 0; i < b->num_array_elements; i++) { > >> - if (b->array_elements[i] == idx) > >> -break; > >> - } > >> + unsigned i; > >> + for (i = 0; i < b->num_array_elements; i++) { > >> + if (b->array_elements[i] == idx) > >> +break; > >> + } > >> > >> - assert(i <= b->num_array_elements); > >> + assert(i <= b->num_array_elements); > >> > >> - if (i == b->num_array_elements) { > >> - b->array_elements = reralloc(this->mem_ctx, > >> - b->array_elements, > >> - unsigned, > >> - b->num_array_elements + 1); > >> + if (i == b->num_array_elements) { > >> + b->array_elements = reralloc(this->mem_ctx, > >> + b->array_elements, > >> + unsigned, > >> + b->num_array_elements + 1); > >> > >> - b->array_elements[b->num_array_elements] = idx; > >> + b->array_elements[b->num_array_elements] = idx; > >> > >> - b->num_array_elements++; > >> + b->num_array_elements++; > >> + } > >> + } else { > >> + /* The array index is not a constant, so mark the entire array used. */ > >> + assert(b->type->is_array()); > >> + if (b->num_array_elements < b->type->length) { > > > > This condition is a little different than the other case. Is this > > basically to cover the first time that the array is indexed > > dynamically? > > Right, so if this check fails, then we've already marked every element > and so don't need to do anything. > Otherwise, we need to expand to the full size of the array and mark > everything in one go. > > > > >> + b->num_array_elements = b->type->length; > >> + b->array_elements = reralloc(this->mem_ctx, > >> + b->array_elements, > >> + unsigned, > >> + b->num_array_elements); > >> + > >> + unsigned i; > > > > You'll get yelled at by the MSVC people for this... needs to be at the > > beginning of a block. > > This file is C++, so that shouldn't be a problem. I'll happily move it > to the start of the block though. Personally, I'd just fold it into the for loop - in the other case, we wanted to use 'i' after the loop. But it doesn't really matter. > > > >> + for (i = 0; i < b->num_array_elements; i++) { > >> +b->array_elements[i] = i; > >> + } > > > > I think this is fine, but just want to raise the issue of a situation > > where you start out with some const accesses, and then do a nonconst > > access. The nonconst will erase all the old idx's but since they only > > exist in the array_elements list (at this point in the compilation), > > the reassignment won't cause any issues, right? > > That's the idea. I think it's safe (or alternatively, I've completely > misunderstood something) but more scrutiny wouldn't hurt :) Yeah, this looks fine to me. It looks like nothing uses this info until the pass is compl
[Mesa-dev] [PATCH 4/4] glsl: Fix bad indentation
From: Ian Romanick Signed-off-by: Ian Romanick --- src/glsl/ast_to_hir.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp index 885bee5..888f5ec 100644 --- a/src/glsl/ast_to_hir.cpp +++ b/src/glsl/ast_to_hir.cpp @@ -5137,7 +5137,7 @@ ast_process_structure_or_interface_block(exec_list *instructions, fields[i].row_major = false; } - i++; + i++; } } -- 1.8.1.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 0/4] Minor warning clean-ups
These are just some warning clean ups that I made while doing other work. I was going to send them with the other work, but that series is taking longer (and getting longer) than I had hoped. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/4] i965: Silence many unused parameter warnings
From: Ian Romanick brw_inst.h: In function 'brw_inst_set_src1_vstride': brw_inst.h:118:76: warning: unused parameter 'brw' [-Wunused-parameter] Signed-off-by: Ian Romanick --- src/mesa/drivers/dri/i965/brw_inst.h | 10 ++ 1 file changed, 10 insertions(+) diff --git a/src/mesa/drivers/dri/i965/brw_inst.h b/src/mesa/drivers/dri/i965/brw_inst.h index e880c9f..719ac8e 100644 --- a/src/mesa/drivers/dri/i965/brw_inst.h +++ b/src/mesa/drivers/dri/i965/brw_inst.h @@ -56,6 +56,7 @@ brw_inst_set_##name(const struct brw_context *brw, \ brw_inst *inst, uint64_t v) \ { \ assert(assertions);\ + (void) brw;\ brw_inst_set_bits(inst, high, low, v); \ } \ static inline uint64_t\ @@ -63,6 +64,7 @@ brw_inst_##name(const struct brw_context *brw, \ brw_inst *inst) \ { \ assert(assertions);\ + (void) brw;\ return brw_inst_bits(inst, high, low); \ } @@ -306,12 +308,14 @@ brw_inst_set_##name(const struct brw_context *brw, brw_inst *inst, int16_t v) \ assert(assertions);\ assert(v <= (1 << 16) - 1);\ assert(v > -(1 << 16));\ + (void) brw;\ brw_inst_set_bits(inst, high, low, (uint16_t) v); \ } \ static inline int16_t \ brw_inst_##name(const struct brw_context *brw, brw_inst *inst)\ { \ assert(assertions);\ + (void) brw;\ return brw_inst_bits(inst, high, low); \ } @@ -544,12 +548,14 @@ F(pi_message_data, MD(7), MD(0)) static inline int brw_inst_imm_d(const struct brw_context *brw, brw_inst *insn) { + (void) brw; return brw_inst_bits(insn, 127, 96); } static inline unsigned brw_inst_imm_ud(const struct brw_context *brw, brw_inst *insn) { + (void) brw; return brw_inst_bits(insn, 127, 96); } @@ -557,6 +563,7 @@ static inline float brw_inst_imm_f(const struct brw_context *brw, brw_inst *insn) { fi_type ft; + (void) brw; ft.u = brw_inst_bits(insn, 127, 96); return ft.f; } @@ -565,6 +572,7 @@ static inline void brw_inst_set_imm_d(const struct brw_context *brw, brw_inst *insn, int value) { + (void) brw; return brw_inst_set_bits(insn, 127, 96, value); } @@ -572,6 +580,7 @@ static inline void brw_inst_set_imm_ud(const struct brw_context *brw, brw_inst *insn, unsigned value) { + (void) brw; return brw_inst_set_bits(insn, 127, 96, value); } @@ -580,6 +589,7 @@ brw_inst_set_imm_f(const struct brw_context *brw, brw_inst *insn, float value) { fi_type ft; + (void) brw; ft.f = value; brw_inst_set_bits(insn, 127, 96, ft.u); } -- 1.8.1.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/4] i965: Silence unused parameter warning
From: Ian Romanick brw_fs_visitor.cpp:2400:1: warning: unused parameter 'ir' [-Wunused-parameter] Signed-off-by: Ian Romanick --- src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp index 90bf3fa..3b03eef 100644 --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp @@ -2496,7 +2496,7 @@ fs_visitor::visit(ir_call *ir) } void -fs_visitor::visit(ir_return *ir) +fs_visitor::visit(ir_return *) { unreachable("FINISHME"); } -- 1.8.1.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/4] i965: Silence 'comparison is always true' warning
From: Ian Romanick The parameter is an int16_t, and we're check that it's value will fit in 16-bits. Yes, the value that is stored in 16-bits will surely fit in 16-bits. brw_inst.h: In function 'brw_inst_set_gen6_jump_count': brw_inst.h:321:66: warning: comparison is always true due to limited range of data type [-Wtype-limits] brw_inst.h:321:66: warning: comparison is always true due to limited range of data type [-Wtype-limits] Signed-off-by: Ian Romanick --- src/mesa/drivers/dri/i965/brw_inst.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_inst.h b/src/mesa/drivers/dri/i965/brw_inst.h index 719ac8e..c1ff10d 100644 --- a/src/mesa/drivers/dri/i965/brw_inst.h +++ b/src/mesa/drivers/dri/i965/brw_inst.h @@ -306,8 +306,6 @@ static inline void \ brw_inst_set_##name(const struct brw_context *brw, brw_inst *inst, int16_t v) \ { \ assert(assertions);\ - assert(v <= (1 << 16) - 1);\ - assert(v > -(1 << 16));\ (void) brw;\ brw_inst_set_bits(inst, high, low, (uint16_t) v); \ } \ -- 1.8.1.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 00/26] GLSL memory diet
On Mon, Jul 14, 2014 at 3:48 PM, Ian Romanick wrote: > Most of these patches have been sent to the list already in one form or > another. There are a few changes, removals, and additions. The series > has also been re-ordered. > > - The extra memory accounting code has been removed. This was suggested > by Ken. Instead, all memory usage data is from Valgrind massiv. > > - The "store short names where padding would be" code is still there, > but many of the patches to shorten the names of temporaries has been > removed. Those patches became unnecessary because... > > - In release builds (when MESA_GLSL is also unset), all temporaries get > the name "compiler_temp" that is in static storage. > > - A small set of names that frequently occur in shader-db are kept in > static storage. When a variable is created with one of these names, > no additional storage is allocated. We could probably give local > variables and post-linking globals the same treatment as temporaries, > but a lot of the frequently occuring names (that don't fit in padding) > are uniforms or shader inputs. These need to keep their names. > > I suspect the last six patches will be contentious. I'd like to get the > first 20 (or at least the first two!) reviewed and landed sooner... then > we can debate the last few more leisurely. The first 9 (so far) are Reviewed-by: Matt Turner ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 09/10] glsl: Optimize A - neg(B) == A + B
I haven't really looked at the other patches yet, but the subject of this one caught my eye. I'm not sure this is useful. I believe we lower A-B to A+neg(B) for all architectures. I'm pretty sure we also already have a pass that converts A+neg(neg(B)) to A+B. Did you write any shaders to try to tickle this code? What did something like the following produce before and after this change? uniform vec4 a; uniform vec4 b; void main() { gl_Position = a - (-b); } For the neg(A)-B case, I think the existing code is better for most architectures. You end up with a single ADD r2, -r1, -r0 instruction currently. With this change you would potentially need an extra move to apply the negation. I believe at least the i965 backend has a pass to push those negations "down" the tree... effectively undoing what your pass does. :) On 07/14/2014 03:22 PM, thomashellan...@gmail.com wrote: > From: Thomas Helland > > ...and neg(A) - B == neg(A + B) > > Signed-off-by: Thomas Helland > --- > src/glsl/opt_algebraic.cpp | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/src/glsl/opt_algebraic.cpp b/src/glsl/opt_algebraic.cpp > index 2361d0f..fba9de6 100644 > --- a/src/glsl/opt_algebraic.cpp > +++ b/src/glsl/opt_algebraic.cpp > @@ -454,6 +454,12 @@ ir_algebraic_visitor::handle_expression(ir_expression > *ir) >/* X - X == 0 */ >if (ir->operands[0]->equals(ir->operands[1])) > return ir_constant::zero(ir, ir->type); > + /* A - neg(B) = A + B */ > + if (op_expr[1] && op_expr[1]->operation == ir_unop_neg) > + return add(ir->operands[0], op_expr[1]->operands[0]); > + /* neg(A) - B = neg(A + B) */ > + if (op_expr[0] && op_expr[0]->operation == ir_unop_neg) > + return neg(add(op_expr[0]->operands[0], ir->operands[1])); >break; > > case ir_binop_mul: > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] util: Add util_memcpy_cpu_to_le()
On Tue, Jul 15, 2014 at 11:19 AM, Tom Stellard wrote: > --- > src/gallium/auxiliary/util/u_math.h | 22 ++ > src/gallium/drivers/radeonsi/si_shader.c | 8 +--- > 2 files changed, 23 insertions(+), 7 deletions(-) > > diff --git a/src/gallium/auxiliary/util/u_math.h > b/src/gallium/auxiliary/util/u_math.h > index b9ed197..cd3cf04 100644 > --- a/src/gallium/auxiliary/util/u_math.h > +++ b/src/gallium/auxiliary/util/u_math.h > @@ -812,6 +812,28 @@ util_bswap16(uint16_t n) >(n << 8); > } > > +static INLINE void* > +util_memcpy_cpu_to_le(void *dest, void *src, size_t n) > +{ > +#ifdef PIPE_ARCH_BIG_ENDIAN > + size_t i, e; > + for (i = 0, e = n % 8; i < e; i++) { > + char *d = (char*)dest; > + char *s = (char*)src; > + d[i] = s[e - i - 1]; > + } > + dest += i; > + n -= i; > + for (i = 0, e = n / 8; i < e; i++) { > + uint64_t *d = (uint64_t*)dest; > + uint64_t *s = (uint64_t*)src; > + d[i] = util_bswap64(s[e - i - 1]); > + } > Doesn't this reverse all of the byte (as if it were a list) without preserving word boundaries? e.g. |a, b, c, d | e, f, g, h | i, j, k, l | m, n, o, p | -> |p, o, n, m | l, j, k, i | h, g, f, e | d, c, b, a | The old code did something like this, didn't it?: |a, b, c, d | e, f, g, h | i, j, k, l | m, n, o, p | -> |d, c, b, a | h, g, f, e | l, k, j, i | p, o, n, m | I don't know which is correct, but it does seem like a behavior change. Or am I misreading the code? + return dest; > +#else > + return memcpy(dest, src, n); > +#endif > +} > > /** > * Clamp X to [MIN, MAX]. > diff --git a/src/gallium/drivers/radeonsi/si_shader.c > b/src/gallium/drivers/radeonsi/si_shader.c > index f0650f4..6f0504b 100644 > --- a/src/gallium/drivers/radeonsi/si_shader.c > +++ b/src/gallium/drivers/radeonsi/si_shader.c > @@ -2559,13 +2559,7 @@ int si_compile_llvm(struct si_context *sctx, struct > si_pipe_shader *shader, > } > > ptr = (uint32_t*)sctx->b.ws->buffer_map(shader->bo->cs_buf, > sctx->b.rings.gfx.cs, PIPE_TRANSFER_WRITE); > - if (SI_BIG_ENDIAN) { > - for (i = 0; i < binary.code_size / 4; ++i) { > - ptr[i] = > util_cpu_to_le32((*(uint32_t*)(binary.code + i*4))); > - } > - } else { > - memcpy(ptr, binary.code, binary.code_size); > - } > + util_memcpy_cpu_to_le(ptr, binary.code, binary.code_size); > sctx->b.ws->buffer_unmap(shader->bo->cs_buf); > > free(binary.code); > -- > 1.8.1.5 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 81174] Gallium: GL_LINE_LOOP broken with more than 512 points
https://bugs.freedesktop.org/show_bug.cgi?id=81174 --- Comment #13 from Roland Scheidegger --- (In reply to comment #12) > (In reply to comment #9) > > (In reply to comment #8) > > > Line loops aren't so critical, since they are rarely used. But what about > > > triangle strips... every split kills 2 triangles... > > > > That shouldn't happen, the code is supposed to copy over the necessary two > > tris. > > What about primitive winding/facing? If you wrap a triangle strip and start > on an odd triangle, you'll end up with correct front/back faces. If you > start on an even triangle, the facing is inverted, isn't it? This should also be handled correctly I believe. See the comments about "parity issue" in vbo_copy_vertices(). Essentially the code ensures the new buffer always starts on an odd triangle. (Though on a quick look I wouldn't be able to tell if it's all correct but it probably is as otherwise there'd probably have been bug reports about it.) -- You are receiving this mail because: You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] radeonsi: Read rodata from ELF and append it to the end of shaders
The is used for programs that have arrays of constants that are accessed using dynamic indices. The shader will compute the base address of the constants and then access them using SMRD instructions. --- src/gallium/drivers/radeon/r600_pipe_common.h | 5 + src/gallium/drivers/radeon/radeon_elf_util.c | 5 + src/gallium/drivers/radeonsi/si_shader.c | 16 +--- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/gallium/drivers/radeon/r600_pipe_common.h b/src/gallium/drivers/radeon/r600_pipe_common.h index d82adf5..8f1a0a5 100644 --- a/src/gallium/drivers/radeon/r600_pipe_common.h +++ b/src/gallium/drivers/radeon/r600_pipe_common.h @@ -108,6 +108,11 @@ struct radeon_shader_binary { unsigned char *config; unsigned config_size; + /** Constant data accessed by the shader. This will be uploaded +* into a constant buffer. */ + unsigned char *rodata; + unsigned rodata_size; + /** Set to 1 if the disassembly for this binary has been dumped to * stderr. */ int disassembled; diff --git a/src/gallium/drivers/radeon/radeon_elf_util.c b/src/gallium/drivers/radeon/radeon_elf_util.c index 7d92962..7c5f93e 100644 --- a/src/gallium/drivers/radeon/radeon_elf_util.c +++ b/src/gallium/drivers/radeon/radeon_elf_util.c @@ -80,6 +80,11 @@ void radeon_elf_read(const char *elf_data, unsigned elf_size, fprintf(stderr, "\nShader Disassembly:\n\n"); fprintf(stderr, "%.*s\n", (int)section_data->d_size, (char *)section_data->d_buf); + } else if (!strncmp(name, ".rodata", 7)) { + section_data = elf_getdata(section, section_data); + binary->rodata_size = section_data->d_size; + binary->rodata = MALLOC(binary->rodata_size * sizeof(unsigned char)); + memcpy(binary->rodata, section_data->d_buf, binary->rodata_size); } } diff --git a/src/gallium/drivers/radeonsi/si_shader.c b/src/gallium/drivers/radeonsi/si_shader.c index 6f0504b..f07dbab 100644 --- a/src/gallium/drivers/radeonsi/si_shader.c +++ b/src/gallium/drivers/radeonsi/si_shader.c @@ -2499,11 +2499,12 @@ int si_compile_llvm(struct si_context *sctx, struct si_pipe_shader *shader, { unsigned r; /* llvm_compile result */ unsigned i; - uint32_t *ptr; + unsigned char *ptr; struct radeon_shader_binary binary; bool dump = r600_can_dump_shader(&sctx->screen->b, shader->selector ? shader->selector->tokens : NULL); const char * gpu_family = r600_get_llvm_processor_name(sctx->screen->b.family); + unsigned code_size; /* Use LLVM to compile shader */ memset(&binary, 0, sizeof(binary)); @@ -2551,19 +2552,28 @@ int si_compile_llvm(struct si_context *sctx, struct si_pipe_shader *shader, } /* copy new shader */ + code_size = binary.code_size + binary.rodata_size; r600_resource_reference(&shader->bo, NULL); shader->bo = si_resource_create_custom(sctx->b.b.screen, PIPE_USAGE_IMMUTABLE, - binary.code_size); + code_size); if (shader->bo == NULL) { return -ENOMEM; } - ptr = (uint32_t*)sctx->b.ws->buffer_map(shader->bo->cs_buf, sctx->b.rings.gfx.cs, PIPE_TRANSFER_WRITE); + ptr = sctx->b.ws->buffer_map(shader->bo->cs_buf, sctx->b.rings.gfx.cs, + PIPE_TRANSFER_WRITE); util_memcpy_cpu_to_le(ptr, binary.code, binary.code_size); + /* Copy read only data if any. */ + if (binary.rodata_size > 0) { + ptr += binary.code_size; + util_memcpy_cpu_to_le(ptr, binary.rodata, binary.rodata_size); + } + sctx->b.ws->buffer_unmap(shader->bo->cs_buf); free(binary.code); free(binary.config); + free(binary.rodata); return r; } -- 1.8.1.5 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] util: Add util_memcpy_cpu_to_le()
--- src/gallium/auxiliary/util/u_math.h | 22 ++ src/gallium/drivers/radeonsi/si_shader.c | 8 +--- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/gallium/auxiliary/util/u_math.h b/src/gallium/auxiliary/util/u_math.h index b9ed197..cd3cf04 100644 --- a/src/gallium/auxiliary/util/u_math.h +++ b/src/gallium/auxiliary/util/u_math.h @@ -812,6 +812,28 @@ util_bswap16(uint16_t n) (n << 8); } +static INLINE void* +util_memcpy_cpu_to_le(void *dest, void *src, size_t n) +{ +#ifdef PIPE_ARCH_BIG_ENDIAN + size_t i, e; + for (i = 0, e = n % 8; i < e; i++) { + char *d = (char*)dest; + char *s = (char*)src; + d[i] = s[e - i - 1]; + } + dest += i; + n -= i; + for (i = 0, e = n / 8; i < e; i++) { + uint64_t *d = (uint64_t*)dest; + uint64_t *s = (uint64_t*)src; + d[i] = util_bswap64(s[e - i - 1]); + } + return dest; +#else + return memcpy(dest, src, n); +#endif +} /** * Clamp X to [MIN, MAX]. diff --git a/src/gallium/drivers/radeonsi/si_shader.c b/src/gallium/drivers/radeonsi/si_shader.c index f0650f4..6f0504b 100644 --- a/src/gallium/drivers/radeonsi/si_shader.c +++ b/src/gallium/drivers/radeonsi/si_shader.c @@ -2559,13 +2559,7 @@ int si_compile_llvm(struct si_context *sctx, struct si_pipe_shader *shader, } ptr = (uint32_t*)sctx->b.ws->buffer_map(shader->bo->cs_buf, sctx->b.rings.gfx.cs, PIPE_TRANSFER_WRITE); - if (SI_BIG_ENDIAN) { - for (i = 0; i < binary.code_size / 4; ++i) { - ptr[i] = util_cpu_to_le32((*(uint32_t*)(binary.code + i*4))); - } - } else { - memcpy(ptr, binary.code, binary.code_size); - } + util_memcpy_cpu_to_le(ptr, binary.code, binary.code_size); sctx->b.ws->buffer_unmap(shader->bo->cs_buf); free(binary.code); -- 1.8.1.5 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 81174] Gallium: GL_LINE_LOOP broken with more than 512 points
https://bugs.freedesktop.org/show_bug.cgi?id=81174 --- Comment #12 from Marek Olšák --- (In reply to comment #9) > (In reply to comment #8) > > Line loops aren't so critical, since they are rarely used. But what about > > triangle strips... every split kills 2 triangles... > > That shouldn't happen, the code is supposed to copy over the necessary two > tris. What about primitive winding/facing? If you wrap a triangle strip and start on an odd triangle, you'll end up with correct front/back faces. If you start on an even triangle, the facing is inverted, isn't it? -- You are receiving this mail because: You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 81174] Gallium: GL_LINE_LOOP broken with more than 512 points
https://bugs.freedesktop.org/show_bug.cgi?id=81174 --- Comment #11 from Marek Olšák --- (In reply to comment #10) > What I don't understand yet is why the effect is not stable (if my example > is repainted several times, the wrap appears at different positions over > time). The wrap is done when the buffer is full, not at the beginning of a frame. If the buffer is half full when a frame starts, the wrap will occur earlier. -- You are receiving this mail because: You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/6] radeonsi: move sampler descriptors from IB to memory
On Tue, Jul 15, 2014 at 11:53 AM, Michel Dänzer wrote: > On 13.07.2014 01:35, Marek Olšák wrote: >> >> Border colors have been broken if texturing from multiple shader stages is >> used. This patch doesn't change that. > > [...] > >> +/* Upload border colors and update the pointers in resource descriptors. >> + * There can only be 4096 border colors per context. >> + * >> + * XXX: This is broken if sampler states are bound to multiple shader >> stages, >> + * because TA_BC_BASE_ADDR is shared by all of them and we overwrite it >> + * for stages which were set earlier. This is also broken for >> + * fine-grained sampler state updates. >> + */ > > I don't think that's accurate, as the BO for storing the border colours > is per-context, not per-shader-stage. Ah yes. The problem only occurs when the BO is reallocated. Consider this: set_sampler_states(SHADER_VERTEX) // This sets TA_BC_BASE_ADDR and sets the border color pointers // in the sampler descriptors. The pointers are relative to the base address. set_sampler_states(SHADER_FRAGMENT) // If the buffer is reallocated, TA_BC_BASE_ADDR is changed. // All border color pointers for fragment sampler states are set and valid. // All border color pointers for vertex sampler states are now invalid, // because TA_BC_BASE_ADDR has been changed. The reallocation can also happen halfway through setting up border colors, e.g. you set border colors 0,1,2,3, then you have to reallocate, and then you set border colors 4,5,6,7, so the first four border color pointers end up being incorrect, because the previous buffer has been thrown away. > > AFAICS the only thing missing is flushing the command stream when we > need to allocate a new BO for that. Might also be a good idea to always > allocate a new BO for this after a flush. I don't think so. I don't understand why a flush would be needed here. We don't have to flush the command stream or allocate a new BO after a flush, because the BO is mapped without synchronization. The only thing we need to do is to flush the correct read caches after the upload, but that's usually not needed if you upload to memory which hasn't been used before. I think the proper solution would be to update all border colors for all bound sampler states again when the buffer is reallocated. Also, to prevent frequent reallocations, we can check if the current border color pointer in a sampler state is still valid and if it is, we can skip the upload. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/5] wglinfo: query and report multisample information
Before, we always reported zeros in the multisample columns of the format list. Since PIXELFORMATDESCRIPTOR doesn't have fields for multisample, we use a new format_info structure to extend that type. We can only query this info with the wglGetPixelFormatAttribivARB() function which is part of the WGL_ARB_pixel_format extension. --- src/wgl/wglinfo.c | 237 - 1 file changed, 178 insertions(+), 59 deletions(-) diff --git a/src/wgl/wglinfo.c b/src/wgl/wglinfo.c index 2b2c921..e14ebd6 100644 --- a/src/wgl/wglinfo.c +++ b/src/wgl/wglinfo.c @@ -50,6 +50,21 @@ typedef enum } InfoMode; +static GLboolean have_WGL_ARB_pixel_format; +static GLboolean have_WGL_ARB_multisample; + +static PFNWGLGETPIXELFORMATATTRIBIVARBPROC wglGetPixelFormatAttribivARB_func; + + +/** + * An extension of PIXELFORMATDESCRIPTOR to handle multisample, etc. + */ +struct format_info { + PIXELFORMATDESCRIPTOR pfd; + int sampleBuffers, numSamples; + int transparency; +}; + static LRESULT CALLBACK WndProc(HWND hWnd, @@ -159,6 +174,12 @@ print_screen_info(HDC _hdc, GLboolean limits, GLboolean singleLine) printf("WGL extensions:\n"); print_extension_list(wglExtensions, singleLine); } + if (extension_supported("WGL_ARB_pixel_format", wglExtensions)) { +have_WGL_ARB_pixel_format = GL_TRUE; + } + if (extension_supported("WGL_ARB_multisample", wglExtensions)) { +have_WGL_ARB_multisample = GL_TRUE; + } } #endif printf("OpenGL vendor string: %s\n", glVendor); @@ -208,27 +229,27 @@ visual_render_type_name(BYTE iPixelType) } static void -print_visual_attribs_verbose(int iPixelFormat, LPPIXELFORMATDESCRIPTOR ppfd) +print_visual_attribs_verbose(int iPixelFormat, const struct format_info *info) { printf("Visual ID: %x generic=%d native=%d\n", iPixelFormat, - ppfd->dwFlags & PFD_GENERIC_FORMAT ? 1 : 0, - ppfd->dwFlags & PFD_DRAW_TO_WINDOW ? 1 : 0); + info->pfd.dwFlags & PFD_GENERIC_FORMAT ? 1 : 0, + info->pfd.dwFlags & PFD_DRAW_TO_WINDOW ? 1 : 0); printf("bufferSize=%d level=%d renderType=%s doubleBuffer=%d stereo=%d\n", - 0 /* ppfd->bufferSize */, 0 /* ppfd->level */, - visual_render_type_name(ppfd->iPixelType), - ppfd->dwFlags & PFD_DOUBLEBUFFER ? 1 : 0, - ppfd->dwFlags & PFD_STEREO ? 1 : 0); + 0 /* info->pfd.bufferSize */, 0 /* info->pfd.level */, + visual_render_type_name(info->pfd.iPixelType), + info->pfd.dwFlags & PFD_DOUBLEBUFFER ? 1 : 0, + info->pfd.dwFlags & PFD_STEREO ? 1 : 0); printf("rgba: cRedBits=%d cGreenBits=%d cBlueBits=%d cAlphaBits=%d\n", - ppfd->cRedBits, ppfd->cGreenBits, - ppfd->cBlueBits, ppfd->cAlphaBits); + info->pfd.cRedBits, info->pfd.cGreenBits, + info->pfd.cBlueBits, info->pfd.cAlphaBits); printf("cAuxBuffers=%d cDepthBits=%d cStencilBits=%d\n", - ppfd->cAuxBuffers, ppfd->cDepthBits, ppfd->cStencilBits); + info->pfd.cAuxBuffers, info->pfd.cDepthBits, info->pfd.cStencilBits); printf("accum: cRedBits=%d cGreenBits=%d cBlueBits=%d cAlphaBits=%d\n", - ppfd->cAccumRedBits, ppfd->cAccumGreenBits, - ppfd->cAccumBlueBits, ppfd->cAccumAlphaBits); + info->pfd.cAccumRedBits, info->pfd.cAccumGreenBits, + info->pfd.cAccumBlueBits, info->pfd.cAccumAlphaBits); printf("multiSample=%d multiSampleBuffers=%d\n", - 0 /* ppfd->numSamples */, 0 /* ppfd->numMultisample */); + info->numSamples, info->sampleBuffers); } @@ -242,32 +263,32 @@ print_visual_attribs_short_header(void) static void -print_visual_attribs_short(int iPixelFormat, LPPIXELFORMATDESCRIPTOR ppfd) +print_visual_attribs_short(int iPixelFormat, const struct format_info *info) { char *caveat = "None"; printf("0x%02x %2d %2d %2d %2d %2d %c%c %c %c %2d %2d %2d %2d %2d %2d %2d", iPixelFormat, - ppfd->dwFlags & PFD_GENERIC_FORMAT ? 1 : 0, - ppfd->dwFlags & PFD_DRAW_TO_WINDOW ? 1 : 0, - 0, - 0 /* ppfd->bufferSize */, - 0 /* ppfd->level */, - ppfd->iPixelType == PFD_TYPE_RGBA ? 'r' : ' ', - ppfd->iPixelType == PFD_TYPE_COLORINDEX ? 'c' : ' ', - ppfd->dwFlags & PFD_DOUBLEBUFFER ? 'y' : '.', - ppfd->dwFlags & PFD_STEREO ? 'y' : '.', - ppfd->cRedBits, ppfd->cGreenBits, - ppfd->cBlueBits, ppfd->cAlphaBits, - ppfd->cAuxBuffers, - ppfd->cDepthBits, - ppfd->cStencilBits + info->pfd.dwFlags & PFD_GENERIC_FORMAT ? 1 : 0, + info->pfd.dwFlags & PFD_DRAW_TO_WINDOW ? 1 : 0, + info->transparency, + info->pfd.cColorBits, + 0 /* info->pfd.level */, + info->pfd.iPixelType == PFD_TYPE_RGBA ? 'r' : ' ', + info->pfd.iPixelType == PFD_TYPE_COL
[Mesa-dev] [PATCH 5/5] wglinfo: adjust column spacing for pixel format info
--- src/wgl/wglinfo.c |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/wgl/wglinfo.c b/src/wgl/wglinfo.c index 22d8d64..f628768 100644 --- a/src/wgl/wglinfo.c +++ b/src/wgl/wglinfo.c @@ -347,9 +347,9 @@ print_visual_attribs_verbose(int iPixelFormat, const struct format_info *info) static void print_visual_attribs_short_header(void) { - printf(" visual x bf lv rg d st colorbuffer ax dp st accumbuffer ms cav\n"); - printf(" id gen win sp sz l ci b ro r g b a bf th cl r g b a ns b eat\n"); - printf("---\n"); + printf(" visual x bf lv rg d st colorbuffer ax dp st accumbuffer ms cav\n"); + printf(" id gen win sp sz l ci b ro r g b a bf th cl r g b a ns b eat\n"); + printf("\n"); } @@ -358,7 +358,7 @@ print_visual_attribs_short(int iPixelFormat, const struct format_info *info) { char *caveat = "None"; - printf("0x%02x %2d %2d %2d %2d %2d %c%c %c %c %2d %2d %2d %2d %2d %2d %2d", + printf("0x%02x %2d %2d %2d %3d %2d %c%c %c %c %2d %2d %2d %2d %2d %2d %2d", iPixelFormat, info->pfd.dwFlags & PFD_GENERIC_FORMAT ? 1 : 0, info->pfd.dwFlags & PFD_DRAW_TO_WINDOW ? 1 : 0, -- 1.7.10.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/5] wglinfo: print swap method in print_visual_attribs_verbose()
--- src/wgl/wglinfo.c |7 +++ 1 file changed, 7 insertions(+) diff --git a/src/wgl/wglinfo.c b/src/wgl/wglinfo.c index 16008d2..22d8d64 100644 --- a/src/wgl/wglinfo.c +++ b/src/wgl/wglinfo.c @@ -334,6 +334,13 @@ print_visual_attribs_verbose(int iPixelFormat, const struct format_info *info) info->pfd.cAccumBlueBits, info->pfd.cAccumAlphaBits); printf("multiSample=%d multiSampleBuffers=%d\n", info->numSamples, info->sampleBuffers); + if (info->pfd.dwFlags & PFD_SWAP_EXCHANGE) + printf("swapMethod = Exchange\n"); + else if (info->pfd.dwFlags & PFD_SWAP_COPY) + printf("swapMethod = Copy\n"); + else + printf("swapMethod = Undefined\n"); + } -- 1.7.10.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/5] glxinfo/wglinfo: move argument parsing into common code
--- src/wgl/wglinfo.c | 59 +++--- src/xdemos/glinfo_common.c | 68 +++ src/xdemos/glinfo_common.h | 25 + src/xdemos/glxinfo.c | 85 4 files changed, 113 insertions(+), 124 deletions(-) diff --git a/src/wgl/wglinfo.c b/src/wgl/wglinfo.c index e14ebd6..fe94dcc 100644 --- a/src/wgl/wglinfo.c +++ b/src/wgl/wglinfo.c @@ -42,14 +42,6 @@ #include "glinfo_common.h" -typedef enum -{ - Normal, - Wide, - Verbose -} InfoMode; - - static GLboolean have_WGL_ARB_pixel_format; static GLboolean have_WGL_ARB_multisample; @@ -534,67 +526,26 @@ find_best_visual(HDC hdc) } -static void -usage(void) -{ - printf("Usage: glxinfo [-v] [-t] [-h] [-i] [-b] [-display ]\n"); - printf("\t-v: Print visuals info in verbose form.\n"); - printf("\t-t: Print verbose table.\n"); - printf("\t-h: This information.\n"); - printf("\t-b: Find the 'best' visual and print its number.\n"); - printf("\t-l: Print interesting OpenGL limits.\n"); - printf("\t-s: Print a single extension per line.\n"); -} - int main(int argc, char *argv[]) { HDC hdc; - InfoMode mode = Normal; - GLboolean findBest = GL_FALSE; - GLboolean limits = GL_FALSE; - GLboolean singleLine = GL_FALSE; - int i; + struct options opts; - for (i = 1; i < argc; i++) { - if (strcmp(argv[i], "-t") == 0) { - mode = Wide; - } - else if (strcmp(argv[i], "-v") == 0) { - mode = Verbose; - } - else if (strcmp(argv[i], "-b") == 0) { - findBest = GL_TRUE; - } - else if (strcmp(argv[i], "-l") == 0) { - limits = GL_TRUE; - } - else if (strcmp(argv[i], "-h") == 0) { - usage(); - return 0; - } - else if(strcmp(argv[i], "-s") == 0) { - singleLine = GL_TRUE; - } - else { - printf("Unknown option `%s'\n", argv[i]); - usage(); - return 0; - } - } + parse_args(argc, argv, &opts); hdc = CreateDC(TEXT("DISPLAY"), NULL, NULL, NULL); - if (findBest) { + if (opts.findBest) { int b; b = find_best_visual(hdc); printf("%d\n", b); } else { - print_screen_info(hdc, limits, singleLine); + print_screen_info(hdc, opts.limits, opts.singleLine); printf("\n"); - print_visual_info(hdc, mode); + print_visual_info(hdc, opts.mode); } return 0; diff --git a/src/xdemos/glinfo_common.c b/src/xdemos/glinfo_common.c index 248b937..95ef545 100644 --- a/src/xdemos/glinfo_common.c +++ b/src/xdemos/glinfo_common.c @@ -718,3 +718,71 @@ context_flags_string(int mask) } +static void +usage(void) +{ +#ifdef _WIN32 + printf("Usage: wglinfo [-v] [-t] [-h] [-b] [-l] [-s]\n"); +#else + printf("Usage: glxinfo [-v] [-t] [-h] [-b] [-l] [-s] [-i] [-display ]\n"); + printf("\t-display : Print GLX visuals on specified server.\n"); + printf("\t-i: Force an indirect rendering context.\n"); +#endif + printf("\t-v: Print visuals info in verbose form.\n"); + printf("\t-t: Print verbose table.\n"); + printf("\t-h: This information.\n"); + printf("\t-b: Find the 'best' visual and print its number.\n"); + printf("\t-l: Print interesting OpenGL limits.\n"); + printf("\t-s: Print a single extension per line.\n"); +} + + +void +parse_args(int argc, char *argv[], struct options *options) +{ + int i; + + options->mode = Normal; + options->findBest = GL_FALSE; + options->limits = GL_FALSE; + options->singleLine = GL_FALSE; + options->displayName = NULL; + options->allowDirect = GL_FALSE; + + for (i = 1; i < argc; i++) { +#ifndef _WIN32 + if (strcmp(argv[i], "-display") == 0 && i + 1 < argc) { + options->displayName = argv[i + 1]; + i++; + } + else if (strcmp(argv[i], "-i") == 0) { + options->allowDirect = GL_FALSE; + } + else +#endif + if (strcmp(argv[i], "-t") == 0) { + options->mode = Wide; + } + else if (strcmp(argv[i], "-v") == 0) { + options->mode = Verbose; + } + else if (strcmp(argv[i], "-b") == 0) { + options->findBest = GL_TRUE; + } + else if (strcmp(argv[i], "-l") == 0) { + options->limits = GL_TRUE; + } + else if (strcmp(argv[i], "-h") == 0) { + usage(); + exit(0); + } + else if(strcmp(argv[i], "-s") == 0) { + options->singleLine = GL_TRUE; + } + else { + printf("Unknown option `%s'\n", argv[i]); + usage(); + exit(0); + } + } +} diff --git a/src/xdemos/glinfo_common.h b/src/xdemos/glinfo_common.h index d0daf4d..a268727 100644 --- a/src/xdemos/glinfo_common.h +++ b/src/xdemos/glinfo_common.h @@ -63,6 +63,27 @@ struct bit_info }; +typedef enum +{ + Normal, + Wide, + Verbose +} InfoMode; + + +struct options +{ + InfoMode mode; + GLboolean findBest; + GLboolean limits; + GLbo
[Mesa-dev] [PATCH 3/5] wglinfo: Add support for reporting core profile info
As with glxinfo, we first report version/extension info for a compatibility profile context, then report version/extension info for a core profile context. --- src/wgl/wglinfo.c | 134 +--- src/xdemos/glinfo_common.h | 24 src/xdemos/glxinfo.c | 24 3 files changed, 138 insertions(+), 44 deletions(-) diff --git a/src/wgl/wglinfo.c b/src/wgl/wglinfo.c index fe94dcc..16008d2 100644 --- a/src/wgl/wglinfo.c +++ b/src/wgl/wglinfo.c @@ -42,10 +42,12 @@ #include "glinfo_common.h" +static GLboolean have_WGL_ARB_create_context; static GLboolean have_WGL_ARB_pixel_format; static GLboolean have_WGL_ARB_multisample; static PFNWGLGETPIXELFORMATATTRIBIVARBPROC wglGetPixelFormatAttribivARB_func; +static PFNWGLCREATECONTEXTATTRIBSARBPROC wglCreateContextAttribsARB_func; /** @@ -77,7 +79,8 @@ WndProc(HWND hWnd, static void -print_screen_info(HDC _hdc, GLboolean limits, GLboolean singleLine) +print_screen_info(HDC _hdc, GLboolean limits, GLboolean singleLine, + GLboolean coreProfile) { WNDCLASS wc; HWND win; @@ -153,36 +156,79 @@ print_screen_info(HDC _hdc, GLboolean limits, GLboolean singleLine) PFNWGLGETEXTENSIONSSTRINGARBPROC wglGetExtensionsStringARB_func = (PFNWGLGETEXTENSIONSSTRINGARBPROC)wglGetProcAddress("wglGetExtensionsStringARB"); #endif - const char *glVendor = (const char *) glGetString(GL_VENDOR); - const char *glRenderer = (const char *) glGetString(GL_RENDERER); - const char *glVersion = (const char *) glGetString(GL_VERSION); - const char *glExtensions = (const char *) glGetString(GL_EXTENSIONS); + const char *glVendor, *glRenderer, *glVersion, *glExtensions; + const char *wglExtensions = NULL; struct ext_functions extfuncs; #if defined(WGL_ARB_extensions_string) if (wglGetExtensionsStringARB_func) { - const char *wglExtensions = wglGetExtensionsStringARB_func(hdc); - if(wglExtensions) { -printf("WGL extensions:\n"); -print_extension_list(wglExtensions, singleLine); - } + wglExtensions = wglGetExtensionsStringARB_func(hdc); if (extension_supported("WGL_ARB_pixel_format", wglExtensions)) { have_WGL_ARB_pixel_format = GL_TRUE; } if (extension_supported("WGL_ARB_multisample", wglExtensions)) { have_WGL_ARB_multisample = GL_TRUE; } + if (extension_supported("WGL_ARB_create_context", wglExtensions)) { +have_WGL_ARB_create_context = GL_TRUE; + } } #endif - printf("OpenGL vendor string: %s\n", glVendor); - printf("OpenGL renderer string: %s\n", glRenderer); - printf("OpenGL version string: %s\n", glVersion); -#ifdef GL_VERSION_2_0 - if (glVersion[0] >= '2' && glVersion[1] == '.') { - char *v = (char *) glGetString(GL_SHADING_LANGUAGE_VERSION); - printf("OpenGL shading language version string: %s\n", v); + + if (coreProfile && have_WGL_ARB_create_context) { + /* Try to create a new, core context */ + HGLRC core_ctx = 0; + int i; + + wglCreateContextAttribsARB_func = +(PFNWGLCREATECONTEXTATTRIBSARBPROC) +wglGetProcAddress("wglCreateContextAttribsARB"); + assert(wglCreateContextAttribsARB_func); + if (!wglCreateContextAttribsARB_func) { +printf("Failed to get wglCreateContextAttribsARB pointer."); +return; + } + + for (i = NUM_GL_VERSIONS - 2; i > 0 ; i--) { +int attribs[10], n; + +/* don't bother below GL 3.1 */ +if (gl_versions[i].major == 3 && gl_versions[i].minor == 0) { + break; +} + +n = 0; +attribs[n++] = WGL_CONTEXT_MAJOR_VERSION_ARB; +attribs[n++] = gl_versions[i].major; +attribs[n++] = WGL_CONTEXT_MINOR_VERSION_ARB; +attribs[n++] = gl_versions[i].minor; +if (gl_versions[i].major * 10 + gl_versions[i].minor > 31) { + attribs[n++] = WGL_CONTEXT_PROFILE_MASK_ARB; + attribs[n++] = WGL_CONTEXT_CORE_PROFILE_BIT_ARB; +} +attribs[n++] = 0; + +core_ctx = wglCreateContextAttribsARB_func(hdc, 0, attribs); +if (core_ctx) { + break; +} + } + + if (!core_ctx) { +printf("Failed to create core profile context.\n"); +return; + } + + ctx = core_ctx; + if (!wglMakeCurrent(hdc, ctx)) { +printf("Failed to bind core profile context.\n"); +return; + } + oglString = "OpenGL core profile"; + } + else { + coreProfile = GL_FALSE; } -#endif extfuncs.GetProgramivARB = (PFNGLGETPROGRAMIVARBPROC) wglGetProcAddress("glGetProgramivARB"); @@ -191,9 +237,55 @@ pri
Re: [Mesa-dev] [PATCH 1/2] mesa/st: add support for dynamic ubo selection
On 07/15/2014 05:28 AM, Ilia Mirkin wrote: On Wed, Jul 9, 2014 at 9:06 AM, Brian Paul wrote: On 07/08/2014 08:40 PM, Ilia Mirkin wrote: Signed-off-by: Ilia Mirkin --- With ChrisF's patches to add support for this in core mesa, this generates code like: FRAG DCL OUT[0], COLOR DCL CONST[0] DCL CONST[1][0] DCL CONST[2][0] DCL CONST[3][0] DCL CONST[4][0] DCL TEMP[0], LOCAL DCL ADDR[0..1] IMM[0] UINT32 {0, 0, 0, 0} IMM[1] INT32 {1, 0, 0, 0} 0: UADD TEMP[0].x, CONST[0]., IMM[1]. 1: UARL ADDR[1].x, TEMP[0]. 2: UARL ADDR[1].x, TEMP[0]. 3: MOV TEMP[0], CONST[ADDR[1].x][0] 4: MOV OUT[0], TEMP[0] 5: END Not sure what the deal is with the two UARL's, but nouveau's backend removes one of them pretty easily. I assume others handle this too. Yeah, I noticed this too when I was doing some UBO work last week. I haven't had time to investigate yet. Does this change look good though? Looks like Chris's non-const UBO indexing patches are ~ready. With some tweaks to the nvc0 logic, the simple tests pass on nouveau (and there are no 'complex' tests). I'm not an expert on this code, but look OK AFAICT. Reviewed-by: Brian Paul -Brian Unfortunately the core patches aren't quite ready yet, but this patch doesn't regress anything. src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 20 +--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp index 9bc7500..3202c56 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp @@ -1947,16 +1947,16 @@ glsl_to_tgsi_visitor::visit(ir_expression *ir) break; case ir_binop_ubo_load: { - ir_constant *uniform_block = ir->operands[0]->as_constant(); + ir_constant *const_uniform_block = ir->operands[0]->as_constant(); ir_constant *const_offset_ir = ir->operands[1]->as_constant(); unsigned const_offset = const_offset_ir ? const_offset_ir->value.u[0] : 0; + unsigned const_block = const_uniform_block ? const_uniform_block->value.u[0] + 1 : 0; st_src_reg index_reg = get_temp(glsl_type::uint_type); st_src_reg cbuf; cbuf.type = glsl_type::vec4_type->base_type; cbuf.file = PROGRAM_CONSTANT; cbuf.index = 0; - cbuf.index2D = uniform_block->value.u[0] + 1; cbuf.reladdr = NULL; cbuf.negate = 0; @@ -1966,7 +1966,6 @@ glsl_to_tgsi_visitor::visit(ir_expression *ir) /* Constant index into constant buffer */ cbuf.reladdr = NULL; cbuf.index = const_offset / 16; - cbuf.has_index2 = true; } else { /* Relative/variable index into constant buffer */ @@ -1976,6 +1975,21 @@ glsl_to_tgsi_visitor::visit(ir_expression *ir) memcpy(cbuf.reladdr, &index_reg, sizeof(index_reg)); } + if (const_uniform_block) { + /* Constant constant buffer */ + cbuf.reladdr2 = NULL; + cbuf.index2D = const_block; + cbuf.has_index2 = true; + } + else { + /* Relative/variable constant buffer */ + emit(ir, TGSI_OPCODE_UADD, st_dst_reg(index_reg), op[0], + st_src_reg_for_int(1)); + cbuf.reladdr2 = ralloc(mem_ctx, st_src_reg); + memcpy(cbuf.reladdr2, &index_reg, sizeof(index_reg)); + cbuf.has_index2 = true; + } + cbuf.swizzle = swizzle_for_size(ir->type->vector_elements); cbuf.swizzle += MAKE_SWIZZLE4(const_offset % 16 / 4, const_offset % 16 / 4, ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://urldefense.proofpoint.com/v1/url?u=http://lists.freedesktop.org/mailman/listinfo/mesa-dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=lGQMzzTgII0I7jefp2FHq7WtZ%2BTLs8wadB%2BiIj9xpBY%3D%0A&m=2PxC4euC4MUhdxrARICGkoYmsaKgMNJkIz%2BtzTOqBt0%3D%0A&s=d21974c40a550eabfa7fea826f13ec3c3f1e9a1f0fd99ad856975bac472793b5 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] mesa/st: add support for dynamic ubo selection
On Wed, Jul 9, 2014 at 9:06 AM, Brian Paul wrote: > On 07/08/2014 08:40 PM, Ilia Mirkin wrote: >> >> Signed-off-by: Ilia Mirkin >> --- >> >> With ChrisF's patches to add support for this in core mesa, this generates >> code like: >> >> FRAG >> DCL OUT[0], COLOR >> DCL CONST[0] >> DCL CONST[1][0] >> DCL CONST[2][0] >> DCL CONST[3][0] >> DCL CONST[4][0] >> DCL TEMP[0], LOCAL >> DCL ADDR[0..1] >> IMM[0] UINT32 {0, 0, 0, 0} >> IMM[1] INT32 {1, 0, 0, 0} >>0: UADD TEMP[0].x, CONST[0]., IMM[1]. >>1: UARL ADDR[1].x, TEMP[0]. >>2: UARL ADDR[1].x, TEMP[0]. >>3: MOV TEMP[0], CONST[ADDR[1].x][0] >>4: MOV OUT[0], TEMP[0] >>5: END >> >> Not sure what the deal is with the two UARL's, but nouveau's backend >> removes >> one of them pretty easily. I assume others handle this too. > > > Yeah, I noticed this too when I was doing some UBO work last week. I > haven't had time to investigate yet. Does this change look good though? Looks like Chris's non-const UBO indexing patches are ~ready. With some tweaks to the nvc0 logic, the simple tests pass on nouveau (and there are no 'complex' tests). > > -Brian > > >> >> Unfortunately the core patches aren't quite ready yet, but this patch >> doesn't >> regress anything. >> >> src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 20 +--- >> 1 file changed, 17 insertions(+), 3 deletions(-) >> >> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp >> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp >> index 9bc7500..3202c56 100644 >> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp >> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp >> @@ -1947,16 +1947,16 @@ glsl_to_tgsi_visitor::visit(ir_expression *ir) >> break; >> >> case ir_binop_ubo_load: { >> - ir_constant *uniform_block = ir->operands[0]->as_constant(); >> + ir_constant *const_uniform_block = ir->operands[0]->as_constant(); >> ir_constant *const_offset_ir = ir->operands[1]->as_constant(); >> unsigned const_offset = const_offset_ir ? >> const_offset_ir->value.u[0] : 0; >> + unsigned const_block = const_uniform_block ? >> const_uniform_block->value.u[0] + 1 : 0; >> st_src_reg index_reg = get_temp(glsl_type::uint_type); >> st_src_reg cbuf; >> >> cbuf.type = glsl_type::vec4_type->base_type; >> cbuf.file = PROGRAM_CONSTANT; >> cbuf.index = 0; >> - cbuf.index2D = uniform_block->value.u[0] + 1; >> cbuf.reladdr = NULL; >> cbuf.negate = 0; >> >> @@ -1966,7 +1966,6 @@ glsl_to_tgsi_visitor::visit(ir_expression *ir) >>/* Constant index into constant buffer */ >>cbuf.reladdr = NULL; >>cbuf.index = const_offset / 16; >> - cbuf.has_index2 = true; >> } >> else { >>/* Relative/variable index into constant buffer */ >> @@ -1976,6 +1975,21 @@ glsl_to_tgsi_visitor::visit(ir_expression *ir) >>memcpy(cbuf.reladdr, &index_reg, sizeof(index_reg)); >> } >> >> + if (const_uniform_block) { >> + /* Constant constant buffer */ >> + cbuf.reladdr2 = NULL; >> + cbuf.index2D = const_block; >> + cbuf.has_index2 = true; >> + } >> + else { >> + /* Relative/variable constant buffer */ >> + emit(ir, TGSI_OPCODE_UADD, st_dst_reg(index_reg), op[0], >> + st_src_reg_for_int(1)); >> + cbuf.reladdr2 = ralloc(mem_ctx, st_src_reg); >> + memcpy(cbuf.reladdr2, &index_reg, sizeof(index_reg)); >> + cbuf.has_index2 = true; >> + } >> + >> cbuf.swizzle = swizzle_for_size(ir->type->vector_elements); >> cbuf.swizzle += MAKE_SWIZZLE4(const_offset % 16 / 4, >> const_offset % 16 / 4, >> > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Fwd: [PATCH 2/6] glsl: Mark entire UBO array active if indexed with non-constant.
On Tue, Jul 15, 2014 at 6:28 AM, Chris Forbes wrote: > Accidentally replied privately only, sorry. > > -- Forwarded message -- > From: Chris Forbes > Date: Tue, Jul 15, 2014 at 10:27 PM > Subject: Re: [Mesa-dev] [PATCH 2/6] glsl: Mark entire UBO array active > if indexed with non-constant. > To: Ilia Mirkin > > > On Tue, Jul 15, 2014 at 10:20 PM, Ilia Mirkin wrote: >> On Sat, Jul 12, 2014 at 9:51 PM, Chris Forbes wrote: >>> Without doing a lot more work, we have no idea which indices may >>> be used at runtime, so just mark them all. >>> >>> Signed-off-by: Chris Forbes >>> --- >>> src/glsl/link_uniform_block_active_visitor.cpp | 51 >>> -- >>> 1 file changed, 32 insertions(+), 19 deletions(-) >>> >>> diff --git a/src/glsl/link_uniform_block_active_visitor.cpp >>> b/src/glsl/link_uniform_block_active_visitor.cpp >>> index d19ce20..4bf7349 100644 >>> --- a/src/glsl/link_uniform_block_active_visitor.cpp >>> +++ b/src/glsl/link_uniform_block_active_visitor.cpp >>> @@ -109,32 +109,45 @@ >>> link_uniform_block_active_visitor::visit_enter(ir_dereference_array *ir) >>> assert((b->num_array_elements == 0) == (b->array_elements == NULL)); >>> assert(b->type != NULL); >>> >>> - /* Determine whether or not this array index has already been added to >>> the >>> -* list of active array indices. At this point all constant folding >>> must >>> -* have occured, and the array index must be a constant. >>> -*/ >>> ir_constant *c = ir->array_index->as_constant(); >>> - assert(c != NULL); >>> >>> - const unsigned idx = c->get_uint_component(0); >>> + if (c) { >>> + /* Index is a constant, so mark just that element used, if not >>> already */ >>> + const unsigned idx = c->get_uint_component(0); >>> >>> - unsigned i; >>> - for (i = 0; i < b->num_array_elements; i++) { >>> - if (b->array_elements[i] == idx) >>> -break; >>> - } >>> + unsigned i; >>> + for (i = 0; i < b->num_array_elements; i++) { >>> + if (b->array_elements[i] == idx) >>> +break; >>> + } >>> >>> - assert(i <= b->num_array_elements); >>> + assert(i <= b->num_array_elements); >>> >>> - if (i == b->num_array_elements) { >>> - b->array_elements = reralloc(this->mem_ctx, >>> - b->array_elements, >>> - unsigned, >>> - b->num_array_elements + 1); >>> + if (i == b->num_array_elements) { >>> + b->array_elements = reralloc(this->mem_ctx, >>> + b->array_elements, >>> + unsigned, >>> + b->num_array_elements + 1); >>> >>> - b->array_elements[b->num_array_elements] = idx; >>> + b->array_elements[b->num_array_elements] = idx; >>> >>> - b->num_array_elements++; >>> + b->num_array_elements++; >>> + } >>> + } else { >>> + /* The array index is not a constant, so mark the entire array used. >>> */ >>> + assert(b->type->is_array()); >>> + if (b->num_array_elements < b->type->length) { >> >> This condition is a little different than the other case. Is this >> basically to cover the first time that the array is indexed >> dynamically? > > Right, so if this check fails, then we've already marked every element > and so don't need to do anything. > Otherwise, we need to expand to the full size of the array and mark > everything in one go. > >> >>> + b->num_array_elements = b->type->length; >>> + b->array_elements = reralloc(this->mem_ctx, >>> + b->array_elements, >>> + unsigned, >>> + b->num_array_elements); >>> + >>> + unsigned i; >> >> You'll get yelled at by the MSVC people for this... needs to be at the >> beginning of a block. > > This file is C++, so that shouldn't be a problem. I'll happily move it > to the start of the block though. Ah right. I got fooled into thinking it was C by the "declare variable not-in-for loop" thing, which seems to be the style elsewhere in the file too. This is fine as-is then. > >> >>> + for (i = 0; i < b->num_array_elements; i++) { >>> +b->array_elements[i] = i; >>> + } >> >> I think this is fine, but just want to raise the issue of a situation >> where you start out with some const accesses, and then do a nonconst >> access. The nonconst will erase all the old idx's but since they only >> exist in the array_elements list (at this point in the compilation), >> the reassignment won't cause any issues, right? > > That's the idea. I think it's safe (or alternatively, I've completely > misunderstood something) but more scrutiny wouldn't hurt :) > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.fr
Re: [Mesa-dev] [PATCH 0/6] Dynamically uniform UBO array indexing, Part 1
On Sat, Jul 12, 2014 at 9:51 PM, Chris Forbes wrote: > This series adds various relaxations and extra plumbing to allow indexing > arrays of uniform blocks with dynamically uniform expressions rather than > only constant expressions. > > Covers only the GLSL part -- there are still a few loose ends in the > corresponding i965 patches, so I'll send that support as a separate series > later. I had some minor feedback on a couple of the patches, with that stuff fixed, series is Reviewed-by: Ilia Mirkin And it also worked in my (very limited) testing. Might want to wait for someone to review who knows what they're doing though. -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/6] glsl: Accept nonconstant array references in lower_ubo_reference
Oh dear. This hunk should have been squashed together with the change to it later. By itself, it doesn't get us anywhere. On Tue, Jul 15, 2014 at 10:26 PM, Ilia Mirkin wrote: > On Sat, Jul 12, 2014 at 9:51 PM, Chris Forbes wrote: >> Instead of falling back to just the block name (which we won't find), >> look for the first element of the block array. We'll deal with the rest >> in the backend by arranging for the blocks to be laid out contiguously. >> >> Signed-off-by: Chris Forbes >> --- >> src/glsl/lower_ubo_reference.cpp | 10 ++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/src/glsl/lower_ubo_reference.cpp >> b/src/glsl/lower_ubo_reference.cpp >> index 90e65bd..85e9c7d 100644 >> --- a/src/glsl/lower_ubo_reference.cpp >> +++ b/src/glsl/lower_ubo_reference.cpp >> @@ -102,6 +102,16 @@ interface_field_name(void *mem_ctx, char *base_name, >> ir_dereference *d) >> >> d = a->array->as_dereference(); >> previous_index = a->array_index->as_constant(); >> + >> + if (!previous_index) { >> +/* The array index is not a constant, so let's find >> + * the first element of the array. Elsewhere we guarantee >> + * that the entire array is valid if used with a non-constant >> + * index. >> + */ >> +previous_index = new(mem_ctx) ir_constant(0u); >> + } > > You remove this code in 5/6 -- what's the point of this patch? > >> + >> break; >>} >> >> -- >> 2.0.1 >> >> ___ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/6] glsl: Convert uniform_block in lower_ubo_reference to ir_rvalue.
Indeed, will fix. On Tue, Jul 15, 2014 at 10:24 PM, Ilia Mirkin wrote: > On Sat, Jul 12, 2014 at 9:51 PM, Chris Forbes wrote: >> Previously this was a block index with special semantics for -1. >> With ARB_gpu_shader5, this need not be a compile-time constant, so >> allow any rvalue here and convert the -1 to a NULL pointer. >> >> Signed-off-by: Chris Forbes >> --- >> src/glsl/lower_ubo_reference.cpp | 15 --- >> 1 file changed, 8 insertions(+), 7 deletions(-) >> >> diff --git a/src/glsl/lower_ubo_reference.cpp >> b/src/glsl/lower_ubo_reference.cpp >> index 85e9c7d..c49ae34 100644 >> --- a/src/glsl/lower_ubo_reference.cpp >> +++ b/src/glsl/lower_ubo_reference.cpp >> @@ -57,7 +57,7 @@ public: >> void *mem_ctx; >> struct gl_shader *shader; >> struct gl_uniform_buffer_variable *ubo_var; >> - unsigned uniform_block; >> + ir_rvalue *uniform_block; >> bool progress; >> }; >> >> @@ -145,10 +145,10 @@ lower_ubo_reference_visitor::handle_rvalue(ir_rvalue >> **rvalue) >>interface_field_name(mem_ctx, (char *) >> var->get_interface_type()->name, >> deref); >> >> - this->uniform_block = -1; >> + this->uniform_block = NULL; >> for (unsigned i = 0; i < shader->NumUniformBlocks; i++) { >>if (strcmp(field_name, shader->UniformBlocks[i].Name) == 0) { >> - this->uniform_block = i; >> + this->uniform_block = new(mem_ctx) ir_constant(i); >> >> struct gl_uniform_block *block = &shader->UniformBlocks[i]; >> >> @@ -159,7 +159,7 @@ lower_ubo_reference_visitor::handle_rvalue(ir_rvalue >> **rvalue) >>} >> } >> >> - assert(this->uniform_block != (unsigned) -1); >> + assert(this->uniform_block); >> >> ir_rvalue *offset = new(mem_ctx) ir_constant(0u); >> unsigned const_offset = 0; >> @@ -277,11 +277,12 @@ ir_expression * >> lower_ubo_reference_visitor::ubo_load(const glsl_type *type, >> ir_rvalue *offset) >> { >> + ir_rvalue * block_ref = this->uniform_block->clone(mem_ctx, NULL); > > ir_value *block_ref = ... > > I think is the more common way to write this. (i.e. remove the space > after the * ). > >> return new(mem_ctx) >>ir_expression(ir_binop_ubo_load, >> - type, >> - new(mem_ctx) ir_constant(this->uniform_block), >> - offset); >> +type, >> +block_ref, >> +offset); >> >> } >> >> -- >> 2.0.1 >> >> ___ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] Fwd: [PATCH 2/6] glsl: Mark entire UBO array active if indexed with non-constant.
Accidentally replied privately only, sorry. -- Forwarded message -- From: Chris Forbes Date: Tue, Jul 15, 2014 at 10:27 PM Subject: Re: [Mesa-dev] [PATCH 2/6] glsl: Mark entire UBO array active if indexed with non-constant. To: Ilia Mirkin On Tue, Jul 15, 2014 at 10:20 PM, Ilia Mirkin wrote: > On Sat, Jul 12, 2014 at 9:51 PM, Chris Forbes wrote: >> Without doing a lot more work, we have no idea which indices may >> be used at runtime, so just mark them all. >> >> Signed-off-by: Chris Forbes >> --- >> src/glsl/link_uniform_block_active_visitor.cpp | 51 >> -- >> 1 file changed, 32 insertions(+), 19 deletions(-) >> >> diff --git a/src/glsl/link_uniform_block_active_visitor.cpp >> b/src/glsl/link_uniform_block_active_visitor.cpp >> index d19ce20..4bf7349 100644 >> --- a/src/glsl/link_uniform_block_active_visitor.cpp >> +++ b/src/glsl/link_uniform_block_active_visitor.cpp >> @@ -109,32 +109,45 @@ >> link_uniform_block_active_visitor::visit_enter(ir_dereference_array *ir) >> assert((b->num_array_elements == 0) == (b->array_elements == NULL)); >> assert(b->type != NULL); >> >> - /* Determine whether or not this array index has already been added to >> the >> -* list of active array indices. At this point all constant folding must >> -* have occured, and the array index must be a constant. >> -*/ >> ir_constant *c = ir->array_index->as_constant(); >> - assert(c != NULL); >> >> - const unsigned idx = c->get_uint_component(0); >> + if (c) { >> + /* Index is a constant, so mark just that element used, if not >> already */ >> + const unsigned idx = c->get_uint_component(0); >> >> - unsigned i; >> - for (i = 0; i < b->num_array_elements; i++) { >> - if (b->array_elements[i] == idx) >> -break; >> - } >> + unsigned i; >> + for (i = 0; i < b->num_array_elements; i++) { >> + if (b->array_elements[i] == idx) >> +break; >> + } >> >> - assert(i <= b->num_array_elements); >> + assert(i <= b->num_array_elements); >> >> - if (i == b->num_array_elements) { >> - b->array_elements = reralloc(this->mem_ctx, >> - b->array_elements, >> - unsigned, >> - b->num_array_elements + 1); >> + if (i == b->num_array_elements) { >> + b->array_elements = reralloc(this->mem_ctx, >> + b->array_elements, >> + unsigned, >> + b->num_array_elements + 1); >> >> - b->array_elements[b->num_array_elements] = idx; >> + b->array_elements[b->num_array_elements] = idx; >> >> - b->num_array_elements++; >> + b->num_array_elements++; >> + } >> + } else { >> + /* The array index is not a constant, so mark the entire array used. >> */ >> + assert(b->type->is_array()); >> + if (b->num_array_elements < b->type->length) { > > This condition is a little different than the other case. Is this > basically to cover the first time that the array is indexed > dynamically? Right, so if this check fails, then we've already marked every element and so don't need to do anything. Otherwise, we need to expand to the full size of the array and mark everything in one go. > >> + b->num_array_elements = b->type->length; >> + b->array_elements = reralloc(this->mem_ctx, >> + b->array_elements, >> + unsigned, >> + b->num_array_elements); >> + >> + unsigned i; > > You'll get yelled at by the MSVC people for this... needs to be at the > beginning of a block. This file is C++, so that shouldn't be a problem. I'll happily move it to the start of the block though. > >> + for (i = 0; i < b->num_array_elements; i++) { >> +b->array_elements[i] = i; >> + } > > I think this is fine, but just want to raise the issue of a situation > where you start out with some const accesses, and then do a nonconst > access. The nonconst will erase all the old idx's but since they only > exist in the array_elements list (at this point in the compilation), > the reassignment won't cause any issues, right? That's the idea. I think it's safe (or alternatively, I've completely misunderstood something) but more scrutiny wouldn't hurt :) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/6] glsl: Accept nonconstant array references in lower_ubo_reference
On Sat, Jul 12, 2014 at 9:51 PM, Chris Forbes wrote: > Instead of falling back to just the block name (which we won't find), > look for the first element of the block array. We'll deal with the rest > in the backend by arranging for the blocks to be laid out contiguously. > > Signed-off-by: Chris Forbes > --- > src/glsl/lower_ubo_reference.cpp | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/src/glsl/lower_ubo_reference.cpp > b/src/glsl/lower_ubo_reference.cpp > index 90e65bd..85e9c7d 100644 > --- a/src/glsl/lower_ubo_reference.cpp > +++ b/src/glsl/lower_ubo_reference.cpp > @@ -102,6 +102,16 @@ interface_field_name(void *mem_ctx, char *base_name, > ir_dereference *d) > > d = a->array->as_dereference(); > previous_index = a->array_index->as_constant(); > + > + if (!previous_index) { > +/* The array index is not a constant, so let's find > + * the first element of the array. Elsewhere we guarantee > + * that the entire array is valid if used with a non-constant > + * index. > + */ > +previous_index = new(mem_ctx) ir_constant(0u); > + } You remove this code in 5/6 -- what's the point of this patch? > + > break; >} > > -- > 2.0.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/6] glsl: Convert uniform_block in lower_ubo_reference to ir_rvalue.
On Sat, Jul 12, 2014 at 9:51 PM, Chris Forbes wrote: > Previously this was a block index with special semantics for -1. > With ARB_gpu_shader5, this need not be a compile-time constant, so > allow any rvalue here and convert the -1 to a NULL pointer. > > Signed-off-by: Chris Forbes > --- > src/glsl/lower_ubo_reference.cpp | 15 --- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/src/glsl/lower_ubo_reference.cpp > b/src/glsl/lower_ubo_reference.cpp > index 85e9c7d..c49ae34 100644 > --- a/src/glsl/lower_ubo_reference.cpp > +++ b/src/glsl/lower_ubo_reference.cpp > @@ -57,7 +57,7 @@ public: > void *mem_ctx; > struct gl_shader *shader; > struct gl_uniform_buffer_variable *ubo_var; > - unsigned uniform_block; > + ir_rvalue *uniform_block; > bool progress; > }; > > @@ -145,10 +145,10 @@ lower_ubo_reference_visitor::handle_rvalue(ir_rvalue > **rvalue) >interface_field_name(mem_ctx, (char *) var->get_interface_type()->name, > deref); > > - this->uniform_block = -1; > + this->uniform_block = NULL; > for (unsigned i = 0; i < shader->NumUniformBlocks; i++) { >if (strcmp(field_name, shader->UniformBlocks[i].Name) == 0) { > - this->uniform_block = i; > + this->uniform_block = new(mem_ctx) ir_constant(i); > > struct gl_uniform_block *block = &shader->UniformBlocks[i]; > > @@ -159,7 +159,7 @@ lower_ubo_reference_visitor::handle_rvalue(ir_rvalue > **rvalue) >} > } > > - assert(this->uniform_block != (unsigned) -1); > + assert(this->uniform_block); > > ir_rvalue *offset = new(mem_ctx) ir_constant(0u); > unsigned const_offset = 0; > @@ -277,11 +277,12 @@ ir_expression * > lower_ubo_reference_visitor::ubo_load(const glsl_type *type, > ir_rvalue *offset) > { > + ir_rvalue * block_ref = this->uniform_block->clone(mem_ctx, NULL); ir_value *block_ref = ... I think is the more common way to write this. (i.e. remove the space after the * ). > return new(mem_ctx) >ir_expression(ir_binop_ubo_load, > - type, > - new(mem_ctx) ir_constant(this->uniform_block), > - offset); > +type, > +block_ref, > +offset); > > } > > -- > 2.0.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/6] glsl: Mark entire UBO array active if indexed with non-constant.
On Sat, Jul 12, 2014 at 9:51 PM, Chris Forbes wrote: > Without doing a lot more work, we have no idea which indices may > be used at runtime, so just mark them all. > > Signed-off-by: Chris Forbes > --- > src/glsl/link_uniform_block_active_visitor.cpp | 51 > -- > 1 file changed, 32 insertions(+), 19 deletions(-) > > diff --git a/src/glsl/link_uniform_block_active_visitor.cpp > b/src/glsl/link_uniform_block_active_visitor.cpp > index d19ce20..4bf7349 100644 > --- a/src/glsl/link_uniform_block_active_visitor.cpp > +++ b/src/glsl/link_uniform_block_active_visitor.cpp > @@ -109,32 +109,45 @@ > link_uniform_block_active_visitor::visit_enter(ir_dereference_array *ir) > assert((b->num_array_elements == 0) == (b->array_elements == NULL)); > assert(b->type != NULL); > > - /* Determine whether or not this array index has already been added to the > -* list of active array indices. At this point all constant folding must > -* have occured, and the array index must be a constant. > -*/ > ir_constant *c = ir->array_index->as_constant(); > - assert(c != NULL); > > - const unsigned idx = c->get_uint_component(0); > + if (c) { > + /* Index is a constant, so mark just that element used, if not already > */ > + const unsigned idx = c->get_uint_component(0); > > - unsigned i; > - for (i = 0; i < b->num_array_elements; i++) { > - if (b->array_elements[i] == idx) > -break; > - } > + unsigned i; > + for (i = 0; i < b->num_array_elements; i++) { > + if (b->array_elements[i] == idx) > +break; > + } > > - assert(i <= b->num_array_elements); > + assert(i <= b->num_array_elements); > > - if (i == b->num_array_elements) { > - b->array_elements = reralloc(this->mem_ctx, > - b->array_elements, > - unsigned, > - b->num_array_elements + 1); > + if (i == b->num_array_elements) { > + b->array_elements = reralloc(this->mem_ctx, > + b->array_elements, > + unsigned, > + b->num_array_elements + 1); > > - b->array_elements[b->num_array_elements] = idx; > + b->array_elements[b->num_array_elements] = idx; > > - b->num_array_elements++; > + b->num_array_elements++; > + } > + } else { > + /* The array index is not a constant, so mark the entire array used. */ > + assert(b->type->is_array()); > + if (b->num_array_elements < b->type->length) { This condition is a little different than the other case. Is this basically to cover the first time that the array is indexed dynamically? > + b->num_array_elements = b->type->length; > + b->array_elements = reralloc(this->mem_ctx, > + b->array_elements, > + unsigned, > + b->num_array_elements); > + > + unsigned i; You'll get yelled at by the MSVC people for this... needs to be at the beginning of a block. > + for (i = 0; i < b->num_array_elements; i++) { > +b->array_elements[i] = i; > + } I think this is fine, but just want to raise the issue of a situation where you start out with some const accesses, and then do a nonconst access. The nonconst will erase all the old idx's but since they only exist in the array_elements list (at this point in the compilation), the reassignment won't cause any issues, right? > + } > } > > return visit_continue_with_parent; > -- > 2.0.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] EGL FBDEV platform broken by commit 91ff0d4c6510dc38f279c586ced17fba917873e7
Hello. I am using Mesa with EGL platform FBDEV. In this setup the native_window is always NULL. It worked good, but after update to new version of Mesa it shows error in creating EGLsurface. I tracked it down to commit 91ff0d4c6510dc38f279c586ced17fba917873e7 egl: Check for NULL native_window in eglCreateWindowSurface where this is added: @@ -571,6 +571,10 @@ _eglCreateWindowSurfaceCommon(_EGLDisplay *disp, EGLConfig config, EGLSurface ret; _EGL_CHECK_CONFIG(disp, conf, EGL_NO_SURFACE, drv); + + if (native_window == NULL) +RETURN_EGL_ERROR(disp, EGL_BAD_NATIVE_WINDOW, EGL_NO_SURFACE); + So when native_window is NULL it returns error. I commented that if out. then my rotating cube works again. Is the EGL FBDEV platform supported? How should the change be to work with NULL native_window in fbdev. Thanks. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/6] radeonsi: move sampler descriptors from IB to memory
On 13.07.2014 01:35, Marek Olšák wrote: > > Border colors have been broken if texturing from multiple shader stages is > used. This patch doesn't change that. [...] > +/* Upload border colors and update the pointers in resource descriptors. > + * There can only be 4096 border colors per context. > + * > + * XXX: This is broken if sampler states are bound to multiple shader stages, > + * because TA_BC_BASE_ADDR is shared by all of them and we overwrite it > + * for stages which were set earlier. This is also broken for > + * fine-grained sampler state updates. > + */ I don't think that's accurate, as the BO for storing the border colours is per-context, not per-shader-stage. AFAICS the only thing missing is flushing the command stream when we need to allocate a new BO for that. Might also be a good idea to always allocate a new BO for this after a flush. Other than that, the change looks good to me. -- Earthling Michel Dänzer| http://www.amd.com Libre software enthusiast |Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] A proposal for new testing requirements for stable releases
On Tue, Jul 15, 2014 at 2:18 AM, Michel Dänzer wrote: > On 09.07.2014 08:10, Carl Worth wrote: >> >> 3. I'd like to receive a testing report from each driver team >> >> This is the meat of my proposal. I'm requesting that each driver >> team designate one (or more) people that will be responsible for >> running (or collecting) tests for each release-candidate and >> reporting the results to me. >> >> With a new release-candidate pushed by the end of the day on >> Monday, and me starting the actual release work on Friday, that >> gives 72 hours for each team to perform testing. >> >> I'm happy to let each driver team decide its own testing >> strategy. I would hope it would be based on running piglit >> across a set of "interesting" hardware configurations, (and >> augmenting piglit as necessary as bug fixes are performed). But >> I do not plan to be involved in the specification of what those >> configurations are. All I need is something along the lines of: >> >> "The radeon team is OK with releasing commit " >> >> sent to me before the scheduled day of the release. >> >> Obviously, any negative report from any team can trigger changes >> to the patches to be included in the release. > > This sounds good, but... > >> And in order to put some teeth into this scheme: >> >> I propose that on the day of the release, the release manager >> drop all driver-specific patches for any driver for which the >> driver team has not provided an affirmative testing report. > > ... this seems a bit harsh. > > A lot of backported fixes are specific to one driver and have zero > impact on anything else. Surely it should be up to the discretion of the > driver maintainers whether or not such changes should be backported. > > OTOH, who's supposed to give that sort of OK for changes to say st/mesa > or Mesa core? The former may affect all Gallium based drivers, the > latter basically anything. It's unlikely that any individual or > organization can test all affected setups in advance. Perhaps that should have read s/testing//? I think all he's looking for is a "yes" from each driver team with changes in stable. This can be derived from your supreme confidence in your mesa-stable tagging (and Carl's super-human cherry-picking), or just seeing if the thing builds with your driver, or actually running it on any hw you deem necessary. FTR, I believe I'm the one who suggested adding teeth to this since I felt that no one would actually do any checking unless there was some reason to. The mesa/core and mesa/st changes get a free ride since (a) Carl can test them on his setup via soft/llvmpipe and (b) the shared responsibility makes it awkward. Cheers, -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 80266] Many instances of 1<<31, which is undefined in C99
https://bugs.freedesktop.org/show_bug.cgi?id=80266 Eero Tamminen changed: What|Removed |Added URL||http://blog.llvm.org/2011/0 ||5/what-every-c-programmer-s ||hould-know.html --- Comment #14 from Eero Tamminen --- (In reply to comment #10) > And as tedious as this is to mention, C99 section 6.5.7.4 does state this is > undefined behavior. I know the "C abstract machine" is far removed from real > problems with drivers and whatnot -- and I appreciate the awesome work you > all do on Mesa. I just wanted to clarify that there is spec support for this > claim. LLVM compiler site has 3 article series on issues with undefined behavior (see URL field), it's a very good read. Coccinelle might be able to help detection and patching of issues like this: http://lwn.net/Articles/380835/ -- You are receiving this mail because: You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev