Re: [Mesa-dev] [PATCH] mesa/st: Check for a NULL _LinkedShader[i] before using it.

2011-10-10 Thread Ian Romanick
Yeah, this is correct. The pointer will be NULL if there is no shader  
for that stage.


Reviewed-by: Ian Romanick ian.d.roman...@intel.com

(Sent from my phone.)

On Oct 9, 2011, at 5:13 PM, Stéphane Marchesin marc...@chromium.org  
wrote:


The rest of the linker/glsl translation code checks for NULL, so I  
suppose we should check here too. Fixes crash on exit with i915g  
instanced drawing.

---
src/mesa/state_tracker/st_program.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/src/mesa/state_tracker/st_program.c b/src/mesa/ 
state_tracker/st_program.c

index acd3b56..c419c40 100644
--- a/src/mesa/state_tracker/st_program.c
+++ b/src/mesa/state_tracker/st_program.c
@@ -1154,7 +1154,8 @@ destroy_shader_program_variants_cb(GLuint key,  
void *data, void *userData)

 }

for (i = 0; i  Elements(shProg-_LinkedShaders); i++) {
-destroy_program_variants(st, shProg-_LinkedShaders[i]- 
Program);

+if (shProg-_LinkedShaders[i])
+   destroy_program_variants(st, shProg-_LinkedShaders 
[i]-Program);

}
  }
  break;
--
1.7.5.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 1/2] hash_table: Make string_to_uint_map make a copy of the name

2011-10-10 Thread Kenneth Graunke
On 10/07/2011 04:33 PM, Ian Romanick wrote:
 From: Ian Romanick ian.d.roman...@intel.com
 
 The hash table needs a copy of the key that it can keep for
 comparisons during searches.
 
 Signed-off-by: Ian Romanick ian.d.roman...@intel.com
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=41499
 Cc: Stéphane Marchesin stephane.marche...@gmail.com
 Cc: Luzipher luziphermcl...@yahoo.ie
 Cc: Michał Lipski tall...@o2.pl
 ---
  src/mesa/program/hash_table.h |   18 +-
  1 files changed, 17 insertions(+), 1 deletions(-)

For both patches:
Reviewed-by: Kenneth Graunke kenn...@whitecape.org

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


Re: [Mesa-dev] [PATCH 1/2] hash_table: Make string_to_uint_map make a copy of the name

2011-10-10 Thread Luzipher McLeod
Hi all :-)

first, I'm not really sure how to handle mailing list ... I hope Reply All 
was the right button.
This patch solves the texture problems for me (Bug 41499). Thanks :-)

Regards,
Luzipher




--- On Sat, 8/10/11, Ian Romanick i...@freedesktop.org wrote:

 From: Ian Romanick i...@freedesktop.org
 Subject: [PATCH 1/2] hash_table: Make string_to_uint_map make a copy of the 
 name
 To: mesa-dev@lists.freedesktop.org
 Cc: Ian Romanick ian.d.roman...@intel.com, Stéphane Marchesin 
 stephane.marche...@gmail.com, Luzipher luziphermcl...@yahoo.ie, Michał 
 Lipski tall...@o2.pl
 Date: Saturday, 8 October, 2011, 0:33
 From: Ian Romanick ian.d.roman...@intel.com
 
 The hash table needs a copy of the key that it can keep
 for
 comparisons during searches.
 
 Signed-off-by: Ian Romanick ian.d.roman...@intel.com
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=41499
 Cc: Stéphane Marchesin stephane.marche...@gmail.com
 Cc: Luzipher luziphermcl...@yahoo.ie
 Cc: Michał Lipski tall...@o2.pl
 ---
  src/mesa/program/hash_table.h |   18
 +-
  1 files changed, 17 insertions(+), 1 deletions(-)
 
 diff --git a/src/mesa/program/hash_table.h
 b/src/mesa/program/hash_table.h
 index bfe221b..941d28a 100644
 --- a/src/mesa/program/hash_table.h
 +++ b/src/mesa/program/hash_table.h
 @@ -32,6 +32,7 @@
  #define HASH_TABLE_H
  
  #include string.h
 +#include stdlib.h
  #include stdint.h
  #include limits.h
  #include assert.h
 @@ -100,6 +101,10 @@ extern void *hash_table_find(struct
 hash_table *ht, const void *key);
   * calls to \c hash_table_find and \c
 hash_table_remove will return or remove,
   * repsectively, the most recently added instance of
 \c key.
   *
 + * \warning
 + * The value passed by \c key is kept in the hash table
 and is used by later
 + * calls to \c hash_table_find.
 + *
   * \sa hash_table_replace
   */
  extern void hash_table_insert(struct hash_table *ht, void
 *data,
 @@ -204,6 +209,7 @@ public:
  
     ~string_to_uint_map()
     {
 +      hash_table_call_foreach(this-ht,
 delete_key, NULL);
    
    hash_table_dtor(this-ht);
     }
  
 @@ -243,10 +249,20 @@ public:
         * because UINT_MAX+1 = 0.
         */
        assert(value != UINT_MAX);
 -      hash_table_replace(ht, (void *)
 (intptr_t) (value + 1), key);
 +      hash_table_replace(this-ht,
 +       
  (void *) (intptr_t) (value +
 1),
 +       
  strdup(key));
     }
  
  private:
 +   static void delete_key(const void *key,
 void *data, void *closure)
 +   {
 +      (void) data;
 +      (void) closure;
 +
 +      free((char *)key);
 +   }
 +
     struct hash_table *ht;
  };
  
 -- 
 1.7.6
 
 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 41630] r600g nexuiz segfault on quit since recent LinkedShaders changes

2011-10-10 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=41630

Andy Furniss li...@andyfurniss.entadsl.com changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution||FIXED

--- Comment #1 from Andy Furniss li...@andyfurniss.entadsl.com 2011-10-10 
02:44:08 PDT ---
Fixed now I assume by

ddba509c16c4cb0630a4f4841b31953f02be6b3f
mesa/st: Check for a NULL _LinkedShader[i] before using it.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- 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] Mesa (master): u_blitter: don' t create integer vertex elements unless shader supports them

2011-10-10 Thread Brian Paul

On 10/09/2011 09:36 AM, Dave Airlie wrote:

Module: Mesa
Branch: master
Commit: dd20256a1c1566f11e1fa970028f3bb4f05445b7
URL:
http://cgit.freedesktop.org/mesa/mesa/commit/?id=dd20256a1c1566f11e1fa970028f3bb4f05445b7

Author: Dave Airlieairl...@redhat.com
Date:   Sun Oct  9 16:35:28 2011 +0100

u_blitter: don't create integer vertex elements unless shader supports them

Should fix https://bugs.freedesktop.org/show_bug.cgi?id=41613

We don't want to create these vertex elements unless the pipe driver
vertex stage can handle integers.

Signed-off-by: Dave Airlieairl...@redhat.com

---

  src/gallium/auxiliary/util/u_blitter.c |   49 +++-
  1 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/src/gallium/auxiliary/util/u_blitter.c 
b/src/gallium/auxiliary/util/u_blitter.c
index 1f8b679..73d1af0 100644
--- a/src/gallium/auxiliary/util/u_blitter.c
+++ b/src/gallium/auxiliary/util/u_blitter.c
@@ -109,6 +109,7 @@ struct blitter_context_priv
 unsigned dst_height;

 boolean has_geometry_shader;
+   boolean vertex_has_integers;
  };

  static void blitter_draw_rectangle(struct blitter_context *blitter,
@@ -152,6 +153,9 @@ struct blitter_context *util_blitter_create(struct 
pipe_context *pipe)
 ctx-has_geometry_shader =
pipe-screen-get_shader_param(pipe-screen, PIPE_SHADER_GEOMETRY,
   PIPE_SHADER_CAP_MAX_INSTRUCTIONS)  0;
+   ctx-vertex_has_integers =
+  pipe-screen-get_shader_param(pipe-screen, PIPE_SHADER_GEOMETRY,
+ PIPE_SHADER_CAP_INTEGERS);

 /* blend state objects */
 memset(blend, 0, sizeof(blend));
@@ -211,27 +215,30 @@ struct blitter_context *util_blitter_create(struct 
pipe_context *pipe)
 }
 ctx-velem_state = pipe-create_vertex_elements_state(pipe, 2,velem[0]);

-   memset(velem[0], 0, sizeof(velem[0]) * 2);
-   for (i = 0; i  2; i++) {
-  velem[i].src_offset = i * 4 * sizeof(float);
-  if (i == 0) {
- velem[i].src_format = PIPE_FORMAT_R32G32B32A32_FLOAT;
-  } else {
- velem[i].src_format = PIPE_FORMAT_R32G32B32A32_SINT;
+   if (ctx-vertex_has_integers) {
+  memset(velem[0], 0, sizeof(velem[0]) * 2);
+  for (i = 0; i  2; i++) {
+ velem[i].src_offset = i * 4 * sizeof(float);
+ if (i == 0) {
+velem[i].src_format = PIPE_FORMAT_R32G32B32A32_FLOAT;
+ } else {
+velem[i].src_format = PIPE_FORMAT_R32G32B32A32_SINT;
+ }
}


I realize that the current code uses a loop here, but it seems like 
overkill.  This seems simpler:


memset(velem, 0, sizeof(velem));
velem[0].src_offset = 0;
velem[0].src_format = PIPE_FORMAT_R32G32B32A32_FLOAT;
velem[1].src_offset = 4 * sizeof(float);
velem[1].src_format = PIPE_FORMAT_R32G32B32A32_SINT;

It's about half the LOC and is easier to read.

Same thing below.



-   }
-   ctx-velem_sint_state = pipe-create_vertex_elements_state(pipe, 
2,velem[0]);
-
-   memset(velem[0], 0, sizeof(velem[0]) * 2);
-   for (i = 0; i  2; i++) {
-  velem[i].src_offset = i * 4 * sizeof(float);
-  if (i == 0) {
- velem[i].src_format = PIPE_FORMAT_R32G32B32A32_FLOAT;
-  } else {
- velem[i].src_format = PIPE_FORMAT_R32G32B32A32_UINT;
+  ctx-velem_sint_state = pipe-create_vertex_elements_state(pipe, 
2,velem[0]);
+
+  memset(velem[0], 0, sizeof(velem[0]) * 2);
+  for (i = 0; i  2; i++) {
+ velem[i].src_offset = i * 4 * sizeof(float);
+ if (i == 0) {
+velem[i].src_format = PIPE_FORMAT_R32G32B32A32_FLOAT;
+ } else {
+velem[i].src_format = PIPE_FORMAT_R32G32B32A32_UINT;
+ }
}
+  ctx-velem_uint_state = pipe-create_vertex_elements_state(pipe, 
2,velem[0]);
 }
-   ctx-velem_uint_state = pipe-create_vertex_elements_state(pipe, 
2,velem[0]);
+
 /* fragment shaders are created on-demand */

 /* vertex shader */
@@ -274,8 +281,10 @@ void util_blitter_destroy(struct blitter_context *blitter)
 pipe-delete_rasterizer_state(pipe, ctx-rs_state);
 pipe-delete_vs_state(pipe, ctx-vs);
 pipe-delete_vertex_elements_state(pipe, ctx-velem_state);
-   pipe-delete_vertex_elements_state(pipe, ctx-velem_sint_state);
-   pipe-delete_vertex_elements_state(pipe, ctx-velem_uint_state);
+   if (ctx-vertex_has_integers) {
+  pipe-delete_vertex_elements_state(pipe, ctx-velem_sint_state);
+  pipe-delete_vertex_elements_state(pipe, ctx-velem_uint_state);
+   }

 for (i = 0; i  PIPE_MAX_TEXTURE_TYPES; i++) {
if (ctx-fs_texfetch_col[i])



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


[Mesa-dev] [RFC][PATCH] glx/dri2: Implement a throttle dri extension.

2011-10-10 Thread Thomas Hellstrom
The X server has limited throttle support on the server side,
but doing this in the client has some benefits:

1) X server throttling is per client. Client side throttling can be done
per drawable.

2) It's easier to control the throttling based on what client is run,
for example using driconf.

3) X server throttling requires drm swap complete events.

Before I go on implementing this in the dri state tracker I wanted to hear
whether there are any objections or something important that I missed.

Thanks,
Thomas
---
 include/GL/internal/dri_interface.h |   20 +
 src/glx/dri2_glx.c  |   54 +--
 2 files changed, 71 insertions(+), 3 deletions(-)

diff --git a/include/GL/internal/dri_interface.h 
b/include/GL/internal/dri_interface.h
index 8a9ca19..1f5c679 100644
--- a/include/GL/internal/dri_interface.h
+++ b/include/GL/internal/dri_interface.h
@@ -84,6 +84,7 @@ typedef struct __DRIbufferRec __DRIbuffer;
 typedef struct __DRIdri2ExtensionRec   __DRIdri2Extension;
 typedef struct __DRIdri2LoaderExtensionRec __DRIdri2LoaderExtension;
 typedef struct __DRI2flushExtensionRec __DRI2flushExtension;
+typedef struct __DRI2throttleExtensionRec  __DRI2throttleExtension;
 
 /*@}*/
 
@@ -284,6 +285,25 @@ struct __DRI2flushExtensionRec {
 
 
 /**
+ * Extension that the driver uses to request
+ * throttle callbacks.
+ */
+
+enum __DRI2throttleReason {
+   __DRI2_THROTTLE_SWAPBUFFER,
+   __DRI2_THROTTLE_COPYSUBBUFFER,
+   __DRI2_THROTTLE_FLUSHFRONT
+};
+
+#define __DRI2_THROTTLE DRI2_Throttle
+struct __DRI2throttleExtensionRec {
+   __DRIextension base;
+   void (*throttle)(__DRIcontext *ctx,
+   __DRIdrawable *drawable,
+   enum __DRI2throttleReason reason);
+};
+
+/**
  * XML document describing the configuration options supported by the
  * driver.
  */
diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
index 01e3fd6..37da073 100644
--- a/src/glx/dri2_glx.c
+++ b/src/glx/dri2_glx.c
@@ -85,6 +85,7 @@ struct dri2_screen {
const __DRI2flushExtension *f;
const __DRI2configQueryExtension *config;
const __DRItexBufferExtension *texBuffer;
+   const __DRI2throttleExtension *throttle;
const __DRIconfig **driver_configs;
 
void *driver;
@@ -369,7 +370,9 @@ dri2WaitForSBC(__GLXDRIdrawable *pdraw, int64_t target_sbc, 
int64_t *ust,
 #endif /* X_DRI2WaitMSC */
 
 static void
-dri2CopySubBuffer(__GLXDRIdrawable *pdraw, int x, int y, int width, int height)
+__dri2CopySubBuffer(__GLXDRIdrawable *pdraw, int x, int y,
+   int width, int height,
+   enum __DRI2throttleReason reason)
 {
struct dri2_drawable *priv = (struct dri2_drawable *) pdraw;
struct dri2_screen *psc = (struct dri2_screen *) pdraw-psc;
@@ -390,6 +393,15 @@ dri2CopySubBuffer(__GLXDRIdrawable *pdraw, int x, int y, 
int width, int height)
   (*psc-f-flush) (priv-driDrawable);
 #endif
 
+   if (psc-throttle) {
+  struct glx_context *gc = __glXGetCurrentContext();
+  struct dri2_context *dri2Ctx = (struct dri2_context *)gc;
+  __DRIcontext *ctx =
+(dri2Ctx) ? dri2Ctx-driContext : NULL;
+
+  psc-throttle-throttle(ctx, priv-driDrawable, reason);
+   }
+
region = XFixesCreateRegion(psc-base.dpy, xrect, 1);
DRI2CopyRegion(psc-base.dpy, pdraw-xDrawable, region,
   DRI2BufferFrontLeft, DRI2BufferBackLeft);
@@ -405,6 +417,15 @@ dri2CopySubBuffer(__GLXDRIdrawable *pdraw, int x, int y, 
int width, int height)
 }
 
 static void
+dri2CopySubBuffer(__GLXDRIdrawable *pdraw, int x, int y,
+ int width, int height)
+{
+   __dri2CopySubBuffer(pdraw, x, y, width, height,
+  __DRI2_THROTTLE_COPYSUBBUFFER);
+}
+
+
+static void
 dri2_copy_drawable(struct dri2_drawable *priv, int dest, int src)
 {
XRectangle xrect;
@@ -458,6 +479,7 @@ dri2FlushFrontBuffer(__DRIdrawable *driDrawable, void 
*loaderPrivate)
struct dri2_display *pdp;
struct glx_context *gc;
struct dri2_drawable *pdraw = loaderPrivate;
+   struct dri2_screen *psc;
 
if (!pdraw)
   return;
@@ -465,10 +487,22 @@ dri2FlushFrontBuffer(__DRIdrawable *driDrawable, void 
*loaderPrivate)
if (!pdraw-base.psc)
   return;
 
-   priv = __glXInitialize(pdraw-base.psc-dpy);
+   psc = (struct dri2_screen *) pdraw-base.psc;
+
+   priv = __glXInitialize(psc-base.dpy);
pdp = (struct dri2_display *) priv-dri2Display;
gc = __glXGetCurrentContext();
 
+   if (psc-throttle) {
+  struct glx_context *gc = __glXGetCurrentContext();
+  struct dri2_context *dri2Ctx = (struct dri2_context *)gc;
+  __DRIcontext *ctx = (dri2Ctx) ? dri2Ctx-driContext : NULL;
+
+  psc-throttle-throttle(ctx,
+ pdraw-driDrawable,
+ __DRI2_THROTTLE_FLUSHFRONT);
+   }
+
/* Old servers don't send invalidate events */
if (!pdp-invalidateAvailable)
dri2InvalidateBuffers(priv-dpy, 

Re: [Mesa-dev] [PATCH] st/mesa: kill instruction if writemask=0 in eliminate_dead_code_advanced()

2011-10-10 Thread Brian Paul


I tested with a shader that generates an ARL instruction and found 
inst-dst is:


{file = PROGRAM_ADDRESS, index = 0, writemask = 1, cond_mask = 8, type 
= 2, reladdr = 0x0}


in the code in question.  I don't think there's any way that the dest 
register's writemask could be set to zero.


Other instructions that don't use the dst register (like 
OPCODE_BGNLOOP, OPCODE_IF, etc) use undef_dst to initialize the 
instructions dst register.


BTW, the initialization of undef_dst seems incorrect:

static st_dst_reg undef_dst = st_dst_reg(PROGRAM_UNDEFINED, 
SWIZZLE_NOOP, GLSL_TYPE_ERROR);


I think SWIZZLE_NOOP should be WRITEMASK_XYZW since the parameter is a 
writemask, not a swizzle.


-Brian



On 10/09/2011 01:22 PM, Marek Olšák wrote:

Correct.

The thing was: even though we have an addressing register in TGSI
(e.g. tgsi_full_dst|src_register::Indirect), everybody except nv50 and
partially svga doesn't use it now. They expect the indirect register
file is always TGSI_FILE_ADDRESS and the index is 0, making the
destination of ARL irrelevant. This is probably a bug in those drivers
and that's what made me think that ARL doesn't need a destination
register. Sorry for the noise.

Marek

On Sun, Oct 9, 2011 at 8:51 PM, Bryan Cainbryanca...@gmail.com  wrote:

What does it do if there's no destination register?  In any case, I
don't think glsl_to_tgsi emits any ARLs of that form, so it shouldn't be
a problem.

Bryan

On 10/07/2011 01:06 PM, Marek Olšák wrote:

I think ARL is allowed to have no destination register, right? In that
case, there should be a special case not to eliminate ARLs.

Marek

On Fri, Oct 7, 2011 at 5:40 PM, Brian Paulbrian.e.p...@gmail.com  wrote:

From: Brian Paulbri...@vmware.com

This fixes a bug where we'd wind up emitting an invalid instruction like
MOVE R[0]., R[1];  - note the empty/zero writemask.  If we don't write to
any dest register channels, cull the instruction.
---
  src/mesa/state_tracker/st_glsl_to_tgsi.cpp |8 +++-
  1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp 
b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index d8ef8a3..44b1149 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -3776,8 +3776,14 @@ glsl_to_tgsi_visitor::eliminate_dead_code_advanced(void)
  iter.remove();
  delete inst;
  removed++;
-  } else
+  } else {
  inst-dst.writemask= ~(inst-dead_mask);
+ if (inst-dst.writemask == 0) {
+iter.remove();
+delete inst;
+removed++;
+ }
+  }
}

ralloc_free(write_level);
--
1.7.3.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 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 0/9] Elminate redudnant Mesa IR references

2011-10-10 Thread Ian Romanick

On 10/09/2011 07:41 AM, Marcin Slusarz wrote:

On Tue, Oct 04, 2011 at 01:44:03PM -0700, Ian Romanick wrote:

This is the next in my series of internal shader API cleaning.  The
primary purpose of this series is to remove the extra tracking of Mesa
IR shaders.  The gl_shader_program has a reference to the Mesa IR
shader for each stage, and the entry in gl_shader_program::_LinkedShaders
has a reference to the same Mesa IR.

The first three patches eliminate the extra references, and the
remaining six implement some refactors and clean-ups enabled by the
first three.

  src/glsl/ir.cpp|8 ---
  src/glsl/ir.h  |8 ---
  src/mesa/drivers/dri/i965/brw_fs.cpp   |8 ++-
  src/mesa/drivers/dri/i965/brw_fs.h |3 +-
  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |3 +-
  src/mesa/drivers/dri/i965/brw_vs.c |8 ++-
  src/mesa/drivers/dri/i965/brw_wm.c |2 +-
  src/mesa/main/context.c|8 +--
  src/mesa/main/ff_fragment_shader.cpp   |   10 ++--
  src/mesa/main/mtypes.h |3 -
  src/mesa/main/shaderapi.c  |   32 +--
  src/mesa/main/shaderobj.c  |4 -
  src/mesa/main/state.c  |   18 --
  src/mesa/main/texstate.c   |   18 +++---
  src/mesa/main/uniforms.c   |   42 +-
  src/mesa/program/ir_to_mesa.cpp|   37 
  src/mesa/program/prog_uniform.c|   74 
  src/mesa/program/prog_uniform.h|4 -
  src/mesa/state_tracker/st_glsl_to_tgsi.cpp |   44 ---
  src/mesa/state_tracker/st_program.c|9 +--
  20 files changed, 111 insertions(+), 232 deletions(-)



Commits 1-5 are unbuildable and one of them introduces crashes in many
piglit tests on nv50, e.g. glx-shader-sharing, glsl-bindattriblocation.

glx-shader-sharing:
Program received signal SIGSEGV, Segmentation fault.
0x737b1dac in destroy_shader_program_variants_cb (userData=optimized out, 
data=optimized out, key=optimized out) at state_tracker/st_program.c:1157
warning: Source file is more recent than executable.
1157  
shProg-_LinkedShaders[MESA_SHADER_VERTEX]-Program);


Does Stephane's patch from the weekend 
(http://marc.info/?l=mesa3d-devm=131820561425054w=2) fix this issue? 
If it does, you should send him a Tested-by.

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


Re: [Mesa-dev] [PATCH] i965: Replace incorrect use of GLboolean with enum brw_compression.

2011-10-10 Thread Chad Versace
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 10/07/2011 08:28 PM, Kenneth Graunke wrote:
 brw_set_compression_control took a GLboolean as an argument, then
 promptly used a switch statement to compare it with various enumeration
 values.  Clearly it's not actually a boolean.
 
 Introduce a new enumeration type, enum brw_compression, and use that.
 
 Found by converting GLboolean to bool; clang then gave warnings about
 switching on a boolean and ultimately duplicated case errors.
 
 Signed-off-by: Kenneth Graunke kenn...@whitecape.org
 ---
  src/mesa/drivers/dri/i965/brw_defines.h |8 +---
  src/mesa/drivers/dri/i965/brw_eu.c  |4 +++-
  src/mesa/drivers/dri/i965/brw_eu.h  |2 +-
  3 files changed, 9 insertions(+), 5 deletions(-)

Reviewed-by: Chad Versace c...@chad-versace.us

- -- 
Chad Versace
c...@chad-versace.us
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJOkynLAAoJEAIvNt057x8ivg0P/2iEPIREnHsRu8ESPFYEkW6N
Ws13EbfJYmPqGaIoOKycF0bnGqnxRlXz8Wsmnz38q49CFoYpdlSnCW6NUTbrgUmS
Ink+hPe+qfk5aXXPmyOow+mfzPUOiXy9iTIb2eagH/0H2CUeRy8qVwjflxfuziPt
fJpb4/Ku8W1ZG87tJEmyaswgV3xwXeoo+QdQ9El1Pk9izQM/M8YTM5tXdTG9AFBr
BIryxGs67eYaKfN++j/zHx0hKXls17dDb5NKdIFTFNlthq8Me7L0bSLAzjG14wKT
VhccyY6IMoEgSssV74bH7xLgYir5NIsxwiKea0d2H8ZUWj+9NyBCzyyqYRhDM+UA
y3g3yt+/tjVS/nWY3lb6nmwlRELQbDUf7nFSPPjIjEE32/dKnc2LkOUQ8LYMdWH/
cAhBAQP3EYUGFFwFt3BhgtYO1yzndrvyrcGWjKd7OS1ZNEB3ekTTd8Y9sSXSqUBF
1z9aTyQ6WFBT0iI4jm1pFZD35VW/U2vXXVLtClXp9VzDkcZ2P91e7GSPCkJ2NREt
VyMgNlLJTtfCTiLW0Z25EUzIxAaHt1C0CSG51C76/8ZlastZc6Z7YebhqTibu9lx
7ocX5AYEV81stbkE5/vF0rq7tmyyFYKoVJ7y0qtvjkBCL+7X8VXFph+MPvxP9TUc
DTInTP/4iHvu0jjGkWTX
=3KOh
-END PGP SIGNATURE-
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] i965: Replace incorrect use of GLboolean with enum brw_compression.

2011-10-10 Thread Eric Anholt
On Fri,  7 Oct 2011 20:28:35 -0700, Kenneth Graunke kenn...@whitecape.org 
wrote:
 brw_set_compression_control took a GLboolean as an argument, then
 promptly used a switch statement to compare it with various enumeration
 values.  Clearly it's not actually a boolean.
 
 Introduce a new enumeration type, enum brw_compression, and use that.
 
 Found by converting GLboolean to bool; clang then gave warnings about
 switching on a boolean and ultimately duplicated case errors.
 
 Signed-off-by: Kenneth Graunke kenn...@whitecape.org

 ---
  src/mesa/drivers/dri/i965/brw_defines.h |8 +---
  src/mesa/drivers/dri/i965/brw_eu.c  |4 +++-
  src/mesa/drivers/dri/i965/brw_eu.h  |2 +-
  3 files changed, 9 insertions(+), 5 deletions(-)
 
 Only compile tested.  I think this demonstrates that using stdbool instead of
 GLboolean is worthwhile: the compiler actually recognizes it as a boolean
 data type and offers appropriate warnings---even errors!---when you do stupid
 things like this.
 
 I have a follow-on patch that actually does the GLboolean-bool conversion,
 should we decide to go that route.  Needs a bit more clean-up but shouldn't
 take too long.

Reviewed-by: Eric Anholt e...@anholt.net

I'm looking forward to GLboolean - bool conversion in the driver.
GLboolean needs to die in a chemical fire.


pgpg9XD8o7Dpz.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] Add an autoconf option for mangling Mesa.

2011-10-10 Thread tfogal
From: Tom Fogal tfo...@alumni.unh.edu

In addition to setting up the flags correctly, this renames the
generated libraries to ensure they get 'Mangled' in the name.
This is very useful for distros and the like, where mangled Mesa
and non-mangled GL libraries typically need to be installed
side-by-side.
---
 configs/autoconf.in |4 ++--
 configure.ac|   27 ---
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/configs/autoconf.in b/configs/autoconf.in
index 9bbafc9..96fe5da 100644
--- a/configs/autoconf.in
+++ b/configs/autoconf.in
@@ -64,8 +64,8 @@ FLEX = @FLEX@
 BISON = @BISON@
 
 # Library names (base name)
-GL_LIB = GL
-GLU_LIB = GLU
+GL_LIB = @GL_LIB@
+GLU_LIB = @GLU_LIB@
 GLW_LIB = GLw
 OSMESA_LIB = @OSMESA_LIB@
 GLESv1_CM_LIB = GLESv1_CM
diff --git a/configure.ac b/configure.ac
index 49e81ad..df909dd 100644
--- a/configure.ac
+++ b/configure.ac
@@ -342,6 +342,28 @@ else
 fi
 
 dnl
+dnl Mangled Mesa support
+dnl
+AC_ARG_ENABLE([mangling],
+  [AS_HELP_STRING([--enable-mangling],
+[enable mangled symbols and library name @:@default=disabled@:@])],
+  [enable_mangling=${enableval}],
+  [enable_mangling=no]
+)
+GL_LIB=GL
+GLU_LIB=GLU
+OSMESA_LIB=OSMesa
+if test x${enable_mangling} = xyes ; then
+  DEFINES=${DEFINES} -DUSE_MGL_NAMESPACE
+  GL_LIB=MangledGL
+  GLU_LIB=MangledGLU
+  OSMESA_LIB=MangledOSMesa
+fi
+AC_SUBST([GL_LIB])
+AC_SUBST([GLU_LIB])
+AC_SUBST([OSMESA_LIB])
+
+dnl
 dnl potentially-infringing-but-nobody-knows-for-sure stuff
 dnl
 AC_ARG_ENABLE([texture-float],
@@ -1280,17 +1302,16 @@ if test x$osmesa_bits != x8; then
 fi
 case x$osmesa_bits in
 x8)
-OSMESA_LIB=OSMesa
+OSMESA_LIB=${OSMESA_LIB}
 ;;
 x16|x32)
-OSMESA_LIB=OSMesa$osmesa_bits
+OSMESA_LIB=${OSMESA_LIB}$osmesa_bits
 DEFINES=$DEFINES -DCHAN_BITS=$osmesa_bits 
-DDEFAULT_SOFTWARE_DEPTH_BITS=31
 ;;
 *)
 AC_MSG_ERROR([OSMesa bits '$osmesa_bits' is not a valid option])
 ;;
 esac
-AC_SUBST([OSMESA_LIB])
 
 if test x$enable_osmesa = xyes; then
 # only link libraries with osmesa if shared
-- 
1.7.3.4

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


Re: [Mesa-dev] [PATCH] i965: Fix computation of abs(-x) in FS

2011-10-10 Thread Eric Anholt
On Fri,  7 Oct 2011 12:25:55 -0700, Paul Berry stereotype...@gmail.com wrote:
 When updating a register reference to reflect the fact that we were
 taking its absolute value, the fragment shader back-end failed to
 clear the negate flag, resulting in abs(-x) getting computed as
 -abs(x).
 
 I also found (and fixed) a similar problem in brw_eu.h, but I'm not
 aware of an actual manifestation of that problem.
 
 Fixes piglit test glsl-fs-abs-neg-with-intermediate.

Reviewed-by: Eric Anholt e...@anholt.net


pgpVHe9D1x2NB.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/2] i965: Split brw_set_prim into brw/gen6 variants

2011-10-10 Thread Eric Anholt
On Fri,  7 Oct 2011 10:16:47 -0700, Chad Versace c...@chad-versace.us wrote:
 The slight optimization to avoid the GS program in brw_set_prim() is not
 used by Gen 6, since Gen 6 doesn't use a GS program. Also, Gen 6 doesn't use
 reduced primitives.
 
 Also, document that intel_context.reduced_primitive is only used for Gen  6
 
 Signed-off-by: Chad Versace c...@chad-versace.us

 diff --git a/src/mesa/drivers/dri/intel/intel_context.h 
 b/src/mesa/drivers/dri/intel/intel_context.h
 index eb78c00..cf7ab9e 100644
 --- a/src/mesa/drivers/dri/intel/intel_context.h
 +++ b/src/mesa/drivers/dri/intel/intel_context.h
 @@ -253,7 +253,7 @@ struct intel_context
 GLuint RenderIndex;
 GLmatrix ViewportMatrix;
 GLenum render_primitive;
 -   GLenum reduced_primitive;
 +   GLenum reduced_primitive; /* Only gen  6 */

If this is intended to be doxygen, that would be /** whatever */

Other than that, Reviewed-by: Eric Anholt e...@anholt.net


pgpm2sehsmzd0.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 0/9] Elminate redudnant Mesa IR references

2011-10-10 Thread Marcin Slusarz
On Mon, Oct 10, 2011 at 10:18:26AM -0700, Ian Romanick wrote:
 On 10/09/2011 07:41 AM, Marcin Slusarz wrote:
  On Tue, Oct 04, 2011 at 01:44:03PM -0700, Ian Romanick wrote:
  This is the next in my series of internal shader API cleaning.  The
  primary purpose of this series is to remove the extra tracking of Mesa
  IR shaders.  The gl_shader_program has a reference to the Mesa IR
  shader for each stage, and the entry in gl_shader_program::_LinkedShaders
  has a reference to the same Mesa IR.
 
  The first three patches eliminate the extra references, and the
  remaining six implement some refactors and clean-ups enabled by the
  first three.
 
src/glsl/ir.cpp|8 ---
src/glsl/ir.h  |8 ---
src/mesa/drivers/dri/i965/brw_fs.cpp   |8 ++-
src/mesa/drivers/dri/i965/brw_fs.h |3 +-
src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |3 +-
src/mesa/drivers/dri/i965/brw_vs.c |8 ++-
src/mesa/drivers/dri/i965/brw_wm.c |2 +-
src/mesa/main/context.c|8 +--
src/mesa/main/ff_fragment_shader.cpp   |   10 ++--
src/mesa/main/mtypes.h |3 -
src/mesa/main/shaderapi.c  |   32 +--
src/mesa/main/shaderobj.c  |4 -
src/mesa/main/state.c  |   18 --
src/mesa/main/texstate.c   |   18 +++---
src/mesa/main/uniforms.c   |   42 +-
src/mesa/program/ir_to_mesa.cpp|   37 
src/mesa/program/prog_uniform.c|   74 
  
src/mesa/program/prog_uniform.h|4 -
src/mesa/state_tracker/st_glsl_to_tgsi.cpp |   44 ---
src/mesa/state_tracker/st_program.c|9 +--
20 files changed, 111 insertions(+), 232 deletions(-)
 
 
  Commits 1-5 are unbuildable and one of them introduces crashes in many
  piglit tests on nv50, e.g. glx-shader-sharing, glsl-bindattriblocation.
 
  glx-shader-sharing:
  Program received signal SIGSEGV, Segmentation fault.
  0x737b1dac in destroy_shader_program_variants_cb 
  (userData=optimized out, data=optimized out, key=optimized out) at 
  state_tracker/st_program.c:1157
  warning: Source file is more recent than executable.
  1157  
  shProg-_LinkedShaders[MESA_SHADER_VERTEX]-Program);
 
 Does Stephane's patch from the weekend 
 (http://marc.info/?l=mesa3d-devm=131820561425054w=2) fix this issue? 
 If it does, you should send him a Tested-by.

Yes, this patch fixes it.

Thanks.

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


Re: [Mesa-dev] [PATCH 07/13] linker: Use gl_shader_program::AttributeBindings for attrib locations

2011-10-10 Thread Marcin Slusarz
On Sun, Oct 09, 2011 at 04:36:19PM +0200, Marcin Slusarz wrote:
 On Thu, Oct 06, 2011 at 06:54:38PM -0700, Stéphane Marchesin wrote:
  Hi Ian,
  
  This regresses Chrome GPU acceleration for all GPUs (I tested i915g,
  llvmpipe, i965).
  
 
 FYI, I bisected black screen in OilRush (glretrace of OilRush trace
 actually) to this commit too.

This is fixed by hash_table: Make string_to_uint_map make a copy of the name

Thanks.

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


Re: [Mesa-dev] [PATCH 3/5] glsl: Add gl_MESACurrentAttrib{Vert, Frag} internal builtin uniforms.

2011-10-10 Thread Eric Anholt
On Thu, 06 Oct 2011 14:41:35 -0700, Ian Romanick i...@freedesktop.org wrote:
 On 10/06/2011 02:11 PM, Marek Olšák wrote:
  If I get this right... Why not use a vertex array with stride == 0 instead?
 
 In the GL API, stride == 0 is a magic value that tells the GL 
 implementation to figure out what the stride should be.  As far as I 
 know, most hardware can't do true zero-stride arrays.

The 965 is certainly fine with stride == 0.  We use it.  The 915 doesn't
because it requires fixed vertex layouts.

However, this patch is just retaining existing behavior of the FF code
for the conversion to GLSL IR.  It removes huge numbers of instructions
in the shaders for fixed-function applications.  Whether the complexity
and CPU overhead is worth that, I don't know.  We certainly get tons of
bugs from this dodgy optimization.


pgpnsXH5pkmqX.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] i965: Replace incorrect use of GLboolean with enum brw_compression.

2011-10-10 Thread Ian Romanick

On 10/07/2011 08:28 PM, Kenneth Graunke wrote:

brw_set_compression_control took a GLboolean as an argument, then
promptly used a switch statement to compare it with various enumeration
values.  Clearly it's not actually a boolean.

Introduce a new enumeration type, enum brw_compression, and use that.

Found by converting GLboolean to bool; clang then gave warnings about
switching on a boolean and ultimately duplicated case errors.

Signed-off-by: Kenneth Graunkekenn...@whitecape.org
---
  src/mesa/drivers/dri/i965/brw_defines.h |8 +---
  src/mesa/drivers/dri/i965/brw_eu.c  |4 +++-
  src/mesa/drivers/dri/i965/brw_eu.h  |2 +-
  3 files changed, 9 insertions(+), 5 deletions(-)

Only compile tested.  I think this demonstrates that using stdbool instead of


I think this warrants at least a piglit run.  I'd like to avoid a repeat 
of ebca47a and fe006a7. :)



GLboolean is worthwhile: the compiler actually recognizes it as a boolean
data type and offers appropriate warnings---even errors!---when you do stupid
things like this.

I have a follow-on patch that actually does the GLboolean-bool conversion,
should we decide to go that route.  Needs a bit more clean-up but shouldn't
take too long.

diff --git a/src/mesa/drivers/dri/i965/brw_defines.h 
b/src/mesa/drivers/dri/i965/brw_defines.h
index a111630..21a115b 100644
--- a/src/mesa/drivers/dri/i965/brw_defines.h
+++ b/src/mesa/drivers/dri/i965/brw_defines.h
@@ -486,9 +486,11 @@
  #define BRW_CHANNEL_Z 2
  #define BRW_CHANNEL_W 3

-#define BRW_COMPRESSION_NONE  0
-#define BRW_COMPRESSION_2NDHALF   1
-#define BRW_COMPRESSION_COMPRESSED2
+enum brw_compression {
+   BRW_COMPRESSION_NONE   = 0,
+   BRW_COMPRESSION_2NDHALF= 1,
+   BRW_COMPRESSION_COMPRESSED = 2,
+};

  #define GEN6_COMPRESSION_1Q   0
  #define GEN6_COMPRESSION_2Q   1
diff --git a/src/mesa/drivers/dri/i965/brw_eu.c 
b/src/mesa/drivers/dri/i965/brw_eu.c
index 0e04af9..b5a858b 100644
--- a/src/mesa/drivers/dri/i965/brw_eu.c
+++ b/src/mesa/drivers/dri/i965/brw_eu.c
@@ -99,7 +99,9 @@ void brw_set_access_mode( struct brw_compile *p, GLuint 
access_mode )
 p-current-header.access_mode = access_mode;
  }

-void brw_set_compression_control( struct brw_compile *p, GLboolean 
compression_control )
+void
+brw_set_compression_control(struct brw_compile *p,
+   enum brw_compression compression_control)
  {
 p-compressed = (compression_control == BRW_COMPRESSION_COMPRESSED);

diff --git a/src/mesa/drivers/dri/i965/brw_eu.h 
b/src/mesa/drivers/dri/i965/brw_eu.h
index 31334ce..8bb 100644
--- a/src/mesa/drivers/dri/i965/brw_eu.h
+++ b/src/mesa/drivers/dri/i965/brw_eu.h
@@ -790,7 +790,7 @@ void brw_push_insn_state( struct brw_compile *p );
  void brw_set_mask_control( struct brw_compile *p, GLuint value );
  void brw_set_saturate( struct brw_compile *p, GLuint value );
  void brw_set_access_mode( struct brw_compile *p, GLuint access_mode );
-void brw_set_compression_control( struct brw_compile *p, GLboolean control );
+void brw_set_compression_control(struct brw_compile *p, enum brw_compression 
c);
  void brw_set_predicate_control_flag_value( struct brw_compile *p, GLuint 
value );
  void brw_set_predicate_control( struct brw_compile *p, GLuint pc );
  void brw_set_predicate_inverse(struct brw_compile *p, bool predicate_inverse);


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


Re: [Mesa-dev] [PATCH 3/5] glsl: Add gl_MESACurrentAttrib{Vert, Frag} internal builtin uniforms.

2011-10-10 Thread Eric Anholt
On Thu, 06 Oct 2011 14:40:28 -0700, Ian Romanick i...@freedesktop.org wrote:
 On 10/06/2011 09:36 AM, Eric Anholt wrote:
  These will be used by the FF VS/FS to represent the current attributes
  when they don't have an active vertex array.
 
 I just want to make sure I grok this completely.  These are arrays of 
 vec4 uniforms that are used when, for example, glColor is called outside 
 glBegin.  Yes?

Exactly.

 If that's the case, what's the advantage of having it in the fragment 
 shader (as opposed to using uniforms to interpolate the potentially 
 constant values)?

Now I'm not grokking what you're suggesting.

  @@ -284,6 +292,8 @@ const struct gl_builtin_uniform_desc 
  _mesa_builtin_uniform_desc[] = {
   STATEVAR(gl_MESABumpRotMatrix0),
   STATEVAR(gl_MESABumpRotMatrix1),
   STATEVAR(gl_MESAFogParamsOptimized),
  +   STATEVAR(gl_MESACurrentAttribVert),
  +   STATEVAR(gl_MESACurrentAttribFrag),
 
 My preference would be to use the regular GLSL extension naming 
 convention (even though we're not exposing these names directly).  That 
 is, gl_CurrentAttribVertMESA and gl_CurrentAttribFragMESA.  I would have 
 made the same comment about the other statevar names here, but I was too 
 slow getting reviews out.

I'm up for that.


pgpdycES7A9rO.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] intel: Assert that no batch is emitted if a region is mapped

2011-10-10 Thread Eric Anholt
On Thu,  6 Oct 2011 14:18:35 -0700, Chad Versace c...@chad-versace.us wrote:
 What I would prefer to assert is that, for each region that is currently
 mapped, no batch is emitted that uses that region's bo. However, it's much
 easier to implement this big hammer.

 diff --git a/src/mesa/drivers/dri/intel/intel_regions.c 
 b/src/mesa/drivers/dri/intel/intel_regions.c
 index 7faf4ca..d46c470 100644
 --- a/src/mesa/drivers/dri/intel/intel_regions.c
 +++ b/src/mesa/drivers/dri/intel/intel_regions.c
 @@ -111,15 +111,17 @@ debug_backtrace(void)
  GLubyte *
  intel_region_map(struct intel_context *intel, struct intel_region *region)
  {
 -   intel_flush(intel-ctx);
 -
 _DBG(%s %p\n, __FUNCTION__, region);
 if (!region-map_refcount++) {
 +  intel_flush(intel-ctx);
 +
if (region-tiling != I915_TILING_NONE)
drm_intel_gem_bo_map_gtt(region-bo);
else
drm_intel_bo_map(region-bo, GL_TRUE);
 +
region-map = region-bo-virtual;
 +  ++intel-num_mapped_regions;
 }

There's a more interesting change in here, which is the move of the
flush inside of the if statement.

We agreed that moving it in was correct, but it would be nice to record
why we thought it was correct.  The thinking I recall was:

We have the region-map_refcount controlling mapping of the BO because
in software fallbacks we may end up mapping the same buffer multiple
times on Mesa's behalf, so we refcount our mappings to make sure that
the pointer stays valid until the end of the unmap chain.  However, we
must not emit any batchbuffers between the start of mapping and the end
of unmapping, or further use of the map will be incoherent with the GPU
rendering done by that batchbuffer.  Thus we should assert that that
doesn't happen, which means that the flush is only needed on first map
of the buffer.



pgpYKQ8fvgZnv.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 3/5] glsl: Add gl_MESACurrentAttrib{Vert, Frag} internal builtin uniforms.

2011-10-10 Thread Ian Romanick

On 10/10/2011 11:06 AM, Eric Anholt wrote:

On Thu, 06 Oct 2011 14:40:28 -0700, Ian Romanicki...@freedesktop.org  wrote:

On 10/06/2011 09:36 AM, Eric Anholt wrote:

These will be used by the FF VS/FS to represent the current attributes
when they don't have an active vertex array.


I just want to make sure I grok this completely.  These are arrays of
vec4 uniforms that are used when, for example, glColor is called outside
glBegin.  Yes?


Exactly.


If that's the case, what's the advantage of having it in the fragment
shader (as opposed to using uniforms to interpolate the potentially

   
That should have been varyings.


constant values)?


Now I'm not grokking what you're suggesting.


There's two ways to get data into the fragment shader.  On is using 
uniforms, and that's what the current patch is doing.  The other is 
using varyings.  I'm just trying to understand the advantage of one over 
the other.


Or is this just a case of keeping the same behavior as the old code?


@@ -284,6 +292,8 @@ const struct gl_builtin_uniform_desc 
_mesa_builtin_uniform_desc[] = {
  STATEVAR(gl_MESABumpRotMatrix0),
  STATEVAR(gl_MESABumpRotMatrix1),
  STATEVAR(gl_MESAFogParamsOptimized),
+   STATEVAR(gl_MESACurrentAttribVert),
+   STATEVAR(gl_MESACurrentAttribFrag),


My preference would be to use the regular GLSL extension naming
convention (even though we're not exposing these names directly).  That
is, gl_CurrentAttribVertMESA and gl_CurrentAttribFragMESA.  I would have
made the same comment about the other statevar names here, but I was too
slow getting reviews out.


I'm up for that.

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


Re: [Mesa-dev] [PATCH] intel: Assert that no batch is emitted if a region is mapped

2011-10-10 Thread Ian Romanick

On 10/10/2011 11:17 AM, Eric Anholt wrote:

On Thu,  6 Oct 2011 14:18:35 -0700, Chad Versacec...@chad-versace.us  wrote:

What I would prefer to assert is that, for each region that is currently
mapped, no batch is emitted that uses that region's bo. However, it's much
easier to implement this big hammer.



diff --git a/src/mesa/drivers/dri/intel/intel_regions.c 
b/src/mesa/drivers/dri/intel/intel_regions.c
index 7faf4ca..d46c470 100644
--- a/src/mesa/drivers/dri/intel/intel_regions.c
+++ b/src/mesa/drivers/dri/intel/intel_regions.c
@@ -111,15 +111,17 @@ debug_backtrace(void)
  GLubyte *
  intel_region_map(struct intel_context *intel, struct intel_region *region)
  {
-   intel_flush(intel-ctx);
-
 _DBG(%s %p\n, __FUNCTION__, region);
 if (!region-map_refcount++) {
+  intel_flush(intel-ctx);
+
if (region-tiling != I915_TILING_NONE)
 drm_intel_gem_bo_map_gtt(region-bo);
else
 drm_intel_bo_map(region-bo, GL_TRUE);
+
region-map = region-bo-virtual;
+  ++intel-num_mapped_regions;
 }


There's a more interesting change in here, which is the move of the
flush inside of the if statement.

We agreed that moving it in was correct, but it would be nice to record
why we thought it was correct.  The thinking I recall was:

We have the region-map_refcount controlling mapping of the BO because
in software fallbacks we may end up mapping the same buffer multiple
times on Mesa's behalf, so we refcount our mappings to make sure that
the pointer stays valid until the end of the unmap chain.  However, we
must not emit any batchbuffers between the start of mapping and the end
of unmapping, or further use of the map will be incoherent with the GPU
rendering done by that batchbuffer.  Thus we should assert that that
doesn't happen, which means that the flush is only needed on first map
of the buffer.


I was going to comment that the first patch change behavior.  Having the 
above text as a comment in the code is a good idea.

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


Re: [Mesa-dev] Help for a beginner

2011-10-10 Thread Ian Romanick

On 10/06/2011 11:46 AM, Romain Failliot wrote:

2011/10/6 Ian Romanicki...@freedesktop.org:

You will have to *completely* uninstall the NVIDIA closed-source driver in
order to use the Mesa driver.  NVIDIA installs their own kernel module and
their own libGL, so it is impossible for both to live on the same system.


Well, last time I checked (2 months ago), nouveau drivers were bad for
my graphic card, its temperature increased until the system froze...
Have you heard of this issue and eventually a fix?


Usually this means that the fan throttling built-in to the card doesn't 
work correctly.  Every NVIDIA card I've ever owned has had this 
problem... even with their own drivers.  The usual solution is to use a 
fan-tweak utility to increase the fan speed.



I'll retry nouveau driver, but if the issue is still here, I'll have
to buy an ATI card.


If you're going to try and develop the drivers, you have to be able to 
run them. :)


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


Re: [Mesa-dev] GL_DEPTH_BUFFER and GL_STENCIL_BUFFER

2011-10-10 Thread Ian Romanick

On 10/08/2011 05:55 AM, Jose Fonseca wrote:

Just noticed that the latest glext.h doesn't define DEPTH_BUFFER/STENCIL_BUFFER 
anymore. This is what enum.spec says:

# Due to a syncing problem between the ARB_framebuffer_object extension
# specification and the core API specification during development, the
# following tokens were present in the .spec file for some time. They are
# not actually used anywhere in the OpenGL API or extensions and have been
# withdrawn (use DEPTH or STENCIL respectively, instead, asattachment
# parameters to GetFramebufferAttachmentParameteriv).
#DEPTH_BUFFER= 0x8223
#STENCIL_BUFFER  = 0x8224

But Mesa code actually seems to do the opposite, i.e., 
GetFramebufferAttachmentParameteriv handles DEPTH_BUFFER but not DEPTH.

The GL specs is consistent with the comment.

So I think it's better to remove this. Any objects?


As Brian mentioned, that's my patch series:

http://marc.info/?l=mesa3d-devm=131805740908194w=2

I'm planning to push both those patches today, so there's still time for 
review comments. :)

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


Re: [Mesa-dev] GL_DEPTH_BUFFER and GL_STENCIL_BUFFER

2011-10-10 Thread Brian Paul
On Mon, Oct 10, 2011 at 12:34 PM, Ian Romanick i...@freedesktop.org wrote:
 On 10/08/2011 05:55 AM, Jose Fonseca wrote:

 Just noticed that the latest glext.h doesn't define
 DEPTH_BUFFER/STENCIL_BUFFER anymore. This is what enum.spec says:

 # Due to a syncing problem between the ARB_framebuffer_object extension
 # specification and the core API specification during development, the
 # following tokens were present in the .spec file for some time. They are
 # not actually used anywhere in the OpenGL API or extensions and have been
 # withdrawn (use DEPTH or STENCIL respectively, instead, asattachment
 # parameters to GetFramebufferAttachmentParameteriv).
 #    DEPTH_BUFFER                    = 0x8223
 #    STENCIL_BUFFER                  = 0x8224

 But Mesa code actually seems to do the opposite, i.e.,
 GetFramebufferAttachmentParameteriv handles DEPTH_BUFFER but not DEPTH.

 The GL specs is consistent with the comment.

 So I think it's better to remove this. Any objects?

 As Brian mentioned, that's my patch series:

 http://marc.info/?l=mesa3d-devm=131805740908194w=2

 I'm planning to push both those patches today, so there's still time for
 review comments. :)

http://marc.info/?l=mesa3d-devm=131805740908202w=2

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


Re: [Mesa-dev] [PATCH 1/2] i965: Split brw_set_prim into brw/gen6 variants

2011-10-10 Thread Chad Versace
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 10/10/2011 10:53 AM, Eric Anholt wrote:
 On Fri,  7 Oct 2011 10:16:47 -0700, Chad Versace c...@chad-versace.us wrote:
 The slight optimization to avoid the GS program in brw_set_prim() is not
 used by Gen 6, since Gen 6 doesn't use a GS program. Also, Gen 6 doesn't use
 reduced primitives.

 Also, document that intel_context.reduced_primitive is only used for Gen  6

 Signed-off-by: Chad Versace c...@chad-versace.us
 
 diff --git a/src/mesa/drivers/dri/intel/intel_context.h 
 b/src/mesa/drivers/dri/intel/intel_context.h
 index eb78c00..cf7ab9e 100644
 --- a/src/mesa/drivers/dri/intel/intel_context.h
 +++ b/src/mesa/drivers/dri/intel/intel_context.h
 @@ -253,7 +253,7 @@ struct intel_context
 GLuint RenderIndex;
 GLmatrix ViewportMatrix;
 GLenum render_primitive;
 -   GLenum reduced_primitive;
 +   GLenum reduced_primitive; /* Only gen  6 */
 
 If this is intended to be doxygen, that would be /** whatever */
 
 Other than that, Reviewed-by: Eric Anholt e...@anholt.net

Thanks for catching that. I'll doxygenate it before pushing.

- -- 
Chad Versace
c...@chad-versace.us
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJOkz4qAAoJEAIvNt057x8iWp0P/RS97RabqRhJ7KX52hor+/w3
D4jYyBZU59Giyg3MdLkwLyCgFilyyelo83R55tKQfHU87nS2aTGTQflIEyq0L/bZ
DRJklSz/pOEU4k49WNYcomkbMruLAbt+3ps/1RqK/tx/d4Jj9JIc9dEAXNMIBAjp
SP/sf6+dEplzu4EPfAdA3cJN8ysiXox0NAd0wt1oTdxZNwxn8zn89DW5dtF8uD47
yzh4xA+PjoNZ9p9eHW+asHdsbEDog1eSHZ3dqztm1yNeBKpLjwR0J3U7s9miUXVl
QtXd/mad6vDHFfITMpu7GaBx+TQC8Evx3Nc+gCeClWWcKGQgkM4frurKL4PtxyWJ
y9JnN81vmo29j5SpK1DeWgC+sm8rbrY48+XOiiknb6dz8JxICZYwrD396v7bM+Z5
TF3muyTrngGV3WA4RU1jLrtQjI9T847hGyKD/C2dLke92VZ5onK2gmRZ05UDAtB+
5fq3WbwlN5h1jS99FjggWfObGUWiqpLgHk5jxevLFPx8sbZHBVLNHv/raiJ3RETe
aWiVefwV73HLIdxjRVOQv5EzTqPqLN2pOIG3KDdjmSudMaUA3bPzeGcDaI/8QTYe
WrIJ5Mi6cN6UwcyPsWEsy8Z/EPTlczQSJNRDkTXUNktG1UsbsV3qFEUwZprc7aTh
C3JlF/bVrQSlHt8I7wob
=WiPo
-END PGP SIGNATURE-
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] GL_DEPTH_BUFFER and GL_STENCIL_BUFFER

2011-10-10 Thread Jose Fonseca


- Original Message -
 On 10/08/2011 05:55 AM, Jose Fonseca wrote:
  Just noticed that the latest glext.h doesn't define
  DEPTH_BUFFER/STENCIL_BUFFER anymore. This is what enum.spec says:
 
  # Due to a syncing problem between the ARB_framebuffer_object
  extension
  # specification and the core API specification during development,
  the
  # following tokens were present in the .spec file for some time.
  They are
  # not actually used anywhere in the OpenGL API or extensions and
  have been
  # withdrawn (use DEPTH or STENCIL respectively, instead,
  asattachment
  # parameters to GetFramebufferAttachmentParameteriv).
  #DEPTH_BUFFER= 0x8223
  #STENCIL_BUFFER  = 0x8224
 
  But Mesa code actually seems to do the opposite, i.e.,
  GetFramebufferAttachmentParameteriv handles DEPTH_BUFFER but not
  DEPTH.
 
  The GL specs is consistent with the comment.
 
  So I think it's better to remove this. Any objects?
 
 As Brian mentioned, that's my patch series:
 
 http://marc.info/?l=mesa3d-devm=131805740908194w=2
 
 I'm planning to push both those patches today, so there's still time
 for
 review comments. :)
 

Great. Looks good to me AFAICS.

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


[Mesa-dev] [PATCH] gallium: add PIPE_BIND_BLENDABLE flag

2011-10-10 Thread Christoph Bumiller
This is required for d3d1x's CheckFormatSupport query.

It also seems generally useful for state trackers, which could
choose alternative rendering paths or formats if blending would
come at a significant performance loss.
---
 src/gallium/docs/source/screen.rst   |3 +++
 src/gallium/include/pipe/p_defines.h |9 +
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/gallium/docs/source/screen.rst 
b/src/gallium/docs/source/screen.rst
index 924858e..4a61835 100644
--- a/src/gallium/docs/source/screen.rst
+++ b/src/gallium/docs/source/screen.rst
@@ -160,6 +160,9 @@ resources might be created and handled quite differently.
 * ``PIPE_BIND_DEPTH_STENCIL``: A depth (Z) buffer and/or stencil buffer. Any
   depth/stencil surface/resource attached to pipe_framebuffer_state::zsbuf must
   have this flag set.
+* ``PIPE_BIND_BLENDABLE``: Used in conjunction with PIPE_BIND_RENDER_TARGET to 
query
+  whether a device supports blending for a given format.
+  If surface creation succeeds with this flag set, the driver must emulate 
blending.
 * ``PIPE_BIND_DISPLAY_TARGET``: A surface that can be presented to screen. 
Arguments to
   pipe_screen::flush_front_buffer must have this flag set.
 * ``PIPE_BIND_SAMPLER_VIEW``: A texture that may be sampled from in a fragment
diff --git a/src/gallium/include/pipe/p_defines.h 
b/src/gallium/include/pipe/p_defines.h
index acae4b1..447df35 100644
--- a/src/gallium/include/pipe/p_defines.h
+++ b/src/gallium/include/pipe/p_defines.h
@@ -295,10 +295,11 @@ enum pipe_transfer_usage {
  */
 #define PIPE_BIND_DEPTH_STENCIL(1  0) /* create_surface */
 #define PIPE_BIND_RENDER_TARGET(1  1) /* create_surface */
-#define PIPE_BIND_SAMPLER_VIEW (1  2) /* create_sampler_view */
-#define PIPE_BIND_VERTEX_BUFFER(1  3) /* set_vertex_buffers */
-#define PIPE_BIND_INDEX_BUFFER (1  4) /* draw_elements */
-#define PIPE_BIND_CONSTANT_BUFFER  (1  5) /* set_constant_buffer */
+#define PIPE_BIND_BLENDABLE(1  2) /* create_surface */
+#define PIPE_BIND_SAMPLER_VIEW (1  3) /* create_sampler_view */
+#define PIPE_BIND_VERTEX_BUFFER(1  4) /* set_vertex_buffers */
+#define PIPE_BIND_INDEX_BUFFER (1  5) /* draw_elements */
+#define PIPE_BIND_CONSTANT_BUFFER  (1  6) /* set_constant_buffer */
 #define PIPE_BIND_DISPLAY_TARGET   (1  8) /* flush_front_buffer */
 #define PIPE_BIND_TRANSFER_WRITE   (1  9) /* get_transfer */
 #define PIPE_BIND_TRANSFER_READ(1  10) /* get_transfer */
-- 
1.7.3.4

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


[Mesa-dev] [PATCH] intel: Assert that no batch is emitted if a region is mapped [v2]

2011-10-10 Thread Chad Versace
What I would prefer to assert is that, for each region that is currently
mapped, no batch is emitted that uses that region's bo. However, it's much
easier to implement this big hammer.

Observe that this requires that the batch flush in intel_region_map() be
moved to within the map_refcount guard.

v2: Add comments (borrowed from anholt's reply) explaining why the
assertion is a good idea.

CC: Eric Anholt e...@anholt.net
CC: Ian Romanick i...@freedesktop.org
Signed-off-by: Chad Versace c...@chad-versace.us
---
 src/mesa/drivers/dri/intel/intel_batchbuffer.c |7 +++
 src/mesa/drivers/dri/intel/intel_context.h |8 
 src/mesa/drivers/dri/intel/intel_regions.c |   18 +-
 3 files changed, 32 insertions(+), 1 deletions(-)

diff --git a/src/mesa/drivers/dri/intel/intel_batchbuffer.c 
b/src/mesa/drivers/dri/intel/intel_batchbuffer.c
index 37c13c9..8dfae677 100644
--- a/src/mesa/drivers/dri/intel/intel_batchbuffer.c
+++ b/src/mesa/drivers/dri/intel/intel_batchbuffer.c
@@ -148,6 +148,13 @@ void
 _intel_batchbuffer_flush(struct intel_context *intel,
 const char *file, int line)
 {
+   /* No batch should be emitted that uses a mapped region, because that would
+* cause the map to be incoherent with GPU rendering done by the
+* batchbuffer. To ensure that condition, we assert a condition that is
+* stronger but easier to implement: that *no* region is mapped.
+*/
+   assert(intel-num_mapped_regions == 0);
+
if (intel-batch.used == 0)
   return;
 
diff --git a/src/mesa/drivers/dri/intel/intel_context.h 
b/src/mesa/drivers/dri/intel/intel_context.h
index eb78c00..f3d0bcc 100644
--- a/src/mesa/drivers/dri/intel/intel_context.h
+++ b/src/mesa/drivers/dri/intel/intel_context.h
@@ -287,6 +287,14 @@ struct intel_context
 */
GLboolean is_front_buffer_reading;
 
+   /**
+* Count of intel_regions that are mapped.
+*
+* This allows us to assert that no batch buffer is emitted if a
+* region is mapped.
+*/
+   int num_mapped_regions;
+
GLboolean use_texture_tiling;
GLboolean use_early_z;
 
diff --git a/src/mesa/drivers/dri/intel/intel_regions.c 
b/src/mesa/drivers/dri/intel/intel_regions.c
index 7faf4ca..67142a1 100644
--- a/src/mesa/drivers/dri/intel/intel_regions.c
+++ b/src/mesa/drivers/dri/intel/intel_regions.c
@@ -111,15 +111,28 @@ debug_backtrace(void)
 GLubyte *
 intel_region_map(struct intel_context *intel, struct intel_region *region)
 {
-   intel_flush(intel-ctx);
+   /* We have the region-map_refcount controlling mapping of the BO because
+* in software fallbacks we may end up mapping the same buffer multiple
+* times on Mesa's behalf, so we refcount our mappings to make sure that
+* the pointer stays valid until the end of the unmap chain.  However, we
+* must not emit any batchbuffers between the start of mapping and the end
+* of unmapping, or further use of the map will be incoherent with the GPU
+* rendering done by that batchbuffer. Hence we assert in
+* intel_batchbuffer_flush() that that doesn't happen, which means that the
+* flush is only needed on first map of the buffer.
+*/
 
_DBG(%s %p\n, __FUNCTION__, region);
if (!region-map_refcount++) {
+  intel_flush(intel-ctx);
+
   if (region-tiling != I915_TILING_NONE)
 drm_intel_gem_bo_map_gtt(region-bo);
   else
 drm_intel_bo_map(region-bo, GL_TRUE);
+
   region-map = region-bo-virtual;
+  ++intel-num_mapped_regions;
}
 
return region-map;
@@ -134,7 +147,10 @@ intel_region_unmap(struct intel_context *intel, struct 
intel_region *region)
 drm_intel_gem_bo_unmap_gtt(region-bo);
   else
 drm_intel_bo_unmap(region-bo);
+
   region-map = NULL;
+  --intel-num_mapped_regions;
+  assert(intel-num_mapped_regions = 0);
}
 }
 
-- 
1.7.6.4

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


Re: [Mesa-dev] R: Re: libgallium.so and miscelaneous buildsystem patches

2011-10-10 Thread Eric Anholt
On Thu, 6 Oct 2011 13:24:20 +0200 (CEST), Fabio fabio@libero.it wrote:
 Da: kei...@vmware.com
 Data: 05/10/2011 11.39
 A: Christopher James Halse Rogerschristopher.halse.rog...@canonical.com
 Cc: Joakim Sindholtopensou...@zhasha.com, mesa-dev@lists.freedesktop.
 org, Fabiofabio@libero.it
 Ogg: Re: [Mesa-dev] libgallium.so and miscelaneous buildsystem patches
 
 On Wed, 2011-10-05 at 20:14 +1100, Christopher James Halse Rogers wrote:
  On Wed, 2011-10-05 at 09:24 +0200, Joakim Sindholt wrote:
   On Tue, 2011-10-04 at 17:58 +0200, Fabio wrote:
Can the patches at
http://lists.freedesktop.org/archives/mesa-dev/2011-August/011099.html
be considered for merging?

Sharing libgallium should save some MB of installed space.
   
   And be an ABI nightmare for distributions
  
  No; it's a private library.  Distributions will happily ship a
  libgallium built from exactly the same source that the DRI drivers are
  built from.  Indeed, that's what currently happens for those
  distributions with ship with --enable-shared-dricore, and what happens
  in Ubuntu, where we've got this patch series applied in our never-ending
  quest to cram a fully-featured linux system on a 700MB CD.
  
  Saving 20-odd megabytes is really useful there :)
 
 An alternative would be to build all the drivers into a single library
 for maximal sharing.
 
 A single library with all the drivers will however be bigger (compared to the 
 single driver + eventual shared libs) and it would use more memory.

It would be larger than any one of the drivers, but I don't see how it
would be larger than the sum of the split drivers plus the shared code,
which is what matters for disks.  And, with paging, I don't see how it
would use more memory at runtime.


pgpk6VwRfojkF.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] i965 Gen6+: De-compact clip plane constants for old VS backend.

2011-10-10 Thread Paul Berry
In commit 018ea68d8780ab5baeef0b8122b8410e5e55ae6d, when I
de-compacted clip planes on Gen6+, I updated both the old and new VS
back-ends to reflect the change in how clip planes are stored, but I
failed to change the code in gen6_vs_state.c that uploads clip plane
constants when using the old VS back-end.

As a result, if the set of enabled clip planes wasn't contiguous
starting with 0, then clipping would not occur properly.  This patch
corrects gen6_vs_state.c to upload clip plane constants in the new
de-compacted form.

This only affects the old VS back-end (which is used for
fixed-function and ARB vertex programs, not for GLSL vertex shaders).

Fixes Piglit test fixed-clip-enables.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=41603
---
 src/mesa/drivers/dri/i965/gen6_vs_state.c |   15 +++
 1 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/gen6_vs_state.c 
b/src/mesa/drivers/dri/i965/gen6_vs_state.c
index 14a96fc..1c57a3b 100644
--- a/src/mesa/drivers/dri/i965/gen6_vs_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_vs_state.c
@@ -76,15 +76,14 @@ gen6_prepare_vs_push_constants(struct brw_context *brw)
  /* This should be loaded like any other param, but it's ad-hoc
   * until we redo the VS backend.
   */
- if (!uses_clip_distance) {
+ if (ctx-Transform.ClipPlanesEnabled != 0  !uses_clip_distance) {
 gl_clip_plane *clip_planes = brw_select_clip_planes(ctx);
-for (i = 0; i  MAX_CLIP_PLANES; i++) {
-   if (ctx-Transform.ClipPlanesEnabled  (1  i)) {
-  memcpy(param, clip_planes[i], 4 * sizeof(float));
-  param += 4;
-  params_uploaded++;
-   }
-}
+int num_userclip_plane_consts
+   = _mesa_logbase2(ctx-Transform.ClipPlanesEnabled) + 1;
+int num_floats = 4 * num_userclip_plane_consts;
+memcpy(param, clip_planes, num_floats * sizeof(float));
+param += num_floats;
+params_uploaded += num_userclip_plane_consts;
  }
 
  /* Align to a reg for convenience for brw_vs_emit.c */
-- 
1.7.6.4

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


[Mesa-dev] gl_NormalMatrix issue on Intel driver

2011-10-10 Thread tom fogal
One of our programs which relies on shaders heavily is having issues,
and I have tracked it down to unexpected values in gl_NormalMatrix
within the fragment shader.

Using the attached program (patch against mesademos repo) I did
printf-esque debugging to get the values of gl_NormalMatrix.  Search
for 'fragShaderText' to see the shaders I'm using.  I get different
results on a Mesa 7.10.2 (Ubuntu 11.04) system with an Intel card
versus an nvidia-binary-blob system.  The nvidia-based system also
agrees with what I get using any card on a Mac and nvidia or ATI
cards on Windows (native drivers, not Mesa); we have no results for
Windows/Intel yet.

  nvidia  intel
[ 1.0   0.0   0.0 ][ 1.0   0.0   0.0  ]
[ 0.0  -0.0   1.0 ][ 0.0   0.0  0.0, 0.0001 ]
[ 0.0  -1.0  -0.0 ][ 1.0  15.0   0.0  ]

I used -0.0 for a couple slots on the nvidia system; the value was
smaller than 0.0, but larger than 0-epsilon for some small epsilon,
similar to gl_NormalMatrix[1].z on intel but in the opposite direction.

I spot-checked gl_NormalMatrix[2].y with LIBGL_ALWAYS_SOFTWARE=1.  In
that case, Mesa agrees with the nvidia driver: the value is -1.0.  My
application also produces imagery identical (via a subjective eye test,
haven't tried image differencing the two) to the nvidia system when I
run it with LIBGL_ALWAYS_SOFTWARE=1.

On the intel system:

  GL_VENDOR: Tungsten Graphics, Inc
  GL_RENDERER: Mesa DRI Intel(R) Sandybridge Desktop GEM 20100330 DEVELOPMENT
  GL_VERSION: 2.1 Mesa 7.10.2

From 'dmesg', I gather the GPU is an i915.

Is this a known issue?  Any workarounds available?  Anything else I
could do to help debug?

Thanks,

-tom

From c4b3d54e4d0ca991b8d76a4391d9c820fbb22747 Mon Sep 17 00:00:00 2001
From: Tom Fogal tfo...@alumni.unh.edu
Date: Thu, 6 Oct 2011 14:37:40 -0600
Subject: [PATCH] Add new test program to test gl_NormalMatrix issue.

Play with the 'if' condition in fragShaderText to identify what
any given gl_NormalMatrix entry is.
---
 src/demos/CMakeLists.txt |1 +
 src/demos/normal.c   |  611 ++
 2 files changed, 612 insertions(+), 0 deletions(-)
 create mode 100644 src/demos/normal.c

diff --git a/src/demos/CMakeLists.txt b/src/demos/CMakeLists.txt
index f1bcc03..b35fae6 100644
--- a/src/demos/CMakeLists.txt
+++ b/src/demos/CMakeLists.txt
@@ -51,6 +51,7 @@ set (targets
 	lodbias
 	morph3d
 	multiarb
+	normal
 	paltex
 	pixeltest
 	pointblast
diff --git a/src/demos/normal.c b/src/demos/normal.c
new file mode 100644
index 000..04ae5c5
--- /dev/null
+++ b/src/demos/normal.c
@@ -0,0 +1,611 @@
+/**
+ * Test gl_NormalMatrix.
+ * Tom Fogal
+ * 5 Oct 2011
+ *
+ * Based on Test OpenGL 2.0 vertex/fragment shaders.
+ * Brian Paul
+ * 1 November 2006
+ *
+ * Based on ARB version by:
+ * Michal Krol
+ * 20 February 2006
+ *
+ * Based on the original demo by:
+ * Brian Paul
+ * 17 April 2003
+ */
+
+#include assert.h
+#include string.h
+#include stdio.h
+#include stdlib.h
+#include math.h
+#include GL/glew.h
+#include glut_wrap.h
+
+
+#define TEXTURE 0
+
+static GLint CoordAttrib = 0;
+
+static char *FragProgFile = NULL;
+static char *VertProgFile = NULL;
+
+static GLfloat diffuse[4] = { 0.5f, 0.5f, 1.0f, 1.0f };
+static GLfloat specular[4] = { 0.8f, 0.8f, 0.8f, 1.0f };
+static GLfloat lightPos[4] = { 0.0f, 10.0f, 20.0f, 0.0f };
+static GLfloat delta = 1.0f;
+
+static GLuint fragShader;
+static GLuint vertShader;
+static GLuint program;
+
+static GLuint SphereList, RectList, CurList;
+static GLint win = 0;
+static GLboolean anim = GL_TRUE;
+static GLboolean wire = GL_FALSE;
+static GLboolean pixelLight = GL_TRUE;
+
+static GLint t0 = 0;
+static GLint frames = 0;
+
+static GLfloat xRot = 90.0f, yRot = 0.0f;
+
+
+static void
+normalize(GLfloat *dst, const GLfloat *src)
+{
+   GLfloat len = sqrt(src[0] * src[0] + src[1] * src[1] + src[2] * src[2]);
+   dst[0] = src[0] / len;
+   dst[1] = src[1] / len;
+   dst[2] = src[2] / len;
+   dst[3] = src[3];
+}
+
+
+static void
+Redisplay(void)
+{
+   GLfloat vec[4];
+
+   glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
+
+   /* update light position */
+   normalize(vec, lightPos);
+   glLightfv(GL_LIGHT0, GL_POSITION, vec);
+   
+   if (pixelLight) {
+  glUseProgram(program);
+  glDisable(GL_LIGHTING);
+   }
+   else {
+  glUseProgram(0);
+  glEnable(GL_LIGHTING);
+   }
+
+   glPushMatrix();
+   glRotatef(xRot, 1.0f, 0.0f, 0.0f);
+   glRotatef(yRot, 0.0f, 1.0f, 0.0f);
+   /*
+   glutSolidSphere(2.0, 10, 5);
+   */
+   glCallList(CurList);
+   glPopMatrix();
+
+   glutSwapBuffers();
+   frames++;
+
+   if (anim) {
+  GLint t = glutGet(GLUT_ELAPSED_TIME);
+  if (t - t0 = 5000) {
+ GLfloat seconds =(GLfloat)(t - t0) / 1000.0f;
+ GLfloat fps = frames / seconds;
+ printf(%d frames in %6.3f seconds = %6.3f FPS\n,
+frames, seconds, fps);
+ fflush(stdout);
+ t0 = t;
+ frames = 0;
+  }
+   }
+}
+
+
+static void

[Mesa-dev] i965 Data Port fixes and clean-ups

2011-10-10 Thread Kenneth Graunke
Patch 1 fixes the variable indexing Piglit tests on Ivybridge.  I was
hoping it would also fix bug 41318 (rendering issues in GLBenchmark/PRO)
as well, but alas.  Still, seems worthwhile; the old code was clearly wrong.

Patch 2 may be somewhat controversial, and implements a change in behavior
on Sandybridge.  Not observed to help or hurt anything yet.

The rest of the series cleans up and documents the code.

Tested via full Piglit runs on GM45, Ironlake, Sandybridge, Ivybridge (GT2).
Not tested on Broadwater/Crestline because I didn't have one of those handy.

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


[Mesa-dev] [PATCH 01/10] i965: Use Ivybridge's Legacy Data Port for reads/writes.

2011-10-10 Thread Kenneth Graunke
Using the constant cache for reads isn't going to work for scratch
reads (variably-indexed arrays or register spills), as these aren't
constant at all.

Also, in the new VS backend, use the proper message number for OWord
Dual Block Write messages.  It's now 10, instead of 9.  Prior to this,
we were attempting to use the render cache with message 9, which was
Memory Barrier.  Not quite the desired effect.

+205 piglits.

NOTE: This is a candidate for the 7.11 branch.

Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
 src/mesa/drivers/dri/i965/brw_defines.h |5 +
 src/mesa/drivers/dri/i965/brw_eu_emit.c |   12 
 src/mesa/drivers/dri/i965/brw_vec4_emit.cpp |4 +++-
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_defines.h 
b/src/mesa/drivers/dri/i965/brw_defines.h
index 21a115b..dee4cef 100644
--- a/src/mesa/drivers/dri/i965/brw_defines.h
+++ b/src/mesa/drivers/dri/i965/brw_defines.h
@@ -758,6 +758,8 @@ enum opcode {
 #define GEN6_MESSAGE_TARGET_DP_RENDER_CACHE   5
 #define GEN6_MESSAGE_TARGET_DP_CONST_CACHE9
 
+#define GEN7_MESSAGE_TARGET_DP_DATA_CACHE 10
+
 #define BRW_SAMPLER_RETURN_FORMAT_FLOAT32 0
 #define BRW_SAMPLER_RETURN_FORMAT_UINT32  2
 #define BRW_SAMPLER_RETURN_FORMAT_SINT32  3
@@ -855,6 +857,9 @@ enum opcode {
 #define GEN6_DATAPORT_WRITE_MESSAGE_STREAMED_VB_WRITE   13
 #define GEN6_DATAPORT_WRITE_MESSAGE_RENDER_TARGET_UNORM_WRITE   14
 
+/* GEN7 */
+#define GEN7_DATAPORT_WRITE_MESSAGE_OWORD_DUAL_BLOCK_WRITE  10
+
 #define BRW_MATH_FUNCTION_INV  1
 #define BRW_MATH_FUNCTION_LOG  2
 #define BRW_MATH_FUNCTION_EXP  3
diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c 
b/src/mesa/drivers/dri/i965/brw_eu_emit.c
index 5caebfc..bcd640e 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
@@ -546,6 +546,13 @@ brw_set_dp_write_message(struct brw_compile *p,
brw_set_src1(p, insn, brw_imm_ud(0));
 
if (intel-gen = 7) {
+  /* Use the Render Cache for RT writes; otherwise use the Data Cache */
+  unsigned sfid = GEN7_MESSAGE_TARGET_DP_DATA_CACHE;
+  if (msg_type == GEN6_DATAPORT_WRITE_MESSAGE_RENDER_TARGET_WRITE)
+sfid = GEN6_MESSAGE_TARGET_DP_RENDER_CACHE;
+
+  insn-header.destreg__conditionalmod = sfid;
+
   insn-bits3.gen7_dp.binding_table_index = binding_table_index;
   insn-bits3.gen7_dp.msg_control = msg_control;
   insn-bits3.gen7_dp.pixel_scoreboard_clear = pixel_scoreboard_clear;
@@ -554,9 +561,6 @@ brw_set_dp_write_message(struct brw_compile *p,
   insn-bits3.gen7_dp.response_length = response_length;
   insn-bits3.gen7_dp.msg_length = msg_length;
   insn-bits3.gen7_dp.end_of_thread = end_of_thread;
-
-  /* We always use the render cache for write messages */
-  insn-header.destreg__conditionalmod = 
GEN6_MESSAGE_TARGET_DP_RENDER_CACHE;
} else if (intel-gen == 6) {
   insn-bits3.gen6_dp.binding_table_index = binding_table_index;
   insn-bits3.gen6_dp.msg_control = msg_control;
@@ -618,7 +622,7 @@ brw_set_dp_read_message(struct brw_compile *p,
   insn-bits3.gen7_dp.response_length = response_length;
   insn-bits3.gen7_dp.msg_length = msg_length;
   insn-bits3.gen7_dp.end_of_thread = 0;
-  insn-header.destreg__conditionalmod = 
GEN6_MESSAGE_TARGET_DP_CONST_CACHE;
+  insn-header.destreg__conditionalmod = GEN7_MESSAGE_TARGET_DP_DATA_CACHE;
} else if (intel-gen == 6) {
   uint32_t target_function;
 
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp
index ccbad25..750b6c3 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp
@@ -450,7 +450,9 @@ vec4_visitor::generate_scratch_write(vec4_instruction *inst,
 
uint32_t msg_type;
 
-   if (intel-gen = 6)
+   if (intel-gen = 7)
+  msg_type = GEN7_DATAPORT_WRITE_MESSAGE_OWORD_DUAL_BLOCK_WRITE;
+   else if (intel-gen == 6)
   msg_type = GEN6_DATAPORT_WRITE_MESSAGE_OWORD_DUAL_BLOCK_WRITE;
else
   msg_type = BRW_DATAPORT_WRITE_MESSAGE_OWORD_DUAL_BLOCK_WRITE;
-- 
1.7.7

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


[Mesa-dev] [PATCH 03/10] i965: Rename BRW_MESSAGE_TARGET_* to BRW_SFID_* and document them.

2011-10-10 Thread Kenneth Graunke
When reading the data port code, it was not clear to me what these
values meant, nor where I could find them in the documentation.
Especially since the latest BSpec and older PRMs document them in
radically different places...neither of which are near the descriptions
of individual messages.

Cite the documentation, and rename them to SFID to signify that these
are Shared Function IDs that one can read about in the GPU overview,
rather than arbitrary bitfields.  While we're add it, make them an enum.

Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
 src/mesa/drivers/dri/i965/brw_defines.h |   36 ---
 src/mesa/drivers/dri/i965/brw_disasm.c  |   49 ---
 src/mesa/drivers/dri/i965/brw_eu_emit.c |   46 ++--
 3 files changed, 73 insertions(+), 58 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_defines.h 
b/src/mesa/drivers/dri/i965/brw_defines.h
index dee4cef..308a842 100644
--- a/src/mesa/drivers/dri/i965/brw_defines.h
+++ b/src/mesa/drivers/dri/i965/brw_defines.h
@@ -745,18 +745,32 @@ enum opcode {
 #define BRW_POLYGON_FACING_FRONT  0
 #define BRW_POLYGON_FACING_BACK   1
 
-#define BRW_MESSAGE_TARGET_NULL   0
-#define BRW_MESSAGE_TARGET_MATH   1 /* reserved on GEN6 */
-#define BRW_MESSAGE_TARGET_SAMPLER2
-#define BRW_MESSAGE_TARGET_GATEWAY3
-#define BRW_MESSAGE_TARGET_DATAPORT_READ  4
-#define BRW_MESSAGE_TARGET_DATAPORT_WRITE 5
-#define BRW_MESSAGE_TARGET_URB6
-#define BRW_MESSAGE_TARGET_THREAD_SPAWNER 7
+/**
+ * Message target: Shared Function ID for where to SEND a message.
+ *
+ * These are enumerated in the ISA reference under send - Send Message.
+ * In particular, see the following tables:
+ * - G45 PRM, Volume 4, Table 14-15 Message Descriptor Definition
+ * - Sandybridge PRM, Volume 4 Part 2, Table 8-16 Extended Message Descriptor
+ * - BSpec, Volume 1a (GPU Overview) / Graphics Processing Engine (GPE) /
+ *   Overview / GPE Function IDs
+ */
+enum brw_message_target {
+   BRW_SFID_NULL = 0,
+   BRW_SFID_MATH = 1, /* Only valid on Gen4-5 */
+   BRW_SFID_SAMPLER  = 2,
+   BRW_SFID_MESSAGE_GATEWAY  = 3,
+   BRW_SFID_DATAPORT_READ= 4,
+   BRW_SFID_DATAPORT_WRITE   = 5,
+   BRW_SFID_URB  = 6,
+   BRW_SFID_THREAD_SPAWNER   = 7,
 
-#define GEN6_MESSAGE_TARGET_DP_SAMPLER_CACHE  4
-#define GEN6_MESSAGE_TARGET_DP_RENDER_CACHE   5
-#define GEN6_MESSAGE_TARGET_DP_CONST_CACHE9
+   GEN6_SFID_DATAPORT_SAMPLER_CACHE  = 4,
+   GEN6_SFID_DATAPORT_RENDER_CACHE   = 5,
+   GEN6_SFID_DATAPORT_CONSTANT_CACHE = 9,
+
+   GEN7_SFID_DATAPORT_DATA_CACHE = 10,
+};
 
 #define GEN7_MESSAGE_TARGET_DP_DATA_CACHE 10
 
diff --git a/src/mesa/drivers/dri/i965/brw_disasm.c 
b/src/mesa/drivers/dri/i965/brw_disasm.c
index 5782106..515fd7b 100644
--- a/src/mesa/drivers/dri/i965/brw_disasm.c
+++ b/src/mesa/drivers/dri/i965/brw_disasm.c
@@ -299,26 +299,27 @@ char *end_of_thread[2] = {
 };
 
 char *target_function[16] = {
-[BRW_MESSAGE_TARGET_NULL] = null,
-[BRW_MESSAGE_TARGET_MATH] = math,
-[BRW_MESSAGE_TARGET_SAMPLER] = sampler,
-[BRW_MESSAGE_TARGET_GATEWAY] = gateway,
-[BRW_MESSAGE_TARGET_DATAPORT_READ] = read,
-[BRW_MESSAGE_TARGET_DATAPORT_WRITE] = write,
-[BRW_MESSAGE_TARGET_URB] = urb,
-[BRW_MESSAGE_TARGET_THREAD_SPAWNER] = thread_spawner
+[BRW_SFID_NULL] = null,
+[BRW_SFID_MATH] = math,
+[BRW_SFID_SAMPLER] = sampler,
+[BRW_SFID_MESSAGE_GATEWAY] = gateway,
+[BRW_SFID_DATAPORT_READ] = read,
+[BRW_SFID_DATAPORT_WRITE] = write,
+[BRW_SFID_URB] = urb,
+[BRW_SFID_THREAD_SPAWNER] = thread_spawner
 };
 
 char *target_function_gen6[16] = {
-[BRW_MESSAGE_TARGET_NULL] = null,
-[BRW_MESSAGE_TARGET_MATH] = math,
-[BRW_MESSAGE_TARGET_SAMPLER] = sampler,
-[BRW_MESSAGE_TARGET_GATEWAY] = gateway,
-[GEN6_MESSAGE_TARGET_DP_SAMPLER_CACHE] = sampler,
-[GEN6_MESSAGE_TARGET_DP_RENDER_CACHE] = render,
-[GEN6_MESSAGE_TARGET_DP_CONST_CACHE] = const,
-[BRW_MESSAGE_TARGET_URB] = urb,
-[BRW_MESSAGE_TARGET_THREAD_SPAWNER] = thread_spawner
+[BRW_SFID_NULL] = null,
+[BRW_SFID_MATH] = math,
+[BRW_SFID_SAMPLER] = sampler,
+[BRW_SFID_MESSAGE_GATEWAY] = gateway,
+[BRW_SFID_URB] = urb,
+[BRW_SFID_THREAD_SPAWNER] = thread_spawner,
+[GEN6_SFID_DATAPORT_SAMPLER_CACHE] = sampler,
+[GEN6_SFID_DATAPORT_RENDER_CACHE] = render,
+[GEN6_SFID_DATAPORT_CONSTANT_CACHE] = const,
+[GEN7_SFID_DATAPORT_DATA_CACHE] = data
 };
 
 char *dp_rc_msg_type_gen6[16] = {
@@ -944,7 +945,7 @@ int brw_disasm (FILE *file, struct brw_instruction *inst, 
int gen)
 
 if (inst-header.opcode == BRW_OPCODE_SEND ||
inst-header.opcode == BRW_OPCODE_SENDC) {
-   int target;
+   enum brw_message_target target;
 
if (gen = 6)
target = 

[Mesa-dev] [PATCH 04/10] i965: Document the brw_instruction Message Descriptor structures.

2011-10-10 Thread Kenneth Graunke
Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
 src/mesa/drivers/dri/i965/brw_structs.h |   29 +++--
 1 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_structs.h 
b/src/mesa/drivers/dri/i965/brw_structs.h
index e1947d5..b47be69 100644
--- a/src/mesa/drivers/dri/i965/brw_structs.h
+++ b/src/mesa/drivers/dri/i965/brw_structs.h
@@ -915,7 +915,11 @@ struct brw_instruction
   GLuint predicate_control:4;
   GLuint predicate_inverse:1;
   GLuint execution_size:3;
-  GLuint destreg__conditionalmod:4; /* destreg - send, conditionalmod - 
others */
+  /**
+   * Conditional Modifier for most instructions.  On Gen6+, this is also
+   * used for the SEND instruction's Message Target/SFID.
+   */
+  GLuint destreg__conditionalmod:4;
   GLuint acc_wr_control:1;
   GLuint cmpt_control:1;
   GLuint debug_control:1;
@@ -1060,6 +1064,11 @@ struct brw_instruction
 GLuint pad1:6;
   } ia16;
 
+  /* Extended Message Descriptor for Ironlake (Gen5) SEND instruction.
+   *
+   * Does not apply to Gen6+.  The SFID/message target moved to bits
+   * 27:24 of the header (destreg__conditionalmod); EOT is in bits3.
+   */
struct 
{
GLuint pad:26;
@@ -1373,6 +1382,11 @@ struct brw_instruction
 GLuint end_of_thread:1;
   } gen7_dp;
 
+  /**
+   * Message Descriptor for Gen4 SEND instructions (no particular message).
+   *
+   * See the G45 PRM, Volume 4, Table 14-15.
+   */
   struct {
 GLuint function_control:16;
 GLuint response_length:4;
@@ -1382,7 +1396,18 @@ struct brw_instruction
 GLuint end_of_thread:1;
   } generic;
 
-  /* Of this struct, only end_of_thread is not present for gen6. */
+  /**
+   * Message Descriptor for Gen5-7 SEND instructions.
+   *
+   * See the Sandybridge PRM, Volume 2 Part 2, Table 8-15.  (Sadly, most
+   * of the information on the SEND instruction is missing from the public
+   * Ironlake PRM.)
+   *
+   * The table claims that bit 31 is reserved/MBZ on Gen6+, but it lies.
+   * According to the SEND instruction description:
+   * The MSb of the message description, the EOT field, always comes from
+   *  bit 127 of the instruction word...which is bit 31 of this field.
+   */
   struct {
 GLuint function_control:19;
 GLuint header_present:1;
-- 
1.7.7

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


[Mesa-dev] [PATCH 02/10] i965: Use the Render Cache for all data port reads on Sandybridge.

2011-10-10 Thread Kenneth Graunke
I can see no reason why we'd use the sampler cache; these reads are
either scratch data (spilling or variably-indexed arrays) or pull
constants.  We write through the render cache, so being symmetric is
probably a good thing.

From a code archaeology perspective, our use of the sampler cache likely
came about on accident.  Gen4-5 had separate READ/WRITE data port SFIDs,
while Gen6+ have separate SFIDs for each cache.  It turns out that the
Gen6 Sampler Cache and Gen4-5 READ data port both had SFID 4.  The
original code apparently tried to preserve the notion of read/write
data ports until later cleanups labeled it properly as Sampler Cache.

NOTE: This is a candidate for the 7.11 branch.

Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
 src/mesa/drivers/dri/i965/brw_eu_emit.c |9 +
 1 files changed, 1 insertions(+), 8 deletions(-)

Eric seemed to think that using the sampler cache was intentional.  Reading
through git log/git blame leads me to believe that it was an accident...

We may decide to leave the existing behavior, but I'd at least like to see it
commented.

diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c 
b/src/mesa/drivers/dri/i965/brw_eu_emit.c
index bcd640e..6775ce2 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
@@ -624,13 +624,6 @@ brw_set_dp_read_message(struct brw_compile *p,
   insn-bits3.gen7_dp.end_of_thread = 0;
   insn-header.destreg__conditionalmod = GEN7_MESSAGE_TARGET_DP_DATA_CACHE;
} else if (intel-gen == 6) {
-  uint32_t target_function;
-
-  if (target_cache == BRW_DATAPORT_READ_TARGET_DATA_CACHE)
-target_function = GEN6_MESSAGE_TARGET_DP_SAMPLER_CACHE;
-  else
-target_function = GEN6_MESSAGE_TARGET_DP_RENDER_CACHE;
-
   insn-bits3.gen6_dp.binding_table_index = binding_table_index;
   insn-bits3.gen6_dp.msg_control = msg_control;
   insn-bits3.gen6_dp.pixel_scoreboard_clear = 0;
@@ -640,7 +633,7 @@ brw_set_dp_read_message(struct brw_compile *p,
   insn-bits3.gen6_dp.response_length = response_length;
   insn-bits3.gen6_dp.msg_length = msg_length;
   insn-bits3.gen6_dp.end_of_thread = 0;
-  insn-header.destreg__conditionalmod = target_function;
+  insn-header.destreg__conditionalmod = 
GEN6_MESSAGE_TARGET_DP_RENDER_CACHE;
} else if (intel-gen == 5) {
   insn-bits3.dp_read_gen5.binding_table_index = binding_table_index;
   insn-bits3.dp_read_gen5.msg_control = msg_control;
-- 
1.7.7

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


[Mesa-dev] [PATCH 05/10] i965: Remove EOT parameter from brw_SAMPLE and brw_set_sampler_message.

2011-10-10 Thread Kenneth Graunke
The existing code asserted that eot == 0, as it doesn't make sense for
a thread to sample a texture as the last thing it does.

It doesn't make much sense to pass around a dead parameter either.
Especially for a function which already has a long parameter list.

So, remove the parameter and just set EOT to 0.

Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
 src/mesa/drivers/dri/i965/brw_eu.h|1 -
 src/mesa/drivers/dri/i965/brw_eu_emit.c   |   14 +-
 src/mesa/drivers/dri/i965/brw_fs_emit.cpp |1 -
 src/mesa/drivers/dri/i965/brw_wm_emit.c   |2 --
 4 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_eu.h 
b/src/mesa/drivers/dri/i965/brw_eu.h
index 8bb..716a917 100644
--- a/src/mesa/drivers/dri/i965/brw_eu.h
+++ b/src/mesa/drivers/dri/i965/brw_eu.h
@@ -926,7 +926,6 @@ void brw_SAMPLE(struct brw_compile *p,
GLuint msg_type,
GLuint response_length,
GLuint msg_length,
-   GLboolean eot,
GLuint header_present,
GLuint simd_mode);
 
diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c 
b/src/mesa/drivers/dri/i965/brw_eu_emit.c
index 78ddf94..28bd52f 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
@@ -676,13 +676,11 @@ static void brw_set_sampler_message(struct brw_compile *p,
 GLuint msg_type,
 GLuint response_length,
 GLuint msg_length,
-GLboolean eot,
 GLuint header_present,
 GLuint simd_mode)
 {
struct brw_context *brw = p-brw;
struct intel_context *intel = brw-intel;
-   assert(eot == 0);
brw_set_src1(p, insn, brw_imm_d(0));
 
if (intel-gen = 7) {
@@ -693,7 +691,7 @@ static void brw_set_sampler_message(struct brw_compile *p,
   insn-bits3.sampler_gen7.header_present = header_present;
   insn-bits3.sampler_gen7.response_length = response_length;
   insn-bits3.sampler_gen7.msg_length = msg_length;
-  insn-bits3.sampler_gen7.end_of_thread = eot;
+  insn-bits3.sampler_gen7.end_of_thread = 0;
   insn-header.destreg__conditionalmod = BRW_SFID_SAMPLER;
} else if (intel-gen = 5) {
   insn-bits3.sampler_gen5.binding_table_index = binding_table_index;
@@ -703,12 +701,12 @@ static void brw_set_sampler_message(struct brw_compile *p,
   insn-bits3.sampler_gen5.header_present = header_present;
   insn-bits3.sampler_gen5.response_length = response_length;
   insn-bits3.sampler_gen5.msg_length = msg_length;
-  insn-bits3.sampler_gen5.end_of_thread = eot;
+  insn-bits3.sampler_gen5.end_of_thread = 0;
   if (intel-gen = 6)
  insn-header.destreg__conditionalmod = BRW_SFID_SAMPLER;
   else {
  insn-bits2.send_gen5.sfid = BRW_SFID_SAMPLER;
- insn-bits2.send_gen5.end_of_thread = eot;
+ insn-bits2.send_gen5.end_of_thread = 0;
   }
} else if (intel-is_g4x) {
   insn-bits3.sampler_g4x.binding_table_index = binding_table_index;
@@ -716,7 +714,7 @@ static void brw_set_sampler_message(struct brw_compile *p,
   insn-bits3.sampler_g4x.msg_type = msg_type;
   insn-bits3.sampler_g4x.response_length = response_length;
   insn-bits3.sampler_g4x.msg_length = msg_length;
-  insn-bits3.sampler_g4x.end_of_thread = eot;
+  insn-bits3.sampler_g4x.end_of_thread = 0;
   insn-bits3.sampler_g4x.msg_target = BRW_SFID_SAMPLER;
} else {
   insn-bits3.sampler.binding_table_index = binding_table_index;
@@ -725,7 +723,7 @@ static void brw_set_sampler_message(struct brw_compile *p,
   insn-bits3.sampler.return_format = BRW_SAMPLER_RETURN_FORMAT_FLOAT32;
   insn-bits3.sampler.response_length = response_length;
   insn-bits3.sampler.msg_length = msg_length;
-  insn-bits3.sampler.end_of_thread = eot;
+  insn-bits3.sampler.end_of_thread = 0;
   insn-bits3.sampler.msg_target = BRW_SFID_SAMPLER;
}
 }
@@ -2128,7 +2126,6 @@ void brw_SAMPLE(struct brw_compile *p,
GLuint msg_type,
GLuint response_length,
GLuint msg_length,
-   GLboolean eot,
GLuint header_present,
GLuint simd_mode)
 {
@@ -2223,7 +2220,6 @@ void brw_SAMPLE(struct brw_compile *p,
  msg_type,
  response_length, 
  msg_length,
- eot,
  header_present,
  simd_mode);
}
diff --git a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
index 4c158fe..a3c2a98 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
@@ -343,7 +343,6 @@ 

[Mesa-dev] [PATCH 06/10] i965: Factor out code for setting Message Descriptors.

2011-10-10 Thread Kenneth Graunke
Every brw_set_???_message function had duplicated code, per-generation,
to set the Message Descriptor and Extended Message Descriptor bits
(SFID, message length, response length, header present, end of thread).

However, these fields are actually specified as part of the SEND
instruction itself; individual types of messages don't even specify
them (except for header present, but that's in the same bit location).

Since these are exactly the same regardless of the message type, just
create a function to set them, using the generic message structs.  This
not only shortens the code, but hides a lot of the per-generation
complexity (like the SFID being in destreg__conditionalmod) in one spot.

Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
 src/mesa/drivers/dri/i965/brw_eu_emit.c |  195 ---
 1 files changed, 73 insertions(+), 122 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c 
b/src/mesa/drivers/dri/i965/brw_eu_emit.c
index 28bd52f..29aa39b 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
@@ -370,7 +370,48 @@ void brw_set_src1(struct brw_compile *p,
}
 }
 
+/**
+ * Set the Message Descriptor and Extended Message Descriptor fields
+ * for SEND messages.
+ *
+ * \note This zeroes out the Function Control bits, so it must be called
+ *   \b before filling out any message-specific data.  Callers can
+ *   choose not to fill in irrelevant bits; they will be zero.
+ */
+static void
+brw_set_message_descriptor(struct brw_compile *p,
+  struct brw_instruction *inst,
+  enum brw_message_target sfid,
+  unsigned msg_length,
+  unsigned response_length,
+  bool header_present,
+  bool end_of_thread)
+{
+   struct intel_context *intel = p-brw-intel;
+
+   brw_set_src1(p, inst, brw_imm_d(0));
+
+   if (intel-gen = 5) {
+  inst-bits3.generic_gen5.header_present = header_present;
+  inst-bits3.generic_gen5.response_length = response_length;
+  inst-bits3.generic_gen5.msg_length = msg_length;
+  inst-bits3.generic_gen5.end_of_thread = end_of_thread;
 
+  if (intel-gen = 6) {
+/* On Gen6+ Message target/SFID goes in bits 27:24 of the header */
+inst-header.destreg__conditionalmod = sfid;
+  } else {
+/* Set Extended Message Descriptor (ex_desc) */
+inst-bits2.send_gen5.sfid = sfid;
+inst-bits2.send_gen5.end_of_thread = end_of_thread;
+  }
+   } else {
+  inst-bits3.generic.response_length = response_length;
+  inst-bits3.generic.msg_length = msg_length;
+  inst-bits3.generic.msg_target = sfid;
+  inst-bits3.generic.end_of_thread = end_of_thread;
+   }
+}
 
 static void brw_set_math_message( struct brw_compile *p,
  struct brw_instruction *insn,
@@ -409,7 +450,8 @@ static void brw_set_math_message( struct brw_compile *p,
   break;
}
 
-   brw_set_src1(p, insn, brw_imm_d(0));
+   brw_set_message_descriptor(p, insn, BRW_SFID_MATH,
+ msg_length, response_length, false, false);
if (intel-gen == 5) {
   insn-bits3.math_gen5.function = function;
   insn-bits3.math_gen5.int_type = integer_type;
@@ -417,22 +459,12 @@ static void brw_set_math_message( struct brw_compile *p,
   insn-bits3.math_gen5.saturate = saturate;
   insn-bits3.math_gen5.data_type = dataType;
   insn-bits3.math_gen5.snapshot = 0;
-  insn-bits3.math_gen5.header_present = 0;
-  insn-bits3.math_gen5.response_length = response_length;
-  insn-bits3.math_gen5.msg_length = msg_length;
-  insn-bits3.math_gen5.end_of_thread = 0;
-  insn-bits2.send_gen5.sfid = BRW_SFID_MATH;
-  insn-bits2.send_gen5.end_of_thread = 0;
} else {
   insn-bits3.math.function = function;
   insn-bits3.math.int_type = integer_type;
   insn-bits3.math.precision = low_precision;
   insn-bits3.math.saturate = saturate;
   insn-bits3.math.data_type = dataType;
-  insn-bits3.math.response_length = response_length;
-  insn-bits3.math.msg_length = msg_length;
-  insn-bits3.math.msg_target = BRW_SFID_MATH;
-  insn-bits3.math.end_of_thread = 0;
}
 }
 
@@ -445,24 +477,15 @@ static void brw_set_ff_sync_message(struct brw_compile *p,
 {
struct brw_context *brw = p-brw;
struct intel_context *intel = brw-intel;
-   brw_set_src1(p, insn, brw_imm_d(0));
 
+   brw_set_message_descriptor(p, insn, BRW_SFID_URB,
+ 1, response_length, true, end_of_thread);
insn-bits3.urb_gen5.opcode = 1; /* FF_SYNC */
insn-bits3.urb_gen5.offset = 0; /* Not used by FF_SYNC */
insn-bits3.urb_gen5.swizzle_control = 0; /* Not used by FF_SYNC */
insn-bits3.urb_gen5.allocate = allocate;
insn-bits3.urb_gen5.used = 0; /* Not used by FF_SYNC */
insn-bits3.urb_gen5.complete = 0; /* Not 

[Mesa-dev] [PATCH 09/10] i965: Document most of the brw_instruction message structs.

2011-10-10 Thread Kenneth Graunke
Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
 src/mesa/drivers/dri/i965/brw_structs.h |  118 +--
 1 files changed, 79 insertions(+), 39 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_structs.h 
b/src/mesa/drivers/dri/i965/brw_structs.h
index af83511..85f8318 100644
--- a/src/mesa/drivers/dri/i965/brw_structs.h
+++ b/src/mesa/drivers/dri/i965/brw_structs.h
@@ -1165,6 +1165,50 @@ struct brw_instruction
 int uip:16;
   } break_cont;
 
+  /**
+   * \defgroup SEND instructions / Message Descriptors
+   *
+   * @{
+   */
+
+  /**
+   * Generic Message Descriptor for Gen4 SEND instructions.  The structs
+   * below expand function_control to something specific for their
+   * message.  Due to struct packing issues, they duplicate these bits.
+   *
+   * See the G45 PRM, Volume 4, Table 14-15.
+   */
+  struct {
+GLuint function_control:16;
+GLuint response_length:4;
+GLuint msg_length:4;
+GLuint msg_target:4;
+GLuint pad1:3;
+GLuint end_of_thread:1;
+  } generic;
+
+  /**
+   * Generic Message Descriptor for Gen5-7 SEND instructions.
+   *
+   * See the Sandybridge PRM, Volume 2 Part 2, Table 8-15.  (Sadly, most
+   * of the information on the SEND instruction is missing from the public
+   * Ironlake PRM.)
+   *
+   * The table claims that bit 31 is reserved/MBZ on Gen6+, but it lies.
+   * According to the SEND instruction description:
+   * The MSb of the message description, the EOT field, always comes from
+   *  bit 127 of the instruction word...which is bit 31 of this field.
+   */
+  struct {
+GLuint function_control:19;
+GLuint header_present:1;
+GLuint response_length:5;
+GLuint msg_length:4;
+GLuint pad1:2;
+GLuint end_of_thread:1;
+  } generic_gen5;
+
+  /** G45 PRM, Volume 4, Section 6.1.1.1 */
   struct {
 GLuint function:4;
 GLuint int_type:1;
@@ -1179,6 +1223,7 @@ struct brw_instruction
 GLuint end_of_thread:1;
   } math;
 
+  /** Ironlake PRM, Volume 4 Part 1, Section 6.1.1.1 */
   struct {
 GLuint function:4;
 GLuint int_type:1;
@@ -1194,6 +1239,7 @@ struct brw_instruction
 GLuint end_of_thread:1;
   } math_gen5;
 
+  /** G45 PRM, Volume 4, Section 4.8.1.1.1 [DevBW] and [DevCL] */
   struct {
 GLuint binding_table_index:8;
 GLuint sampler:4;
@@ -1206,6 +1252,7 @@ struct brw_instruction
 GLuint end_of_thread:1;
   } sampler;
 
+  /** G45 PRM, Volume 4, Section 4.8.1.1.2 [DevCTG] */
   struct {
  GLuint binding_table_index:8;
  GLuint sampler:4;
@@ -1217,6 +1264,7 @@ struct brw_instruction
  GLuint end_of_thread:1;
   } sampler_g4x;
 
+  /** Ironlake PRM, Volume 4 Part 1, Section 4.11.1.1.3 */
   struct {
 GLuint binding_table_index:8;
 GLuint sampler:4;
@@ -1274,6 +1322,7 @@ struct brw_instruction
 GLuint end_of_thread:1;
   } urb_gen7;
 
+  /** 965 PRM, Volume 4, Section 5.10.1.1: Message Descriptor */
   struct {
 GLuint binding_table_index:8;
 GLuint msg_control:4;  
@@ -1286,6 +1335,7 @@ struct brw_instruction
 GLuint end_of_thread:1;
   } dp_read;
 
+  /** G45 PRM, Volume 4, Section 5.10.1.1.2 */
   struct {
 GLuint binding_table_index:8;
 GLuint msg_control:3;
@@ -1298,6 +1348,7 @@ struct brw_instruction
 GLuint end_of_thread:1;
   } dp_read_g4x;
 
+  /** Ironlake PRM, Volume 4 Part 1, Section 5.10.2.1.2. */
   struct {
 GLuint binding_table_index:8;
 GLuint msg_control:3;  
@@ -1311,6 +1362,7 @@ struct brw_instruction
 GLuint end_of_thread:1;
   } dp_read_gen5;
 
+  /** G45 PRM, Volume 4, Section 5.10.1.1.2.  For both Gen4 and G45. */
   struct {
 GLuint binding_table_index:8;
 GLuint msg_control:3;
@@ -1324,6 +1376,7 @@ struct brw_instruction
 GLuint end_of_thread:1;
   } dp_write;
 
+  /** Ironlake PRM, Volume 4 Part 1, Section 5.10.2.1.2. */
   struct {
 GLuint binding_table_index:8;
 GLuint msg_control:3;
@@ -1338,7 +1391,11 @@ struct brw_instruction
 GLuint end_of_thread:1;
   } dp_write_gen5;
 
-  /* Sandybridge DP for sample cache, constant cache, render cache */
+  /**
+   * Message for the Sandybridge Sampler Cache or Constant Cache Data Port.
+   *
+   * See the Sandybridge PRM, Volume 4 Part 1, Section 3.9.2.1.1.
+   **/
   struct {
 GLuint binding_table_index:8;
 GLuint msg_control:5;
@@ -1349,8 +1406,18 @@ struct brw_instruction
 GLuint msg_length:4;
 GLuint pad1:2;
 GLuint end_of_thread:1;
-  } dp_sampler_const_cache;
+  } gen6_dp_sampler_const_cache;
 
+  /**
+   * Message 

[Mesa-dev] [PATCH 1/2] draw/llvm: fix hard-coded number of total clip planes

2011-10-10 Thread Brian Paul
From: Brian Paul bri...@vmware.com

Instead of 12 use DRAW_TOTAL_CLIP_PLANES.  The max number of user-defined
clip planes was increased to 8 so the total number of planes is 14.
This doesn't fix any specific bug, but clearly the old code was wrong.
---
 src/gallium/auxiliary/draw/draw_context.c  |2 +-
 src/gallium/auxiliary/draw/draw_llvm.c |   19 ---
 src/gallium/auxiliary/draw/draw_llvm.h |2 +-
 src/gallium/auxiliary/draw/draw_private.h  |   12 
 .../draw/draw_pt_fetch_shade_pipeline_llvm.c   |2 +-
 5 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/src/gallium/auxiliary/draw/draw_context.c 
b/src/gallium/auxiliary/draw/draw_context.c
index 6a85b79..e1b9a15 100644
--- a/src/gallium/auxiliary/draw/draw_context.c
+++ b/src/gallium/auxiliary/draw/draw_context.c
@@ -369,7 +369,7 @@ draw_set_mapped_constant_buffer(struct draw_context *draw,
case PIPE_SHADER_VERTEX:
   draw-pt.user.vs_constants[slot] = buffer;
   draw-pt.user.vs_constants_size[slot] = size;
-  draw-pt.user.planes = (float (*) [12][4]) (draw-plane[0]);
+  draw-pt.user.planes = (float (*) [DRAW_TOTAL_CLIP_PLANES][4]) 
(draw-plane[0]);
   draw_vs_set_constants(draw, slot, buffer, size);
   break;
case PIPE_SHADER_GEOMETRY:
diff --git a/src/gallium/auxiliary/draw/draw_llvm.c 
b/src/gallium/auxiliary/draw/draw_llvm.c
index d427d2c..01659fe 100644
--- a/src/gallium/auxiliary/draw/draw_llvm.c
+++ b/src/gallium/auxiliary/draw/draw_llvm.c
@@ -191,7 +191,8 @@ create_jit_context_type(struct gallivm_state *gallivm,
 
elem_types[0] = LLVMPointerType(float_type, 0); /* vs_constants */
elem_types[1] = LLVMPointerType(float_type, 0); /* gs_constants */
-   elem_types[2] = LLVMPointerType(LLVMArrayType(LLVMArrayType(float_type, 4), 
12), 0); /* planes */
+   elem_types[2] = LLVMPointerType(LLVMArrayType(LLVMArrayType(float_type, 4),
+ DRAW_TOTAL_CLIP_PLANES), 0);
elem_types[3] = LLVMPointerType(float_type, 0); /* viewport */
elem_types[4] = LLVMArrayType(texture_type,
  PIPE_MAX_VERTEX_SAMPLERS); /* textures */
@@ -708,17 +709,21 @@ store_aos(struct gallivm_state *gallivm,
LLVMValueRef id_ptr = draw_jit_header_id(gallivm, io_ptr);
LLVMValueRef data_ptr = draw_jit_header_data(gallivm, io_ptr);
LLVMValueRef indices[3];
-   LLVMValueRef val, shift;
+   LLVMValueRef val;
+   int vertex_id_pad_edgeflag;
 
indices[0] = lp_build_const_int32(gallivm, 0);
indices[1] = index;
indices[2] = lp_build_const_int32(gallivm, 0);
 
-   /* initialize vertex id:16 = 0x, pad:3 = 0, edgeflag:1 = 1 */
-   val = lp_build_const_int32(gallivm, 0x1);
-   shift = lp_build_const_int32(gallivm, 12);
-   val = LLVMBuildShl(builder, val, shift, );
-   /* add clipmask:12 */   
+   /* If this assertion fails, it means we need to update the bit twidding
+* code here.  See struct vertex_header in draw_private.h.
+*/
+   assert(DRAW_TOTAL_CLIP_PLANES==14);
+   /* initialize vertex id:16 = 0x, pad:1 = 0, edgeflag:1 = 1 */
+   vertex_id_pad_edgeflag = (0x  16) | (1  DRAW_TOTAL_CLIP_PLANES);
+   val = lp_build_const_int32(gallivm, vertex_id_pad_edgeflag);
+   /* OR with the clipmask */
val = LLVMBuildOr(builder, val, clipmask, );   
 
/* store vertex header */
diff --git a/src/gallium/auxiliary/draw/draw_llvm.h 
b/src/gallium/auxiliary/draw/draw_llvm.h
index 375b7b8..bc36135 100644
--- a/src/gallium/auxiliary/draw/draw_llvm.h
+++ b/src/gallium/auxiliary/draw/draw_llvm.h
@@ -98,7 +98,7 @@ struct draw_jit_context
 {
const float *vs_constants;
const float *gs_constants;
-   float (*planes) [12][4];
+   float (*planes) [DRAW_TOTAL_CLIP_PLANES][4];
float *viewport;
 
struct draw_jit_texture textures[PIPE_MAX_VERTEX_SAMPLERS];
diff --git a/src/gallium/auxiliary/draw/draw_private.h 
b/src/gallium/auxiliary/draw/draw_private.h
index ef77266..b84d2b7 100644
--- a/src/gallium/auxiliary/draw/draw_private.h
+++ b/src/gallium/auxiliary/draw/draw_private.h
@@ -52,6 +52,10 @@ struct draw_llvm;
 #endif
 
 
+/** Sum of frustum planes and user-defined planes */
+#define DRAW_TOTAL_CLIP_PLANES (6 + PIPE_MAX_CLIP_PLANES)
+
+
 struct pipe_context;
 struct draw_vertex_shader;
 struct draw_context;
@@ -66,9 +70,9 @@ struct tgsi_sampler;
  * Carry some useful information around with the vertices in the prim pipe.  
  */
 struct vertex_header {
-   unsigned clipmask:12;
+   unsigned clipmask:DRAW_TOTAL_CLIP_PLANES;
unsigned edgeflag:1;
-   unsigned pad:3;
+   unsigned pad:1;
unsigned vertex_id:16;
 
float clip[4];
@@ -179,7 +183,7 @@ struct draw_context
  unsigned gs_constants_size[PIPE_MAX_CONSTANT_BUFFERS];
  
  /* pointer to planes */
- float (*planes)[12][4]; 
+ float (*planes)[DRAW_TOTAL_CLIP_PLANES][4]; 
   } user;
 
   boolean test_fse; /* 

[Mesa-dev] [PATCH 2/2] swrast: Remove redundant term in logic expression

2011-10-10 Thread Chad Versace
Fix is in {read,draw}_depth_stencil_pixels().  If depthRb == stencilRb,
then it is redundant to check depthRb-x *and* stencilRb-x.

Signed-off-by: Chad Versace c...@chad-versace.us
---
 src/mesa/swrast/s_drawpix.c |1 -
 src/mesa/swrast/s_readpix.c |1 -
 2 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/src/mesa/swrast/s_drawpix.c b/src/mesa/swrast/s_drawpix.c
index 55b31df..6535a8f 100644
--- a/src/mesa/swrast/s_drawpix.c
+++ b/src/mesa/swrast/s_drawpix.c
@@ -590,7 +590,6 @@ draw_depth_stencil_pixels(struct gl_context *ctx, GLint x, 
GLint y,
ASSERT(stencilRb);
 
if (depthRb-_BaseFormat == GL_DEPTH_STENCIL_EXT 
-   stencilRb-_BaseFormat == GL_DEPTH_STENCIL_EXT 
depthRb-Format == MESA_FORMAT_Z24_S8 
type == GL_UNSIGNED_INT_24_8 
depthRb == stencilRb 
diff --git a/src/mesa/swrast/s_readpix.c b/src/mesa/swrast/s_readpix.c
index 0f1f0ff..d120468 100644
--- a/src/mesa/swrast/s_readpix.c
+++ b/src/mesa/swrast/s_readpix.c
@@ -391,7 +391,6 @@ read_depth_stencil_pixels(struct gl_context *ctx,
stencilRb = ctx-ReadBuffer-Attachment[BUFFER_STENCIL].Renderbuffer;
 
if (depthRb-_BaseFormat == GL_DEPTH_STENCIL_EXT 
-   stencilRb-_BaseFormat == GL_DEPTH_STENCIL_EXT 
depthRb-Format == MESA_FORMAT_Z24_S8 
type == GL_UNSIGNED_INT_24_8 
depthRb == stencilRb 
-- 
1.7.6.4

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


[Mesa-dev] [PATCH 1/2] swrast: Fix fastpaths in glRead/WritePixels(GL_DEPTH_STENCIL)

2011-10-10 Thread Chad Versace
For glReadPixels, the user supplied pixels have format
GL_UNSIGNED_INT_24_8.  But, when the depthstencil buffer's format was
MESA_FORMAT_S8_Z24, the fastpath read from the buffer without reordering
the depth and stencil bits. To fix this, this patch just skips the
fastpath when the format is not MESA_FORMAT_Z24_S8.

The problem and fix for glWritePixels is analagous.

Fixes the Piglit tests below on i965/gen6 and causes no regressions.
   general/depthstencil-default_fb-drawpixels-24_8
   general/depthstencil-default_fb-readpixels-24_8
   EXT_packed_depth_stencil/fbo-depthstencil-GL_DEPTH24_STENCIL8-drawpixels-24_8
   EXT_packed_depth_stencil/fbo-depthstencil-GL_DEPTH24_STENCIL8-readpixels-24_8

Signed-off-by: Chad Versace c...@chad-versace.us
---
 src/mesa/swrast/s_drawpix.c |2 ++
 src/mesa/swrast/s_readpix.c |2 ++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/src/mesa/swrast/s_drawpix.c b/src/mesa/swrast/s_drawpix.c
index 63bfa79..55b31df 100644
--- a/src/mesa/swrast/s_drawpix.c
+++ b/src/mesa/swrast/s_drawpix.c
@@ -591,6 +591,8 @@ draw_depth_stencil_pixels(struct gl_context *ctx, GLint x, 
GLint y,
 
if (depthRb-_BaseFormat == GL_DEPTH_STENCIL_EXT 
stencilRb-_BaseFormat == GL_DEPTH_STENCIL_EXT 
+   depthRb-Format == MESA_FORMAT_Z24_S8 
+   type == GL_UNSIGNED_INT_24_8 
depthRb == stencilRb 
!scaleOrBias 
!zoom 
diff --git a/src/mesa/swrast/s_readpix.c b/src/mesa/swrast/s_readpix.c
index 6eec2fc..0f1f0ff 100644
--- a/src/mesa/swrast/s_readpix.c
+++ b/src/mesa/swrast/s_readpix.c
@@ -392,6 +392,8 @@ read_depth_stencil_pixels(struct gl_context *ctx,
 
if (depthRb-_BaseFormat == GL_DEPTH_STENCIL_EXT 
stencilRb-_BaseFormat == GL_DEPTH_STENCIL_EXT 
+   depthRb-Format == MESA_FORMAT_Z24_S8 
+   type == GL_UNSIGNED_INT_24_8 
depthRb == stencilRb 
!scaleOrBias 
!stencilTransfer) {
-- 
1.7.6.4

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


[Mesa-dev] [PATCH 2/3] mesa: remove _mesa_alloc_texmemory(), _mesa_free_texmemory()

2011-10-10 Thread Brian Paul
From: Brian Paul bri...@vmware.com

Core Mesa no longer does any texture memory allocation.
---
 src/mesa/drivers/dri/intel/intel_mipmap_tree.c   |4 +-
 src/mesa/drivers/dri/intel/intel_tex.c   |2 +-
 src/mesa/drivers/dri/intel/intel_tex_image.c |1 +
 src/mesa/drivers/dri/radeon/radeon_mipmap_tree.c |2 +-
 src/mesa/drivers/dri/radeon/radeon_texture.c |7 ++-
 src/mesa/main/teximage.c |   46 --
 src/mesa/main/teximage.h |   11 -
 7 files changed, 9 insertions(+), 64 deletions(-)

diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c 
b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
index 6b15c42..bca77f0 100644
--- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
@@ -436,8 +436,8 @@ intel_miptree_copy_teximage(struct intel_context *intel,
   }
}
 
-   if (!src_mt) {
-  _mesa_free_texmemory(intelImage-base.Data);
+   if (!src_mt  intelImage-base.Data) {
+  _mesa_align_free(intelImage-base.Data);
   intelImage-base.Data = NULL;
}
 
diff --git a/src/mesa/drivers/dri/intel/intel_tex.c 
b/src/mesa/drivers/dri/intel/intel_tex.c
index 7cbd0f5..183bba3 100644
--- a/src/mesa/drivers/dri/intel/intel_tex.c
+++ b/src/mesa/drivers/dri/intel/intel_tex.c
@@ -130,7 +130,7 @@ intel_free_texture_image_buffer(struct gl_context * ctx,
intel_miptree_release(intelImage-mt);
 
if (intelImage-base.Data) {
-  _mesa_free_texmemory(intelImage-base.Data);
+  _mesa_align_free(intelImage-base.Data);
   intelImage-base.Data = NULL;
}
 
diff --git a/src/mesa/drivers/dri/intel/intel_tex_image.c 
b/src/mesa/drivers/dri/intel/intel_tex_image.c
index c34b4c5..618368e 100644
--- a/src/mesa/drivers/dri/intel/intel_tex_image.c
+++ b/src/mesa/drivers/dri/intel/intel_tex_image.c
@@ -14,6 +14,7 @@
 #include main/texgetimage.h
 #include main/texobj.h
 #include main/teximage.h
+#include main/texstore.h
 
 #include intel_context.h
 #include intel_mipmap_tree.h
diff --git a/src/mesa/drivers/dri/radeon/radeon_mipmap_tree.c 
b/src/mesa/drivers/dri/radeon/radeon_mipmap_tree.c
index 8daeb5e..d251670 100644
--- a/src/mesa/drivers/dri/radeon/radeon_mipmap_tree.c
+++ b/src/mesa/drivers/dri/radeon/radeon_mipmap_tree.c
@@ -499,7 +499,7 @@ static void migrate_image_to_miptree(radeon_mipmap_tree *mt,
copy_rows(dest, dstlvl-rowstride, image-base.Data, 
srcrowstride,
  rows, srcrowstride);
 
-   _mesa_free_texmemory(image-base.Data);
+   _mesa_align_free(image-base.Data);
image-base.Data = 0;
}
 
diff --git a/src/mesa/drivers/dri/radeon/radeon_texture.c 
b/src/mesa/drivers/dri/radeon/radeon_texture.c
index 2d17d30..abe7510 100644
--- a/src/mesa/drivers/dri/radeon/radeon_texture.c
+++ b/src/mesa/drivers/dri/radeon/radeon_texture.c
@@ -106,14 +106,14 @@ void radeonFreeTextureImageBuffer(struct gl_context *ctx, 
struct gl_texture_imag
radeon_miptree_unreference(image-mt);
assert(!image-base.Data);
} else {
-   _mesa_free_texture_image_data(ctx, timage);
+   _swrast_free_texture_image_buffer(ctx, timage);
}
if (image-bo) {
radeon_bo_unref(image-bo);
image-bo = NULL;
}
if (image-base.Data) {
-   _mesa_free_texmemory(image-base.Data);
+   _mesa_align_free(image-base.Data);
image-base.Data = NULL;
}
 
@@ -828,7 +828,8 @@ static void radeon_teximage(
texImage-Width,

texImage-Height,

texImage-Depth);
-   image-base.Data = _mesa_alloc_texmemory(size);
+   image-base.Data = _mesa_align_malloc(size, 512);
+
radeon_print(RADEON_TEXTURE, RADEON_VERBOSE,
%s %dd: texObj %p, texImage %p, 
 no miptree assigned, using local 
memory %p\n,
diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
index 3a96693..317bab7 100644
--- a/src/mesa/main/teximage.c
+++ b/src/mesa/main/teximage.c
@@ -58,27 +58,6 @@
 
 
 /**
- * We allocate texture memory on 512-byte boundaries so we can use MMX/SSE
- * elsewhere.
- */
-void *
-_mesa_alloc_texmemory(GLsizei bytes)
-{
-   return _mesa_align_malloc(bytes, 512);
-}
-
-
-/**
- * Free texture memory allocated with _mesa_alloc_texmemory()
- */
-void
-_mesa_free_texmemory(void *m)
-{
-   _mesa_align_free(m);
-}
-
-
-/**
  * Return the simple base format for a given internal texture format.
  * For example, given GL_LUMINANCE12_ALPHA4, return GL_LUMINANCE_ALPHA.
  *
@@ -599,31 +578,6 @@ _mesa_new_texture_image( struct gl_context *ctx )
 
 

[Mesa-dev] [PATCH 3/3] mesa: add swrast_texture_image::Buffer

2011-10-10 Thread Brian Paul
From: Brian Paul bri...@vmware.com

In the past, swrast_texture_image::Data has been overloaded.  It could
either point to malloc'd memory storing texture data, or it could point
to a current mapping of GPU memory.

Now, Buffer always points to malloc'd memory (if we're not using GPU
memory) and Data always points to mapped memory.  The next step would
be to rename Data - Map.

This change also involves adding swrast functions for mapping textures
and renderbuffers prior to rendering to setup the Data pointer.  Plus,
corresponding functions to unmap texures and renderbuffers.  This is
very much like similar code in the dri drivers.
---
 src/mesa/drivers/dri/intel/intel_tex.c   |   10 +-
 src/mesa/drivers/dri/radeon/radeon_texture.c |   14 ++--
 src/mesa/swrast/s_context.c  |   24 
 src/mesa/swrast/s_context.h  |   28 +
 src/mesa/swrast/s_texrender.c|   31 +++---
 src/mesa/swrast/s_texture.c  |  151 --
 src/mesa/tnl/t_context.h |1 -
 src/mesa/tnl/t_vb_program.c  |2 +
 8 files changed, 225 insertions(+), 36 deletions(-)

diff --git a/src/mesa/drivers/dri/intel/intel_tex.c 
b/src/mesa/drivers/dri/intel/intel_tex.c
index 183bba3..9ca5156 100644
--- a/src/mesa/drivers/dri/intel/intel_tex.c
+++ b/src/mesa/drivers/dri/intel/intel_tex.c
@@ -129,9 +129,9 @@ intel_free_texture_image_buffer(struct gl_context * ctx,
 
intel_miptree_release(intelImage-mt);
 
-   if (intelImage-base.Data) {
-  _mesa_align_free(intelImage-base.Data);
-  intelImage-base.Data = NULL;
+   if (intelImage-base.Buffer) {
+  _mesa_align_free(intelImage-base.Buffer);
+  intelImage-base.Buffer = NULL;
}
 
if (intelImage-base.ImageOffsets) {
@@ -208,11 +208,11 @@ intel_map_texture_image(struct gl_context *ctx,
   assert(map);
 
   *stride = _mesa_format_row_stride(tex_image-TexFormat, width);
-  *map = intel_image-base.Data + (slice * height + y) * *stride + x * 
texelSize;
+  *map = intel_image-base.Buffer + (slice * height + y) * *stride + x * 
texelSize;
 
   DBG(%s: %d,%d %dx%d from data %p = %p/%d\n, __FUNCTION__,
  x, y, w, h,
- intel_image-base.Data, *map, *stride);
+ intel_image-base.Buffer, *map, *stride);
}
 }
 
diff --git a/src/mesa/drivers/dri/radeon/radeon_texture.c 
b/src/mesa/drivers/dri/radeon/radeon_texture.c
index abe7510..4f8daa7 100644
--- a/src/mesa/drivers/dri/radeon/radeon_texture.c
+++ b/src/mesa/drivers/dri/radeon/radeon_texture.c
@@ -104,7 +104,7 @@ void radeonFreeTextureImageBuffer(struct gl_context *ctx, 
struct gl_texture_imag
 
if (image-mt) {
radeon_miptree_unreference(image-mt);
-   assert(!image-base.Data);
+   assert(!image-base.Buffer);
} else {
_swrast_free_texture_image_buffer(ctx, timage);
}
@@ -112,9 +112,9 @@ void radeonFreeTextureImageBuffer(struct gl_context *ctx, 
struct gl_texture_imag
radeon_bo_unref(image-bo);
image-bo = NULL;
}
-   if (image-base.Data) {
-   _mesa_align_free(image-base.Data);
-   image-base.Data = NULL;
+   if (image-base.Buffer) {
+   _mesa_align_free(image-base.Buffer);
+   image-base.Buffer = NULL;
}
 
if (image-base.ImageOffsets) {
@@ -314,7 +314,7 @@ radeon_map_texture_image(struct gl_context *ctx,
assert(map);
 
*stride = _mesa_format_row_stride(texImage-TexFormat, width);
-   *map = image-base.Data + (slice * height) * *stride;
+   *map = image-base.Buffer + (slice * height) * *stride;
}
 
*map += y * *stride + x * texel_size;
@@ -828,12 +828,12 @@ static void radeon_teximage(
texImage-Width,

texImage-Height,

texImage-Depth);
-   image-base.Data = _mesa_align_malloc(size, 512);
+   image-base.Buffer = _mesa_align_malloc(size, 512);
 
radeon_print(RADEON_TEXTURE, RADEON_VERBOSE,
%s %dd: texObj %p, texImage %p, 
 no miptree assigned, using local 
memory %p\n,
-   __func__, dims, texObj, texImage, 
image-base.Data);
+   __func__, dims, texObj, texImage, 
image-base.Buffer);
}
}
 
diff --git a/src/mesa/swrast/s_context.c b/src/mesa/swrast/s_context.c
index 5287671..9112cf3 100644
--- a/src/mesa/swrast/s_context.c
+++ b/src/mesa/swrast/s_context.c
@@ -747,6 +747,12 @@ _swrast_CreateContext( struct gl_context *ctx )
swrast-AllowVertexFog = GL_TRUE;
swrast-AllowPixelFog = 

Re: [Mesa-dev] [PATCH 1/2] swrast: Fix fastpaths in glRead/WritePixels(GL_DEPTH_STENCIL)

2011-10-10 Thread Brian Paul
On Mon, Oct 10, 2011 at 7:10 PM, Chad Versace c...@chad-versace.us wrote:
 For glReadPixels, the user supplied pixels have format
 GL_UNSIGNED_INT_24_8.  But, when the depthstencil buffer's format was
 MESA_FORMAT_S8_Z24, the fastpath read from the buffer without reordering
 the depth and stencil bits. To fix this, this patch just skips the
 fastpath when the format is not MESA_FORMAT_Z24_S8.

 The problem and fix for glWritePixels is analagous.

 Fixes the Piglit tests below on i965/gen6 and causes no regressions.
   general/depthstencil-default_fb-drawpixels-24_8
   general/depthstencil-default_fb-readpixels-24_8
   
 EXT_packed_depth_stencil/fbo-depthstencil-GL_DEPTH24_STENCIL8-drawpixels-24_8
   
 EXT_packed_depth_stencil/fbo-depthstencil-GL_DEPTH24_STENCIL8-readpixels-24_8

 Signed-off-by: Chad Versace c...@chad-versace.us

Reviewed-by: Brian Paul bri...@vmware.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] swrast: Remove redundant term in logic expression

2011-10-10 Thread Brian Paul
On Mon, Oct 10, 2011 at 7:11 PM, Chad Versace c...@chad-versace.us wrote:
 Fix is in {read,draw}_depth_stencil_pixels().  If depthRb == stencilRb,
 then it is redundant to check depthRb-x *and* stencilRb-x.

 Signed-off-by: Chad Versace c...@chad-versace.us
 ---
  src/mesa/swrast/s_drawpix.c |    1 -
  src/mesa/swrast/s_readpix.c |    1 -
  2 files changed, 0 insertions(+), 2 deletions(-)

Reviewed-by: Brian Paul bri...@vmware.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 1/6] mesa: make _mesa_update_depth/stencil_buffer() static

2011-10-10 Thread Brian Paul
From: Brian Paul bri...@vmware.com

These functions were only called in framebuffer.c where they were defined.
Remove the unneeded attIndex parameter too.
---
 src/mesa/main/framebuffer.c |   34 ++
 src/mesa/main/framebuffer.h |8 
 2 files changed, 10 insertions(+), 32 deletions(-)

diff --git a/src/mesa/main/framebuffer.c b/src/mesa/main/framebuffer.c
index 42da176..8beda31 100644
--- a/src/mesa/main/framebuffer.c
+++ b/src/mesa/main/framebuffer.c
@@ -608,19 +608,12 @@ _mesa_update_framebuffer_visual(struct gl_context *ctx,
  * create and install a depth wrapper/adaptor.
  *
  * \param fb  the framebuffer whose _DepthBuffer field to update
- * \param attIndex  indicates the renderbuffer to possibly wrap
  */
-void
-_mesa_update_depth_buffer(struct gl_context *ctx,
-  struct gl_framebuffer *fb,
-  GLuint attIndex)
+static void
+update_depth_buffer(struct gl_context *ctx, struct gl_framebuffer *fb)
 {
-   struct gl_renderbuffer *depthRb;
-
-   /* only one possiblity for now */
-   ASSERT(attIndex == BUFFER_DEPTH);
-
-   depthRb = fb-Attachment[attIndex].Renderbuffer;
+   struct gl_renderbuffer *depthRb =
+  fb-Attachment[BUFFER_DEPTH].Renderbuffer;
 
if (depthRb  _mesa_is_format_packed_depth_stencil(depthRb-Format)) {
   /* The attached depth buffer is a GL_DEPTH_STENCIL renderbuffer */
@@ -655,19 +648,12 @@ _mesa_update_depth_buffer(struct gl_context *ctx,
  * create and install a stencil wrapper/adaptor.
  *
  * \param fb  the framebuffer whose _StencilBuffer field to update
- * \param attIndex  indicates the renderbuffer to possibly wrap
  */
-void
-_mesa_update_stencil_buffer(struct gl_context *ctx,
-struct gl_framebuffer *fb,
-GLuint attIndex)
+static void
+update_stencil_buffer(struct gl_context *ctx, struct gl_framebuffer *fb)
 {
-   struct gl_renderbuffer *stencilRb;
-
-   ASSERT(attIndex == BUFFER_DEPTH ||
-  attIndex == BUFFER_STENCIL);
-
-   stencilRb = fb-Attachment[attIndex].Renderbuffer;
+   struct gl_renderbuffer *stencilRb =
+  fb-Attachment[BUFFER_STENCIL].Renderbuffer;
 
if (stencilRb  _mesa_is_format_packed_depth_stencil(stencilRb-Format)) {
   /* The attached stencil buffer is a GL_DEPTH_STENCIL renderbuffer */
@@ -826,8 +812,8 @@ update_framebuffer(struct gl_context *ctx, struct 
gl_framebuffer *fb)
 */
update_color_draw_buffers(ctx, fb);
update_color_read_buffer(ctx, fb);
-   _mesa_update_depth_buffer(ctx, fb, BUFFER_DEPTH);
-   _mesa_update_stencil_buffer(ctx, fb, BUFFER_STENCIL);
+   update_depth_buffer(ctx, fb);
+   update_stencil_buffer(ctx, fb);
 
compute_depth_max(fb);
 }
diff --git a/src/mesa/main/framebuffer.h b/src/mesa/main/framebuffer.h
index 7aef3e0..ad53d8c 100644
--- a/src/mesa/main/framebuffer.h
+++ b/src/mesa/main/framebuffer.h
@@ -82,14 +82,6 @@ _mesa_update_framebuffer_visual(struct gl_context *ctx,
struct gl_framebuffer *fb);
 
 extern void
-_mesa_update_depth_buffer(struct gl_context *ctx, struct gl_framebuffer *fb,
-GLuint attIndex);
-
-extern void
-_mesa_update_stencil_buffer(struct gl_context *ctx, struct gl_framebuffer *fb,
-GLuint attIndex);
-
-extern void
 _mesa_update_framebuffer(struct gl_context *ctx);
 
 extern GLboolean
-- 
1.7.3.4

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


[Mesa-dev] [PATCH 4/6] mesa: remove redundant buffer checks in copytexture_error_check()

2011-10-10 Thread Brian Paul
From: Brian Paul bri...@vmware.com

There was already a call to _mesa_source_buffer_exists() earlier in
the function.
---
 src/mesa/main/teximage.c |   16 
 1 files changed, 0 insertions(+), 16 deletions(-)

diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
index 8efe715..28c9705 100644
--- a/src/mesa/main/teximage.c
+++ b/src/mesa/main/teximage.c
@@ -1975,22 +1975,6 @@ copytexture_error_check( struct gl_context *ctx, GLuint 
dimensions,
  return GL_TRUE;
   }
}
-   else if (_mesa_is_depth_format(internalFormat)) {
-  /* make sure we have depth/stencil buffers */
-  if (!ctx-ReadBuffer-_DepthBuffer) {
- _mesa_error(ctx, GL_INVALID_OPERATION,
- glCopyTexImage%dD(no depth), dimensions);
- return GL_TRUE;
-  }
-   }
-   else if (_mesa_is_depthstencil_format(internalFormat)) {
-  /* make sure we have depth/stencil buffers */
-  if (!ctx-ReadBuffer-_DepthBuffer || !ctx-ReadBuffer-_StencilBuffer) {
- _mesa_error(ctx, GL_INVALID_OPERATION,
- glCopyTexImage%dD(no depth/stencil buffer), dimensions);
- return GL_TRUE;
-  }
-   }
 
/* if we get here, the parameters are OK */
return GL_FALSE;
-- 
1.7.3.4

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


[Mesa-dev] [PATCH 6/6] s/format/baseFormat/ to be more explicit

2011-10-10 Thread Brian Paul
From: Brian Paul bri...@vmware.com

---
 src/mesa/main/teximage.c |   10 +-
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
index 84eed23..2d06f84 100644
--- a/src/mesa/main/teximage.c
+++ b/src/mesa/main/teximage.c
@@ -1893,7 +1893,7 @@ copytexture_error_check( struct gl_context *ctx, GLuint 
dimensions,
const GLenum proxyTarget = get_proxy_target(target);
const GLenum type = GL_FLOAT;
GLboolean sizeOK;
-   GLint format;
+   GLint baseFormat;
 
/* check target */
if (!legal_texsubimage_target(ctx, dimensions, target)) {
@@ -1928,14 +1928,14 @@ copytexture_error_check( struct gl_context *ctx, GLuint 
dimensions,
   return GL_TRUE;
}
 
-   format = _mesa_base_tex_format(ctx, internalFormat);
-   if (format  0) {
+   baseFormat = _mesa_base_tex_format(ctx, internalFormat);
+   if (baseFormat  0) {
   _mesa_error(ctx, GL_INVALID_VALUE,
   glCopyTexImage%dD(internalFormat), dimensions);
   return GL_TRUE;
}
 
-   if (!_mesa_source_buffer_exists(ctx, format)) {
+   if (!_mesa_source_buffer_exists(ctx, baseFormat)) {
   _mesa_error(ctx, GL_INVALID_OPERATION,
   glCopyTexImage%dD(missing readbuffer), dimensions);
   return GL_TRUE;
@@ -1946,7 +1946,7 @@ copytexture_error_check( struct gl_context *ctx, GLuint 
dimensions,
   ? (width == height) : 1;
 
sizeOK = sizeOK  ctx-Driver.TestProxyTexImage(ctx, proxyTarget, level,
-internalFormat, format,
+internalFormat, baseFormat,
 type, width, height,
 1, border);
 
-- 
1.7.3.4

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


[Mesa-dev] [PATCH 5/6] mesa: remove redundant buffer checks in copytexsubimage_error_check2()

2011-10-10 Thread Brian Paul
From: Brian Paul bri...@vmware.com

Again, there was already a call to _mesa_source_buffer_exists() earlier in
the function.
---
 src/mesa/main/teximage.c |   17 -
 1 files changed, 0 insertions(+), 17 deletions(-)

diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
index 28c9705..84eed23 100644
--- a/src/mesa/main/teximage.c
+++ b/src/mesa/main/teximage.c
@@ -2133,23 +2133,6 @@ copytexsubimage_error_check2( struct gl_context *ctx, 
GLuint dimensions,
   return GL_TRUE;
}
 
-   if (teximage-_BaseFormat == GL_DEPTH_COMPONENT) {
-  if (!ctx-ReadBuffer-_DepthBuffer) {
- _mesa_error(ctx, GL_INVALID_OPERATION,
- glCopyTexSubImage%dD(no depth buffer),
- dimensions);
- return GL_TRUE;
-  }
-   }
-   else if (teximage-_BaseFormat == GL_DEPTH_STENCIL_EXT) {
-  if (!ctx-ReadBuffer-_DepthBuffer || !ctx-ReadBuffer-_StencilBuffer) {
- _mesa_error(ctx, GL_INVALID_OPERATION,
- glCopyTexSubImage%dD(no depth/stencil buffer),
- dimensions);
- return GL_TRUE;
-  }
-   }
-
/* If copying into an integer texture, the source buffer must also be
 * integer-valued.
 */
-- 
1.7.3.4

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