[Mesa-dev] [Bug 73371] EGL_NOK_texture_from_pixmap is advertised but not properly set.
https://bugs.freedesktop.org/show_bug.cgi?id=73371 Tapani Pälli changed: What|Removed |Added CC||lem...@gmail.com --- Comment #3 from Tapani Pälli --- The Piglit test for the extension is not passing either. If the intention of the bit is to tell if Y is inverted by the driver (can't find the spec), then Mesa is correctly reporting false. -- 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 4/9] gallium-tgsi: add TGSI_OPCODE_{FMA-POPCNT-MSB-LSB} description
On Tue, 2014-01-07 at 21:49 +0100, Marek Olšák wrote: > FYI, Evergreen has dedicated instructions for both MAD and FMA. FMA > seems to be available on DX11 chips only. FWIW, not all evergreen chips support FMA, only high-end chips that support FP64 (I guess cypress only), according to the isa docs: > Instructions > FMA > Description > Fused single-precision multiply-add. Only for double-precision parts. > dst = src0 * src1 + src2 > Vadim > > Marek > > On Tue, Jan 7, 2014 at 8:20 PM, Roland Scheidegger wrote: > > Yes that is certainly related. I'm actually not entirely sure what is > > allowed in glsl by default as OpenGL seems to have some lax rules > > regarding precision in any case (float calculations not required but > > allowed to use denorms, at least earlier versions weren't required to > > support Infs neither and so on). > > It is quite possible the "MAD" we were always using would have been > > allowed to really do fma (at least with OpenGL), unless the "precise" > > qualifier was used (which isn't supported yet?). > > TGSI also isn't really watertight about such issues neither (that is if > > you use it with hw such as r300 then you certainly don't expect ieee754 > > rules to be followed but if you've got a d3d10-capable backend then you > > are expected to follow rules specified there which are _mostly_ > > ieee754-2008). > > So I'm not really sure if TGSI MAD should be allowed to do either > > rounding or not, but someday it should be figured out and spelled out > > explicitly in docs. > > > > Roland > > > > > > Am 07.01.2014 19:24, schrieb Maxence Le Doré: > >> I forgot the link : > >> > >> https://urldefense.proofpoint.com/v1/url?u=http://www.geeks3d.com/20120106/precise-qualifier-in-glsl-and-nvidia-geforce-cards/&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=F4msKE2WxRzA%2BwN%2B25muztFm5TSPwE8HKJfWfR2NgfY%3D%0A&m=%2FzSAl55KOH0z7T5qkRj6BX164wf6QpYOnJLIzojXBQc%3D%0A&s=0ac5e0fbd69867705f0c52090c9ddf84e7832be80e724a0983c5aa2f5dde72e0 > >> > >> 2014/1/7 Maxence Le Doré : > >>> For this reason, GLSL 4.0 introduces the 'precise' qualifier. I invite > >>> you to take a look at this article. > >>> > >>> 2014/1/6 Roland Scheidegger : > Am 05.01.2014 01:34, schrieb Maxence Le Doré: > > FMA(a,b,c) keeps extra precision (usually 1 more bit of mantissa, > > afaik) for the result a*b and add this to c, to finally produce a > > IEEE754 32bit float result. > > > > MAD(a,b,c) product a IEEE754 32bit float product a*b and add it to C. > > > > So, fma can be slightly more accurate. An accuracy that is something > > very appreciate. > > Actually in "newer" languages (such as opencl) "mad" is used to indicate > intermediate rounding does not matter, so if your cpu can do fma but not > mul+add in a single cycle it is allowed to use fma instead. > FMA OTOH of course forces no intermediate rounding. > Our tgsi definitions certainly initially were meaning intermediate > rounding should take place, I don't know if we need to keep it that way > or could repurpose that slightly (so if you require the intermediate > rounding you'd just use mul+add). > > Roland > > > > > > > > > 2014/1/5 Marek Olšák : > >> How is FMA different from MAD? > >> > >> Please document the new opcodes in src/gallium/docs/source/tgsi.rst. > >> > >> Marek > >> > >> On Sun, Jan 5, 2014 at 12:42 AM, Maxence Le Doré > >> wrote: > >>> From: Maxence Le Doré > >>> > >>> --- > >>> src/gallium/auxiliary/tgsi/tgsi_info.c | 16 > >>> src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h | 6 ++ > >>> src/gallium/include/pipe/p_shader_tokens.h | 9 - > >>> 3 files changed, 30 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_info.c > >>> b/src/gallium/auxiliary/tgsi/tgsi_info.c > >>> index 0beef44..ed55940 100644 > >>> --- a/src/gallium/auxiliary/tgsi/tgsi_info.c > >>> +++ b/src/gallium/auxiliary/tgsi/tgsi_info.c > >>> @@ -221,6 +221,12 @@ static const struct tgsi_opcode_info > >>> opcode_info[TGSI_OPCODE_LAST] = > >>> { 1, 3, 1, 0, 0, 0, OTHR, "TXL2", TGSI_OPCODE_TXL2 }, > >>> { 1, 2, 0, 0, 0, 0, COMP, "IMUL_HI", TGSI_OPCODE_IMUL_HI }, > >>> { 1, 2, 0, 0, 0, 0, COMP, "UMUL_HI", TGSI_OPCODE_UMUL_HI }, > >>> + { 1, 3, 0, 0, 0, 0, COMP, "FMA", TGSI_OPCODE_FMA }, > >>> + { 1, 1, 0, 0, 0, 0, COMP, "POPCNT", TGSI_OPCODE_POPCNT }, > >>> + { 1, 1, 0, 0, 0, 0, COMP, "IMSB", TGSI_OPCODE_IMSB }, > >>> + { 1, 1, 0, 0, 0, 0, COMP, "ILSB", TGSI_OPCODE_ILSB }, > >>> + { 1, 1, 0, 0, 0, 0, COMP, "UMSB", TGSI_OPCODE_UMSB }, > >>> + { 1, 1, 0, 0, 0, 0, COMP, "ULSB", TGSI_OPCODE_ULSB }, > >>> }; > >>> > >>> const struct tgsi_opcode_info * > >>> @@ -321,6 +327,11 @@ tgsi_opcode_infer_type( uint opcode ) > >>> case
Re: [Mesa-dev] [PATCH] llvmpipe: fix primitive input to geom shaders
Yea, this sucks. Geometry shaders can take primitive id (system value) for passed in primitives and generate one (semantic) for primitives generated in the geometry shader. TBH, I thought we already handled it... Maybe wlk doesn't test it, we'll see if it regresses. z - Original Message - > Well we were using a system value for prim id in gs, hence this was not > necessary. I'm always confused though about system value / normal > semantic usage though, Zack might know better. > > Roland > > Am 07.01.2014 09:55, schrieb Dave Airlie: > > Not sure this is 100% the correct way to do this, since it may be a change > > at the glsl->tgsi level that is required, either way open discussions! > > > > fixes piglit > > tests/spec/glsl-1.50/execution/geometry/primitive-id-in.shader_test > > with llvmpipe with fake MSAA > > > > Signed-off-by: Dave Airlie > > --- > > src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c | 5 + > > src/gallium/auxiliary/tgsi/tgsi_scan.c | 3 +++ > > 2 files changed, 8 insertions(+) > > > > diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c > > b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c > > index 6d8dc8c..de2c64f 100644 > > --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c > > +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c > > @@ -1173,6 +1173,10 @@ emit_fetch_gs_input( > > LLVMValueRef swizzle_index = lp_build_const_int32(gallivm, swizzle); > > LLVMValueRef res; > > > > + if (bld_base->info->input_semantic_name[reg->Register.Index] == > > TGSI_SEMANTIC_PRIMID) { > > + res = bld->system_values.prim_id; > > + goto out; > > + } > > if (reg->Register.Indirect) { > >attrib_index = get_indirect_index(bld, > > reg->Register.File, > > @@ -1200,6 +1204,7 @@ emit_fetch_gs_input( > > > > assert(res); > > > > + out: > > if (stype == TGSI_TYPE_UNSIGNED) { > >res = LLVMBuildBitCast(builder, res, bld_base->uint_bld.vec_type, > >""); > > } else if (stype == TGSI_TYPE_SIGNED) { > > diff --git a/src/gallium/auxiliary/tgsi/tgsi_scan.c > > b/src/gallium/auxiliary/tgsi/tgsi_scan.c > > index 0f10556..ce1f7b6 100644 > > --- a/src/gallium/auxiliary/tgsi/tgsi_scan.c > > +++ b/src/gallium/auxiliary/tgsi/tgsi_scan.c > > @@ -198,6 +198,9 @@ tgsi_scan_shader(const struct tgsi_token *tokens, > > info->uses_primid = TRUE; > > else if (semName == TGSI_SEMANTIC_FACE) > > info->uses_frontface = TRUE; > > + } else if (procType == TGSI_PROCESSOR_GEOMETRY) { > > + if (semName == TGSI_SEMANTIC_PRIMID) > > +info->uses_primid = TRUE; > >} > > } > > else if (file == TGSI_FILE_SYSTEM_VALUE) { > > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH V2] i965: Fix the region's pitch condition to use blitter
intelEmitCopyBlit uses a signed 16-bit integer to represent buffer pitch, so it can only handle buffer pitches < 32k. Cc: mesa-sta...@lists.freedesktop.org Signed-off-by: Anuj Phogat --- Dropping few changes made in earlier patch. If region->bo->size >= max_gtt_map_object_size, we can't fall back to intel_miptree_map_gtt(). src/mesa/drivers/dri/i965/intel_blit.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mesa/drivers/dri/i965/intel_blit.c b/src/mesa/drivers/dri/i965/intel_blit.c index 7bc289f..32431b9 100644 --- a/src/mesa/drivers/dri/i965/intel_blit.c +++ b/src/mesa/drivers/dri/i965/intel_blit.c @@ -199,9 +199,9 @@ intel_miptree_blit(struct brw_context *brw, * As a result of these two limitations, we can only use the blitter to do * this copy when the region's pitch is less than 32k. */ - if (src_mt->region->pitch > 32768 || - dst_mt->region->pitch > 32768) { - perf_debug("Falling back due to >32k pitch\n"); + if (src_mt->region->pitch >= 32768 || + dst_mt->region->pitch >= 32768) { + perf_debug("Falling back due to >=32k pitch\n"); return false; } -- 1.8.3.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/9] gallium-tgsi: add TGSI_OPCODE_{FMA-POPCNT-MSB-LSB} description
Yes, and I guess just about all GPUs can do that. For llvmpipe we'd be vaguely interested in being able to do opencl-style MAD with undefined rounding as FMA since (x86) cpus never have MAD but may have FMA. I am not sure actually though this is ok for d3d10 - interestingly d3d10 has only MAD for SP (saying it's just mul+add but I'm unsure if that really means skipping the intermediate rounding is prohibited), but only FMA for DP... Roland Am 07.01.2014 21:49, schrieb Marek Olšák: > FYI, Evergreen has dedicated instructions for both MAD and FMA. FMA > seems to be available on DX11 chips only. > > Marek > > On Tue, Jan 7, 2014 at 8:20 PM, Roland Scheidegger wrote: >> Yes that is certainly related. I'm actually not entirely sure what is >> allowed in glsl by default as OpenGL seems to have some lax rules >> regarding precision in any case (float calculations not required but >> allowed to use denorms, at least earlier versions weren't required to >> support Infs neither and so on). >> It is quite possible the "MAD" we were always using would have been >> allowed to really do fma (at least with OpenGL), unless the "precise" >> qualifier was used (which isn't supported yet?). >> TGSI also isn't really watertight about such issues neither (that is if >> you use it with hw such as r300 then you certainly don't expect ieee754 >> rules to be followed but if you've got a d3d10-capable backend then you >> are expected to follow rules specified there which are _mostly_ >> ieee754-2008). >> So I'm not really sure if TGSI MAD should be allowed to do either >> rounding or not, but someday it should be figured out and spelled out >> explicitly in docs. >> >> Roland >> >> >> Am 07.01.2014 19:24, schrieb Maxence Le Doré: >>> I forgot the link : >>> >>> https://urldefense.proofpoint.com/v1/url?u=http://www.geeks3d.com/20120106/precise-qualifier-in-glsl-and-nvidia-geforce-cards/&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=F4msKE2WxRzA%2BwN%2B25muztFm5TSPwE8HKJfWfR2NgfY%3D%0A&m=%2FzSAl55KOH0z7T5qkRj6BX164wf6QpYOnJLIzojXBQc%3D%0A&s=0ac5e0fbd69867705f0c52090c9ddf84e7832be80e724a0983c5aa2f5dde72e0 >>> >>> 2014/1/7 Maxence Le Doré : For this reason, GLSL 4.0 introduces the 'precise' qualifier. I invite you to take a look at this article. 2014/1/6 Roland Scheidegger : > Am 05.01.2014 01:34, schrieb Maxence Le Doré: >> FMA(a,b,c) keeps extra precision (usually 1 more bit of mantissa, >> afaik) for the result a*b and add this to c, to finally produce a >> IEEE754 32bit float result. >> >> MAD(a,b,c) product a IEEE754 32bit float product a*b and add it to C. >> >> So, fma can be slightly more accurate. An accuracy that is something >> very appreciate. > > Actually in "newer" languages (such as opencl) "mad" is used to indicate > intermediate rounding does not matter, so if your cpu can do fma but not > mul+add in a single cycle it is allowed to use fma instead. > FMA OTOH of course forces no intermediate rounding. > Our tgsi definitions certainly initially were meaning intermediate > rounding should take place, I don't know if we need to keep it that way > or could repurpose that slightly (so if you require the intermediate > rounding you'd just use mul+add). > > Roland > > > >> >> >> 2014/1/5 Marek Olšák : >>> How is FMA different from MAD? >>> >>> Please document the new opcodes in src/gallium/docs/source/tgsi.rst. >>> >>> Marek >>> >>> On Sun, Jan 5, 2014 at 12:42 AM, Maxence Le Doré >>> wrote: From: Maxence Le Doré --- src/gallium/auxiliary/tgsi/tgsi_info.c | 16 src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h | 6 ++ src/gallium/include/pipe/p_shader_tokens.h | 9 - 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/gallium/auxiliary/tgsi/tgsi_info.c b/src/gallium/auxiliary/tgsi/tgsi_info.c index 0beef44..ed55940 100644 --- a/src/gallium/auxiliary/tgsi/tgsi_info.c +++ b/src/gallium/auxiliary/tgsi/tgsi_info.c @@ -221,6 +221,12 @@ static const struct tgsi_opcode_info opcode_info[TGSI_OPCODE_LAST] = { 1, 3, 1, 0, 0, 0, OTHR, "TXL2", TGSI_OPCODE_TXL2 }, { 1, 2, 0, 0, 0, 0, COMP, "IMUL_HI", TGSI_OPCODE_IMUL_HI }, { 1, 2, 0, 0, 0, 0, COMP, "UMUL_HI", TGSI_OPCODE_UMUL_HI }, + { 1, 3, 0, 0, 0, 0, COMP, "FMA", TGSI_OPCODE_FMA }, + { 1, 1, 0, 0, 0, 0, COMP, "POPCNT", TGSI_OPCODE_POPCNT }, + { 1, 1, 0, 0, 0, 0, COMP, "IMSB", TGSI_OPCODE_IMSB }, + { 1, 1, 0, 0, 0, 0, COMP, "ILSB", TGSI_OPCODE_ILSB }, + { 1, 1, 0, 0, 0, 0, COMP, "UMSB", TGSI_OPCODE_UMSB }, + { 1, 1, 0, 0, 0, 0, COMP, "ULSB", TGSI_OPCODE_ULSB }, }; const struct tgsi_opcode_info * @@ -321,6 +327,
Re: [Mesa-dev] [Mesa-stable] [PATCH 3/3] i965/gen6/blorp: Remove too heavy HiZ workaround
On Tue, Jan 07, 2014 at 06:01:57AM -0800, Paul Berry wrote: > On 6 January 2014 17:05, Chad Versace wrote: > > Commit 1a92881 added extra flushes to fix a HiZ hang in > WebGL Google Maps. With the extra flushes emitted by the previous two > patches, the flushes added by 1a92881 are redundant. > > Tested with the same criteria as in 1a92881: by zooming in and out > continuously for 2 hours on Sandybridge Chrome OS (codename > Stumpy) without a hang. > > CC: mesa-sta...@lists.freedesktop.org > CC: Kenneth Graunke > CC: Paul Berry > CC: Stéphane Marchesin > Signed-off-by: Chad Versace > --- > src/mesa/drivers/dri/i965/gen6_blorp.cpp | 14 -- > 1 file changed, 14 deletions(-) > > > I'm baffled by this. My understanding of patches 1 and 2 is that they both > set > need_workaround_flush redundantly, so they should have no effect on what is > sent to the hardware. Therefore, it seems like this patch puts us back in the > buggy state we were in before commit 1a92881. Have I missed something in my > analysis? Please read the commit message to patch 2, v2. That should clarify your misunderstanding. Patch 2 does not set need_workaround_flush redundantly. > What was the previous MTBF on your system before commit 1a92881? Is it > possible that 2 hours isn't a long enoug test? (For example, if the MTBF was > 40 minutes, and the failures were Poisson-distributed, then I believe that 2 > hours without a failure only confirms the bug fix at 95% confidence.) I haven't calculated the MTBF. IIRC, it was around 20 or 30 minutes. But, to be honest, I did those experiments the first week of December, so my memory may be inaccurate. With the clarification on patch 2, do you still require a calculation of MTBF? ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/3] i965/gen6/blorp: Remove redundant HiZ workaround
Commit 1a92881 added extra flushes to fix a HiZ hang in WebGL Google Maps. With the extra flushes emitted by the previous two patches, the flushes added by 1a92881 are redundant. Tested with the same criteria as in 1a92881: by zooming in and out continuously for 2 hours on Sandybridge Chrome OS (codename Stumpy) without a hang. CC: Kenneth Graunke CC: Paul Berry CC: Stéphane Marchesin Signed-off-by: Chad Versace --- src/mesa/drivers/dri/i965/gen6_blorp.cpp | 14 -- 1 file changed, 14 deletions(-) diff --git a/src/mesa/drivers/dri/i965/gen6_blorp.cpp b/src/mesa/drivers/dri/i965/gen6_blorp.cpp index 9db0840..2e8e8ab 100644 --- a/src/mesa/drivers/dri/i965/gen6_blorp.cpp +++ b/src/mesa/drivers/dri/i965/gen6_blorp.cpp @@ -1015,19 +1015,6 @@ gen6_blorp_emit_primitive(struct brw_context *brw, brw->batch.need_workaround_flush = true; } -static void -gen6_emit_hiz_workaround(struct brw_context *brw, enum gen6_hiz_op hiz_op) -{ - /* This fixes a HiZ hang in WebGL Google Maps. A more minimal fix likely -* exists, but this gets the job done. -*/ - if (hiz_op == GEN6_HIZ_OP_DEPTH_RESOLVE || - hiz_op == GEN6_HIZ_OP_HIZ_RESOLVE) { - intel_emit_post_sync_nonzero_flush(brw); - intel_emit_depth_stall_flushes(brw); - } -} - /** * \brief Execute a blit or render pass operation. * @@ -1053,7 +1040,6 @@ gen6_blorp_exec(struct brw_context *brw, /* Emit workaround flushes when we switch from drawing to blorping. */ brw->batch.need_workaround_flush = true; - gen6_emit_hiz_workaround(brw, params->hiz_op); gen6_emit_3dstate_multisample(brw, params->dst.num_samples); gen6_emit_3dstate_sample_mask(brw, params->dst.num_samples > 1 ? -- 1.8.5.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/3] i965/gen6/blorp: Set need_workaround_flush at top of blorp (v2)
Unconditionally set brw->need_workaround_flush at the top of gen6 blorp state emission. The art of emitting workaround flushes on Sandybridge is mysterious and not fully understood. Ken and I believe that intel_emit_post_sync_nonzero_flush() may be required when switching from regular drawing to blorp. This is an extra safety measure to prevent undiscovered difficult-to-diagnose gpu hangs. I verified that on ChromeOS, pre-patch, need_workaround_flush was not set at the top of blorp, as Paul expected. To verify, I inserted the following debug code at the top of gen6_blorp_exec(), restarted the ui, and inspected the logs in /var/log/ui. The abort gets triggered so early that the browser never appears on the display. static void gen6_blorp_exec(...) { if (!brw->need_workaround_flush) { fprintf(stderr, "chadv: %s:%d\n", __FILE__, __LINE__); abort(); } ... } v2: Explain how I determined that need_workaround_flush wasn't getting set when expected. CC: Kenneth Graunke CC: Paul Berry CC: Stéphane Marchesin Signed-off-by: Chad Versace --- src/mesa/drivers/dri/i965/gen6_blorp.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/mesa/drivers/dri/i965/gen6_blorp.cpp b/src/mesa/drivers/dri/i965/gen6_blorp.cpp index 929d7b5..9db0840 100644 --- a/src/mesa/drivers/dri/i965/gen6_blorp.cpp +++ b/src/mesa/drivers/dri/i965/gen6_blorp.cpp @@ -1023,7 +1023,6 @@ gen6_emit_hiz_workaround(struct brw_context *brw, enum gen6_hiz_op hiz_op) */ if (hiz_op == GEN6_HIZ_OP_DEPTH_RESOLVE || hiz_op == GEN6_HIZ_OP_HIZ_RESOLVE) { - brw->batch.need_workaround_flush = true; intel_emit_post_sync_nonzero_flush(brw); intel_emit_depth_stall_flushes(brw); } @@ -1051,6 +1050,9 @@ gen6_blorp_exec(struct brw_context *brw, uint32_t prog_offset = params->get_wm_prog(brw, &prog_data); + /* Emit workaround flushes when we switch from drawing to blorping. */ + brw->batch.need_workaround_flush = true; + gen6_emit_hiz_workaround(brw, params->hiz_op); gen6_emit_3dstate_multisample(brw, params->dst.num_samples); gen6_emit_3dstate_sample_mask(brw, -- 1.8.5.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 0/3] i965/gen6: Emit more flushes
In solving a HiZ hang before Christmas vacation, I discovered that Mesa wasn't emitting sufficient workaround flushes. That bug was solved in commit 1a92881. This series, prompted by Ken, adds some more carefully placed flushes to pre-emptively fix undiscovered gpu hangs. Gpu hangs are difficult to diagnose, hurt users pretty hard, and are overall nasty, so it's a good idea to put safeguards into the driver to prevent them. v2: - Rewrite patch 1 for Paul. - Explain more in patch 2 commit message to persuade Paul. - Remove CC mesa-stable. I'll send to stable after review completes and they get committed to master. Chad Versace (3): i965/gen6/blorp: Set need_workaround_flush immediately after primitive i965/gen6/blorp: Set need_workaround_flush at top of blorp i965/gen6/blorp: Remove redundant HiZ workaround src/mesa/drivers/dri/i965/brw_blorp.cpp | 1 - src/mesa/drivers/dri/i965/gen6_blorp.cpp | 19 +-- 2 files changed, 5 insertions(+), 15 deletions(-) -- 1.8.5.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/3] i965/gen6/blorp: Set need_workaround_flush immediately after primitive
This patch makes the workaround code in gen6 blorp follow the pattern established in the regular draw path. It shouldn't result in any behavioral change. On gen6, there are two places where we emit 3D_CMD_PRIM: brw_emit_prim() and gen6_blorp_emit_primitive(). brw_emit_prim() sets need_workaround_flush immediately after emitting the primitive, but blorp does not. Blorp sets need_workaround_flush at the bottom of brw_blorp_exec(). This patch moves the need_workaround_flush from brw_blorp_exec() to gen6_blorp_emit_primitive(). There is no need to set need_workaround_flush in gen7_blorp_emit_primitive() because the workaround applies only to gen6. Reviewed-by: Paul Berry Signed-off-by: Chad Versace --- src/mesa/drivers/dri/i965/brw_blorp.cpp | 1 - src/mesa/drivers/dri/i965/gen6_blorp.cpp | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/mesa/drivers/dri/i965/brw_blorp.cpp b/src/mesa/drivers/dri/i965/brw_blorp.cpp index ab3e75c..0939a31 100644 --- a/src/mesa/drivers/dri/i965/brw_blorp.cpp +++ b/src/mesa/drivers/dri/i965/brw_blorp.cpp @@ -274,7 +274,6 @@ retry: */ brw->state.dirty.brw = ~0; brw->state.dirty.cache = ~0; - brw->batch.need_workaround_flush = true; brw->ib.type = -1; intel_batchbuffer_clear_cache(brw); diff --git a/src/mesa/drivers/dri/i965/gen6_blorp.cpp b/src/mesa/drivers/dri/i965/gen6_blorp.cpp index 441d61f..929d7b5 100644 --- a/src/mesa/drivers/dri/i965/gen6_blorp.cpp +++ b/src/mesa/drivers/dri/i965/gen6_blorp.cpp @@ -1010,6 +1010,9 @@ gen6_blorp_emit_primitive(struct brw_context *brw, OUT_BATCH(0); OUT_BATCH(0); ADVANCE_BATCH(); + + /* Only used on Sandybridge; harmless to set elsewhere. */ + brw->batch.need_workaround_flush = true; } static void -- 1.8.5.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 73371] EGL_NOK_texture_from_pixmap is advertised but not properly set.
https://bugs.freedesktop.org/show_bug.cgi?id=73371 --- Comment #2 from Carsten Haitzler --- oh - thanks gustavo. someone else was filing a bug too. i've also prepared a workaround for now where we blacklist mesa + intel drvires in egl mode from detecting this extension. i will need to know when this is fixed so i can add a version check to disable the blacklist in versions at and after the fix. -- 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 2/7] mesa: Store gl_shader_stage enum in gl_shader objects.
On 01/07/2014 02:13 PM, Paul Berry wrote: > This reduces confusion since gl_shader::Type is sometimes > GL_SHADER_PROGRAM_MESA but is more frequently > GL_SHADER_{VERTEX,GEOMETRY,FRAGMENT}. It also has the advantage that > when switching on gl_shader::Stage, the compiler will alert if one of > the possible enum types is unhandled. Finally, many functions in > src/glsl (especially those dealing with linking) already use > gl_shader_stage to represent pipeline stages; storing a > gl_shader_stage in the gl_shader object avoids the need for a > conversion. If you'll permit me a bit of grumbling... This patch does more than it says. The clear purpose of this patch is to add a new field. (Which is a non-trivial amount of work - moving definitions around, adding new code to set it in at least 4 places...) But then it goes and does a bunch of follow-on cleaning to /use/ the new field in a bunch of random places. This would've been great as a second patch...but alas. Not worth changing at this point, but for future reference, please...if you're going to rename something, rename that one thing. If you're going to add a field, add that one field. More patches are fine. [snip] > diff --git a/src/mesa/program/prog_print.c b/src/mesa/program/prog_print.c > index fa120cc..9391e99 100644 > --- a/src/mesa/program/prog_print.c > +++ b/src/mesa/program/prog_print.c > @@ -1005,16 +1005,21 @@ _mesa_print_parameter_list(const struct > gl_program_parameter_list *list) > void > _mesa_write_shader_to_file(const struct gl_shader *shader) > { > - const char *type; > + const char *type = ""; As far as I can tell this is a spurious change which is neither: - adding the new enum to the field (supposed purpose of patch) - changing things to use the new field directly (secondary change done in this patch) Other than those minor grumbles, though, I really like what you're doing here! As is, the series is: Reviewed-by: Kenneth Graunke > char filename[100]; > FILE *f; > > - if (shader->Type == GL_FRAGMENT_SHADER) > + switch (shader->Stage) { > + case MESA_SHADER_FRAGMENT: >type = "frag"; > - else if (shader->Type == GL_VERTEX_SHADER) > + break; > + case MESA_SHADER_VERTEX: >type = "vert"; > - else > + break; > + case MESA_SHADER_GEOMETRY: >type = "geom"; > + break; > + } > > _mesa_snprintf(filename, sizeof(filename), "shader_%u.%s", shader->Name, > type); > f = fopen(filename, "w"); > @@ -1061,7 +1066,7 @@ _mesa_append_uniforms_to_file(const struct gl_shader > *shader) > char filename[100]; > FILE *f; > > - if (shader->Type == GL_FRAGMENT_SHADER) > + if (shader->Stage == MESA_SHADER_FRAGMENT) >type = "frag"; > else >type = "vert"; > diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp > b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp > index b2131ed..bd4eb5e 100644 > --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp > +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp > @@ -5207,6 +5207,7 @@ st_new_shader(struct gl_context *ctx, GLuint name, > GLuint type) > shader = rzalloc(NULL, struct gl_shader); > if (shader) { >shader->Type = type; > + shader->Stage = _mesa_shader_enum_to_shader_stage(type); >shader->Name = name; >_mesa_init_shader(ctx, shader); > } > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/4] MSVC2013: Work around internal compiler error
On 2014-01-07 23:05, Ian Romanick wrote: On 01/07/2014 12:31 PM, Thomas Sondergaard wrote: This small rearrangement avoids MSVC 2013 ICE. Also, this should be a better memory access order. Can you explain this better? As far as I can tell, this just changes the order the indices are visited (and has a spurious, incorrect whitespace change). This shouldn't affect the correctness of the code at all. No, the error is in the MSVC 2013 c++ compiler which dies with an internal compiler error (ICE). It is not intended to affect the correctness of the code. I will also report the ICE to microsoft. Regards, Thomas ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/4] MSVC 2013: Preliminary support for MSVC_VERSION=12.0
--- common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common.py b/common.py index 1d618e6..22c1725 100644 --- a/common.py +++ b/common.py @@ -100,4 +100,4 @@ def AddOptions(opts): opts.Add(BoolOption('quiet', 'DEPRECATED: profile build', 'yes')) opts.Add(BoolOption('texture_float', 'enable floating-point textures and renderbuffers', 'no')) if host_platform == 'windows': - opts.Add(EnumOption('MSVC_VERSION', 'MS Visual C++ version', None, allowed_values=('7.1', '8.0', '9.0', '10.0', '11.0'))) + opts.Add(EnumOption('MSVC_VERSION', 'MS Visual C++ version', None, allowed_values=('7.1', '8.0', '9.0', '10.0', '11.0', '12.0'))) -- 1.8.4.msysgit.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/4] MSVC2013: Work around internal compiler error
This small rearrangement avoids MSVC 2013 ICE. Also, this should be a better memory access order. --- src/gallium/drivers/softpipe/sp_quad_blend.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gallium/drivers/softpipe/sp_quad_blend.c b/src/gallium/drivers/softpipe/sp_quad_blend.c index 48d1a2e..d122586 100644 --- a/src/gallium/drivers/softpipe/sp_quad_blend.c +++ b/src/gallium/drivers/softpipe/sp_quad_blend.c @@ -860,8 +860,8 @@ clamp_colors(float (*quadColor)[4]) { unsigned i, j; - for (j = 0; j < TGSI_QUAD_SIZE; j++) { - for (i = 0; i < 4; i++) { + for (i = 0; i < 4; i++) { + for (j = 0; j < TGSI_QUAD_SIZE; j++) { quadColor[i][j] = CLAMP(quadColor[i][j], 0.0F, 1.0F); } } -- 1.8.4.msysgit.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/4] MSVC2013: Fix compile error with MSVC 2013
This fixes the following compile error: src\glsl\ir_constant_expression.cpp(1405) : error C2666: 'copysign' : 3 overloads have similar conversions --- src/glsl/ir_constant_expression.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/glsl/ir_constant_expression.cpp b/src/glsl/ir_constant_expression.cpp index 7ca865e..f811fd1 100644 --- a/src/glsl/ir_constant_expression.cpp +++ b/src/glsl/ir_constant_expression.cpp @@ -1402,7 +1402,7 @@ ir_expression::constant_expression_value(struct hash_table *variable_context) data.f[c] = ldexp(op[0]->value.f[c], op[1]->value.i[c]); /* Flush subnormal values to zero. */ if (!isnormal(data.f[c])) -data.f[c] = copysign(0.0, op[0]->value.f[c]); +data.f[c] = copysign(0.0f, op[0]->value.f[c]); } break; -- 1.8.4.msysgit.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/4] MSVC2013: Namespace qualify fma to override ambiguity with fma from math.h
MSVC 2013 version of math.h includes an fma() function. --- src/glsl/builtin_functions.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/glsl/builtin_functions.cpp b/src/glsl/builtin_functions.cpp index 10127f3..b3e407a 100644 --- a/src/glsl/builtin_functions.cpp +++ b/src/glsl/builtin_functions.cpp @@ -3936,7 +3936,7 @@ builtin_builder::_fma(const glsl_type *type) ir_variable *c = in_var(type, "c"); MAKE_SIG(type, gpu_shader5, 3, a, b, c); - body.emit(ret(fma(a, b, c))); + body.emit(ret(ir_builder::fma(a, b, c))); return sig; } -- 1.8.4.msysgit.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/4] MSVC2013: Namespace qualify fma to override ambiguity with fma from math.h
On 2014-01-07 23:45, Kenneth Graunke wrote: On 01/07/2014 12:31 PM, Thomas Sondergaard wrote: MSVC 2013 version of math.h includes an fma() function. --- src/glsl/builtin_functions.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/glsl/builtin_functions.cpp b/src/glsl/builtin_functions.cpp index 10127f3..b3e407a 100644 --- a/src/glsl/builtin_functions.cpp +++ b/src/glsl/builtin_functions.cpp @@ -3936,7 +3936,7 @@ builtin_builder::_fma(const glsl_type *type) ir_variable *c = in_var(type, "c"); MAKE_SIG(type, gpu_shader5, 3, a, b, c); - body.emit(ret(fma(a, b, c))); + body.emit(ret(ir_builder::fma(a, b, c))); return sig; } Shouldn't function overloading already make this work out, though? The fma() in math.h certainly doesn't have the same operand types. That's basically why we've gotten away with using incredibly generic names like abs()... --Ken Here is the compiler error from MSVC 2013: src\glsl\builtin_functions.cpp(3939) : error C2664: 'ir_return *ir_builder::ret(ir_builder::operand)' : cannot convert argument 1 from 'float' to 'ir_builder::operand' No constructor could take the source type, or constructor overload resolution was ambiguous scons: *** [build\windows-x86\glsl\builtin_functions.obj] Error 2 It means the compiler has selected the wrong overload, one that returns a float. There are three overloads in C:/Program Files (x86)/Microsoft Visual Studio 12.0/VC/include/math.h: _CRTIMP double __cdecl fma(_In_ double _X, _In_ double _Y, _In_ double _Z); inline float __CRTDECL fma(_In_ float _X, _In_ float _Y, _In_ float _Z) throw() inline long double __CRTDECL fma(_In_ long double _X, _In_ long double _Y, _In_ long double _Z) throw() I don't see how it can pick any of those with the given arguments, but the patch makes the problem go away. Regards, Thomas ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/4] MSVC2013: Work around internal compiler error
On 01/07/2014 03:30 PM, Ian Romanick wrote: On 01/07/2014 02:22 PM, Thomas Sondergaard wrote: On 2014-01-07 23:05, Ian Romanick wrote: On 01/07/2014 12:31 PM, Thomas Sondergaard wrote: This small rearrangement avoids MSVC 2013 ICE. Also, this should be a better memory access order. Can you explain this better? As far as I can tell, this just changes the order the indices are visited (and has a spurious, incorrect whitespace change). This shouldn't affect the correctness of the code at all. No, the error is in the MSVC 2013 c++ compiler which dies with an internal compiler error (ICE). It is not intended to affect the correctness of the code. I will also report the ICE to microsoft. Excellent. Hurray for finding compiler bugs. :) With the extra whitespace change removed, Reviewed-by: Ian Romanick I already fixed the whitespace change here. I'll hold off on pushing the series in light of the other comments. -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/7] glsl: Change _mesa_glsl_parse_state ctor to use gl_shader_stage enum.
On 01/07/2014 03:13 PM, Paul Berry wrote: --- src/glsl/glsl_parser_extras.cpp | 12 +--- src/glsl/glsl_parser_extras.h| 2 +- src/glsl/main.cpp| 2 +- src/glsl/test_optpass.cpp| 2 +- src/glsl/tests/builtin_variable_test.cpp | 2 +- src/mesa/main/ff_fragment_shader.cpp | 2 +- 6 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp index bdd8bf4..f9aa1d6 100644 --- a/src/glsl/glsl_parser_extras.cpp +++ b/src/glsl/glsl_parser_extras.cpp @@ -54,14 +54,12 @@ static unsigned known_desktop_glsl_versions[] = _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct gl_context *_ctx, - GLenum target, void *mem_ctx) + gl_shader_stage target, s/target/stage/ ?? + void *mem_ctx) : ctx(_ctx), switch_state() { - switch (target) { - case GL_VERTEX_SHADER: this->target = MESA_SHADER_VERTEX; break; - case GL_FRAGMENT_SHADER: this->target = MESA_SHADER_FRAGMENT; break; - case GL_GEOMETRY_SHADER: this->target = MESA_SHADER_GEOMETRY; break; - } + assert(target < MESA_SHADER_STAGES); + this->target = target; this->scanner = NULL; this->translation_unit.make_empty(); @@ -1479,7 +1477,7 @@ _mesa_glsl_compile_shader(struct gl_context *ctx, struct gl_shader *shader, bool dump_ast, bool dump_hir) { struct _mesa_glsl_parse_state *state = - new(shader) _mesa_glsl_parse_state(ctx, shader->Type, shader); + new(shader) _mesa_glsl_parse_state(ctx, shader->Stage, shader); const char *source = shader->Source; state->error = glcpp_preprocess(state, &source, &state->info_log, diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h index a9c5117..2a7ae3d 100644 --- a/src/glsl/glsl_parser_extras.h +++ b/src/glsl/glsl_parser_extras.h @@ -68,7 +68,7 @@ extern void _mesa_glsl_error(YYLTYPE *locp, _mesa_glsl_parse_state *state, struct _mesa_glsl_parse_state { - _mesa_glsl_parse_state(struct gl_context *_ctx, GLenum target, + _mesa_glsl_parse_state(struct gl_context *_ctx, gl_shader_stage target, s/target/stage/ void *mem_ctx); DECLARE_RALLOC_CXX_OPERATORS(_mesa_glsl_parse_state); diff --git a/src/glsl/main.cpp b/src/glsl/main.cpp index 3260c44..736689e 100644 --- a/src/glsl/main.cpp +++ b/src/glsl/main.cpp @@ -276,7 +276,7 @@ void compile_shader(struct gl_context *ctx, struct gl_shader *shader) { struct _mesa_glsl_parse_state *state = - new(shader) _mesa_glsl_parse_state(ctx, shader->Type, shader); + new(shader) _mesa_glsl_parse_state(ctx, shader->Stage, shader); _mesa_glsl_compile_shader(ctx, shader, dump_ast, dump_hir); diff --git a/src/glsl/test_optpass.cpp b/src/glsl/test_optpass.cpp index bdb1e8f..1a15f3c 100644 --- a/src/glsl/test_optpass.cpp +++ b/src/glsl/test_optpass.cpp @@ -209,7 +209,7 @@ int test_optpass(int argc, char **argv) string input = read_stdin_to_eof(); struct _mesa_glsl_parse_state *state - = new(shader) _mesa_glsl_parse_state(ctx, shader->Type, shader); + = new(shader) _mesa_glsl_parse_state(ctx, shader->Stage, shader); if (input_format_ir) { shader->ir = new(shader) exec_list; diff --git a/src/glsl/tests/builtin_variable_test.cpp b/src/glsl/tests/builtin_variable_test.cpp index 9b4a097..3fdfce5 100644 --- a/src/glsl/tests/builtin_variable_test.cpp +++ b/src/glsl/tests/builtin_variable_test.cpp @@ -68,7 +68,7 @@ common_builtin::SetUp() this->shader->Stage = _mesa_shader_enum_to_shader_stage(this->shader_type); this->state = - new(mem_ctx) _mesa_glsl_parse_state(&this->ctx, this->shader->Type, + new(mem_ctx) _mesa_glsl_parse_state(&this->ctx, this->shader->Stage, this->shader); _mesa_glsl_initialize_types(this->state); diff --git a/src/mesa/main/ff_fragment_shader.cpp b/src/mesa/main/ff_fragment_shader.cpp index ba6258d..00ca025 100644 --- a/src/mesa/main/ff_fragment_shader.cpp +++ b/src/mesa/main/ff_fragment_shader.cpp @@ -1296,7 +1296,7 @@ create_new_program(struct gl_context *ctx, struct state_key *key) p.mem_ctx = ralloc_context(NULL); p.shader = ctx->Driver.NewShader(ctx, 0, GL_FRAGMENT_SHADER); p.shader->ir = new(p.shader) exec_list; - state = new(p.shader) _mesa_glsl_parse_state(ctx, GL_FRAGMENT_SHADER, + state = new(p.shader) _mesa_glsl_parse_state(ctx, MESA_SHADER_FRAGMENT, p.shader); p.shader->symbols = state->symbols; p.top_instructions = p.shader->ir; ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/7] glsl: Make more use of gl_shader_stage enum in link_varyings.cpp.
On 01/07/2014 03:13 PM, Paul Berry wrote: --- src/glsl/link_varyings.cpp | 48 +++--- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp index da97e9f..eefd725 100644 --- a/src/glsl/link_varyings.cpp +++ b/src/glsl/link_varyings.cpp @@ -48,13 +48,13 @@ static void cross_validate_types_and_qualifiers(struct gl_shader_program *prog, const ir_variable *input, const ir_variable *output, -GLenum consumer_type, -GLenum producer_type) +gl_shader_stage consumer_stage, +gl_shader_stage producer_stage) { /* Check that the types match between stages. */ const glsl_type *type_to_match = input->type; - if (consumer_type == GL_GEOMETRY_SHADER) { + if (consumer_stage == MESA_SHADER_GEOMETRY) { assert(type_to_match->is_array()); /* Enforced by ast_to_hir */ type_to_match = type_to_match->element_type(); } @@ -82,10 +82,10 @@ cross_validate_types_and_qualifiers(struct gl_shader_program *prog, linker_error(prog, "%s shader output `%s' declared as type `%s', " "but %s shader input declared as type `%s'\n", - _mesa_progshader_enum_to_string(producer_type), + _mesa_shader_stage_to_string(producer_stage), output->name, output->type->name, - _mesa_progshader_enum_to_string(consumer_type), + _mesa_shader_stage_to_string(consumer_stage), input->type->name); return; } @@ -97,10 +97,10 @@ cross_validate_types_and_qualifiers(struct gl_shader_program *prog, linker_error(prog, "%s shader output `%s' %s centroid qualifier, " "but %s shader input %s centroid qualifier\n", - _mesa_progshader_enum_to_string(producer_type), + _mesa_shader_stage_to_string(producer_stage), output->name, (output->data.centroid) ? "has" : "lacks", - _mesa_progshader_enum_to_string(consumer_type), + _mesa_shader_stage_to_string(consumer_stage), (input->data.centroid) ? "has" : "lacks"); return; } @@ -109,10 +109,10 @@ cross_validate_types_and_qualifiers(struct gl_shader_program *prog, linker_error(prog, "%s shader output `%s' %s sample qualifier, " "but %s shader input %s sample qualifier\n", - _mesa_progshader_enum_to_string(producer_type), + _mesa_shader_stage_to_string(producer_stage), output->name, (output->data.sample) ? "has" : "lacks", - _mesa_progshader_enum_to_string(consumer_type), + _mesa_shader_stage_to_string(consumer_stage), (input->data.sample) ? "has" : "lacks"); return; } @@ -121,10 +121,10 @@ cross_validate_types_and_qualifiers(struct gl_shader_program *prog, linker_error(prog, "%s shader output `%s' %s invariant qualifier, " "but %s shader input %s invariant qualifier\n", - _mesa_progshader_enum_to_string(producer_type), + _mesa_shader_stage_to_string(producer_stage), output->name, (output->data.invariant) ? "has" : "lacks", - _mesa_progshader_enum_to_string(consumer_type), + _mesa_shader_stage_to_string(consumer_stage), (input->data.invariant) ? "has" : "lacks"); return; } @@ -135,10 +135,10 @@ cross_validate_types_and_qualifiers(struct gl_shader_program *prog, "interpolation qualifier, " "but %s shader input specifies %s " "interpolation qualifier\n", - _mesa_progshader_enum_to_string(producer_type), + _mesa_shader_stage_to_string(producer_stage), output->name, interpolation_string(output->data.interpolation), - _mesa_progshader_enum_to_string(consumer_type), + _mesa_shader_stage_to_string(consumer_stage), interpolation_string(input->data.interpolation)); return; } @@ -152,16 +152,16 @@ cross_validate_front_and_back_color(struct gl_shader_program *prog, const ir_variable *input, const ir_variable *front_color, const ir_variable *back
Re: [Mesa-dev] [PATCH 1/7] mesa: Clean up nomenclature for pipeline stages.
On 01/07/2014 03:13 PM, Paul Berry wrote: Previously, we had an enum called gl_shader_type which represented pipeline stages in the order they occur in the pipeline (i.e. MESA_SHADER_VERTEX=0, MESA_SHADER_GEOMETRY=1, etc), and several inconsistently named functions for converting between it and other representations: - _mesa_shader_type_to_string: gl_shader_type -> string - _mesa_shader_type_to_index: GLenum (GL_*_SHADER) -> gl_shader_type - _mesa_program_target_to_index: GLenum (GL_*_PROGRAM) -> gl_shader_type - _mesa_shader_enum_to_string: GLenum (GL_*_{SHADER,PROGRAM}) -> string This patch tries to clean things up so that we use more consistent terminology: the enum is now called gl_shader_stage (to emphasize that it is in the order of pipeline stages), and the conversion functions are: - _mesa_shader_stage_to_string: gl_shader_stage -> string - _mesa_shader_enum_to_shader_stage: GLenum (GL_*_SHADER) -> gl_shader_stage - _mesa_program_enum_to_shader_stage: GLenum (GL_*_PROGRAM) -> gl_shader_type - _mesa_progshader_enum_to_string: GLenum (GL_*_{SHADER,PROGRAM}) -> string In addition, MESA_SHADER_TYPES has been renamed to MESA_SHADER_STAGES, for consistency with the new name for the enum. This sounds great, Paul. Minor comments below and in a few other patches. Reviewed-by: Brian Paul --- src/glsl/ast_to_hir.cpp | 12 ++--- src/glsl/glsl_parser_extras.cpp | 14 +++--- src/glsl/glsl_parser_extras.h| 8 +-- src/glsl/ir_uniform.h| 2 +- src/glsl/link_atomics.cpp| 22 - src/glsl/link_uniform_initializers.cpp | 8 +-- src/glsl/link_uniforms.cpp | 12 ++--- src/glsl/link_varyings.cpp | 24 - src/glsl/linker.cpp | 62 src/glsl/main.cpp| 2 +- src/glsl/standalone_scaffolding.cpp | 2 +- src/glsl/standalone_scaffolding.h| 6 +-- src/glsl/test_optpass.cpp| 2 +- src/glsl/tests/set_uniform_initializer_tests.cpp | 4 +- src/mesa/drivers/dri/i965/brw_context.c | 2 +- src/mesa/drivers/dri/i965/brw_shader.cpp | 10 ++-- src/mesa/main/context.c | 8 +-- src/mesa/main/mtypes.h | 12 ++--- src/mesa/main/shaderapi.c| 14 +++--- src/mesa/main/shaderapi.h| 2 +- src/mesa/main/shaderobj.c| 4 +- src/mesa/main/shaderobj.h| 6 +-- src/mesa/main/uniform_query.cpp | 6 +-- src/mesa/main/uniforms.c | 4 +- src/mesa/program/ir_to_mesa.cpp | 18 +++ src/mesa/program/program.c | 2 +- src/mesa/program/program.h | 8 +-- src/mesa/program/sampler.cpp | 2 +- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 12 ++--- 29 files changed, 145 insertions(+), 145 deletions(-) diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp index 91810f9..9f1e4e6 100644 --- a/src/glsl/ast_to_hir.cpp +++ b/src/glsl/ast_to_hir.cpp @@ -1882,7 +1882,7 @@ ast_fully_specified_type::glsl_type(const char **name, * this function will produce undefined results. */ static bool -is_varying_var(ir_variable *var, gl_shader_type target) +is_varying_var(ir_variable *var, gl_shader_stage target) { switch (target) { case MESA_SHADER_VERTEX: @@ -2110,7 +2110,7 @@ validate_explicit_location(const struct ast_type_qualifier *qual, _mesa_glsl_error(loc, state, "%s cannot be given an explicit location in %s shader", mode_string(var), - _mesa_shader_type_to_string(state->target)); + _mesa_shader_stage_to_string(state->target)); } else { var->data.explicit_location = true; @@ -2188,7 +2188,7 @@ apply_type_qualifier_to_variable(const struct ast_type_qualifier *qual, _mesa_glsl_error(loc, state, "`attribute' variables may not be declared in the " "%s shader", - _mesa_shader_type_to_string(state->target)); + _mesa_shader_stage_to_string(state->target)); } /* Section 6.1.1 (Function Calling Conventions) of the GLSL 1.10 spec says: @@ -2599,7 +2599,7 @@ process_initializer(ir_variable *var, ast_declaration *decl, if ((var->data.mode == ir_var_shader_in) && (state->current_function == NULL)) { _mesa_glsl_error(& initializer_loc, state, "cannot initialize %s shader input / %s", - _mesa_shader_type_to_string(state->target), + _mesa_shader_stage_to_string(state->target), (st
Re: [Mesa-dev] [PATCH 2/7] mesa: Store gl_shader_stage enum in gl_shader objects.
On 01/07/2014 03:13 PM, Paul Berry wrote: This reduces confusion since gl_shader::Type is sometimes GL_SHADER_PROGRAM_MESA but is more frequently GL_SHADER_{VERTEX,GEOMETRY,FRAGMENT}. It also has the advantage that when switching on gl_shader::Stage, the compiler will alert if one of the possible enum types is unhandled. Finally, many functions in src/glsl (especially those dealing with linking) already use gl_shader_stage to represent pipeline stages; storing a gl_shader_stage in the gl_shader object avoids the need for a conversion. --- src/glsl/glsl_parser_extras.cpp | 4 +-- src/glsl/glsl_parser_extras.h| 15 +- src/glsl/link_interface_blocks.cpp | 2 +- src/glsl/link_varyings.cpp | 22 +++ src/glsl/linker.cpp | 16 +-- src/glsl/lower_packed_varyings.cpp | 2 +- src/glsl/main.cpp| 1 + src/glsl/opt_dead_builtin_varyings.cpp | 4 +-- src/glsl/standalone_scaffolding.cpp | 1 + src/glsl/test_optpass.cpp| 1 + src/glsl/tests/builtin_variable_test.cpp | 1 + src/mesa/drivers/dri/i965/brw_shader.cpp | 6 ++-- src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 2 +- src/mesa/main/mtypes.h | 35 src/mesa/main/shaderapi.c| 8 +++--- src/mesa/main/shaderobj.c| 1 + src/mesa/program/ir_to_mesa.cpp | 17 ++-- src/mesa/program/prog_print.c| 15 ++ src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 1 + 19 files changed, 83 insertions(+), 71 deletions(-) diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp index 492f2ac..bdd8bf4 100644 --- a/src/glsl/glsl_parser_extras.cpp +++ b/src/glsl/glsl_parser_extras.cpp @@ -1448,7 +1448,7 @@ static void set_shader_inout_layout(struct gl_shader *shader, struct _mesa_glsl_parse_state *state) { - if (shader->Type != GL_GEOMETRY_SHADER) { + if (shader->Stage != MESA_SHADER_GEOMETRY) { /* Should have been prevented by the parser. */ assert(!state->gs_input_prim_type_specified); assert(!state->out_qualifier->flags.i); @@ -1516,7 +1516,7 @@ _mesa_glsl_compile_shader(struct gl_context *ctx, struct gl_shader *shader, if (!state->error && !shader->ir->is_empty()) { struct gl_shader_compiler_options *options = - &ctx->ShaderCompilerOptions[_mesa_shader_enum_to_shader_stage(shader->Type)]; + &ctx->ShaderCompilerOptions[shader->Stage]; /* Do some optimization at compile time to reduce shader IR size * and reduce later work if the same shader is linked multiple times diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h index ecf832f..a9c5117 100644 --- a/src/glsl/glsl_parser_extras.h +++ b/src/glsl/glsl_parser_extras.h @@ -423,14 +423,6 @@ extern bool _mesa_glsl_process_extension(const char *name, YYLTYPE *name_locp, YYLTYPE *behavior_locp, _mesa_glsl_parse_state *state); -/** - * Get the textual name of the specified shader target (which is a - * gl_shader_stage). - */ -extern const char * -_mesa_shader_stage_to_string(unsigned target); - - #endif /* __cplusplus */ @@ -441,6 +433,13 @@ _mesa_shader_stage_to_string(unsigned target); extern "C" { #endif +/** + * Get the textual name of the specified shader target (which is a + * gl_shader_stage). + */ +extern const char * +_mesa_shader_stage_to_string(unsigned target); + extern const char * _mesa_progshader_enum_to_string(GLenum type); diff --git a/src/glsl/link_interface_blocks.cpp b/src/glsl/link_interface_blocks.cpp index 4769636..52552cc 100644 --- a/src/glsl/link_interface_blocks.cpp +++ b/src/glsl/link_interface_blocks.cpp @@ -313,7 +313,7 @@ validate_interstage_inout_blocks(struct gl_shader_program *prog, const gl_shader *consumer) { interface_block_definitions definitions; - const bool extra_array_level = consumer->Type == GL_GEOMETRY_SHADER; + const bool extra_array_level = consumer->Stage == MESA_SHADER_GEOMETRY; /* Add input interfaces from the consumer to the symbol table. */ foreach_list(node, consumer->ir) { diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp index ab40d45..da97e9f 100644 --- a/src/glsl/link_varyings.cpp +++ b/src/glsl/link_varyings.cpp @@ -1072,7 +1072,7 @@ assign_varying_locations(struct gl_context *ctx, const unsigned producer_base = VARYING_SLOT_VAR0; const unsigned consumer_base = VARYING_SLOT_VAR0; varying_matches matches(ctx->Const.DisableVaryingPacking, - consumer && consumer->Type == GL_FRAGMENT_SHADER); +
Re: [Mesa-dev] [PATCH 1/7] mesa: Clean up nomenclature for pipeline stages.
On 01/07/2014 02:13 PM, Paul Berry wrote: > Previously, we had an enum called gl_shader_type which represented > pipeline stages in the order they occur in the pipeline > (i.e. MESA_SHADER_VERTEX=0, MESA_SHADER_GEOMETRY=1, etc), and several > inconsistently named functions for converting between it and other > representations: > > - _mesa_shader_type_to_string: gl_shader_type -> string > - _mesa_shader_type_to_index: GLenum (GL_*_SHADER) -> gl_shader_type > - _mesa_program_target_to_index: GLenum (GL_*_PROGRAM) -> gl_shader_type > - _mesa_shader_enum_to_string: GLenum (GL_*_{SHADER,PROGRAM}) -> string > > This patch tries to clean things up so that we use more consistent > terminology: the enum is now called gl_shader_stage (to emphasize that > it is in the order of pipeline stages), and the conversion functions are: > > - _mesa_shader_stage_to_string: gl_shader_stage -> string > - _mesa_shader_enum_to_shader_stage: GLenum (GL_*_SHADER) -> gl_shader_stage > - _mesa_program_enum_to_shader_stage: GLenum (GL_*_PROGRAM) -> gl_shader_type You mean "-> gl_shader_stage" on the line above. Patch 1 is: Reviewed-by: Kenneth Graunke > - _mesa_progshader_enum_to_string: GLenum (GL_*_{SHADER,PROGRAM}) -> string > > In addition, MESA_SHADER_TYPES has been renamed to MESA_SHADER_STAGES, > for consistency with the new name for the enum. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/4] MSVC2013: Namespace qualify fma to override ambiguity with fma from math.h
On 01/07/2014 12:31 PM, Thomas Sondergaard wrote: > MSVC 2013 version of math.h includes an fma() function. > --- > src/glsl/builtin_functions.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/glsl/builtin_functions.cpp b/src/glsl/builtin_functions.cpp > index 10127f3..b3e407a 100644 > --- a/src/glsl/builtin_functions.cpp > +++ b/src/glsl/builtin_functions.cpp > @@ -3936,7 +3936,7 @@ builtin_builder::_fma(const glsl_type *type) > ir_variable *c = in_var(type, "c"); > MAKE_SIG(type, gpu_shader5, 3, a, b, c); > > - body.emit(ret(fma(a, b, c))); > + body.emit(ret(ir_builder::fma(a, b, c))); > > return sig; > } > Shouldn't function overloading already make this work out, though? The fma() in math.h certainly doesn't have the same operand types. That's basically why we've gotten away with using incredibly generic names like abs()... --Ken ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] mesa: implement missing glGet(GL_RGBA_SIGNED_COMPONENTS_EXT) query
There are fixes on top of this fix. When they get picked over to the stable branch, I think at least this and the one from 050961.html should get squashed together. http://lists.freedesktop.org/archives/mesa-dev/2014-January/050961.html http://lists.freedesktop.org/archives/mesa-dev/2014-January/050962.html On 01/06/2014 11:57 AM, Brian Paul wrote: > This is part of the GL_EXT_packed_float extension. > > Bugzilla: http://bugs.freedesktop.org/show_bug.cgi?id=73096 > Cc: 10.0 > --- > src/mesa/main/formats.c | 19 +++ > src/mesa/main/formats.h |3 +++ > src/mesa/main/get.c | 21 + > src/mesa/main/get_hash_params.py |3 +++ > 4 files changed, 46 insertions(+) > > diff --git a/src/mesa/main/formats.c b/src/mesa/main/formats.c > index 07d2a72..eb2a07e 100644 > --- a/src/mesa/main/formats.c > +++ b/src/mesa/main/formats.c > @@ -1967,6 +1967,25 @@ _mesa_is_format_unsigned(gl_format format) > > > /** > + * Does the given format store signed values? > + */ > +GLboolean > +_mesa_is_format_signed(gl_format format) > +{ > + if (format == MESA_FORMAT_R11_G11_B10_FLOAT) { > + /* this packed float format only stores unsigned values */ > + return GL_FALSE; > + } > + else { > + const struct gl_format_info *info = _mesa_get_format_info(format); > + return (info->DataType == GL_SIGNED_NORMALIZED || > + info->DataType == GL_INT || > + info->DataType == GL_FLOAT); > + } > +} > + > + > +/** > * Return color encoding for given format. > * \return GL_LINEAR or GL_SRGB > */ > diff --git a/src/mesa/main/formats.h b/src/mesa/main/formats.h > index 64b4b9a..0c91aea 100644 > --- a/src/mesa/main/formats.h > +++ b/src/mesa/main/formats.h > @@ -341,6 +341,9 @@ _mesa_is_format_integer_color(gl_format format); > extern GLboolean > _mesa_is_format_unsigned(gl_format format); > > +extern GLboolean > +_mesa_is_format_signed(gl_format format); > + > extern GLenum > _mesa_get_format_color_encoding(gl_format format); > > diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c > index 6913808..596942c 100644 > --- a/src/mesa/main/get.c > +++ b/src/mesa/main/get.c > @@ -327,6 +327,12 @@ static const int > extra_EXT_framebuffer_sRGB_and_new_buffers[] = { > EXTRA_END > }; > > +static const int extra_EXT_packed_float[] = { > + EXT(EXT_packed_float), > + EXTRA_NEW_BUFFERS, > + EXTRA_END > +}; > + > static const int extra_EXT_texture_array_es3[] = { > EXT(EXT_texture_array), > EXTRA_API_ES3, > @@ -757,6 +763,21 @@ find_custom_value(struct gl_context *ctx, const struct > value_desc *d, union valu >ctx->Texture.Unit[unit].CurrentTex[d->offset]->Name; >break; > > + /* GL_EXT_packed_float */ > + case GL_RGBA_SIGNED_COMPONENTS_EXT: > + { > + /* Note: we only check the 0th color attachment. */ > + const struct gl_renderbuffer *rb = > +ctx->DrawBuffer->_ColorDrawBuffers[0]; > + const GLboolean signd = > +rb ? _mesa_is_format_signed(rb->Format) : GL_FALSE; > + v->value_int_4[0] = > + v->value_int_4[1] = > + v->value_int_4[2] = > + v->value_int_4[3] = signd; > + } > + break; > + > /* GL_ARB_vertex_buffer_object */ > case GL_VERTEX_ARRAY_BUFFER_BINDING_ARB: > case GL_NORMAL_ARRAY_BUFFER_BINDING_ARB: > diff --git a/src/mesa/main/get_hash_params.py > b/src/mesa/main/get_hash_params.py > index 653bf62..f763e09 100644 > --- a/src/mesa/main/get_hash_params.py > +++ b/src/mesa/main/get_hash_params.py > @@ -611,6 +611,9 @@ descriptor=[ > # GL_ARB_fragment_program >[ "FRAGMENT_PROGRAM_ARB", "CONTEXT_BOOL(FragmentProgram.Enabled), > extra_ARB_fragment_program" ], > > +# GL_EXT_packed_depth_stencil > + [ "RGBA_SIGNED_COMPONENTS_EXT", "LOC_CUSTOM, TYPE_INT_4, 0, > extra_EXT_packed_float" ], > + > # GL_EXT_depth_bounds_test >[ "DEPTH_BOUNDS_TEST_EXT", "CONTEXT_BOOL(Depth.BoundsTest), > extra_EXT_depth_bounds_test" ], >[ "DEPTH_BOUNDS_EXT", "CONTEXT_FLOAT2(Depth.BoundsMin), > extra_EXT_depth_bounds_test" ], > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/4] MSVC2013: Work around internal compiler error
On 01/07/2014 02:22 PM, Thomas Sondergaard wrote: > On 2014-01-07 23:05, Ian Romanick wrote: >> On 01/07/2014 12:31 PM, Thomas Sondergaard wrote: >>> This small rearrangement avoids MSVC 2013 ICE. Also, this should be a >>> better >>> memory access order. >> >> Can you explain this better? As far as I can tell, this just changes >> the order the indices are visited (and has a spurious, incorrect >> whitespace change). This shouldn't affect the correctness of the code >> at all. > > No, the error is in the MSVC 2013 c++ compiler which dies with an > internal compiler error (ICE). It is not intended to affect the > correctness of the code. I will also report the ICE to microsoft. Excellent. Hurray for finding compiler bugs. :) With the extra whitespace change removed, Reviewed-by: Ian Romanick > Regards, > Thomas ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/4] MSVC2013: Work around internal compiler error
On Tue, Jan 7, 2014 at 2:05 PM, Ian Romanick wrote: > On 01/07/2014 12:31 PM, Thomas Sondergaard wrote: >> This small rearrangement avoids MSVC 2013 ICE. Also, this should be a better >> memory access order. > > Can you explain this better? As far as I can tell, this just changes > the order the indices are visited (and has a spurious, incorrect > whitespace change). This shouldn't affect the correctness of the code > at all. I can't which thing you're asking about (the ICE, or the memory access pattern), but the memory access pattern claim seems correct. He's reversing the loops, so that consecutive iterations of the inner loop operate on contiguous memory. The correctness of the code doesn't ever seem to have been in question. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/egl: Flush resources before presentation
Hi Marek, Since it seems no one else have any comments on this, maybe you could commit it for me? //Martin On Thu, Dec 26, 2013 at 1:15 PM, Marek Olšák wrote: > Reviewed-by: Marek Olšák > > Marek > > On Thu, Dec 26, 2013 at 10:33 AM, Martin Andersson wrote: >> Fixes wayland regression on r600g due to fast clear introduced by commit >> edbbfac6. >> --- >> src/gallium/state_trackers/egl/common/native_helper.c | 15 +++ >> src/gallium/state_trackers/egl/common/native_helper.h | 5 + >> src/gallium/state_trackers/egl/wayland/native_wayland.c | 4 >> 3 files changed, 24 insertions(+) >> >> diff --git a/src/gallium/state_trackers/egl/common/native_helper.c >> b/src/gallium/state_trackers/egl/common/native_helper.c >> index 4a77a50..856cbb6 100644 >> --- a/src/gallium/state_trackers/egl/common/native_helper.c >> +++ b/src/gallium/state_trackers/egl/common/native_helper.c >> @@ -341,6 +341,21 @@ resource_surface_throttle(struct resource_surface >> *rsurf) >> } >> >> boolean >> +resource_surface_flush_resource(struct resource_surface *rsurf, >> +struct native_display *ndpy, >> +enum native_attachment which) >> +{ >> + struct pipe_context *pipe = ndpy_get_copy_context(ndpy); >> + >> + if (!pipe) >> + return FALSE; >> + >> + pipe->flush_resource(pipe, rsurf->resources[which]); >> + >> + return TRUE; >> +} >> + >> +boolean >> resource_surface_flush(struct resource_surface *rsurf, >>struct native_display *ndpy) >> { >> diff --git a/src/gallium/state_trackers/egl/common/native_helper.h >> b/src/gallium/state_trackers/egl/common/native_helper.h >> index 4c369a7..0b53b28 100644 >> --- a/src/gallium/state_trackers/egl/common/native_helper.h >> +++ b/src/gallium/state_trackers/egl/common/native_helper.h >> @@ -91,6 +91,11 @@ resource_surface_copy_swap(struct resource_surface *rsurf, >> boolean >> resource_surface_throttle(struct resource_surface *rsurf); >> >> +boolean >> +resource_surface_flush_resource(struct resource_surface *rsurf, >> +struct native_display *ndpy, >> +enum native_attachment which); >> + >> /** >> * Flush pending rendering using the copy context. This function saves a >> * marker for upcoming throttles. >> diff --git a/src/gallium/state_trackers/egl/wayland/native_wayland.c >> b/src/gallium/state_trackers/egl/wayland/native_wayland.c >> index cfdf4f8..0ab4be6 100644 >> --- a/src/gallium/state_trackers/egl/wayland/native_wayland.c >> +++ b/src/gallium/state_trackers/egl/wayland/native_wayland.c >> @@ -259,6 +259,10 @@ wayland_surface_swap_buffers(struct native_surface >> *nsurf) >> if (ret == -1) >>return EGL_FALSE; >> >> + (void) resource_surface_flush_resource(surface->rsurf, &display->base, >> + NATIVE_ATTACHMENT_BACK_LEFT); >> + (void) resource_surface_flush(surface->rsurf, &display->base); >> + >> surface->frame_callback = wl_surface_frame(surface->win->surface); >> wl_callback_add_listener(surface->frame_callback, &frame_listener, >> surface); >> wl_proxy_set_queue((struct wl_proxy *) surface->frame_callback, >> -- >> 1.8.5.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] [PATCH 2/7] mesa: Store gl_shader_stage enum in gl_shader objects.
This reduces confusion since gl_shader::Type is sometimes GL_SHADER_PROGRAM_MESA but is more frequently GL_SHADER_{VERTEX,GEOMETRY,FRAGMENT}. It also has the advantage that when switching on gl_shader::Stage, the compiler will alert if one of the possible enum types is unhandled. Finally, many functions in src/glsl (especially those dealing with linking) already use gl_shader_stage to represent pipeline stages; storing a gl_shader_stage in the gl_shader object avoids the need for a conversion. --- src/glsl/glsl_parser_extras.cpp | 4 +-- src/glsl/glsl_parser_extras.h| 15 +- src/glsl/link_interface_blocks.cpp | 2 +- src/glsl/link_varyings.cpp | 22 +++ src/glsl/linker.cpp | 16 +-- src/glsl/lower_packed_varyings.cpp | 2 +- src/glsl/main.cpp| 1 + src/glsl/opt_dead_builtin_varyings.cpp | 4 +-- src/glsl/standalone_scaffolding.cpp | 1 + src/glsl/test_optpass.cpp| 1 + src/glsl/tests/builtin_variable_test.cpp | 1 + src/mesa/drivers/dri/i965/brw_shader.cpp | 6 ++-- src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 2 +- src/mesa/main/mtypes.h | 35 src/mesa/main/shaderapi.c| 8 +++--- src/mesa/main/shaderobj.c| 1 + src/mesa/program/ir_to_mesa.cpp | 17 ++-- src/mesa/program/prog_print.c| 15 ++ src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 1 + 19 files changed, 83 insertions(+), 71 deletions(-) diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp index 492f2ac..bdd8bf4 100644 --- a/src/glsl/glsl_parser_extras.cpp +++ b/src/glsl/glsl_parser_extras.cpp @@ -1448,7 +1448,7 @@ static void set_shader_inout_layout(struct gl_shader *shader, struct _mesa_glsl_parse_state *state) { - if (shader->Type != GL_GEOMETRY_SHADER) { + if (shader->Stage != MESA_SHADER_GEOMETRY) { /* Should have been prevented by the parser. */ assert(!state->gs_input_prim_type_specified); assert(!state->out_qualifier->flags.i); @@ -1516,7 +1516,7 @@ _mesa_glsl_compile_shader(struct gl_context *ctx, struct gl_shader *shader, if (!state->error && !shader->ir->is_empty()) { struct gl_shader_compiler_options *options = - &ctx->ShaderCompilerOptions[_mesa_shader_enum_to_shader_stage(shader->Type)]; + &ctx->ShaderCompilerOptions[shader->Stage]; /* Do some optimization at compile time to reduce shader IR size * and reduce later work if the same shader is linked multiple times diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h index ecf832f..a9c5117 100644 --- a/src/glsl/glsl_parser_extras.h +++ b/src/glsl/glsl_parser_extras.h @@ -423,14 +423,6 @@ extern bool _mesa_glsl_process_extension(const char *name, YYLTYPE *name_locp, YYLTYPE *behavior_locp, _mesa_glsl_parse_state *state); -/** - * Get the textual name of the specified shader target (which is a - * gl_shader_stage). - */ -extern const char * -_mesa_shader_stage_to_string(unsigned target); - - #endif /* __cplusplus */ @@ -441,6 +433,13 @@ _mesa_shader_stage_to_string(unsigned target); extern "C" { #endif +/** + * Get the textual name of the specified shader target (which is a + * gl_shader_stage). + */ +extern const char * +_mesa_shader_stage_to_string(unsigned target); + extern const char * _mesa_progshader_enum_to_string(GLenum type); diff --git a/src/glsl/link_interface_blocks.cpp b/src/glsl/link_interface_blocks.cpp index 4769636..52552cc 100644 --- a/src/glsl/link_interface_blocks.cpp +++ b/src/glsl/link_interface_blocks.cpp @@ -313,7 +313,7 @@ validate_interstage_inout_blocks(struct gl_shader_program *prog, const gl_shader *consumer) { interface_block_definitions definitions; - const bool extra_array_level = consumer->Type == GL_GEOMETRY_SHADER; + const bool extra_array_level = consumer->Stage == MESA_SHADER_GEOMETRY; /* Add input interfaces from the consumer to the symbol table. */ foreach_list(node, consumer->ir) { diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp index ab40d45..da97e9f 100644 --- a/src/glsl/link_varyings.cpp +++ b/src/glsl/link_varyings.cpp @@ -1072,7 +1072,7 @@ assign_varying_locations(struct gl_context *ctx, const unsigned producer_base = VARYING_SLOT_VAR0; const unsigned consumer_base = VARYING_SLOT_VAR0; varying_matches matches(ctx->Const.DisableVaryingPacking, - consumer && consumer->Type == GL_FRAGMENT_SHADER); + consumer && consumer->Stage == MESA_SHADER_FRAGMENT); hash_ta
[Mesa-dev] [PATCH 3/7] glsl: Change _mesa_glsl_parse_state ctor to use gl_shader_stage enum.
--- src/glsl/glsl_parser_extras.cpp | 12 +--- src/glsl/glsl_parser_extras.h| 2 +- src/glsl/main.cpp| 2 +- src/glsl/test_optpass.cpp| 2 +- src/glsl/tests/builtin_variable_test.cpp | 2 +- src/mesa/main/ff_fragment_shader.cpp | 2 +- 6 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp index bdd8bf4..f9aa1d6 100644 --- a/src/glsl/glsl_parser_extras.cpp +++ b/src/glsl/glsl_parser_extras.cpp @@ -54,14 +54,12 @@ static unsigned known_desktop_glsl_versions[] = _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct gl_context *_ctx, - GLenum target, void *mem_ctx) + gl_shader_stage target, + void *mem_ctx) : ctx(_ctx), switch_state() { - switch (target) { - case GL_VERTEX_SHADER: this->target = MESA_SHADER_VERTEX; break; - case GL_FRAGMENT_SHADER: this->target = MESA_SHADER_FRAGMENT; break; - case GL_GEOMETRY_SHADER: this->target = MESA_SHADER_GEOMETRY; break; - } + assert(target < MESA_SHADER_STAGES); + this->target = target; this->scanner = NULL; this->translation_unit.make_empty(); @@ -1479,7 +1477,7 @@ _mesa_glsl_compile_shader(struct gl_context *ctx, struct gl_shader *shader, bool dump_ast, bool dump_hir) { struct _mesa_glsl_parse_state *state = - new(shader) _mesa_glsl_parse_state(ctx, shader->Type, shader); + new(shader) _mesa_glsl_parse_state(ctx, shader->Stage, shader); const char *source = shader->Source; state->error = glcpp_preprocess(state, &source, &state->info_log, diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h index a9c5117..2a7ae3d 100644 --- a/src/glsl/glsl_parser_extras.h +++ b/src/glsl/glsl_parser_extras.h @@ -68,7 +68,7 @@ extern void _mesa_glsl_error(YYLTYPE *locp, _mesa_glsl_parse_state *state, struct _mesa_glsl_parse_state { - _mesa_glsl_parse_state(struct gl_context *_ctx, GLenum target, + _mesa_glsl_parse_state(struct gl_context *_ctx, gl_shader_stage target, void *mem_ctx); DECLARE_RALLOC_CXX_OPERATORS(_mesa_glsl_parse_state); diff --git a/src/glsl/main.cpp b/src/glsl/main.cpp index 3260c44..736689e 100644 --- a/src/glsl/main.cpp +++ b/src/glsl/main.cpp @@ -276,7 +276,7 @@ void compile_shader(struct gl_context *ctx, struct gl_shader *shader) { struct _mesa_glsl_parse_state *state = - new(shader) _mesa_glsl_parse_state(ctx, shader->Type, shader); + new(shader) _mesa_glsl_parse_state(ctx, shader->Stage, shader); _mesa_glsl_compile_shader(ctx, shader, dump_ast, dump_hir); diff --git a/src/glsl/test_optpass.cpp b/src/glsl/test_optpass.cpp index bdb1e8f..1a15f3c 100644 --- a/src/glsl/test_optpass.cpp +++ b/src/glsl/test_optpass.cpp @@ -209,7 +209,7 @@ int test_optpass(int argc, char **argv) string input = read_stdin_to_eof(); struct _mesa_glsl_parse_state *state - = new(shader) _mesa_glsl_parse_state(ctx, shader->Type, shader); + = new(shader) _mesa_glsl_parse_state(ctx, shader->Stage, shader); if (input_format_ir) { shader->ir = new(shader) exec_list; diff --git a/src/glsl/tests/builtin_variable_test.cpp b/src/glsl/tests/builtin_variable_test.cpp index 9b4a097..3fdfce5 100644 --- a/src/glsl/tests/builtin_variable_test.cpp +++ b/src/glsl/tests/builtin_variable_test.cpp @@ -68,7 +68,7 @@ common_builtin::SetUp() this->shader->Stage = _mesa_shader_enum_to_shader_stage(this->shader_type); this->state = - new(mem_ctx) _mesa_glsl_parse_state(&this->ctx, this->shader->Type, + new(mem_ctx) _mesa_glsl_parse_state(&this->ctx, this->shader->Stage, this->shader); _mesa_glsl_initialize_types(this->state); diff --git a/src/mesa/main/ff_fragment_shader.cpp b/src/mesa/main/ff_fragment_shader.cpp index ba6258d..00ca025 100644 --- a/src/mesa/main/ff_fragment_shader.cpp +++ b/src/mesa/main/ff_fragment_shader.cpp @@ -1296,7 +1296,7 @@ create_new_program(struct gl_context *ctx, struct state_key *key) p.mem_ctx = ralloc_context(NULL); p.shader = ctx->Driver.NewShader(ctx, 0, GL_FRAGMENT_SHADER); p.shader->ir = new(p.shader) exec_list; - state = new(p.shader) _mesa_glsl_parse_state(ctx, GL_FRAGMENT_SHADER, + state = new(p.shader) _mesa_glsl_parse_state(ctx, MESA_SHADER_FRAGMENT, p.shader); p.shader->symbols = state->symbols; p.top_instructions = p.shader->ir; -- 1.8.5.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 6/7] glsl: Make more use of gl_shader_stage enum in ir_set_program_inouts.cpp.
--- src/glsl/ir.h | 2 +- src/glsl/ir_set_program_inouts.cpp | 29 +++-- src/mesa/drivers/dri/i965/brw_shader.cpp | 2 +- src/mesa/program/ir_to_mesa.cpp| 2 +- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 2 +- 5 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/glsl/ir.h b/src/glsl/ir.h index 780959b..dfeda34 100644 --- a/src/glsl/ir.h +++ b/src/glsl/ir.h @@ -2345,7 +2345,7 @@ ir_has_call(ir_instruction *ir); extern void do_set_program_inouts(exec_list *instructions, struct gl_program *prog, - GLenum shader_type); + gl_shader_stage shader_stage); extern char * prototype_string(const glsl_type *return_type, const char *name, diff --git a/src/glsl/ir_set_program_inouts.cpp b/src/glsl/ir_set_program_inouts.cpp index 0b49eb2..5163eb2 100644 --- a/src/glsl/ir_set_program_inouts.cpp +++ b/src/glsl/ir_set_program_inouts.cpp @@ -46,10 +46,11 @@ namespace { class ir_set_program_inouts_visitor : public ir_hierarchical_visitor { public: - ir_set_program_inouts_visitor(struct gl_program *prog, GLenum shader_type) + ir_set_program_inouts_visitor(struct gl_program *prog, + gl_shader_stage shader_stage) { this->prog = prog; - this->shader_type = shader_type; + this->shader_stage = shader_stage; } ~ir_set_program_inouts_visitor() { @@ -67,7 +68,7 @@ private: bool try_mark_partial_variable(ir_variable *var, ir_rvalue *index); struct gl_program *prog; - GLenum shader_type; + gl_shader_stage shader_stage; }; } /* anonymous namespace */ @@ -124,13 +125,13 @@ void ir_set_program_inouts_visitor::mark_whole_variable(ir_variable *var) { const glsl_type *type = var->type; - if (this->shader_type == GL_GEOMETRY_SHADER && + if (this->shader_stage == MESA_SHADER_GEOMETRY && var->data.mode == ir_var_shader_in && type->is_array()) { type = type->fields.array; } mark(this->prog, var, 0, type->count_attribute_slots(), -this->shader_type == GL_FRAGMENT_SHADER); +this->shader_stage == MESA_SHADER_FRAGMENT); } /* Default handler: Mark all the locations in the variable as used. */ @@ -163,7 +164,7 @@ ir_set_program_inouts_visitor::try_mark_partial_variable(ir_variable *var, { const glsl_type *type = var->type; - if (this->shader_type == GL_GEOMETRY_SHADER && + if (this->shader_stage == MESA_SHADER_GEOMETRY && var->data.mode == ir_var_shader_in) { /* The only geometry shader input that is not an array is * gl_PrimitiveIDIn, and in that case, this code will never be reached, @@ -227,7 +228,7 @@ ir_set_program_inouts_visitor::try_mark_partial_variable(ir_variable *var, } mark(this->prog, var, index_as_constant->value.u[0] * elem_width, -elem_width, this->shader_type == GL_FRAGMENT_SHADER); +elem_width, this->shader_stage == MESA_SHADER_FRAGMENT); return true; } @@ -245,7 +246,7 @@ ir_set_program_inouts_visitor::visit_enter(ir_dereference_array *ir) */ if (ir_dereference_variable * const deref_var = inner_array->array->as_dereference_variable()) { - if (this->shader_type == GL_GEOMETRY_SHADER && + if (this->shader_stage == MESA_SHADER_GEOMETRY && deref_var->var->data.mode == ir_var_shader_in) { /* foo is a geometry shader input, so i is the vertex, and j the * part of the input we're accessing. @@ -264,7 +265,7 @@ ir_set_program_inouts_visitor::visit_enter(ir_dereference_array *ir) } else if (ir_dereference_variable * const deref_var = ir->array->as_dereference_variable()) { /* ir => foo[i], where foo is a variable. */ - if (this->shader_type == GL_GEOMETRY_SHADER && + if (this->shader_stage == MESA_SHADER_GEOMETRY && deref_var->var->data.mode == ir_var_shader_in) { /* foo is a geometry shader input, so i is the vertex, and we're * accessing the entire input. @@ -304,7 +305,7 @@ ir_set_program_inouts_visitor::visit_enter(ir_function_signature *ir) ir_visitor_status ir_set_program_inouts_visitor::visit_enter(ir_expression *ir) { - if (this->shader_type == GL_FRAGMENT_SHADER && + if (this->shader_stage == MESA_SHADER_FRAGMENT && ir->operation == ir_unop_dFdy) { gl_fragment_program *fprog = (gl_fragment_program *) prog; fprog->UsesDFdy = true; @@ -316,7 +317,7 @@ ir_visitor_status ir_set_program_inouts_visitor::visit_enter(ir_discard *) { /* discards are only allowed in fragment shaders. */ - assert(this->shader_type == GL_FRAGMENT_SHADER); + assert(this->shader_stage == MESA_SHADER_FRAGMENT); gl_fragment_program *fprog = (gl_fragment_program *) prog; fprog->UsesKill = true; @@ -334,14 +335,14 @@ ir_set_program_inouts_visitor::visit_enter(ir_texture *ir) void do_set_program_inouts(exec_l
[Mesa-dev] [PATCH 1/7] mesa: Clean up nomenclature for pipeline stages.
Previously, we had an enum called gl_shader_type which represented pipeline stages in the order they occur in the pipeline (i.e. MESA_SHADER_VERTEX=0, MESA_SHADER_GEOMETRY=1, etc), and several inconsistently named functions for converting between it and other representations: - _mesa_shader_type_to_string: gl_shader_type -> string - _mesa_shader_type_to_index: GLenum (GL_*_SHADER) -> gl_shader_type - _mesa_program_target_to_index: GLenum (GL_*_PROGRAM) -> gl_shader_type - _mesa_shader_enum_to_string: GLenum (GL_*_{SHADER,PROGRAM}) -> string This patch tries to clean things up so that we use more consistent terminology: the enum is now called gl_shader_stage (to emphasize that it is in the order of pipeline stages), and the conversion functions are: - _mesa_shader_stage_to_string: gl_shader_stage -> string - _mesa_shader_enum_to_shader_stage: GLenum (GL_*_SHADER) -> gl_shader_stage - _mesa_program_enum_to_shader_stage: GLenum (GL_*_PROGRAM) -> gl_shader_type - _mesa_progshader_enum_to_string: GLenum (GL_*_{SHADER,PROGRAM}) -> string In addition, MESA_SHADER_TYPES has been renamed to MESA_SHADER_STAGES, for consistency with the new name for the enum. --- src/glsl/ast_to_hir.cpp | 12 ++--- src/glsl/glsl_parser_extras.cpp | 14 +++--- src/glsl/glsl_parser_extras.h| 8 +-- src/glsl/ir_uniform.h| 2 +- src/glsl/link_atomics.cpp| 22 - src/glsl/link_uniform_initializers.cpp | 8 +-- src/glsl/link_uniforms.cpp | 12 ++--- src/glsl/link_varyings.cpp | 24 - src/glsl/linker.cpp | 62 src/glsl/main.cpp| 2 +- src/glsl/standalone_scaffolding.cpp | 2 +- src/glsl/standalone_scaffolding.h| 6 +-- src/glsl/test_optpass.cpp| 2 +- src/glsl/tests/set_uniform_initializer_tests.cpp | 4 +- src/mesa/drivers/dri/i965/brw_context.c | 2 +- src/mesa/drivers/dri/i965/brw_shader.cpp | 10 ++-- src/mesa/main/context.c | 8 +-- src/mesa/main/mtypes.h | 12 ++--- src/mesa/main/shaderapi.c| 14 +++--- src/mesa/main/shaderapi.h| 2 +- src/mesa/main/shaderobj.c| 4 +- src/mesa/main/shaderobj.h| 6 +-- src/mesa/main/uniform_query.cpp | 6 +-- src/mesa/main/uniforms.c | 4 +- src/mesa/program/ir_to_mesa.cpp | 18 +++ src/mesa/program/program.c | 2 +- src/mesa/program/program.h | 8 +-- src/mesa/program/sampler.cpp | 2 +- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 12 ++--- 29 files changed, 145 insertions(+), 145 deletions(-) diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp index 91810f9..9f1e4e6 100644 --- a/src/glsl/ast_to_hir.cpp +++ b/src/glsl/ast_to_hir.cpp @@ -1882,7 +1882,7 @@ ast_fully_specified_type::glsl_type(const char **name, * this function will produce undefined results. */ static bool -is_varying_var(ir_variable *var, gl_shader_type target) +is_varying_var(ir_variable *var, gl_shader_stage target) { switch (target) { case MESA_SHADER_VERTEX: @@ -2110,7 +2110,7 @@ validate_explicit_location(const struct ast_type_qualifier *qual, _mesa_glsl_error(loc, state, "%s cannot be given an explicit location in %s shader", mode_string(var), - _mesa_shader_type_to_string(state->target)); + _mesa_shader_stage_to_string(state->target)); } else { var->data.explicit_location = true; @@ -2188,7 +2188,7 @@ apply_type_qualifier_to_variable(const struct ast_type_qualifier *qual, _mesa_glsl_error(loc, state, "`attribute' variables may not be declared in the " "%s shader", - _mesa_shader_type_to_string(state->target)); + _mesa_shader_stage_to_string(state->target)); } /* Section 6.1.1 (Function Calling Conventions) of the GLSL 1.10 spec says: @@ -2599,7 +2599,7 @@ process_initializer(ir_variable *var, ast_declaration *decl, if ((var->data.mode == ir_var_shader_in) && (state->current_function == NULL)) { _mesa_glsl_error(& initializer_loc, state, "cannot initialize %s shader input / %s", - _mesa_shader_type_to_string(state->target), + _mesa_shader_stage_to_string(state->target), (state->target == MESA_SHADER_VERTEX) ? "attribute" : "varying"); } @@ -4890,7 +4890,7 @@ ast_interface_block::hir(exec_list *instructions, _mesa_glsl_error(
[Mesa-dev] [PATCH 4/7] glsl: Make more use of gl_shader_stage enum in link_varyings.cpp.
--- src/glsl/link_varyings.cpp | 48 +++--- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp index da97e9f..eefd725 100644 --- a/src/glsl/link_varyings.cpp +++ b/src/glsl/link_varyings.cpp @@ -48,13 +48,13 @@ static void cross_validate_types_and_qualifiers(struct gl_shader_program *prog, const ir_variable *input, const ir_variable *output, -GLenum consumer_type, -GLenum producer_type) +gl_shader_stage consumer_stage, +gl_shader_stage producer_stage) { /* Check that the types match between stages. */ const glsl_type *type_to_match = input->type; - if (consumer_type == GL_GEOMETRY_SHADER) { + if (consumer_stage == MESA_SHADER_GEOMETRY) { assert(type_to_match->is_array()); /* Enforced by ast_to_hir */ type_to_match = type_to_match->element_type(); } @@ -82,10 +82,10 @@ cross_validate_types_and_qualifiers(struct gl_shader_program *prog, linker_error(prog, "%s shader output `%s' declared as type `%s', " "but %s shader input declared as type `%s'\n", - _mesa_progshader_enum_to_string(producer_type), + _mesa_shader_stage_to_string(producer_stage), output->name, output->type->name, - _mesa_progshader_enum_to_string(consumer_type), + _mesa_shader_stage_to_string(consumer_stage), input->type->name); return; } @@ -97,10 +97,10 @@ cross_validate_types_and_qualifiers(struct gl_shader_program *prog, linker_error(prog, "%s shader output `%s' %s centroid qualifier, " "but %s shader input %s centroid qualifier\n", - _mesa_progshader_enum_to_string(producer_type), + _mesa_shader_stage_to_string(producer_stage), output->name, (output->data.centroid) ? "has" : "lacks", - _mesa_progshader_enum_to_string(consumer_type), + _mesa_shader_stage_to_string(consumer_stage), (input->data.centroid) ? "has" : "lacks"); return; } @@ -109,10 +109,10 @@ cross_validate_types_and_qualifiers(struct gl_shader_program *prog, linker_error(prog, "%s shader output `%s' %s sample qualifier, " "but %s shader input %s sample qualifier\n", - _mesa_progshader_enum_to_string(producer_type), + _mesa_shader_stage_to_string(producer_stage), output->name, (output->data.sample) ? "has" : "lacks", - _mesa_progshader_enum_to_string(consumer_type), + _mesa_shader_stage_to_string(consumer_stage), (input->data.sample) ? "has" : "lacks"); return; } @@ -121,10 +121,10 @@ cross_validate_types_and_qualifiers(struct gl_shader_program *prog, linker_error(prog, "%s shader output `%s' %s invariant qualifier, " "but %s shader input %s invariant qualifier\n", - _mesa_progshader_enum_to_string(producer_type), + _mesa_shader_stage_to_string(producer_stage), output->name, (output->data.invariant) ? "has" : "lacks", - _mesa_progshader_enum_to_string(consumer_type), + _mesa_shader_stage_to_string(consumer_stage), (input->data.invariant) ? "has" : "lacks"); return; } @@ -135,10 +135,10 @@ cross_validate_types_and_qualifiers(struct gl_shader_program *prog, "interpolation qualifier, " "but %s shader input specifies %s " "interpolation qualifier\n", - _mesa_progshader_enum_to_string(producer_type), + _mesa_shader_stage_to_string(producer_stage), output->name, interpolation_string(output->data.interpolation), - _mesa_progshader_enum_to_string(consumer_type), + _mesa_shader_stage_to_string(consumer_stage), interpolation_string(input->data.interpolation)); return; } @@ -152,16 +152,16 @@ cross_validate_front_and_back_color(struct gl_shader_program *prog, const ir_variable *input, const ir_variable *front_color, const ir_variable *back_color, -GLenum consumer_type, -
[Mesa-dev] [PATCH 5/7] glsl: Make more use of gl_shader_stage enum in lower_clip_distance.cpp.
--- src/glsl/lower_clip_distance.cpp | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/glsl/lower_clip_distance.cpp b/src/glsl/lower_clip_distance.cpp index bb4f6ab..2d6138d 100644 --- a/src/glsl/lower_clip_distance.cpp +++ b/src/glsl/lower_clip_distance.cpp @@ -54,10 +54,10 @@ namespace { class lower_clip_distance_visitor : public ir_rvalue_visitor { public: - explicit lower_clip_distance_visitor(GLenum shader_type) + explicit lower_clip_distance_visitor(gl_shader_stage shader_stage) : progress(false), old_clip_distance_1d_var(NULL), old_clip_distance_2d_var(NULL), new_clip_distance_1d_var(NULL), -new_clip_distance_2d_var(NULL), shader_type(shader_type) +new_clip_distance_2d_var(NULL), shader_stage(shader_stage) { } @@ -96,9 +96,9 @@ public: ir_variable *new_clip_distance_2d_var; /** -* Type of shader we are compiling (e.g. GL_VERTEX_SHADER) +* Type of shader we are compiling (e.g. MESA_SHADER_VERTEX) */ - const GLenum shader_type; + const gl_shader_stage shader_stage; }; } /* anonymous namespace */ @@ -142,7 +142,7 @@ lower_clip_distance_visitor::visit(ir_variable *ir) } else { /* 2D gl_ClipDistance (used for geometry input). */ assert(ir->data.mode == ir_var_shader_in && - this->shader_type == GL_GEOMETRY_SHADER_ARB); + this->shader_stage == MESA_SHADER_GEOMETRY); if (this->old_clip_distance_2d_var) return visit_continue; @@ -253,7 +253,7 @@ lower_clip_distance_visitor::is_clip_distance_vec8(ir_rvalue *ir) } if (this->old_clip_distance_2d_var) { /* 2D clip distance is only possible as a geometry input */ - assert(this->shader_type == GL_GEOMETRY_SHADER_ARB); + assert(this->shader_stage == MESA_SHADER_GEOMETRY); ir_dereference_array *array_ref = ir->as_dereference_array(); if (array_ref) { @@ -288,7 +288,7 @@ lower_clip_distance_visitor::lower_clip_distance_vec8(ir_rvalue *ir) } if (this->old_clip_distance_2d_var) { /* 2D clip distance is only possible as a geometry input */ - assert(this->shader_type == GL_GEOMETRY_SHADER_ARB); + assert(this->shader_stage == MESA_SHADER_GEOMETRY); ir_dereference_array *array_ref = ir->as_dereference_array(); if (array_ref) { @@ -536,7 +536,7 @@ lower_clip_distance_visitor::visit_leave(ir_call *ir) bool lower_clip_distance(gl_shader *shader) { - lower_clip_distance_visitor v(shader->Type); + lower_clip_distance_visitor v(shader->Stage); visit_list_elements(&v, shader->ir); -- 1.8.5.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 7/7] mesa: Remove _mesa_progshader_enum_to_string(), which is no longer used.
--- src/glsl/glsl_parser_extras.cpp| 29 - src/glsl/glsl_parser_extras.h | 3 --- src/mesa/main/uniform_query.cpp| 2 +- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 2 +- 4 files changed, 2 insertions(+), 34 deletions(-) diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp index f9aa1d6..311a332 100644 --- a/src/glsl/glsl_parser_extras.cpp +++ b/src/glsl/glsl_parser_extras.cpp @@ -329,35 +329,6 @@ _mesa_glsl_parse_state::process_version_directive(YYLTYPE *locp, int version, } } -extern "C" { - -/** - * Translate a GLenum to a short shader stage name for debug printouts and - * error messages. - * - * It recognizes the PROGRAM variants of the names so it can be used - * with a struct gl_program->Target, not just a struct - * gl_shader->Type. - */ -const char * -_mesa_progshader_enum_to_string(GLenum type) -{ - switch (type) { - case GL_VERTEX_SHADER: - case GL_VERTEX_PROGRAM_ARB: - return "vertex"; - case GL_FRAGMENT_SHADER: - case GL_FRAGMENT_PROGRAM_ARB: - return "fragment"; - case GL_GEOMETRY_SHADER: - return "geometry"; - default: - assert(!"Should not get here."); - return "unknown"; - } -} - -} /* extern "C" */ /** * Translate a gl_shader_stage to a short shader stage name for debug diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h index 2a7ae3d..abbbc00 100644 --- a/src/glsl/glsl_parser_extras.h +++ b/src/glsl/glsl_parser_extras.h @@ -440,9 +440,6 @@ extern "C" { extern const char * _mesa_shader_stage_to_string(unsigned target); -extern const char * -_mesa_progshader_enum_to_string(GLenum type); - extern int glcpp_preprocess(void *ctx, const char **shader, char **info_log, const struct gl_extensions *extensions, struct gl_context *gl_ctx); diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp index d90193e..82d7628 100644 --- a/src/mesa/main/uniform_query.cpp +++ b/src/mesa/main/uniform_query.cpp @@ -452,7 +452,7 @@ log_program_parameters(const struct gl_shader_program *shProg) const struct gl_program *const prog = shProg->_LinkedShaders[i]->Program; printf("Program %d %s shader parameters:\n", - shProg->Name, _mesa_progshader_enum_to_string(prog->Target)); + shProg->Name, _mesa_shader_stage_to_string(i)); for (unsigned j = 0; j < prog->Parameters->NumParameters; j++) { printf("%s: %p %f %f %f %f\n", prog->Parameters->Parameters[j].Name, diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp index 9cb97a0..c4dc77f 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp @@ -5143,7 +5143,7 @@ get_mesa_program(struct gl_context *ctx, if (ctx->Shader.Flags & GLSL_DUMP) { printf("\n"); printf("GLSL IR for linked %s program %d:\n", - _mesa_progshader_enum_to_string(shader->Type), + _mesa_shader_stage_to_string(shader->Stage), shader_program->Name); _mesa_print_ir(shader->ir, NULL); printf("\n"); -- 1.8.5.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] mesa: Clean up enums used to keep track of shader types.
In my early preparations for working on compute shaders, I've run into some stumbling blocks due to the way the GLSL compiler keeps track of which pipeline stage a given shader is being compiled for. In some places, it uses a GLenum to store values like GL_VERTEX_SHADER, GL_GEOMETRY_SHADER, or GL_COMPUTE_SHADER. In a small minority of places, it uses a GLenum to store values like GL_VERTEX_PROGRAM_ARB, GL_FRAGMENT_PROGRAM_ARB, or GL_GEOMETRY_PROGRAM_NV. In other places, it uses the enum gl_shader_type, which uses the values 0, 1, and 2 to represent vertex shaders, geometry shaders, and fragment shaders, respectively. To make matters worse, the functions for converting between these representations are inconsistenly named, so when it's necessary to convert from one to another, it's difficult to know which function to call. This patch series attempts to clean all that up by adopting a more consistent nomenclature, and preferring to use the gl_shader_type enum inside the shader compiler. There are three advantages of preferring gl_shader_type over the GLenums: 1. When switching on the shader type, the compiler will warn if one of the enum values is unhandled. 2. It can be used to index into an array of shader stages. 3. It avoids uncertainty between the use of GL_*_SHADER enums and GL_*_PROGRAM enums. The series also renames gl_shader_type to gl_shader_stage to emphasize that this enum represents shader stages in the order in which they appear in the pipeline (a fact which the linker takes advantage of). An additional advantage of this name change is that it makes it easier to make the nomenclature consistent, because the term "stage" isn't used very much in other parts of Mesa. [PATCH 1/7] mesa: Clean up nomenclature for pipeline stages. [PATCH 2/7] mesa: Store gl_shader_stage enum in gl_shader objects. [PATCH 3/7] glsl: Change _mesa_glsl_parse_state ctor to use gl_shader_stage enum. [PATCH 4/7] glsl: Make more use of gl_shader_stage enum in link_varyings.cpp. [PATCH 5/7] glsl: Make more use of gl_shader_stage enum in lower_clip_distance.cpp. [PATCH 6/7] glsl: Make more use of gl_shader_stage enum in ir_set_program_inouts.cpp. [PATCH 7/7] mesa: Remove _mesa_progshader_enum_to_string(), which is no longer used. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/4] MSVC2013: Work around internal compiler error
On 01/07/2014 12:31 PM, Thomas Sondergaard wrote: > This small rearrangement avoids MSVC 2013 ICE. Also, this should be a better > memory access order. Can you explain this better? As far as I can tell, this just changes the order the indices are visited (and has a spurious, incorrect whitespace change). This shouldn't affect the correctness of the code at all. > --- > src/gallium/drivers/softpipe/sp_quad_blend.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/gallium/drivers/softpipe/sp_quad_blend.c > b/src/gallium/drivers/softpipe/sp_quad_blend.c > index 48d1a2e..174e60c 100644 > --- a/src/gallium/drivers/softpipe/sp_quad_blend.c > +++ b/src/gallium/drivers/softpipe/sp_quad_blend.c > @@ -860,9 +860,9 @@ clamp_colors(float (*quadColor)[4]) > { > unsigned i, j; > > - for (j = 0; j < TGSI_QUAD_SIZE; j++) { > - for (i = 0; i < 4; i++) { > - quadColor[i][j] = CLAMP(quadColor[i][j], 0.0F, 1.0F); > + for (i = 0; i < 4; i++) { > + for (j = 0; j < TGSI_QUAD_SIZE; j++) { > + quadColor[i][j] = CLAMP(quadColor[i][j], 0.0F, 1.0F); >} > } > } > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/4] MSVC 2013: Preliminary support for MSVC_VERSION=12.0
On 2014-01-07 22:37, Brian Paul wrote: On 01/07/2014 01:31 PM, Thomas Sondergaard wrote: --- common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common.py b/common.py index 1d618e6..22c1725 100644 --- a/common.py +++ b/common.py @@ -100,4 +100,4 @@ def AddOptions(opts): opts.Add(BoolOption('quiet', 'DEPRECATED: profile build', 'yes')) opts.Add(BoolOption('texture_float', 'enable floating-point textures and renderbuffers', 'no')) if host_platform == 'windows': -opts.Add(EnumOption('MSVC_VERSION', 'MS Visual C++ version', None, allowed_values=('7.1', '8.0', '9.0', '10.0', '11.0'))) +opts.Add(EnumOption('MSVC_VERSION', 'MS Visual C++ version', None, allowed_values=('7.1', '8.0', '9.0', '10.0', '11.0', '12.0'))) The series looks good to me. Could probably be applied to the 10.0 branch too. Reviewed-by: Brian Paul Do you need me to push this for you? -Brian Yes please. On the 10.0 branch would be good too. Thanks, Thomas ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/4] MSVC 2013: Preliminary support for MSVC_VERSION=12.0
On 01/07/2014 01:31 PM, Thomas Sondergaard wrote: --- common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common.py b/common.py index 1d618e6..22c1725 100644 --- a/common.py +++ b/common.py @@ -100,4 +100,4 @@ def AddOptions(opts): opts.Add(BoolOption('quiet', 'DEPRECATED: profile build', 'yes')) opts.Add(BoolOption('texture_float', 'enable floating-point textures and renderbuffers', 'no')) if host_platform == 'windows': - opts.Add(EnumOption('MSVC_VERSION', 'MS Visual C++ version', None, allowed_values=('7.1', '8.0', '9.0', '10.0', '11.0'))) + opts.Add(EnumOption('MSVC_VERSION', 'MS Visual C++ version', None, allowed_values=('7.1', '8.0', '9.0', '10.0', '11.0', '12.0'))) The series looks good to me. Could probably be applied to the 10.0 branch too. Reviewed-by: Brian Paul Do you need me to push this for you? -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/4] MSVC2013: Fix compile error with MSVC 2013
This fixes the following compile error: src\glsl\ir_constant_expression.cpp(1405) : error C2666: 'copysign' : 3 overloads have similar conversions --- src/glsl/ir_constant_expression.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/glsl/ir_constant_expression.cpp b/src/glsl/ir_constant_expression.cpp index 7ca865e..f811fd1 100644 --- a/src/glsl/ir_constant_expression.cpp +++ b/src/glsl/ir_constant_expression.cpp @@ -1402,7 +1402,7 @@ ir_expression::constant_expression_value(struct hash_table *variable_context) data.f[c] = ldexp(op[0]->value.f[c], op[1]->value.i[c]); /* Flush subnormal values to zero. */ if (!isnormal(data.f[c])) -data.f[c] = copysign(0.0, op[0]->value.f[c]); +data.f[c] = copysign(0.0f, op[0]->value.f[c]); } break; -- 1.8.4.msysgit.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/4] MSVC2013: Namespace qualify fma to override ambiguity with fma from math.h
MSVC 2013 version of math.h includes an fma() function. --- src/glsl/builtin_functions.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/glsl/builtin_functions.cpp b/src/glsl/builtin_functions.cpp index 10127f3..b3e407a 100644 --- a/src/glsl/builtin_functions.cpp +++ b/src/glsl/builtin_functions.cpp @@ -3936,7 +3936,7 @@ builtin_builder::_fma(const glsl_type *type) ir_variable *c = in_var(type, "c"); MAKE_SIG(type, gpu_shader5, 3, a, b, c); - body.emit(ret(fma(a, b, c))); + body.emit(ret(ir_builder::fma(a, b, c))); return sig; } -- 1.8.4.msysgit.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/4] MSVC2013: Work around internal compiler error
This small rearrangement avoids MSVC 2013 ICE. Also, this should be a better memory access order. --- src/gallium/drivers/softpipe/sp_quad_blend.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/gallium/drivers/softpipe/sp_quad_blend.c b/src/gallium/drivers/softpipe/sp_quad_blend.c index 48d1a2e..174e60c 100644 --- a/src/gallium/drivers/softpipe/sp_quad_blend.c +++ b/src/gallium/drivers/softpipe/sp_quad_blend.c @@ -860,9 +860,9 @@ clamp_colors(float (*quadColor)[4]) { unsigned i, j; - for (j = 0; j < TGSI_QUAD_SIZE; j++) { - for (i = 0; i < 4; i++) { - quadColor[i][j] = CLAMP(quadColor[i][j], 0.0F, 1.0F); + for (i = 0; i < 4; i++) { + for (j = 0; j < TGSI_QUAD_SIZE; j++) { + quadColor[i][j] = CLAMP(quadColor[i][j], 0.0F, 1.0F); } } } -- 1.8.4.msysgit.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/4] MSVC 2013: Preliminary support for MSVC_VERSION=12.0
--- common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common.py b/common.py index 1d618e6..22c1725 100644 --- a/common.py +++ b/common.py @@ -100,4 +100,4 @@ def AddOptions(opts): opts.Add(BoolOption('quiet', 'DEPRECATED: profile build', 'yes')) opts.Add(BoolOption('texture_float', 'enable floating-point textures and renderbuffers', 'no')) if host_platform == 'windows': - opts.Add(EnumOption('MSVC_VERSION', 'MS Visual C++ version', None, allowed_values=('7.1', '8.0', '9.0', '10.0', '11.0'))) + opts.Add(EnumOption('MSVC_VERSION', 'MS Visual C++ version', None, allowed_values=('7.1', '8.0', '9.0', '10.0', '11.0', '12.0'))) -- 1.8.4.msysgit.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/4] MSVC 2013: Preliminary support for MSVC_VERSION=12.0
--- common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common.py b/common.py index 1d618e6..22c1725 100644 --- a/common.py +++ b/common.py @@ -100,4 +100,4 @@ def AddOptions(opts): opts.Add(BoolOption('quiet', 'DEPRECATED: profile build', 'yes')) opts.Add(BoolOption('texture_float', 'enable floating-point textures and renderbuffers', 'no')) if host_platform == 'windows': - opts.Add(EnumOption('MSVC_VERSION', 'MS Visual C++ version', None, allowed_values=('7.1', '8.0', '9.0', '10.0', '11.0'))) + opts.Add(EnumOption('MSVC_VERSION', 'MS Visual C++ version', None, allowed_values=('7.1', '8.0', '9.0', '10.0', '11.0', '12.0'))) -- 1.8.4.msysgit.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] st/mesa: use sRGB formats for MSAA resolving if destination is sRGB
From: Marek Olšák Copied from the i965 driver, including the big comment. Cc: 9.2 10.0 --- src/mesa/state_tracker/st_cb_blit.c | 32 1 file changed, 32 insertions(+) diff --git a/src/mesa/state_tracker/st_cb_blit.c b/src/mesa/state_tracker/st_cb_blit.c index a236a08..82c2e38 100644 --- a/src/mesa/state_tracker/st_cb_blit.c +++ b/src/mesa/state_tracker/st_cb_blit.c @@ -44,6 +44,34 @@ static void +st_adjust_blit_for_msaa_resolve(struct pipe_blit_info *blit) +{ + /* Even though we do multisample resolves at the time of the blit, OpenGL +* specification defines them as if they happen at the time of rendering, +* which means that the type of averaging we do during the resolve should +* only depend on the source format; the destination format should be +* ignored. But, specification doesn't seem to be strict about it. +* +* It has been observed that mulitisample resolves produce slightly better +* looking images when averaging is done using destination format. NVIDIA's +* proprietary OpenGL driver also follows this approach. +* +* When multisampling, if the source and destination formats are equal +* (aside from the color space), we choose to blit in sRGB space to get +* this higher quality image. +*/ + if (blit->src.resource->nr_samples > 1 && + blit->dst.resource->nr_samples <= 1) { + blit->dst.format = blit->dst.resource->format; + + if (util_format_is_srgb(blit->dst.resource->format)) + blit->src.format = util_format_srgb(blit->src.resource->format); + else + blit->src.format = util_format_linear(blit->src.resource->format); + } +} + +static void st_BlitFramebuffer(struct gl_context *ctx, GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1, GLint dstX0, GLint dstY0, GLint dstX1, GLint dstY1, @@ -192,6 +220,8 @@ st_BlitFramebuffer(struct gl_context *ctx, blit.src.box.z = srcAtt->Zoffset + srcAtt->CubeMapFace; blit.src.format = util_format_linear(srcObj->pt->format); + st_adjust_blit_for_msaa_resolve(&blit); + st->pipe->blit(st->pipe, &blit); } } @@ -227,6 +257,8 @@ st_BlitFramebuffer(struct gl_context *ctx, blit.src.box.z = srcSurf->u.tex.first_layer; blit.src.format = util_format_linear(srcSurf->format); + st_adjust_blit_for_msaa_resolve(&blit); + st->pipe->blit(st->pipe, &blit); } } -- 1.8.3.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/9] gallium-tgsi: add TGSI_OPCODE_{FMA-POPCNT-MSB-LSB} description
FYI, Evergreen has dedicated instructions for both MAD and FMA. FMA seems to be available on DX11 chips only. Marek On Tue, Jan 7, 2014 at 8:20 PM, Roland Scheidegger wrote: > Yes that is certainly related. I'm actually not entirely sure what is > allowed in glsl by default as OpenGL seems to have some lax rules > regarding precision in any case (float calculations not required but > allowed to use denorms, at least earlier versions weren't required to > support Infs neither and so on). > It is quite possible the "MAD" we were always using would have been > allowed to really do fma (at least with OpenGL), unless the "precise" > qualifier was used (which isn't supported yet?). > TGSI also isn't really watertight about such issues neither (that is if > you use it with hw such as r300 then you certainly don't expect ieee754 > rules to be followed but if you've got a d3d10-capable backend then you > are expected to follow rules specified there which are _mostly_ > ieee754-2008). > So I'm not really sure if TGSI MAD should be allowed to do either > rounding or not, but someday it should be figured out and spelled out > explicitly in docs. > > Roland > > > Am 07.01.2014 19:24, schrieb Maxence Le Doré: >> I forgot the link : >> >> https://urldefense.proofpoint.com/v1/url?u=http://www.geeks3d.com/20120106/precise-qualifier-in-glsl-and-nvidia-geforce-cards/&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=F4msKE2WxRzA%2BwN%2B25muztFm5TSPwE8HKJfWfR2NgfY%3D%0A&m=%2FzSAl55KOH0z7T5qkRj6BX164wf6QpYOnJLIzojXBQc%3D%0A&s=0ac5e0fbd69867705f0c52090c9ddf84e7832be80e724a0983c5aa2f5dde72e0 >> >> 2014/1/7 Maxence Le Doré : >>> For this reason, GLSL 4.0 introduces the 'precise' qualifier. I invite >>> you to take a look at this article. >>> >>> 2014/1/6 Roland Scheidegger : Am 05.01.2014 01:34, schrieb Maxence Le Doré: > FMA(a,b,c) keeps extra precision (usually 1 more bit of mantissa, > afaik) for the result a*b and add this to c, to finally produce a > IEEE754 32bit float result. > > MAD(a,b,c) product a IEEE754 32bit float product a*b and add it to C. > > So, fma can be slightly more accurate. An accuracy that is something > very appreciate. Actually in "newer" languages (such as opencl) "mad" is used to indicate intermediate rounding does not matter, so if your cpu can do fma but not mul+add in a single cycle it is allowed to use fma instead. FMA OTOH of course forces no intermediate rounding. Our tgsi definitions certainly initially were meaning intermediate rounding should take place, I don't know if we need to keep it that way or could repurpose that slightly (so if you require the intermediate rounding you'd just use mul+add). Roland > > > 2014/1/5 Marek Olšák : >> How is FMA different from MAD? >> >> Please document the new opcodes in src/gallium/docs/source/tgsi.rst. >> >> Marek >> >> On Sun, Jan 5, 2014 at 12:42 AM, Maxence Le Doré >> wrote: >>> From: Maxence Le Doré >>> >>> --- >>> src/gallium/auxiliary/tgsi/tgsi_info.c | 16 >>> src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h | 6 ++ >>> src/gallium/include/pipe/p_shader_tokens.h | 9 - >>> 3 files changed, 30 insertions(+), 1 deletion(-) >>> >>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_info.c >>> b/src/gallium/auxiliary/tgsi/tgsi_info.c >>> index 0beef44..ed55940 100644 >>> --- a/src/gallium/auxiliary/tgsi/tgsi_info.c >>> +++ b/src/gallium/auxiliary/tgsi/tgsi_info.c >>> @@ -221,6 +221,12 @@ static const struct tgsi_opcode_info >>> opcode_info[TGSI_OPCODE_LAST] = >>> { 1, 3, 1, 0, 0, 0, OTHR, "TXL2", TGSI_OPCODE_TXL2 }, >>> { 1, 2, 0, 0, 0, 0, COMP, "IMUL_HI", TGSI_OPCODE_IMUL_HI }, >>> { 1, 2, 0, 0, 0, 0, COMP, "UMUL_HI", TGSI_OPCODE_UMUL_HI }, >>> + { 1, 3, 0, 0, 0, 0, COMP, "FMA", TGSI_OPCODE_FMA }, >>> + { 1, 1, 0, 0, 0, 0, COMP, "POPCNT", TGSI_OPCODE_POPCNT }, >>> + { 1, 1, 0, 0, 0, 0, COMP, "IMSB", TGSI_OPCODE_IMSB }, >>> + { 1, 1, 0, 0, 0, 0, COMP, "ILSB", TGSI_OPCODE_ILSB }, >>> + { 1, 1, 0, 0, 0, 0, COMP, "UMSB", TGSI_OPCODE_UMSB }, >>> + { 1, 1, 0, 0, 0, 0, COMP, "ULSB", TGSI_OPCODE_ULSB }, >>> }; >>> >>> const struct tgsi_opcode_info * >>> @@ -321,6 +327,11 @@ tgsi_opcode_infer_type( uint opcode ) >>> case TGSI_OPCODE_IABS: >>> case TGSI_OPCODE_ISSG: >>> case TGSI_OPCODE_IMUL_HI: >>> + case TGSI_OPCODE_POPCNT: >>> + case TGSI_OPCODE_ILSB: >>> + case TGSI_OPCODE_IMSB: >>> + case TGSI_OPCODE_ULSB: >>> + case TGSI_OPCODE_UMSB: >>>return TGSI_TYPE_SIGNED; >>> default: >>>return TGSI_TYPE_FLOAT; >>> @@ -344,9 +355,14 @@ tgsi_opcode_infer_src_type( uint opcode ) >>> case TGSI_OPCODE_SAMPLE_I: >>> case TGSI_OPCODE_SAMPLE_I_MS: >>>
Re: [Mesa-dev] [PATCH] i965/fs: do SEL optimization only when src type for MOV matches
On Tue, Jan 7, 2014 at 12:25 AM, Tapani Pälli wrote: > Fixes a bug where then branch operates with ivec4 while else uses vec4. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=72379 > > Signed-off-by: Tapani Pälli > --- > src/mesa/drivers/dri/i965/brw_fs_sel_peephole.cpp | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/src/mesa/drivers/dri/i965/brw_fs_sel_peephole.cpp > b/src/mesa/drivers/dri/i965/brw_fs_sel_peephole.cpp > index d314799..a1f7e21 100644 > --- a/src/mesa/drivers/dri/i965/brw_fs_sel_peephole.cpp > +++ b/src/mesa/drivers/dri/i965/brw_fs_sel_peephole.cpp > @@ -169,6 +169,12 @@ fs_visitor::opt_peephole_sel() > break; > } > > + /* check that source types for mov operations match */ Capital C in check. Period to end the sentence. > + if(then_mov[i]->src[0].type != else_mov[i]->src[0].type) { Space between if and (. > +movs = i; > +break; > + } > + > if (!then_mov[i]->src[0].equals(else_mov[i]->src[0])) { > /* Only the last source register can be a constant, so if the MOV > * in the "then" clause uses a constant, we need to put it in a > -- > 1.8.3.1 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 5/9] tgsi: implement micro_fma() in tgsi exec machine
Very nice. I originally thought about conditionnaly defining a fmaf for MSVC < 1400 like Cygwin and opensource apple code, but I think this is pushing the problem to later, with double fma(double), that will be required with ARB_gpu_shader_fp64 : In that case, we'll have to use long doubles (80 bits), which is x86 specific -> this may not fit well this time when building mesa for ARM Android. The implementation in gnulib seems to take a different approach. I think you were talking about this one : https://github.com/gagern/gnulib/blob/master/lib/fma.c 2014/1/7 Erik Faye-Lund : > On Tue, Jan 7, 2014 at 6:46 PM, Maxence Le Doré > wrote: >> Indeed, this patch compiles on linux but may break complilation at >> least on MSVC version < 1400. >> I'm planing to install visual studio 2005 on a ms xp or seven virtual >> machine to check the behavior when compiling in microsoft windows >> environment. >> > > I can save you the trouble: I have MSVC 2008 installed, and it does > not support fmaf. There's an effort to implement it in Cygwin here: > http://www.cygwin.com/ml/libc-alpha/2010-10/msg7.html > > However, it depends on fetestexcept(), which is also not supported. > Another implementation I found was this, which claims to be exact: > http://opensource.apple.com/source/Libm/Libm-2026/Source/ARM/fmaf.c > > There's also an implementation in gnulib, but it looks quite complicated to > me. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] mesa: check bits per channel for GL_RGBA_SIGNED_COMPONENTS_EXT query
On 01/07/2014 08:35 AM, Brian Paul wrote: > If a channel has zero bits it's not signed. > --- > src/mesa/main/get.c | 28 +--- > 1 file changed, 21 insertions(+), 7 deletions(-) Series is: Reviewed-by: Kenneth Graunke ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 73371] EGL_NOK_texture_from_pixmap is advertised but not properly set.
https://bugs.freedesktop.org/show_bug.cgi?id=73371 Iván Briano changed: What|Removed |Added CC||sachi...@gmail.com --- Comment #1 from Iván Briano --- I can confirm this happens, at least when using the Intel driver. The extension returns the yinvert value from egl_config that never seems to be set. -- 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/2] glsl: Refactor is_zero/one/negative_one into an is_value() method.
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 2/2] glsl: Optimize pow(2, x) --> exp2(x).
Reviewed-by: Matt Turner ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 73371] EGL_NOK_texture_from_pixmap is advertised but not properly set.
https://bugs.freedesktop.org/show_bug.cgi?id=73371 Gustavo Sverzut Barbieri changed: What|Removed |Added URL||https://git.enlightenment.o ||rg/core/efl.git/commit/?id= ||e9783c3caf9e5f21c264277d35d ||045cfcc532fc7 -- 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 73371] EGL_NOK_texture_from_pixmap is advertised but not properly set.
https://bugs.freedesktop.org/show_bug.cgi?id=73371 Gustavo Sverzut Barbieri changed: What|Removed |Added CC||ras...@rasterman.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
[Mesa-dev] [Bug 73371] EGL_NOK_texture_from_pixmap is advertised but not properly set.
https://bugs.freedesktop.org/show_bug.cgi?id=73371 Gustavo Sverzut Barbieri changed: What|Removed |Added CC||barbi...@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
[Mesa-dev] [Bug 73371] New: EGL_NOK_texture_from_pixmap is advertised but not properly set.
https://bugs.freedesktop.org/show_bug.cgi?id=73371 Priority: medium Bug ID: 73371 Assignee: mesa-dev@lists.freedesktop.org Summary: EGL_NOK_texture_from_pixmap is advertised but not properly set. Severity: normal Classification: Unclassified OS: Linux (All) Reporter: barbi...@gmail.com Hardware: x86 (IA32) Status: NEW Version: unspecified Component: EGL Product: Mesa Intel Mesa driver advertises EGL_NOK_texture_from_pixmap however it doesn't set it correctly. It will make an upside down image for applications that take such value in account, as does Evas (EFL canvas), see the required work around: https://git.enlightenment.org/core/efl. git/commit/?id=e9783c3caf9e5f21c264277d35d045cfcc532fc7 -- 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 5/9] tgsi: implement micro_fma() in tgsi exec machine
On Tue, Jan 7, 2014 at 6:46 PM, Maxence Le Doré wrote: > Indeed, this patch compiles on linux but may break complilation at > least on MSVC version < 1400. > I'm planing to install visual studio 2005 on a ms xp or seven virtual > machine to check the behavior when compiling in microsoft windows > environment. > I can save you the trouble: I have MSVC 2008 installed, and it does not support fmaf. There's an effort to implement it in Cygwin here: http://www.cygwin.com/ml/libc-alpha/2010-10/msg7.html However, it depends on fetestexcept(), which is also not supported. Another implementation I found was this, which claims to be exact: http://opensource.apple.com/source/Libm/Libm-2026/Source/ARM/fmaf.c There's also an implementation in gnulib, but it looks quite complicated to me. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/9] gallium-tgsi: add TGSI_OPCODE_{FMA-POPCNT-MSB-LSB} description
Yes that is certainly related. I'm actually not entirely sure what is allowed in glsl by default as OpenGL seems to have some lax rules regarding precision in any case (float calculations not required but allowed to use denorms, at least earlier versions weren't required to support Infs neither and so on). It is quite possible the "MAD" we were always using would have been allowed to really do fma (at least with OpenGL), unless the "precise" qualifier was used (which isn't supported yet?). TGSI also isn't really watertight about such issues neither (that is if you use it with hw such as r300 then you certainly don't expect ieee754 rules to be followed but if you've got a d3d10-capable backend then you are expected to follow rules specified there which are _mostly_ ieee754-2008). So I'm not really sure if TGSI MAD should be allowed to do either rounding or not, but someday it should be figured out and spelled out explicitly in docs. Roland Am 07.01.2014 19:24, schrieb Maxence Le Doré: > I forgot the link : > > https://urldefense.proofpoint.com/v1/url?u=http://www.geeks3d.com/20120106/precise-qualifier-in-glsl-and-nvidia-geforce-cards/&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=F4msKE2WxRzA%2BwN%2B25muztFm5TSPwE8HKJfWfR2NgfY%3D%0A&m=%2FzSAl55KOH0z7T5qkRj6BX164wf6QpYOnJLIzojXBQc%3D%0A&s=0ac5e0fbd69867705f0c52090c9ddf84e7832be80e724a0983c5aa2f5dde72e0 > > 2014/1/7 Maxence Le Doré : >> For this reason, GLSL 4.0 introduces the 'precise' qualifier. I invite >> you to take a look at this article. >> >> 2014/1/6 Roland Scheidegger : >>> Am 05.01.2014 01:34, schrieb Maxence Le Doré: FMA(a,b,c) keeps extra precision (usually 1 more bit of mantissa, afaik) for the result a*b and add this to c, to finally produce a IEEE754 32bit float result. MAD(a,b,c) product a IEEE754 32bit float product a*b and add it to C. So, fma can be slightly more accurate. An accuracy that is something very appreciate. >>> >>> Actually in "newer" languages (such as opencl) "mad" is used to indicate >>> intermediate rounding does not matter, so if your cpu can do fma but not >>> mul+add in a single cycle it is allowed to use fma instead. >>> FMA OTOH of course forces no intermediate rounding. >>> Our tgsi definitions certainly initially were meaning intermediate >>> rounding should take place, I don't know if we need to keep it that way >>> or could repurpose that slightly (so if you require the intermediate >>> rounding you'd just use mul+add). >>> >>> Roland >>> >>> >>> 2014/1/5 Marek Olšák : > How is FMA different from MAD? > > Please document the new opcodes in src/gallium/docs/source/tgsi.rst. > > Marek > > On Sun, Jan 5, 2014 at 12:42 AM, Maxence Le Doré > wrote: >> From: Maxence Le Doré >> >> --- >> src/gallium/auxiliary/tgsi/tgsi_info.c | 16 >> src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h | 6 ++ >> src/gallium/include/pipe/p_shader_tokens.h | 9 - >> 3 files changed, 30 insertions(+), 1 deletion(-) >> >> diff --git a/src/gallium/auxiliary/tgsi/tgsi_info.c >> b/src/gallium/auxiliary/tgsi/tgsi_info.c >> index 0beef44..ed55940 100644 >> --- a/src/gallium/auxiliary/tgsi/tgsi_info.c >> +++ b/src/gallium/auxiliary/tgsi/tgsi_info.c >> @@ -221,6 +221,12 @@ static const struct tgsi_opcode_info >> opcode_info[TGSI_OPCODE_LAST] = >> { 1, 3, 1, 0, 0, 0, OTHR, "TXL2", TGSI_OPCODE_TXL2 }, >> { 1, 2, 0, 0, 0, 0, COMP, "IMUL_HI", TGSI_OPCODE_IMUL_HI }, >> { 1, 2, 0, 0, 0, 0, COMP, "UMUL_HI", TGSI_OPCODE_UMUL_HI }, >> + { 1, 3, 0, 0, 0, 0, COMP, "FMA", TGSI_OPCODE_FMA }, >> + { 1, 1, 0, 0, 0, 0, COMP, "POPCNT", TGSI_OPCODE_POPCNT }, >> + { 1, 1, 0, 0, 0, 0, COMP, "IMSB", TGSI_OPCODE_IMSB }, >> + { 1, 1, 0, 0, 0, 0, COMP, "ILSB", TGSI_OPCODE_ILSB }, >> + { 1, 1, 0, 0, 0, 0, COMP, "UMSB", TGSI_OPCODE_UMSB }, >> + { 1, 1, 0, 0, 0, 0, COMP, "ULSB", TGSI_OPCODE_ULSB }, >> }; >> >> const struct tgsi_opcode_info * >> @@ -321,6 +327,11 @@ tgsi_opcode_infer_type( uint opcode ) >> case TGSI_OPCODE_IABS: >> case TGSI_OPCODE_ISSG: >> case TGSI_OPCODE_IMUL_HI: >> + case TGSI_OPCODE_POPCNT: >> + case TGSI_OPCODE_ILSB: >> + case TGSI_OPCODE_IMSB: >> + case TGSI_OPCODE_ULSB: >> + case TGSI_OPCODE_UMSB: >>return TGSI_TYPE_SIGNED; >> default: >>return TGSI_TYPE_FLOAT; >> @@ -344,9 +355,14 @@ tgsi_opcode_infer_src_type( uint opcode ) >> case TGSI_OPCODE_SAMPLE_I: >> case TGSI_OPCODE_SAMPLE_I_MS: >> case TGSI_OPCODE_UMUL_HI: >> + case TGSI_OPCODE_POPCNT: >> + case TGSI_OPCODE_ULSB: >> + case TGSI_OPCODE_UMSB: >>return TGSI_TYPE_UNSIGNED; >> case TGSI_OPCODE_IMUL_HI: >> case TGSI_OPCODE_I2F: >> + case TGSI_OPCODE_ILSB: >> + case TGSI_OPCO
Re: [Mesa-dev] [Mesa-stable] [PATCH 0/3] i965/gen6: Emit more flushes
On Tue, Jan 07, 2014 at 05:41:56AM -0800, Paul Berry wrote: > On 6 January 2014 17:05, Chad Versace wrote: > > In solving a HiZ hang before Christmas vacation, I discovered that Mesa > wasn't > emitting sufficient workaround flushes. That bug was solved in commit > 1a92881. > > This series, prompted by Ken, adds some more carefully placed flushes to > pre-emptively fix undiscovered gpu hangs. Gpu hangs are difficult to > diagnose, > hurt users pretty hard, and are overall nasty, so it's a good idea to put > safeguards into the driver to prevent them. > > Chad Versace (3): > i965/gen6: Set need_workaround_flush after each primitive > i965/gen6/blorp: Set need_workaround_flush at top of blorp > i965/gen6/blorp: Remove too heavy HiZ workaround > > src/mesa/drivers/dri/i965/gen6_blorp.cpp | 19 +-- > 1 file changed, 5 insertions(+), 14 deletions(-) > > -- > 1.8.5.2 > > > > I suspect you meant to send this series to mesa-dev@lists.freedesktop.org? > You > actually sent it to m...@lists.freedesktop.org instead. I'll send a v2 of the series, and reply to your comments there. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] mesa: check bits per channel for GL_RGBA_SIGNED_COMPONENTS_EXT query
Am 07.01.2014 17:35, schrieb Brian Paul: > If a channel has zero bits it's not signed. > --- > src/mesa/main/get.c | 28 +--- > 1 file changed, 21 insertions(+), 7 deletions(-) > > diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c > index c45a8d1..bb138c8 100644 > --- a/src/mesa/main/get.c > +++ b/src/mesa/main/get.c > @@ -769,13 +769,27 @@ find_custom_value(struct gl_context *ctx, const struct > value_desc *d, union valu > /* Note: we only check the 0th color attachment. */ > const struct gl_renderbuffer *rb = > ctx->DrawBuffer->_ColorDrawBuffers[0]; > - const GLboolean is_signed = > -rb ? _mesa_is_format_signed(rb->Format) : GL_FALSE; > - /* At this time, all color channels have same signedness */ > - v->value_int_4[0] = > - v->value_int_4[1] = > - v->value_int_4[2] = > - v->value_int_4[3] = is_signed; > + if (rb && _mesa_is_format_signed(rb->Format)) { > +/* Issue 17 of GL_EXT_packed_float: If a component (such as > + * alpha) has zero bits, the component should not be considered > + * signed and so the bit for the respective component should be > + * zeroed. > + */ > +v->value_int_4[0] = > + _mesa_get_format_bits(rb->Format, GL_RED_BITS) > 0; > +v->value_int_4[1] = > + _mesa_get_format_bits(rb->Format, GL_GREEN_BITS) > 0; > +v->value_int_4[2] = > + _mesa_get_format_bits(rb->Format, GL_BLUE_BITS) > 0; > +v->value_int_4[3] = > + _mesa_get_format_bits(rb->Format, GL_ALPHA_BITS) > 0; > + } > + else { > +v->value_int_4[0] = > +v->value_int_4[1] = > +v->value_int_4[2] = > +v->value_int_4[3] = 0; > + } >} >break; > > Series looks good to me. Though I really wonder who would care if a zero-bit component is considered to be signed or not :-). Roland ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] llvmpipe: fix primitive input to geom shaders
Well we were using a system value for prim id in gs, hence this was not necessary. I'm always confused though about system value / normal semantic usage though, Zack might know better. Roland Am 07.01.2014 09:55, schrieb Dave Airlie: > Not sure this is 100% the correct way to do this, since it may be a change > at the glsl->tgsi level that is required, either way open discussions! > > fixes piglit > tests/spec/glsl-1.50/execution/geometry/primitive-id-in.shader_test > with llvmpipe with fake MSAA > > Signed-off-by: Dave Airlie > --- > src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c | 5 + > src/gallium/auxiliary/tgsi/tgsi_scan.c | 3 +++ > 2 files changed, 8 insertions(+) > > diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c > b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c > index 6d8dc8c..de2c64f 100644 > --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c > +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c > @@ -1173,6 +1173,10 @@ emit_fetch_gs_input( > LLVMValueRef swizzle_index = lp_build_const_int32(gallivm, swizzle); > LLVMValueRef res; > > + if (bld_base->info->input_semantic_name[reg->Register.Index] == > TGSI_SEMANTIC_PRIMID) { > + res = bld->system_values.prim_id; > + goto out; > + } > if (reg->Register.Indirect) { >attrib_index = get_indirect_index(bld, > reg->Register.File, > @@ -1200,6 +1204,7 @@ emit_fetch_gs_input( > > assert(res); > > + out: > if (stype == TGSI_TYPE_UNSIGNED) { >res = LLVMBuildBitCast(builder, res, bld_base->uint_bld.vec_type, ""); > } else if (stype == TGSI_TYPE_SIGNED) { > diff --git a/src/gallium/auxiliary/tgsi/tgsi_scan.c > b/src/gallium/auxiliary/tgsi/tgsi_scan.c > index 0f10556..ce1f7b6 100644 > --- a/src/gallium/auxiliary/tgsi/tgsi_scan.c > +++ b/src/gallium/auxiliary/tgsi/tgsi_scan.c > @@ -198,6 +198,9 @@ tgsi_scan_shader(const struct tgsi_token *tokens, > info->uses_primid = TRUE; > else if (semName == TGSI_SEMANTIC_FACE) > info->uses_frontface = TRUE; > + } else if (procType == TGSI_PROCESSOR_GEOMETRY) { > + if (semName == TGSI_SEMANTIC_PRIMID) > +info->uses_primid = TRUE; >} > } > else if (file == TGSI_FILE_SYSTEM_VALUE) { > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/9] gallium-tgsi: add TGSI_OPCODE_{FMA-POPCNT-MSB-LSB} description
For this reason, GLSL 4.0 introduces the 'precise' qualifier. I invite you to take a look at this article. 2014/1/6 Roland Scheidegger : > Am 05.01.2014 01:34, schrieb Maxence Le Doré: >> FMA(a,b,c) keeps extra precision (usually 1 more bit of mantissa, >> afaik) for the result a*b and add this to c, to finally produce a >> IEEE754 32bit float result. >> >> MAD(a,b,c) product a IEEE754 32bit float product a*b and add it to C. >> >> So, fma can be slightly more accurate. An accuracy that is something >> very appreciate. > > Actually in "newer" languages (such as opencl) "mad" is used to indicate > intermediate rounding does not matter, so if your cpu can do fma but not > mul+add in a single cycle it is allowed to use fma instead. > FMA OTOH of course forces no intermediate rounding. > Our tgsi definitions certainly initially were meaning intermediate > rounding should take place, I don't know if we need to keep it that way > or could repurpose that slightly (so if you require the intermediate > rounding you'd just use mul+add). > > Roland > > > >> >> >> 2014/1/5 Marek Olšák : >>> How is FMA different from MAD? >>> >>> Please document the new opcodes in src/gallium/docs/source/tgsi.rst. >>> >>> Marek >>> >>> On Sun, Jan 5, 2014 at 12:42 AM, Maxence Le Doré >>> wrote: From: Maxence Le Doré --- src/gallium/auxiliary/tgsi/tgsi_info.c | 16 src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h | 6 ++ src/gallium/include/pipe/p_shader_tokens.h | 9 - 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/gallium/auxiliary/tgsi/tgsi_info.c b/src/gallium/auxiliary/tgsi/tgsi_info.c index 0beef44..ed55940 100644 --- a/src/gallium/auxiliary/tgsi/tgsi_info.c +++ b/src/gallium/auxiliary/tgsi/tgsi_info.c @@ -221,6 +221,12 @@ static const struct tgsi_opcode_info opcode_info[TGSI_OPCODE_LAST] = { 1, 3, 1, 0, 0, 0, OTHR, "TXL2", TGSI_OPCODE_TXL2 }, { 1, 2, 0, 0, 0, 0, COMP, "IMUL_HI", TGSI_OPCODE_IMUL_HI }, { 1, 2, 0, 0, 0, 0, COMP, "UMUL_HI", TGSI_OPCODE_UMUL_HI }, + { 1, 3, 0, 0, 0, 0, COMP, "FMA", TGSI_OPCODE_FMA }, + { 1, 1, 0, 0, 0, 0, COMP, "POPCNT", TGSI_OPCODE_POPCNT }, + { 1, 1, 0, 0, 0, 0, COMP, "IMSB", TGSI_OPCODE_IMSB }, + { 1, 1, 0, 0, 0, 0, COMP, "ILSB", TGSI_OPCODE_ILSB }, + { 1, 1, 0, 0, 0, 0, COMP, "UMSB", TGSI_OPCODE_UMSB }, + { 1, 1, 0, 0, 0, 0, COMP, "ULSB", TGSI_OPCODE_ULSB }, }; const struct tgsi_opcode_info * @@ -321,6 +327,11 @@ tgsi_opcode_infer_type( uint opcode ) case TGSI_OPCODE_IABS: case TGSI_OPCODE_ISSG: case TGSI_OPCODE_IMUL_HI: + case TGSI_OPCODE_POPCNT: + case TGSI_OPCODE_ILSB: + case TGSI_OPCODE_IMSB: + case TGSI_OPCODE_ULSB: + case TGSI_OPCODE_UMSB: return TGSI_TYPE_SIGNED; default: return TGSI_TYPE_FLOAT; @@ -344,9 +355,14 @@ tgsi_opcode_infer_src_type( uint opcode ) case TGSI_OPCODE_SAMPLE_I: case TGSI_OPCODE_SAMPLE_I_MS: case TGSI_OPCODE_UMUL_HI: + case TGSI_OPCODE_POPCNT: + case TGSI_OPCODE_ULSB: + case TGSI_OPCODE_UMSB: return TGSI_TYPE_UNSIGNED; case TGSI_OPCODE_IMUL_HI: case TGSI_OPCODE_I2F: + case TGSI_OPCODE_ILSB: + case TGSI_OPCODE_IMSB: return TGSI_TYPE_SIGNED; case TGSI_OPCODE_ARL: case TGSI_OPCODE_ARR: diff --git a/src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h b/src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h index 1ef78dd..cba0975 100644 --- a/src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h +++ b/src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h @@ -206,6 +206,12 @@ OP13(UCMP) OP12(IMUL_HI) OP12(UMUL_HI) +OP13(FMA) +OP11(POPCNT) +OP11(IMSB) +OP11(ILSB) +OP11(UMSB) +OP11(ULSB) #undef OP00 #undef OP01 diff --git a/src/gallium/include/pipe/p_shader_tokens.h b/src/gallium/include/pipe/p_shader_tokens.h index 8010902..5ed0c34 100644 --- a/src/gallium/include/pipe/p_shader_tokens.h +++ b/src/gallium/include/pipe/p_shader_tokens.h @@ -453,7 +453,14 @@ struct tgsi_property_data { #define TGSI_OPCODE_IMUL_HI 180 #define TGSI_OPCODE_UMUL_HI 181 -#define TGSI_OPCODE_LAST182 +#define TGSI_OPCODE_FMA 182 +#define TGSI_OPCODE_POPCNT 183 +#define TGSI_OPCODE_IMSB184 +#define TGSI_OPCODE_ILSB185 +#define TGSI_OPCODE_UMSB186 +#define TGSI_OPCODE_ULSB187 + +#define TGSI_OPCODE_LAST188 #define TGSI_SAT_NONE0 /* do not saturate */ #define TGSI_SAT_ZERO_ONE1 /* clamp to [0,1] */ -- 1.8.5.2 _
Re: [Mesa-dev] [PATCH 4/9] gallium-tgsi: add TGSI_OPCODE_{FMA-POPCNT-MSB-LSB} description
I forgot the link : http://www.geeks3d.com/20120106/precise-qualifier-in-glsl-and-nvidia-geforce-cards/ 2014/1/7 Maxence Le Doré : > For this reason, GLSL 4.0 introduces the 'precise' qualifier. I invite > you to take a look at this article. > > 2014/1/6 Roland Scheidegger : >> Am 05.01.2014 01:34, schrieb Maxence Le Doré: >>> FMA(a,b,c) keeps extra precision (usually 1 more bit of mantissa, >>> afaik) for the result a*b and add this to c, to finally produce a >>> IEEE754 32bit float result. >>> >>> MAD(a,b,c) product a IEEE754 32bit float product a*b and add it to C. >>> >>> So, fma can be slightly more accurate. An accuracy that is something >>> very appreciate. >> >> Actually in "newer" languages (such as opencl) "mad" is used to indicate >> intermediate rounding does not matter, so if your cpu can do fma but not >> mul+add in a single cycle it is allowed to use fma instead. >> FMA OTOH of course forces no intermediate rounding. >> Our tgsi definitions certainly initially were meaning intermediate >> rounding should take place, I don't know if we need to keep it that way >> or could repurpose that slightly (so if you require the intermediate >> rounding you'd just use mul+add). >> >> Roland >> >> >> >>> >>> >>> 2014/1/5 Marek Olšák : How is FMA different from MAD? Please document the new opcodes in src/gallium/docs/source/tgsi.rst. Marek On Sun, Jan 5, 2014 at 12:42 AM, Maxence Le Doré wrote: > From: Maxence Le Doré > > --- > src/gallium/auxiliary/tgsi/tgsi_info.c | 16 > src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h | 6 ++ > src/gallium/include/pipe/p_shader_tokens.h | 9 - > 3 files changed, 30 insertions(+), 1 deletion(-) > > diff --git a/src/gallium/auxiliary/tgsi/tgsi_info.c > b/src/gallium/auxiliary/tgsi/tgsi_info.c > index 0beef44..ed55940 100644 > --- a/src/gallium/auxiliary/tgsi/tgsi_info.c > +++ b/src/gallium/auxiliary/tgsi/tgsi_info.c > @@ -221,6 +221,12 @@ static const struct tgsi_opcode_info > opcode_info[TGSI_OPCODE_LAST] = > { 1, 3, 1, 0, 0, 0, OTHR, "TXL2", TGSI_OPCODE_TXL2 }, > { 1, 2, 0, 0, 0, 0, COMP, "IMUL_HI", TGSI_OPCODE_IMUL_HI }, > { 1, 2, 0, 0, 0, 0, COMP, "UMUL_HI", TGSI_OPCODE_UMUL_HI }, > + { 1, 3, 0, 0, 0, 0, COMP, "FMA", TGSI_OPCODE_FMA }, > + { 1, 1, 0, 0, 0, 0, COMP, "POPCNT", TGSI_OPCODE_POPCNT }, > + { 1, 1, 0, 0, 0, 0, COMP, "IMSB", TGSI_OPCODE_IMSB }, > + { 1, 1, 0, 0, 0, 0, COMP, "ILSB", TGSI_OPCODE_ILSB }, > + { 1, 1, 0, 0, 0, 0, COMP, "UMSB", TGSI_OPCODE_UMSB }, > + { 1, 1, 0, 0, 0, 0, COMP, "ULSB", TGSI_OPCODE_ULSB }, > }; > > const struct tgsi_opcode_info * > @@ -321,6 +327,11 @@ tgsi_opcode_infer_type( uint opcode ) > case TGSI_OPCODE_IABS: > case TGSI_OPCODE_ISSG: > case TGSI_OPCODE_IMUL_HI: > + case TGSI_OPCODE_POPCNT: > + case TGSI_OPCODE_ILSB: > + case TGSI_OPCODE_IMSB: > + case TGSI_OPCODE_ULSB: > + case TGSI_OPCODE_UMSB: >return TGSI_TYPE_SIGNED; > default: >return TGSI_TYPE_FLOAT; > @@ -344,9 +355,14 @@ tgsi_opcode_infer_src_type( uint opcode ) > case TGSI_OPCODE_SAMPLE_I: > case TGSI_OPCODE_SAMPLE_I_MS: > case TGSI_OPCODE_UMUL_HI: > + case TGSI_OPCODE_POPCNT: > + case TGSI_OPCODE_ULSB: > + case TGSI_OPCODE_UMSB: >return TGSI_TYPE_UNSIGNED; > case TGSI_OPCODE_IMUL_HI: > case TGSI_OPCODE_I2F: > + case TGSI_OPCODE_ILSB: > + case TGSI_OPCODE_IMSB: >return TGSI_TYPE_SIGNED; > case TGSI_OPCODE_ARL: > case TGSI_OPCODE_ARR: > diff --git a/src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h > b/src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h > index 1ef78dd..cba0975 100644 > --- a/src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h > +++ b/src/gallium/auxiliary/tgsi/tgsi_opcode_tmp.h > @@ -206,6 +206,12 @@ OP13(UCMP) > > OP12(IMUL_HI) > OP12(UMUL_HI) > +OP13(FMA) > +OP11(POPCNT) > +OP11(IMSB) > +OP11(ILSB) > +OP11(UMSB) > +OP11(ULSB) > > #undef OP00 > #undef OP01 > diff --git a/src/gallium/include/pipe/p_shader_tokens.h > b/src/gallium/include/pipe/p_shader_tokens.h > index 8010902..5ed0c34 100644 > --- a/src/gallium/include/pipe/p_shader_tokens.h > +++ b/src/gallium/include/pipe/p_shader_tokens.h > @@ -453,7 +453,14 @@ struct tgsi_property_data { > #define TGSI_OPCODE_IMUL_HI 180 > #define TGSI_OPCODE_UMUL_HI 181 > > -#define TGSI_OPCODE_LAST182 > +#define TGSI_OPCODE_FMA 182 > +#define TGSI_OPCODE_POPCNT 183 > +#define TGSI_OPCODE_IMSB184 > +#define TGSI_OPCODE_ILSB185 > +#define TGSI_OPCODE_UMSB186 > +#de
[Mesa-dev] [PATCH 2/4] radeon: Add bo statistics dumping support
No measurable overhead when off (glxgears within 0.5%). v2: Cosmetic changes. v3: Moved file handling into winsys Signed-off-by: Lauri Kasanen --- src/gallium/drivers/radeon/r600_pipe_common.c | 5 src/gallium/drivers/radeon/r600_pipe_common.h | 1 + src/gallium/winsys/radeon/drm/radeon_drm_bo.c | 16 +++ src/gallium/winsys/radeon/drm/radeon_drm_cs.c | 9 ++ src/gallium/winsys/radeon/drm/radeon_drm_winsys.c | 35 +++ src/gallium/winsys/radeon/drm/radeon_drm_winsys.h | 2 ++ src/gallium/winsys/radeon/drm/radeon_winsys.h | 5 7 files changed, 73 insertions(+) diff --git a/src/gallium/drivers/radeon/r600_pipe_common.c b/src/gallium/drivers/radeon/r600_pipe_common.c index 28921be..f2a5794 100644 --- a/src/gallium/drivers/radeon/r600_pipe_common.c +++ b/src/gallium/drivers/radeon/r600_pipe_common.c @@ -38,6 +38,7 @@ static const struct debug_named_value common_debug_options[] = { { "compute", DBG_COMPUTE, "Print compute info" }, { "vm", DBG_VM, "Print virtual addresses when creating resources" }, { "trace_cs", DBG_TRACE_CS, "Trace cs and write rlockup_.c file with faulty cs" }, + { "bostats", DBG_BO_STATS, "Write bo statistics to /tmp/bostats.[.name]" }, /* shaders */ { "fs", DBG_FS, "Print fetch shaders" }, @@ -209,6 +210,10 @@ bool r600_common_screen_init(struct r600_common_screen *rscreen, return false; } + if (rscreen->debug_flags & DBG_BO_STATS) { + ws->enable_bo_stats(ws); + } + util_format_s3tc_init(); pipe_mutex_init(rscreen->aux_context_lock); diff --git a/src/gallium/drivers/radeon/r600_pipe_common.h b/src/gallium/drivers/radeon/r600_pipe_common.h index bf0b968..4c35e66 100644 --- a/src/gallium/drivers/radeon/r600_pipe_common.h +++ b/src/gallium/drivers/radeon/r600_pipe_common.h @@ -67,6 +67,7 @@ #define DBG_COMPUTE(1 << 2) #define DBG_VM (1 << 3) #define DBG_TRACE_CS (1 << 4) +#define DBG_BO_STATS (1 << 5) /* shaders */ #define DBG_FS (1 << 8) #define DBG_VS (1 << 9) diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c index ca569a1..ea99351 100644 --- a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c +++ b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c @@ -370,6 +370,7 @@ static void radeon_bo_destroy(struct pb_buffer *_buf) { struct radeon_bo *bo = radeon_bo(_buf); struct radeon_bomgr *mgr = bo->mgr; +struct radeon_drm_winsys *ws = mgr->rws; struct drm_gem_close args; memset(&args, 0, sizeof(args)); @@ -399,6 +400,11 @@ static void radeon_bo_destroy(struct pb_buffer *_buf) bo->rws->allocated_vram -= align(bo->base.size, 4096); else if (bo->initial_domain & RADEON_DOMAIN_GTT) bo->rws->allocated_gtt -= align(bo->base.size, 4096); + +if (ws->bo_stats_file) { +fprintf(ws->bo_stats_file, "%p destroyed @%llu\n", bo, stats_time_get(ws)); +} + FREE(bo); } @@ -450,6 +456,7 @@ static void *radeon_bo_map(struct radeon_winsys_cs_handle *buf, { struct radeon_bo *bo = (struct radeon_bo*)buf; struct radeon_drm_cs *cs = (struct radeon_drm_cs*)rcs; +struct radeon_drm_winsys *ws = bo->mgr->rws; /* If it's not unsynchronized bo_map, flush CS if needed and then wait. */ if (!(usage & PIPE_TRANSFER_UNSYNCHRONIZED)) { @@ -518,6 +525,10 @@ static void *radeon_bo_map(struct radeon_winsys_cs_handle *buf, } } +if (ws->bo_stats_file) { +fprintf(ws->bo_stats_file, "%p cpu mapped @%llu\n", bo, stats_time_get(ws)); +} + return radeon_bo_do_map(bo); } @@ -636,6 +647,11 @@ static struct pb_buffer *radeon_bomgr_create_bo(struct pb_manager *_mgr, else if (rdesc->initial_domains & RADEON_DOMAIN_GTT) rws->allocated_gtt += align(size, 4096); +if (rws->bo_stats_file) { +fprintf(rws->bo_stats_file, "%p created, size %u, prio %u, @%llu\n", bo, size, + bo->stats.high_prio, stats_time_get(rws)); +} + return &bo->base; } diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_cs.c b/src/gallium/winsys/radeon/drm/radeon_drm_cs.c index d8ad297..845603d 100644 --- a/src/gallium/winsys/radeon/drm/radeon_drm_cs.c +++ b/src/gallium/winsys/radeon/drm/radeon_drm_cs.c @@ -344,6 +344,7 @@ static unsigned radeon_drm_cs_add_reloc(struct radeon_winsys_cs *rcs, { struct radeon_drm_cs *cs = radeon_drm_cs(rcs); struct radeon_bo *bo = (struct radeon_bo*)buf; +struct radeon_drm_winsys *ws = cs->ws; enum radeon_bo_domain added_domains; unsigned index = radeon_add_reloc(cs, bo, usage, domains, &added_domains); @@ -352,6 +353,14 @@ static unsigned radeon_drm_cs_add_reloc(struct radeon_winsys_cs *rcs, if (added_domains & RADEON_DOMAIN_VRAM) cs->csc->used_vram += bo->base.size;
[Mesa-dev] [PATCH 3/4] winsys/radeon: Keep bo statistics
These will be used later on for optimizing the VRAM placement. No measurable overhead (glxgears, torcs). v2: Get accurate stats by taking dirty_masks into account Marek: I'm not sure I was testing torcs right; I started a race on Forza, drove around, and looked at the fps. Doesn't seem to do timedemos. Signed-off-by: Lauri Kasanen --- src/gallium/drivers/r600/r600_state_common.c | 23 + src/gallium/winsys/radeon/drm/radeon_drm_bo.c | 3 +++ src/gallium/winsys/radeon/drm/radeon_drm_bo.h | 16 +++ src/gallium/winsys/radeon/drm/radeon_drm_cs.c | 8 +--- src/gallium/winsys/radeon/drm/radeon_drm_winsys.c | 25 +++ src/gallium/winsys/radeon/drm/radeon_winsys.h | 11 ++ 6 files changed, 79 insertions(+), 7 deletions(-) diff --git a/src/gallium/drivers/r600/r600_state_common.c b/src/gallium/drivers/r600/r600_state_common.c index 3dc7991..ba37af8 100644 --- a/src/gallium/drivers/r600/r600_state_common.c +++ b/src/gallium/drivers/r600/r600_state_common.c @@ -518,6 +518,7 @@ static void r600_set_vertex_buffers(struct pipe_context *ctx, struct pipe_vertex_buffer *vb = state->vb + start_slot; unsigned i; uint32_t disable_mask = 0; + uint32_t skipped_mask = 0; /* These are the new buffers set by this function. */ uint32_t new_buffer_mask = 0; @@ -553,6 +554,17 @@ static void r600_set_vertex_buffers(struct pipe_context *ctx, rctx->vertex_buffer_state.dirty_mask |= new_buffer_mask; r600_vertex_buffers_dirty(rctx); + + /* Update statistics for the skipped vertex buffers */ + skipped_mask = rctx->vertex_buffer_state.enabled_mask & + ~rctx->vertex_buffer_state.dirty_mask; + while (skipped_mask) { + struct r600_resource *rbuffer; + i = u_bit_scan(&skipped_mask); + + rbuffer = (struct r600_resource*)vb[i].buffer; + rctx->b.ws->update_bo_stats(rctx->b.ws, rbuffer->cs_buf, RADEON_USAGE_READ); + } } void r600_sampler_views_dirty(struct r600_context *rctx, @@ -575,6 +587,7 @@ static void r600_set_sampler_views(struct pipe_context *pipe, unsigned shader, struct r600_pipe_sampler_view **rviews = (struct r600_pipe_sampler_view **)views; uint32_t dirty_sampler_states_mask = 0; unsigned i; + uint32_t skipped_mask = 0; /* This sets 1-bit for textures with index >= count. */ uint32_t disable_mask = ~((1ull << count) - 1); /* These are the new textures set by this function. */ @@ -654,6 +667,16 @@ static void r600_set_sampler_views(struct pipe_context *pipe, unsigned shader, dst->states.dirty_mask |= dirty_sampler_states_mask; r600_sampler_states_dirty(rctx, &dst->states); } + + /* Update statistics for the skipped samplers */ + skipped_mask = dst->views.enabled_mask & ~dst->views.dirty_mask; + while (skipped_mask) { + i = u_bit_scan(&skipped_mask); + + rctx->b.ws->update_bo_stats(rctx->b.ws, + rviews[i]->tex_resource->cs_buf, + RADEON_USAGE_READ); + } } static void r600_set_viewport_states(struct pipe_context *ctx, diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c index ea99351..606b65a 100644 --- a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c +++ b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c @@ -529,6 +529,9 @@ static void *radeon_bo_map(struct radeon_winsys_cs_handle *buf, fprintf(ws->bo_stats_file, "%p cpu mapped @%llu\n", bo, stats_time_get(ws)); } +bo->stats.num_cpu_ops++; +bo->stats.last_cpu_time = stats_time_get(ws); + return radeon_bo_do_map(bo); } diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.h b/src/gallium/winsys/radeon/drm/radeon_drm_bo.h index 5536bc1..651694b 100644 --- a/src/gallium/winsys/radeon/drm/radeon_drm_bo.h +++ b/src/gallium/winsys/radeon/drm/radeon_drm_bo.h @@ -44,6 +44,20 @@ struct radeon_bo_desc { unsigned initial_domains; }; +struct radeon_bo_stats { +uint64_t num_reads; +uint64_t num_writes; +uint64_t num_cpu_ops; + +/* Milliseconds in a monotonic clock */ +uint64_t last_read_time; +uint64_t last_write_time; +uint64_t last_cpu_time; + +/* Depth, MSAA, etc. */ +bool high_prio; +}; + struct radeon_bo { struct pb_buffer base; @@ -67,6 +81,8 @@ struct radeon_bo { boolean flinked; uint32_t flink; + +struct radeon_bo_stats stats; }; struct pb_manager *radeon_bomgr_create(struct radeon_drm_winsys *rws); diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_cs.c b/src/gallium/winsys/radeon/drm/radeon_drm_cs.c index 845603d..5c1b756 100644 --- a/src/gallium/winsys/radeon/drm/radeon_drm_cs.c +++ b/src/gallium/winsys/radeon/drm/r
[Mesa-dev] [PATCH 1/4] winsys/radeon: Add a millisecond time function
v2: Move to a timing thread to minimize overhead. Signed-off-by: Lauri Kasanen --- src/gallium/winsys/radeon/drm/radeon_drm_winsys.c | 25 +++ src/gallium/winsys/radeon/drm/radeon_drm_winsys.h | 10 + 2 files changed, 35 insertions(+) diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c index dc9d183..7bedf92 100644 --- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c +++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c @@ -442,6 +442,9 @@ static void radeon_winsys_destroy(struct radeon_winsys *rws) } pipe_semaphore_destroy(&ws->cs_queued); +ws->kill_timing_thread = 1; +pipe_thread_wait(ws->timing_thread); + pipe_mutex_destroy(ws->hyperz_owner_mutex); pipe_mutex_destroy(ws->cmask_owner_mutex); pipe_mutex_destroy(ws->cs_stack_lock); @@ -596,6 +599,22 @@ static PIPE_THREAD_ROUTINE(radeon_drm_cs_emit_ioctl, param) return NULL; } +static PIPE_THREAD_ROUTINE(radeon_drm_timing_thread, param) +{ +struct radeon_drm_winsys *ws = (struct radeon_drm_winsys *) param; +uint64_t * const time = &ws->time; + +while (!ws->kill_timing_thread) { +uint64_t tmp = os_time_get_nano() / 100; +p_atomic_set(time, tmp); + +/* We want ms accuracy, so sleep for the Nyquist freq - 0.5ms */ +usleep(500); +} + +return NULL; +} + DEBUG_GET_ONCE_BOOL_OPTION(thread, "RADEON_THREAD", TRUE) static PIPE_THREAD_ROUTINE(radeon_drm_cs_emit_ioctl, param); @@ -660,6 +679,12 @@ PUBLIC struct radeon_winsys *radeon_drm_winsys_create(int fd) if (ws->num_cpus > 1 && debug_get_option_thread()) ws->thread = pipe_thread_create(radeon_drm_cs_emit_ioctl, ws); +/* Set the time once to avoid races - stats_time_get may be called + * just after this function returns. + */ +ws->time = os_time_get_nano() / 100; +ws->timing_thread = pipe_thread_create(radeon_drm_timing_thread, ws); + return &ws->base; fail: diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h index ed90194..01c2d9c 100644 --- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h +++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.h @@ -32,6 +32,7 @@ #include "radeon_winsys.h" #include "os/os_thread.h" +#include "os/os_time.h" struct radeon_drm_cs; @@ -71,6 +72,11 @@ struct radeon_drm_winsys { int kill_thread; int ncs; struct radeon_drm_cs *cs_stack[RING_LAST]; + +/* Timing thread for the stats. */ +pipe_thread timing_thread; +int kill_timing_thread; +uint64_t time; }; static INLINE struct radeon_drm_winsys * @@ -79,6 +85,10 @@ radeon_drm_winsys(struct radeon_winsys *base) return (struct radeon_drm_winsys*)base; } +static INLINE unsigned long long stats_time_get(struct radeon_drm_winsys * const dws) { +return p_atomic_read(&dws->time); +} + void radeon_drm_ws_queue_cs(struct radeon_drm_winsys *ws, struct radeon_drm_cs *cs); #endif -- 1.8.3.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/4] radeon: Determine the bo priority (MSAA, depth, UVD are high)
v2: Moved the high priority check to r600_texture_create_object Signed-off-by: Lauri Kasanen --- src/gallium/drivers/r600/r600_state_common.c| 2 +- src/gallium/drivers/radeon/r600_buffer_common.c | 6 -- src/gallium/drivers/radeon/r600_pipe_common.h | 3 ++- src/gallium/drivers/radeon/r600_texture.c | 9 - src/gallium/drivers/radeon/radeon_uvd.c | 4 ++-- src/gallium/winsys/radeon/drm/radeon_drm_bo.c | 4 src/gallium/winsys/radeon/drm/radeon_drm_bo.h | 1 + src/gallium/winsys/radeon/drm/radeon_drm_cs.c | 2 +- src/gallium/winsys/radeon/drm/radeon_winsys.h | 2 ++ 9 files changed, 25 insertions(+), 8 deletions(-) diff --git a/src/gallium/drivers/r600/r600_state_common.c b/src/gallium/drivers/r600/r600_state_common.c index ba37af8..2f833ba 100644 --- a/src/gallium/drivers/r600/r600_state_common.c +++ b/src/gallium/drivers/r600/r600_state_common.c @@ -2106,7 +2106,7 @@ static void r600_invalidate_buffer(struct pipe_context *ctx, struct pipe_resourc /* Create a new one in the same pipe_resource. */ r600_init_resource(&rctx->screen->b, rbuffer, rbuffer->b.b.width0, alignment, - TRUE, rbuffer->b.b.usage); + TRUE, rbuffer->b.b.usage, FALSE); /* We changed the buffer, now we need to bind it where the old one was bound. */ /* Vertex buffers. */ diff --git a/src/gallium/drivers/radeon/r600_buffer_common.c b/src/gallium/drivers/radeon/r600_buffer_common.c index ac5fbcc..6083253 100644 --- a/src/gallium/drivers/radeon/r600_buffer_common.c +++ b/src/gallium/drivers/radeon/r600_buffer_common.c @@ -98,7 +98,8 @@ void *r600_buffer_map_sync_with_rings(struct r600_common_context *ctx, bool r600_init_resource(struct r600_common_screen *rscreen, struct r600_resource *res, unsigned size, unsigned alignment, - bool use_reusable_pool, unsigned usage) + bool use_reusable_pool, unsigned usage, + bool high_prio) { uint32_t initial_domain, domains; @@ -131,6 +132,7 @@ bool r600_init_resource(struct r600_common_screen *rscreen, res->buf = rscreen->ws->buffer_create(rscreen->ws, size, alignment, use_reusable_pool, + high_prio, initial_domain); if (!res->buf) { return false; @@ -314,7 +316,7 @@ struct pipe_resource *r600_buffer_create(struct pipe_screen *screen, rbuffer->b.vtbl = &r600_buffer_vtbl; util_range_init(&rbuffer->valid_buffer_range); - if (!r600_init_resource(rscreen, rbuffer, templ->width0, alignment, TRUE, templ->usage)) { + if (!r600_init_resource(rscreen, rbuffer, templ->width0, alignment, TRUE, templ->usage, FALSE)) { FREE(rbuffer); return NULL; } diff --git a/src/gallium/drivers/radeon/r600_pipe_common.h b/src/gallium/drivers/radeon/r600_pipe_common.h index 4c35e66..75c78e3 100644 --- a/src/gallium/drivers/radeon/r600_pipe_common.h +++ b/src/gallium/drivers/radeon/r600_pipe_common.h @@ -278,7 +278,8 @@ void *r600_buffer_map_sync_with_rings(struct r600_common_context *ctx, bool r600_init_resource(struct r600_common_screen *rscreen, struct r600_resource *res, unsigned size, unsigned alignment, - bool use_reusable_pool, unsigned usage); + bool use_reusable_pool, unsigned usage, + bool high_prio); struct pipe_resource *r600_buffer_create(struct pipe_screen *screen, const struct pipe_resource *templ, unsigned alignment); diff --git a/src/gallium/drivers/radeon/r600_texture.c b/src/gallium/drivers/radeon/r600_texture.c index caf3743..26c27c9 100644 --- a/src/gallium/drivers/radeon/r600_texture.c +++ b/src/gallium/drivers/radeon/r600_texture.c @@ -620,7 +620,14 @@ r600_texture_create_object(struct pipe_screen *screen, unsigned usage = rtex->surface.level[0].mode >= RADEON_SURF_MODE_1D ? PIPE_USAGE_STATIC : base->usage; - if (!r600_init_resource(rscreen, resource, rtex->size, base_align, FALSE, usage)) { + bool high_prio = false; + + /* If it's depth or MSAA, consider it high priority */ + if (rtex->is_depth || base->nr_samples > 1) + high_prio = true; + + if (!r600_init_resource(rscreen, resource, rtex->size, + base_align, FALSE, usage, high_prio)) { FREE(rtex); return NULL; } diff --git a/src/gallium/drivers/radeon/radeon_uvd.c b/src/gallium/drivers/radeon/radeon_uvd.c ind
[Mesa-dev] [PATCH] glsl: rename min(), max() functions to fix MSVC build
Thanks Brian. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Naming everything in src/gallium/drivers/radeonsi si_*
Do you think it's too much change in general or that the patches are too large? They were honestly simple find / sed jobs, so I could fairly easily redo them file by file. I'm not sure how to even argue about that, but I think suitable names are very important. rctx goes as "mysterious name" / "historical reasons" at best in SI code, and r600, well, that's just wrong. I can live with leaving them as is, but I'd rather not. On Tuesday 07 January 2014 18:02:47 Marek Olšák wrote: > I agree with everything except these two: > > "Rename the commonly occurring rctx/r600 to now more suitable sctx." > "Rename the commonly occurring rscreen to now more suitable sscreen." > > It's too much for my eye to handle. > > Marek > > On Tue, Jan 7, 2014 at 5:27 PM, Andreas Hartmetz wrote: > > We have talked on IRC meanwhile: > > "Everywhere" was supposed to mean file names and data structures. > > > > I have made a patch series (git link because file renames produce huge > > diffs) that renames *everything* away from r600 (and also radeonsi) > > to si, where it is actually about SI. In the such modified code it is > > then clear at first glance that only resources, textures and some > > other low-level interface code from R600 / generic Radeon are actually > > used in SI code. > > > > The patch series is ordered by increasing controversy potential due > > to destruction of git blame history, so the last parts can be omitted > > if they are deemed too destructive to history. In my opinion, it is > > better to have code that is readable now than code that is less > > readable but with the possibility to look up how it became like that. > > > > Michel said on IRC that he'd prefer to keep the name radeonsi_pipe.h/c, > > I disagree: If the library name is to be kept, there must be a break > > between radeonsi and si *somewhere*, and it is normal for library names > > to not correspond to any file name in the library. The same scheme is > > used in llvmpipe, llvmpipe lib / directory versus lp_* file names. > > > > Here's the repository (branch is master): > > git git://anongit.kde.org/scratch/ahartmetz/mesa.git > > web http://quickgit.kde.org/?p=scratch%2Fahartmetz%2Fmesa.git > > > > On Monday 06 January 2014 15:50:05 Marek Olšák wrote: > >> It sounds good, but I'd like the prefix to be si_ everywhere. > >> > >> Marek > >> > >> On Mon, Jan 6, 2014 at 2:47 PM, Andreas Hartmetz > > > > wrote: > >> > Hello, > >> > > >> > many of the files in radeonsi originally came from other places where > >> > they had different names and were never renamed. > >> > Most of them now have names that don't tell what the files are for > >> > (r600 is not actually the first hardware supported by them, they start > >> > at radeonsi), and even those with radeonsi are split between > >> > radeonsi_ and si_. > >> > si_ is shorter than radeonsi_, but inconsistent with the directory and > >> > library name. I still think it's the best option, but no strong opinion > >> > from me. If and when the files are renamed, the next step would be > >> > doing the same with the r600_ struct and function names. > >> > Does that sound good? I'll send the patches shortly if so. > >> > > >> > Cheers, > >> > Andreas > >> > ___ > >> > 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 5/9] tgsi: implement micro_fma() in tgsi exec machine
Indeed, this patch compiles on linux but may break complilation at least on MSVC version < 1400. I'm planing to install visual studio 2005 on a ms xp or seven virtual machine to check the behavior when compiling in microsoft windows environment. 2014/1/6 Erik Faye-Lund : > On Sun, Jan 5, 2014 at 12:42 AM, Maxence Le Doré > wrote: >> From: Maxence Le Doré > > Huh, user.email not configured properly? > >> static void >> +micro_fma(union tgsi_exec_channel *dst, >> + const union tgsi_exec_channel *src0, >> + const union tgsi_exec_channel *src1, >> + const union tgsi_exec_channel *src2) >> +{ >> + dst->f[0] = fmaf(src0->f[0],src1->f[0],src2->f[0]); > > fmaf isn't portable enough, no? ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Naming everything in src/gallium/drivers/radeonsi si_*
On Tuesday 07 January 2014 17:37:07 Christian König wrote: > Don't worry to much about history keeping, anybody who really needs that > should be capable of digging that up anyway. > > I would just squash together the changes "Apply si_ file naming scheme > in src/gallium/drive…" and "Fix up file renaming: change file names in > commen…". Also please change the subject lines to something like > "radeonsi: ..." > I tried doing squashing that fixup (and did so for a few others) by reordering in git rebase -i but hit merge conflicts. I can still do it when I need to change the patch series anyway, among other things for the commit messages. > Assuming that we already moved everything that r600 and radeonsi should > have in common under drivers/radeon the idea looks good to me. > > Christian. > > Am 07.01.2014 17:27, schrieb Andreas Hartmetz: > > We have talked on IRC meanwhile: > > "Everywhere" was supposed to mean file names and data structures. > > > > I have made a patch series (git link because file renames produce huge > > diffs) that renames *everything* away from r600 (and also radeonsi) > > to si, where it is actually about SI. In the such modified code it is > > then clear at first glance that only resources, textures and some > > other low-level interface code from R600 / generic Radeon are actually > > used in SI code. > > > > The patch series is ordered by increasing controversy potential due > > to destruction of git blame history, so the last parts can be omitted > > if they are deemed too destructive to history. In my opinion, it is > > better to have code that is readable now than code that is less > > readable but with the possibility to look up how it became like that. > > > > Michel said on IRC that he'd prefer to keep the name radeonsi_pipe.h/c, > > I disagree: If the library name is to be kept, there must be a break > > between radeonsi and si *somewhere*, and it is normal for library names > > to not correspond to any file name in the library. The same scheme is > > used in llvmpipe, llvmpipe lib / directory versus lp_* file names. > > > > Here's the repository (branch is master): > > git git://anongit.kde.org/scratch/ahartmetz/mesa.git > > web http://quickgit.kde.org/?p=scratch%2Fahartmetz%2Fmesa.git > > > > On Monday 06 January 2014 15:50:05 Marek Olšák wrote: > >> It sounds good, but I'd like the prefix to be si_ everywhere. > >> > >> Marek > >> > >> On Mon, Jan 6, 2014 at 2:47 PM, Andreas Hartmetz > > > > wrote: > >>> Hello, > >>> > >>> many of the files in radeonsi originally came from other places where > >>> they had different names and were never renamed. > >>> Most of them now have names that don't tell what the files are for > >>> (r600 is not actually the first hardware supported by them, they start > >>> at radeonsi), and even those with radeonsi are split between > >>> radeonsi_ and si_. > >>> si_ is shorter than radeonsi_, but inconsistent with the directory and > >>> library name. I still think it's the best option, but no strong opinion > >>> from me. If and when the files are renamed, the next step would be > >>> doing the same with the r600_ struct and function names. > >>> Does that sound good? I'll send the patches shortly if so. > >>> > >>> Cheers, > >>> Andreas ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/7] intel: support factors for min/max blending
Thanks Ian, I got this after a rebase. Didn't know were it comes from. 2014/1/6 Ian Romanick : > On 01/02/2014 05:18 PM, Maxence Le Doré wrote: >> --- >> src/mesa/drivers/dri/i915/i830_state.c | 12 >> src/mesa/drivers/dri/i965/brw_util.c | 24 +--- >> 2 files changed, 25 insertions(+), 11 deletions(-) >> >> diff --git a/src/mesa/drivers/dri/i915/i830_state.c >> b/src/mesa/drivers/dri/i915/i830_state.c >> index bbf0cef..6a5db43 100644 >> --- a/src/mesa/drivers/dri/i915/i830_state.c >> +++ b/src/mesa/drivers/dri/i915/i830_state.c > > i915_state.c should get the same treatment. > >> @@ -303,10 +303,16 @@ i830_set_blend_state(struct gl_context * ctx) >>eqnRGB = BLENDFUNC_MIN; >>funcRGB = SRC_BLND_FACT(BLENDFACT_ONE) | DST_BLND_FACT(BLENDFACT_ONE); >>break; >> + case GL_FACTOR_MIN_AMD: >> + eqnRGB = BLENDFUNC_MIN; >> + break; >> case GL_MAX: >>eqnRGB = BLENDFUNC_MAX; >>funcRGB = SRC_BLND_FACT(BLENDFACT_ONE) | DST_BLND_FACT(BLENDFACT_ONE); >>break; >> + case GL_FACTOR_MAX_AMD: >> + eqnRGB = BLENDFUNC_MAX; >> + break; >> case GL_FUNC_SUBTRACT: >>eqnRGB = BLENDFUNC_SUB; >>break; >> @@ -331,10 +337,16 @@ i830_set_blend_state(struct gl_context * ctx) >>eqnA = BLENDFUNC_MIN; >>funcA = SRC_BLND_FACT(BLENDFACT_ONE) | DST_BLND_FACT(BLENDFACT_ONE); >>break; >> + case GL_FACTOR_MIN_AMD: >> + eqnA = BLENDFUNC_MIN; >> + break; >> case GL_MAX: >>eqnA = BLENDFUNC_MAX; >>funcA = SRC_BLND_FACT(BLENDFACT_ONE) | DST_BLND_FACT(BLENDFACT_ONE); >>break; >> + case GL_FACTOR_MAX_AMD: >> + eqnA = BLENDFUNC_MAX; >> + break; >> case GL_FUNC_SUBTRACT: >>eqnA = BLENDFUNC_SUB; >>break; >> diff --git a/src/mesa/drivers/dri/i965/brw_util.c >> b/src/mesa/drivers/dri/i965/brw_util.c >> index 4a303cd..292b7e5 100644 >> --- a/src/mesa/drivers/dri/i965/brw_util.c >> +++ b/src/mesa/drivers/dri/i965/brw_util.c >> @@ -40,17 +40,19 @@ >> GLuint brw_translate_blend_equation( GLenum mode ) >> { >> switch (mode) { >> - case GL_FUNC_ADD: >> - return BRW_BLENDFUNCTION_ADD; >> - case GL_MIN: >> - return BRW_BLENDFUNCTION_MIN; >> - case GL_MAX: >> - return BRW_BLENDFUNCTION_MAX; >> - case GL_FUNC_SUBTRACT: >> - return BRW_BLENDFUNCTION_SUBTRACT; >> - case GL_FUNC_REVERSE_SUBTRACT: >> - return BRW_BLENDFUNCTION_REVERSE_SUBTRACT; >> - default: >> + case GL_FUNC_ADD: >> + return BRW_BLENDFUNCTION_ADD; >> + case GL_MIN: >> + case GL_FACTOR_MIN_AMD: >> + return BRW_BLENDFUNCTION_MIN; >> + case GL_MAX: >> + case GL_FACTOR_MAX_AMD: >> + return BRW_BLENDFUNCTION_MAX; >> + case GL_FUNC_SUBTRACT: >> + return BRW_BLENDFUNCTION_SUBTRACT; >> + case GL_FUNC_REVERSE_SUBTRACT: >> + return BRW_BLENDFUNCTION_REVERSE_SUBTRACT; >> + default: >>assert(0); >>return BRW_BLENDFUNCTION_ADD; >> } >> > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Naming everything in src/gallium/drivers/radeonsi si_*
On Tue, Jan 7, 2014 at 5:37 PM, Christian König wrote: > Don't worry to much about history keeping, anybody who really needs that > should be capable of digging that up anyway. > > I would just squash together the changes "Apply si_ file naming scheme in > src/gallium/drive…" and "Fix up file renaming: change file names in > commen…". Also please change the subject lines to something like "radeonsi: > ..." > > Assuming that we already moved everything that r600 and radeonsi should have > in common under drivers/radeon the idea looks good to me. No, we didn't. These are things that can be moved to drivers/radeon: - whole pipe_screen implementation - *_resource.c - queries (occlusion, timestamp, time_elapsed, streamout, statistics), the idea is to nuke all of the radeonsi query code and use the one from r600g - maybe the framebuffer MSAA state and sample mask (it's the same as Cayman) - maybe some of the *_blit.c code Marek > > Christian. > > Am 07.01.2014 17:27, schrieb Andreas Hartmetz: > >> We have talked on IRC meanwhile: >> "Everywhere" was supposed to mean file names and data structures. >> >> I have made a patch series (git link because file renames produce huge >> diffs) that renames *everything* away from r600 (and also radeonsi) >> to si, where it is actually about SI. In the such modified code it is >> then clear at first glance that only resources, textures and some >> other low-level interface code from R600 / generic Radeon are actually >> used in SI code. >> >> The patch series is ordered by increasing controversy potential due >> to destruction of git blame history, so the last parts can be omitted >> if they are deemed too destructive to history. In my opinion, it is >> better to have code that is readable now than code that is less >> readable but with the possibility to look up how it became like that. >> >> Michel said on IRC that he'd prefer to keep the name radeonsi_pipe.h/c, >> I disagree: If the library name is to be kept, there must be a break >> between radeonsi and si *somewhere*, and it is normal for library names >> to not correspond to any file name in the library. The same scheme is >> used in llvmpipe, llvmpipe lib / directory versus lp_* file names. >> >> Here's the repository (branch is master): >> git git://anongit.kde.org/scratch/ahartmetz/mesa.git >> web http://quickgit.kde.org/?p=scratch%2Fahartmetz%2Fmesa.git >> >> On Monday 06 January 2014 15:50:05 Marek Olšák wrote: >>> >>> It sounds good, but I'd like the prefix to be si_ everywhere. >>> >>> Marek >>> >>> On Mon, Jan 6, 2014 at 2:47 PM, Andreas Hartmetz >> >> wrote: Hello, many of the files in radeonsi originally came from other places where they had different names and were never renamed. Most of them now have names that don't tell what the files are for (r600 is not actually the first hardware supported by them, they start at radeonsi), and even those with radeonsi are split between radeonsi_ and si_. si_ is shorter than radeonsi_, but inconsistent with the directory and library name. I still think it's the best option, but no strong opinion from me. If and when the files are renamed, the next step would be doing the same with the r600_ struct and function names. Does that sound good? I'll send the patches shortly if so. Cheers, Andreas ___ 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 mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] mesa: check for MESA_FORMAT_RGB9_E5_FLOAT in _mesa_is_format_signed()
Reviewed-by: Marek Olšák Marek On Tue, Jan 7, 2014 at 5:35 PM, Brian Paul wrote: > This packed floating point format only stores positive values. > --- > src/mesa/main/formats.c |5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/src/mesa/main/formats.c b/src/mesa/main/formats.c > index eb2a07e..7e0ec23 100644 > --- a/src/mesa/main/formats.c > +++ b/src/mesa/main/formats.c > @@ -1972,8 +1972,9 @@ _mesa_is_format_unsigned(gl_format format) > GLboolean > _mesa_is_format_signed(gl_format format) > { > - if (format == MESA_FORMAT_R11_G11_B10_FLOAT) { > - /* this packed float format only stores unsigned values */ > + if (format == MESA_FORMAT_R11_G11_B10_FLOAT || > + format == MESA_FORMAT_RGB9_E5_FLOAT) { > + /* these packed float formats only store unsigned values */ >return GL_FALSE; > } > else { > -- > 1.7.10.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] Naming everything in src/gallium/drivers/radeonsi si_*
I agree with everything except these two: "Rename the commonly occurring rctx/r600 to now more suitable sctx." "Rename the commonly occurring rscreen to now more suitable sscreen." It's too much for my eye to handle. Marek On Tue, Jan 7, 2014 at 5:27 PM, Andreas Hartmetz wrote: > We have talked on IRC meanwhile: > "Everywhere" was supposed to mean file names and data structures. > > I have made a patch series (git link because file renames produce huge > diffs) that renames *everything* away from r600 (and also radeonsi) > to si, where it is actually about SI. In the such modified code it is > then clear at first glance that only resources, textures and some > other low-level interface code from R600 / generic Radeon are actually > used in SI code. > > The patch series is ordered by increasing controversy potential due > to destruction of git blame history, so the last parts can be omitted > if they are deemed too destructive to history. In my opinion, it is > better to have code that is readable now than code that is less > readable but with the possibility to look up how it became like that. > > Michel said on IRC that he'd prefer to keep the name radeonsi_pipe.h/c, > I disagree: If the library name is to be kept, there must be a break > between radeonsi and si *somewhere*, and it is normal for library names > to not correspond to any file name in the library. The same scheme is > used in llvmpipe, llvmpipe lib / directory versus lp_* file names. > > Here's the repository (branch is master): > git git://anongit.kde.org/scratch/ahartmetz/mesa.git > web http://quickgit.kde.org/?p=scratch%2Fahartmetz%2Fmesa.git > > On Monday 06 January 2014 15:50:05 Marek Olšák wrote: >> It sounds good, but I'd like the prefix to be si_ everywhere. >> >> Marek >> >> On Mon, Jan 6, 2014 at 2:47 PM, Andreas Hartmetz > wrote: >> > Hello, >> > >> > many of the files in radeonsi originally came from other places where >> > they had different names and were never renamed. >> > Most of them now have names that don't tell what the files are for >> > (r600 is not actually the first hardware supported by them, they start >> > at radeonsi), and even those with radeonsi are split between >> > radeonsi_ and si_. >> > si_ is shorter than radeonsi_, but inconsistent with the directory and >> > library name. I still think it's the best option, but no strong opinion >> > from me. If and when the files are renamed, the next step would be >> > doing the same with the r600_ struct and function names. >> > Does that sound good? I'll send the patches shortly if so. >> > >> > Cheers, >> > Andreas >> > ___ >> > 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 3/7] mesa: implement glBindTextures()
Thanks Fredrik, I was nearly sure this was ok from what I read of the specs. And you're also right again : I totally forget a case where a texture has never been bound ! 2014/1/7 Maxence Le Doré : > Thanks Fredrik, > I was nearly sure this was ok from what I read of the specs. And > you're also right again : I totally forget a case where a texture has > never been bound ! > > 2014/1/7 Fredrik Höglund : >> On Friday 03 January 2014, Marek Olšák wrote: >>> On Fri, Jan 3, 2014 at 2:04 PM, Marek Olšák wrote: >>> > On Fri, Jan 3, 2014 at 1:27 AM, Maxence Le Doré >>> > wrote: >>> >> --- >>> >> src/mesa/main/texobj.c | 52 >>> >> ++ >>> >> src/mesa/main/texobj.h | 3 +++ >>> >> 2 files changed, 55 insertions(+) >>> >> >>> >> diff --git a/src/mesa/main/texobj.c b/src/mesa/main/texobj.c >>> >> index bddbc50..66e2fb0 100644 >>> >> --- a/src/mesa/main/texobj.c >>> >> +++ b/src/mesa/main/texobj.c >>> >> @@ -1686,4 +1686,56 @@ _mesa_InvalidateTexImage(GLuint texture, GLint >>> >> level) >>> >> return; >>> >> } >>> >> >>> >> +/** ARB_multi_bind / OpenGL 4.4 */ >>> >> + >>> >> +void GLAPIENTRY >>> >> +_mesa_BindTextures(GLuint first, GLsizei count, const GLuint *textures) >>> >> +{ >>> >> + GET_CURRENT_CONTEXT(ctx); >>> >> + struct GLuint currentTexUnit = 0; >>> >> + int i = 0; >>> >> + >>> >> + currentTexUnit = ctx->Texture.CurrentUnit; >>> >> + >>> >> + if(first + count > ctx->Const.MaxCombinedTextureImageUnits) { >>> >> + _mesa_error(ctx, GL_INVALID_OPERATION, >>> >> "glBindTextures(first+count)"); >>> >> + return; >>> >> + } >>> >> + >>> >> + for(i = 0 ; i < count ; i++) { >>> >> + GLuint texture; >>> >> + struct gl_texture_object *texObj; >>> >> + GLenum texTarget; >>> >> + int j = 0; >>> >> + >>> >> + if(textures == NULL) >>> >> +texture = 0; >>> >> + else >>> >> +texture = textures[i]; >>> >> + >>> >> + _mesa_ActiveTexture(GL_TEXTURE0 + first + i); >>> >> + if(texture != 0) { >>> >> +texObj = _mesa_lookup_texture(ctx, texture); >>> >> +if(texObj) { >>> >> + texTarget = texObj->Target; >>> >> + _mesa_BindTexture(texTarget, texture); >>> >> +} >>> >> +else >>> >> + _mesa_error(ctx, GL_INVALID_OPERATION, >>> >> + "glBindTextures(textures[%i])", i); >>> > >>> > This error is set too late. It should be done before changing textures. >>> >>> Note that you make the same mistake in the other patches too. Also >>> please double-check that none of the _mesa_ functions generate errors. >> >> This is actually not the case with the ARB_multi_bind functions: >> >> (11) Typically, OpenGL specifies that if an error is generated by a >> command, that command has no effect. This is somewhat unfortunate >> for multi-bind commands, because it would require a first pass to >> scan the entire list of bound objects for errors and then a second >> pass to actually perform the bindings. Should we have different >> error semantics? >> >> RESOLVED: Yes. In this specification, when the parameters for one of >> the binding points are invalid, that binding point is not >> updated and an error will be generated. However, other binding points >> in the same command will be updated if their parameters are valid and >> no >> other error occurs. >> >> The code is still wrong for a different reason though; when a texture has >> has never been bound, it doesn't have a target. That case needs to be >> handled correctly. >> >> Fredrik >> ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] mesa: check bits per channel for GL_RGBA_SIGNED_COMPONENTS_EXT query
If a channel has zero bits it's not signed. --- src/mesa/main/get.c | 28 +--- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c index c45a8d1..bb138c8 100644 --- a/src/mesa/main/get.c +++ b/src/mesa/main/get.c @@ -769,13 +769,27 @@ find_custom_value(struct gl_context *ctx, const struct value_desc *d, union valu /* Note: we only check the 0th color attachment. */ const struct gl_renderbuffer *rb = ctx->DrawBuffer->_ColorDrawBuffers[0]; - const GLboolean is_signed = -rb ? _mesa_is_format_signed(rb->Format) : GL_FALSE; - /* At this time, all color channels have same signedness */ - v->value_int_4[0] = - v->value_int_4[1] = - v->value_int_4[2] = - v->value_int_4[3] = is_signed; + if (rb && _mesa_is_format_signed(rb->Format)) { +/* Issue 17 of GL_EXT_packed_float: If a component (such as + * alpha) has zero bits, the component should not be considered + * signed and so the bit for the respective component should be + * zeroed. + */ +v->value_int_4[0] = + _mesa_get_format_bits(rb->Format, GL_RED_BITS) > 0; +v->value_int_4[1] = + _mesa_get_format_bits(rb->Format, GL_GREEN_BITS) > 0; +v->value_int_4[2] = + _mesa_get_format_bits(rb->Format, GL_BLUE_BITS) > 0; +v->value_int_4[3] = + _mesa_get_format_bits(rb->Format, GL_ALPHA_BITS) > 0; + } + else { +v->value_int_4[0] = +v->value_int_4[1] = +v->value_int_4[2] = +v->value_int_4[3] = 0; + } } break; -- 1.7.10.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Naming everything in src/gallium/drivers/radeonsi si_*
Don't worry to much about history keeping, anybody who really needs that should be capable of digging that up anyway. I would just squash together the changes "Apply si_ file naming scheme in src/gallium/drive…" and "Fix up file renaming: change file names in commen…". Also please change the subject lines to something like "radeonsi: ..." Assuming that we already moved everything that r600 and radeonsi should have in common under drivers/radeon the idea looks good to me. Christian. Am 07.01.2014 17:27, schrieb Andreas Hartmetz: We have talked on IRC meanwhile: "Everywhere" was supposed to mean file names and data structures. I have made a patch series (git link because file renames produce huge diffs) that renames *everything* away from r600 (and also radeonsi) to si, where it is actually about SI. In the such modified code it is then clear at first glance that only resources, textures and some other low-level interface code from R600 / generic Radeon are actually used in SI code. The patch series is ordered by increasing controversy potential due to destruction of git blame history, so the last parts can be omitted if they are deemed too destructive to history. In my opinion, it is better to have code that is readable now than code that is less readable but with the possibility to look up how it became like that. Michel said on IRC that he'd prefer to keep the name radeonsi_pipe.h/c, I disagree: If the library name is to be kept, there must be a break between radeonsi and si *somewhere*, and it is normal for library names to not correspond to any file name in the library. The same scheme is used in llvmpipe, llvmpipe lib / directory versus lp_* file names. Here's the repository (branch is master): git git://anongit.kde.org/scratch/ahartmetz/mesa.git web http://quickgit.kde.org/?p=scratch%2Fahartmetz%2Fmesa.git On Monday 06 January 2014 15:50:05 Marek Olšák wrote: It sounds good, but I'd like the prefix to be si_ everywhere. Marek On Mon, Jan 6, 2014 at 2:47 PM, Andreas Hartmetz wrote: Hello, many of the files in radeonsi originally came from other places where they had different names and were never renamed. Most of them now have names that don't tell what the files are for (r600 is not actually the first hardware supported by them, they start at radeonsi), and even those with radeonsi are split between radeonsi_ and si_. si_ is shorter than radeonsi_, but inconsistent with the directory and library name. I still think it's the best option, but no strong opinion from me. If and when the files are renamed, the next step would be doing the same with the r600_ struct and function names. Does that sound good? I'll send the patches shortly if so. Cheers, Andreas ___ 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 mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] mesa: check for MESA_FORMAT_RGB9_E5_FLOAT in _mesa_is_format_signed()
This packed floating point format only stores positive values. --- src/mesa/main/formats.c |5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/mesa/main/formats.c b/src/mesa/main/formats.c index eb2a07e..7e0ec23 100644 --- a/src/mesa/main/formats.c +++ b/src/mesa/main/formats.c @@ -1972,8 +1972,9 @@ _mesa_is_format_unsigned(gl_format format) GLboolean _mesa_is_format_signed(gl_format format) { - if (format == MESA_FORMAT_R11_G11_B10_FLOAT) { - /* this packed float format only stores unsigned values */ + if (format == MESA_FORMAT_R11_G11_B10_FLOAT || + format == MESA_FORMAT_RGB9_E5_FLOAT) { + /* these packed float formats only store unsigned values */ return GL_FALSE; } else { -- 1.7.10.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/7] mesa: implement glBindBuffersBase() and gl BindBuffersRange()
You are absolutly right. I followed the way specification illustrate implementation for simplicity reasons but had always in mind that this was not the way to get any performance benefit and hoped that I are somebody else could improve this later. That's a great thing you already did this and don't be sorry about that, there's still lot of work in Mesa I could focus on instead :) 2014/1/7 Fredrik Höglund : > Maxence, while I think it's great that you're interested in working > on this extension, I'm afraid I have another implementation in > a branch in my mesa tree: > > http://cgit.freedesktop.org/~fredrik/mesa/log/?h=arb-multi-bind > > I've looked at your patches, and noticed you've implemented the > functions by calling the _mesa_Bind*() functions in a loop. I know > that the specification uses examples that look very much like this > to illustrate the intended effect of calling each function. But when > actually implemented in this this way you don't get the performance > benefit you would get by writing a specialized implementation of > each function. > > For example you can avoid locking the mutex that protects the hash > table more than once when you look up the pointers to the objects. > There is also some state validation that doesn't need to be repeated > for each object. > > Another downside is that when an error occurs, the _mesa_Bind*() > functions will report the wrong entry point in the error message. > The quality of those messages is important now that we support > KHR_debug, because they are reported to the client. > > While your implementation is simpler than mine, I think my approach > is better in the long term. But I think others should chime in on this. > > Fredrik > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Naming everything in src/gallium/drivers/radeonsi si_*
We have talked on IRC meanwhile: "Everywhere" was supposed to mean file names and data structures. I have made a patch series (git link because file renames produce huge diffs) that renames *everything* away from r600 (and also radeonsi) to si, where it is actually about SI. In the such modified code it is then clear at first glance that only resources, textures and some other low-level interface code from R600 / generic Radeon are actually used in SI code. The patch series is ordered by increasing controversy potential due to destruction of git blame history, so the last parts can be omitted if they are deemed too destructive to history. In my opinion, it is better to have code that is readable now than code that is less readable but with the possibility to look up how it became like that. Michel said on IRC that he'd prefer to keep the name radeonsi_pipe.h/c, I disagree: If the library name is to be kept, there must be a break between radeonsi and si *somewhere*, and it is normal for library names to not correspond to any file name in the library. The same scheme is used in llvmpipe, llvmpipe lib / directory versus lp_* file names. Here's the repository (branch is master): git git://anongit.kde.org/scratch/ahartmetz/mesa.git web http://quickgit.kde.org/?p=scratch%2Fahartmetz%2Fmesa.git On Monday 06 January 2014 15:50:05 Marek Olšák wrote: > It sounds good, but I'd like the prefix to be si_ everywhere. > > Marek > > On Mon, Jan 6, 2014 at 2:47 PM, Andreas Hartmetz wrote: > > Hello, > > > > many of the files in radeonsi originally came from other places where > > they had different names and were never renamed. > > Most of them now have names that don't tell what the files are for > > (r600 is not actually the first hardware supported by them, they start > > at radeonsi), and even those with radeonsi are split between > > radeonsi_ and si_. > > si_ is shorter than radeonsi_, but inconsistent with the directory and > > library name. I still think it's the best option, but no strong opinion > > from me. If and when the files are renamed, the next step would be > > doing the same with the r600_ struct and function names. > > Does that sound good? I'll send the patches shortly if so. > > > > Cheers, > > Andreas > > ___ > > 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] glsl: rename min(), max() functions to fix MSVC build
On Tue, Jan 7, 2014 at 4:11 PM, Brian Paul wrote: > On 01/07/2014 08:03 AM, Erik Faye-Lund wrote: >> Why was this pushed out prematurely, then? Reviewing within two hours >> seems to be reasonable, no? > > > I got Kenneth's R-b so I pushed it. I saw no reason to wait. Also, I don't > like build breaks to linger any longer than necessary. > Fair enough, thanks. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] i965: Fix clears of layered framebuffers with mismatched layer counts.
Previously, Mesa enforced the following rule (from ARB_geometry_shader4's list of criteria for framebuffer completeness): * If any framebuffer attachment is layered, all attachments must have the same layer count. For three-dimensional textures, the layer count is the depth of the attached volume. For cube map textures, the layer count is always six. For one- and two-dimensional array textures, the layer count is simply the number of layers in the array texture. { FRAMEBUFFER_INCOMPLETE_LAYER_COUNT_ARB } However, when ARB_geometry_shader4 was adopted into GL 3.2, this rule was dropped; GL 3.2 permits different attachments to have different layer counts. This patch brings Mesa in line with GL 3.2. In order to ensure that layered clears properly clear all layers, we now have to keep track of the maximum number of layers in a layered framebuffer. Fixes the following piglit tests in spec/!OpenGL 3.2/layered-rendering: - clear-color-all-types 1d_array mipmapped - clear-color-all-types 1d_array single_level - clear-color-mismatched-layer-count - framebuffer-layer-count-mismatch --- src/mesa/drivers/common/meta.c | 4 ++-- src/mesa/drivers/dri/i965/brw_blorp_clear.cpp| 8 src/mesa/drivers/dri/i965/brw_clear.c| 6 +++--- src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 2 +- src/mesa/drivers/dri/i965/gen6_clip_state.c | 2 +- src/mesa/drivers/dri/i965/gen7_misc_state.c | 2 +- src/mesa/main/fbobject.c | 26 src/mesa/main/mtypes.h | 9 8 files changed, 29 insertions(+), 30 deletions(-) diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c index 7b41876..1294514 100644 --- a/src/mesa/drivers/common/meta.c +++ b/src/mesa/drivers/common/meta.c @@ -2407,9 +2407,9 @@ _mesa_meta_glsl_Clear(struct gl_context *ctx, GLbitfield buffers) GL_DYNAMIC_DRAW_ARB); /* draw quad(s) */ - if (fb->NumLayers > 0) { + if (fb->MaxNumLayers > 0) { unsigned layer; - for (layer = 0; layer < fb->NumLayers; layer++) { + for (layer = 0; layer < fb->MaxNumLayers; layer++) { if (fb->_IntegerColor) _mesa_Uniform1i(clear->IntegerLayerLocation, layer); else diff --git a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp index 072ad55..c55108a 100644 --- a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp +++ b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp @@ -573,14 +573,14 @@ brw_blorp_clear_color(struct brw_context *brw, struct gl_framebuffer *fb, if (rb == NULL) continue; - if (fb->NumLayers > 0) { + if (fb->MaxNumLayers > 0) { unsigned layer_multiplier = (irb->mt->msaa_layout == INTEL_MSAA_LAYOUT_UMS || irb->mt->msaa_layout == INTEL_MSAA_LAYOUT_CMS) ? irb->mt->num_samples : 1; - assert(fb->NumLayers * layer_multiplier == -irb->mt->level[irb->mt_level].depth); - for (unsigned layer = 0; layer < fb->NumLayers; layer++) { + unsigned num_layers = +irb->mt->level[irb->mt_level].depth / layer_multiplier; + for (unsigned layer = 0; layer < num_layers; layer++) { if (!do_single_blorp_clear(brw, fb, rb, buf, partial_clear, layer * layer_multiplier)) { return false; diff --git a/src/mesa/drivers/dri/i965/brw_clear.c b/src/mesa/drivers/dri/i965/brw_clear.c index 1cac996..fe68d9e 100644 --- a/src/mesa/drivers/dri/i965/brw_clear.c +++ b/src/mesa/drivers/dri/i965/brw_clear.c @@ -181,9 +181,9 @@ brw_fast_clear_depth(struct gl_context *ctx) */ intel_batchbuffer_emit_mi_flush(brw); - if (fb->NumLayers > 0) { - assert(fb->NumLayers == depth_irb->mt->level[depth_irb->mt_level].depth); - for (unsigned layer = 0; layer < fb->NumLayers; layer++) { + if (fb->MaxNumLayers > 0) { + unsigned num_layers = depth_irb->mt->level[depth_irb->mt_level].depth; + for (unsigned layer = 0; layer < num_layers; layer++) { intel_hiz_exec(brw, mt, depth_irb->mt_level, layer, GEN6_HIZ_OP_DEPTH_CLEAR); } diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c index 5236eda..6bd4a29 100644 --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c @@ -700,7 +700,7 @@ brw_update_renderbuffer_surfaces(struct brw_context *brw) for (i = 0; i < ctx->DrawBuffer->_NumColorDrawBuffers; i++) { if (intel_renderbuffer(ctx->DrawBuffer->_ColorDrawBuffers[i])) { brw->vtbl.update_renderbuffer_surface(brw, ctx->DrawBuffer->_ColorDrawBuffers[i], - ctx->DrawBuffer->NumLayers > 0, i); +
[Mesa-dev] [PATCH 1/2] main: check texture target when validating layered framebuffers.
>From section 4.4.4 (Framebuffer Completeness) of the GL 3.2 spec: If any framebuffer attachment is layered, all populated attachments must be layered. Additionally, all populated color attachments must be from textures of the same target. We weren't checking that the attachments were from textures of the same target. --- src/mesa/main/fbobject.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c index 2892784..c3f634f 100644 --- a/src/mesa/main/fbobject.c +++ b/src/mesa/main/fbobject.c @@ -877,8 +877,9 @@ _mesa_test_framebuffer_completeness(struct gl_context *ctx, GLint fixedSampleLocations = -1; GLint i; GLuint j; - bool layer_count_valid = false; + bool layer_info_valid = false; /* Covers layer_count and layer_tex_target */ GLuint layer_count = 0, att_layer_count; + GLenum layer_tex_target = 0; assert(_mesa_is_user_fbo(fb)); @@ -1062,9 +1063,14 @@ _mesa_test_framebuffer_completeness(struct gl_context *ctx, } else { att_layer_count = 0; } - if (!layer_count_valid) { + if (!layer_info_valid) { layer_count = att_layer_count; - layer_count_valid = true; + layer_tex_target = att_tex_target; + layer_info_valid = true; + } else if (layer_count > 0 && layer_tex_target != att_tex_target) { + fb->_Status = GL_FRAMEBUFFER_INCOMPLETE_LAYER_TARGETS; + fbo_incomplete(ctx, "layered framebuffer has mismatched targets", i); + return; } else if (layer_count != att_layer_count) { if (layer_count == 0 || att_layer_count == 0) { fb->_Status = GL_FRAMEBUFFER_INCOMPLETE_LAYER_TARGETS; -- 1.8.5.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: rename min(), max() functions to fix MSVC build
- Original Message - > On Tue, Jan 7, 2014 at 3:59 PM, Brian Paul wrote: > > On 01/06/2014 05:22 PM, Erik Faye-Lund wrote: > >> > >> On Tue, Jan 7, 2014 at 12:12 AM, Brian Paul wrote: > >>> > >>> Evidently, there's some other definition of "min" and "max" that > >>> causes MSVC to choke on these function names. Renaming to min2() > >>> and max2() fixes things. > >> > >> > >> Wouldn't it be easier to just make sure NOMINMAX is defined before > >> including any system-headers? > > > > > > I wasn't aware of that option. I think I'll just leave things as-is for > > now > > to avoid churn. > > > > Why was this pushed out prematurely, then? Reviewing within two hours > seems to be reasonable, no? The MSVC build was broken: ie, no builds, no tests, nothing. Everything was blocked until this is fixed, so if another unrelated change is pushed in and happens to introduce a regression we don't see it until the build is fixed. Build failures are also a pain when one day one needs to bisect across this change. This is why _any_ fix is better than no fix in such circunstances. I even think that review is optional for trivial changes. The most important is get builds going again. Jose ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/7] mesa: implement glBindBuffersBase() and gl BindBuffersRange()
On 01/07/2014 12:49 AM, Fredrik Höglund wrote: Maxence, while I think it's great that you're interested in working on this extension, I'm afraid I have another implementation in a branch in my mesa tree: https://urldefense.proofpoint.com/v1/url?u=http://cgit.freedesktop.org/~fredrik/mesa/log/?h%3Darb-multi-bind&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=lGQMzzTgII0I7jefp2FHq7WtZ%2BTLs8wadB%2BiIj9xpBY%3D%0A&m=Qnaa0wxMyN0kizD7ysRrgyEbk4PXjC2dP%2BLpd%2FgAmuk%3D%0A&s=8fa229ff9512e319f49a8dede5aa158170e34178a81f3aff3e3d4164c084d574 I've looked at your patches, and noticed you've implemented the functions by calling the _mesa_Bind*() functions in a loop. I know that the specification uses examples that look very much like this to illustrate the intended effect of calling each function. But when actually implemented in this this way you don't get the performance benefit you would get by writing a specialized implementation of each function. For example you can avoid locking the mutex that protects the hash table more than once when you look up the pointers to the objects. There is also some state validation that doesn't need to be repeated for each object. Another downside is that when an error occurs, the _mesa_Bind*() functions will report the wrong entry point in the error message. The quality of those messages is important now that we support KHR_debug, because they are reported to the client. While your implementation is simpler than mine, I think my approach is better in the long term. But I think others should chime in on this. I think your series looks pretty good, Fredrik. I don't recall you posting the patches for review yet. When you do I'll have a few small comments. Will you be writing some piglit tests too? -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: rename min(), max() functions to fix MSVC build
On 01/07/2014 08:03 AM, Erik Faye-Lund wrote: On Tue, Jan 7, 2014 at 3:59 PM, Brian Paul wrote: On 01/06/2014 05:22 PM, Erik Faye-Lund wrote: On Tue, Jan 7, 2014 at 12:12 AM, Brian Paul wrote: Evidently, there's some other definition of "min" and "max" that causes MSVC to choke on these function names. Renaming to min2() and max2() fixes things. Wouldn't it be easier to just make sure NOMINMAX is defined before including any system-headers? I wasn't aware of that option. I think I'll just leave things as-is for now to avoid churn. Why was this pushed out prematurely, then? Reviewing within two hours seems to be reasonable, no? I got Kenneth's R-b so I pushed it. I saw no reason to wait. Also, I don't like build breaks to linger any longer than necessary. -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 69101] prime: black window
https://bugs.freedesktop.org/show_bug.cgi?id=69101 --- Comment #16 from Mike Lothian --- If you're running the lastest verion of Heaven or running Valley on drivers that don't yet have OpenGL 3.2 it'll fail to render (SNB doesn't) - try passing MESA_GL_VERSION_OVERRIDE=3.2 MESA_GLSL_VERSION_OVERRIDE=150 to see if it then works -- 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 69101] prime: black window
https://bugs.freedesktop.org/show_bug.cgi?id=69101 --- Comment #15 from Jan Vesely --- (In reply to comment #12) > I see only comments about intel+radeon here. > It would be interesting to know if there are same problems with > intel+nouveau or radeon+radeon On my nvd9 + snb (gnome3): glxgears works ok openarena works ok (fullscreen) Unigine Heaven/Valley shows black screen, it seems to be running (even blind clicking works) it just shows black window. The window is not resizable, but moving it around or minimize/restore does not help. -- 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] glsl: rename min(), max() functions to fix MSVC build
On Tue, Jan 7, 2014 at 3:59 PM, Brian Paul wrote: > On 01/06/2014 05:22 PM, Erik Faye-Lund wrote: >> >> On Tue, Jan 7, 2014 at 12:12 AM, Brian Paul wrote: >>> >>> Evidently, there's some other definition of "min" and "max" that >>> causes MSVC to choke on these function names. Renaming to min2() >>> and max2() fixes things. >> >> >> Wouldn't it be easier to just make sure NOMINMAX is defined before >> including any system-headers? > > > I wasn't aware of that option. I think I'll just leave things as-is for now > to avoid churn. > Why was this pushed out prematurely, then? Reviewing within two hours seems to be reasonable, no? ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] swrast: fix delayed texel buffer allocation regression for OpenMP
On 01/07/2014 03:10 AM, Andreas Fänger wrote: Commit 9119269ca14ed42b51c7d8e2e662500311b29fa3 moved the texel buffer allocation to _swrast_texture_span(), however, when compiled with OpenMP support this code already runs multi-threaded so a critical section is required to prevent multiple allocations and rendering errors. --- src/mesa/swrast/s_texcombine.c | 12 1 files changed, 12 insertions(+), 0 deletions(-) diff --git a/src/mesa/swrast/s_texcombine.c b/src/mesa/swrast/s_texcombine.c index 7e07f4f..297491e 100644 --- a/src/mesa/swrast/s_texcombine.c +++ b/src/mesa/swrast/s_texcombine.c @@ -602,6 +602,14 @@ _swrast_texture_span( struct gl_context *ctx, SWspan *span ) if (!swrast->TexelBuffer) { #ifdef _OPENMP const GLint maxThreads = omp_get_max_threads(); + + /* TexelBuffer memory allocation needs to be done in a critical section + * as this code runs in a parallel loop. + * When entering the section, first check if TexelBuffer has been + * initialized already by another thread while this thread was waiting. + */ + #pragma omp critical + if (!swrast->TexelBuffer) { #else const GLint maxThreads = 1; #endif @@ -613,6 +621,10 @@ _swrast_texture_span( struct gl_context *ctx, SWspan *span ) swrast->TexelBuffer = malloc(ctx->Const.FragmentProgram.MaxTextureImageUnits * maxThreads * SWRAST_MAX_WIDTH * 4 * sizeof(GLfloat)); +#ifdef _OPENMP + } /* critical section */ +#endif + if (!swrast->TexelBuffer) { _mesa_error(ctx, GL_OUT_OF_MEMORY, "texture_combine"); return; Reviewed-by: Brian Paul I'll push this soon. Thanks. -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: rename min(), max() functions to fix MSVC build
On 01/06/2014 05:22 PM, Erik Faye-Lund wrote: On Tue, Jan 7, 2014 at 12:12 AM, Brian Paul wrote: Evidently, there's some other definition of "min" and "max" that causes MSVC to choke on these function names. Renaming to min2() and max2() fixes things. Wouldn't it be easier to just make sure NOMINMAX is defined before including any system-headers? I wasn't aware of that option. I think I'll just leave things as-is for now to avoid churn. -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 69101] prime: black window
https://bugs.freedesktop.org/show_bug.cgi?id=69101 --- Comment #14 from Alex Deucher --- Prime only works with compositing, so you have to have compositing enabled. -- 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 3/3] i965/gen6/blorp: Remove too heavy HiZ workaround
On 6 January 2014 17:05, Chad Versace wrote: > Commit 1a92881 added extra flushes to fix a HiZ hang in > WebGL Google Maps. With the extra flushes emitted by the previous two > patches, the flushes added by 1a92881 are redundant. > > Tested with the same criteria as in 1a92881: by zooming in and out > continuously for 2 hours on Sandybridge Chrome OS (codename > Stumpy) without a hang. > > CC: mesa-sta...@lists.freedesktop.org > CC: Kenneth Graunke > CC: Paul Berry > CC: Stéphane Marchesin > Signed-off-by: Chad Versace > --- > src/mesa/drivers/dri/i965/gen6_blorp.cpp | 14 -- > 1 file changed, 14 deletions(-) > I'm baffled by this. My understanding of patches 1 and 2 is that they both set need_workaround_flush redundantly, so they should have no effect on what is sent to the hardware. Therefore, it seems like this patch puts us back in the buggy state we were in before commit 1a92881. Have I missed something in my analysis? What was the previous MTBF on your system before commit 1a92881? Is it possible that 2 hours isn't a long enoug test? (For example, if the MTBF was 40 minutes, and the failures were Poisson-distributed, then I believe that 2 hours without a failure only confirms the bug fix at 95% confidence.) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 69101] prime: black window
https://bugs.freedesktop.org/show_bug.cgi?id=69101 --- Comment #13 from Armin K --- (In reply to comment #5) > > And without DRI_PRIME=1 everything works everywhere, with or without > > compositing, windowed or fullscreen. > > I would imagine this is because you're not offloading anything at that point > and running it on the IGP. > > >I don't recall having any problems under KWin or Xfwm4. > > This is because KWin defaults to Xrender, and Xfwm4 is *only* Xrender. > xcompmgr works fine, too, as do the default settings on Compton. > I've used KWin OGL3.1 backend. For Xfwm4, if you don't enable Display Compositing in Settings -> Window Manager Tweaks -> Compositor, you'll get black screen too. > If you try this on: > > * GNOME3 > * Cinnamon > * Compton with --backend glx > > You will get a black screen. > > However, if the screen is *not* full screen, you can resize the screen and > the image will appear (such as with glxgears or glxspheres). > > Unfortunately, in screens that are "full screen windowed", as are many Wine > games, it still shows up black and there's no way to resize the screen in > order to make the image show up. In any case, that's not really an > acceptable workaround because it just shows that there's something wrong > with PRIME offloading that causes this odd behavior on GLX-based compositors. -- 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 2/3] i965/gen6/blorp: Set need_workaround_flush at top of blorp
On 6 January 2014 17:05, Chad Versace wrote: > Unconditionally set brw->need_workaround_flush at the top of gen6 blorp > state emission. This is an extra safety measure to prevent undiscovered > difficult-to-diagnose gpu hangs. > > The art of emitting workaround flushes on Sandybridge is mysterious and > not fully understood. Ken and I believe that > intel_emit_post_sync_nonzero_flush() may be required when switching from > regular drawing to blorp. > > CC: mesa-sta...@lists.freedesktop.org > CC: Kenneth Graunke > CC: Paul Berry > CC: Stéphane Marchesin > Signed-off-by: Chad Versace > --- > src/mesa/drivers/dri/i965/gen6_blorp.cpp | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/src/mesa/drivers/dri/i965/gen6_blorp.cpp > b/src/mesa/drivers/dri/i965/gen6_blorp.cpp > index 929d7b5..9db0840 100644 > --- a/src/mesa/drivers/dri/i965/gen6_blorp.cpp > +++ b/src/mesa/drivers/dri/i965/gen6_blorp.cpp > @@ -1023,7 +1023,6 @@ gen6_emit_hiz_workaround(struct brw_context *brw, > enum gen6_hiz_op hiz_op) > */ > if (hiz_op == GEN6_HIZ_OP_DEPTH_RESOLVE || > hiz_op == GEN6_HIZ_OP_HIZ_RESOLVE) { > - brw->batch.need_workaround_flush = true; >intel_emit_post_sync_nonzero_flush(brw); >intel_emit_depth_stall_flushes(brw); > } > @@ -1051,6 +1050,9 @@ gen6_blorp_exec(struct brw_context *brw, > > uint32_t prog_offset = params->get_wm_prog(brw, &prog_data); > > + /* Emit workaround flushes when we switch from drawing to blorping. */ > + brw->batch.need_workaround_flush = true; > + > This seems unnecessary to me. We already set need_workaround_flush whenever we emit a primitive, so isn't it guaranteed to be set whenever we reach this point in the code? ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] i965/gen6: Set need_workaround_flush after each primitive
On 6 January 2014 17:05, Chad Versace wrote: > Set brw->need_workaround_flush immediately after each 3D_CMD_PRIM. This > may prevent undiscovered difficult-to-diagnose gpu hangs, according to > Ken. > > The art of emitting workaround flushes on Sandybridge is mysterious and > not fully understood. Ken's intuition says that > intel_emit_post_sync_nonzero_flush() is required after each 3D_CMD_PRIM. > > On gen6, there are two places where we emit 3D_CMD_PRIM: brw_emit_prim() > and gen6_blorp_emit_primitive(). The former already sets > need_workaround_flush, so this patch needs only to change the latter. > > There is no need to set need_workaround_flush in > gen7_blorp_emit_primitive() because the workaround applies only to gen6. > > CC: mesa-sta...@lists.freedesktop.org > CC: Kenneth Graunke > CC: Paul Berry > CC: Stéphane Marchesin > Signed-off-by: Chad Versace > --- > src/mesa/drivers/dri/i965/gen6_blorp.cpp | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/src/mesa/drivers/dri/i965/gen6_blorp.cpp > b/src/mesa/drivers/dri/i965/gen6_blorp.cpp > index 441d61f..929d7b5 100644 > --- a/src/mesa/drivers/dri/i965/gen6_blorp.cpp > +++ b/src/mesa/drivers/dri/i965/gen6_blorp.cpp > @@ -1010,6 +1010,9 @@ gen6_blorp_emit_primitive(struct brw_context *brw, > OUT_BATCH(0); > OUT_BATCH(0); > ADVANCE_BATCH(); > + > + /* Only used on Sandybridge; harmless to set elsewhere. */ > + brw->batch.need_workaround_flush = true; > This is not strictly necessary, since we already set brw->batch.need_workaround_flush in brw_blorp_exec() after running the blorp operation. However, doing it here in gen6_blorp_emit_primitive() seems slightly better, since it makes the blorp codepath more similar to the non-blorp codepath. My personal preference would be to go ahead with this patch, but squash in a removal of the code in brw_blorp_exec() that sets need_workaround_flush. But I don't have a strong preference. Either way the patch is: Reviewed-by: Paul Berry ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/3] i965/gen6: Emit more flushes
On 6 January 2014 17:05, Chad Versace wrote: > In solving a HiZ hang before Christmas vacation, I discovered that Mesa > wasn't > emitting sufficient workaround flushes. That bug was solved in commit > 1a92881. > > This series, prompted by Ken, adds some more carefully placed flushes to > pre-emptively fix undiscovered gpu hangs. Gpu hangs are difficult to > diagnose, > hurt users pretty hard, and are overall nasty, so it's a good idea to put > safeguards into the driver to prevent them. > > Chad Versace (3): > i965/gen6: Set need_workaround_flush after each primitive > i965/gen6/blorp: Set need_workaround_flush at top of blorp > i965/gen6/blorp: Remove too heavy HiZ workaround > > src/mesa/drivers/dri/i965/gen6_blorp.cpp | 19 +-- > 1 file changed, 5 insertions(+), 14 deletions(-) > > -- > 1.8.5.2 > > I suspect you meant to send this series to mesa-dev@lists.freedesktop.org? You actually sent it to m...@lists.freedesktop.org instead. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/4] winsys/radeon: Keep bo statistics
On Tue, Jan 7, 2014 at 11:14 AM, Lauri Kasanen wrote: > On Tue, 7 Jan 2014 01:44:28 +0100 > Marek Olšák wrote: > >> On Mon, Jan 6, 2014 at 12:17 PM, Lauri Kasanen wrote: >> > These will be used later on for optimizing the VRAM placement. >> > >> > No measurable overhead (glxgears). >> >> I recommend testing torcs (the Forza track) next time. glxgears is not >> useful here. > > Can you elaborate why Torcs? I used glxgears, because it indeed was > useful (if you check the previous patches, before the timing was moved > into a thread, there was a 7% glxgears regression). Torcs has higher CPU overhead and it's all in the Mesa driver. glxgears only spends 50% of the time there, DRI2SwapBuffers and the kernel takes the other 50%. > >> > +++ b/src/gallium/winsys/radeon/drm/radeon_drm_cs.c >> > @@ -361,6 +361,14 @@ static unsigned radeon_drm_cs_add_reloc(struct >> > radeon_winsys_cs *rcs, >> > +if (usage & RADEON_USAGE_WRITE) { >> > +bo->stats.num_writes++; >> > +bo->stats.last_write_time = stats_time_get(ws); >> > +} else { >> > +bo->stats.num_reads++; >> > +bo->stats.last_read_time = stats_time_get(ws); >> > +} >> >> You do know that this is mostly useless, don't you? You sure won't >> know which resources are used for reading only, because >> glBufferSubData or glTexSubImage may generate a write, and an app >> which uploads data every frame or every couple of draw calls can cause >> the resource to appear as being used for writing all the time, while >> in reality it's only used for reading during rendering. > > That's ok, as they are both implemented as a blit - a write. Doing that > write on the gpu hw, but in system RAM, is not ideal. > > I don't see how it would skew the results badly, since if the app > bothers to update a texture every frame, that texture is surely > important to the app. > > As the cost also takes into account the buffer size, having a small but > often-written texture (such as a HUD that is read once per frame) would > not displace a large read-only texture. > >> You also won't >> know how many draw calls the resource is used in, because the function >> is called only once per state change and there can be many draw calls >> per state change. ... which may be called many >> times for the same buffer. > > That sounds contradictory ;) *"there can be a lot of draw calls after every state change" > > I see that r600_context_bo_reloc is called on every call of > r600_draw_vbo, r600_emit_shader. Though for vertex, constant buffers In draw_vbo, r600_context_bo_reloc should only be called for the index buffer and the streamout "filled_size" buffer without dirty tracking. > and sampler views there's indeed dirty_mask checks. For those, > additional stats calls will need to be added in the places that update > that dirty_mask for correct results. This would add unnecessary overhead. I don't think you need so accurate information, especially when you have to sacrifice CPU time. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev