[Mesa-dev] [Bug 73371] EGL_NOK_texture_from_pixmap is advertised but not properly set.

2014-01-07 Thread bugzilla-daemon
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

2014-01-07 Thread Vadim Girlin
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

2014-01-07 Thread Zack Rusin
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

2014-01-07 Thread Anuj Phogat
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

2014-01-07 Thread Roland Scheidegger
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

2014-01-07 Thread Chad Versace
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

2014-01-07 Thread Chad Versace
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)

2014-01-07 Thread Chad Versace
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

2014-01-07 Thread Chad Versace
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

2014-01-07 Thread Chad Versace
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.

2014-01-07 Thread bugzilla-daemon
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.

2014-01-07 Thread Kenneth Graunke
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

2014-01-07 Thread Thomas Sondergaard

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

2014-01-07 Thread Thomas Sondergaard
---
 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

2014-01-07 Thread Thomas Sondergaard
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

2014-01-07 Thread Thomas Sondergaard
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

2014-01-07 Thread Thomas Sondergaard
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

2014-01-07 Thread Thomas Sondergaard

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

2014-01-07 Thread Brian Paul

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.

2014-01-07 Thread Brian Paul

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.

2014-01-07 Thread Brian Paul

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.

2014-01-07 Thread Brian Paul

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.

2014-01-07 Thread Brian Paul

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.

2014-01-07 Thread Kenneth Graunke
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

2014-01-07 Thread Kenneth Graunke
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

2014-01-07 Thread Ian Romanick
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

2014-01-07 Thread Ian Romanick
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

2014-01-07 Thread Matt Turner
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

2014-01-07 Thread Martin Andersson
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.

2014-01-07 Thread Paul Berry
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.

2014-01-07 Thread Paul Berry
---
 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.

2014-01-07 Thread Paul Berry
---
 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.

2014-01-07 Thread Paul Berry
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.

2014-01-07 Thread Paul Berry
---
 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.

2014-01-07 Thread Paul Berry
---
 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.

2014-01-07 Thread Paul Berry
---
 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.

2014-01-07 Thread Paul Berry
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

2014-01-07 Thread Ian Romanick
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

2014-01-07 Thread Thomas Sondergaard

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

2014-01-07 Thread Brian Paul

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

2014-01-07 Thread Thomas Sondergaard
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

2014-01-07 Thread Thomas Sondergaard
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

2014-01-07 Thread Thomas Sondergaard
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

2014-01-07 Thread Thomas Sondergaard
---
 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

2014-01-07 Thread Thomas Sondergaard

---
 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

2014-01-07 Thread Marek Olšák
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

2014-01-07 Thread 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,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

2014-01-07 Thread Matt Turner
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

2014-01-07 Thread Maxence Le Doré
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

2014-01-07 Thread Kenneth Graunke
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.

2014-01-07 Thread bugzilla-daemon
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.

2014-01-07 Thread Matt Turner
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).

2014-01-07 Thread Matt Turner
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.

2014-01-07 Thread bugzilla-daemon
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.

2014-01-07 Thread bugzilla-daemon
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.

2014-01-07 Thread bugzilla-daemon
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.

2014-01-07 Thread bugzilla-daemon
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

2014-01-07 Thread 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 4/9] gallium-tgsi: add TGSI_OPCODE_{FMA-POPCNT-MSB-LSB} description

2014-01-07 Thread Roland Scheidegger
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

2014-01-07 Thread Chad Versace
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

2014-01-07 Thread Roland Scheidegger
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

2014-01-07 Thread Roland Scheidegger
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

2014-01-07 Thread 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
 +#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

2014-01-07 Thread Maxence Le Doré
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

2014-01-07 Thread Lauri Kasanen
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

2014-01-07 Thread Lauri Kasanen
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

2014-01-07 Thread Lauri Kasanen
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)

2014-01-07 Thread Lauri Kasanen
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

2014-01-07 Thread Maxence Le Doré
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_*

2014-01-07 Thread Andreas Hartmetz
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

2014-01-07 Thread Maxence Le Doré
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_*

2014-01-07 Thread Andreas Hartmetz
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

2014-01-07 Thread Maxence Le Doré
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_*

2014-01-07 Thread Marek Olšák
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()

2014-01-07 Thread Marek Olšák
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_*

2014-01-07 Thread Marek Olšák
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()

2014-01-07 Thread 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 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

2014-01-07 Thread 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;
 
-- 
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_*

2014-01-07 Thread Christian König
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()

2014-01-07 Thread Brian Paul
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()

2014-01-07 Thread Maxence Le Doré
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_*

2014-01-07 Thread 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


Re: [Mesa-dev] [PATCH] glsl: rename min(), max() functions to fix MSVC build

2014-01-07 Thread Erik Faye-Lund
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.

2014-01-07 Thread Paul Berry
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.

2014-01-07 Thread Paul Berry
>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

2014-01-07 Thread Jose Fonseca
- 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()

2014-01-07 Thread Brian Paul

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

2014-01-07 Thread Brian Paul

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

2014-01-07 Thread bugzilla-daemon
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

2014-01-07 Thread bugzilla-daemon
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

2014-01-07 Thread Erik Faye-Lund
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

2014-01-07 Thread Brian Paul

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

2014-01-07 Thread Brian Paul

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

2014-01-07 Thread bugzilla-daemon
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

2014-01-07 Thread Paul Berry
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

2014-01-07 Thread bugzilla-daemon
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

2014-01-07 Thread Paul Berry
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

2014-01-07 Thread Paul Berry
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

2014-01-07 Thread Paul Berry
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

2014-01-07 Thread Marek Olšák
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


  1   2   >