Re: [Mesa-dev] [PATCH 3/4] nine: handle D3DISSUE_END without previous D3DISSUE_BEGIN

2014-11-21 Thread Ilia Mirkin
On Sat, Nov 22, 2014 at 1:45 AM, John Ettedgui  wrote:
> On Fri Nov 21 2014 at 10:36:29 PM Ilia Mirkin  wrote:
>>
>> Right, I figured as much. My point is that doing what I'm proposing
>> there would allow such usage even though it is illegal based on the
>> API description.
>>
> Alright.
>>
>> It requires a begin_query for some but not all queries.
>>
>> Perhaps the code should read like
>>
>> } else {
>> if (This->state != NINE_QUERY_STATE_RUNNING)
>> pipe->begin_query(pipe, This->pq);
>> pipe->end_query(pipe, This->pq);
>> This->state = NINE_QUERY_STATE_ENDED;
>> }
>>
>> This seems like it would also take care of the Ended -> Ended
>> transition which is supposed to restart the query.
>
> My understanding is that the End->End restart is only in the final state of
> the query, so it shouldn't happen in the others.
> Do you see that differently?

No, but it seems like my proposed code above handles all the
situations... if it's in the Building state (aka 'RUNNING'), then it
just executes end_query. If it's in any other state (i.e. FRESH,
ENDED, or FLUSHED) it will start a query anew and flip back to the
ENDED state (which roughly corresponds to Issued). I guess
FRESH/FLUSHED correspond to 'Signaled', although ENDED can mean
'Signaled' as well since nothing changes the state if
pipe->get_query_results succeeds.

BTW, please note that I know exceedingly little about the d3d9 api --
I'm just going by the msdn page you linked to + knowledge of gallium.
I could well have misunderstood something.

Cheers,

  -ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 3/4] nine: handle D3DISSUE_END without previous D3DISSUE_BEGIN

2014-11-21 Thread Ilia Mirkin
On Sat, Nov 22, 2014 at 1:28 AM, John Ettedgui  wrote:
> On Fri Nov 21 2014 at 10:14:19 PM Ilia Mirkin  wrote:
>>
>> Oh, I think i see the problem.
>>
>> In GetData:
>>
>> user_assert(This->state != NINE_QUERY_STATE_RUNNING,
>> D3DERR_INVALIDCALL);
>>
>> should be
>>
>>   user_assert(This->state != ... || This->state |= FRESH,
>> D3DERR_INVALIDCALL)
>>
>> This will allow someone to call GetData without first having done an
>> Issue() but... meh.
>
> Not saying your idea is wrong but Issue was called before GetData in my
> case.
> It was just called on D3DISSUE_END, with no call on D3DISSUE_BEGIN.

Right, I figured as much. My point is that doing what I'm proposing
there would allow such usage even though it is illegal based on the
API description.

>>
>> You could alternatively rejigger the state
>> variables, but that seems excessive.
>>
>> Actually perhaps it's supposed to actually call ->end_query()
>> unconditionally in ->Issue even though it hasn't called
>> ->begin_query()? That might make even more sense...
>
> I thought about that as well, though I thought that end_query() would
> require a begin_query() first (though issue does not so maybe...)

It requires a begin_query for some but not all queries.

Perhaps the code should read like

} else {
if (This->state != NINE_QUERY_STATE_RUNNING)
pipe->begin_query(pipe, This->pq);
pipe->end_query(pipe, This->pq);
This->state = NINE_QUERY_STATE_ENDED;
}

This seems like it would also take care of the Ended -> Ended
transition which is supposed to restart the query.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 3/4] nine: handle D3DISSUE_END without previous D3DISSUE_BEGIN

2014-11-21 Thread Ilia Mirkin
On Sat, Nov 22, 2014 at 12:57 AM, Ilia Mirkin  wrote:
> On Sat, Nov 22, 2014 at 12:44 AM, John Ettedgui  
> wrote:
>> On Fri Nov 21 2014 at 7:50:03 PM Ilia Mirkin  wrote:
>>>
>>> What's the actual problem?
>>
>> If you mean in-game, a blocking black screen, the game wants to display
>> something but is unable to and keeps trying pretty much forever.
>
> I meant at the API level. i.e. what is the API call sequence leading
> to the problem, what is being returned, and what ought to be getting
> returned.
>
>>>
>>> If there's no begin, then a new query would
>>> remain marked as FRESH.
>>
>> This is not my understanding of the Query on msdn:
>> http://msdn.microsoft.com/en-us/library/windows/desktop/bb147308%28v=vs.85%29.aspx
>> As you can see on the 1st diagram, we have either cases:
>> Signaled State moved to Building State on D3DISSUE_BEGIN and then to Issued
>> State on D3DISSUE_END (this part worked fine before).
>>
>> Then we have Signaled Sated moved to Issued State on D3DISSUE_END, that's
>> the part I believe was not working before this patch.
>
> These are hidden states, and query9.c does not implement them
> directly. What's the actual API call sequence, and what is being
> wrongly returned?
>
>>
>> Of course all of that is based on me thinking FRESH is nine's version of
>> Signaled.
>
> From my read of the code, FRESH is a newly constructed query, ENDED is
> when pipe->end_query was issued. All the state usage is entirely
> contained in query9.c -- take a look at it. However this is purely an
> implementation detail. At the end of the day, some API call is made
> and it's not returning the expected data. It's not clear from the
> commit log as to what you believe the issue is.
>
>>
>>>
>>> Should _GetData 0 out the result in that case
>>> perhaps instead of insta-returning?
>>>
>> You are correct that this is the exact reason why I made the change.
>> In my game, GetData kept getting called, and kept returning no data because
>> of the State the Queue was in (in its current implementation).
>>
>> What do you mean by "0 out the result" though?
>
> memset(pData, 0, dwSize) in GetData if This->state == FRESH.

Oh, I think i see the problem.

In GetData:

user_assert(This->state != NINE_QUERY_STATE_RUNNING, D3DERR_INVALIDCALL);

should be

  user_assert(This->state != ... || This->state |= FRESH, D3DERR_INVALIDCALL)

This will allow someone to call GetData without first having done an
Issue() but... meh. You could alternatively rejigger the state
variables, but that seems excessive.

Actually perhaps it's supposed to actually call ->end_query()
unconditionally in ->Issue even though it hasn't called
->begin_query()? That might make even more sense...

And then you'd remove the explicit FRESH handling in GetData since you
can't get there due to the user_assert.

  -ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 3/4] nine: handle D3DISSUE_END without previous D3DISSUE_BEGIN

2014-11-21 Thread Ilia Mirkin
On Sat, Nov 22, 2014 at 12:44 AM, John Ettedgui  wrote:
> On Fri Nov 21 2014 at 7:50:03 PM Ilia Mirkin  wrote:
>>
>> What's the actual problem?
>
> If you mean in-game, a blocking black screen, the game wants to display
> something but is unable to and keeps trying pretty much forever.

I meant at the API level. i.e. what is the API call sequence leading
to the problem, what is being returned, and what ought to be getting
returned.

>>
>> If there's no begin, then a new query would
>> remain marked as FRESH.
>
> This is not my understanding of the Query on msdn:
> http://msdn.microsoft.com/en-us/library/windows/desktop/bb147308%28v=vs.85%29.aspx
> As you can see on the 1st diagram, we have either cases:
> Signaled State moved to Building State on D3DISSUE_BEGIN and then to Issued
> State on D3DISSUE_END (this part worked fine before).
>
> Then we have Signaled Sated moved to Issued State on D3DISSUE_END, that's
> the part I believe was not working before this patch.

These are hidden states, and query9.c does not implement them
directly. What's the actual API call sequence, and what is being
wrongly returned?

>
> Of course all of that is based on me thinking FRESH is nine's version of
> Signaled.

From my read of the code, FRESH is a newly constructed query, ENDED is
when pipe->end_query was issued. All the state usage is entirely
contained in query9.c -- take a look at it. However this is purely an
implementation detail. At the end of the day, some API call is made
and it's not returning the expected data. It's not clear from the
commit log as to what you believe the issue is.

>
>>
>> Should _GetData 0 out the result in that case
>> perhaps instead of insta-returning?
>>
> You are correct that this is the exact reason why I made the change.
> In my game, GetData kept getting called, and kept returning no data because
> of the State the Queue was in (in its current implementation).
>
> What do you mean by "0 out the result" though?

memset(pData, 0, dwSize) in GetData if This->state == FRESH.

>
> John
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] nine: the .pc file should not follow mesa version

2014-11-21 Thread Emil Velikov
The version provided by it should be the same as the one
provided/handled by the module. Add the missing tiny version.

Cc: 
Signed-off-by: Emil Velikov 
---
 configure.ac| 2 ++
 src/gallium/targets/d3dadapter9/Makefile.am | 2 +-
 src/gallium/targets/d3dadapter9/d3d.pc.in   | 2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index 33cbf22..f99ea3d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2085,6 +2085,8 @@ AM_CONDITIONAL(HAVE_SPARC_ASM, test "x$asm_arch" = xsparc)
 
 AC_SUBST([NINE_MAJOR], 1)
 AC_SUBST([NINE_MINOR], 0)
+AC_SUBST([NINE_TINY], 0)
+AC_SUBST([NINE_VERSION], "$NINE_MAJOR.$NINE_MINOR.$NINE_TINY")
 
 AC_SUBST([VDPAU_MAJOR], 1)
 AC_SUBST([VDPAU_MINOR], 0)
diff --git a/src/gallium/targets/d3dadapter9/Makefile.am 
b/src/gallium/targets/d3dadapter9/Makefile.am
index 6231236..dc97931 100644
--- a/src/gallium/targets/d3dadapter9/Makefile.am
+++ b/src/gallium/targets/d3dadapter9/Makefile.am
@@ -62,7 +62,7 @@ d3dadapter9_la_LDFLAGS = \
-shrext .so \
-module \
-no-undefined \
-   -version-number $(NINE_MAJOR):$(NINE_MINOR) \
+   -version-number $(NINE_MAJOR):$(NINE_MINOR):$(NINE_TINY) \
$(GC_SECTIONS) \
$(LD_NO_UNDEFINED)
 
diff --git a/src/gallium/targets/d3dadapter9/d3d.pc.in 
b/src/gallium/targets/d3dadapter9/d3d.pc.in
index 9a18a5c..d34b2a7 100644
--- a/src/gallium/targets/d3dadapter9/d3d.pc.in
+++ b/src/gallium/targets/d3dadapter9/d3d.pc.in
@@ -6,6 +6,6 @@ moduledir=@D3D_DRIVER_INSTALL_DIR@
 
 Name: d3d
 Description: Native D3D driver modules
-Version: @VERSION@
+Version: @NINE_VERSION@
 Requires.private: @DRI_PC_REQ_PRIV@
 Cflags: -I${includedir}
-- 
2.1.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] Mesa 10.4.0 release candidate 2

2014-11-21 Thread Emil Velikov
Mesa 10.4.0 release candidate 2 is now available for testing. The
current plan is to have an additional release candidate each Friday
until the eventual 10.4.0 release on Friday, Dec 5th.

The tag in the git repository for Mesa 10.4.0-rc2 is 'mesa-10.4.0-rc2'.

Mesa 10.4.0 release candidate 2 is available for download from
ftp://freedesktop.org/pub/mesa/10.4.0/

sha256sums:

9249c534dd78e129b111091cd6271197029178566daae0f667fd86107503a4e2  
MesaLib-10.4.0-rc2.tar.gz
1b2346bfefe8b706de0dd1909ff8503c9ad25df7b96be7a0c787ab54633dbbda  
MesaLib-10.4.0-rc2.tar.bz2
bba0177187c23d9a6349b4442442af0e8d058450ea1a6e7efa68e0dd556f59d2  
MesaLib-10.4.0-rc2.zip


I have verified building from the .tar.bz2 file by doing:

tar xjf MesaLib-10.4.0-rc2.tar.bz2
cd Mesa-10.4.0-rc2
./configure --enable-gallium-llvm
make -j6
make -j6 install

-Emil

--

Changes from 10.4.0-rc1 to 10.4.0-rc2:

Brian Paul (1):
  st/mesa: copy sampler_array_size field when copying instructions

Chad Versace (1):
  i965: Fix segfault in WebGL Conformance on Ivybridge

Dave Airlie (5):
  r600g/cayman: fix integer multiplication output overwrite (v2)
  r600g/cayman: fix texture gather tests
  r600g/cayman: handle empty vertex shaders
  r600g: geom shaders: always load texture src regs from inputs
  r600g: limit texture offset application to specific types (v2)

Emil Velikov (2):
  configure.ac: roll up a program for the sse4.1 check
  Increment version to 10.4.0-rc2

Ilia Mirkin (2):
  nv50,nvc0: use clip_halfz setting when creating rasterizer state
  st/mesa: add a fallback for clear_with_quad when no vs_layer

Marek Olšák (1):
  radeonsi: support per-sample gl_FragCoord

Michel Dänzer (1):
  radeonsi: Disable asynchronous DMA except for PIPE_BUFFER

Vinson Lee (1):
  scons: Require glproto >= 1.4.13 for X11.





signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] Mesa 10.3.4

2014-11-21 Thread Emil Velikov
Mesa 10.3.4 has been released. Mesa 10.3.4 is a bug fix release
fixing bugs since the 10.3.3 release, (see below for a list of
changes).

The tag in the git repository for Mesa 10.3.4 is 'mesa-10.3.4'.

Mesa 10.3.4 is available for download at
ftp://freedesktop.org/pub/mesa/10.3.4/

SHA-256 checksums (can be verified with the sha256sum program):
26482495ef6177f889dbd87c7edcccfedd995598785bbbd7e3e066352574c8e0  
MesaLib-10.3.4.tar.gz
e6373913142338d10515daf619d659433bfd2989988198930c13b0945a15e98a  
MesaLib-10.3.4.tar.bz2
8c3ebbb6535daf3414305860ebca6ac67dbb6e3d35058c7a6ce18b84b5945b7f  
MesaLib-10.3.4.zip


I have verified building from the .tar.bz2 file by doing:

tar xjf MesaLib-10.3.4.tar.bz2
cd Mesa-10.3.4
./configure --enable-gallium-llvm
make -j6
make -j6 install

I have also verified that I pushed the tag.

-Emil

--

Changes from 10.3.3 to 10.3.4:

Brian Paul (1):
  st/mesa: copy sampler_array_size field when copying instructions

Chad Versace (1):
  i965: Fix segfault in WebGL Conformance on Ivybridge

Dave Airlie (5):
  r600g/cayman: fix integer multiplication output overwrite (v2)
  r600g/cayman: fix texture gather tests
  r600g/cayman: handle empty vertex shaders
  r600g: geom shaders: always load texture src regs from inputs
  r600g: limit texture offset application to specific types (v2)

Emil Velikov (5):
  docs: Add sha256 sums for the 10.3.3 release
  configure.ac: roll up a program for the sse4.1 check
  get-pick-list.sh: Require explicit "10.3" for nominating stable patches
  Update version to 10.3.4
  Add release notes for the 10.3.4 release

Ilia Mirkin (1):
  st/mesa: add a fallback for clear_with_quad when no vs_layer

José Fonseca (1):
  llvmpipe: Avoid deadlock when unloading opengl32.dll

Kenneth Graunke (1):
  i915g: we also have more than 0 viewports!

Michel Dänzer (1):
  radeonsi: Disable asynchronous DMA except for PIPE_BUFFER




signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 3/4] nine: handle D3DISSUE_END without previous D3DISSUE_BEGIN

2014-11-21 Thread Ilia Mirkin
On Fri, Nov 21, 2014 at 10:13 PM, David Heidelberg  wrote:
> From: John Ettedgui 
>
> This patch fixes black screen with games based on the Unreal Engine 3.
> It was tested that it fixed the issue in Tera Online, Borderlands 2 and 
> Homefront.

What's the actual problem? If there's no begin, then a new query would
remain marked as FRESH. Should _GetData 0 out the result in that case
perhaps instead of insta-returning?

>
> Cc: "10.4" 
> Reviewed-by: Axel Davy 
> Reviewed-by: David Heidelberg 
> Signed-off-by: John Ettedgui 
> ---
>  src/gallium/state_trackers/nine/query9.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/gallium/state_trackers/nine/query9.c 
> b/src/gallium/state_trackers/nine/query9.c
> index 86762d2..5006d9f 100644
> --- a/src/gallium/state_trackers/nine/query9.c
> +++ b/src/gallium/state_trackers/nine/query9.c
> @@ -188,8 +188,8 @@ NineQuery9_Issue( struct NineQuery9 *This,
>  } else {
>  if (This->state == NINE_QUERY_STATE_RUNNING) {
>  pipe->end_query(pipe, This->pq);
> -This->state = NINE_QUERY_STATE_ENDED;
> }
> +   This->state = NINE_QUERY_STATE_ENDED;
>  }
>  return D3D_OK;
>  }
> --
> 2.1.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 4/4] nine: call DBG() at more external entry points

2014-11-21 Thread David Heidelberg

Sorry, for now please without this patch.

This one needs " nine: Refactor Resource9 BaseTexture9 Surface9 and 
Texture9 initialization", which isn't part of batch.


Thanks, David.

On 11/22/2014 04:14 AM, David Heidelberg wrote:

From: Stanislaw Halik 

Cc: "10.4" 
Reviewed-by: David Heidelberg 
Reviewed-by: Axel Davy 
Signed-off-by: Stanislaw Halik 
---
  src/gallium/state_trackers/nine/basetexture9.c | 17 
  src/gallium/state_trackers/nine/cubetexture9.c | 17 
  src/gallium/state_trackers/nine/device9.c  | 46 ++
  src/gallium/state_trackers/nine/device9ex.c| 15 +++
  src/gallium/state_trackers/nine/iunknown.c |  2 +
  src/gallium/state_trackers/nine/pixelshader9.c |  4 ++
  src/gallium/state_trackers/nine/query9.c   |  7 
  src/gallium/state_trackers/nine/resource9.c| 11 ++
  src/gallium/state_trackers/nine/stateblock9.c  |  3 ++
  src/gallium/state_trackers/nine/surface9.c |  7 
  src/gallium/state_trackers/nine/swapchain9.c   |  5 +++
  src/gallium/state_trackers/nine/swapchain9ex.c |  4 ++
  src/gallium/state_trackers/nine/texture9.c |  5 +++
  .../state_trackers/nine/vertexdeclaration9.c   |  5 +++
  src/gallium/state_trackers/nine/vertexshader9.c|  3 ++
  src/gallium/state_trackers/nine/volume9.c  |  8 
  src/gallium/state_trackers/nine/volumetexture9.c   | 12 ++
  17 files changed, 171 insertions(+)

diff --git a/src/gallium/state_trackers/nine/basetexture9.c 
b/src/gallium/state_trackers/nine/basetexture9.c
index 7bf2f56..12da1e0 100644
--- a/src/gallium/state_trackers/nine/basetexture9.c
+++ b/src/gallium/state_trackers/nine/basetexture9.c
@@ -51,6 +51,9 @@ NineBaseTexture9_ctor( struct NineBaseTexture9 *This,
  (format != D3DFMT_NULL);
  HRESULT hr;
  
+DBG("This=%p, pParams=%p initResource=%p Type=%d format=%d Pool=%d Usage=%d\n",

+This, pParams, initResource, Type, format, Pool, Usage);
+
  user_assert(!(Usage & (D3DUSAGE_RENDERTARGET | D3DUSAGE_DEPTHSTENCIL)) ||
  Pool == D3DPOOL_DEFAULT, D3DERR_INVALIDCALL);
  user_assert(!(Usage & D3DUSAGE_DYNAMIC) ||
@@ -93,6 +96,8 @@ NineBaseTexture9_SetLOD( struct NineBaseTexture9 *This,
  {
  DWORD old = This->lod;
  
+DBG("This=%p LODNew=%d\n", This, LODNew);

+
  user_assert(This->base.pool == D3DPOOL_MANAGED, 0);
  
  This->lod = MIN2(LODNew, This->base.info.last_level);

@@ -106,12 +111,16 @@ NineBaseTexture9_SetLOD( struct NineBaseTexture9 *This,
  DWORD WINAPI
  NineBaseTexture9_GetLOD( struct NineBaseTexture9 *This )
  {
+DBG("This=%p\n", This);
+
  return This->lod;
  }
  
  DWORD WINAPI

  NineBaseTexture9_GetLevelCount( struct NineBaseTexture9 *This )
  {
+DBG("This=%p\n", This);
+
  if (This->base.usage & D3DUSAGE_AUTOGENMIPMAP)
  return 1;
  return This->base.info.last_level + 1;
@@ -121,6 +130,8 @@ HRESULT WINAPI
  NineBaseTexture9_SetAutoGenFilterType( struct NineBaseTexture9 *This,
 D3DTEXTUREFILTERTYPE FilterType )
  {
+DBG("This=%p FilterType=%d\n", This, FilterType);
+
  if (!(This->base.usage & D3DUSAGE_AUTOGENMIPMAP))
  return D3D_OK;
  user_assert(FilterType != D3DTEXF_NONE, D3DERR_INVALIDCALL);
@@ -133,6 +144,8 @@ NineBaseTexture9_SetAutoGenFilterType( struct 
NineBaseTexture9 *This,
  D3DTEXTUREFILTERTYPE WINAPI
  NineBaseTexture9_GetAutoGenFilterType( struct NineBaseTexture9 *This )
  {
+DBG("This=%p\n", This);
+
  return This->mipfilter;
  }
  
@@ -433,6 +446,8 @@ NineBaseTexture9_UpdateSamplerView( struct NineBaseTexture9 *This,

  struct pipe_sampler_view templ;
  uint8_t swizzle[4];
  
+DBG("This=%p sRGB=%d\n", This, sRGB);

+
  if (unlikely(!resource)) {
if (unlikely(This->format == D3DFMT_NULL))
  return D3D_OK;
@@ -485,6 +500,8 @@ NineBaseTexture9_UpdateSamplerView( struct NineBaseTexture9 
*This,
  void WINAPI
  NineBaseTexture9_PreLoad( struct NineBaseTexture9 *This )
  {
+DBG("This=%p\n", This);
+
  if (This->dirty && This->base.pool == D3DPOOL_MANAGED)
  NineBaseTexture9_UploadSelf(This);
  }
diff --git a/src/gallium/state_trackers/nine/cubetexture9.c 
b/src/gallium/state_trackers/nine/cubetexture9.c
index 5ef09f7..9f5d8e2 100644
--- a/src/gallium/state_trackers/nine/cubetexture9.c
+++ b/src/gallium/state_trackers/nine/cubetexture9.c
@@ -42,6 +42,11 @@ NineCubeTexture9_ctor( struct NineCubeTexture9 *This,
  D3DSURFACE_DESC sfdesc;
  HRESULT hr;
  
+DBG("This=%p pParams=%p EdgeLength=%u Levels=%u Usage=%d "

+"Format=%d Pool=%d pSharedHandle=%p\n",
+This, pParams, EdgeLength, Levels, Usage,
+Format, Pool, pSharedHandle);
+
  user_assert(!(Usage & D3DUSAGE_AUTOGENMIPMAP) ||
  (Pool != D3DPOOL_SYSTEMMEM && Levels <= 1), 
D3DERR_INVALIDCALL);
  
@@ -135,6 +140,8 @@ NineCubeTexture9_GetLevelDesc( struct 

[Mesa-dev] [PATCH 3/4] nine: handle D3DISSUE_END without previous D3DISSUE_BEGIN

2014-11-21 Thread David Heidelberg
From: John Ettedgui 

This patch fixes black screen with games based on the Unreal Engine 3.
It was tested that it fixed the issue in Tera Online, Borderlands 2 and 
Homefront.

Cc: "10.4" 
Reviewed-by: Axel Davy 
Reviewed-by: David Heidelberg 
Signed-off-by: John Ettedgui 
---
 src/gallium/state_trackers/nine/query9.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/state_trackers/nine/query9.c 
b/src/gallium/state_trackers/nine/query9.c
index 86762d2..5006d9f 100644
--- a/src/gallium/state_trackers/nine/query9.c
+++ b/src/gallium/state_trackers/nine/query9.c
@@ -188,8 +188,8 @@ NineQuery9_Issue( struct NineQuery9 *This,
 } else {
 if (This->state == NINE_QUERY_STATE_RUNNING) {
 pipe->end_query(pipe, This->pq);
-This->state = NINE_QUERY_STATE_ENDED;
}
+   This->state = NINE_QUERY_STATE_ENDED;
 }
 return D3D_OK;
 }
-- 
2.1.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 2/4] nine: Add pool check to SetTexture (v2)

2014-11-21 Thread David Heidelberg
From: Axel Davy 

D3DPOOL_SCRATCH is disallowed according to spec.
D3DPOOL_SYSTEMMEM should be allowed but we don't handle it right for now.

v2: Fixes segfault in SetTexture when unsetting the texture

Cc: "10.4" 
Tested-by: David Heidelberg 
Signed-off-by: Axel Davy 
---
 src/gallium/state_trackers/nine/device9.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/gallium/state_trackers/nine/device9.c 
b/src/gallium/state_trackers/nine/device9.c
index 5f95b42..db410a1 100644
--- a/src/gallium/state_trackers/nine/device9.c
+++ b/src/gallium/state_trackers/nine/device9.c
@@ -2172,6 +2172,7 @@ NineDevice9_SetTexture( struct NineDevice9 *This,
 IDirect3DBaseTexture9 *pTexture )
 {
 struct nine_state *state = This->update;
+struct NineBaseTexture9 *tex = NineBaseTexture9(pTexture);
 
 DBG("This=%p Stage=%u pTexture=%p\n", This, Stage, pTexture);
 
@@ -2179,12 +2180,19 @@ NineDevice9_SetTexture( struct NineDevice9 *This,
 Stage == D3DDMAPSAMPLER ||
 (Stage >= D3DVERTEXTEXTURESAMPLER0 &&
  Stage <= D3DVERTEXTEXTURESAMPLER3), D3DERR_INVALIDCALL);
+user_assert(!tex || tex->base.pool != D3DPOOL_SCRATCH, D3DERR_INVALIDCALL);
+
+if (unlikely(tex && tex->base.pool == D3DPOOL_SYSTEMMEM)) {
+/* TODO: Currently not implemented. Better return error
+ * with message telling what's wrong */
+ERR("This=%p D3DPOOL_SYSTEMMEM not implemented for SetTexture\n", 
This);
+user_assert(tex->base.pool != D3DPOOL_SYSTEMMEM, D3DERR_INVALIDCALL);
+}
 
 if (Stage >= D3DDMAPSAMPLER)
 Stage = Stage - D3DDMAPSAMPLER + NINE_MAX_SAMPLERS_PS;
 
 if (!This->is_recording) {
-struct NineBaseTexture9 *tex = NineBaseTexture9(pTexture);
 struct NineBaseTexture9 *old = state->texture[Stage];
 if (old == tex)
 return D3D_OK;
-- 
2.1.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 4/4] nine: call DBG() at more external entry points

2014-11-21 Thread David Heidelberg
From: Stanislaw Halik 

Cc: "10.4" 
Reviewed-by: David Heidelberg 
Reviewed-by: Axel Davy 
Signed-off-by: Stanislaw Halik 
---
 src/gallium/state_trackers/nine/basetexture9.c | 17 
 src/gallium/state_trackers/nine/cubetexture9.c | 17 
 src/gallium/state_trackers/nine/device9.c  | 46 ++
 src/gallium/state_trackers/nine/device9ex.c| 15 +++
 src/gallium/state_trackers/nine/iunknown.c |  2 +
 src/gallium/state_trackers/nine/pixelshader9.c |  4 ++
 src/gallium/state_trackers/nine/query9.c   |  7 
 src/gallium/state_trackers/nine/resource9.c| 11 ++
 src/gallium/state_trackers/nine/stateblock9.c  |  3 ++
 src/gallium/state_trackers/nine/surface9.c |  7 
 src/gallium/state_trackers/nine/swapchain9.c   |  5 +++
 src/gallium/state_trackers/nine/swapchain9ex.c |  4 ++
 src/gallium/state_trackers/nine/texture9.c |  5 +++
 .../state_trackers/nine/vertexdeclaration9.c   |  5 +++
 src/gallium/state_trackers/nine/vertexshader9.c|  3 ++
 src/gallium/state_trackers/nine/volume9.c  |  8 
 src/gallium/state_trackers/nine/volumetexture9.c   | 12 ++
 17 files changed, 171 insertions(+)

diff --git a/src/gallium/state_trackers/nine/basetexture9.c 
b/src/gallium/state_trackers/nine/basetexture9.c
index 7bf2f56..12da1e0 100644
--- a/src/gallium/state_trackers/nine/basetexture9.c
+++ b/src/gallium/state_trackers/nine/basetexture9.c
@@ -51,6 +51,9 @@ NineBaseTexture9_ctor( struct NineBaseTexture9 *This,
 (format != D3DFMT_NULL);
 HRESULT hr;
 
+DBG("This=%p, pParams=%p initResource=%p Type=%d format=%d Pool=%d 
Usage=%d\n",
+This, pParams, initResource, Type, format, Pool, Usage);
+
 user_assert(!(Usage & (D3DUSAGE_RENDERTARGET | D3DUSAGE_DEPTHSTENCIL)) ||
 Pool == D3DPOOL_DEFAULT, D3DERR_INVALIDCALL);
 user_assert(!(Usage & D3DUSAGE_DYNAMIC) ||
@@ -93,6 +96,8 @@ NineBaseTexture9_SetLOD( struct NineBaseTexture9 *This,
 {
 DWORD old = This->lod;
 
+DBG("This=%p LODNew=%d\n", This, LODNew);
+
 user_assert(This->base.pool == D3DPOOL_MANAGED, 0);
 
 This->lod = MIN2(LODNew, This->base.info.last_level);
@@ -106,12 +111,16 @@ NineBaseTexture9_SetLOD( struct NineBaseTexture9 *This,
 DWORD WINAPI
 NineBaseTexture9_GetLOD( struct NineBaseTexture9 *This )
 {
+DBG("This=%p\n", This);
+
 return This->lod;
 }
 
 DWORD WINAPI
 NineBaseTexture9_GetLevelCount( struct NineBaseTexture9 *This )
 {
+DBG("This=%p\n", This);
+
 if (This->base.usage & D3DUSAGE_AUTOGENMIPMAP)
 return 1;
 return This->base.info.last_level + 1;
@@ -121,6 +130,8 @@ HRESULT WINAPI
 NineBaseTexture9_SetAutoGenFilterType( struct NineBaseTexture9 *This,
D3DTEXTUREFILTERTYPE FilterType )
 {
+DBG("This=%p FilterType=%d\n", This, FilterType);
+
 if (!(This->base.usage & D3DUSAGE_AUTOGENMIPMAP))
 return D3D_OK;
 user_assert(FilterType != D3DTEXF_NONE, D3DERR_INVALIDCALL);
@@ -133,6 +144,8 @@ NineBaseTexture9_SetAutoGenFilterType( struct 
NineBaseTexture9 *This,
 D3DTEXTUREFILTERTYPE WINAPI
 NineBaseTexture9_GetAutoGenFilterType( struct NineBaseTexture9 *This )
 {
+DBG("This=%p\n", This);
+
 return This->mipfilter;
 }
 
@@ -433,6 +446,8 @@ NineBaseTexture9_UpdateSamplerView( struct NineBaseTexture9 
*This,
 struct pipe_sampler_view templ;
 uint8_t swizzle[4];
 
+DBG("This=%p sRGB=%d\n", This, sRGB);
+
 if (unlikely(!resource)) {
if (unlikely(This->format == D3DFMT_NULL))
 return D3D_OK;
@@ -485,6 +500,8 @@ NineBaseTexture9_UpdateSamplerView( struct NineBaseTexture9 
*This,
 void WINAPI
 NineBaseTexture9_PreLoad( struct NineBaseTexture9 *This )
 {
+DBG("This=%p\n", This);
+
 if (This->dirty && This->base.pool == D3DPOOL_MANAGED)
 NineBaseTexture9_UploadSelf(This);
 }
diff --git a/src/gallium/state_trackers/nine/cubetexture9.c 
b/src/gallium/state_trackers/nine/cubetexture9.c
index 5ef09f7..9f5d8e2 100644
--- a/src/gallium/state_trackers/nine/cubetexture9.c
+++ b/src/gallium/state_trackers/nine/cubetexture9.c
@@ -42,6 +42,11 @@ NineCubeTexture9_ctor( struct NineCubeTexture9 *This,
 D3DSURFACE_DESC sfdesc;
 HRESULT hr;
 
+DBG("This=%p pParams=%p EdgeLength=%u Levels=%u Usage=%d "
+"Format=%d Pool=%d pSharedHandle=%p\n",
+This, pParams, EdgeLength, Levels, Usage,
+Format, Pool, pSharedHandle);
+
 user_assert(!(Usage & D3DUSAGE_AUTOGENMIPMAP) ||
 (Pool != D3DPOOL_SYSTEMMEM && Levels <= 1), 
D3DERR_INVALIDCALL);
 
@@ -135,6 +140,8 @@ NineCubeTexture9_GetLevelDesc( struct NineCubeTexture9 
*This,
UINT Level,
D3DSURFACE_DESC *pDesc )
 {
+DBG("This=%p Level=%u pDesc=%p\n", This, Level, pDesc);
+
 user_assert(Level <= This->base.base.info.last_level, D3DERR_INVALIDCALL);
 user_assert(Level == 0 || !(This

[Mesa-dev] [PATCH 1/4] nine: propertly declare constants

2014-11-21 Thread David Heidelberg
From: Axel Davy 

Fixes "Error : CONST[20]: Undeclared source register" when running
dx9_alpha_blending_material. Also artifacts on ilo.

Cc: "10.4" 
Tested-by: David Heidelberg 
Signed-off-by: Axel Davy 
---
 src/gallium/state_trackers/nine/nine_ff.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/gallium/state_trackers/nine/nine_ff.c 
b/src/gallium/state_trackers/nine/nine_ff.c
index 6890933..f2dbae4 100644
--- a/src/gallium/state_trackers/nine/nine_ff.c
+++ b/src/gallium/state_trackers/nine/nine_ff.c
@@ -189,10 +189,10 @@ static void nine_ureg_tgsi_dump(struct ureg_program 
*ureg, boolean override)
 
 /* AL should contain base address of lights table. */
 #define LIGHT_CONST(i)\
-ureg_src_indirect(ureg_src_register(TGSI_FILE_CONSTANT, (i)), _X(AL))
+ureg_src_indirect(ureg_DECL_constant(ureg, i), _X(AL))
 
 #define MATERIAL_CONST(i) \
-ureg_src_register(TGSI_FILE_CONSTANT, 19 + (i))
+ureg_DECL_constant(ureg, 19 + (i))
 
 #define MISC_CONST(i) \
 ureg_src_register(TGSI_FILE_CONSTANT, (i))
-- 
2.1.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 4/5] i965/gen7: Don't allocate hiz miptree structure

2014-11-21 Thread Matt Turner
On Fri, Nov 21, 2014 at 3:09 PM, Jordan Justen
 wrote:
> We now skip allocating a hiz miptree for gen7. Instead, we calculate
> the required hiz buffer parameters and allocate a bo directly.
>
> v2:
>  * Update hz_height calculation as suggested by Topi
> v3:
>  * Bail if we failed to create the bo (Ben)
>
> Signed-off-by: Jordan Justen 
> Reviewed-by: Topi Pohjolainen 
> ---
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 102 
> +-
>  1 file changed, 100 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c 
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 8800867..e6ee8d7 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -851,7 +851,10 @@ intel_miptree_release(struct intel_mipmap_tree **mt)
>drm_intel_bo_unreference((*mt)->bo);
>intel_miptree_release(&(*mt)->stencil_mt);
>if ((*mt)->hiz_buf) {
> - intel_miptree_release(&(*mt)->hiz_buf->mt);
> + if ((*mt)->hiz_buf->mt)
> +intel_miptree_release(&(*mt)->hiz_buf->mt);
> + else
> +drm_intel_bo_unreference((*mt)->hiz_buf->bo);
>   free((*mt)->hiz_buf);
>}
>intel_miptree_release(&(*mt)->mcs_mt);
> @@ -1404,6 +1407,96 @@ intel_miptree_level_enable_hiz(struct brw_context *brw,
>  }
>
>
> +/**
> + * Helper for intel_miptree_alloc_hiz() that determines the required hiz
> + * buffer dimensions and allocates a bo for the hiz buffer.
> + */
> +static struct intel_miptree_aux_buffer *
> +intel_gen7_hiz_buf_create(struct brw_context *brw,
> +  struct intel_mipmap_tree *mt)
> +{
> +   unsigned z_width = mt->logical_width0;
> +   unsigned z_height = mt->logical_height0;
> +   const unsigned z_depth = mt->logical_depth0;
> +   unsigned hz_width, hz_height;
> +   struct intel_miptree_aux_buffer *buf = calloc(sizeof(*buf), 1);
> +
> +   if (!buf)
> +  return NULL;
> +
> +   /* Gen7 PRM Volume 2, Part 1, 11.5.3 "Hierarchical Depth Buffer" documents
> +* adjustments required for Z_Height and Z_Width based on multisampling.
> +*/
> +   switch (mt->num_samples) {
> +   case 0:
> +   case 1:
> +  break;
> +   case 2:
> +   case 4:
> +  z_width *= 2;
> +  z_height *= 2;
> +  break;
> +   case 8:
> +  z_width *= 4;
> +  z_height *= 2;
> +  break;
> +   default:
> +  assert(!"Unsupported sample count!");

unreachable here.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] radeonsi: use minnum and maxnum LLVM intrinsics for MIN and MAX opcodes

2014-11-21 Thread Michel Dänzer

On 21.11.2014 06:21, Marek Olšák wrote:

From: Marek Olšák 

So far it has been compiled into pretty ugly code (8 instructions or so
for either opcode).
---
  src/gallium/drivers/radeonsi/si_shader.c | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/src/gallium/drivers/radeonsi/si_shader.c 
b/src/gallium/drivers/radeonsi/si_shader.c
index ee08d1a..973bac2 100644
--- a/src/gallium/drivers/radeonsi/si_shader.c
+++ b/src/gallium/drivers/radeonsi/si_shader.c
@@ -2792,6 +2792,13 @@ int si_shader_create(struct si_screen *sscreen, struct 
si_shader *shader)
bld_base->op_actions[TGSI_OPCODE_EMIT].emit = si_llvm_emit_vertex;
bld_base->op_actions[TGSI_OPCODE_ENDPRIM].emit = si_llvm_emit_primitive;

+   if (HAVE_LLVM >= 0x0306) {
+   bld_base->op_actions[TGSI_OPCODE_MAX].emit = 
build_tgsi_intrinsic_nomem;
+   bld_base->op_actions[TGSI_OPCODE_MAX].intr_name = 
"llvm.maxnum.f32";
+   bld_base->op_actions[TGSI_OPCODE_MIN].emit = 
build_tgsi_intrinsic_nomem;
+   bld_base->op_actions[TGSI_OPCODE_MIN].intr_name = 
"llvm.minnum.f32";
+   }
+
si_shader_ctx.radeon_bld.load_system_value = declare_system_value;
si_shader_ctx.tokens = sel->tokens;
tgsi_parse_init(&si_shader_ctx.parse, si_shader_ctx.tokens);



Shouldn't this be done in 
src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c, so it benefits 
r600g as well?



--
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 4/5] i965/gen7: Don't allocate hiz miptree structure

2014-11-21 Thread Ben Widawsky
On Fri, Nov 21, 2014 at 03:09:02PM -0800, Jordan Justen wrote:
> We now skip allocating a hiz miptree for gen7. Instead, we calculate
> the required hiz buffer parameters and allocate a bo directly.
> 
> v2:
>  * Update hz_height calculation as suggested by Topi
> v3:
>  * Bail if we failed to create the bo (Ben)
> 
> Signed-off-by: Jordan Justen 
> Reviewed-by: Topi Pohjolainen 

I really don't want to review this one. I think some of the same things I said
in patch 5 apply here. This patch improves the current situation, so let's move
it forward.

Acked-by: Ben Widawsky 

> ---
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 102 
> +-
>  1 file changed, 100 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c 
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 8800867..e6ee8d7 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -851,7 +851,10 @@ intel_miptree_release(struct intel_mipmap_tree **mt)
>drm_intel_bo_unreference((*mt)->bo);
>intel_miptree_release(&(*mt)->stencil_mt);
>if ((*mt)->hiz_buf) {
> - intel_miptree_release(&(*mt)->hiz_buf->mt);
> + if ((*mt)->hiz_buf->mt)
> +intel_miptree_release(&(*mt)->hiz_buf->mt);
> + else
> +drm_intel_bo_unreference((*mt)->hiz_buf->bo);
>   free((*mt)->hiz_buf);
>}
>intel_miptree_release(&(*mt)->mcs_mt);
> @@ -1404,6 +1407,96 @@ intel_miptree_level_enable_hiz(struct brw_context *brw,
>  }
>  
>  
> +/**
> + * Helper for intel_miptree_alloc_hiz() that determines the required hiz
> + * buffer dimensions and allocates a bo for the hiz buffer.
> + */
> +static struct intel_miptree_aux_buffer *
> +intel_gen7_hiz_buf_create(struct brw_context *brw,
> +  struct intel_mipmap_tree *mt)
> +{
> +   unsigned z_width = mt->logical_width0;
> +   unsigned z_height = mt->logical_height0;
> +   const unsigned z_depth = mt->logical_depth0;
> +   unsigned hz_width, hz_height;
> +   struct intel_miptree_aux_buffer *buf = calloc(sizeof(*buf), 1);
> +
> +   if (!buf)
> +  return NULL;
> +
> +   /* Gen7 PRM Volume 2, Part 1, 11.5.3 "Hierarchical Depth Buffer" documents
> +* adjustments required for Z_Height and Z_Width based on multisampling.
> +*/
> +   switch (mt->num_samples) {
> +   case 0:
> +   case 1:
> +  break;
> +   case 2:
> +   case 4:
> +  z_width *= 2;
> +  z_height *= 2;
> +  break;
> +   case 8:
> +  z_width *= 4;
> +  z_height *= 2;
> +  break;
> +   default:
> +  assert(!"Unsupported sample count!");
> +   }
> +
> +   const unsigned vertical_align = 8; /* 'j' in the docs */
> +   const unsigned H0 = z_height;
> +   const unsigned h0 = ALIGN(H0, vertical_align);
> +   const unsigned h1 = ALIGN(minify(H0, 1), vertical_align);
> +   const unsigned Z0 = z_depth;
> +
> +   /* HZ_Width (bytes) = ceiling(Z_Width / 16) * 16 */
> +   hz_width = ALIGN(z_width, 16);
> +
> +   if (mt->target == GL_TEXTURE_3D) {
> +  unsigned H_i = H0;
> +  unsigned Z_i = Z0;
> +  hz_height = 0;
> +  for (int level = mt->first_level; level <= mt->last_level; ++level) {
> + unsigned h_i = ALIGN(H_i, vertical_align);
> + /* sum(i=0 to m; h_i * max(1, floor(Z_Depth/2**i))) */
> + hz_height += h_i * Z_i;
> + H_i = minify(H_i, 1);
> + Z_i = minify(Z_i, 1);
> +  }
> +  /* HZ_Height =
> +   *(1/2) * sum(i=0 to m; h_i * max(1, floor(Z_Depth/2**i)))
> +   */
> +  hz_height = CEILING(hz_height, 2);
> +   } else {
> +  const unsigned hz_qpitch = h0 + h1 + (12 * vertical_align);
> +  if (mt->target == GL_TEXTURE_CUBE_MAP_ARRAY ||
> +  mt->target == GL_TEXTURE_CUBE_MAP) {
> + /* HZ_Height (rows) = Ceiling ( ( Q_pitch * Z_depth * 6/2) /8 ) * 8 
> */
> + hz_height = CEILING(hz_qpitch * Z0 * 6, 2 * 8) * 8;
> +  } else {
> + /* HZ_Height (rows) = Ceiling ( ( Q_pitch * Z_depth/2) /8 ) * 8 */
> + hz_height = CEILING(hz_qpitch * Z0, 2 * 8) * 8;
> +  }
> +   }
> +
> +   unsigned long pitch;
> +   uint32_t tiling = I915_TILING_Y;
> +   buf->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "hiz",
> +  hz_width, hz_height, 1,
> +  &tiling, &pitch,
> +  BO_ALLOC_FOR_RENDER);
> +   if (!buf->bo) {
> +  free(buf);
> +  return NULL;
> +   }
> +
> +   buf->pitch = pitch;
> +
> +   return buf;
> +}
> +
> +
>  static struct intel_miptree_aux_buffer *
>  intel_hiz_miptree_buf_create(struct brw_context *brw,
>   struct intel_mipmap_tree *mt)
> @@ -1444,7 +1537,12 @@ intel_miptree_alloc_hiz(struct brw_context *brw,
>   struct intel_mipmap_tree *mt)
>  {
> assert(mt->hiz_buf == NULL);
> -   mt->hiz_buf = intel_hiz_miptree_buf_create(brw, 

Re: [Mesa-dev] [PATCH] glsl: Fix tautological comparison.

2014-11-21 Thread Kenneth Graunke
On Friday, November 21, 2014 03:19:12 PM Matt Turner wrote:
> Caught by clang.
> 
> warning: comparison of constant -1 with expression of type
>  'ir_texture_opcode' is always false
>   [-Wtautological-constant-out-of-range-compare]
>   if (op == -1)
>   ~~ ^  ~~
> ---
>  src/glsl/ir_reader.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/glsl/ir_reader.cpp b/src/glsl/ir_reader.cpp
> index ae00e79..fd318c0 100644
> --- a/src/glsl/ir_reader.cpp
> +++ b/src/glsl/ir_reader.cpp
> @@ -972,7 +972,7 @@ ir_reader::read_texture(s_expression *expr)
>op = ir_query_levels;
> } else if (MATCH(expr, other_pattern)) {
>op = ir_texture::get_opcode(tag->value());
> -  if (op == -1)
> +  if (op == (ir_texture_opcode) -1)
>return NULL;
> } else {
>ir_read_error(NULL, "unexpected texture pattern %s", tag->value());
> 

Reviewed-by: Kenneth Graunke 

signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] i965/gen6/gs: Don't declare a src_reg with struct.

2014-11-21 Thread Kenneth Graunke
On Friday, November 21, 2014 03:19:32 PM Matt Turner wrote:
> ---
>  src/mesa/drivers/dri/i965/gen6_gs_visitor.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/gen6_gs_visitor.cpp 
> b/src/mesa/drivers/dri/i965/gen6_gs_visitor.cpp
> index d16cc6e..564b4cb 100644
> --- a/src/mesa/drivers/dri/i965/gen6_gs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/gen6_gs_visitor.cpp
> @@ -632,7 +632,7 @@ gen6_gs_visitor::xfb_write()
> emit(CMP(dst_null_d(), sol_temp, this->max_svbi, BRW_CONDITIONAL_LE));
> emit(IF(BRW_PREDICATE_NORMAL));
> {
> -  struct src_reg destination_indices_uw =
> +  src_reg destination_indices_uw =
>   retype(destination_indices, BRW_REGISTER_TYPE_UW);
>  
>vec4_instruction *inst = emit(MOV(dst_reg(destination_indices_uw),
> 

Reviewed-by: Kenneth Graunke 

signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 5/5] i965/gen8: Don't allocate hiz miptree structure

2014-11-21 Thread Matt Turner
On Fri, Nov 21, 2014 at 3:09 PM, Jordan Justen
 wrote:
> We now skip allocating a hiz miptree for gen8. Instead, we calculate
> the required hiz buffer parameters and allocate a bo directly.
>
> v2:
>  * Update hz_height calculation as suggested by Topi
> v3:
>  * Bail if we failed to create the bo (Ben)
>
> Signed-off-by: Jordan Justen 
> Reviewed-by: Topi Pohjolainen 
> ---
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 100 
> ++
>  1 file changed, 100 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c 
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index e6ee8d7..c62b0b8 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -1497,6 +1497,104 @@ intel_gen7_hiz_buf_create(struct brw_context *brw,
>  }
>
>
> +/**
> + * Helper for intel_miptree_alloc_hiz() that determines the required hiz
> + * buffer dimensions and allocates a bo for the hiz buffer.
> + */
> +static struct intel_miptree_aux_buffer *
> +intel_gen8_hiz_buf_create(struct brw_context *brw,
> +  struct intel_mipmap_tree *mt)
> +{
> +   unsigned z_width = mt->logical_width0;
> +   unsigned z_height = mt->logical_height0;
> +   const unsigned z_depth = mt->logical_depth0;
> +   unsigned hz_width, hz_height;
> +   struct intel_miptree_aux_buffer *buf = calloc(sizeof(*buf), 1);
> +
> +   if (!buf)
> +  return NULL;
> +
> +   /* Gen7 PRM Volume 2, Part 1, 11.5.3 "Hierarchical Depth Buffer" documents
> +* adjustments required for Z_Height and Z_Width based on multisampling.
> +*/
> +   switch (mt->num_samples) {
> +   case 0:
> +   case 1:
> +  break;
> +   case 2:
> +   case 4:
> +  z_width *= 2;
> +  z_height *= 2;
> +  break;
> +   case 8:
> +  z_width *= 4;
> +  z_height *= 2;
> +  break;
> +   default:
> +  assert(!"Unsupported sample count!");

Use unreachable().
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/3] nine: Don't use the otherwise-dead SFL opcode in an unreachable path.

2014-11-21 Thread David Heidelberg

Reviewed-by: David Heidelberg 

for the series.

David
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] util: Implement assume() for clang.

2014-11-21 Thread Matt Turner
On Fri, Nov 21, 2014 at 4:23 PM, Jordan Justen
 wrote:
> So ...  assert(expr) instead?

Yes. :(
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] util: Implement assume() for clang.

2014-11-21 Thread Jordan Justen
On 2014-11-21 15:53:35, Matt Turner wrote:
> On Fri, Nov 21, 2014 at 3:47 PM, Jordan Justen
>  wrote:
> > On 2014-11-21 15:17:00, Matt Turner wrote:
> >> ---
> >>  src/util/macros.h | 12 +++-
> >>  1 file changed, 11 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/src/util/macros.h b/src/util/macros.h
> >> index da5daff..b67596d 100644
> >> --- a/src/util/macros.h
> >> +++ b/src/util/macros.h
> >> @@ -29,6 +29,10 @@
> >>  #  define ARRAY_SIZE(x) (sizeof(x) / sizeof(*(x)))
> >>  #endif
> >>
> >> +/* For compatibility with Clang's __has_builtin() */
> >> +#ifndef __has_builtin
> >> +#  define __has_builtin(x) 0
> >> +#endif
> >>
> >>  /**
> >>   * __builtin_expect macros
> >> @@ -85,7 +89,13 @@ do {\
> >>   * Assume macro. Useful for expressing our assumptions to the compiler,
> >>   * typically for purposes of silencing warnings.
> >>   */
> >> -#ifdef HAVE___BUILTIN_UNREACHABLE
> >> +#if __has_builtin(__builtin_assume)
> >> +#define assume(expr) \
> >> +do { \
> >> +   assert(!"assumption failed"); \
> >
> > Did you mean to remove this assert?
> >
> > With that removed
> > Reviewed-by: Jordan Justen 
> 
> No. Clang's documentation says that if the expression you're assuming
> is false, behavior is undefined.

Worth noting in the commit message or a comment?

> The assertion is to help us catch
> cases where that happens.

So ...  assert(expr) instead?

-Jordan
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 5/5] i965/gen8: Don't allocate hiz miptree structure

2014-11-21 Thread Ben Widawsky
On Fri, Nov 21, 2014 at 03:09:03PM -0800, Jordan Justen wrote:
> We now skip allocating a hiz miptree for gen8. Instead, we calculate
> the required hiz buffer parameters and allocate a bo directly.
> 
> v2:
>  * Update hz_height calculation as suggested by Topi
> v3:
>  * Bail if we failed to create the bo (Ben)
> 
> Signed-off-by: Jordan Justen 
> Reviewed-by: Topi Pohjolainen 

Add the bugzilla

> ---
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 100 
> ++
>  1 file changed, 100 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c 
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index e6ee8d7..c62b0b8 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -1497,6 +1497,104 @@ intel_gen7_hiz_buf_create(struct brw_context *brw,
>  }
>  
>  
> +/**
> + * Helper for intel_miptree_alloc_hiz() that determines the required hiz
> + * buffer dimensions and allocates a bo for the hiz buffer.
> + */
> +static struct intel_miptree_aux_buffer *
> +intel_gen8_hiz_buf_create(struct brw_context *brw,
> +  struct intel_mipmap_tree *mt)
> +{
> +   unsigned z_width = mt->logical_width0;
> +   unsigned z_height = mt->logical_height0;
> +   const unsigned z_depth = mt->logical_depth0;
> +   unsigned hz_width, hz_height;
> +   struct intel_miptree_aux_buffer *buf = calloc(sizeof(*buf), 1);
> +
> +   if (!buf)
> +  return NULL;
> +
> +   /* Gen7 PRM Volume 2, Part 1, 11.5.3 "Hierarchical Depth Buffer" documents
> +* adjustments required for Z_Height and Z_Width based on multisampling.
> +*/
> +   switch (mt->num_samples) {
> +   case 0:
> +   case 1:
> +  break;
> +   case 2:
> +   case 4:
> +  z_width *= 2;
> +  z_height *= 2;
> +  break;
> +   case 8:
> +  z_width *= 4;
> +  z_height *= 2;
> +  break;

I think that you need:
case 16:
z_height *= 4;
z_width *= 4; /* Not 100% certain about this one */

I'd really like if you put a 0 in the variable names, but I can live without it.

> +   default:
> +  assert(!"Unsupported sample count!");
> +   }
> +
> +   const unsigned vertical_align = 8; /* 'j' in the docs */
> +   const unsigned H0 = z_height;
> +   const unsigned h0 = ALIGN(H0, vertical_align);
> +   const unsigned h1 = ALIGN(minify(H0, 1), vertical_align);
> +   const unsigned Z0 = z_depth;
> +
> +   /* HZ_Width (bytes) = ceiling(Z_Width / 16) * 16 */
> +   hz_width = ALIGN(z_width, 16);
> +
> +   unsigned H_i = H0;
> +   unsigned Z_i = Z0;
> +   unsigned sum_h_i = 0;
> +   unsigned hz_height_3d_sum = 0;
> +   for (int level = mt->first_level; level <= mt->last_level; ++level) {
> +  unsigned i = level - mt->first_level;
> +  unsigned h_i = ALIGN(H_i, vertical_align);
> +  /* sum(i=2 to m; h_i) */
> +  if (i >= 2) {
> + sum_h_i += h_i;
> +  }
> +  /* sum(i=0 to m; h_i * max(1, floor(Z_Depth/2**i))) */
> +  hz_height_3d_sum += h_i * Z_i;

Is it possible for Z_i to be zero on the first iteration? If so, you need to do
something.

> +  H_i = minify(H_i, 1);
> +  Z_i = minify(Z_i, 1);
> +   }
> +   /* HZ_QPitch = h0 + max(h1, sum(i=2 to m; h_i)) */
> +   buf->qpitch = h0 + MAX2(h1, sum_h_i);

Don't ask me why, but I think you can optimize:
if (level < 2)
buf->qpitch = h0/2.

> +
> +   if (mt->target == GL_TEXTURE_3D) {
> +  /* (1/2) * sum(i=0 to m; h_i * max(1, floor(Z_Depth/2**i))) */
> +  hz_height = CEILING(hz_height_3d_sum, 2);
> +   } else {
> +  /* HZ_Height (rows) = ceiling( (HZ_QPitch/2)/8) *8 * Z_Depth */
> +  hz_height = CEILING(buf->qpitch, 2 * 8) * 8 * Z0;
> +  if (mt->target == GL_TEXTURE_CUBE_MAP_ARRAY ||
> +  mt->target == GL_TEXTURE_CUBE_MAP) {
> + /* HZ_Height (rows) = ceiling( (HZ_QPitch/2)/8) *8 * 6
> +  *
> +  * Assumption: the doc meant to multiply by Z_Depth as well.
> +  */
> + hz_height *= 6;

I don't think this assumption is correct. Isn't the depth just always 6 in a
cube map?

> +  }
> +   }
> +
> +   unsigned long pitch;
> +   uint32_t tiling = I915_TILING_Y;
> +   buf->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "hiz",
> +  hz_width, hz_height, 1,
> +  &tiling, &pitch,
> +  BO_ALLOC_FOR_RENDER);
> +   if (!buf->bo) {
> +  free(buf);
> +  return NULL;
> +   }
> +
> +   buf->pitch = pitch;
> +
> +   return buf;
> +}

I'm fine with you not handling the potential tiling coming back different, but
put a warn or something at least.

> +
> +
>  static struct intel_miptree_aux_buffer *
>  intel_hiz_miptree_buf_create(struct brw_context *brw,
>   struct intel_mipmap_tree *mt)
> @@ -1540,6 +1638,8 @@ intel_miptree_alloc_hiz(struct brw_context *brw,
>  
> if (brw->gen == 7) {
>mt->hiz_buf = intel_gen7_hiz_buf_create(brw, mt);
> 

Re: [Mesa-dev] [PATCH] util: Implement assume() for clang.

2014-11-21 Thread Matt Turner
On Fri, Nov 21, 2014 at 3:47 PM, Jordan Justen
 wrote:
> On 2014-11-21 15:17:00, Matt Turner wrote:
>> ---
>>  src/util/macros.h | 12 +++-
>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/util/macros.h b/src/util/macros.h
>> index da5daff..b67596d 100644
>> --- a/src/util/macros.h
>> +++ b/src/util/macros.h
>> @@ -29,6 +29,10 @@
>>  #  define ARRAY_SIZE(x) (sizeof(x) / sizeof(*(x)))
>>  #endif
>>
>> +/* For compatibility with Clang's __has_builtin() */
>> +#ifndef __has_builtin
>> +#  define __has_builtin(x) 0
>> +#endif
>>
>>  /**
>>   * __builtin_expect macros
>> @@ -85,7 +89,13 @@ do {\
>>   * Assume macro. Useful for expressing our assumptions to the compiler,
>>   * typically for purposes of silencing warnings.
>>   */
>> -#ifdef HAVE___BUILTIN_UNREACHABLE
>> +#if __has_builtin(__builtin_assume)
>> +#define assume(expr) \
>> +do { \
>> +   assert(!"assumption failed"); \
>
> Did you mean to remove this assert?
>
> With that removed
> Reviewed-by: Jordan Justen 

No. Clang's documentation says that if the expression you're assuming
is false, behavior is undefined. The assertion is to help us catch
cases where that happens.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] util: Implement assume() for clang.

2014-11-21 Thread Jordan Justen
On 2014-11-21 15:17:00, Matt Turner wrote:
> ---
>  src/util/macros.h | 12 +++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/src/util/macros.h b/src/util/macros.h
> index da5daff..b67596d 100644
> --- a/src/util/macros.h
> +++ b/src/util/macros.h
> @@ -29,6 +29,10 @@
>  #  define ARRAY_SIZE(x) (sizeof(x) / sizeof(*(x)))
>  #endif
>  
> +/* For compatibility with Clang's __has_builtin() */
> +#ifndef __has_builtin
> +#  define __has_builtin(x) 0
> +#endif
>  
>  /**
>   * __builtin_expect macros
> @@ -85,7 +89,13 @@ do {\
>   * Assume macro. Useful for expressing our assumptions to the compiler,
>   * typically for purposes of silencing warnings.
>   */
> -#ifdef HAVE___BUILTIN_UNREACHABLE
> +#if __has_builtin(__builtin_assume)
> +#define assume(expr) \
> +do { \
> +   assert(!"assumption failed"); \

Did you mean to remove this assert?

With that removed
Reviewed-by: Jordan Justen 

> +   __builtin_assume(expr);   \
> +} while (0)
> +#elif defined HAVE___BUILTIN_UNREACHABLE
>  #define assume(expr) ((expr) ? ((void) 0) \
>   : (assert(!"assumption failed"), \
>  __builtin_unreachable()))
> -- 
> 2.0.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] [PATCH] i965/disasm: Fix all32h/any32h predicate disassembly.

2014-11-21 Thread Chris Forbes
Reviewed-by: Chris Forbes 

On Sat, Nov 22, 2014 at 12:19 PM, Matt Turner  wrote:
> ---
>  src/mesa/drivers/dri/i965/brw_disasm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_disasm.c 
> b/src/mesa/drivers/dri/i965/brw_disasm.c
> index b211a0f..e235fd4 100644
> --- a/src/mesa/drivers/dri/i965/brw_disasm.c
> +++ b/src/mesa/drivers/dri/i965/brw_disasm.c
> @@ -275,7 +275,7 @@ static const char *const pred_ctrl_align1[16] = {
> [BRW_PREDICATE_ALIGN1_ANY16H] = ".any16h",
> [BRW_PREDICATE_ALIGN1_ALL16H] = ".all16h",
> [BRW_PREDICATE_ALIGN1_ANY32H] = ".any32h",
> -   [BRW_PREDICATE_ALIGN1_ANY32H] = ".all32h",
> +   [BRW_PREDICATE_ALIGN1_ALL32H] = ".all32h",
>  };
>
>  static const char *const thread_ctrl[4] = {
> --
> 2.0.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] [PATCH] util: Prefer atomic intrinsics to inline assembly.

2014-11-21 Thread Matt Turner
On Fri, Nov 21, 2014 at 3:18 PM, Matt Turner  wrote:
> Cuts a little more than 1k of .text size from i915g.
>
> This was previously done in commit 5f66b340 and subsequently reverted in
> commit 3661f757 after bug 30514 was filed. I believe the cause of bug
> 30514 wasn't anything related to cross compiling, but rather that the
> toolchain used defaulted to -march=i386, and i386 doesn't have the
> CMPXCHG or XADD instructions used to implement the intrinsics.
>
> So we reverted a patch that improved things so that we didn't break
> compilation for a platform that never could have worked anyway.
> ---
> I wouldn't mind some kind of check that this hardware support is available
> but I don't know how to do it. gcc -m32 defines __i386__, which doesn't
> tell you that you're compiling with -march=i386, but rather that you're
> compiling with -m32. Using -march=i486 defines both __i386__ and __i486__.
>
>  src/gallium/auxiliary/util/u_atomic.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/auxiliary/util/u_atomic.h 
> b/src/gallium/auxiliary/util/u_atomic.h
> index 9731aa0..b01aa38 100644
> --- a/src/gallium/auxiliary/util/u_atomic.h
> +++ b/src/gallium/auxiliary/util/u_atomic.h
> @@ -23,13 +23,13 @@
>  #elif defined(PIPE_CC_MSVC)
>  #define PIPE_ATOMIC_MSVC_INTRINSIC
>  #elif (defined(PIPE_CC_MSVC) && defined(PIPE_ARCH_X86))
> +#elif defined(PIPE_CC_GCC) && (PIPE_CC_GCC_VERSION >= 401)
> +#define PIPE_ATOMIC_GCC_INTRINSIC

Sending patches too quickly... these two lines obviously need to be
moved after the following one.

>  #define PIPE_ATOMIC_ASM_MSVC_X86
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] glsl: Silence clang warning about class vs no class.

2014-11-21 Thread Matt Turner
On Fri, Nov 21, 2014 at 3:19 PM, Matt Turner  wrote:
> note: hidden overloaded virtual function 
> 'ir_hierarchical_visitor::visit_enter'
>   declared here: type mismatch at 1st parameter
>   ('class ir_loop *' vs 'ir_dereference_variable *')
>virtual ir_visitor_status visit_enter(class ir_loop *);
>
> I don't see much value in this warning, but it spits out so many of them
> that finding useful ones is difficult.
> ---

On second though, I might have misinterpreted the warning. Ignore this patch.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] util: Prefer atomic intrinsics to inline assembly.

2014-11-21 Thread Ilia Mirkin
On Fri, Nov 21, 2014 at 6:18 PM, Matt Turner  wrote:
> Cuts a little more than 1k of .text size from i915g.
>
> This was previously done in commit 5f66b340 and subsequently reverted in
> commit 3661f757 after bug 30514 was filed. I believe the cause of bug
> 30514 wasn't anything related to cross compiling, but rather that the
> toolchain used defaulted to -march=i386, and i386 doesn't have the
> CMPXCHG or XADD instructions used to implement the intrinsics.
>
> So we reverted a patch that improved things so that we didn't break
> compilation for a platform that never could have worked anyway.
> ---
> I wouldn't mind some kind of check that this hardware support is available
> but I don't know how to do it. gcc -m32 defines __i386__, which doesn't
> tell you that you're compiling with -march=i386, but rather that you're
> compiling with -m32. Using -march=i486 defines both __i386__ and __i486__.

FWIW linux kernel dropped support for i386 a while back (3.0 or so?)
so I'd just ignore those bugs if I were you...

  -ilia

>
>  src/gallium/auxiliary/util/u_atomic.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/auxiliary/util/u_atomic.h 
> b/src/gallium/auxiliary/util/u_atomic.h
> index 9731aa0..b01aa38 100644
> --- a/src/gallium/auxiliary/util/u_atomic.h
> +++ b/src/gallium/auxiliary/util/u_atomic.h
> @@ -23,13 +23,13 @@
>  #elif defined(PIPE_CC_MSVC)
>  #define PIPE_ATOMIC_MSVC_INTRINSIC
>  #elif (defined(PIPE_CC_MSVC) && defined(PIPE_ARCH_X86))
> +#elif defined(PIPE_CC_GCC) && (PIPE_CC_GCC_VERSION >= 401)
> +#define PIPE_ATOMIC_GCC_INTRINSIC
>  #define PIPE_ATOMIC_ASM_MSVC_X86
>  #elif (defined(PIPE_CC_GCC) && defined(PIPE_ARCH_X86))
>  #define PIPE_ATOMIC_ASM_GCC_X86
>  #elif (defined(PIPE_CC_GCC) && defined(PIPE_ARCH_X86_64))
>  #define PIPE_ATOMIC_ASM_GCC_X86_64
> -#elif defined(PIPE_CC_GCC) && (PIPE_CC_GCC_VERSION >= 401)
> -#define PIPE_ATOMIC_GCC_INTRINSIC
>  #else
>  #error "Unsupported platform"
>  #endif
> --
> 2.0.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


[Mesa-dev] [PATCH] i965/disasm: Fix all32h/any32h predicate disassembly.

2014-11-21 Thread Matt Turner
---
 src/mesa/drivers/dri/i965/brw_disasm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/brw_disasm.c 
b/src/mesa/drivers/dri/i965/brw_disasm.c
index b211a0f..e235fd4 100644
--- a/src/mesa/drivers/dri/i965/brw_disasm.c
+++ b/src/mesa/drivers/dri/i965/brw_disasm.c
@@ -275,7 +275,7 @@ static const char *const pred_ctrl_align1[16] = {
[BRW_PREDICATE_ALIGN1_ANY16H] = ".any16h",
[BRW_PREDICATE_ALIGN1_ALL16H] = ".all16h",
[BRW_PREDICATE_ALIGN1_ANY32H] = ".any32h",
-   [BRW_PREDICATE_ALIGN1_ANY32H] = ".all32h",
+   [BRW_PREDICATE_ALIGN1_ALL32H] = ".all32h",
 };
 
 static const char *const thread_ctrl[4] = {
-- 
2.0.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] i965/gen6/gs: Don't declare a src_reg with struct.

2014-11-21 Thread Matt Turner
---
 src/mesa/drivers/dri/i965/gen6_gs_visitor.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/gen6_gs_visitor.cpp 
b/src/mesa/drivers/dri/i965/gen6_gs_visitor.cpp
index d16cc6e..564b4cb 100644
--- a/src/mesa/drivers/dri/i965/gen6_gs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/gen6_gs_visitor.cpp
@@ -632,7 +632,7 @@ gen6_gs_visitor::xfb_write()
emit(CMP(dst_null_d(), sol_temp, this->max_svbi, BRW_CONDITIONAL_LE));
emit(IF(BRW_PREDICATE_NORMAL));
{
-  struct src_reg destination_indices_uw =
+  src_reg destination_indices_uw =
  retype(destination_indices, BRW_REGISTER_TYPE_UW);
 
   vec4_instruction *inst = emit(MOV(dst_reg(destination_indices_uw),
-- 
2.0.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] glsl: Fix tautological comparison.

2014-11-21 Thread Matt Turner
Caught by clang.

warning: comparison of constant -1 with expression of type
 'ir_texture_opcode' is always false
  [-Wtautological-constant-out-of-range-compare]
  if (op == -1)
  ~~ ^  ~~
---
 src/glsl/ir_reader.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/glsl/ir_reader.cpp b/src/glsl/ir_reader.cpp
index ae00e79..fd318c0 100644
--- a/src/glsl/ir_reader.cpp
+++ b/src/glsl/ir_reader.cpp
@@ -972,7 +972,7 @@ ir_reader::read_texture(s_expression *expr)
   op = ir_query_levels;
} else if (MATCH(expr, other_pattern)) {
   op = ir_texture::get_opcode(tag->value());
-  if (op == -1)
+  if (op == (ir_texture_opcode) -1)
 return NULL;
} else {
   ir_read_error(NULL, "unexpected texture pattern %s", tag->value());
-- 
2.0.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] glsl: Silence clang warning about class vs no class.

2014-11-21 Thread Matt Turner
note: hidden overloaded virtual function 'ir_hierarchical_visitor::visit_enter'
  declared here: type mismatch at 1st parameter
  ('class ir_loop *' vs 'ir_dereference_variable *')
   virtual ir_visitor_status visit_enter(class ir_loop *);

I don't see much value in this warning, but it spits out so many of them
that finding useful ones is difficult.
---
 src/glsl/ir_hierarchical_visitor.cpp   | 10 +++
 src/glsl/ir_print_visitor.cpp  | 40 +-
 src/glsl/ir_set_program_inouts.cpp |  4 +--
 src/glsl/ir_validate.cpp   |  8 +++---
 src/glsl/ir_variable_refcount.cpp  |  4 +--
 src/glsl/link_uniform_block_active_visitor.cpp |  4 +--
 src/glsl/loop_analysis.cpp | 10 +++
 src/glsl/lower_clip_distance.cpp   |  4 +--
 src/glsl/lower_output_reads.cpp|  2 +-
 src/glsl/lower_vertex_id.cpp   |  4 +--
 src/glsl/opt_array_splitting.cpp   |  8 +++---
 src/glsl/opt_constant_variable.cpp |  4 +--
 src/glsl/opt_copy_propagation.cpp  |  2 +-
 src/glsl/opt_cse.cpp   |  4 +--
 src/glsl/opt_structure_splitting.cpp   |  8 +++---
 15 files changed, 58 insertions(+), 58 deletions(-)

diff --git a/src/glsl/ir_hierarchical_visitor.cpp 
b/src/glsl/ir_hierarchical_visitor.cpp
index adb6294..b2ae8a2 100644
--- a/src/glsl/ir_hierarchical_visitor.cpp
+++ b/src/glsl/ir_hierarchical_visitor.cpp
@@ -35,7 +35,7 @@ ir_hierarchical_visitor::ir_hierarchical_visitor()
 }
 
 ir_visitor_status
-ir_hierarchical_visitor::visit(ir_rvalue *ir)
+ir_hierarchical_visitor::visit(class ir_rvalue *ir)
 {
if (this->callback_enter != NULL)
   this->callback_enter(ir, this->data_enter);
@@ -44,7 +44,7 @@ ir_hierarchical_visitor::visit(ir_rvalue *ir)
 }
 
 ir_visitor_status
-ir_hierarchical_visitor::visit(ir_variable *ir)
+ir_hierarchical_visitor::visit(class ir_variable *ir)
 {
if (this->callback_enter != NULL)
   this->callback_enter(ir, this->data_enter);
@@ -53,7 +53,7 @@ ir_hierarchical_visitor::visit(ir_variable *ir)
 }
 
 ir_visitor_status
-ir_hierarchical_visitor::visit(ir_constant *ir)
+ir_hierarchical_visitor::visit(class ir_constant *ir)
 {
if (this->callback_enter != NULL)
   this->callback_enter(ir, this->data_enter);
@@ -62,7 +62,7 @@ ir_hierarchical_visitor::visit(ir_constant *ir)
 }
 
 ir_visitor_status
-ir_hierarchical_visitor::visit(ir_loop_jump *ir)
+ir_hierarchical_visitor::visit(class ir_loop_jump *ir)
 {
if (this->callback_enter != NULL)
   this->callback_enter(ir, this->data_enter);
@@ -71,7 +71,7 @@ ir_hierarchical_visitor::visit(ir_loop_jump *ir)
 }
 
 ir_visitor_status
-ir_hierarchical_visitor::visit(ir_dereference_variable *ir)
+ir_hierarchical_visitor::visit(class ir_dereference_variable *ir)
 {
if (this->callback_enter != NULL)
   this->callback_enter(ir, this->data_enter);
diff --git a/src/glsl/ir_print_visitor.cpp b/src/glsl/ir_print_visitor.cpp
index bd39805..b120713 100644
--- a/src/glsl/ir_print_visitor.cpp
+++ b/src/glsl/ir_print_visitor.cpp
@@ -152,12 +152,12 @@ print_type(FILE *f, const glsl_type *t)
}
 }
 
-void ir_print_visitor::visit(ir_rvalue *)
+void ir_print_visitor::visit(class ir_rvalue *)
 {
fprintf(f, "error");
 }
 
-void ir_print_visitor::visit(ir_variable *ir)
+void ir_print_visitor::visit(class ir_variable *ir)
 {
fprintf(f, "(declare ");
 
@@ -182,7 +182,7 @@ void ir_print_visitor::visit(ir_variable *ir)
 }
 
 
-void ir_print_visitor::visit(ir_function_signature *ir)
+void ir_print_visitor::visit(class ir_function_signature *ir)
 {
_mesa_symbol_table_push_scope(symbols);
fprintf(f, "(signature ");
@@ -223,7 +223,7 @@ void ir_print_visitor::visit(ir_function_signature *ir)
 }
 
 
-void ir_print_visitor::visit(ir_function *ir)
+void ir_print_visitor::visit(class ir_function *ir)
 {
fprintf(f, "(function %s\n", ir->name);
indentation++;
@@ -238,7 +238,7 @@ void ir_print_visitor::visit(ir_function *ir)
 }
 
 
-void ir_print_visitor::visit(ir_expression *ir)
+void ir_print_visitor::visit(class ir_expression *ir)
 {
fprintf(f, "(expression ");
 
@@ -254,7 +254,7 @@ void ir_print_visitor::visit(ir_expression *ir)
 }
 
 
-void ir_print_visitor::visit(ir_texture *ir)
+void ir_print_visitor::visit(class ir_texture *ir)
 {
fprintf(f, "(%s ", ir->opcode_string());
 
@@ -327,7 +327,7 @@ void ir_print_visitor::visit(ir_texture *ir)
 }
 
 
-void ir_print_visitor::visit(ir_swizzle *ir)
+void ir_print_visitor::visit(class ir_swizzle *ir)
 {
const unsigned swiz[4] = {
   ir->mask.x,
@@ -346,14 +346,14 @@ void ir_print_visitor::visit(ir_swizzle *ir)
 }
 
 
-void ir_print_visitor::visit(ir_dereference_variable *ir)
+void ir_print_visitor::visit(class ir_dereference_variable *ir)
 {
ir_variable *var = ir->variable_referenced();
fprintf(f, "(var_ref %s) ", unique_name(var));
 }
 
 
-void ir_print_visitor::visit

[Mesa-dev] [PATCH] util: Prefer atomic intrinsics to inline assembly.

2014-11-21 Thread Matt Turner
Cuts a little more than 1k of .text size from i915g.

This was previously done in commit 5f66b340 and subsequently reverted in
commit 3661f757 after bug 30514 was filed. I believe the cause of bug
30514 wasn't anything related to cross compiling, but rather that the
toolchain used defaulted to -march=i386, and i386 doesn't have the
CMPXCHG or XADD instructions used to implement the intrinsics.

So we reverted a patch that improved things so that we didn't break
compilation for a platform that never could have worked anyway.
---
I wouldn't mind some kind of check that this hardware support is available
but I don't know how to do it. gcc -m32 defines __i386__, which doesn't
tell you that you're compiling with -march=i386, but rather that you're
compiling with -m32. Using -march=i486 defines both __i386__ and __i486__.

 src/gallium/auxiliary/util/u_atomic.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/gallium/auxiliary/util/u_atomic.h 
b/src/gallium/auxiliary/util/u_atomic.h
index 9731aa0..b01aa38 100644
--- a/src/gallium/auxiliary/util/u_atomic.h
+++ b/src/gallium/auxiliary/util/u_atomic.h
@@ -23,13 +23,13 @@
 #elif defined(PIPE_CC_MSVC)
 #define PIPE_ATOMIC_MSVC_INTRINSIC
 #elif (defined(PIPE_CC_MSVC) && defined(PIPE_ARCH_X86))
+#elif defined(PIPE_CC_GCC) && (PIPE_CC_GCC_VERSION >= 401)
+#define PIPE_ATOMIC_GCC_INTRINSIC
 #define PIPE_ATOMIC_ASM_MSVC_X86
 #elif (defined(PIPE_CC_GCC) && defined(PIPE_ARCH_X86))
 #define PIPE_ATOMIC_ASM_GCC_X86
 #elif (defined(PIPE_CC_GCC) && defined(PIPE_ARCH_X86_64))
 #define PIPE_ATOMIC_ASM_GCC_X86_64
-#elif defined(PIPE_CC_GCC) && (PIPE_CC_GCC_VERSION >= 401)
-#define PIPE_ATOMIC_GCC_INTRINSIC
 #else
 #error "Unsupported platform"
 #endif
-- 
2.0.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] util: Implement assume() for clang.

2014-11-21 Thread Matt Turner
---
 src/util/macros.h | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/src/util/macros.h b/src/util/macros.h
index da5daff..b67596d 100644
--- a/src/util/macros.h
+++ b/src/util/macros.h
@@ -29,6 +29,10 @@
 #  define ARRAY_SIZE(x) (sizeof(x) / sizeof(*(x)))
 #endif
 
+/* For compatibility with Clang's __has_builtin() */
+#ifndef __has_builtin
+#  define __has_builtin(x) 0
+#endif
 
 /**
  * __builtin_expect macros
@@ -85,7 +89,13 @@ do {\
  * Assume macro. Useful for expressing our assumptions to the compiler,
  * typically for purposes of silencing warnings.
  */
-#ifdef HAVE___BUILTIN_UNREACHABLE
+#if __has_builtin(__builtin_assume)
+#define assume(expr) \
+do { \
+   assert(!"assumption failed"); \
+   __builtin_assume(expr);   \
+} while (0)
+#elif defined HAVE___BUILTIN_UNREACHABLE
 #define assume(expr) ((expr) ? ((void) 0) \
  : (assert(!"assumption failed"), \
 __builtin_unreachable()))
-- 
2.0.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 2/5] i965/gen7: Don't rely directly on the hiz miptree structure

2014-11-21 Thread Jordan Justen
We are still allocating a miptree for hiz, but we only use fields from
intel_miptree_aux_buffer. This will allow us to switch over to not
allocating a miptree.

Signed-off-by: Jordan Justen 
Reviewed-by: Topi Pohjolainen 
Reviewed-by: Kenneth Graunke 
Reviewed-by: Ben Widawsky 
---
 src/mesa/drivers/dri/i965/gen7_blorp.cpp| 6 +++---
 src/mesa/drivers/dri/i965/gen7_misc_state.c | 7 ---
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp 
b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
index 6ba65d6..fb6a0dd 100644
--- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
+++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
@@ -695,13 +695,13 @@ gen7_blorp_emit_depth_stencil_config(struct brw_context 
*brw,
 
/* 3DSTATE_HIER_DEPTH_BUFFER */
{
-  struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_buf->mt;
+  struct intel_miptree_aux_buffer *hiz_buf = params->depth.mt->hiz_buf;
 
   BEGIN_BATCH(3);
   OUT_BATCH((GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16) | (3 - 2));
   OUT_BATCH((mocs << 25) |
-(hiz_mt->pitch - 1));
-  OUT_RELOC(hiz_mt->bo,
+(hiz_buf->pitch - 1));
+  OUT_RELOC(hiz_buf->bo,
 I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
 0);
   ADVANCE_BATCH();
diff --git a/src/mesa/drivers/dri/i965/gen7_misc_state.c 
b/src/mesa/drivers/dri/i965/gen7_misc_state.c
index 6c6a79b..8f8c33e 100644
--- a/src/mesa/drivers/dri/i965/gen7_misc_state.c
+++ b/src/mesa/drivers/dri/i965/gen7_misc_state.c
@@ -145,12 +145,13 @@ gen7_emit_depth_stencil_hiz(struct brw_context *brw,
   OUT_BATCH(0);
   ADVANCE_BATCH();
} else {
-  struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_buf->mt;
+  struct intel_miptree_aux_buffer *hiz_buf = depth_mt->hiz_buf;
+
   BEGIN_BATCH(3);
   OUT_BATCH(GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16 | (3 - 2));
   OUT_BATCH((mocs << 25) |
-(hiz_mt->pitch - 1));
-  OUT_RELOC(hiz_mt->bo,
+(hiz_buf->pitch - 1));
+  OUT_RELOC(hiz_buf->bo,
 I915_GEM_DOMAIN_RENDER,
 I915_GEM_DOMAIN_RENDER,
 0);
-- 
2.1.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 3/5] i965/gen8: Don't rely directly on the hiz miptree structure

2014-11-21 Thread Jordan Justen
We are still allocating a miptree for hiz, but we only use fields from
intel_miptree_aux_buffer. This will allow us to switch over to not
allocating a miptree.

Signed-off-by: Jordan Justen 
Reviewed-by: Topi Pohjolainen 
Reviewed-by: Kenneth Graunke 
Reviewed-by: Ben Widawsky 
---
 src/mesa/drivers/dri/i965/gen8_depth_state.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/gen8_depth_state.c 
b/src/mesa/drivers/dri/i965/gen8_depth_state.c
index 5291968..1d4f829 100644
--- a/src/mesa/drivers/dri/i965/gen8_depth_state.c
+++ b/src/mesa/drivers/dri/i965/gen8_depth_state.c
@@ -92,10 +92,10 @@ emit_depth_packets(struct brw_context *brw,
} else {
   BEGIN_BATCH(5);
   OUT_BATCH(GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16 | (5 - 2));
-  OUT_BATCH((depth_mt->hiz_buf->mt->pitch - 1) | mocs_wb << 25);
-  OUT_RELOC64(depth_mt->hiz_buf->mt->bo,
+  OUT_BATCH((depth_mt->hiz_buf->pitch - 1) | mocs_wb << 25);
+  OUT_RELOC64(depth_mt->hiz_buf->bo,
   I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, 0);
-  OUT_BATCH(depth_mt->hiz_buf->mt->qpitch >> 2);
+  OUT_BATCH(depth_mt->hiz_buf->qpitch >> 2);
   ADVANCE_BATCH();
}
 
-- 
2.1.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 5/5] i965/gen8: Don't allocate hiz miptree structure

2014-11-21 Thread Jordan Justen
We now skip allocating a hiz miptree for gen8. Instead, we calculate
the required hiz buffer parameters and allocate a bo directly.

v2:
 * Update hz_height calculation as suggested by Topi
v3:
 * Bail if we failed to create the bo (Ben)

Signed-off-by: Jordan Justen 
Reviewed-by: Topi Pohjolainen 
---
 src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 100 ++
 1 file changed, 100 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c 
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index e6ee8d7..c62b0b8 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -1497,6 +1497,104 @@ intel_gen7_hiz_buf_create(struct brw_context *brw,
 }
 
 
+/**
+ * Helper for intel_miptree_alloc_hiz() that determines the required hiz
+ * buffer dimensions and allocates a bo for the hiz buffer.
+ */
+static struct intel_miptree_aux_buffer *
+intel_gen8_hiz_buf_create(struct brw_context *brw,
+  struct intel_mipmap_tree *mt)
+{
+   unsigned z_width = mt->logical_width0;
+   unsigned z_height = mt->logical_height0;
+   const unsigned z_depth = mt->logical_depth0;
+   unsigned hz_width, hz_height;
+   struct intel_miptree_aux_buffer *buf = calloc(sizeof(*buf), 1);
+
+   if (!buf)
+  return NULL;
+
+   /* Gen7 PRM Volume 2, Part 1, 11.5.3 "Hierarchical Depth Buffer" documents
+* adjustments required for Z_Height and Z_Width based on multisampling.
+*/
+   switch (mt->num_samples) {
+   case 0:
+   case 1:
+  break;
+   case 2:
+   case 4:
+  z_width *= 2;
+  z_height *= 2;
+  break;
+   case 8:
+  z_width *= 4;
+  z_height *= 2;
+  break;
+   default:
+  assert(!"Unsupported sample count!");
+   }
+
+   const unsigned vertical_align = 8; /* 'j' in the docs */
+   const unsigned H0 = z_height;
+   const unsigned h0 = ALIGN(H0, vertical_align);
+   const unsigned h1 = ALIGN(minify(H0, 1), vertical_align);
+   const unsigned Z0 = z_depth;
+
+   /* HZ_Width (bytes) = ceiling(Z_Width / 16) * 16 */
+   hz_width = ALIGN(z_width, 16);
+
+   unsigned H_i = H0;
+   unsigned Z_i = Z0;
+   unsigned sum_h_i = 0;
+   unsigned hz_height_3d_sum = 0;
+   for (int level = mt->first_level; level <= mt->last_level; ++level) {
+  unsigned i = level - mt->first_level;
+  unsigned h_i = ALIGN(H_i, vertical_align);
+  /* sum(i=2 to m; h_i) */
+  if (i >= 2) {
+ sum_h_i += h_i;
+  }
+  /* sum(i=0 to m; h_i * max(1, floor(Z_Depth/2**i))) */
+  hz_height_3d_sum += h_i * Z_i;
+  H_i = minify(H_i, 1);
+  Z_i = minify(Z_i, 1);
+   }
+   /* HZ_QPitch = h0 + max(h1, sum(i=2 to m; h_i)) */
+   buf->qpitch = h0 + MAX2(h1, sum_h_i);
+
+   if (mt->target == GL_TEXTURE_3D) {
+  /* (1/2) * sum(i=0 to m; h_i * max(1, floor(Z_Depth/2**i))) */
+  hz_height = CEILING(hz_height_3d_sum, 2);
+   } else {
+  /* HZ_Height (rows) = ceiling( (HZ_QPitch/2)/8) *8 * Z_Depth */
+  hz_height = CEILING(buf->qpitch, 2 * 8) * 8 * Z0;
+  if (mt->target == GL_TEXTURE_CUBE_MAP_ARRAY ||
+  mt->target == GL_TEXTURE_CUBE_MAP) {
+ /* HZ_Height (rows) = ceiling( (HZ_QPitch/2)/8) *8 * 6
+  *
+  * Assumption: the doc meant to multiply by Z_Depth as well.
+  */
+ hz_height *= 6;
+  }
+   }
+
+   unsigned long pitch;
+   uint32_t tiling = I915_TILING_Y;
+   buf->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "hiz",
+  hz_width, hz_height, 1,
+  &tiling, &pitch,
+  BO_ALLOC_FOR_RENDER);
+   if (!buf->bo) {
+  free(buf);
+  return NULL;
+   }
+
+   buf->pitch = pitch;
+
+   return buf;
+}
+
+
 static struct intel_miptree_aux_buffer *
 intel_hiz_miptree_buf_create(struct brw_context *brw,
  struct intel_mipmap_tree *mt)
@@ -1540,6 +1638,8 @@ intel_miptree_alloc_hiz(struct brw_context *brw,
 
if (brw->gen == 7) {
   mt->hiz_buf = intel_gen7_hiz_buf_create(brw, mt);
+   } else if (brw->gen >= 8) {
+  mt->hiz_buf = intel_gen8_hiz_buf_create(brw, mt);
} else {
   mt->hiz_buf = intel_hiz_miptree_buf_create(brw, mt);
}
-- 
2.1.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 1/5] i965/hiz: Start to separate miptree out from hiz buffers

2014-11-21 Thread Jordan Justen
Today we allocate a miptree's for the hiz buffer. We needed this in
the past because we would point the hardware at offsets of the hiz
buffer. Since the hiz format is not documented, this is not a good
idea.

Since moving to support layered rendering on Gen7+, we no longer point
at an offset into the buffer on Gen7+.

Therefore, to support hiz on Gen7+, we don't need a full miptree
structure allocated.

This patch starts to create a new auxiliary buffer structure
(intel_miptree_aux_buffer) that can be a more simplistic miptree
side-band buffer associated with a miptree. (For example, to serve the
needs of the hiz buffer.)

Signed-off-by: Jordan Justen 
Reviewed-by: Topi Pohjolainen 
Reviewed-by: Kenneth Graunke 
Reviewed-by: Ben Widawsky 
---
 git://people.freedesktop.org/~jljusten/mesa shrink-hiz-buf

 src/mesa/drivers/dri/i965/brw_misc_state.c|  4 +-
 src/mesa/drivers/dri/i965/gen6_blorp.cpp  |  2 +-
 src/mesa/drivers/dri/i965/gen6_depth_state.c  |  2 +-
 src/mesa/drivers/dri/i965/gen7_blorp.cpp  |  2 +-
 src/mesa/drivers/dri/i965/gen7_misc_state.c   |  2 +-
 src/mesa/drivers/dri/i965/gen8_depth_state.c  |  6 +--
 src/mesa/drivers/dri/i965/intel_fbo.c |  4 +-
 src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 59 +++
 src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 29 +++--
 9 files changed, 79 insertions(+), 31 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c 
b/src/mesa/drivers/dri/i965/brw_misc_state.c
index 99fcddc..68a20a9 100644
--- a/src/mesa/drivers/dri/i965/brw_misc_state.c
+++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
@@ -182,7 +182,7 @@ brw_get_depthstencil_tile_masks(struct intel_mipmap_tree 
*depth_mt,
 
   if (intel_miptree_level_has_hiz(depth_mt, depth_level)) {
  uint32_t hiz_tile_mask_x, hiz_tile_mask_y;
- intel_miptree_get_tile_masks(depth_mt->hiz_mt,
+ intel_miptree_get_tile_masks(depth_mt->hiz_buf->mt,
   &hiz_tile_mask_x, &hiz_tile_mask_y,
   false);
 
@@ -643,7 +643,7 @@ brw_emit_depth_stencil_hiz(struct brw_context *brw,
 
   /* Emit hiz buffer. */
   if (hiz) {
- struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_mt;
+ struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_buf->mt;
 BEGIN_BATCH(3);
 OUT_BATCH((_3DSTATE_HIER_DEPTH_BUFFER << 16) | (3 - 2));
 OUT_BATCH(hiz_mt->pitch - 1);
diff --git a/src/mesa/drivers/dri/i965/gen6_blorp.cpp 
b/src/mesa/drivers/dri/i965/gen6_blorp.cpp
index d4aa955..85bfc82 100644
--- a/src/mesa/drivers/dri/i965/gen6_blorp.cpp
+++ b/src/mesa/drivers/dri/i965/gen6_blorp.cpp
@@ -859,7 +859,7 @@ gen6_blorp_emit_depth_stencil_config(struct brw_context 
*brw,
 
/* 3DSTATE_HIER_DEPTH_BUFFER */
{
-  struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_mt;
+  struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_buf->mt;
   uint32_t offset = 0;
 
   if (hiz_mt->array_layout == ALL_SLICES_AT_EACH_LOD) {
diff --git a/src/mesa/drivers/dri/i965/gen6_depth_state.c 
b/src/mesa/drivers/dri/i965/gen6_depth_state.c
index f92c1af..3ecefc3 100644
--- a/src/mesa/drivers/dri/i965/gen6_depth_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_depth_state.c
@@ -161,7 +161,7 @@ gen6_emit_depth_stencil_hiz(struct brw_context *brw,
 
   /* Emit hiz buffer. */
   if (hiz) {
- struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_mt;
+ struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_buf->mt;
  uint32_t offset = 0;
 
  if (hiz_mt->array_layout == ALL_SLICES_AT_EACH_LOD) {
diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp 
b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
index 206a6ff..6ba65d6 100644
--- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
+++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
@@ -695,7 +695,7 @@ gen7_blorp_emit_depth_stencil_config(struct brw_context 
*brw,
 
/* 3DSTATE_HIER_DEPTH_BUFFER */
{
-  struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_mt;
+  struct intel_mipmap_tree *hiz_mt = params->depth.mt->hiz_buf->mt;
 
   BEGIN_BATCH(3);
   OUT_BATCH((GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16) | (3 - 2));
diff --git a/src/mesa/drivers/dri/i965/gen7_misc_state.c 
b/src/mesa/drivers/dri/i965/gen7_misc_state.c
index 22911bf..6c6a79b 100644
--- a/src/mesa/drivers/dri/i965/gen7_misc_state.c
+++ b/src/mesa/drivers/dri/i965/gen7_misc_state.c
@@ -145,7 +145,7 @@ gen7_emit_depth_stencil_hiz(struct brw_context *brw,
   OUT_BATCH(0);
   ADVANCE_BATCH();
} else {
-  struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_mt;
+  struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_buf->mt;
   BEGIN_BATCH(3);
   OUT_BATCH(GEN7_3DSTATE_HIER_DEPTH_BUFFER << 16 | (3 - 2));
   OUT_BATCH((mocs << 25) |
diff --git a/src/mesa/drivers/dri/i965/gen8_depth_state.c 
b/src/mesa/drivers/dri/i965/gen8_depth_state.c
index e716141..5291968 100644
--- a/src/mesa/drivers/dri/i965/gen

[Mesa-dev] [PATCH 4/5] i965/gen7: Don't allocate hiz miptree structure

2014-11-21 Thread Jordan Justen
We now skip allocating a hiz miptree for gen7. Instead, we calculate
the required hiz buffer parameters and allocate a bo directly.

v2:
 * Update hz_height calculation as suggested by Topi
v3:
 * Bail if we failed to create the bo (Ben)

Signed-off-by: Jordan Justen 
Reviewed-by: Topi Pohjolainen 
---
 src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 102 +-
 1 file changed, 100 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c 
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 8800867..e6ee8d7 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -851,7 +851,10 @@ intel_miptree_release(struct intel_mipmap_tree **mt)
   drm_intel_bo_unreference((*mt)->bo);
   intel_miptree_release(&(*mt)->stencil_mt);
   if ((*mt)->hiz_buf) {
- intel_miptree_release(&(*mt)->hiz_buf->mt);
+ if ((*mt)->hiz_buf->mt)
+intel_miptree_release(&(*mt)->hiz_buf->mt);
+ else
+drm_intel_bo_unreference((*mt)->hiz_buf->bo);
  free((*mt)->hiz_buf);
   }
   intel_miptree_release(&(*mt)->mcs_mt);
@@ -1404,6 +1407,96 @@ intel_miptree_level_enable_hiz(struct brw_context *brw,
 }
 
 
+/**
+ * Helper for intel_miptree_alloc_hiz() that determines the required hiz
+ * buffer dimensions and allocates a bo for the hiz buffer.
+ */
+static struct intel_miptree_aux_buffer *
+intel_gen7_hiz_buf_create(struct brw_context *brw,
+  struct intel_mipmap_tree *mt)
+{
+   unsigned z_width = mt->logical_width0;
+   unsigned z_height = mt->logical_height0;
+   const unsigned z_depth = mt->logical_depth0;
+   unsigned hz_width, hz_height;
+   struct intel_miptree_aux_buffer *buf = calloc(sizeof(*buf), 1);
+
+   if (!buf)
+  return NULL;
+
+   /* Gen7 PRM Volume 2, Part 1, 11.5.3 "Hierarchical Depth Buffer" documents
+* adjustments required for Z_Height and Z_Width based on multisampling.
+*/
+   switch (mt->num_samples) {
+   case 0:
+   case 1:
+  break;
+   case 2:
+   case 4:
+  z_width *= 2;
+  z_height *= 2;
+  break;
+   case 8:
+  z_width *= 4;
+  z_height *= 2;
+  break;
+   default:
+  assert(!"Unsupported sample count!");
+   }
+
+   const unsigned vertical_align = 8; /* 'j' in the docs */
+   const unsigned H0 = z_height;
+   const unsigned h0 = ALIGN(H0, vertical_align);
+   const unsigned h1 = ALIGN(minify(H0, 1), vertical_align);
+   const unsigned Z0 = z_depth;
+
+   /* HZ_Width (bytes) = ceiling(Z_Width / 16) * 16 */
+   hz_width = ALIGN(z_width, 16);
+
+   if (mt->target == GL_TEXTURE_3D) {
+  unsigned H_i = H0;
+  unsigned Z_i = Z0;
+  hz_height = 0;
+  for (int level = mt->first_level; level <= mt->last_level; ++level) {
+ unsigned h_i = ALIGN(H_i, vertical_align);
+ /* sum(i=0 to m; h_i * max(1, floor(Z_Depth/2**i))) */
+ hz_height += h_i * Z_i;
+ H_i = minify(H_i, 1);
+ Z_i = minify(Z_i, 1);
+  }
+  /* HZ_Height =
+   *(1/2) * sum(i=0 to m; h_i * max(1, floor(Z_Depth/2**i)))
+   */
+  hz_height = CEILING(hz_height, 2);
+   } else {
+  const unsigned hz_qpitch = h0 + h1 + (12 * vertical_align);
+  if (mt->target == GL_TEXTURE_CUBE_MAP_ARRAY ||
+  mt->target == GL_TEXTURE_CUBE_MAP) {
+ /* HZ_Height (rows) = Ceiling ( ( Q_pitch * Z_depth * 6/2) /8 ) * 8 */
+ hz_height = CEILING(hz_qpitch * Z0 * 6, 2 * 8) * 8;
+  } else {
+ /* HZ_Height (rows) = Ceiling ( ( Q_pitch * Z_depth/2) /8 ) * 8 */
+ hz_height = CEILING(hz_qpitch * Z0, 2 * 8) * 8;
+  }
+   }
+
+   unsigned long pitch;
+   uint32_t tiling = I915_TILING_Y;
+   buf->bo = drm_intel_bo_alloc_tiled(brw->bufmgr, "hiz",
+  hz_width, hz_height, 1,
+  &tiling, &pitch,
+  BO_ALLOC_FOR_RENDER);
+   if (!buf->bo) {
+  free(buf);
+  return NULL;
+   }
+
+   buf->pitch = pitch;
+
+   return buf;
+}
+
+
 static struct intel_miptree_aux_buffer *
 intel_hiz_miptree_buf_create(struct brw_context *brw,
  struct intel_mipmap_tree *mt)
@@ -1444,7 +1537,12 @@ intel_miptree_alloc_hiz(struct brw_context *brw,
struct intel_mipmap_tree *mt)
 {
assert(mt->hiz_buf == NULL);
-   mt->hiz_buf = intel_hiz_miptree_buf_create(brw, mt);
+
+   if (brw->gen == 7) {
+  mt->hiz_buf = intel_gen7_hiz_buf_create(brw, mt);
+   } else {
+  mt->hiz_buf = intel_hiz_miptree_buf_create(brw, mt);
+   }
 
if (!mt->hiz_buf)
   return false;
-- 
2.1.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] i965/fs: Clarify and optimize register coalescing interference check.

2014-11-21 Thread Matt Turner
With bblock_t::end_ip, we can skip entire blocks rather than checking
every instruction in them. Also by making the loops nested (explicitly,
it already was a nested loop), the continue seen at the end of the first
hunk (when viewed with git show -w) will do what the programmer expects.

No functional change.
---
 .../drivers/dri/i965/brw_fs_register_coalesce.cpp  | 28 +++---
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_register_coalesce.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_register_coalesce.cpp
index 62788cd..9c8ca32 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_register_coalesce.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_register_coalesce.cpp
@@ -126,26 +126,26 @@ can_coalesce_vars(brw::fs_live_variables *live_intervals,
   return false;
 
int start_ip = MIN2(start_to, start_from);
-   int scan_ip = -1;
 
-   foreach_block_and_inst(block, fs_inst, scan_inst, cfg) {
-  scan_ip++;
-
-  if (scan_ip < start_ip)
+   foreach_block(block, cfg) {
+  if (start_ip > block->end_ip)
  continue;
 
-  if (scan_inst->is_control_flow())
- return false;
+  int scan_ip = block->start_ip - 1;
 
-  if (scan_ip <= live_intervals->start[var_to])
- continue;
+  foreach_inst_in_block(fs_inst, scan_inst, block) {
+ scan_ip++;
+
+ if (scan_ip <= live_intervals->start[var_to])
+continue;
 
-  if (scan_ip > live_intervals->end[var_to])
- return true;
+ if (scan_ip > live_intervals->end[var_to])
+return true;
 
-  if (scan_inst->dst.equals(inst->dst) ||
-  scan_inst->dst.equals(inst->src[0]))
- return false;
+ if (scan_inst->dst.equals(inst->dst) ||
+ scan_inst->dst.equals(inst->src[0]))
+return false;
+  }
}
 
return true;
-- 
2.0.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 2/2] i965: Don't overwrite the math SFID with conditional mod.

2014-11-21 Thread Matt Turner
Ben was asking about the undocumented restriction that the math
instruction cannot use the dependency control hints. I went to reconfirm
and disabled the is_math() check in opt_set_dependency_control() and saw
that the disassembled math instructions with dependency hints had a
bogus SFID. We were mistakenly overwriting it by setting an empty
conditional mod.

Unfortunately, this wasn't the cause of the aforementioned problem (I
reproduced it). This bug is benign, since we don't set dependeny hints
on math instructions -- but maybe some day.
---
Interestingly the BSpec now says about the math instruction:

Restriction : DepCtrl must not be used.

but only lists BDW and newer generations, so it seems like if pre-BDW is
known broken and BDW and newer can't set DepCtrl, it might be a while.

 src/mesa/drivers/dri/i965/brw_fs_generator.cpp   | 3 ++-
 src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
index 4af9cbe..06b0f34 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
@@ -2010,7 +2010,8 @@ fs_generator::generate_code(const cfg_t *cfg, int 
dispatch_width)
 
  brw_inst *last = &p->store[last_insn_offset / 16];
 
- brw_inst_set_cond_modifier(brw, last, inst->conditional_mod);
+ if (inst->conditional_mod)
+brw_inst_set_cond_modifier(brw, last, inst->conditional_mod);
  brw_inst_set_no_dd_clear(brw, last, inst->no_dd_clear);
  brw_inst_set_no_dd_check(brw, last, inst->no_dd_check);
   }
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
index d027fda..6a70a2f 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
@@ -1509,7 +1509,8 @@ vec4_generator::generate_code(const cfg_t *cfg)
 
  brw_inst *last = &p->store[pre_emit_nr_insn];
 
- brw_inst_set_cond_modifier(brw, last, inst->conditional_mod);
+ if (inst->conditional_mod)
+brw_inst_set_cond_modifier(brw, last, inst->conditional_mod);
  brw_inst_set_no_dd_clear(brw, last, inst->no_dd_clear);
  brw_inst_set_no_dd_check(brw, last, inst->no_dd_check);
   }
-- 
2.0.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 1/2] i965: Assert that math instructions don't have conditional mod.

2014-11-21 Thread Matt Turner
---
Strangely, the suggested implementations of double-precision sqrt/rcp
in the BSpec show using a math instruction with an "eo" conditional
modifier. I have no idea what that could possibly mean, or how it could
work since conditional mod shares the same four bits with math SFID.

 src/mesa/drivers/dri/i965/brw_fs_generator.cpp   | 2 ++
 src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
index ab5d223..4af9cbe 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
@@ -1817,6 +1817,7 @@ fs_generator::generate_code(const cfg_t *cfg, int 
dispatch_width)
   case SHADER_OPCODE_SIN:
   case SHADER_OPCODE_COS:
  assert(brw->gen < 6 || inst->mlen == 0);
+ assert(inst->conditional_mod == BRW_CONDITIONAL_NONE);
 if (brw->gen >= 7) {
 gen6_math(p, dst, brw_math_function(inst->opcode), src[0],
   brw_null_reg());
@@ -1832,6 +1833,7 @@ fs_generator::generate_code(const cfg_t *cfg, int 
dispatch_width)
   case SHADER_OPCODE_INT_REMAINDER:
   case SHADER_OPCODE_POW:
  assert(brw->gen < 6 || inst->mlen == 0);
+ assert(inst->conditional_mod == BRW_CONDITIONAL_NONE);
 if (brw->gen >= 7 && inst->opcode == SHADER_OPCODE_POW) {
 gen6_math(p, dst, brw_math_function(inst->opcode), src[0], src[1]);
 } else if (brw->gen >= 6) {
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
index 0776a91..d027fda 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
@@ -1361,6 +1361,7 @@ vec4_generator::generate_code(const cfg_t *cfg)
   case SHADER_OPCODE_LOG2:
   case SHADER_OPCODE_SIN:
   case SHADER_OPCODE_COS:
+ assert(inst->conditional_mod == BRW_CONDITIONAL_NONE);
  if (brw->gen >= 7) {
 gen6_math(p, dst, brw_math_function(inst->opcode), src[0],
   brw_null_reg());
@@ -1374,6 +1375,7 @@ vec4_generator::generate_code(const cfg_t *cfg)
   case SHADER_OPCODE_POW:
   case SHADER_OPCODE_INT_QUOTIENT:
   case SHADER_OPCODE_INT_REMAINDER:
+ assert(inst->conditional_mod == BRW_CONDITIONAL_NONE);
  if (brw->gen >= 7) {
 gen6_math(p, dst, brw_math_function(inst->opcode), src[0], src[1]);
  } else if (brw->gen == 6) {
-- 
2.0.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] i965: Make Gen4-5 push constants call _mesa_load_state_parameters too.

2014-11-21 Thread Mark Janes
In my tests, this fixes piglit on gen4/5.

-Mark

Kenneth Graunke  writes:

> In commit 5e37a2a4a8a, I made the pull constant code stop calling
> _mesa_load_state_parameters() when there were no pull parameters.
>
> This worked fine on Gen6+ because the push constant code also called
> it if there were any push constants.  However, the Gen4-5 push constant
> code wasn't doing this.  This patch makes it do so, like the Gen6+ code.
>
> A better long term solution would be to make core Mesa just handle this
> for us when necessary.
>
> Fixes around 8766 Piglit tests on Ironlake, and probably Gen4 as well.
>
> Reported-by: Mark Janes 
> Signed-off-by: Kenneth Graunke 
> ---
>  src/mesa/drivers/dri/i965/brw_curbe.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_curbe.c 
> b/src/mesa/drivers/dri/i965/brw_curbe.c
> index 1a828ed..fea5d332 100644
> --- a/src/mesa/drivers/dri/i965/brw_curbe.c
> +++ b/src/mesa/drivers/dri/i965/brw_curbe.c
> @@ -211,6 +211,8 @@ brw_upload_constant_buffer(struct brw_context *brw)
>  
> /* fragment shader constants */
> if (brw->curbe.wm_size) {
> +  _mesa_load_state_parameters(ctx, 
> brw->fragment_program->Base.Parameters);
> +
>/* BRW_NEW_CURBE_OFFSETS */
>GLuint offset = brw->curbe.wm_start * 16;
>  
> @@ -251,6 +253,8 @@ brw_upload_constant_buffer(struct brw_context *brw)
>  
> /* vertex shader constants */
> if (brw->curbe.vs_size) {
> +  _mesa_load_state_parameters(ctx, brw->vertex_program->Base.Parameters);
> +
>GLuint offset = brw->curbe.vs_start * 16;
>  
>/* CACHE_NEW_VS_PROG | _NEW_PROGRAM_CONSTANTS: copy uniform values */
> -- 
> 2.1.2
>
> ___
> 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/3] glapi: Remove dead mesadef.py.

2014-11-21 Thread Jose Fonseca

On 21/11/14 18:46, Matt Turner wrote:

On Fri, Nov 21, 2014 at 10:42 AM, Ilia Mirkin  wrote:

What about src/mesa/drivers/windows/gdi/mesa.def -- it claims to be
generated by mesadef.py. I have no idea what it is, but it should
either also be deleted, or the mesadef.py script kept around.


Good point. Maybe Brian knows (Cc'd).
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=AAIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=zfmBZnnVGHeYde45pMKNnVyzeaZbdIqVLprmZCM2zzE&m=fmXAA0WbK-5OnmrssLHDausZYYaPM2tl6mSNxDySxYo&s=zkw6gth0NE7nK10rHcksWQXoVMp_WFP1JXeQoJtB-y4&e=



Don't know about mesadef.py, but mesa.def itself is used, as `git grep` 
 shows:


$ git grep -C1 '\'
[...]
--
src/mesa/drivers/windows/gdi/SConscript-sources = [
src/mesa/drivers/windows/gdi/SConscript:'mesa.def',
src/mesa/drivers/windows/gdi/SConscript-'wgl.c',


But I think the warning at the top of mesa.def is misleading.  That file 
has benn manually edited several times now.



Strictly speaking .def aren't necessary -- `declspec(dllexport)` are 
sufficient, but I personally like .def files because they cause build 
failures if symbols are missing, or sometimes have different types (due 
to symbol decorators used by stdcall convention).



In short, let's keep mesa.def, but we should update the comment.


Jose
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/2] i965: Extract depctrl hazards

2014-11-21 Thread Matt Turner
On Fri, Nov 21, 2014 at 11:18 AM, Ben Widawsky  wrote:
> On Fri, Nov 21, 2014 at 11:04:18AM -0800, Matt Turner wrote:
>> On Fri, Nov 21, 2014 at 10:50 AM, Ben Widawsky
>>  wrote:
>> > +*/ +   return !(inst->mlen || inst->predicate || inst->is_math());
>>
>> The return value is a ! expression, and below our only caller is inverting 
>> the
>> result. I'd probably just make it is_dep_ctrl_unsafe() or something.
>
> I liked the sound of _safe better (and it fits in with my eventual plans 
> better,
> should I actually get around to it). It's fine with me to change it.

Okay, either way.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] i965/gen8: Handle the MUL dest hazard exception

2014-11-21 Thread Ben Widawsky
On Fri, Nov 21, 2014 at 11:09:44AM -0800, Matt Turner wrote:
> On Fri, Nov 21, 2014 at 10:50 AM, Ben Widawsky
>  wrote:
> > Fix one of the few cases where we can't reliable touch the destination 
> > hazard
> > bits. I am explicitly doing this patch individually so it is easy to 
> > backport. I
> > was tempted to do this patch before the previous patch which reorganized the
> > code, but I believe even doing that first, this is still easy to backport.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86268
> > Signed-off-by: Ben Widawsky 
> > ---
> >  src/mesa/drivers/dri/i965/brw_vec4.cpp | 20 ++--
> >  src/mesa/drivers/dri/i965/brw_vec4.h   |  1 +
> >  2 files changed, 19 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp 
> > b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> > index 0c2bbe9..d583f27 100644
> > --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> > @@ -841,9 +841,25 @@ vec4_visitor::move_push_constants_to_pull_constants()
> >  }
> >
> >  /* Conditions for which we want to avoid setting the dependency control 
> > bits */
> > -static bool
> > -is_dep_ctrl_safe(vec4_instruction *inst)
> > +bool
> > +vec4_visitor::is_dep_ctrl_safe(vec4_instruction *inst)
> >  {
> > +#define IS_DWORD(reg) \
> > +   (reg.type == BRW_REGISTER_TYPE_UD || \
> > +reg.type == BRW_REGISTER_TYPE_D)
> 
> I don't really love the macro, but meh.
> 
> With the comments about #1 addressed, these two are
> 
> Reviewed-by: Matt Turner 
> 
> Did they fix anything that you can tell?

Yes. It does fix the hang (and make the ES3 test pass). I've asked QA to
confirm. They confirmed a more rudimentary form yesterday.

-- 
Ben Widawsky, Intel Open Source Technology Center
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/2] i965: Extract depctrl hazards

2014-11-21 Thread Ben Widawsky
On Fri, Nov 21, 2014 at 11:04:18AM -0800, Matt Turner wrote:
> On Fri, Nov 21, 2014 at 10:50 AM, Ben Widawsky
>  wrote:
> > Move this to a separate function so that we can begin to add other little
> > caveats without making too big a mess.
> >
> > NOTE: There is some desire to improve this function eventually, but we need 
> > to
> > fix a bug first.
> >
> > Signed-off-by: Ben Widawsky 
> > ---
> >  src/mesa/drivers/dri/i965/brw_vec4.cpp | 42 
> > --
> >  1 file changed, 20 insertions(+), 22 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp 
> > b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> > index df589b8..0c2bbe9 100644
> > --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> > @@ -840,6 +840,25 @@ vec4_visitor::move_push_constants_to_pull_constants()
> > pack_uniform_registers();
> >  }
> >
> > +/* Conditions for which we want to avoid setting the dependency control 
> > bits */
> > +static bool
> > +is_dep_ctrl_safe(vec4_instruction *inst)
> 
> Can mark as const.
> 
> > +{
> > +   /*
> > +* In the presence of send messages, totally interrupt dependency
> > +* control. They're long enough that the chance of dependency
> > +* control around them just doesn't matter.
> > +*
> > +* It looks like setting dependency control on a predicated
> > +* instruction hangs the GPU.
> > +* NB: I can find no evidence of this in modern specs.
> 
> It's in the Destination Hazard section.
> 
> Quoting the IVB PRM:
> 
> When a sequence of NoDDChk and NoDDClr are used, the last instruction
> that completes the scoreboard
> clear must have a non-zero execution mask. This means, if any kind of
> predication can change the
> execution mask or channel enable of the last instruction, the
> optimization must be avoided. This is to
> avoid instructions being shot down the pipeline when no writes are required.
> 
> Example:
> (f0.0) mov r10.0 r11.0 {NoDDClr}
> (-f0.0) mov r10.0 r11.0 {NoDDChk, NoDDClr}
> 
> In the above case, if predication can disable all writes to r10 for
> the second instructions, the instruction
> maybe shot down the pipeline resulting in un-deterministic behavior.
> Hence, This optimization must not
> be used in these cases.

Thanks. I'll add this quote to the blurb.

> 
> > +*
> > +* Dependency control does not work well over math instructions.
> > +* NB: I can find no evidence of this in modern specs.
> 
> I don't know what NB means. This one was empirically determined, so
> I'd just say it's undocumented if you want to say anything at all.

NB is the commonly used term in the kernel for "NOTE" basically, ie. this is
non-obvious. I thought it was a fairly common convention... guess not.

I intentionally didn't write it was empirically discovered because I [falsely]
assumed that came from an ancient chipset (or Sandybridge, which I don't trust).
I didn't want to imply that the empirical evidence was modern. Looking at the
commit now, it was probably run on Ivybridge, so it makes sense to add that
info.

> 
> > +*/ +   return !(inst->mlen || inst->predicate || inst->is_math());
> 
> The return value is a ! expression, and below our only caller is inverting the
> result. I'd probably just make it is_dep_ctrl_unsafe() or something.

I liked the sound of _safe better (and it fits in with my eventual plans better,
should I actually get around to it). It's fine with me to change it.

-- 
Ben Widawsky, Intel Open Source Technology Center
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] i965/gen8: Handle the MUL dest hazard exception

2014-11-21 Thread Matt Turner
On Fri, Nov 21, 2014 at 10:50 AM, Ben Widawsky
 wrote:
> Fix one of the few cases where we can't reliable touch the destination hazard
> bits. I am explicitly doing this patch individually so it is easy to 
> backport. I
> was tempted to do this patch before the previous patch which reorganized the
> code, but I believe even doing that first, this is still easy to backport.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86268
> Signed-off-by: Ben Widawsky 
> ---
>  src/mesa/drivers/dri/i965/brw_vec4.cpp | 20 ++--
>  src/mesa/drivers/dri/i965/brw_vec4.h   |  1 +
>  2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp 
> b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index 0c2bbe9..d583f27 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -841,9 +841,25 @@ vec4_visitor::move_push_constants_to_pull_constants()
>  }
>
>  /* Conditions for which we want to avoid setting the dependency control bits 
> */
> -static bool
> -is_dep_ctrl_safe(vec4_instruction *inst)
> +bool
> +vec4_visitor::is_dep_ctrl_safe(vec4_instruction *inst)
>  {
> +#define IS_DWORD(reg) \
> +   (reg.type == BRW_REGISTER_TYPE_UD || \
> +reg.type == BRW_REGISTER_TYPE_D)

I don't really love the macro, but meh.

With the comments about #1 addressed, these two are

Reviewed-by: Matt Turner 

Did they fix anything that you can tell?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/2] i965: Extract depctrl hazards

2014-11-21 Thread Matt Turner
On Fri, Nov 21, 2014 at 10:50 AM, Ben Widawsky
 wrote:
> Move this to a separate function so that we can begin to add other little
> caveats without making too big a mess.
>
> NOTE: There is some desire to improve this function eventually, but we need to
> fix a bug first.
>
> Signed-off-by: Ben Widawsky 
> ---
>  src/mesa/drivers/dri/i965/brw_vec4.cpp | 42 
> --
>  1 file changed, 20 insertions(+), 22 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp 
> b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index df589b8..0c2bbe9 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -840,6 +840,25 @@ vec4_visitor::move_push_constants_to_pull_constants()
> pack_uniform_registers();
>  }
>
> +/* Conditions for which we want to avoid setting the dependency control bits 
> */
> +static bool
> +is_dep_ctrl_safe(vec4_instruction *inst)

Can mark as const.

> +{
> +   /*
> +* In the presence of send messages, totally interrupt dependency
> +* control. They're long enough that the chance of dependency
> +* control around them just doesn't matter.
> +*
> +* It looks like setting dependency control on a predicated
> +* instruction hangs the GPU.
> +* NB: I can find no evidence of this in modern specs.

It's in the Destination Hazard section.

Quoting the IVB PRM:

When a sequence of NoDDChk and NoDDClr are used, the last instruction
that completes the scoreboard
clear must have a non-zero execution mask. This means, if any kind of
predication can change the
execution mask or channel enable of the last instruction, the
optimization must be avoided. This is to
avoid instructions being shot down the pipeline when no writes are required.

Example:
(f0.0) mov r10.0 r11.0 {NoDDClr}
(-f0.0) mov r10.0 r11.0 {NoDDChk, NoDDClr}

In the above case, if predication can disable all writes to r10 for
the second instructions, the instruction
maybe shot down the pipeline resulting in un-deterministic behavior.
Hence, This optimization must not
be used in these cases.

> +*
> +* Dependency control does not work well over math instructions.
> +* NB: I can find no evidence of this in modern specs.

I don't know what NB means. This one was empirically determined, so
I'd just say it's undocumented if you want to say anything at all.

> +*/
> +   return !(inst->mlen || inst->predicate || inst->is_math());

The return value is a ! expression, and below our only caller is
inverting the result. I'd probably just make it is_dep_ctrl_unsafe()
or something.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] glsl: Remove unused ast copy constructors.

2014-11-21 Thread Ilia Mirkin
On Fri, Nov 21, 2014 at 1:56 PM, Matt Turner  wrote:
> On Fri, Nov 21, 2014 at 10:52 AM, Ilia Mirkin  wrote:
>> On Fri, Nov 21, 2014 at 1:47 PM, Matt Turner  wrote:
>>> These were added in commits a760c738 and 43757135 to be used in
>>> implementing C-style aggregate initializers (commit 1b0d6aef). Paul
>>> rewrote that code in commit 0da1a2cc to use GLSL types, rather than
>>> AST types, leaving these copy constructors unused.
>>>
>>> Tested by making them private and providing no definition.
>>> ---
>>>  src/glsl/ast.h | 29 -
>>>  1 file changed, 29 deletions(-)
>>>
>>> diff --git a/src/glsl/ast.h b/src/glsl/ast.h
>>> index 15bf086..6995ae8 100644
>>> --- a/src/glsl/ast.h
>>> +++ b/src/glsl/ast.h
>>> @@ -640,19 +640,6 @@ class ast_declarator_list;
>>>
>>>  class ast_struct_specifier : public ast_node {
>>>  public:
>>> -   /**
>>> -* \brief Make a shallow copy of an ast_struct_specifier.
>>> -*
>>> -* Use only if the objects are allocated from the same context and will 
>>> not
>>> -* be modified. Zeros the inherited ast_node's fields.
>>> -*/
>>> -   ast_struct_specifier(const ast_struct_specifier& that):
>>> -  ast_node(), name(that.name), declarations(that.declarations),
>>> -  is_declaration(that.is_declaration)
>>> -   {
>>> -  /* empty */
>>> -   }
>>> -
>>
>> Not sure what the style in mesa is, but IME, C++ copy constructors can
>> spring up on you at fairly unexpected times, esp implicit ones. It
>> will also happily generate a default one for you, which is almost
>> never what you want. The style I've usually seen is to create private
>> copy constructors without definition to catch such situations.
>
> That's true, but they were added for usage in one place and that one
> place has been removed, and we've never needed it anywhere else.

Exactly. That's why I'm suggesting making a private version without
definition and leaving it in. That way accidental uses won't be added,
which tends to be easy to do due to implicit copy constructors.

  -ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] glsl: Remove unused ast copy constructors.

2014-11-21 Thread Matt Turner
On Fri, Nov 21, 2014 at 10:52 AM, Ilia Mirkin  wrote:
> On Fri, Nov 21, 2014 at 1:47 PM, Matt Turner  wrote:
>> These were added in commits a760c738 and 43757135 to be used in
>> implementing C-style aggregate initializers (commit 1b0d6aef). Paul
>> rewrote that code in commit 0da1a2cc to use GLSL types, rather than
>> AST types, leaving these copy constructors unused.
>>
>> Tested by making them private and providing no definition.
>> ---
>>  src/glsl/ast.h | 29 -
>>  1 file changed, 29 deletions(-)
>>
>> diff --git a/src/glsl/ast.h b/src/glsl/ast.h
>> index 15bf086..6995ae8 100644
>> --- a/src/glsl/ast.h
>> +++ b/src/glsl/ast.h
>> @@ -640,19 +640,6 @@ class ast_declarator_list;
>>
>>  class ast_struct_specifier : public ast_node {
>>  public:
>> -   /**
>> -* \brief Make a shallow copy of an ast_struct_specifier.
>> -*
>> -* Use only if the objects are allocated from the same context and will 
>> not
>> -* be modified. Zeros the inherited ast_node's fields.
>> -*/
>> -   ast_struct_specifier(const ast_struct_specifier& that):
>> -  ast_node(), name(that.name), declarations(that.declarations),
>> -  is_declaration(that.is_declaration)
>> -   {
>> -  /* empty */
>> -   }
>> -
>
> Not sure what the style in mesa is, but IME, C++ copy constructors can
> spring up on you at fairly unexpected times, esp implicit ones. It
> will also happily generate a default one for you, which is almost
> never what you want. The style I've usually seen is to create private
> copy constructors without definition to catch such situations.

That's true, but they were added for usage in one place and that one
place has been removed, and we've never needed it anywhere else.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 02/29] mesa: Set normalized=true for float array formats.

2014-11-21 Thread Jason Ekstrand
On Thu, Nov 20, 2014 at 11:33 PM, Iago Toral  wrote:

> On Thu, 2014-11-20 at 10:40 -0800, Jason Ekstrand wrote:
> >
> >
> > On Wed, Nov 19, 2014 at 11:24 PM, Iago Toral 
> > wrote:
> > Hi Jason,
> >
> > we discussed this some weeks ago actually, the detailed
> > explanation is
> > here:
> > https://bugs.freedesktop.org/show_bug.cgi?id=84566#c5
> >
> > the short answer is that this is necessary because there is a
> > normalized
> > parameter to _mesa_swizzle_and_convert, and when we deal with
> > float
> > types we want to set this to true.
> >
> >
> > I went back and looked at that and I thought the result of the
> > discussion was to fix the assert in mesa_format_convert and compute
> > the normalized parameter correctly.  After that, I thought this
> > shouldn't be strictly needed.  It may still be a good idea for
> > consistency, but I want to make sure we're doing the right thing in
> > mesa_format_convert
>
> With this patch, in mesa_format_convert we simply take the "normalized"
> value for mesa_swizzle_and_convert from the normalized field of the
> array format, since we make sure that all float array formats will have
> this set to 1.
>
> Without this patch we would have to do something like this (pseudocode)
> in mesa_format_convert:
>
> normalized = array_format.normalized || array_format.type == FLOAT ||
>  array_format.type == HALF_FLOAT;
>
> We can do it either way, I just think that the latter is a bit
> inconsistent because:
>
> a) why would we want to generate array formats with a normalized setting
> of 0 if we then want to set normalized to true when they are involved?.
>
> b) Other parts of Mesa check if a format is normalized by doing
> normalized = !_mesa_is_enum_format_integer(srcFormat), which will make
> float types normalized.
>
> Iago
>

That's fine.  I just wanted to make sure that this wasn't hiding some
fundamental breakage of the mesa_format_convert function.
--Jason


>
> > Iago
> >
> > On Wed, 2014-11-19 at 11:31 -0800, Jason Ekstrand wrote:
> > > I'm not sure what I think about this.  Does it make a
> > functional
> > > change other than consistancy?
> > >
> > > --Jason
> > >
> > >
> > > On Tue, Nov 18, 2014 at 1:23 AM, Iago Toral Quiroga
> > >  wrote:
> > > In order to check if a format is normalized Mesa
> > does
> > > something like this:
> > > normalized = !
> > _mesa_is_enum_format_integer(srcFormat);
> > >
> > > So all float types will set normalized to true.
> > Since our
> > > mesa_array_format
> > > includes a normalized flag for each type we want to
> > make it
> > > consistent with
> > > this.
> > > ---
> > >  src/mesa/main/format_info.py | 3 ++-
> > >  src/mesa/main/format_utils.c | 2 +-
> > >  2 files changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/src/mesa/main/format_info.py
> > > b/src/mesa/main/format_info.py
> > > index 315767d..d4bc276 100644
> > > --- a/src/mesa/main/format_info.py
> > > +++ b/src/mesa/main/format_info.py
> > > @@ -220,9 +220,10 @@ for fmat in formats:
> > > print '  {{ {0} }},'.format(',
> > '.join(map(str,
> > > fmat.swizzle)))
> > > if fmat.is_array() and fmat.colorspace in
> > ('rgb', 'srgb'):
> > >chan = fmat.array_element()
> > > +  norm = chan.norm or chan.type == parser.FLOAT
> > >print '   {0} ,'.format(',
> > '.join([
> > >   get_array_format_datatype(chan),
> > > - str(int(chan.norm)),
> > > + str(int(norm)),
> > >   str(len(fmat.channels)),
> > >   str(fmat.swizzle[0]),
> > >   str(fmat.swizzle[1]),
> > > diff --git a/src/mesa/main/format_utils.c
> > > b/src/mesa/main/format_utils.c
> > > index c3815cb..1d65f2b 100644
> > > --- a/src/mesa/main/format_utils.c
> > > +++ b/src/mesa/main/format_utils.c
> > > @@ -30,7 +30,7 @@
> > >
> > >  mesa_array_format RGBA_FLOAT = {{
> > > MESA_ARRAY_FORMAT_TYPE_FLOAT,
> > > -   0,
> > > +   1,
> > > 4,
> > > 0, 1, 2, 3,
> > > 0, 1
> > > --
> > > 1.9.1
> > >
> > > __

Re: [Mesa-dev] [PATCH] glsl: Remove unused ast copy constructors.

2014-11-21 Thread Ilia Mirkin
On Fri, Nov 21, 2014 at 1:47 PM, Matt Turner  wrote:
> These were added in commits a760c738 and 43757135 to be used in
> implementing C-style aggregate initializers (commit 1b0d6aef). Paul
> rewrote that code in commit 0da1a2cc to use GLSL types, rather than
> AST types, leaving these copy constructors unused.
>
> Tested by making them private and providing no definition.
> ---
>  src/glsl/ast.h | 29 -
>  1 file changed, 29 deletions(-)
>
> diff --git a/src/glsl/ast.h b/src/glsl/ast.h
> index 15bf086..6995ae8 100644
> --- a/src/glsl/ast.h
> +++ b/src/glsl/ast.h
> @@ -640,19 +640,6 @@ class ast_declarator_list;
>
>  class ast_struct_specifier : public ast_node {
>  public:
> -   /**
> -* \brief Make a shallow copy of an ast_struct_specifier.
> -*
> -* Use only if the objects are allocated from the same context and will 
> not
> -* be modified. Zeros the inherited ast_node's fields.
> -*/
> -   ast_struct_specifier(const ast_struct_specifier& that):
> -  ast_node(), name(that.name), declarations(that.declarations),
> -  is_declaration(that.is_declaration)
> -   {
> -  /* empty */
> -   }
> -

Not sure what the style in mesa is, but IME, C++ copy constructors can
spring up on you at fairly unexpected times, esp implicit ones. It
will also happily generate a default one for you, which is almost
never what you want. The style I've usually seen is to create private
copy constructors without definition to catch such situations.

> ast_struct_specifier(const char *identifier,
> ast_declarator_list *declarator_list);
> virtual void print(void) const;
> @@ -670,22 +657,6 @@ public:
>
>  class ast_type_specifier : public ast_node {
>  public:
> -   /**
> -* \brief Make a shallow copy of an ast_type_specifier, specifying array
> -*fields.
> -*
> -* Use only if the objects are allocated from the same context and will 
> not
> -* be modified. Zeros the inherited ast_node's fields.
> -*/
> -   ast_type_specifier(const ast_type_specifier *that,
> -  ast_array_specifier *array_specifier)
> -  : ast_node(), type_name(that->type_name), structure(that->structure),
> -array_specifier(array_specifier),
> -default_precision(that->default_precision)
> -   {
> -  /* empty */
> -   }

This one, of course, can't be generated implicitly, so I don't see any
issue in just removing it.

> -
> /** Construct a type specifier from a type name */
> ast_type_specifier(const char *name)
>: type_name(name), structure(NULL), array_specifier(NULL),
> --
> 2.0.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] [PATCH 2/2] i965/gen8: Handle the MUL dest hazard exception

2014-11-21 Thread Ben Widawsky
On Fri, Nov 21, 2014 at 10:50:31AM -0800, Ben Widawsky wrote:
> Fix one of the few cases where we can't reliable touch the destination hazard
> bits. I am explicitly doing this patch individually so it is easy to 
> backport. I
> was tempted to do this patch before the previous patch which reorganized the
> code, but I believe even doing that first, this is still easy to backport.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86268
> Signed-off-by: Ben Widawsky 

This is the wrong bug link. it should be 84212. Fixed locally.


> ---
>  src/mesa/drivers/dri/i965/brw_vec4.cpp | 20 ++--
>  src/mesa/drivers/dri/i965/brw_vec4.h   |  1 +
>  2 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp 
> b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index 0c2bbe9..d583f27 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -841,9 +841,25 @@ vec4_visitor::move_push_constants_to_pull_constants()
>  }
>  
>  /* Conditions for which we want to avoid setting the dependency control bits 
> */
> -static bool
> -is_dep_ctrl_safe(vec4_instruction *inst)
> +bool
> +vec4_visitor::is_dep_ctrl_safe(vec4_instruction *inst)
>  {
> +#define IS_DWORD(reg) \
> +   (reg.type == BRW_REGISTER_TYPE_UD || \
> +reg.type == BRW_REGISTER_TYPE_D)
> +
> +   /* From the destination hazard section of the spec:
> +* > Instructions other than send, may use this control as long as 
> operations
> +* > that have different pipeline latencies are not mixed.
> +*/
> +   if (brw->gen >= 8) {
> +  if (inst->opcode == BRW_OPCODE_MUL &&
> + IS_DWORD(inst->src[0]) &&
> + IS_DWORD(inst->src[1]))
> + return false;
> +   }
> +#undef IS_DWORD
> +
> /*
>  * In the presence of send messages, totally interrupt dependency
>  * control. They're long enough that the chance of dependency
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h 
> b/src/mesa/drivers/dri/i965/brw_vec4.h
> index 8e7dfe1..81bb06c 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.h
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.h
> @@ -393,6 +393,7 @@ public:
> bool opt_cse();
> bool opt_algebraic();
> bool opt_register_coalesce();
> +   bool is_dep_ctrl_safe(vec4_instruction *inst);
> void opt_set_dependency_control();
> void opt_schedule_instructions();
>  
> -- 
> 2.1.3
> 

-- 
Ben Widawsky, Intel Open Source Technology Center
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 1/2] i965: Extract depctrl hazards

2014-11-21 Thread Ben Widawsky
Move this to a separate function so that we can begin to add other little
caveats without making too big a mess.

NOTE: There is some desire to improve this function eventually, but we need to
fix a bug first.

Signed-off-by: Ben Widawsky 
---
 src/mesa/drivers/dri/i965/brw_vec4.cpp | 42 --
 1 file changed, 20 insertions(+), 22 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4.cpp
index df589b8..0c2bbe9 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
@@ -840,6 +840,25 @@ vec4_visitor::move_push_constants_to_pull_constants()
pack_uniform_registers();
 }
 
+/* Conditions for which we want to avoid setting the dependency control bits */
+static bool
+is_dep_ctrl_safe(vec4_instruction *inst)
+{
+   /*
+* In the presence of send messages, totally interrupt dependency
+* control. They're long enough that the chance of dependency
+* control around them just doesn't matter.
+*
+* It looks like setting dependency control on a predicated
+* instruction hangs the GPU.
+* NB: I can find no evidence of this in modern specs.
+*
+* Dependency control does not work well over math instructions.
+* NB: I can find no evidence of this in modern specs.
+*/
+   return !(inst->mlen || inst->predicate || inst->is_math());
+}
+
 /**
  * Sets the dependency control fields on instructions after register
  * allocation and before the generator is run.
@@ -885,28 +904,7 @@ vec4_visitor::opt_set_dependency_control()
 assert(inst->src[i].file != MRF);
  }
 
- /* In the presence of send messages, totally interrupt dependency
-  * control.  They're long enough that the chance of dependency
-  * control around them just doesn't matter.
-  */
- if (inst->mlen) {
-memset(last_grf_write, 0, sizeof(last_grf_write));
-memset(last_mrf_write, 0, sizeof(last_mrf_write));
-continue;
- }
-
- /* It looks like setting dependency control on a predicated
-  * instruction hangs the GPU.
-  */
- if (inst->predicate) {
-memset(last_grf_write, 0, sizeof(last_grf_write));
-memset(last_mrf_write, 0, sizeof(last_mrf_write));
-continue;
- }
-
- /* Dependency control does not work well over math instructions.
-  */
- if (inst->is_math()) {
+ if (!is_dep_ctrl_safe(inst)) {
 memset(last_grf_write, 0, sizeof(last_grf_write));
 memset(last_mrf_write, 0, sizeof(last_mrf_write));
 continue;
-- 
2.1.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 2/2] i965/gen8: Handle the MUL dest hazard exception

2014-11-21 Thread Ben Widawsky
Fix one of the few cases where we can't reliable touch the destination hazard
bits. I am explicitly doing this patch individually so it is easy to backport. I
was tempted to do this patch before the previous patch which reorganized the
code, but I believe even doing that first, this is still easy to backport.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86268
Signed-off-by: Ben Widawsky 
---
 src/mesa/drivers/dri/i965/brw_vec4.cpp | 20 ++--
 src/mesa/drivers/dri/i965/brw_vec4.h   |  1 +
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4.cpp
index 0c2bbe9..d583f27 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
@@ -841,9 +841,25 @@ vec4_visitor::move_push_constants_to_pull_constants()
 }
 
 /* Conditions for which we want to avoid setting the dependency control bits */
-static bool
-is_dep_ctrl_safe(vec4_instruction *inst)
+bool
+vec4_visitor::is_dep_ctrl_safe(vec4_instruction *inst)
 {
+#define IS_DWORD(reg) \
+   (reg.type == BRW_REGISTER_TYPE_UD || \
+reg.type == BRW_REGISTER_TYPE_D)
+
+   /* From the destination hazard section of the spec:
+* > Instructions other than send, may use this control as long as 
operations
+* > that have different pipeline latencies are not mixed.
+*/
+   if (brw->gen >= 8) {
+  if (inst->opcode == BRW_OPCODE_MUL &&
+ IS_DWORD(inst->src[0]) &&
+ IS_DWORD(inst->src[1]))
+ return false;
+   }
+#undef IS_DWORD
+
/*
 * In the presence of send messages, totally interrupt dependency
 * control. They're long enough that the chance of dependency
diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h 
b/src/mesa/drivers/dri/i965/brw_vec4.h
index 8e7dfe1..81bb06c 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4.h
+++ b/src/mesa/drivers/dri/i965/brw_vec4.h
@@ -393,6 +393,7 @@ public:
bool opt_cse();
bool opt_algebraic();
bool opt_register_coalesce();
+   bool is_dep_ctrl_safe(vec4_instruction *inst);
void opt_set_dependency_control();
void opt_schedule_instructions();
 
-- 
2.1.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] glsl: Remove unused ast copy constructors.

2014-11-21 Thread Matt Turner
These were added in commits a760c738 and 43757135 to be used in
implementing C-style aggregate initializers (commit 1b0d6aef). Paul
rewrote that code in commit 0da1a2cc to use GLSL types, rather than
AST types, leaving these copy constructors unused.

Tested by making them private and providing no definition.
---
 src/glsl/ast.h | 29 -
 1 file changed, 29 deletions(-)

diff --git a/src/glsl/ast.h b/src/glsl/ast.h
index 15bf086..6995ae8 100644
--- a/src/glsl/ast.h
+++ b/src/glsl/ast.h
@@ -640,19 +640,6 @@ class ast_declarator_list;
 
 class ast_struct_specifier : public ast_node {
 public:
-   /**
-* \brief Make a shallow copy of an ast_struct_specifier.
-*
-* Use only if the objects are allocated from the same context and will not
-* be modified. Zeros the inherited ast_node's fields.
-*/
-   ast_struct_specifier(const ast_struct_specifier& that):
-  ast_node(), name(that.name), declarations(that.declarations),
-  is_declaration(that.is_declaration)
-   {
-  /* empty */
-   }
-
ast_struct_specifier(const char *identifier,
ast_declarator_list *declarator_list);
virtual void print(void) const;
@@ -670,22 +657,6 @@ public:
 
 class ast_type_specifier : public ast_node {
 public:
-   /**
-* \brief Make a shallow copy of an ast_type_specifier, specifying array
-*fields.
-*
-* Use only if the objects are allocated from the same context and will not
-* be modified. Zeros the inherited ast_node's fields.
-*/
-   ast_type_specifier(const ast_type_specifier *that,
-  ast_array_specifier *array_specifier)
-  : ast_node(), type_name(that->type_name), structure(that->structure),
-array_specifier(array_specifier),
-default_precision(that->default_precision)
-   {
-  /* empty */
-   }
-
/** Construct a type specifier from a type name */
ast_type_specifier(const char *name) 
   : type_name(name), structure(NULL), array_specifier(NULL),
-- 
2.0.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 3/3] glapi: Remove dead mesadef.py.

2014-11-21 Thread Matt Turner
On Fri, Nov 21, 2014 at 10:42 AM, Ilia Mirkin  wrote:
> What about src/mesa/drivers/windows/gdi/mesa.def -- it claims to be
> generated by mesadef.py. I have no idea what it is, but it should
> either also be deleted, or the mesadef.py script kept around.

Good point. Maybe Brian knows (Cc'd).
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/3] glapi: Remove dead extension_helper.py.

2014-11-21 Thread Matt Turner
On Fri, Nov 21, 2014 at 10:40 AM, Ilia Mirkin  wrote:
> On Fri, Nov 21, 2014 at 1:35 PM, Matt Turner  wrote:
>> Dead since commit 3d16088f.
>> ---
>>  src/mapi/glapi/gen/Makefile.am |   2 -
>>  src/mapi/glapi/gen/extension_helper.py | 324 
>> -
>>  2 files changed, 326 deletions(-)
>>  delete mode 100644 src/mapi/glapi/gen/extension_helper.py
>>
>> diff --git a/src/mapi/glapi/gen/Makefile.am b/src/mapi/glapi/gen/Makefile.am
>> index 72e5095..9c70b26 100644
>> --- a/src/mapi/glapi/gen/Makefile.am
>> +++ b/src/mapi/glapi/gen/Makefile.am
>> @@ -61,12 +61,10 @@ EXTRA_DIST= \
>> $(MESA_GLAPI_DIR)/glapi_x86-64.S \
>> $(MESA_GLAPI_DIR)/glapi_sparc.S \
>> $(COMMON_GLX) \
>> -   extension_helper.py \
>> gl_apitemp.py \
>> gl_enums.py \
>> gl_genexec.py \
>> gl_gentable.py \
>> -   gl_offsets.py \
>
> In an ideal world, this would be part of the next commit...

Oh, it should be. Thanks, I'll move it.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 3/3] glapi: Remove dead mesadef.py.

2014-11-21 Thread Ilia Mirkin
What about src/mesa/drivers/windows/gdi/mesa.def -- it claims to be
generated by mesadef.py. I have no idea what it is, but it should
either also be deleted, or the mesadef.py script kept around.

On Fri, Nov 21, 2014 at 1:35 PM, Matt Turner  wrote:
> Dead since commit 4e120c97, in which apiparser (which mesadef.py imports)
> was removed.
> ---
>  src/mapi/glapi/gen/Makefile.am |   1 -
>  src/mapi/glapi/gen/mesadef.py  | 215 
> -
>  2 files changed, 216 deletions(-)
>  delete mode 100644 src/mapi/glapi/gen/mesadef.py
>
> diff --git a/src/mapi/glapi/gen/Makefile.am b/src/mapi/glapi/gen/Makefile.am
> index 9c70b26..7f76f19 100644
> --- a/src/mapi/glapi/gen/Makefile.am
> +++ b/src/mapi/glapi/gen/Makefile.am
> @@ -74,7 +74,6 @@ EXTRA_DIST= \
> glX_proto_send.py \
> glX_proto_size.py \
> glX_server_table.py \
> -   mesadef.py \
> remap_helper.py \
> gl_API.dtd
>
> diff --git a/src/mapi/glapi/gen/mesadef.py b/src/mapi/glapi/gen/mesadef.py
> deleted file mode 100644
> index 77cc4a3..000
> --- a/src/mapi/glapi/gen/mesadef.py
> +++ /dev/null
> @@ -1,215 +0,0 @@
> -#!/usr/bin/env python
> -
> -
> -# Mesa 3-D graphics library
> -#
> -# Copyright (C) 1999-2001  Brian Paul   All Rights Reserved.
> -#
> -# Permission is hereby granted, free of charge, to any person obtaining a
> -# copy of this software and associated documentation files (the "Software"),
> -# to deal in the Software without restriction, including without limitation
> -# the rights to use, copy, modify, merge, publish, distribute, sublicense,
> -# and/or sell copies of the Software, and to permit persons to whom the
> -# Software is furnished to do so, subject to the following conditions:
> -#
> -# The above copyright notice and this permission notice shall be included
> -# in all copies or substantial portions of the Software.
> -#
> -# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
> -# OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> -# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> -# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> -# OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> -# ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> -# OTHER DEALINGS IN THE SOFTWARE.
> -
> -
> -# Generate the mesa.def file for Windows.
> -#
> -# Usage:
> -#mesadef.py >mesa.def
> -#Then copy to src/mesa/drivers/windows/gdi
> -#
> -# Dependencies:
> -#The apispec file must be in the current directory.
> -
> -
> -
> -import apiparser
> -import string
> -
> -
> -def PrintHead():
> -print '; DO NOT EDIT - This file generated automatically by mesadef.py 
> script'
> -print 'DESCRIPTION \'Mesa (OpenGL work-alike) for Win32\''
> -print 'VERSION 6.0'
> -print ';'
> -print '; Module definition file for Mesa (OPENGL32.DLL)'
> -print ';'
> -print '; Note: The OpenGL functions use the STDCALL'
> -print '; function calling convention.  Microsoft\'s'
> -print '; OPENGL32 uses this convention and so must the'
> -print '; Mesa OPENGL32 so that the Mesa DLL can be used'
> -print '; as a drop-in replacement.'
> -print ';'
> -print '; The linker exports STDCALL entry points with'
> -print '; \'decorated\' names; e.g., _glBegin@0, where the'
> -print '; trailing number is the number of bytes of '
> -print '; parameter data pushed onto the stack.  The'
> -print '; callee is responsible for popping this data'
> -print '; off the stack, usually via a RETF n instruction.'
> -print ';'
> -print '; However, the Microsoft OPENGL32.DLL does not export'
> -print '; the decorated names, even though the calling convention'
> -print '; is STDCALL.  So, this module definition file is'
> -print '; needed to force the Mesa OPENGL32.DLL to export the'
> -print '; symbols in the same manner as the Microsoft DLL.'
> -print '; Were it not for this problem, this file would not'
> -print '; be needed (for the gl* functions) since the entry'
> -print '; points are compiled with dllexport declspec.'
> -print ';'
> -print '; However, this file is still needed to export "internal"'
> -print '; Mesa symbols for the benefit of the OSMESA32.DLL.'
> -print ';'
> -print 'EXPORTS'
> -return
> -#enddef
> -
> -
> -def PrintTail():
> -print ';'
> -print '; WGL API'
> -print '\twglChoosePixelFormat'
> -print '\twglCopyContext'
> -print '\twglCreateContext'
> -print '\twglCreateLayerContext'
> -print '\twglDeleteContext'
> -print '\twglDescribeLayerPlane'
> -print '\twglDescribePixelFormat'
> -print '\twglGetCurrentContext'
> -print '\twglGetCurrentDC'
> -print '\twglGetExtensionsStringARB'
> -print '\twglGetLayerPaletteEntries'
> -print '\twglGetPixelFormat'
> -print '\twglGetProcAddre

Re: [Mesa-dev] [PATCH 1/3] glapi: Remove dead extension_helper.py.

2014-11-21 Thread Ilia Mirkin
On Fri, Nov 21, 2014 at 1:35 PM, Matt Turner  wrote:
> Dead since commit 3d16088f.
> ---
>  src/mapi/glapi/gen/Makefile.am |   2 -
>  src/mapi/glapi/gen/extension_helper.py | 324 
> -
>  2 files changed, 326 deletions(-)
>  delete mode 100644 src/mapi/glapi/gen/extension_helper.py
>
> diff --git a/src/mapi/glapi/gen/Makefile.am b/src/mapi/glapi/gen/Makefile.am
> index 72e5095..9c70b26 100644
> --- a/src/mapi/glapi/gen/Makefile.am
> +++ b/src/mapi/glapi/gen/Makefile.am
> @@ -61,12 +61,10 @@ EXTRA_DIST= \
> $(MESA_GLAPI_DIR)/glapi_x86-64.S \
> $(MESA_GLAPI_DIR)/glapi_sparc.S \
> $(COMMON_GLX) \
> -   extension_helper.py \
> gl_apitemp.py \
> gl_enums.py \
> gl_genexec.py \
> gl_gentable.py \
> -   gl_offsets.py \

In an ideal world, this would be part of the next commit...

> gl_procs.py \
> gl_SPARC_asm.py \
> gl_table.py \
> diff --git a/src/mapi/glapi/gen/extension_helper.py 
> b/src/mapi/glapi/gen/extension_helper.py
> deleted file mode 100644
> index da633dc..000
> --- a/src/mapi/glapi/gen/extension_helper.py
> +++ /dev/null
> @@ -1,324 +0,0 @@
> -#!/usr/bin/env python
> -
> -# (C) Copyright IBM Corporation 2005
> -# All Rights Reserved.
> -#
> -# Permission is hereby granted, free of charge, to any person obtaining a
> -# copy of this software and associated documentation files (the "Software"),
> -# to deal in the Software without restriction, including without limitation
> -# on the rights to use, copy, modify, merge, publish, distribute, sub
> -# license, and/or sell copies of the Software, and to permit persons to whom
> -# the Software is furnished to do so, subject to the following conditions:
> -#
> -# The above copyright notice and this permission notice (including the next
> -# paragraph) shall be included in all copies or substantial portions of the
> -# Software.
> -#
> -# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> -# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> -# FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.  IN NO EVENT SHALL
> -# IBM AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> -# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> -# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> DEALINGS
> -# IN THE SOFTWARE.
> -#
> -# Authors:
> -#Ian Romanick 
> -
> -import gl_XML
> -import license
> -import sys, getopt, string
> -
> -vtxfmt = [
> -"ArrayElement", \
> -"Color3f", \
> -"Color3fv", \
> -"Color4f", \
> -"Color4fv", \
> -"EdgeFlag", \
> -"EdgeFlagv", \
> -"EvalCoord1f", \
> -"EvalCoord1fv", \
> -"EvalCoord2f", \
> -"EvalCoord2fv", \
> -"EvalPoint1", \
> -"EvalPoint2", \
> -"FogCoordfEXT", \
> -"FogCoordfvEXT", \
> -"Indexf", \
> -"Indexfv", \
> -"Materialfv", \
> -"MultiTexCoord1fARB", \
> -"MultiTexCoord1fvARB", \
> -"MultiTexCoord2fARB", \
> -"MultiTexCoord2fvARB", \
> -"MultiTexCoord3fARB", \
> -"MultiTexCoord3fvARB", \
> -"MultiTexCoord4fARB", \
> -"MultiTexCoord4fvARB", \
> -"Normal3f", \
> -"Normal3fv", \
> -"SecondaryColor3fEXT", \
> -"SecondaryColor3fvEXT", \
> -"TexCoord1f", \
> -"TexCoord1fv", \
> -"TexCoord2f", \
> -"TexCoord2fv", \
> -"TexCoord3f", \
> -"TexCoord3fv", \
> -"TexCoord4f", \
> -"TexCoord4fv", \
> -"Vertex2f", \
> -"Vertex2fv", \
> -"Vertex3f", \
> -"Vertex3fv", \
> -"Vertex4f", \
> -"Vertex4fv", \
> -"CallList", \
> -"CallLists", \
> -"Begin", \
> -"End", \
> -"VertexAttrib1fNV", \
> -"VertexAttrib1fvNV", \
> -"VertexAttrib2fNV", \
> -"VertexAttrib2fvNV", \
> -"VertexAttrib3fNV", \
> -"VertexAttrib3fvNV", \
> -"VertexAttrib4fNV", \
> -"VertexAttrib4fvNV", \
> -"VertexAttrib1fARB", \
> -"VertexAttrib1fvARB", \
> -"VertexAttrib2fARB", \
> -"VertexAttrib2fvARB", \
> -"VertexAttrib3fARB", \
> -"VertexAttrib3fvARB", \
> -"VertexAttrib4fARB", \
> -"VertexAttrib4fvARB", \
> -"Rectf", \
> -"DrawArrays", \
> -"DrawElements", \
> -"DrawRangeElements", \
> -"EvalMesh1", \
> -"EvalMesh2", \
> -]
> -
> -def all_entrypoints_in_abi(f, abi, api):
> -for n in f.entry_points:
> -[category, num] = api.get_category_for_name( n )
> -if category not in abi:
> -return 0
> -
> -return 1
> -
> -
> -def any_entrypoints_in_abi(f, abi, api):
> -for n in f.entry_points:
> -[category, num] = api.get_category_for_name( n )
> -if category in abi:
> -return 1
> -
> -return 0
> -
> -
> -def condition_for_function(f, abi, all_not_in_ABI):
> -"""Create a C-preprocessor condition for the function.
> -
> -There are two modes of operation.  If all_not_in_ABI is set, a
> -co

[Mesa-dev] [PATCH 3/3] glapi: Remove dead mesadef.py.

2014-11-21 Thread Matt Turner
Dead since commit 4e120c97, in which apiparser (which mesadef.py imports)
was removed.
---
 src/mapi/glapi/gen/Makefile.am |   1 -
 src/mapi/glapi/gen/mesadef.py  | 215 -
 2 files changed, 216 deletions(-)
 delete mode 100644 src/mapi/glapi/gen/mesadef.py

diff --git a/src/mapi/glapi/gen/Makefile.am b/src/mapi/glapi/gen/Makefile.am
index 9c70b26..7f76f19 100644
--- a/src/mapi/glapi/gen/Makefile.am
+++ b/src/mapi/glapi/gen/Makefile.am
@@ -74,7 +74,6 @@ EXTRA_DIST= \
glX_proto_send.py \
glX_proto_size.py \
glX_server_table.py \
-   mesadef.py \
remap_helper.py \
gl_API.dtd
 
diff --git a/src/mapi/glapi/gen/mesadef.py b/src/mapi/glapi/gen/mesadef.py
deleted file mode 100644
index 77cc4a3..000
--- a/src/mapi/glapi/gen/mesadef.py
+++ /dev/null
@@ -1,215 +0,0 @@
-#!/usr/bin/env python
-
-
-# Mesa 3-D graphics library
-# 
-# Copyright (C) 1999-2001  Brian Paul   All Rights Reserved.
-# 
-# Permission is hereby granted, free of charge, to any person obtaining a
-# copy of this software and associated documentation files (the "Software"),
-# to deal in the Software without restriction, including without limitation
-# the rights to use, copy, modify, merge, publish, distribute, sublicense,
-# and/or sell copies of the Software, and to permit persons to whom the
-# Software is furnished to do so, subject to the following conditions:
-# 
-# The above copyright notice and this permission notice shall be included
-# in all copies or substantial portions of the Software.
-# 
-# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
-# OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
-# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
-# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
-# OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
-# ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
-# OTHER DEALINGS IN THE SOFTWARE.
-
-
-# Generate the mesa.def file for Windows.
-#
-# Usage:
-#mesadef.py >mesa.def
-#Then copy to src/mesa/drivers/windows/gdi
-#
-# Dependencies:
-#The apispec file must be in the current directory.
-
-
-
-import apiparser
-import string
-
-
-def PrintHead():
-print '; DO NOT EDIT - This file generated automatically by mesadef.py 
script'
-print 'DESCRIPTION \'Mesa (OpenGL work-alike) for Win32\''
-print 'VERSION 6.0'
-print ';'
-print '; Module definition file for Mesa (OPENGL32.DLL)'
-print ';'
-print '; Note: The OpenGL functions use the STDCALL'
-print '; function calling convention.  Microsoft\'s'
-print '; OPENGL32 uses this convention and so must the'
-print '; Mesa OPENGL32 so that the Mesa DLL can be used'
-print '; as a drop-in replacement.'
-print ';'
-print '; The linker exports STDCALL entry points with'
-print '; \'decorated\' names; e.g., _glBegin@0, where the'
-print '; trailing number is the number of bytes of '
-print '; parameter data pushed onto the stack.  The'
-print '; callee is responsible for popping this data'
-print '; off the stack, usually via a RETF n instruction.'
-print ';'
-print '; However, the Microsoft OPENGL32.DLL does not export'
-print '; the decorated names, even though the calling convention'
-print '; is STDCALL.  So, this module definition file is'
-print '; needed to force the Mesa OPENGL32.DLL to export the'
-print '; symbols in the same manner as the Microsoft DLL.'
-print '; Were it not for this problem, this file would not'
-print '; be needed (for the gl* functions) since the entry'
-print '; points are compiled with dllexport declspec.'
-print ';'
-print '; However, this file is still needed to export "internal"'
-print '; Mesa symbols for the benefit of the OSMESA32.DLL.'
-print ';'
-print 'EXPORTS'
-return
-#enddef
-
-
-def PrintTail():
-print ';'
-print '; WGL API'
-print '\twglChoosePixelFormat'
-print '\twglCopyContext'
-print '\twglCreateContext'
-print '\twglCreateLayerContext'
-print '\twglDeleteContext'
-print '\twglDescribeLayerPlane'
-print '\twglDescribePixelFormat'
-print '\twglGetCurrentContext'
-print '\twglGetCurrentDC'
-print '\twglGetExtensionsStringARB'
-print '\twglGetLayerPaletteEntries'
-print '\twglGetPixelFormat'
-print '\twglGetProcAddress'
-print '\twglMakeCurrent'
-print '\twglRealizeLayerPalette'
-print '\twglSetLayerPaletteEntries'
-print '\twglSetPixelFormat'
-print '\twglShareLists'
-print '\twglSwapBuffers'
-print '\twglSwapLayerBuffers'
-print '\twglUseFontBitmapsA'
-print '\twglUseFontBitmapsW'
-print '\twglUseFontOutlinesA'
-print '\twglUseFontOutlinesW'
-print ';'
-print '; Mesa internals - mostly for OSMESA'
-print '\t_ac_CreateContext'
-print '

[Mesa-dev] [PATCH 2/3] glapi: Remove dead gl_offsets.py.

2014-11-21 Thread Matt Turner
Dead since commit 07b85457.
---
 src/mapi/glapi/gen/gl_offsets.py | 120 ---
 1 file changed, 120 deletions(-)
 delete mode 100644 src/mapi/glapi/gen/gl_offsets.py

diff --git a/src/mapi/glapi/gen/gl_offsets.py b/src/mapi/glapi/gen/gl_offsets.py
deleted file mode 100644
index 897ac18..000
--- a/src/mapi/glapi/gen/gl_offsets.py
+++ /dev/null
@@ -1,120 +0,0 @@
-#!/usr/bin/env python
-
-# (C) Copyright IBM Corporation 2004, 2005
-# All Rights Reserved.
-#
-# Permission is hereby granted, free of charge, to any person obtaining a
-# copy of this software and associated documentation files (the "Software"),
-# to deal in the Software without restriction, including without limitation
-# on the rights to use, copy, modify, merge, publish, distribute, sub
-# license, and/or sell copies of the Software, and to permit persons to whom
-# the Software is furnished to do so, subject to the following conditions:
-#
-# The above copyright notice and this permission notice (including the next
-# paragraph) shall be included in all copies or substantial portions of the
-# Software.
-#
-# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
-# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
-# FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.  IN NO EVENT SHALL
-# IBM AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
-# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
-# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
-# IN THE SOFTWARE.
-#
-# Authors:
-#Ian Romanick 
-
-import gl_XML
-import license
-import sys, getopt
-
-class PrintGlOffsets(gl_XML.gl_print_base):
-def __init__(self, es=False):
-gl_XML.gl_print_base.__init__(self)
-
-self.es = es
-self.name = "gl_offsets.py (from Mesa)"
-self.header_tag = '_GLAPI_OFFSETS_H_'
-self.license = license.bsd_license_template % ( \
-"""Copyright (C) 1999-2001  Brian Paul   All Rights Reserved.
-(C) Copyright IBM Corporation 2004""", "BRIAN PAUL, IBM")
-return
-
-def printBody(self, api):
-print '/* this file should not be included directly in mesa */'
-print ''
-
-functions = []
-abi_functions = []
-alias_functions = []
-count = 0
-for f in api.functionIterateByOffset():
-if not f.is_abi():
-functions.append( [f, count] )
-count += 1
-else:
-abi_functions.append( f )
-
-if self.es:
-# remember functions with aliases
-if len(f.entry_points) > 1:
-alias_functions.append(f)
-
-for f in abi_functions:
-print '#define _gloffset_%s %d' % (f.name, f.offset)
-last_static = f.offset
-
-print ''
-print '#if !defined(_GLAPI_USE_REMAP_TABLE)'
-print ''
-
-for [f, index] in functions:
-print '#define _gloffset_%s %d' % (f.name, f.offset)
-
-print '#define _gloffset_FIRST_DYNAMIC %d' % (api.next_offset)
-
-print ''
-print '#else'
-print ''
-
-for [f, index] in functions:
-print '#define _gloffset_%s driDispatchRemapTable[%s_remap_index]' 
% (f.name, f.name)
-
-print ''
-print '#endif /* !defined(_GLAPI_USE_REMAP_TABLE) */'
-
-if alias_functions:
-print ''
-print '/* define aliases for compatibility */'
-for f in alias_functions:
-for name in f.entry_points:
-if name != f.name:
-print '#define _gloffset_%s _gloffset_%s' % (name, 
f.name)
-return
-
-
-def show_usage():
-print "Usage: %s [-f input_file_name] [-c]" % sys.argv[0]
-print "-cEnable compatibility with OpenGL ES."
-sys.exit(1)
-
-if __name__ == '__main__':
-file_name = "gl_API.xml"
-
-try:
-(args, trail) = getopt.getopt(sys.argv[1:], "f:c")
-except Exception,e:
-show_usage()
-
-es = False
-for (arg,val) in args:
-if arg == "-f":
-file_name = val
-elif arg == "-c":
-es = True
-
-api = gl_XML.parse_GL_API( file_name )
-
-printer = PrintGlOffsets(es)
-printer.Print( api )
-- 
2.0.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 1/3] glapi: Remove dead extension_helper.py.

2014-11-21 Thread Matt Turner
Dead since commit 3d16088f.
---
 src/mapi/glapi/gen/Makefile.am |   2 -
 src/mapi/glapi/gen/extension_helper.py | 324 -
 2 files changed, 326 deletions(-)
 delete mode 100644 src/mapi/glapi/gen/extension_helper.py

diff --git a/src/mapi/glapi/gen/Makefile.am b/src/mapi/glapi/gen/Makefile.am
index 72e5095..9c70b26 100644
--- a/src/mapi/glapi/gen/Makefile.am
+++ b/src/mapi/glapi/gen/Makefile.am
@@ -61,12 +61,10 @@ EXTRA_DIST= \
$(MESA_GLAPI_DIR)/glapi_x86-64.S \
$(MESA_GLAPI_DIR)/glapi_sparc.S \
$(COMMON_GLX) \
-   extension_helper.py \
gl_apitemp.py \
gl_enums.py \
gl_genexec.py \
gl_gentable.py \
-   gl_offsets.py \
gl_procs.py \
gl_SPARC_asm.py \
gl_table.py \
diff --git a/src/mapi/glapi/gen/extension_helper.py 
b/src/mapi/glapi/gen/extension_helper.py
deleted file mode 100644
index da633dc..000
--- a/src/mapi/glapi/gen/extension_helper.py
+++ /dev/null
@@ -1,324 +0,0 @@
-#!/usr/bin/env python
-
-# (C) Copyright IBM Corporation 2005
-# All Rights Reserved.
-#
-# Permission is hereby granted, free of charge, to any person obtaining a
-# copy of this software and associated documentation files (the "Software"),
-# to deal in the Software without restriction, including without limitation
-# on the rights to use, copy, modify, merge, publish, distribute, sub
-# license, and/or sell copies of the Software, and to permit persons to whom
-# the Software is furnished to do so, subject to the following conditions:
-#
-# The above copyright notice and this permission notice (including the next
-# paragraph) shall be included in all copies or substantial portions of the
-# Software.
-#
-# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
-# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
-# FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.  IN NO EVENT SHALL
-# IBM AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
-# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
-# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
-# IN THE SOFTWARE.
-#
-# Authors:
-#Ian Romanick 
-
-import gl_XML
-import license
-import sys, getopt, string
-
-vtxfmt = [
-"ArrayElement", \
-"Color3f", \
-"Color3fv", \
-"Color4f", \
-"Color4fv", \
-"EdgeFlag", \
-"EdgeFlagv", \
-"EvalCoord1f", \
-"EvalCoord1fv", \
-"EvalCoord2f", \
-"EvalCoord2fv", \
-"EvalPoint1", \
-"EvalPoint2", \
-"FogCoordfEXT", \
-"FogCoordfvEXT", \
-"Indexf", \
-"Indexfv", \
-"Materialfv", \
-"MultiTexCoord1fARB", \
-"MultiTexCoord1fvARB", \
-"MultiTexCoord2fARB", \
-"MultiTexCoord2fvARB", \
-"MultiTexCoord3fARB", \
-"MultiTexCoord3fvARB", \
-"MultiTexCoord4fARB", \
-"MultiTexCoord4fvARB", \
-"Normal3f", \
-"Normal3fv", \
-"SecondaryColor3fEXT", \
-"SecondaryColor3fvEXT", \
-"TexCoord1f", \
-"TexCoord1fv", \
-"TexCoord2f", \
-"TexCoord2fv", \
-"TexCoord3f", \
-"TexCoord3fv", \
-"TexCoord4f", \
-"TexCoord4fv", \
-"Vertex2f", \
-"Vertex2fv", \
-"Vertex3f", \
-"Vertex3fv", \
-"Vertex4f", \
-"Vertex4fv", \
-"CallList", \
-"CallLists", \
-"Begin", \
-"End", \
-"VertexAttrib1fNV", \
-"VertexAttrib1fvNV", \
-"VertexAttrib2fNV", \
-"VertexAttrib2fvNV", \
-"VertexAttrib3fNV", \
-"VertexAttrib3fvNV", \
-"VertexAttrib4fNV", \
-"VertexAttrib4fvNV", \
-"VertexAttrib1fARB", \
-"VertexAttrib1fvARB", \
-"VertexAttrib2fARB", \
-"VertexAttrib2fvARB", \
-"VertexAttrib3fARB", \
-"VertexAttrib3fvARB", \
-"VertexAttrib4fARB", \
-"VertexAttrib4fvARB", \
-"Rectf", \
-"DrawArrays", \
-"DrawElements", \
-"DrawRangeElements", \
-"EvalMesh1", \
-"EvalMesh2", \
-]
-
-def all_entrypoints_in_abi(f, abi, api):
-for n in f.entry_points:
-[category, num] = api.get_category_for_name( n )
-if category not in abi:
-return 0
-
-return 1
-
-
-def any_entrypoints_in_abi(f, abi, api):
-for n in f.entry_points:
-[category, num] = api.get_category_for_name( n )
-if category in abi:
-return 1
-
-return 0
-
-
-def condition_for_function(f, abi, all_not_in_ABI):
-"""Create a C-preprocessor condition for the function.
-
-There are two modes of operation.  If all_not_in_ABI is set, a
-condition is only created is all of the entry-point names for f are
-not in the selected ABI.  If all_not_in_ABI is not set, a condition
-is created if any entryp-point name is not in the selected ABI.
-"""
-
-condition = []
-for n in f.entry_points:
-[category, num] = api.get_category_for_name( n )
-if category not in abi:
-condition.append( 'defined(need_%s)' % (gl_XML.real_category_na

Re: [Mesa-dev] [PATCH] i965/fs: Remove try_replace_with_sel().

2014-11-21 Thread Matt Turner
On Tue, Nov 11, 2014 at 9:41 AM, Matt Turner  wrote:
> The rest of our backend optimizations have replaced the need for this
> since it was written.
>
> instructions in affected programs: 30626 -> 30564 (-0.20%)
>
> Hurts a small number of CSGO shaders by one instruction, but helps even
> more. Hurts two by a larger number because of something I noticed when I
> first wrote the SEL peephole: try_replace_with_sel() operates on
> instructions before we've demoted uniforms to pull constants. So code
> like
>
>var.x = ( -abs(r6.w) >= 0.0 ) ? pc[82].x : r9.x;
>var.y = ( -abs(r6.w) >= 0.0 ) ? pc[82].y : r9.y;
>var.z = ( -abs(r6.w) >= 0.0 ) ? pc[82].z : r9.z;
>var.w = ( -abs(r6.w) >= 0.0 ) ? pc[82].w : r9.w;
>
> where pc[82] gets demoted to a pull constant, we end up emitting a
> send(4) instruction to load pc[82] each time, and since they're in
> different basic blocks because we mishandle the ternary operator in this
> case we can't combine them. Once we handle this common ternary pattern
> better the problem will go away.
> ---

Thoughts?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] i965: Make Gen4-5 push constants call _mesa_load_state_parameters too.

2014-11-21 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 00/20] Auto-generate pack/unpack functions

2014-11-21 Thread Samuel Iglesias Gonsálvez
On Wed, 2014-11-19 at 17:09 +0900, Michel Dänzer wrote:
> On 18.11.2014 17:43, Iago Toral Quiroga wrote:
> >
> > For software drivers we worked with a trimmed set of piglit tests (related 
> > to
> > format conversion), ~5700 tests selected with the following filter:
> >
> > -t format -t color -t tex -t image -t swizzle -t clamp -t rgb -t lum -t pix
> > -t fbo -t frame
> 
> Any particular reason for not testing at least piglit gpu.py with 
> llvmpipe? Last time I tried that a few months ago, it didn't take much 
> more than ten minutes on a quad-core A10-7850K.
> 
> 

gpu.py takes much more time on quad core i7-3610QM laptop with gallium
llvmpipe... After 6 hours waiting for piglit to finish, I started
killing the processes that took too much time. If I filter them from
the beginning, it takes about 10 minutes too.

Thanks!

Sam



signature.asc
Description: This is a digitally signed message part
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 03/29] mesa: Do not assert on integer<->non-integer direct pack/unpack fast paths

2014-11-21 Thread Iago Toral
On Thu, 2014-11-20 at 21:35 -0800, Jason Ekstrand wrote:
> 
> 
> On Thu, Nov 20, 2014 at 9:33 PM, Jason Ekstrand 
> wrote:
> 
> 
> On Thu, Nov 20, 2014 at 12:29 AM, Iago Toral
>  wrote:
> It is explained here:
> https://bugs.freedesktop.org/show_bug.cgi?id=84566#c35
> 
> So one example of this was a glReadPixels where we
> want to store the
> pixel data as RGBA UINT, but the render buffer format
> we  read from is
> MESA_FORMAT_B8G8R8A8_UNORM. There are piglit tests
> that hit this case.
> 
> 
> I'm still not seeing how this is allowed.  From the 4.2 core
> spec:
> 
> "If format is one of RED , GREEN , BLUE , RG , RGB , RGBA ,
> BGR , or BGRA , then
> red, green, blue, and alpha values are obtained from the
> selected buffer at each
> pixel location.
> If format is an integer format and the color buffer is not an
> integer format, or
> if the color buffer is an integer format and format is not an
> integer format, an
> INVALID_OPERATION error is generated."
> 
> 
> I also checked the 3.3 compatibility spec and it says the same
> thing.  This seems to indicate that that combination should
> result in GL_INVALID_OPERATION.
> 
> 
> 
> I also just CC'd Ian, our local spec expert.  Maybe he can shed a
> little light on this.

No need. I have reverted the commit and run piglit again on i965 and
swrast and I don't hit the assert any more, so I guess that when I was
hitting that it was because of a bug somewhere in the GL->Mesa type
mapping that I must have fixed after added this patch.

I'll remove the patch in the second version of the series.
>  
> 
> Iago
> 
> On Wed, 2014-11-19 at 12:04 -0800, Jason Ekstrand
> wrote:
> > Can you remind me again as to what formats hit these
> paths?  I
> > remember you hitting them, but I'm still not really
> seeing how it
> > happens.
> >
> > --Jason
> >
> >
> > On Tue, Nov 18, 2014 at 1:23 AM, Iago Toral Quiroga
> >  wrote:
> > We can have conversions from non-integer
> types to integer
> > types, so remove
> > the assertions for these in the pack/unpack
> fast paths. It
> > could be that
> > we do not have all the necessary pack/unpack
> functions in
> > these cases though,
> > so protect these paths with conditionals and
> let
> > _mesa_format_convert use
> > other paths to resolve these kind of
> conversions if necessary.
> > ---
> >  src/mesa/main/format_utils.c | 16
> 
> >  1 file changed, 8 insertions(+), 8
> deletions(-)
> >
> > diff --git a/src/mesa/main/format_utils.c
> > b/src/mesa/main/format_utils.c
> > index 1d65f2b..56a3b8d 100644
> > --- a/src/mesa/main/format_utils.c
> > +++ b/src/mesa/main/format_utils.c
> > @@ -143,8 +143,8 @@
> _mesa_format_convert(void *void_dst,
> > uint32_t dst_format, size_t dst_stride,
> >  dst += dst_stride;
> >   }
> >   return;
> > -  } else if (dst_array_format.as_uint
> ==
> > RGBA_UBYTE.as_uint) {
> > - assert(!
> _mesa_is_format_integer_color(src_format));
> > +  } else if (dst_array_format.as_uint
> ==
> > RGBA_UBYTE.as_uint &&
> > + !
> _mesa_is_format_integer_color(src_format))
> > {
> >   for (row = 0; row < height; ++row)
> {
> >
> _mesa_unpack_ubyte_rgba_row(src_format, width,
> >
> src, (uint8_t
> > (*)[4])dst);
> > @@ -152,8 +152,8 @@
> _mesa_format_convert(void *void_dst,
>

[Mesa-dev] [Bug 82585] geometry shader with optional out variable segfaults

2014-11-21 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=82585

--- Comment #8 from Kenneth Graunke  ---
This is a separate shader objects bug, and is probably related to 79783.  If
you modify the demo application to not use SSO, it works fine.

-- 
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 82585] geometry shader with optional out variable segfaults

2014-11-21 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=82585

Kenneth Graunke  changed:

   What|Removed |Added

  Component|Mesa core   |glsl-compiler
   Assignee|mesa-dev@lists.freedesktop. |i...@freedesktop.org
   |org |
 QA Contact||intel-3d-bugs@lists.freedes
   ||ktop.org

-- 
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] Require micro-benchmarks for performance optimization oriented patches

2014-11-21 Thread Timothy Arceri
On Thu, 2014-11-20 at 18:46 +0200, Eero Tamminen wrote:
> Hi,
> 
>  > Honestly, I think I'm okay with our usual metrics like:
>  > - Increased FPS in a game or benchmark
>  > - Reduced number of instructions or memory accesses in
>  a shader program
>  > - Reduced memory consumption
>  > - Significant cycle reduction in callgrind or better generated code
>  >   (ideally if it's a lot of code I'd like better justification)
> 
> Profiling tools like callgrind are means for analyzing, not for
> measuring.
> 
> The problem with profiler data is that the cost may have just
> been moved elsewhere, *and* grown:
> 
> * Kcachegrind visualization for valgrind/callgrind data shows call
>counts and relative performance.  If relative cost of a given
>function has decreased, that still doesn't tell anything about:
>- its absolute cost, i.e.
>- whether cost just moved somewhere else instead of total cost
>  really decreasing

Sure but Kcachegrind can group that relative performance into libraries
so as long as you don't move the cost into another library (e.g external
library calls) you can get an idea of if the function has really
improved.

> 
> * Callgrind reports instruction counts, not cycles.  While
>they're a good indicator, it doesn't necessarily tell about
>real performance (instruction count e.g. doesn't take into
>account data or instruction cache misses)
> 
> * Valgrind tracks only CPU utilization on the user-space.
>It doesn't notice increased CPU utilization at kernel side.
> 
> * Valgrind tracks only single process, it doesn't notice
>increased CPU utilization in other processes (in graphics
>perf, X server side is sometimes relevant)
> 
> * Valgrind doesn't track GPU utilization.   Change may have
>moved more load there.
> 
> -> Looking just at Callgrind data is NOT enough, there must
> also be some real measurement data.
> 

Sure there are no perfect tools for profiling, a similar list could be
created for the downfalls of using perf/oprofile.

I don't think anyone is suggesting Callgrind data should be blindly
accepted, but isn't that also the point of code review to be able to use
knowledge to decide if a change seems reasonable. This includes looking
at the generated code.

> 
> As to measurements...
> 
> Even large performance improvements fail to show up with
> the wrong benchmark.  One needs to know whether test-case
> performance is bound by what you were trying to optimize.
> 
> If there's no such case known, simplest may be just to write
> a micro-benchmark for the change (preferably two, one for best
> and one for the worst case).

I did some quick googling looks like there has been a couple of attempts
at an OpenGL open source benchmarking suite in the past [1][2] but
neither got very far. Maybe this would make a good student project to at
least setup the infrastructure to be built upon.


[1] http://globs.sourceforge.net/
[2] https://code.google.com/p/freedmark/

> 
> 
>  - Eero
> 
> PS. while analyzing memory usage isn't harder, measuring that
> is, because memory usage can be shared (both in user-space and
> on kernel buffers), and there's a large impact difference on
> whether memory is clean or dirty (unless your problem is running
> out of 32-bit address space when clean memory is as much of a problem).
> ___
> 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 V2] mesa: Permanently enable features supported by target CPU at compile time.

2014-11-21 Thread Siavash Eliasi


On 11/10/2014 04:28 AM, Emil Velikov wrote:
I'm not sure did you just said that you've checked it, or that's what 
it ought to do ? There is a reason why I'm so picky - this bizarre (as 
one might call it) setup is just the tip of the iceberg when it comes 
to people building mesa themselves. Would be nice to get your patch in 
as long as it does not break stuff :) -Emil


Hello Emil, David Heidelberg kindly tested the patch on his machines and 
it was okay. Please consider pushing the patch upstream if it looks good 
to you guys too.


Best regards,
Siavash Eliasi.



On 11/15/2014 08:58 PM, David Heidelberg wrote:

Hello,

I tested it directly on TK-55 laptop, crosscompiled (from my AMD 
A3870 to TK-55) and on my computer (A3870).


Everything worked ok.

Tested-by: David Heidelberg 

On 11/10/2014 04:53 AM, Siavash Eliasi wrote:

Hello sir,

I've sent a patch to mesa-dev which removes the runtime checks for 
CPU features which are known to be supported by that target at 
compile time. Just to make sure that this patch won't break your 
machine (Athlon TK-55?) with your usual build flags, can you please 
try this patch and see if that's ok for you?


Patch:
http://patchwork.freedesktop.org/patch/36488/

Related bug:
https://bugs.freedesktop.org/show_bug.cgi?id=71547#c3

Best regards,
Siavash Eliasi.






___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] i965: Make Gen4-5 push constants call _mesa_load_state_parameters too.

2014-11-21 Thread Kenneth Graunke
In commit 5e37a2a4a8a, I made the pull constant code stop calling
_mesa_load_state_parameters() when there were no pull parameters.

This worked fine on Gen6+ because the push constant code also called
it if there were any push constants.  However, the Gen4-5 push constant
code wasn't doing this.  This patch makes it do so, like the Gen6+ code.

A better long term solution would be to make core Mesa just handle this
for us when necessary.

Fixes around 8766 Piglit tests on Ironlake, and probably Gen4 as well.

Reported-by: Mark Janes 
Signed-off-by: Kenneth Graunke 
---
 src/mesa/drivers/dri/i965/brw_curbe.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_curbe.c 
b/src/mesa/drivers/dri/i965/brw_curbe.c
index 1a828ed..fea5d332 100644
--- a/src/mesa/drivers/dri/i965/brw_curbe.c
+++ b/src/mesa/drivers/dri/i965/brw_curbe.c
@@ -211,6 +211,8 @@ brw_upload_constant_buffer(struct brw_context *brw)
 
/* fragment shader constants */
if (brw->curbe.wm_size) {
+  _mesa_load_state_parameters(ctx, brw->fragment_program->Base.Parameters);
+
   /* BRW_NEW_CURBE_OFFSETS */
   GLuint offset = brw->curbe.wm_start * 16;
 
@@ -251,6 +253,8 @@ brw_upload_constant_buffer(struct brw_context *brw)
 
/* vertex shader constants */
if (brw->curbe.vs_size) {
+  _mesa_load_state_parameters(ctx, brw->vertex_program->Base.Parameters);
+
   GLuint offset = brw->curbe.vs_start * 16;
 
   /* CACHE_NEW_VS_PROG | _NEW_PROGRAM_CONSTANTS: copy uniform values */
-- 
2.1.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 01/29] mesa: Add an implementation of a master convert function.

2014-11-21 Thread Iago Toral
On Fri, 2014-11-21 at 17:40 +0900, Michel Dänzer wrote:
> On 21.11.2014 17:07, Iago Toral wrote:
> > On Thu, 2014-11-20 at 11:10 -0800, Jason Ekstrand wrote:
> >> On Thu, Nov 20, 2014 at 1:48 AM, Iago Toral  wrote:
> >>  Just out of curiosity: is there any gain in avoiding the GL
> >>  types in the
> >>  conversion code?
> >>
> >>
> >> As I said in my reply to Jose on the 00/20 patch, we would like to
> >> eventually move the format conversion stuff to a common helper library
> >> that can be shared by mesa main and the gallium code.  If we are going
> >> to do that, then we don't want any GL dependencies.
> >
> > Yes, I know, I was just wondering why Mesa and Gallium would need/want
> > to avoid the GL dependencies in that helper library, since clients of
> > that library would usually be things like glTexImage or glReadPixels and
> > these deal with GL types.
> 
> Since OpenGL is only one out of many APIs provided by state trackers on 
> top of Gallium (some of which have nothing to do with OpenGL), code 
> below the Gallium interface using GL types directly is a layering violation.

Aha, I see now. That makes sense.
Thanks for the explanation!

Iago


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 01/29] mesa: Add an implementation of a master convert function.

2014-11-21 Thread Michel Dänzer

On 21.11.2014 17:07, Iago Toral wrote:

On Thu, 2014-11-20 at 11:10 -0800, Jason Ekstrand wrote:

On Thu, Nov 20, 2014 at 1:48 AM, Iago Toral  wrote:
 Just out of curiosity: is there any gain in avoiding the GL
 types in the
 conversion code?


As I said in my reply to Jose on the 00/20 patch, we would like to
eventually move the format conversion stuff to a common helper library
that can be shared by mesa main and the gallium code.  If we are going
to do that, then we don't want any GL dependencies.


Yes, I know, I was just wondering why Mesa and Gallium would need/want
to avoid the GL dependencies in that helper library, since clients of
that library would usually be things like glTexImage or glReadPixels and
these deal with GL types.


Since OpenGL is only one out of many APIs provided by state trackers on 
top of Gallium (some of which have nothing to do with OpenGL), code 
below the Gallium interface using GL types directly is a layering violation.



--
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 01/29] mesa: Add an implementation of a master convert function.

2014-11-21 Thread Iago Toral
On Thu, 2014-11-20 at 11:10 -0800, Jason Ekstrand wrote:
> 
> 
> On Thu, Nov 20, 2014 at 1:48 AM, Iago Toral  wrote:
> On Wed, 2014-11-19 at 11:28 -0800, Jason Ekstrand wrote:
> > By and large, this looks good to me.  Most of my comments
> are cosmetic
> > or suggestions for added documentation.  There is one issue
> that I
> > think is subtly wrong with integer format conversion but
> that should
> > be easy to fix.
> >
> > --Jason
> >
> > On Tue, Nov 18, 2014 at 1:23 AM, Iago Toral Quiroga
> >  wrote:
> > From: Jason Ekstrand 
> >
> > v2 by Iago Toral :
> >
> > - When testing if we can directly pack we should use
> the src
> > format to check
> >   if we are packing from an RGBA format. The
> original code
> > used the dst format
> >   for the ubyte case by mistake.
> > - Fixed incorrect number of bits for dst, it was
> computed
> > using the src format
> >   instead of the dst format.
> > - If the dst format is an array format, check if it
> is signed.
> > We were only
> >   checking this for the case where it was not an
> array format,
> > but we need
> >   to know this in both scenarios.
> > - Fixed incorrect swizzle transform for the cases
> where we
> > convert between
> >   array formats.
> > - Compute is_signed and bits only once and for the
> dst format.
> > We were
> >   computing these for the src format too but they
> were
> > overwritten by the
> >   dst values immediately after.
> > - Be more careful when selecting the integer path.
> > Specifically, check that
> >   both src and dst are integer types. Checking only
> one of
> > them should suffice
> >   since OpenGL does not allow conversions between
> normalized
> > and integer types,
> >   but putting extra care here makes sense and also
> makes the
> > actual requirements
> >   for this path more clear.
> > - The format argument for pack functions is the
> destination
> > format we are
> >   packing to, not the source format (which has to be
> RGBA).
> > - Expose RGBA_* to other files. These will come
> in handy
> > when in need to
> >   test if a given array format is RGBA or in need to
> pass RGBA
> > formats to
> >   mesa_format_convert.
> >
> > v3 by Samuel Iglesias :
> >
> > - Add an RGBA_INT definition.
> > ---
> >  src/mesa/main/format_utils.c | 378
> > +++
> >  src/mesa/main/format_utils.h |  10 ++
> >  src/mesa/main/formats.h  |  15 +-
> >  3 files changed, 399 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/mesa/main/format_utils.c
> > b/src/mesa/main/format_utils.c
> > index fcbbba4..c3815cb 100644
> > --- a/src/mesa/main/format_utils.c
> > +++ b/src/mesa/main/format_utils.c
> > @@ -25,6 +25,384 @@
> >  #include "format_utils.h"
> >  #include "glformats.h"
> >  #include "macros.h"
> > +#include "format_pack.h"
> > +#include "format_unpack.h"
> > +
> > +mesa_array_format RGBA_FLOAT = {{
> > +   MESA_ARRAY_FORMAT_TYPE_FLOAT,
> > +   0,
> > +   4,
> > +   0, 1, 2, 3,
> > +   0, 1
> > +}};
> > +
> > +mesa_array_format RGBA_UBYTE = {{
> > +   MESA_ARRAY_FORMAT_TYPE_UBYTE,
> > +   1,
> > +   4,
> > +   0, 1, 2, 3,
> > +   0, 1
> > +}};
> > +
> > +mesa_array_format RGBA_UINT = {{
> > +   MESA_ARRAY_FORMAT_TYPE_UINT,
> > +   0,
> > +   4,
> > +   0, 1, 2, 3,
> > +   0, 1
> > +}};
> > +
> > +mesa_array_format RGBA_INT = {{
> > +   MESA_ARRAY_FORMAT_TYPE_