Re: [Mesa-dev] [PATCH v2 1/2] configure.ac: Enable GLX_USE_TLS if possible.

2012-02-15 Thread nobled
On Mon, Feb 13, 2012 at 5:10 AM, Zhigang Gong
zhigang.g...@linux.intel.com wrote:
 On Sun, Feb 12, 2012 at 09:02:44AM -0500, nobled wrote:
 Judging by the patch that tried to change the tls_model:
 https://bugs.freedesktop.org/show_bug.cgi?id=35268
 It looks like we'd need to disable enable_asm if the tls_model
 attribute isn't supported, since it would probably end up defaulting
 to general-dynamic for PIC code IIRC, and the assembly depends on it
 being initial-exec. Clang still doesn't support the tls_model
 attribute, and it doesn't fail compilation: it just gives a
 -Wattributes warning, so we might need to add '-Werror -Wattributes'
 (to catch the versions before -Wattributes was added, which give an
 'unknown warning option' warning) to the test-compilation CFLAGS...

 Plus, the test would have to be moved to before enable_asm is decided
 and adds the -DUSE_X86_ASM, etc defines.


 Thanks for the comments, I refined the patch according to you and
 Dan's suggestion. Here is the new version. As to the version check
 of CLANG, I haven't handled it as I don't have the environment at
 hand. Could you help to add it?

 From 75de119cf92cb43cecd4813267ed0c76979ddef4 Mon Sep 17 00:00:00 2001
 From: Zhigang Gong zhigang.g...@linux.intel.com
 Date: Mon, 13 Feb 2012 17:26:21 +0800
 Subject: [PATCH 1/2 v2] configure.ac: Enable GLX_USE_TLS if possible.

 If the system support tls, we prefer to enable it by default
 just as xserver does. Actually, the checking code is copied
 from xserver/configure.ac.
 According to nobled's suggestion, move the checking before
 enable_asm. As if tls_model is not supported, then asm may
 can't work correctly. Then we check the tls model first,
 and if doesn't support it, we need to disable the asm code.

 Signed-off-by: Zhigang Gong zhigang.g...@linux.intel.com
 Reviewed-by: Dan Nicholson dbn.li...@gmail.com
 ---
  acinclude.m4 |   36 
  configure.ac |   25 ++---
  2 files changed, 58 insertions(+), 3 deletions(-)

 diff --git a/acinclude.m4 b/acinclude.m4
 index a5b389d..509de5b 100644
 --- a/acinclude.m4
 +++ b/acinclude.m4
 @@ -117,3 +117,39 @@ if test $enable_pic != no; then
  fi
  AC_SUBST([PIC_FLAGS])
  ])# MESA_PIC_FLAGS
 +
 +dnl TLS detection
 +AC_DEFUN([MESA_TLS],
 +[AC_MSG_CHECKING(for thread local storage (TLS) support)
 +AC_CACHE_VAL(ac_cv_tls, [
 +    ac_cv_tls=none
 +    keywords=__thread __declspec(thread)
 +    for kw in $keywords ; do
 +        AC_TRY_COMPILE([int $kw test;], [], ac_cv_tls=$kw)
 +    done
 +])
 +AC_MSG_RESULT($ac_cv_tls)
 +
 +if test $ac_cv_tls != none; then
 +    AC_MSG_CHECKING(for tls_model attribute support)
 +    saved_CFLAGS=$CFLAGS
 +    if test x$acv_mesa_CLANG = xyes; then
Oh, I didn't mean you had to check *specifically* for clang; I just
meant that adding those two flags would make sure that the test would
fail correctly (until they support the attribute). you only need to
check for GCC compatibility when adding -W flags, like the rest of the
script does: `if test x$GCC = xyes; then`.

 +        CFLAGS=$CFLAGS -Wattribute -Werror
 +    fi
 +    AC_CACHE_VAL(ac_cv_tls_model, [
 +        AC_TRY_COMPILE([int $ac_cv_tls 
 __attribute__((tls_model(initial-exec))) test;], [],
 +                       ac_cv_tls_model=yes, ac_cv_tls_model=no)
 +    ])
 +    CFLAGS=$saved_CFLAGS
 +    AC_MSG_RESULT($ac_cv_tls_model)
 +
 +    AC_MSG_CHECKING([for $CC option to define TLS variable])
 +    if test x$ac_cv_tls_model = xyes ; then
 +        TLS=$ac_cv_tls' __attribute__((tls_model(\initial-exec\)))'
 +    else
 +        TLS=$ac_cv_tls
 +    fi
 +    DEFINES=$DEFINES -DTLS=\$TLS\
 +    AC_MSG_RESULT([$TLS])
 +fi
 +])
 diff --git a/configure.ac b/configure.ac
 index b2b1ab8..3423cb2 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -433,6 +433,11 @@ AC_SUBST([VG_LIB_GLOB])
  AC_SUBST([GLAPI_LIB_GLOB])

  dnl
 +dnl Check tls model support
 +dnl
 +MESA_TLS
 +
 +dnl
  dnl Arch/platform-specific settings
  dnl
  AC_ARG_ENABLE([asm],
 @@ -446,6 +451,14 @@ ASM_FLAGS=
  MESA_ASM_SOURCES=
  GLAPI_ASM_SOURCES=
  AC_MSG_CHECKING([whether to enable assembly])
 +
 +if test x$ac_cv_tls_model = xno ; then
 +dnl
 +dnl If tls model is not supported, disable asm.
 +dnl
 +    enable_asm=no
 +fi
 +
  test x$enable_asm = xno  AC_MSG_RESULT([no])
  # disable if cross compiling on x86/x86_64 since we must run gen_matypes
  if test x$enable_asm = xyes  test x$cross_compiling = xyes; then
 @@ -1102,9 +1115,15 @@ dnl

  AC_ARG_ENABLE([glx-tls],
     [AS_HELP_STRING([--enable-glx-tls],
 -        [enable TLS support in GLX @:@default=disabled@:@])],
 -    [GLX_USE_TLS=$enableval],
 -    [GLX_USE_TLS=no])
 +        [enable TLS support in GLX @:@default=auto@:@])],
 +    [GLX_USE_TLS=$enableval
 +     if test x$GLX_USE_TLS = xyes  test ${ac_cv_tls} = none ; then
 +        AC_MSG_ERROR([GLX with TLS support requested, but the compiler does 
 not support it.])
 +     fi],
 +    [GLX_USE_TLS=no
 +     if test ${ac_cv_tls} != 

[Mesa-dev] [PATCH v3 0/2] Enable GLX_USE_TLS if possible.

2012-02-15 Thread zhigang . gong
From: Zhigang Gong zhigang.g...@linux.intel.com

This version fixed the CFLAGS setting when check whether support 
tls model.

---
This patchset enable GLX_USE_TLS if the system support
it. And this patchset will check whether we support tls
model and choose propriate method to define those TLS
variables rather than define them by using hard coded
attribute(..). For the system doesn't support tls model,
this patchset will disable asm according to
nobled nob...@dreamwidth.org's suggestion.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH v3 1/2] configure.ac: Enable GLX_USE_TLS if possible.

2012-02-15 Thread zhigang . gong
From: Zhigang Gong zhigang.g...@linux.intel.com

If the system support tls, we prefer to enable it by default
just as xserver does. Actually, the checking code is copied
from xserver/configure.ac.
According to nobled's suggestion, move the checking before
enable_asm. As if tls_model is not supported, then asm may
can't work correctly. Then we check the tls model first,
and if doesn't support it, we need to disable the asm code.
Here is the reference:
https://bugs.freedesktop.org/show_bug.cgi?id=35268

Signed-off-by: Zhigang Gong zhigang.g...@linux.intel.com
Reviewed-by: Dan Nicholson dbn.li...@gmail.com
---
 acinclude.m4 |   41 +
 configure.ac |   25 ++---
 2 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index a5b389d..23805f3 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -117,3 +117,44 @@ if test $enable_pic != no; then
 fi
 AC_SUBST([PIC_FLAGS])
 ])# MESA_PIC_FLAGS
+
+dnl TLS detection
+AC_DEFUN([MESA_TLS],
+[AC_MSG_CHECKING(for thread local storage (TLS) support)
+AC_CACHE_VAL(ac_cv_tls, [
+ac_cv_tls=none
+keywords=__thread __declspec(thread)
+for kw in $keywords ; do
+AC_TRY_COMPILE([int $kw test;], [], ac_cv_tls=$kw)
+done
+])
+AC_MSG_RESULT($ac_cv_tls)
+
+if test $ac_cv_tls != none; then
+STRICT_CFLAGS=-pedantic -Werror
+# Add -Werror=attributes if supported (gcc 4.2  later)
+AC_MSG_CHECKING([if $CC supports -Werror=attributes])
+save_CFLAGS=$CFLAGS
+CFLAGS=$CFLAGS $STRICT_CFLAGS -Werror=attributes
+AC_TRY_COMPILE([int test;], [],
+   [STRICT_CFLAGS=$STRICT_CFLAGS -Werror=attributes
+AC_MSG_RESULT([yes])],
+   [AC_MSG_RESULT([no])])
+CFLAGS=$save_CFLAGS $STRICT_CFLAGS
+
+AC_MSG_CHECKING(for tls_model attribute support)
+AC_CACHE_VAL(ac_cv_tls_model, [
+AC_TRY_COMPILE([int $ac_cv_tls 
__attribute__((tls_model(initial-exec))) test;], [],
+   ac_cv_tls_model=yes, ac_cv_tls_model=no)
+])
+AC_MSG_RESULT($ac_cv_tls_model)
+
+if test x$ac_cv_tls_model = xyes ; then
+TLS=$ac_cv_tls' __attribute__((tls_model(\initial-exec\)))'
+else
+TLS=$ac_cv_tls
+fi
+DEFINES=$DEFINES -DTLS=\$TLS\
+CFLAGS=$save_CFLAGS
+fi
+])
diff --git a/configure.ac b/configure.ac
index b2b1ab8..3226a09 100644
--- a/configure.ac
+++ b/configure.ac
@@ -433,6 +433,11 @@ AC_SUBST([VG_LIB_GLOB])
 AC_SUBST([GLAPI_LIB_GLOB])
 
 dnl
+dnl Check tls model support
+dnl
+MESA_TLS
+
+dnl
 dnl Arch/platform-specific settings
 dnl
 AC_ARG_ENABLE([asm],
@@ -446,6 +451,14 @@ ASM_FLAGS=
 MESA_ASM_SOURCES=
 GLAPI_ASM_SOURCES=
 AC_MSG_CHECKING([whether to enable assembly])
+
+if test x$ac_cv_tls_model = xno ; then
+dnl
+dnl If tls model is not supported, disable asm.
+dnl
+enable_asm=no
+fi
+
 test x$enable_asm = xno  AC_MSG_RESULT([no])
 # disable if cross compiling on x86/x86_64 since we must run gen_matypes
 if test x$enable_asm = xyes  test x$cross_compiling = xyes; then
@@ -1102,9 +1115,15 @@ dnl
 
 AC_ARG_ENABLE([glx-tls],
 [AS_HELP_STRING([--enable-glx-tls],
-[enable TLS support in GLX @:@default=disabled@:@])],
-[GLX_USE_TLS=$enableval],
-[GLX_USE_TLS=no])
+[enable TLS support in GLX @:@default=auto@:@])],
+[GLX_USE_TLS=$enableval
+ if test x$GLX_USE_TLS = xyes  test ${ac_cv_tls} = none ; then
+AC_MSG_ERROR([GLX with TLS support requested, but the compiler does 
not support it.])
+ fi],
+[GLX_USE_TLS=no
+ if test ${ac_cv_tls} != none ; then
+GLX_USE_TLS=yes
+ fi])
 AC_SUBST(GLX_TLS, ${GLX_USE_TLS})
 
 AS_IF([test x$GLX_USE_TLS = xyes],
-- 
1.7.4.4

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


[Mesa-dev] [PATCH v3 2/2] GLX_TLS: use TLS macros when define those TLS variables.

2012-02-15 Thread zhigang . gong
From: Zhigang Gong zhigang.g...@linux.intel.com

Use the properate way detected in autoconf stage to define
those TLS variables rather than using hard coded
attribute...

Signed-off-by: Zhigang Gong zhigang.g...@linux.intel.com
---
 src/egl/main/eglcurrent.c  |3 +--
 src/glx/glxclient.h|3 +--
 src/glx/glxcurrent.c   |3 +--
 src/mapi/glapi/glapi.h |6 ++
 src/mapi/mapi/u_current.c  |6 ++
 src/mapi/mapi/u_current.h  |6 ++
 src/mesa/drivers/dri/common/dri_test.c |6 ++
 src/mesa/drivers/x11/glxapi.c  |3 +--
 8 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/src/egl/main/eglcurrent.c b/src/egl/main/eglcurrent.c
index 54fc4f7..bbd46ac 100644
--- a/src/egl/main/eglcurrent.c
+++ b/src/egl/main/eglcurrent.c
@@ -51,8 +51,7 @@ static pthread_key_t _egl_TSD;
 static void (*_egl_FreeTSD)(_EGLThreadInfo *);
 
 #ifdef GLX_USE_TLS
-static __thread const _EGLThreadInfo *_egl_TLS
-   __attribute__ ((tls_model(initial-exec)));
+static TLS const _EGLThreadInfo * _egl_TLS;
 #endif
 
 static INLINE void _eglSetTSD(const _EGLThreadInfo *t)
diff --git a/src/glx/glxclient.h b/src/glx/glxclient.h
index f8ae450..06785df 100644
--- a/src/glx/glxclient.h
+++ b/src/glx/glxclient.h
@@ -630,9 +630,8 @@ extern void __glXSetCurrentContext(struct glx_context * c);
 
 # if defined( GLX_USE_TLS )
 
-extern __thread void *__glX_tls_Context
-   __attribute__ ((tls_model(initial-exec)));
 
+extern TLS void *__glX_tls_Context;
 #  define __glXGetCurrentContext() __glX_tls_Context
 
 # else
diff --git a/src/glx/glxcurrent.c b/src/glx/glxcurrent.c
index c92a2fd..f295d2c 100644
--- a/src/glx/glxcurrent.c
+++ b/src/glx/glxcurrent.c
@@ -86,8 +86,7 @@ _X_HIDDEN pthread_mutex_t __glXmutex = 
PTHREAD_MUTEX_INITIALIZER;
  * \b never be \c NULL.  This is important!  Because of this
  * \c __glXGetCurrentContext can be implemented as trivial macro.
  */
-__thread void *__glX_tls_Context __attribute__ ((tls_model(initial-exec)))
-   = dummyContext;
+TLS void *__glX_tls_Context = dummyContext;
 
 _X_HIDDEN void
 __glXSetCurrentContext(struct glx_context * c)
diff --git a/src/mapi/glapi/glapi.h b/src/mapi/glapi/glapi.h
index f685475..ef69e64 100644
--- a/src/mapi/glapi/glapi.h
+++ b/src/mapi/glapi/glapi.h
@@ -85,11 +85,9 @@ struct _glapi_table;
 
 #if defined (GLX_USE_TLS)
 
-_GLAPI_EXPORT extern __thread struct _glapi_table * _glapi_tls_Dispatch
-__attribute__((tls_model(initial-exec)));
+_GLAPI_EXPORT extern TLS struct _glapi_table * _glapi_tls_Dispatch;
 
-_GLAPI_EXPORT extern __thread void * _glapi_tls_Context
-__attribute__((tls_model(initial-exec)));
+_GLAPI_EXPORT extern TLS void * _glapi_tls_Context;
 
 _GLAPI_EXPORT extern const struct _glapi_table *_glapi_Dispatch;
 _GLAPI_EXPORT extern const void *_glapi_Context;
diff --git a/src/mapi/mapi/u_current.c b/src/mapi/mapi/u_current.c
index 21a07ab..f4e241e 100644
--- a/src/mapi/mapi/u_current.c
+++ b/src/mapi/mapi/u_current.c
@@ -99,12 +99,10 @@ extern void (*__glapi_noop_table[])(void);
 /*@{*/
 #if defined(GLX_USE_TLS)
 
-__thread struct mapi_table *u_current_table
-__attribute__((tls_model(initial-exec)))
+TLS struct mapi_table *u_current_table
 = (struct mapi_table *) table_noop_array;
 
-__thread void *u_current_user
-__attribute__((tls_model(initial-exec)));
+TLS void *u_current_user;
 
 #else
 
diff --git a/src/mapi/mapi/u_current.h b/src/mapi/mapi/u_current.h
index f9cffd8..0584dc8 100644
--- a/src/mapi/mapi/u_current.h
+++ b/src/mapi/mapi/u_current.h
@@ -30,11 +30,9 @@ struct mapi_table;
 
 #ifdef GLX_USE_TLS
 
-extern __thread struct mapi_table *u_current_table
-__attribute__((tls_model(initial-exec)));
+extern TLS struct mapi_table *u_current_table;
 
-extern __thread void *u_current_user
-__attribute__((tls_model(initial-exec)));
+extern TLS void *u_current_user;
 
 #else /* GLX_USE_TLS */
 
diff --git a/src/mesa/drivers/dri/common/dri_test.c 
b/src/mesa/drivers/dri/common/dri_test.c
index 793f0c3..aaad90c 100644
--- a/src/mesa/drivers/dri/common/dri_test.c
+++ b/src/mesa/drivers/dri/common/dri_test.c
@@ -11,11 +11,9 @@ extern char __driDriverExtensions[];
 
 #if defined(GLX_USE_TLS)
 
-PUBLIC __thread struct _glapi_table * _glapi_tls_Dispatch
-__attribute__((tls_model(initial-exec)));
+PUBLIC TLS struct _glapi_table * _glapi_tls_Dispatch;
 
-PUBLIC __thread void * _glapi_tls_Context
-__attribute__((tls_model(initial-exec)));
+PUBLIC TLS void * _glapi_tls_Context;
 
 PUBLIC const struct _glapi_table *_glapi_Dispatch;
 PUBLIC const void *_glapi_Context;
diff --git a/src/mesa/drivers/x11/glxapi.c b/src/mesa/drivers/x11/glxapi.c
index 255a37b..57f3e9d 100644
--- a/src/mesa/drivers/x11/glxapi.c
+++ b/src/mesa/drivers/x11/glxapi.c
@@ -159,8 +159,7 @@ get_dispatch(Display *dpy)
  * GLX API current context.
  */
 #if defined(GLX_USE_TLS)
-PUBLIC __thread void * CurrentContext
-

Re: [Mesa-dev] [PATCH v3 2/2] GLX_TLS: use TLS macros when define those TLS variables.

2012-02-15 Thread tf (mobile)
Personally i don't care much about non-autoconf builds, but as this relies on a 
-DTLS= setting during configuration time, it will break non-ac builds (which 
need tls), no?

Other than that, LGTM.

-tom

On 15.02.2012, at 12:41, zhigang.g...@linux.intel.com wrote:

 From: Zhigang Gong zhigang.g...@linux.intel.com
 
 Use the properate way detected in autoconf stage to define
 those TLS variables rather than using hard coded
 attribute...
 
 Signed-off-by: Zhigang Gong zhigang.g...@linux.intel.com
 ---
 src/egl/main/eglcurrent.c  |3 +--
 src/glx/glxclient.h|3 +--
 src/glx/glxcurrent.c   |3 +--
 src/mapi/glapi/glapi.h |6 ++
 src/mapi/mapi/u_current.c  |6 ++
 src/mapi/mapi/u_current.h  |6 ++
 src/mesa/drivers/dri/common/dri_test.c |6 ++
 src/mesa/drivers/x11/glxapi.c  |3 +--
 8 files changed, 12 insertions(+), 24 deletions(-)
 
 diff --git a/src/egl/main/eglcurrent.c b/src/egl/main/eglcurrent.c
 index 54fc4f7..bbd46ac 100644
 --- a/src/egl/main/eglcurrent.c
 +++ b/src/egl/main/eglcurrent.c
 @@ -51,8 +51,7 @@ static pthread_key_t _egl_TSD;
 static void (*_egl_FreeTSD)(_EGLThreadInfo *);
 
 #ifdef GLX_USE_TLS
 -static __thread const _EGLThreadInfo *_egl_TLS
 -   __attribute__ ((tls_model(initial-exec)));
 +static TLS const _EGLThreadInfo * _egl_TLS;
 #endif
 
 static INLINE void _eglSetTSD(const _EGLThreadInfo *t)
 diff --git a/src/glx/glxclient.h b/src/glx/glxclient.h
 index f8ae450..06785df 100644
 --- a/src/glx/glxclient.h
 +++ b/src/glx/glxclient.h
 @@ -630,9 +630,8 @@ extern void __glXSetCurrentContext(struct glx_context * 
 c);
 
 # if defined( GLX_USE_TLS )
 
 -extern __thread void *__glX_tls_Context
 -   __attribute__ ((tls_model(initial-exec)));
 
 +extern TLS void *__glX_tls_Context;
 #  define __glXGetCurrentContext() __glX_tls_Context
 
 # else
 diff --git a/src/glx/glxcurrent.c b/src/glx/glxcurrent.c
 index c92a2fd..f295d2c 100644
 --- a/src/glx/glxcurrent.c
 +++ b/src/glx/glxcurrent.c
 @@ -86,8 +86,7 @@ _X_HIDDEN pthread_mutex_t __glXmutex = 
 PTHREAD_MUTEX_INITIALIZER;
  * \b never be \c NULL.  This is important!  Because of this
  * \c __glXGetCurrentContext can be implemented as trivial macro.
  */
 -__thread void *__glX_tls_Context __attribute__ ((tls_model(initial-exec)))
 -   = dummyContext;
 +TLS void *__glX_tls_Context = dummyContext;
 
 _X_HIDDEN void
 __glXSetCurrentContext(struct glx_context * c)
 diff --git a/src/mapi/glapi/glapi.h b/src/mapi/glapi/glapi.h
 index f685475..ef69e64 100644
 --- a/src/mapi/glapi/glapi.h
 +++ b/src/mapi/glapi/glapi.h
 @@ -85,11 +85,9 @@ struct _glapi_table;
 
 #if defined (GLX_USE_TLS)
 
 -_GLAPI_EXPORT extern __thread struct _glapi_table * _glapi_tls_Dispatch
 -__attribute__((tls_model(initial-exec)));
 +_GLAPI_EXPORT extern TLS struct _glapi_table * _glapi_tls_Dispatch;
 
 -_GLAPI_EXPORT extern __thread void * _glapi_tls_Context
 -__attribute__((tls_model(initial-exec)));
 +_GLAPI_EXPORT extern TLS void * _glapi_tls_Context;
 
 _GLAPI_EXPORT extern const struct _glapi_table *_glapi_Dispatch;
 _GLAPI_EXPORT extern const void *_glapi_Context;
 diff --git a/src/mapi/mapi/u_current.c b/src/mapi/mapi/u_current.c
 index 21a07ab..f4e241e 100644
 --- a/src/mapi/mapi/u_current.c
 +++ b/src/mapi/mapi/u_current.c
 @@ -99,12 +99,10 @@ extern void (*__glapi_noop_table[])(void);
 /*@{*/
 #if defined(GLX_USE_TLS)
 
 -__thread struct mapi_table *u_current_table
 -__attribute__((tls_model(initial-exec)))
 +TLS struct mapi_table *u_current_table
 = (struct mapi_table *) table_noop_array;
 
 -__thread void *u_current_user
 -__attribute__((tls_model(initial-exec)));
 +TLS void *u_current_user;
 
 #else
 
 diff --git a/src/mapi/mapi/u_current.h b/src/mapi/mapi/u_current.h
 index f9cffd8..0584dc8 100644
 --- a/src/mapi/mapi/u_current.h
 +++ b/src/mapi/mapi/u_current.h
 @@ -30,11 +30,9 @@ struct mapi_table;
 
 #ifdef GLX_USE_TLS
 
 -extern __thread struct mapi_table *u_current_table
 -__attribute__((tls_model(initial-exec)));
 +extern TLS struct mapi_table *u_current_table;
 
 -extern __thread void *u_current_user
 -__attribute__((tls_model(initial-exec)));
 +extern TLS void *u_current_user;
 
 #else /* GLX_USE_TLS */
 
 diff --git a/src/mesa/drivers/dri/common/dri_test.c 
 b/src/mesa/drivers/dri/common/dri_test.c
 index 793f0c3..aaad90c 100644
 --- a/src/mesa/drivers/dri/common/dri_test.c
 +++ b/src/mesa/drivers/dri/common/dri_test.c
 @@ -11,11 +11,9 @@ extern char __driDriverExtensions[];
 
 #if defined(GLX_USE_TLS)
 
 -PUBLIC __thread struct _glapi_table * _glapi_tls_Dispatch
 -__attribute__((tls_model(initial-exec)));
 +PUBLIC TLS struct _glapi_table * _glapi_tls_Dispatch;
 
 -PUBLIC __thread void * _glapi_tls_Context
 -__attribute__((tls_model(initial-exec)));
 +PUBLIC TLS void * _glapi_tls_Context;
 
 PUBLIC const struct _glapi_table *_glapi_Dispatch;
 PUBLIC const void *_glapi_Context;
 diff 

[Mesa-dev] [PATCH 00/13] ARB_debug_output

2012-02-15 Thread Marek Olšák
Hi everyone,

this series adds the ARB_debug_output extension. It implements the minimum 
feature set required by the spec, which is GL API error logging.

I've added a new piglit test for this: arb_debug_output-api_error. I'd like to 
push this series in a week if there are no concerns.

The whole series is also available here:
  git://people.freedesktop.org/~mareko/mesa arb-debug-output

Marek Olšák (3):
  mesa: print GL errors via debug_output
  mesa: display list dispatch for ARB_debug_output
  mesa: expose ARB_debug_output

nobled (10):
  mesa: split error handling into its own file
  glapi: add ARB_debug_output.xml
  mesa: add infrastructure for GL_ARB_debug_output
  mesa: add some GL_ARB_debug_output functions
  mesa: add message-toggle booleans for GL_ARB_debug_output
  mesa: add glDebugMessageControlARB
  mesa: add yet more context fields for GL_ARB_debug_output
  mesa: add control for categories of application-provided messages
  mesa: add struct for managing client debug namespaces
  mesa: implement the last of GL_ARB_debug_output

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


[Mesa-dev] [PATCH 01/13] mesa: split error handling into its own file

2012-02-15 Thread Marek Olšák
From: nobled nob...@dreamwidth.org

Also add _mesa_vsnprintf.
---
 src/mesa/SConscript   |1 +
 src/mesa/main/descrip.mms |3 +
 src/mesa/main/errors.c|  276 +
 src/mesa/main/errors.h|   66 +++
 src/mesa/main/imports.c   |  245 +--
 src/mesa/main/imports.h   |   17 +---
 src/mesa/sources.mak  |1 +
 7 files changed, 357 insertions(+), 252 deletions(-)
 create mode 100644 src/mesa/main/errors.c
 create mode 100644 src/mesa/main/errors.h

diff --git a/src/mesa/SConscript b/src/mesa/SConscript
index e9b1f6a..af66896 100644
--- a/src/mesa/SConscript
+++ b/src/mesa/SConscript
@@ -64,6 +64,7 @@ main_sources = [
 'main/drawtex.c',
 'main/enable.c',
 'main/enums.c',
+'main/errors.c',
 'main/eval.c',
 'main/execmem.c',
 'main/extensions.c',
diff --git a/src/mesa/main/descrip.mms b/src/mesa/main/descrip.mms
index 70bc263..91d7225 100644
--- a/src/mesa/main/descrip.mms
+++ b/src/mesa/main/descrip.mms
@@ -42,6 +42,7 @@ SOURCES =accum.c \
drawpix.c \
enable.c \
enums.c \
+   errors.c \
eval.c \
execmem.c \
extensions.c \
@@ -116,6 +117,7 @@ dlist.obj,\
 drawpix.obj,\
 enable.obj,\
 enums.obj,\
+errors.obj,\
 eval.obj,\
 execmem.obj,\
 extensions.obj,\
@@ -200,6 +202,7 @@ dlist.obj : dlist.c
 drawpix.obj : drawpix.c
 enable.obj : enable.c
 enums.obj : enums.c
+errors.obj : errors.c
 eval.obj : eval.c
 execmem.obj : execmem.c
 extensions.obj : extensions.c
diff --git a/src/mesa/main/errors.c b/src/mesa/main/errors.c
new file mode 100644
index 000..a571cdf
--- /dev/null
+++ b/src/mesa/main/errors.c
@@ -0,0 +1,276 @@
+/**
+ * \file errors.c
+ * Mesa debugging and error handling functions.
+ */
+
+/*
+ * Mesa 3-D graphics library
+ * Version:  7.1
+ *
+ * Copyright (C) 1999-2007  Brian Paul   All Rights Reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included
+ * in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS
+ * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * BRIAN PAUL BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
+ * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+
+#include errors.h
+
+#include imports.h
+#include context.h
+#include mtypes.h
+#include version.h
+
+
+#define MAXSTRING 4000  /* for _mesa_vsnprintf() */
+
+/**/
+/** \name Diagnostics */
+/*@{*/
+
+static void
+output_if_debug(const char *prefixString, const char *outputString,
+GLboolean newline)
+{
+   static int debug = -1;
+
+   /* Check the MESA_DEBUG environment variable if it hasn't
+* been checked yet.  We only have to check it once...
+*/
+   if (debug == -1) {
+  char *env = _mesa_getenv(MESA_DEBUG);
+
+  /* In a debug build, we print warning messages *unless*
+   * MESA_DEBUG is 0.  In a non-debug build, we don't
+   * print warning messages *unless* MESA_DEBUG is
+   * set *to any value*.
+   */
+#ifdef DEBUG
+  debug = (env != NULL  atoi(env) == 0) ? 0 : 1;
+#else
+  debug = (env != NULL) ? 1 : 0;
+#endif
+   }
+
+   /* Now only print the string if we're required to do so. */
+   if (debug) {
+  fprintf(stderr, %s: %s, prefixString, outputString);
+  if (newline)
+ fprintf(stderr, \n);
+
+#if defined(_WIN32)  !defined(_WIN32_WCE)
+  /* stderr from windows applications without console is not usually 
+   * visible, so communicate with the debugger instead */ 
+  {
+ char buf[4096];
+ _mesa_snprintf(buf, sizeof(buf), %s: %s%s, prefixString, 
outputString, newline ? \n : );
+ OutputDebugStringA(buf);
+  }
+#endif
+   }
+}
+
+
+/**
+ * Return string version of GL error code.
+ */
+static const char *
+error_string( GLenum error )
+{
+   switch (error) {
+   case GL_NO_ERROR:
+  return GL_NO_ERROR;
+   case GL_INVALID_VALUE:
+  return GL_INVALID_VALUE;
+   case GL_INVALID_ENUM:
+  return GL_INVALID_ENUM;
+   case GL_INVALID_OPERATION:
+  return GL_INVALID_OPERATION;
+   case GL_STACK_OVERFLOW:
+  return GL_STACK_OVERFLOW;
+   case GL_STACK_UNDERFLOW:
+  

[Mesa-dev] [PATCH 02/13] glapi: add ARB_debug_output.xml

2012-02-15 Thread Marek Olšák
From: nobled nob...@dreamwidth.org

Marek v2: replace GLDEBUGPROCARB with void*
---
 src/mapi/glapi/gen/ARB_debug_output.xml |   93 +++
 src/mapi/glapi/gen/Makefile |1 +
 src/mapi/glapi/gen/gl_API.xml   |2 +
 3 files changed, 96 insertions(+), 0 deletions(-)
 create mode 100644 src/mapi/glapi/gen/ARB_debug_output.xml

diff --git a/src/mapi/glapi/gen/ARB_debug_output.xml 
b/src/mapi/glapi/gen/ARB_debug_output.xml
new file mode 100644
index 000..a4b576e
--- /dev/null
+++ b/src/mapi/glapi/gen/ARB_debug_output.xml
@@ -0,0 +1,93 @@
+?xml version=1.0?
+!DOCTYPE OpenGLAPI SYSTEM gl_API.dtd
+
+!-- Note: no GLX protocol info yet. --
+
+
+OpenGLAPI
+
+category name=GL_ARB_debug_output number=104
+
+!-- glEnable/Disable/IsEnabled --
+enum name=DEBUG_OUTPUT_SYNCHRONOUS_ARB  value=0x8242/
+
+!-- glGetIntegerv --
+enum name=MAX_DEBUG_MESSAGE_LENGTH_ARBcount=1 value=0x9143
+size name=Get mode=get/
+/enum
+enum name=MAX_DEBUG_LOGGED_MESSAGES_ARB   count=1 value=0x9144
+size name=Get mode=get/
+/enum
+enum name=DEBUG_LOGGED_MESSAGES_ARB   count=1 value=0x9145
+size name=Get mode=get/
+/enum
+enum name=DEBUG_NEXT_LOGGED_MESSAGE_LENGTH_ARB count=1 value=0x8243
+size name=Get mode=get/
+/enum
+
+!-- glGetPointerv --
+enum name=DEBUG_CALLBACK_FUNCTION_ARB count=1 value=0x8244
+size name=GetPointerv mode=get/
+/enum
+enum name=DEBUG_CALLBACK_USER_PARAM_ARB   count=1 value=0x8245
+size name=GetPointerv mode=get/
+/enum
+
+enum name=DEBUG_SOURCE_API_ARB  value=0x8246/
+enum name=DEBUG_SOURCE_WINDOW_SYSTEM_ARBvalue=0x8247/
+enum name=DEBUG_SOURCE_SHADER_COMPILER_ARB  value=0x8248/
+enum name=DEBUG_SOURCE_THIRD_PARTY_ARB  value=0x8249/
+enum name=DEBUG_SOURCE_APPLICATION_ARB  value=0x824A/
+enum name=DEBUG_SOURCE_OTHER_ARBvalue=0x824B/
+
+enum name=DEBUG_TYPE_ERROR_ARB  value=0x824C/
+enum name=DEBUG_TYPE_DEPRECATED_BEHAVIOR_ARBvalue=0x824D/
+enum name=DEBUG_TYPE_UNDEFINED_BEHAVIOR_ARB value=0x824E/
+enum name=DEBUG_TYPE_PORTABILITY_ARBvalue=0x824F/
+enum name=DEBUG_TYPE_PERFORMANCE_ARBvalue=0x8250/
+enum name=DEBUG_TYPE_OTHER_ARB  value=0x8251/
+
+enum name=DEBUG_SEVERITY_HIGH_ARB   value=0x9146/
+enum name=DEBUG_SEVERITY_MEDIUM_ARB value=0x9147/
+enum name=DEBUG_SEVERITY_LOW_ARBvalue=0x9148/
+
+
+function name=DebugMessageControlARB offset=assign
+param name=source type=GLenum/
+param name=type type=GLenum/
+param name=severity type=GLenum/
+param name=count type=GLsizei counter=true/
+param name=ids type=const GLuint * count=count/
+param name=enabled type=GLboolean/
+/function
+
+function name=DebugMessageInsertARB offset=assign
+param name=source type=GLenum/
+param name=type type=GLenum/
+param name=id type=GLuint/
+param name=severity type=GLenum/
+param name=length type=GLsizei/
+param name=buf type=const GLcharARB */
+/function
+
+function name=DebugMessageCallbackARB offset=assign
+param name=callback type=GLvoid */
+param name=userParam type=GLvoid */
+/function
+
+function name=GetDebugMessageLogARB offset=assign
+return type=GLuint/
+param name=count type=GLuint/
+param name=bufsize type=GLsizei/
+param name=sources type=GLenum * output=true/
+param name=types type=GLenum * output=true/
+param name=ids type=GLuint * output=true/
+param name=severities type=GLenum * output=true/
+param name=lengths type=GLsizei * output=true/
+param name=messageLog type=GLcharARB * output=true/
+/function
+
+/category
+
+
+/OpenGLAPI
diff --git a/src/mapi/glapi/gen/Makefile b/src/mapi/glapi/gen/Makefile
index c409285..44939e3 100644
--- a/src/mapi/glapi/gen/Makefile
+++ b/src/mapi/glapi/gen/Makefile
@@ -65,6 +65,7 @@ API_XML = \
gl_API.xml \
ARB_color_buffer_float.xml \
ARB_copy_buffer.xml \
+   ARB_debug_output.xml \
ARB_depth_clamp.xml \
ARB_draw_buffers_blend.xml \
ARB_draw_elements_base_vertex.xml \
diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml
index 24539c0..eb30719 100644
--- a/src/mapi/glapi/gen/gl_API.xml
+++ b/src/mapi/glapi/gen/gl_API.xml
@@ -7925,6 +7925,8 @@
 
 xi:include href=ARB_color_buffer_float.xml 
xmlns:xi=http://www.w3.org/2001/XInclude/
 
+xi:include href=ARB_debug_output.xml 
xmlns:xi=http://www.w3.org/2001/XInclude/
+
 xi:include href=ARB_robustness.xml 
xmlns:xi=http://www.w3.org/2001/XInclude/
 
 !-- Non-ARB extensions sorted by extension number. --

[Mesa-dev] [PATCH 03/13] mesa: add infrastructure for GL_ARB_debug_output

2012-02-15 Thread Marek Olšák
From: nobled nob...@dreamwidth.org

Marek v2: don't add the extension to extensions.c yet
---
 src/mesa/main/config.h|6 ++
 src/mesa/main/context.c   |1 +
 src/mesa/main/enable.c|5 +
 src/mesa/main/errors.c|   13 -
 src/mesa/main/errors.h|3 +++
 src/mesa/main/get.c   |7 +++
 src/mesa/main/getstring.c |6 ++
 src/mesa/main/mtypes.h|   29 +
 8 files changed, 69 insertions(+), 1 deletions(-)

diff --git a/src/mesa/main/config.h b/src/mesa/main/config.h
index 7b7740e..74b33ed 100644
--- a/src/mesa/main/config.h
+++ b/src/mesa/main/config.h
@@ -298,6 +298,12 @@
 #define MAX_GEOMETRY_TOTAL_OUTPUT_COMPONENTS 1024
 /*@}*/
 
+/** For GL_ARB_debug_output */
+/*@{*/
+#define MAX_DEBUG_LOGGED_MESSAGES   10
+#define MAX_DEBUG_MESSAGE_LENGTH4096
+/*@}*/
+
 
 /**
  * \name Mesa-specific parameters
diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index 43e7438..874d33b 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -764,6 +764,7 @@ init_attrib_groups(struct gl_context *ctx)
_mesa_init_depth( ctx );
_mesa_init_debug( ctx );
_mesa_init_display_list( ctx );
+   _mesa_init_errors( ctx );
_mesa_init_eval( ctx );
_mesa_init_fbobjects( ctx );
_mesa_init_feedback( ctx );
diff --git a/src/mesa/main/enable.c b/src/mesa/main/enable.c
index 270b240..5a998d7 100644
--- a/src/mesa/main/enable.c
+++ b/src/mesa/main/enable.c
@@ -348,6 +348,9 @@ _mesa_set_enable(struct gl_context *ctx, GLenum cap, 
GLboolean state)
  FLUSH_VERTICES(ctx, _NEW_DEPTH);
  ctx-Depth.Test = state;
  break;
+  case GL_DEBUG_OUTPUT_SYNCHRONOUS_ARB:
+ ctx-Debug.SyncOutput = state;
+ break;
   case GL_DITHER:
  if (ctx-Color.DitherFlag == state)
 return;
@@ -1114,6 +1117,8 @@ _mesa_IsEnabled( GLenum cap )
 return ctx-Light.ColorMaterialEnabled;
   case GL_CULL_FACE:
  return ctx-Polygon.CullFlag;
+  case GL_DEBUG_OUTPUT_SYNCHRONOUS_ARB:
+ return ctx-Debug.SyncOutput;
   case GL_DEPTH_TEST:
  return ctx-Depth.Test;
   case GL_DITHER:
diff --git a/src/mesa/main/errors.c b/src/mesa/main/errors.c
index a571cdf..906aafc 100644
--- a/src/mesa/main/errors.c
+++ b/src/mesa/main/errors.c
@@ -36,7 +36,18 @@
 #include version.h
 
 
-#define MAXSTRING 4000  /* for _mesa_vsnprintf() */
+#define MAXSTRING MAX_DEBUG_MESSAGE_LENGTH
+
+void
+_mesa_init_errors(struct gl_context *ctx)
+{
+   ctx-Debug.Callback = NULL;
+   ctx-Debug.SyncOutput = GL_FALSE;
+   ctx-Debug.Log[0].length = 0;
+   ctx-Debug.NumMessages = 0;
+   ctx-Debug.NextMsg = 0;
+   ctx-Debug.NextMsgLength = 0;
+}
 
 /**/
 /** \name Diagnostics */
diff --git a/src/mesa/main/errors.h b/src/mesa/main/errors.h
index 78dd57a..e467f5d 100644
--- a/src/mesa/main/errors.h
+++ b/src/mesa/main/errors.h
@@ -47,6 +47,9 @@ extern C {
 struct gl_context;
 
 extern void
+_mesa_init_errors( struct gl_context *ctx );
+
+extern void
 _mesa_warning( struct gl_context *gc, const char *fmtString, ... ) 
PRINTFLIKE(2, 3);
 
 extern void
diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
index 5ad6012..9a5ca53 100644
--- a/src/mesa/main/get.c
+++ b/src/mesa/main/get.c
@@ -1297,6 +1297,13 @@ static const struct value_desc values[] = {
 
/* GL_ARB_robustness */
{ GL_RESET_NOTIFICATION_STRATEGY_ARB, CONTEXT_ENUM(Const.ResetStrategy), 
NO_EXTRA },
+
+   /* GL_ARB_debug_output */
+   { GL_DEBUG_LOGGED_MESSAGES_ARB, CONTEXT_INT(Debug.NumMessages), NO_EXTRA },
+   { GL_DEBUG_NEXT_LOGGED_MESSAGE_LENGTH_ARB, 
CONTEXT_INT(Debug.NextMsgLength), NO_EXTRA },
+   { GL_MAX_DEBUG_LOGGED_MESSAGES_ARB, CONST(MAX_DEBUG_LOGGED_MESSAGES), 
NO_EXTRA },
+   { GL_MAX_DEBUG_MESSAGE_LENGTH_ARB, CONST(MAX_DEBUG_MESSAGE_LENGTH), 
NO_EXTRA },
+
 #endif /* FEATURE_GL */
 };
 
diff --git a/src/mesa/main/getstring.c b/src/mesa/main/getstring.c
index dbf6c3f..90e0280 100644
--- a/src/mesa/main/getstring.c
+++ b/src/mesa/main/getstring.c
@@ -224,6 +224,12 @@ _mesa_GetPointerv( GLenum pname, GLvoid **params )
  *params = (GLvoid *) 
ctx-Array.ArrayObj-VertexAttrib[VERT_ATTRIB_POINT_SIZE].Ptr;
  break;
 #endif
+  case GL_DEBUG_CALLBACK_FUNCTION_ARB:
+ *params = (GLvoid *) ctx-Debug.Callback;
+ break;
+  case GL_DEBUG_CALLBACK_USER_PARAM_ARB:
+ *params = ctx-Debug.CallbackData;
+ break;
   default:
  _mesa_error( ctx, GL_INVALID_ENUM, glGetPointerv );
  return;
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 5ef97c8..6160c04 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -3194,6 +3194,32 @@ struct gl_dlist_state
} Current;
 };
 
+/**
+ * An error, warning, or other piece of debug information for an application
+ * to consume via GL_ARB_debug_output.
+ */
+struct gl_debug_msg
+{
+   GLenum 

[Mesa-dev] [PATCH 04/13] mesa: add some GL_ARB_debug_output functions

2012-02-15 Thread Marek Olšák
From: nobled nob...@dreamwidth.org

---
 src/mesa/main/api_exec.c |   12 ++-
 src/mesa/main/errors.c   |  274 ++
 src/mesa/main/errors.h   |4 +
 3 files changed, 286 insertions(+), 4 deletions(-)

diff --git a/src/mesa/main/api_exec.c b/src/mesa/main/api_exec.c
index 605af38..15b9f69 100644
--- a/src/mesa/main/api_exec.c
+++ b/src/mesa/main/api_exec.c
@@ -55,6 +55,7 @@
 #include drawpix.h
 #include rastpos.h
 #include enable.h
+#include errors.h
 #include eval.h
 #include get.h
 #include feedback.h
@@ -598,15 +599,18 @@ _mesa_create_exec_table(void)
SET_DrawBuffersARB(exec, _mesa_DrawBuffersARB);
 #endif
 
-   /* ARB 104. GL_ARB_robustness */
+   /* ARB 66. GL_ARB_sync */
+   _mesa_init_sync_dispatch(exec);
+
+   /* ARB 104. GL_ARB_debug_output */
+   _mesa_init_errors_dispatch(exec);
+
+   /* ARB 105. GL_ARB_robustness */
SET_GetGraphicsResetStatusARB(exec, _mesa_GetGraphicsResetStatusARB);
SET_GetnPolygonStippleARB(exec, _mesa_GetnPolygonStippleARB);
SET_GetnTexImageARB(exec, _mesa_GetnTexImageARB);
SET_ReadnPixelsARB(exec, _mesa_ReadnPixelsARB);
 
-   /* GL_ARB_sync */
-   _mesa_init_sync_dispatch(exec);
-
   /* GL_ATI_fragment_shader */
_mesa_init_ati_fragment_shader_dispatch(exec);
 
diff --git a/src/mesa/main/errors.c b/src/mesa/main/errors.c
index 906aafc..e43b31f 100644
--- a/src/mesa/main/errors.c
+++ b/src/mesa/main/errors.c
@@ -32,12 +32,286 @@
 
 #include imports.h
 #include context.h
+#include dispatch.h
 #include mtypes.h
 #include version.h
 
 
 #define MAXSTRING MAX_DEBUG_MESSAGE_LENGTH
 
+
+static char out_of_memory[] = Debugging error: out of memory;
+
+/**
+ * 'buf' is not necessarily a null-terminated string. When logging, copy
+ * 'len' characters from it, store them in a new, null-terminated string,
+ * and remember the number of bytes used by that string, *including*
+ * the null terminator this time.
+ */
+static void
+_mesa_log_msg(struct gl_context *ctx, GLenum source, GLenum type,
+  GLuint id, GLenum severity, GLint len, const char *buf)
+{
+   GLint nextEmpty;
+   struct gl_debug_msg *emptySlot;
+
+   assert(len = 0  len  MAX_DEBUG_MESSAGE_LENGTH);
+
+   if (ctx-Debug.Callback) {
+  ctx-Debug.Callback(source, type, id, severity,
+  len, buf, ctx-Debug.CallbackData);
+  return;
+   }
+
+   if (ctx-Debug.NumMessages == MAX_DEBUG_LOGGED_MESSAGES)
+  return;
+
+   nextEmpty = (ctx-Debug.NextMsg + ctx-Debug.NumMessages)
+  % MAX_DEBUG_LOGGED_MESSAGES;
+   emptySlot = ctx-Debug.Log[nextEmpty];
+
+   assert(!emptySlot-message  !emptySlot-length);
+
+   emptySlot-message = MALLOC(len+1);
+   if (emptySlot-message) {
+  (void) strncpy(emptySlot-message, buf, (size_t)len);
+  emptySlot-message[len] = '\0';
+
+  emptySlot-length = len+1;
+  emptySlot-source = source;
+  emptySlot-type = type;
+  emptySlot-id = id;
+  emptySlot-severity = severity;
+   } else {
+  /* malloc failed! */
+  emptySlot-message = out_of_memory;
+  emptySlot-length = strlen(out_of_memory)+1;
+  emptySlot-source = GL_DEBUG_SOURCE_OTHER_ARB;
+  emptySlot-type = GL_DEBUG_TYPE_ERROR_ARB;
+  emptySlot-id = 1; /* TODO: proper id namespace */
+  emptySlot-severity = GL_DEBUG_SEVERITY_HIGH_ARB;
+   }
+
+   if (ctx-Debug.NumMessages == 0)
+  ctx-Debug.NextMsgLength = ctx-Debug.Log[ctx-Debug.NextMsg].length;
+
+   ctx-Debug.NumMessages++;
+}
+
+/**
+ * Pop the oldest debug message out of the log.
+ * Writes the message string, including the null terminator, into 'buf',
+ * using up to 'bufSize' bytes. If 'bufSize' is too small, or
+ * if 'buf' is NULL, nothing is written.
+ *
+ * Returns the number of bytes written on success, or when 'buf' is NULL,
+ * the number that would have been written. A return value of 0
+ * indicates failure.
+ */
+static GLsizei
+_mesa_get_msg(struct gl_context *ctx, GLenum *source, GLenum *type,
+  GLuint *id, GLenum *severity, GLsizei bufSize, char *buf)
+{
+   struct gl_debug_msg *msg;
+   GLsizei length;
+
+   if (ctx-Debug.NumMessages == 0)
+  return 0;
+
+   msg = ctx-Debug.Log[ctx-Debug.NextMsg];
+   length = msg-length;
+
+   assert(length  0  length == ctx-Debug.NextMsgLength);
+
+   if (bufSize  length  buf != NULL)
+  return 0;
+
+   if (severity)
+  *severity = msg-severity;
+   if (source)
+  *source = msg-source;
+   if (type)
+  *type = msg-type;
+   if (id)
+  *id = msg-id;
+
+   if (buf) {
+  assert(msg-message[length-1] == '\0');
+  (void) strncpy(buf, msg-message, (size_t)length);
+   }
+
+   if (msg-message != (char*)out_of_memory)
+  FREE(msg-message);
+   msg-message = NULL;
+   msg-length = 0;
+
+   ctx-Debug.NumMessages--;
+   ctx-Debug.NextMsg++;
+   ctx-Debug.NextMsg %= MAX_DEBUG_LOGGED_MESSAGES;
+   ctx-Debug.NextMsgLength = ctx-Debug.Log[ctx-Debug.NextMsg].length;
+
+   return length;
+}
+
+/**
+ * Verify that source, 

[Mesa-dev] [PATCH 05/13] mesa: add message-toggle booleans for GL_ARB_debug_output

2012-02-15 Thread Marek Olšák
From: nobled nob...@dreamwidth.org

---
 src/mesa/main/errors.c |2 +-
 src/mesa/main/mtypes.h |   28 +++-
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/src/mesa/main/errors.c b/src/mesa/main/errors.c
index e43b31f..ed94841 100644
--- a/src/mesa/main/errors.c
+++ b/src/mesa/main/errors.c
@@ -88,7 +88,7 @@ _mesa_log_msg(struct gl_context *ctx, GLenum source, GLenum 
type,
   emptySlot-length = strlen(out_of_memory)+1;
   emptySlot-source = GL_DEBUG_SOURCE_OTHER_ARB;
   emptySlot-type = GL_DEBUG_TYPE_ERROR_ARB;
-  emptySlot-id = 1; /* TODO: proper id namespace */
+  emptySlot-id = OTHER_ERROR_OUT_OF_MEMORY;
   emptySlot-severity = GL_DEBUG_SEVERITY_HIGH_ARB;
}
 
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 6160c04..5c2afa2 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -3208,12 +3208,38 @@ struct gl_debug_msg
GLcharARB *message;
 };
 
-/* GL_ARB_debug_output */
+typedef enum {
+   API_ERROR_UNKNOWN,
+   API_ERROR_COUNT
+} gl_api_error;
+
+typedef enum {
+   WINSYS_ERROR_UNKNOWN,
+   WINSYS_ERROR_COUNT
+} gl_winsys_error;
+
+typedef enum {
+   SHADER_ERROR_UNKNOWN,
+   SHADER_ERROR_COUNT
+} gl_shader_error;
+
+typedef enum {
+   OTHER_ERROR_UNKNOWN,
+   OTHER_ERROR_OUT_OF_MEMORY,
+   OTHER_ERROR_COUNT
+} gl_other_error;
+
 struct gl_debug_state
 {
GLDEBUGPROCARB Callback;
GLvoid *CallbackData;
GLboolean SyncOutput;
+   GLboolean ApiErrors[API_ERROR_COUNT];
+   GLboolean WinsysErrors[WINSYS_ERROR_COUNT];
+   GLboolean ShaderErrors[SHADER_ERROR_COUNT];
+   GLboolean OtherErrors[OTHER_ERROR_COUNT];
+   /* TODO: Add an object here that tracks the state of client-provided IDs
+  in the APPLICATION and THIRD_PARTY namespaces. */
struct gl_debug_msg Log[MAX_DEBUG_LOGGED_MESSAGES];
GLint NumMessages;
GLint NextMsg;
-- 
1.7.5.4

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


[Mesa-dev] [PATCH 06/13] mesa: add glDebugMessageControlARB

2012-02-15 Thread Marek Olšák
From: nobled nob...@dreamwidth.org

Controlling the output of client-provided messages
isn't done yet.
---
 src/mesa/main/errors.c |  134 
 1 files changed, 134 insertions(+), 0 deletions(-)

diff --git a/src/mesa/main/errors.c b/src/mesa/main/errors.c
index ed94841..3b0a4ee 100644
--- a/src/mesa/main/errors.c
+++ b/src/mesa/main/errors.c
@@ -42,6 +42,38 @@
 
 static char out_of_memory[] = Debugging error: out of memory;
 
+#define enum_is(e, kind1, kind2) \
+   ((e) == GL_DEBUG_##kind1##_##kind2##_ARB || (e) == GL_DONT_CARE)
+#define severity_is(sev, kind) enum_is(sev, SEVERITY, kind)
+#define source_is(s, kind) enum_is(s, SOURCE, kind)
+#define type_is(t, kind) enum_is(t, TYPE, kind)
+
+/**
+ * Whether a debugging message should be logged or not.
+ * For implementation-controlled namespaces, we keep an array
+ * of booleans per namespace, per context, recording whether
+ * each individual message is enabled or not. The message ID
+ * is an index into the namespace's array.
+ */
+static GLboolean
+should_log(struct gl_context *ctx, GLenum source, GLenum type,
+   GLuint id, GLenum severity)
+{
+   if (type_is(type, ERROR)) {
+  if (source_is(source, API))
+ return ctx-Debug.ApiErrors[id];
+  if (source_is(source, WINDOW_SYSTEM))
+ return ctx-Debug.WinsysErrors[id];
+  if (source_is(source, SHADER_COMPILER))
+ return ctx-Debug.ShaderErrors[id];
+  if (source_is(source, OTHER))
+ return ctx-Debug.OtherErrors[id];
+   }
+
+   /* TODO: tracking client messages' state. */
+   return (severity != GL_DEBUG_SEVERITY_LOW_ARB);
+}
+
 /**
  * 'buf' is not necessarily a null-terminated string. When logging, copy
  * 'len' characters from it, store them in a new, null-terminated string,
@@ -57,6 +89,9 @@ _mesa_log_msg(struct gl_context *ctx, GLenum source, GLenum 
type,
 
assert(len = 0  len  MAX_DEBUG_MESSAGE_LENGTH);
 
+   if (!should_log(ctx, source, type, id, severity))
+  return;
+
if (ctx-Debug.Callback) {
   ctx-Debug.Callback(source, type, id, severity,
   len, buf, ctx-Debug.CallbackData);
@@ -296,6 +331,98 @@ _mesa_GetDebugMessageLogARB(GLuint count, GLsizei logSize, 
GLenum* sources,
return ret;
 }
 
+/**
+ * 'array' is an array representing a particular debugging-message namespace.
+ * I.e., the set of all API errors, or the set of all Shader Compiler errors.
+ * 'size' is the size of 'array'. 'count' is the size of 'ids', an array
+ * of indices into 'array'. All the elements of 'array' at the indices
+ * listed in 'ids' will be overwritten with the value of 'enabled'.
+ *
+ * If 'count' is zero, all elements in 'array' are overwritten with the
+ * value of 'enabled'.
+ */
+static void
+control_messages(GLboolean *array, GLuint size,
+ GLsizei count, const GLuint *ids, GLboolean enabled)
+{
+   GLsizei i;
+
+   if (!count) {
+  GLuint id;
+  for (id = 0; id  size; id++) {
+ array[id] = enabled;
+  }
+  return;
+   }
+
+   for (i = 0; i  count; i++) {
+  if (ids[i] = size) {
+ /* XXX: The spec doesn't say what to do with a non-existent ID. */
+ continue;
+  }
+  array[ids[i]] = enabled;
+   }
+}
+
+/**
+ * Debugging-message namespaces with the source APPLICATION or THIRD_PARTY
+ * require special handling, since the IDs in them are controlled by clients,
+ * not the OpenGL implementation.
+ */
+static void
+control_app_messages(struct gl_context *ctx, GLenum source, GLenum type,
+ GLenum severity, GLsizei count, const GLuint *ids,
+ GLboolean enabled)
+{
+   assert(0  glDebugMessageControlARB():
+   Controlling application debugging messages not implemented);
+}
+
+static void GLAPIENTRY
+_mesa_DebugMessageControlARB(GLenum source, GLenum type, GLenum severity,
+ GLsizei count, const GLuint *ids,
+ GLboolean enabled)
+{
+   GET_CURRENT_CONTEXT(ctx);
+
+   if (count  0) {
+  _mesa_error(ctx, GL_INVALID_VALUE, glDebugMessageControlARB
+ (count=%d : count must not be negative), count);
+  return;
+   }
+
+   if (!validate_params(ctx, CONTROL, source, type, severity))
+  return; /* GL_INVALID_ENUM */
+
+   if (count  (severity != GL_DONT_CARE || type == GL_DONT_CARE
+ || source == GL_DONT_CARE)) {
+  _mesa_error(ctx, GL_INVALID_OPERATION, glDebugMessageControlARB
+ (When passing an array of ids, severity must be
+  GL_DONT_CARE, and source and type must not be GL_DONT_CARE.);
+  return;
+   }
+
+   if (source_is(source, APPLICATION) || source_is(source, THIRD_PARTY))
+  control_app_messages(ctx, source, type, severity, count, ids, enabled);
+
+   if (severity_is(severity, HIGH)) {
+  if (type_is(type, ERROR)) {
+ if (source_is(source, API))
+control_messages(ctx-Debug.ApiErrors, 

[Mesa-dev] [PATCH 07/13] mesa: add yet more context fields for GL_ARB_debug_output

2012-02-15 Thread Marek Olšák
From: nobled nob...@dreamwidth.org

---
 src/mesa/main/mtypes.h |   10 --
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 5c2afa2..be7e721 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -3229,6 +3229,13 @@ typedef enum {
OTHER_ERROR_COUNT
 } gl_other_error;
 
+struct gl_client_debug
+{
+   GLboolean Defaults[3][2][6]; /* severity, source, type */
+   /* TODO: Add an object here that can track the state of an arbitrary
+  number of client-provided IDs. */
+};
+
 struct gl_debug_state
 {
GLDEBUGPROCARB Callback;
@@ -3238,8 +3245,7 @@ struct gl_debug_state
GLboolean WinsysErrors[WINSYS_ERROR_COUNT];
GLboolean ShaderErrors[SHADER_ERROR_COUNT];
GLboolean OtherErrors[OTHER_ERROR_COUNT];
-   /* TODO: Add an object here that tracks the state of client-provided IDs
-  in the APPLICATION and THIRD_PARTY namespaces. */
+   struct gl_client_debug ClientIDs;
struct gl_debug_msg Log[MAX_DEBUG_LOGGED_MESSAGES];
GLint NumMessages;
GLint NextMsg;
-- 
1.7.5.4

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


[Mesa-dev] [PATCH 08/13] mesa: add control for categories of application-provided messages

2012-02-15 Thread Marek Olšák
From: nobled nob...@dreamwidth.org

This state is needed for deciding whether or not to log
application messages with IDs that haven't been specifically
passed to glDebugMessageControlARB yet.

State for each individual ID number ever passed to
glDebugMessageControlARB (per-context) still needs to be added.
---
 src/mesa/main/errors.c |  130 ++--
 1 files changed, 125 insertions(+), 5 deletions(-)

diff --git a/src/mesa/main/errors.c b/src/mesa/main/errors.c
index 3b0a4ee..e18dc8e 100644
--- a/src/mesa/main/errors.c
+++ b/src/mesa/main/errors.c
@@ -48,6 +48,73 @@ static char out_of_memory[] = Debugging error: out of 
memory;
 #define source_is(s, kind) enum_is(s, SOURCE, kind)
 #define type_is(t, kind) enum_is(t, TYPE, kind)
 
+enum {
+   SOURCE_APPLICATION,
+   SOURCE_THIRD_PARTY,
+
+   SOURCE_COUNT,
+   SOURCE_ANY = -1
+};
+
+enum {
+   TYPE_ERROR,
+   TYPE_DEPRECATED,
+   TYPE_UNDEFINED,
+   TYPE_PORTABILITY,
+   TYPE_PERFORMANCE,
+   TYPE_OTHER,
+
+   TYPE_COUNT,
+   TYPE_ANY = -1
+};
+
+enum {
+   SEVERITY_LOW,
+   SEVERITY_MEDIUM,
+   SEVERITY_HIGH,
+
+   SEVERITY_COUNT,
+   SEVERITY_ANY = -1
+};
+
+static int
+enum_to_index(GLenum e)
+{
+   switch (e) {
+   case GL_DEBUG_SOURCE_APPLICATION_ARB:
+  return (int)SOURCE_APPLICATION;
+   case GL_DEBUG_SOURCE_THIRD_PARTY_ARB:
+  return (int)SOURCE_THIRD_PARTY;
+
+   case GL_DEBUG_TYPE_ERROR_ARB:
+  return (int)TYPE_ERROR;
+   case GL_DEBUG_TYPE_DEPRECATED_BEHAVIOR_ARB:
+  return (int)TYPE_DEPRECATED;
+   case GL_DEBUG_TYPE_UNDEFINED_BEHAVIOR_ARB:
+  return (int)TYPE_UNDEFINED;
+   case GL_DEBUG_TYPE_PERFORMANCE_ARB:
+  return (int)TYPE_PERFORMANCE;
+   case GL_DEBUG_TYPE_PORTABILITY_ARB:
+  return (int)TYPE_PORTABILITY;
+   case GL_DEBUG_TYPE_OTHER_ARB:
+  return (int)TYPE_OTHER;
+
+   case GL_DEBUG_SEVERITY_LOW_ARB:
+  return (int)SEVERITY_LOW;
+   case GL_DEBUG_SEVERITY_MEDIUM_ARB:
+  return (int)SEVERITY_MEDIUM;
+   case GL_DEBUG_SEVERITY_HIGH_ARB:
+  return (int)SEVERITY_HIGH;
+
+   case GL_DONT_CARE:
+  return (int)TYPE_ANY;
+
+   default:
+  assert(0  unreachable);
+  return -2;
+   };
+}
+
 /**
  * Whether a debugging message should be logged or not.
  * For implementation-controlled namespaces, we keep an array
@@ -59,6 +126,17 @@ static GLboolean
 should_log(struct gl_context *ctx, GLenum source, GLenum type,
GLuint id, GLenum severity)
 {
+   if (source == GL_DEBUG_SOURCE_APPLICATION_ARB ||
+   source == GL_DEBUG_SOURCE_THIRD_PARTY_ARB) {
+  int s, t, sev;
+  s = enum_to_index(source);
+  t = enum_to_index(type);
+  sev = enum_to_index(severity);
+
+  return ctx-Debug.ClientIDs.Defaults[sev][s][t];
+  /* TODO: tracking individual client IDs. */
+   }
+
if (type_is(type, ERROR)) {
   if (source_is(source, API))
  return ctx-Debug.ApiErrors[id];
@@ -70,7 +148,6 @@ should_log(struct gl_context *ctx, GLenum source, GLenum 
type,
  return ctx-Debug.OtherErrors[id];
}
 
-   /* TODO: tracking client messages' state. */
return (severity != GL_DEBUG_SEVERITY_LOW_ARB);
 }
 
@@ -368,14 +445,51 @@ control_messages(GLboolean *array, GLuint size,
  * Debugging-message namespaces with the source APPLICATION or THIRD_PARTY
  * require special handling, since the IDs in them are controlled by clients,
  * not the OpenGL implementation.
+ *
+ * When controlling entire categories of messages without specifying IDs,
+ * the default state for IDs that haven't been seen yet have to be adjusted.
  */
 static void
-control_app_messages(struct gl_context *ctx, GLenum source, GLenum type,
- GLenum severity, GLsizei count, const GLuint *ids,
+control_app_messages(struct gl_context *ctx, GLenum esource, GLenum etype,
+ GLenum eseverity, GLsizei count, const GLuint *ids,
  GLboolean enabled)
 {
-   assert(0  glDebugMessageControlARB():
-   Controlling application debugging messages not implemented);
+   int source, type, severity, s, t, sev, smax, tmax, sevmax;
+
+   assert(!count  glDebugMessageControlARB():
+controlling for client-provided IDs is not implemented);
+   if (count)
+  return;
+
+   source = enum_to_index(esource);
+   type = enum_to_index(etype);
+   severity = enum_to_index(eseverity);
+
+   if (source == SOURCE_ANY) {
+  source = 0;
+  smax = SOURCE_COUNT;
+   } else {
+  smax = source+1;
+   }
+
+   if (type == TYPE_ANY) {
+  type = 0;
+  tmax = TYPE_COUNT;
+   } else {
+  tmax = type+1;
+   }
+
+   if (severity == SEVERITY_ANY) {
+  severity = 0;
+  sevmax = SEVERITY_COUNT;
+   } else {
+  sevmax = severity+1;
+   }
+
+   for (sev = severity; sev  sevmax; sev++)
+  for (s = source; s  smax; s++)
+ for (t = type; t  tmax; t++)
+ctx-Debug.ClientIDs.Defaults[sev][s][t] = enabled;
 }
 
 static void GLAPIENTRY
@@ -455,6 +569,12 @@ 

[Mesa-dev] [PATCH 9/13] mesa: add struct for managing client debug namespaces

2012-02-15 Thread Marek Olšák
From: nobled nob...@dreamwidth.org

The final piece of the puzzle for GL_ARB_debug_output.
---
 src/mesa/main/mtypes.h |   10 --
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index be7e721..e79c809 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -3229,11 +3229,17 @@ typedef enum {
OTHER_ERROR_COUNT
 } gl_other_error;
 
+struct gl_client_namespace
+{
+   struct _mesa_HashTable *IDs;
+   unsigned ZeroID; /* a HashTable won't take zero, so store its state here */
+   struct simple_node Severity[3]; /* lists of IDs in the hash table */
+};
+
 struct gl_client_debug
 {
GLboolean Defaults[3][2][6]; /* severity, source, type */
-   /* TODO: Add an object here that can track the state of an arbitrary
-  number of client-provided IDs. */
+   struct gl_client_namespace Namespaces[2][6]; /* source, type */
 };
 
 struct gl_debug_state
-- 
1.7.5.4

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


[Mesa-dev] [PATCH 10/13] mesa: implement the last of GL_ARB_debug_output

2012-02-15 Thread Marek Olšák
From: nobled nob...@dreamwidth.org

Store client-defined message IDs in a hash table,
and sort them by severity into three linked lists
so they can be selected by severity level later.
---
 src/mesa/main/context.c |2 +
 src/mesa/main/errors.c  |  268 ++-
 src/mesa/main/errors.h  |3 +
 3 files changed, 245 insertions(+), 28 deletions(-)

diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index 874d33b..37d6ee4 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -1143,6 +1143,8 @@ _mesa_free_context_data( struct gl_context *ctx )
/* needs to be after freeing shared state */
_mesa_free_display_list_data(ctx);
 
+   _mesa_free_errors_data(ctx);
+
if (ctx-Extensions.String)
   free((void *) ctx-Extensions.String);
 
diff --git a/src/mesa/main/errors.c b/src/mesa/main/errors.c
index e18dc8e..e736e04 100644
--- a/src/mesa/main/errors.c
+++ b/src/mesa/main/errors.c
@@ -33,6 +33,7 @@
 #include imports.h
 #include context.h
 #include dispatch.h
+#include hash.h
 #include mtypes.h
 #include version.h
 
@@ -40,6 +41,12 @@
 #define MAXSTRING MAX_DEBUG_MESSAGE_LENGTH
 
 
+struct gl_client_severity
+{
+   struct simple_node link;
+   GLuint ID;
+};
+
 static char out_of_memory[] = Debugging error: out of memory;
 
 #define enum_is(e, kind1, kind2) \
@@ -115,6 +122,135 @@ enum_to_index(GLenum e)
};
 }
 
+
+/*
+ * We store a bitfield in the hash table, with five possible values total.
+ *
+ * The ENABLED_BIT's purpose is self-explanatory.
+ *
+ * The FOUND_BIT is needed to differentiate the value of DISABLED from
+ * the value returned by HashTableLookup() when it can't find the given key.
+ *
+ * The KNOWN_SEVERITY bit is a bit complicated:
+ *
+ * A client may call Control() with an array of IDs, then call Control()
+ * on all message IDs of a certain severity, then Insert() one of the
+ * previously specified IDs, giving us a known severity level, then call
+ * Control() on all message IDs of a certain severity level again.
+ *
+ * After the first call, those IDs will have a FOUND_BIT, but will not
+ * exist in any severity-specific list, so the second call will not
+ * impact them. This is undesirable but unavoidable given the API:
+ * The only entrypoint that gives a severity for a client-defined ID
+ * is the Insert() call.
+ *
+ * For the sake of Control(), we want to maintain the invariant
+ * that an ID will either appear in none of the three severity lists,
+ * or appear once, to minimize pointless duplication and potential surprises.
+ *
+ * Because Insert() is the only place that will learn an ID's severity,
+ * it should insert an ID into the appropriate list, but only if the ID
+ * doesn't exist in it or any other list yet. Because searching all three
+ * lists at O(n) is needlessly expensive, we store KNOWN_SEVERITY.
+ */
+enum {
+   FOUND_BIT = 1  0,
+   ENABLED_BIT = 1  1,
+   KNOWN_SEVERITY = 1  2,
+
+   /* HashTable reserves zero as a return value meaning 'not found' */
+   NOT_FOUND = 0,
+   DISABLED = FOUND_BIT,
+   ENABLED = ENABLED_BIT | FOUND_BIT
+};
+
+/**
+ * Returns the state of the given message ID in a client-controlled
+ * namespace.
+ * 'source', 'type', and 'severity' are array indices like TYPE_ERROR,
+ * not GL enums.
+ */
+static GLboolean
+get_message_state(struct gl_context *ctx, int source, int type,
+  GLuint id, int severity)
+{
+   struct gl_client_namespace *nspace =
+ ctx-Debug.ClientIDs.Namespaces[source][type];
+   uintptr_t state;
+
+   /* In addition to not being able to store zero as a value, HashTable also
+  can't use zero as a key. */
+   if (id)
+  state = (uintptr_t)_mesa_HashLookup(nspace-IDs, id);
+   else
+  state = nspace-ZeroID;
+
+   /* Only do this once for each ID. This makes sure the ID exists in,
+  at most, one list, and does not pointlessly appear multiple times. */
+   if (!(state  KNOWN_SEVERITY)) {
+  struct gl_client_severity *entry;
+
+  if (state == NOT_FOUND) {
+ if (ctx-Debug.ClientIDs.Defaults[severity][source][type])
+state = ENABLED;
+ else
+state = DISABLED;
+  }
+
+  entry = malloc(sizeof *entry);
+  if (!entry)
+ goto out;
+
+  state |= KNOWN_SEVERITY;
+
+  if (id)
+ _mesa_HashInsert(nspace-IDs, id, (void*)state);
+  else
+ nspace-ZeroID = state;
+
+  entry-ID = id;
+  insert_at_tail(nspace-Severity[severity], entry-link);
+   }
+
+out:
+   return !!(state  ENABLED_BIT);
+}
+
+/**
+ * Sets the state of the given message ID in a client-controlled
+ * namespace.
+ * 'source' and 'type' are array indices like TYPE_ERROR, not GL enums.
+ */
+static void
+set_message_state(struct gl_context *ctx, int source, int type,
+  GLuint id, GLboolean enabled)
+{
+   struct gl_client_namespace *nspace =
+ ctx-Debug.ClientIDs.Namespaces[source][type];
+   uintptr_t state;
+
+   /* In addition to 

[Mesa-dev] [PATCH 11/13] mesa: print GL errors via debug_output

2012-02-15 Thread Marek Olšák
---
 src/mesa/main/errors.c |   97 ---
 1 files changed, 66 insertions(+), 31 deletions(-)

diff --git a/src/mesa/main/errors.c b/src/mesa/main/errors.c
index e736e04..611c77d 100644
--- a/src/mesa/main/errors.c
+++ b/src/mesa/main/errors.c
@@ -939,21 +939,8 @@ _mesa_problem( const struct gl_context *ctx, const char 
*fmtString, ... )
}
 }
 
-
-/**
- * Record an OpenGL state error.  These usually occur when the user
- * passes invalid parameters to a GL function.
- *
- * If debugging is enabled (either at compile-time via the DEBUG macro, or
- * run-time via the MESA_DEBUG environment variable), report the error with
- * _mesa_debug().
- * 
- * \param ctx the GL context.
- * \param error the error value.
- * \param fmtString printf() style format string, followed by optional args
- */
-void
-_mesa_error( struct gl_context *ctx, GLenum error, const char *fmtString, ... )
+static GLboolean
+should_output(struct gl_context *ctx, GLenum error, const char *fmtString)
 {
static GLint debug = -1;
 
@@ -975,29 +962,77 @@ _mesa_error( struct gl_context *ctx, GLenum error, const 
char *fmtString, ... )
 #endif
}
 
-   if (debug) {  
-  if (ctx-ErrorValue == error 
-  ctx-ErrorDebugFmtString == fmtString) {
- ctx-ErrorDebugCount++;
+   if (debug) {
+  if (ctx-ErrorValue != error ||
+  ctx-ErrorDebugFmtString != fmtString) {
+ flush_delayed_errors( ctx );
+ ctx-ErrorDebugFmtString = fmtString;
+ ctx-ErrorDebugCount = 0;
+ return GL_TRUE;
   }
-  else {
- char s[MAXSTRING], s2[MAXSTRING];
- va_list args;
+  ctx-ErrorDebugCount++;
+   }
+   return GL_FALSE;
+}
 
- flush_delayed_errors( ctx );
- 
- va_start(args, fmtString);
- _mesa_vsnprintf(s, MAXSTRING, fmtString, args);
- va_end(args);
 
- _mesa_snprintf(s2, MAXSTRING, %s in %s, error_string(error), s);
+/**
+ * Record an OpenGL state error.  These usually occur when the user
+ * passes invalid parameters to a GL function.
+ *
+ * If debugging is enabled (either at compile-time via the DEBUG macro, or
+ * run-time via the MESA_DEBUG environment variable), report the error with
+ * _mesa_debug().
+ * 
+ * \param ctx the GL context.
+ * \param error the error value.
+ * \param fmtString printf() style format string, followed by optional args
+ */
+void
+_mesa_error( struct gl_context *ctx, GLenum error, const char *fmtString, ... )
+{
+   GLboolean do_output, do_log;
+
+   do_output = should_output(ctx, error, fmtString);
+   do_log = should_log(ctx, GL_DEBUG_SOURCE_API_ARB, GL_DEBUG_TYPE_ERROR_ARB,
+   API_ERROR_UNKNOWN, GL_DEBUG_SEVERITY_HIGH_ARB);
+
+   if (do_output || do_log) {
+  char s[MAXSTRING], s2[MAXSTRING];
+  int len;
+  va_list args;
+
+  va_start(args, fmtString);
+  len = _mesa_vsnprintf(s, MAXSTRING, fmtString, args);
+  va_end(args);
+
+  if (len = MAXSTRING) {
+ /* Too long error message. Whoever calls _mesa_error should use
+  * shorter strings. */
+ ASSERT(0);
+ return;
+  }
+
+  len = _mesa_snprintf(s2, MAXSTRING, %s in %s, error_string(error), s);
+  if (len = MAXSTRING) {
+ /* Same as above. */
+ ASSERT(0);
+ return;
+  }
+
+  /* Print the error to stderr if needed. */
+  if (do_output) {
  output_if_debug(Mesa: User error, s2, GL_TRUE);
- 
- ctx-ErrorDebugFmtString = fmtString;
- ctx-ErrorDebugCount = 0;
+  }
+
+  /* Log the error via ARB_debug_output if needed.*/
+  if (do_log) {
+ _mesa_log_msg(ctx, GL_DEBUG_SOURCE_API_ARB, GL_DEBUG_TYPE_ERROR_ARB,
+   API_ERROR_UNKNOWN, GL_DEBUG_SEVERITY_HIGH_ARB, len, s2);
   }
}
 
+   /* Set the GL context error state for glGetError. */
_mesa_record_error(ctx, error);
 }
 
-- 
1.7.5.4

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


[Mesa-dev] [PATCH 12/13] mesa: display list dispatch for ARB_debug_output

2012-02-15 Thread Marek Olšák
---
 src/mesa/main/dlist.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/src/mesa/main/dlist.c b/src/mesa/main/dlist.c
index 677debf..231d7e4 100644
--- a/src/mesa/main/dlist.c
+++ b/src/mesa/main/dlist.c
@@ -10390,6 +10390,9 @@ _mesa_create_save_table(void)
SET_TextureStorage2DEXT(table, _mesa_TextureStorage2DEXT);
SET_TextureStorage3DEXT(table, _mesa_TextureStorage3DEXT);
 
+   /* GL_ARB_debug_output (no dlist support) */
+   _mesa_init_errors_dispatch(table);
+
return table;
 }
 
-- 
1.7.5.4

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


[Mesa-dev] [PATCH 13/13] mesa: expose ARB_debug_output

2012-02-15 Thread Marek Olšák
---
 src/mesa/main/extensions.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/mesa/main/extensions.c b/src/mesa/main/extensions.c
index d945124..732b9a6 100644
--- a/src/mesa/main/extensions.c
+++ b/src/mesa/main/extensions.c
@@ -82,6 +82,7 @@ static const struct extension extension_table[] = {
{ GL_ARB_color_buffer_float,  o(ARB_color_buffer_float),  
GL, 2004 },
{ GL_ARB_copy_buffer, o(ARB_copy_buffer), 
GL, 2008 },
{ GL_ARB_conservative_depth,  o(ARB_conservative_depth),  
GL, 2011 },
+   { GL_ARB_debug_output,o(dummy_true),  
GL, 2009 },
{ GL_ARB_depth_buffer_float,  o(ARB_depth_buffer_float),  
GL, 2008 },
{ GL_ARB_depth_clamp, o(ARB_depth_clamp), 
GL, 2003 },
{ GL_ARB_depth_texture,   o(ARB_depth_texture),   
GL, 2001 },
-- 
1.7.5.4

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


Re: [Mesa-dev] [PATCH v3 1/2] configure.ac: Enable GLX_USE_TLS if possible.

2012-02-15 Thread tf (mobile)
Even if the system supports tls, the x server may not have been built with it.  
As i recall, there is an issue with mismatching tls between x and drivers... 
Can a non-tls server load a tls driver?

Anyway, if there's an issue there, this check must be more complicated -- it 
can't be just, does the system support this,  it would instead need to be 
can the system support this and is it enabled in the x server.

IIRC Eric and ajax and Dan know more, but there should also be a discussion of 
this in the xorg-devel and/or mesa archives.

-tom

On 15.02.2012, at 12:41, zhigang.g...@linux.intel.com wrote:

 From: Zhigang Gong zhigang.g...@linux.intel.com
 
 If the system support tls, we prefer to enable it by default
 just as xserver does. Actually, the checking code is copied
 from xserver/configure.ac.
 According to nobled's suggestion, move the checking before
 enable_asm. As if tls_model is not supported, then asm may
 can't work correctly. Then we check the tls model first,
 and if doesn't support it, we need to disable the asm code.
 Here is the reference:
 https://bugs.freedesktop.org/show_bug.cgi?id=35268
 
 Signed-off-by: Zhigang Gong zhigang.g...@linux.intel.com
 Reviewed-by: Dan Nicholson dbn.li...@gmail.com
 ---
 acinclude.m4 |   41 +
 configure.ac |   25 ++---
 2 files changed, 63 insertions(+), 3 deletions(-)
 
 diff --git a/acinclude.m4 b/acinclude.m4
 index a5b389d..23805f3 100644
 --- a/acinclude.m4
 +++ b/acinclude.m4
 @@ -117,3 +117,44 @@ if test $enable_pic != no; then
 fi
 AC_SUBST([PIC_FLAGS])
 ])# MESA_PIC_FLAGS
 +
 +dnl TLS detection
 +AC_DEFUN([MESA_TLS],
 +[AC_MSG_CHECKING(for thread local storage (TLS) support)
 +AC_CACHE_VAL(ac_cv_tls, [
 +ac_cv_tls=none
 +keywords=__thread __declspec(thread)
 +for kw in $keywords ; do
 +AC_TRY_COMPILE([int $kw test;], [], ac_cv_tls=$kw)
 +done
 +])
 +AC_MSG_RESULT($ac_cv_tls)
 +
 +if test $ac_cv_tls != none; then
 +STRICT_CFLAGS=-pedantic -Werror
 +# Add -Werror=attributes if supported (gcc 4.2  later)
 +AC_MSG_CHECKING([if $CC supports -Werror=attributes])
 +save_CFLAGS=$CFLAGS
 +CFLAGS=$CFLAGS $STRICT_CFLAGS -Werror=attributes
 +AC_TRY_COMPILE([int test;], [],
 +   [STRICT_CFLAGS=$STRICT_CFLAGS -Werror=attributes
 +AC_MSG_RESULT([yes])],
 +   [AC_MSG_RESULT([no])])
 +CFLAGS=$save_CFLAGS $STRICT_CFLAGS
 +
 +AC_MSG_CHECKING(for tls_model attribute support)
 +AC_CACHE_VAL(ac_cv_tls_model, [
 +AC_TRY_COMPILE([int $ac_cv_tls 
 __attribute__((tls_model(initial-exec))) test;], [],
 +   ac_cv_tls_model=yes, ac_cv_tls_model=no)
 +])
 +AC_MSG_RESULT($ac_cv_tls_model)
 +
 +if test x$ac_cv_tls_model = xyes ; then
 +TLS=$ac_cv_tls' __attribute__((tls_model(\initial-exec\)))'
 +else
 +TLS=$ac_cv_tls
 +fi
 +DEFINES=$DEFINES -DTLS=\$TLS\
 +CFLAGS=$save_CFLAGS
 +fi
 +])
 diff --git a/configure.ac b/configure.ac
 index b2b1ab8..3226a09 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -433,6 +433,11 @@ AC_SUBST([VG_LIB_GLOB])
 AC_SUBST([GLAPI_LIB_GLOB])
 
 dnl
 +dnl Check tls model support
 +dnl
 +MESA_TLS
 +
 +dnl
 dnl Arch/platform-specific settings
 dnl
 AC_ARG_ENABLE([asm],
 @@ -446,6 +451,14 @@ ASM_FLAGS=
 MESA_ASM_SOURCES=
 GLAPI_ASM_SOURCES=
 AC_MSG_CHECKING([whether to enable assembly])
 +
 +if test x$ac_cv_tls_model = xno ; then
 +dnl
 +dnl If tls model is not supported, disable asm.
 +dnl
 +enable_asm=no
 +fi
 +
 test x$enable_asm = xno  AC_MSG_RESULT([no])
 # disable if cross compiling on x86/x86_64 since we must run gen_matypes
 if test x$enable_asm = xyes  test x$cross_compiling = xyes; then
 @@ -1102,9 +1115,15 @@ dnl
 
 AC_ARG_ENABLE([glx-tls],
 [AS_HELP_STRING([--enable-glx-tls],
 -[enable TLS support in GLX @:@default=disabled@:@])],
 -[GLX_USE_TLS=$enableval],
 -[GLX_USE_TLS=no])
 +[enable TLS support in GLX @:@default=auto@:@])],
 +[GLX_USE_TLS=$enableval
 + if test x$GLX_USE_TLS = xyes  test ${ac_cv_tls} = none ; then
 +AC_MSG_ERROR([GLX with TLS support requested, but the compiler does 
 not support it.])
 + fi],
 +[GLX_USE_TLS=no
 + if test ${ac_cv_tls} != none ; then
 +GLX_USE_TLS=yes
 + fi])
 AC_SUBST(GLX_TLS, ${GLX_USE_TLS})
 
 AS_IF([test x$GLX_USE_TLS = xyes],
 -- 
 1.7.4.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 v3 1/2] configure.ac: Enable GLX_USE_TLS if possible.

2012-02-15 Thread Matt Turner
On Wed, Feb 15, 2012 at 7:52 AM, tf (mobile) tfo...@sci.utah.edu wrote:
 Even if the system supports tls, the x server may not have been built with 
 it.  As i recall, there is an issue with mismatching tls between x and 
 drivers... Can a non-tls server load a tls driver?

I don't think mismatching TLS/non-TLS Mesa and X server works, but I
don't think that's a combination anyone cares to support (or is
possible to support?).

 Anyway, if there's an issue there, this check must be more complicated -- it 
 can't be just, does the system support this,  it would instead need to be 
 can the system support this and is it enabled in the x server.

The X server isn't required to compile Mesa, so it doesn't make sense
to try to check if the X server supports TLS. I think the idea is, if
the system supports TLS, let's default to enabled for both Mesa and
the X server, and if you do something else and it breaks then you get
to keep the pieces.

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


Re: [Mesa-dev] [PATCH 00/13] ARB_debug_output

2012-02-15 Thread Matt Turner
On Wed, Feb 15, 2012 at 8:28 AM, Marek Olšák mar...@gmail.com wrote:
 nobled (10):

Linus doesn't even accept patches from people who don't set their name
in ~/.gitconfig. Can you please set your name?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] i965/fs: Take # of components into account in try_rewrite_rhs_to_dst.

2012-02-15 Thread Eric Anholt
On Tue, 14 Feb 2012 12:43:22 -0800, Kenneth Graunke kenn...@whitecape.org 
wrote:
 Commit dc7f449d1ac53a66e6efb56ccf2a5953418a26ca introduced a new method
 for avoiding MOVs: try to rewrite the destination of the instruction
 that produced the RHS so it writes into the LHS.
 
 Unfortunately, this is not safe for swizzled texturing operations, as
 they return a set of four contiguous registers.  Consider the following:
 
 (assign (x)
 (var_ref vec_ctor_x)
 (swiz x (tex vec4 (var_ref m_sampY) (var_ref m_cordY) 0 1 (
 
 In this case, the source and destination registers are equal, since
 reg_offset is 0 for both.  Yet, this is only a partial move: the texture
 operation generates four registers, and the LHS only covers one.
 
 Fixes color distortion in XBMC when using GLSL shaders.
 
 NOTE: This is a candidate for the 8.0 branch (with the previous commit).

Nice!  Series is

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


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


[Mesa-dev] [Bug 46082] piglit ext_texture_integer-api-teximage regression

2012-02-15 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=46082

--- Comment #1 from Brian Paul bri...@vmware.com 2012-02-15 14:18:47 PST ---
I think this is actually a bug in the piglit test.

The glTexSubImage call in question can generate either GL_INVALID_ENUM or 
GL_INVALID_OPERATION depending on what order glTexSubImage() does its parameter 
error
checking:

From the spec:
INVALID_ENUM is generated by DrawPixels, TexImage* and
SubTexImage* if format is one of the integer component formats
described in table 3.6 and type is FLOAT.
and:
INVALID_OPERATION is generated by TexImage* and SubTexImage* if
the texture internalformat is an integer format as described in
table 3.16 and format is not one of the integer component
formats described in table 3.6, or if the internalformat is not an
integer format and format is an integer format.

I can fix the piglit test, but after reviewing Mesa it looks like we're missing 
the later error check for glTexSubImage.  I'll work on that too...

-- 
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] [PATCH 1/4] vbo: Remove pedantic warning about 'end' beind out of bounds.

2012-02-15 Thread Ian Romanick

On 02/08/2012 11:41 AM, Kenneth Graunke wrote:

On 02/08/2012 06:59 AM, Brian Paul wrote:

On 02/08/2012 06:08 AM, Kenneth Graunke wrote:

The application supplied [start, end] range is merely a conservative
hint of the ranges of index values inside the index buffer. There is no
requirement that all vertices in the range [start, end] be referenced.

Passing an 'end' value larger than the maximum legal index is perfectly
acceptible; applications can legally pass 0x when they don't
have a tighter bound readily available.

Thus, the warning doesn't indicate a correctness issue; it could only
indicate a performance issue. However, it does not even do that.

glDrawRangeElements is designed to optimize non-VBO vertex data uploads
by providing an upper bound on the size of buffers a driver would need
to allocate. With VBOs, the data is already in an uploaded buffer, so
the range doesn't help.

The clincher is: we only know _MaxElement for VBOs. For user-space
arrays, we just set it to 2,000,000,000 (see mesa/main/varray.h:63.)
So we can only check this in the case where it is not useful.

Many applications, including the Unigine demos, currently trigger this
warning, which suggests the applications are buggy when they're actually
fine. Eliminating the warning should confuse users less while not
actually losing any benefit to application developers.

NOTE: This is a candidate for release branches.

Suggested-by: Jose Fonsecajfons...@vmware.com
Signed-off-by: Kenneth Graunkekenn...@whitecape.org
---
src/mesa/vbo/vbo_exec_array.c | 49
+---
1 files changed, 2 insertions(+), 47 deletions(-)

diff --git a/src/mesa/vbo/vbo_exec_array.c
b/src/mesa/vbo/vbo_exec_array.c
index d6b4d61..ec4cb4f 100644
--- a/src/mesa/vbo/vbo_exec_array.c
+++ b/src/mesa/vbo/vbo_exec_array.c
@@ -708,6 +708,7 @@ vbo_exec_DrawArraysInstanced(GLenum mode, GLint
start, GLsizei count,
* Map GL_ELEMENT_ARRAY_BUFFER and print contents.
* For debugging.
*/
+#if 0
static void
dump_element_buffer(struct gl_context *ctx, GLenum type)
{
@@ -759,6 +760,7 @@ dump_element_buffer(struct gl_context *ctx, GLenum
type)

ctx-Driver.UnmapBuffer(ctx,
ctx-Array.ArrayObj-ElementArrayBufferObj);
}
+#endif


Not a big deal, but someday I might wind up removing the #if 0 / #endif
so that I can use this in gdb when debugging something.


Yeah, it looked like useful functionality, so I wanted to keep it, but I
was getting an unused function warning.


You could make it non-static and surround it with #ifdef DEBUG or some 
such.  That could be a follow-on patch.

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


[Mesa-dev] [PATCH] mesa: add missing texture integer test in glTexSubImage()

2012-02-15 Thread Brian Paul
If the texture format is integer, the incoming user data must also be
integer (and similarly for non-integer textures).

NOTE: This is a candidate for the stable branches.
---
 src/mesa/main/teximage.c |   11 +++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
index e4eb7f6..a3ffb01 100644
--- a/src/mesa/main/teximage.c
+++ b/src/mesa/main/teximage.c
@@ -1852,6 +1852,17 @@ subtexture_error_check2( struct gl_context *ctx, GLuint 
dimensions,
   } 
}
 
+   if (ctx-VersionMajor = 3 || ctx-Extensions.EXT_texture_integer) {
+  /* both source and dest must be integer-valued, or neither */
+  if (_mesa_is_format_integer_color(destTex-TexFormat) !=
+  _mesa_is_integer_format(format)) {
+ _mesa_error(ctx, GL_INVALID_OPERATION,
+ glTexSubImage%d(integer/non-integer format mismatch),
+ dimensions);
+ return GL_TRUE;
+  }
+   }
+
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 2/6] i965/gen7: Skip checking if we need a GS program for now.

2012-02-15 Thread Eric Anholt
We always say no.  Improves VS state change microbenchmark performance
7.68747% +/- 1.40826% (n=10).
---
 src/mesa/drivers/dri/i965/brw_state_upload.c |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c 
b/src/mesa/drivers/dri/i965/brw_state_upload.c
index f5e6fdc..ea50695 100644
--- a/src/mesa/drivers/dri/i965/brw_state_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
@@ -183,7 +183,6 @@ const struct brw_tracked_state *gen7_atoms[] =
 
brw_wm_input_sizes,
brw_vs_prog,
-   brw_gs_prog,
brw_wm_prog,
 
/* Command packets: */
-- 
1.7.9

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


[Mesa-dev] [PATCH 5/6] i965: Split the gen6 GS binding table to a separate table.

2012-02-15 Thread Eric Anholt
Improves VS state change microbenchmark performance by 7.08729% +/-
1.22289% (n=10) on gen7, because we don't upload the 64 dwords of
unused binding table any more.
---
 src/mesa/drivers/dri/i965/brw_context.h  |   23 +++---
 src/mesa/drivers/dri/i965/brw_misc_state.c   |2 +-
 src/mesa/drivers/dri/i965/brw_state.h|1 +
 src/mesa/drivers/dri/i965/brw_state_upload.c |1 +
 src/mesa/drivers/dri/i965/gen6_sol.c |   58 +-
 5 files changed, 75 insertions(+), 10 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
b/src/mesa/drivers/dri/i965/brw_context.h
index 44a01e6..9c89617 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -484,11 +484,6 @@ struct brw_vs_ouput_sizes {
  *|   . | .   |
  *|   : | :   |
  *|  24 | Texture 15  |
- *+-|-+
- *|  25 | SOL Binding 0   |
- *|   . | .   |
- *|   : | :   |
- *|  88 | SOL Binding 63  |
  *+---+
  *
  * Our VS binding tables are programmed as follows:
@@ -502,6 +497,15 @@ struct brw_vs_ouput_sizes {
  *|  16 | Texture 15  |
  *+---+
  *
+ * Our (gen6) GS binding tables are programmed as follows:
+ *
+ *+-+-+
+ *|  0  | SOL Binding 0   |
+ *|   . | .   |
+ *|   : | :   |
+ *|  63 | SOL Binding 63  |
+ *+-+-+
+ *
  * Note that nothing actually uses the SURF_INDEX_DRAW macro, so it has to be
  * the identity function or things will break.  We do want to keep draw buffers
  * first so we can use headerless render target writes for RT 0.
@@ -509,15 +513,17 @@ struct brw_vs_ouput_sizes {
 #define SURF_INDEX_DRAW(d)   (d)
 #define SURF_INDEX_FRAG_CONST_BUFFER (BRW_MAX_DRAW_BUFFERS + 1)
 #define SURF_INDEX_TEXTURE(t)(BRW_MAX_DRAW_BUFFERS + 2 + (t))
-#define SURF_INDEX_SOL_BINDING(t)(SURF_INDEX_TEXTURE(BRW_MAX_TEX_UNIT) + 
(t))
 
 /** Maximum size of the binding table. */
-#define BRW_MAX_SURFACES SURF_INDEX_SOL_BINDING(BRW_MAX_SOL_BINDINGS)
+#define BRW_MAX_SURFACES SURF_INDEX_TEXTURE(BRW_MAX_TEX_UNIT)
 
 #define SURF_INDEX_VERT_CONST_BUFFER (0)
 #define SURF_INDEX_VS_TEXTURE(t) (SURF_INDEX_VERT_CONST_BUFFER + 1 + (t))
 #define BRW_MAX_VS_SURFACES  SURF_INDEX_VS_TEXTURE(BRW_MAX_TEX_UNIT)
 
+#define SURF_INDEX_SOL_BINDING(t)((t))
+#define BRW_MAX_GS_SURFACES  
SURF_INDEX_SOL_BINDING(BRW_MAX_SOL_BINDINGS)
+
 enum brw_cache_id {
BRW_BLEND_STATE,
BRW_DEPTH_STENCIL_STATE,
@@ -868,6 +874,9 @@ struct brw_context
   /** Offset in the program cache to the CLIP program pre-gen6 */
   uint32_t prog_offset;
   uint32_t state_offset;
+
+  uint32_t bind_bo_offset;
+  uint32_t surf_offset[BRW_MAX_VS_SURFACES];
} gs;
 
struct {
diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c 
b/src/mesa/drivers/dri/i965/brw_misc_state.c
index 7bc7e1c..c86755d 100644
--- a/src/mesa/drivers/dri/i965/brw_misc_state.c
+++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
@@ -116,7 +116,7 @@ static void upload_gen6_binding_table_pointers(struct 
brw_context *brw)
 GEN6_BINDING_TABLE_MODIFY_PS |
 (4 - 2));
OUT_BATCH(brw-vs.bind_bo_offset); /* vs */
-   OUT_BATCH(brw-bind.bo_offset); /* gs */
+   OUT_BATCH(brw-gs.bind_bo_offset); /* gs */
OUT_BATCH(brw-bind.bo_offset); /* wm/ps */
ADVANCE_BATCH();
 }
diff --git a/src/mesa/drivers/dri/i965/brw_state.h 
b/src/mesa/drivers/dri/i965/brw_state.h
index 59a2bb3..a58b4b3 100644
--- a/src/mesa/drivers/dri/i965/brw_state.h
+++ b/src/mesa/drivers/dri/i965/brw_state.h
@@ -90,6 +90,7 @@ extern const struct brw_tracked_state gen6_clip_vp;
 extern const struct brw_tracked_state gen6_color_calc_state;
 extern const struct brw_tracked_state gen6_depth_stencil_state;
 extern const struct brw_tracked_state gen6_gs_state;
+extern const struct brw_tracked_state gen6_gs_binding_table;
 extern const struct brw_tracked_state gen6_renderbuffer_surfaces;
 extern const struct brw_tracked_state gen6_sampler_state;
 extern const struct brw_tracked_state gen6_scissor_state;
diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c 
b/src/mesa/drivers/dri/i965/brw_state_upload.c
index 28e4d26..3f5c03d 100644
--- a/src/mesa/drivers/dri/i965/brw_state_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
@@ -148,6 +148,7 @@ static const struct brw_tracked_state *gen6_atoms[] =
brw_texture_surfaces,
gen6_sol_surface,
brw_vs_binding_table,
+   gen6_gs_binding_table,
brw_binding_table,
 
brw_samplers,
diff --git a/src/mesa/drivers/dri/i965/gen6_sol.c 
b/src/mesa/drivers/dri/i965/gen6_sol.c
index 41923b7..fbd8e71 100644
--- 

[Mesa-dev] [PATCH 4/6] i965: Split the VS binding table to a separate table.

2012-02-15 Thread Eric Anholt
This is a step toward making the samplers/binding tables reflect
sampler uniform mappings instead of embedding those in the programs.
No significant performance difference on the microbenchmark (n=10).
---
 src/mesa/drivers/dri/i965/brw_context.h  |   34 +++---
 src/mesa/drivers/dri/i965/brw_misc_state.c   |4 +-
 src/mesa/drivers/dri/i965/brw_state.h|1 +
 src/mesa/drivers/dri/i965/brw_state_upload.c |3 +
 src/mesa/drivers/dri/i965/brw_vec4_emit.cpp  |2 +-
 src/mesa/drivers/dri/i965/brw_vs.c   |5 ++
 src/mesa/drivers/dri/i965/brw_vs_surface_state.c |   51 +-
 src/mesa/drivers/dri/i965/brw_wm_surface_state.c |8 ++-
 src/mesa/drivers/dri/i965/gen7_vs_state.c|3 +-
 9 files changed, 94 insertions(+), 17 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
b/src/mesa/drivers/dri/i965/brw_context.h
index 98f68e7..44a01e6 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -409,6 +409,8 @@ struct brw_vs_prog_data {
bool uses_new_param_layout;
bool uses_vertexid;
bool userclip;
+
+   int num_surfaces;
 };
 
 
@@ -468,7 +470,7 @@ struct brw_vs_ouput_sizes {
  * (VS, HS, DS, GS, PS), we currently share a single binding table for all of
  * them.  This is purely for convenience.
  *
- * Currently our binding tables are (arbitrarily) programmed as follows:
+ * Currently our SOL/WM binding tables are (arbitrarily) programmed as follows:
  *
  *+---+
  *|   0 | Draw buffer 0   | .
@@ -476,18 +478,28 @@ struct brw_vs_ouput_sizes {
  *|   : | :   |Only relevant to the WM.
  *|   7 | Draw buffer 7   |  /
  *|-|-| `
- *|   8 | VS Pull Constant Buffer |
- *|   9 | WM Pull Constant Buffer |
+ *|   8 | WM Pull Constant Buffer |
  *|-|-|
- *|  10 | Texture 0   |
+ *|   9 | Texture 0   |
  *|   . | .   |
  *|   : | :   |
- *|  25 | Texture 15  |
+ *|  24 | Texture 15  |
  *+-|-+
- *|  26 | SOL Binding 0   |
+ *|  25 | SOL Binding 0   |
+ *|   . | .   |
+ *|   : | :   |
+ *|  88 | SOL Binding 63  |
+ *+---+
+ *
+ * Our VS binding tables are programmed as follows:
+ *
+ *+-+-+ `
+ *|   0 | VS Pull Constant Buffer |
+ *+-+-+
+ *|   1 | Texture 0   |
  *|   . | .   |
  *|   : | :   |
- *|  89 | SOL Binding 63  |
+ *|  16 | Texture 15  |
  *+---+
  *
  * Note that nothing actually uses the SURF_INDEX_DRAW macro, so it has to be
@@ -495,7 +507,6 @@ struct brw_vs_ouput_sizes {
  * first so we can use headerless render target writes for RT 0.
  */
 #define SURF_INDEX_DRAW(d)   (d)
-#define SURF_INDEX_VERT_CONST_BUFFER (BRW_MAX_DRAW_BUFFERS + 0)
 #define SURF_INDEX_FRAG_CONST_BUFFER (BRW_MAX_DRAW_BUFFERS + 1)
 #define SURF_INDEX_TEXTURE(t)(BRW_MAX_DRAW_BUFFERS + 2 + (t))
 #define SURF_INDEX_SOL_BINDING(t)(SURF_INDEX_TEXTURE(BRW_MAX_TEX_UNIT) + 
(t))
@@ -503,6 +514,10 @@ struct brw_vs_ouput_sizes {
 /** Maximum size of the binding table. */
 #define BRW_MAX_SURFACES SURF_INDEX_SOL_BINDING(BRW_MAX_SOL_BINDINGS)
 
+#define SURF_INDEX_VERT_CONST_BUFFER (0)
+#define SURF_INDEX_VS_TEXTURE(t) (SURF_INDEX_VERT_CONST_BUFFER + 1 + (t))
+#define BRW_MAX_VS_SURFACES  SURF_INDEX_VS_TEXTURE(BRW_MAX_TEX_UNIT)
+
 enum brw_cache_id {
BRW_BLEND_STATE,
BRW_DEPTH_STENCIL_STATE,
@@ -841,6 +856,9 @@ struct brw_context
   */
   uint8_t *ra_reg_to_grf;
   /** @} */
+
+  uint32_t bind_bo_offset;
+  uint32_t surf_offset[BRW_MAX_VS_SURFACES];
} vs;
 
struct {
diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c 
b/src/mesa/drivers/dri/i965/brw_misc_state.c
index 0343ae1..7bc7e1c 100644
--- a/src/mesa/drivers/dri/i965/brw_misc_state.c
+++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
@@ -77,7 +77,7 @@ static void upload_binding_table_pointers(struct brw_context 
*brw)
 
BEGIN_BATCH(6);
OUT_BATCH(_3DSTATE_BINDING_TABLE_POINTERS  16 | (6 - 2));
-   OUT_BATCH(brw-bind.bo_offset);
+   OUT_BATCH(brw-vs.bind_bo_offset);
OUT_BATCH(0); /* gs */
OUT_BATCH(0); /* clip */
OUT_BATCH(0); /* sf */
@@ -115,7 +115,7 @@ static void upload_gen6_binding_table_pointers(struct 
brw_context *brw)
 GEN6_BINDING_TABLE_MODIFY_GS |
 GEN6_BINDING_TABLE_MODIFY_PS |
 (4 - 2));
-   OUT_BATCH(brw-bind.bo_offset); /* vs */
+   OUT_BATCH(brw-vs.bind_bo_offset); /* vs */

[Mesa-dev] [PATCH 3/6] i965/gen6+: Avoid recomputing whether we use noperspective.

2012-02-15 Thread Eric Anholt
Improves VS state change microbenchmark performance 2.38246% +/-
1.15046% (n=20).
---
 src/mesa/drivers/dri/i965/brw_context.h |4 ---
 src/mesa/drivers/dri/i965/gen6_clip_state.c |   31 --
 src/mesa/drivers/dri/i965/gen7_clip_state.c |   11 -
 3 files changed, 10 insertions(+), 36 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
b/src/mesa/drivers/dri/i965/brw_context.h
index a5a98b2..98f68e7 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -1063,10 +1063,6 @@ brw_update_sol_surface(struct brw_context *brw,
uint32_t *out_offset, unsigned num_vector_components,
unsigned stride_dwords, unsigned offset_dwords);
 
-/* gen6_clip_state.c */
-bool
-brw_fprog_uses_noperspective(const struct gl_fragment_program *fprog);
-
 /* gen6_sol.c */
 void
 brw_begin_transform_feedback(struct gl_context *ctx, GLenum mode,
diff --git a/src/mesa/drivers/dri/i965/gen6_clip_state.c 
b/src/mesa/drivers/dri/i965/gen6_clip_state.c
index b3bb8ae..abf1d76 100644
--- a/src/mesa/drivers/dri/i965/gen6_clip_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_clip_state.c
@@ -31,26 +31,6 @@
 #include brw_util.h
 #include intel_batchbuffer.h
 
-/**
- * Return true if at least one of the inputs used by the given fragment
- * program has the GLSL noperspective interpolation qualifier.
- */
-bool
-brw_fprog_uses_noperspective(const struct gl_fragment_program *fprog)
-{
-   int attr;
-   for (attr = 0; attr  FRAG_ATTRIB_MAX; ++attr) {
-  /* Ignore unused inputs. */
-  if (!(fprog-Base.InputsRead  BITFIELD64_BIT(attr)))
- continue;
-
-  if (fprog-InterpQualifier[attr] == INTERP_QUALIFIER_NOPERSPECTIVE)
- return true;
-   }
-   return false;
-}
-
-
 static void
 upload_clip_state(struct brw_context *brw)
 {
@@ -59,10 +39,10 @@ upload_clip_state(struct brw_context *brw)
uint32_t depth_clamp = 0;
uint32_t provoking, userclip;
uint32_t nonperspective_barycentric_enable_flag = 0;
-   /* BRW_NEW_FRAGMENT_PROGRAM */
-   const struct gl_fragment_program *fprog = brw-fragment_program;
 
-   if (brw_fprog_uses_noperspective(fprog)) {
+   /* CACHE_NEW_WM_PROG */
+   if (brw-wm.prog_data-barycentric_interp_modes 
+   (1  BRW_WM_NONPERSPECTIVE_PIXEL_BARYCENTRIC)) {
   nonperspective_barycentric_enable_flag =
  GEN6_CLIP_NON_PERSPECTIVE_BARYCENTRIC_ENABLE;
}
@@ -106,9 +86,8 @@ upload_clip_state(struct brw_context *brw)
 const struct brw_tracked_state gen6_clip_state = {
.dirty = {
   .mesa  = _NEW_TRANSFORM | _NEW_LIGHT,
-  .brw   = (BRW_NEW_CONTEXT |
-BRW_NEW_FRAGMENT_PROGRAM),
-  .cache = 0
+  .brw   = (BRW_NEW_CONTEXT),
+  .cache = CACHE_NEW_WM_PROG
},
.emit = upload_clip_state,
 };
diff --git a/src/mesa/drivers/dri/i965/gen7_clip_state.c 
b/src/mesa/drivers/dri/i965/gen7_clip_state.c
index c32cd98..f78b089 100644
--- a/src/mesa/drivers/dri/i965/gen7_clip_state.c
+++ b/src/mesa/drivers/dri/i965/gen7_clip_state.c
@@ -36,13 +36,13 @@ upload_clip_state(struct brw_context *brw)
uint32_t provoking, userclip;
uint32_t dw1 = GEN6_CLIP_STATISTICS_ENABLE;
uint32_t nonperspective_barycentric_enable_flag = 0;
-   /* BRW_NEW_FRAGMENT_PROGRAM */
-   const struct gl_fragment_program *fprog = brw-fragment_program;
 
/* _NEW_BUFFERS */
bool render_to_fbo = brw-intel.ctx.DrawBuffer-Name != 0;
 
-   if (brw_fprog_uses_noperspective(fprog)) {
+   /* CACHE_NEW_WM_PROG */
+   if (brw-wm.prog_data-barycentric_interp_modes 
+   (1  BRW_WM_NONPERSPECTIVE_PIXEL_BARYCENTRIC)) {
   nonperspective_barycentric_enable_flag =
  GEN6_CLIP_NON_PERSPECTIVE_BARYCENTRIC_ENABLE;
}
@@ -115,9 +115,8 @@ const struct brw_tracked_state gen7_clip_state = {
 _NEW_POLYGON |
 _NEW_LIGHT |
 _NEW_TRANSFORM),
-  .brw   = (BRW_NEW_CONTEXT |
-BRW_NEW_FRAGMENT_PROGRAM),
-  .cache = 0
+  .brw   = BRW_NEW_CONTEXT,
+  .cache = CACHE_NEW_WM_PROG
},
.emit = upload_clip_state,
 };
-- 
1.7.9

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


[Mesa-dev] [PATCH 1/6] i965: Compute required barycentric interp modes once at FS compile time.

2012-02-15 Thread Eric Anholt
Improves VS state change microbenchmark performance 1.78817% +/-
0.556878% (n=25).
---
 src/mesa/drivers/dri/i965/brw_context.h   |   11 ++-
 src/mesa/drivers/dri/i965/brw_wm.c|9 +
 src/mesa/drivers/dri/i965/gen6_wm_state.c |7 ++-
 src/mesa/drivers/dri/i965/gen7_wm_state.c |   10 --
 4 files changed, 17 insertions(+), 20 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
b/src/mesa/drivers/dri/i965/brw_context.h
index 503585c..a5a98b2 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -271,6 +271,12 @@ struct brw_wm_prog_data {
int dispatch_width;
uint32_t prog_offset_16;
 
+   /**
+* Mask of which interpolation modes are required by the fragment shader.
+* Used in hardware setup on gen6+.
+*/
+   uint32_t barycentric_interp_modes;
+
/* Pointer to tracked values (only valid once
 * _mesa_load_state_parameters has been called at runtime).
 */
@@ -1049,11 +1055,6 @@ int brw_disasm (FILE *file, struct brw_instruction 
*inst, int gen);
 /* brw_vs.c */
 gl_clip_plane *brw_select_clip_planes(struct gl_context *ctx);
 
-/* brw_wm.c */
-unsigned
-brw_compute_barycentric_interp_modes(bool shade_model_flat,
- const struct gl_fragment_program *fprog);
-
 /* brw_wm_surface_state.c */
 void brw_init_surface_formats(struct brw_context *brw);
 void
diff --git a/src/mesa/drivers/dri/i965/brw_wm.c 
b/src/mesa/drivers/dri/i965/brw_wm.c
index 7dee20b..e59ab62 100644
--- a/src/mesa/drivers/dri/i965/brw_wm.c
+++ b/src/mesa/drivers/dri/i965/brw_wm.c
@@ -128,7 +128,7 @@ brw_wm_non_glsl_emit(struct brw_context *brw, struct 
brw_wm_compile *c)
  * Return a bitfield where bit n is set if barycentric interpolation mode n
  * (see enum brw_wm_barycentric_interp_mode) is needed by the fragment shader.
  */
-unsigned
+static unsigned
 brw_compute_barycentric_interp_modes(bool shade_model_flat,
  const struct gl_fragment_program *fprog)
 {
@@ -174,9 +174,7 @@ brw_wm_payload_setup(struct brw_context *brw,
struct intel_context *intel = brw-intel;
bool uses_depth = (c-fp-program.Base.InputsRead 
  (1  FRAG_ATTRIB_WPOS)) != 0;
-   unsigned barycentric_interp_modes =
-  brw_compute_barycentric_interp_modes(c-key.flat_shade,
-   c-fp-program);
+   unsigned barycentric_interp_modes = c-prog_data.barycentric_interp_modes;
int i;
 
if (intel-gen = 6) {
@@ -278,6 +276,9 @@ bool do_wm_prog(struct brw_context *brw,
 
brw_init_compile(brw, c-func, c);
 
+   c-prog_data.barycentric_interp_modes =
+  brw_compute_barycentric_interp_modes(c-key.flat_shade, fp-program);
+
if (prog  prog-_LinkedShaders[MESA_SHADER_FRAGMENT]) {
   if (!brw_wm_fs_emit(brw, c, prog))
 return false;
diff --git a/src/mesa/drivers/dri/i965/gen6_wm_state.c 
b/src/mesa/drivers/dri/i965/gen6_wm_state.c
index 205e648..fd1eca4 100644
--- a/src/mesa/drivers/dri/i965/gen6_wm_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_wm_state.c
@@ -99,9 +99,6 @@ upload_wm_state(struct brw_context *brw)
   brw_fragment_program_const(brw-fragment_program);
uint32_t dw2, dw4, dw5, dw6;
 
-   /* _NEW_LIGHT */
-   bool flat_shade = (ctx-Light.ShadeModel == GL_FLAT);
-
 /* CACHE_NEW_WM_PROG */
if (brw-wm.prog_data-nr_params == 0) {
   /* Disable the push constant buffers. */
@@ -173,7 +170,8 @@ upload_wm_state(struct brw_context *brw)
   dw5 |= GEN6_WM_USES_SOURCE_DEPTH | GEN6_WM_USES_SOURCE_W;
if (fp-program.Base.OutputsWritten  BITFIELD64_BIT(FRAG_RESULT_DEPTH))
   dw5 |= GEN6_WM_COMPUTED_DEPTH;
-   dw6 |= brw_compute_barycentric_interp_modes(flat_shade, fp-program) 
+   /* CACHE_NEW_WM_PROG */
+   dw6 |= brw-wm.prog_data-barycentric_interp_modes 
   GEN6_WM_BARYCENTRIC_INTERPOLATION_MODE_SHIFT;
 
/* _NEW_COLOR */
@@ -210,7 +208,6 @@ upload_wm_state(struct brw_context *brw)
 const struct brw_tracked_state gen6_wm_state = {
.dirty = {
   .mesa  = (_NEW_LINE |
-_NEW_LIGHT |
_NEW_COLOR |
_NEW_BUFFERS |
_NEW_PROGRAM_CONSTANTS |
diff --git a/src/mesa/drivers/dri/i965/gen7_wm_state.c 
b/src/mesa/drivers/dri/i965/gen7_wm_state.c
index 870590f..8037966 100644
--- a/src/mesa/drivers/dri/i965/gen7_wm_state.c
+++ b/src/mesa/drivers/dri/i965/gen7_wm_state.c
@@ -41,9 +41,6 @@ upload_wm_state(struct brw_context *brw)
bool writes_depth = false;
uint32_t dw1;
 
-   /* _NEW_LIGHT */
-   bool flat_shade = (ctx-Light.ShadeModel == GL_FLAT);
-
dw1 = 0;
dw1 |= GEN7_WM_STATISTICS_ENABLE;
dw1 |= GEN7_WM_LINE_AA_WIDTH_1_0;
@@ -64,7 +61,8 @@ upload_wm_state(struct brw_context *brw)
   writes_depth = true;
   dw1 |= GEN7_WM_PSCDEPTH_ON;
}
-   dw1 |= brw_compute_barycentric_interp_modes(flat_shade, fp-program) 
+   /* CACHE_NEW_WM_PROG */
+   dw1 |= 

[Mesa-dev] [RFC] Merging feature work to Mesa master

2012-02-15 Thread Ian Romanick
Over the last six months a lot of feature work has happened in Mesa, and 
the load has been carried by a lot of different people / organization. 
In the process, we discovered a number of development process issues 
that made things more difficult than they needed to be.


The biggest problem the we encountered was the number of features marked 
50% done in GL3.txt.  To complete these features, a significant amount 
of time had to be spent figuring out what was done, and what was left to 
do.  Since the changes had landed on master a very long time ago, this 
was a frustrating and error prone process.  Transform feedback was the 
worst case of this.  One developer spent several weeks trying to assess 
the state of development.  In the process, a couple items were missed 
and not detected until early January.


PROPOSAL: No feature will be merged to master until a vertical slice[1] 
of functionality is complete.


To be specific, this means that some useful workload should be able to 
correctly execute.  This doesn't mean that everything is done or even 
that any particular real application needs to work.  At the very least 
there should be a demo or piglit test that uses the functionality in its 
intended manner that works.


This means that some incomplete features may sit on branches of a long 
time.  That's okay!  It's really easy to see what has been done on a 
branch because you can 'git log origin/master..EXT_some_feature'.  This 
trivializes the assessment of the state of development.


We encountered similar problems with pieces of functionality that were 
ostensibly done.  Many elements of OpenGL functionality are like Koch 
snowflakes:  everything is a corner case.  Integer textures and 
floating-point textures are prime cases of this.  Even though the 
implementation was done and enabled in several drivers, we had no way to 
assess the quality.  The same problem holds in cases of features that 
are known to be incomplete, even if a vertical slice is functional.


PROPOSAL: No feature will be merged to master until a robust set of 
tests are implemented or at least documented.


We don't necessarily need 10,000 tests for some feature, but there 
should be some.  There should also be a list of test this corner case, 
test that corner case, check this error condition, etc.  As an example, 
we've come up with a list of missing test cases for 
EXT_framebuffer_multisample:


- Test turning multisample on and off on a MSAA buffer.
- Test multisample points smooth
- Test multisample points non-smooth
- Test multisample lines smooth
- Test multisample lines non-smooth
- Test multisample line stipple
- Test multisample polygon smooth
- Test multisample polygon stipple
- Test multisample glBitmap
- Test sample alpha to one
- Test sample coverage
- Test sample coverage invert
- Test sample alpha-to-coverage, with alpha-to-one
- Test sample alpha-to-coverage, without alpha-to-one
- Test blit multisample-to-multisample (matching sample count)
- Test blit multisample-to-multisample (non-matching sample count)
- Test blit multisample-to-multisample (non-matching format)
- Test blit singlesample-to-multisample
- Test blit multisample-to-multisample (scaling blit)
- Iterate all the above for all the sample counts
- Iterate all internal formats for some basic MSAA test
- All of the above for depth and stencil
- Try really hard to hit HiZ resolves too

This list represents about 30 or 40 test cases.  By giving some 
indication of areas that need testing, we communicate the parts of the 
feature that are known to be incomplete or just plain broken (Keith's 
1st Law: Any piece of code that isn't tested is broken.).


The test list also need not necessarily come from the developer.  It is 
completely reasonable and desirable for reviewers to suggest or 
implement tests.  All of that information just needs to be captured, and 
consensus must be achieved.


The onus of enforcing these proposals will fall on the reviewers.  This 
still leaves room for sensible things to happen while preventing the 
problems that we encountered.  For example, it should be fine to merge 
Marek and nobled's ARB_debug work with few tests implemented or 
specified.  The same could not be said for the proposed implementation 
of ARB_uniform_buffer_object.


[1] http://en.wikipedia.org/wiki/Vertical_slice
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] mesa: fix an issue with texture border and array textures

2012-02-15 Thread Anuj Phogat
As suggested by Brian, for a 1D texture array, the border only applies to
the width. For a 2D texture array the border applies to the width and
height but not to the depth.  This was not handled correctly in
_mesa_init_teximage_fields(). 

Note: This is a candidate for stable branches

Signed-off-by: Anuj Phogat anuj.pho...@gmail.com
---
Tested the patch with piglit quick.tests. No regressions.

 src/mesa/main/teximage.c |8 ++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
index e4eb7f6..d5f2650 100644
--- a/src/mesa/main/teximage.c
+++ b/src/mesa/main/teximage.c
@@ -1083,11 +1083,13 @@ _mesa_init_teximage_fields(struct gl_context *ctx,
GLint border, GLenum internalFormat,
gl_format format)
 {
+   GLenum target;
ASSERT(img);
ASSERT(width = 0);
ASSERT(height = 0);
ASSERT(depth = 0);
 
+   target = img-TexObject-Target;
img-_BaseFormat = _mesa_base_tex_format( ctx, internalFormat );
ASSERT(img-_BaseFormat  0);
img-InternalFormat = internalFormat;
@@ -1099,7 +1101,8 @@ _mesa_init_teximage_fields(struct gl_context *ctx,
img-Width2 = width - 2 * border;   /* == 1  img-WidthLog2; */
img-WidthLog2 = _mesa_logbase2(img-Width2);
 
-   if (height == 1) { /* 1-D texture */
+   if (target ==  GL_TEXTURE_1D ||
+   target ==  GL_TEXTURE_1D_ARRAY) { /* 1-D texture */
   img-Height2 = 1;
   img-HeightLog2 = 0;
}
@@ -1108,7 +,8 @@ _mesa_init_teximage_fields(struct gl_context *ctx,
   img-HeightLog2 = _mesa_logbase2(img-Height2);
}
 
-   if (depth == 1) {  /* 2-D texture */
+   if (target == GL_TEXTURE_2D ||
+   target == GL_TEXTURE_2D_ARRAY) {  /* 2-D texture */
   img-Depth2 = 1;
   img-DepthLog2 = 0;
}
-- 
1.7.7.6

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


Re: [Mesa-dev] [PATCH v3 1/2] configure.ac: Enable GLX_USE_TLS if possible.

2012-02-15 Thread Zhigang Gong
 -Original Message-
 From: tf (mobile) [mailto:tfo...@sci.utah.edu]
 Sent: Wednesday, February 15, 2012 8:53 PM
 To: zhigang.g...@linux.intel.com
 Cc: dbn.li...@gmail.com; nob...@dreamwidth.org;
 mesa-dev@lists.freedesktop.org
 Subject: Re: [Mesa-dev] [PATCH v3 1/2] configure.ac: Enable GLX_USE_TLS
 if possible.
 
 Even if the system supports tls, the x server may not have been built with
it.
 As i recall, there is an issue with mismatching tls between x and
drivers...
 Can a non-tls server load a tls driver?
AFAIK, a mesa built without TLS can work with a xserver built with TLS
enabled,
Actually, this is current situation. Xserver will enable TLS if the system
support
It, but mesa will not enable TLS by default no matter the system support it.

IMO, the only relevant part may be the indirect GLX in xserver which will
load
DRI driver and thus mesa. But xserver has its own glapi.c and will not share
the mesa's glapi.c, then the mismatch will not be a big issue. If I missed
anything,
please correct me.

Anyway, I still think it's better to keep the xserver and mesa have the same
configuration
when they are built/running on the same system. Right?

Besides that issue, I think mesa should check whether the system support
TLS and tls model before enable it. So I still want this patchset get
accepted.

Any comment?

 
 Anyway, if there's an issue there, this check must be more complicated --
it
 can't be just, does the system support this,  it would instead need to
be
 can the system support this and is it enabled in the x server.
 
 IIRC Eric and ajax and Dan know more, but there should also be a
 discussion of this in the xorg-devel and/or mesa archives.
 
 -tom
 
 On 15.02.2012, at 12:41, zhigang.g...@linux.intel.com wrote:
 
  From: Zhigang Gong zhigang.g...@linux.intel.com
 
  If the system support tls, we prefer to enable it by default just as
  xserver does. Actually, the checking code is copied from
  xserver/configure.ac.
  According to nobled's suggestion, move the checking before
 enable_asm.
  As if tls_model is not supported, then asm may can't work correctly.
  Then we check the tls model first, and if doesn't support it, we need
  to disable the asm code.
  Here is the reference:
  https://bugs.freedesktop.org/show_bug.cgi?id=35268
 
  Signed-off-by: Zhigang Gong zhigang.g...@linux.intel.com
  Reviewed-by: Dan Nicholson dbn.li...@gmail.com
  ---
  acinclude.m4 |   41
 +
  configure.ac |   25 ++---
  2 files changed, 63 insertions(+), 3 deletions(-)
 
  diff --git a/acinclude.m4 b/acinclude.m4 index a5b389d..23805f3
 100644
  --- a/acinclude.m4
  +++ b/acinclude.m4
  @@ -117,3 +117,44 @@ if test $enable_pic != no; then fi
  AC_SUBST([PIC_FLAGS])
  ])# MESA_PIC_FLAGS
  +
  +dnl TLS detection
  +AC_DEFUN([MESA_TLS],
  +[AC_MSG_CHECKING(for thread local storage (TLS) support)
  +AC_CACHE_VAL(ac_cv_tls, [
  +ac_cv_tls=none
  +keywords=__thread __declspec(thread)
  +for kw in $keywords ; do
  +AC_TRY_COMPILE([int $kw test;], [], ac_cv_tls=$kw)
  +done
  +])
  +AC_MSG_RESULT($ac_cv_tls)
  +
  +if test $ac_cv_tls != none; then
  +STRICT_CFLAGS=-pedantic -Werror
  +# Add -Werror=attributes if supported (gcc 4.2  later)
  +AC_MSG_CHECKING([if $CC supports -Werror=attributes])
  +save_CFLAGS=$CFLAGS
  +CFLAGS=$CFLAGS $STRICT_CFLAGS -Werror=attributes
  +AC_TRY_COMPILE([int test;], [],
  +   [STRICT_CFLAGS=$STRICT_CFLAGS
 -Werror=attributes
  +AC_MSG_RESULT([yes])],
  +   [AC_MSG_RESULT([no])])
  +CFLAGS=$save_CFLAGS $STRICT_CFLAGS
  +
  +AC_MSG_CHECKING(for tls_model attribute support)
  +AC_CACHE_VAL(ac_cv_tls_model, [
  +AC_TRY_COMPILE([int $ac_cv_tls
 __attribute__((tls_model(initial-exec))) test;], [],
  +   ac_cv_tls_model=yes, ac_cv_tls_model=no)
  +])
  +AC_MSG_RESULT($ac_cv_tls_model)
  +
  +if test x$ac_cv_tls_model = xyes ; then
  +TLS=$ac_cv_tls' __attribute__((tls_model(\initial-exec\)))'
  +else
  +TLS=$ac_cv_tls
  +fi
  +DEFINES=$DEFINES -DTLS=\$TLS\
  +CFLAGS=$save_CFLAGS
  +fi
  +])
  diff --git a/configure.ac b/configure.ac index b2b1ab8..3226a09 100644
  --- a/configure.ac
  +++ b/configure.ac
  @@ -433,6 +433,11 @@ AC_SUBST([VG_LIB_GLOB])
  AC_SUBST([GLAPI_LIB_GLOB])
 
  dnl
  +dnl Check tls model support
  +dnl
  +MESA_TLS
  +
  +dnl
  dnl Arch/platform-specific settings
  dnl
  AC_ARG_ENABLE([asm],
  @@ -446,6 +451,14 @@ ASM_FLAGS=
  MESA_ASM_SOURCES=
  GLAPI_ASM_SOURCES=
  AC_MSG_CHECKING([whether to enable assembly])
  +
  +if test x$ac_cv_tls_model = xno ; then dnl dnl If tls model is
  +not supported, disable asm.
  +dnl
  +enable_asm=no
  +fi
  +
  test x$enable_asm = xno  AC_MSG_RESULT([no]) # disable if cross
  compiling on x86/x86_64 since we must run gen_matypes if test
  x$enable_asm = xyes  test x$cross_compiling = xyes; 

Re: [Mesa-dev] [PATCH] mesa: add missing texture integer test in glTexSubImage()

2012-02-15 Thread Yuanhan Liu
On Wed, Feb 15, 2012 at 03:41:01PM -0700, Brian Paul wrote:
 If the texture format is integer, the incoming user data must also be
 integer (and similarly for non-integer textures).
 
 NOTE: This is a candidate for the stable branches.
 ---
  src/mesa/main/teximage.c |   11 +++
  1 files changed, 11 insertions(+), 0 deletions(-)
 
 diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
 index e4eb7f6..a3ffb01 100644
 --- a/src/mesa/main/teximage.c
 +++ b/src/mesa/main/teximage.c
 @@ -1852,6 +1852,17 @@ subtexture_error_check2( struct gl_context *ctx, 
 GLuint dimensions,
} 
 }
  
 +   if (ctx-VersionMajor = 3 || ctx-Extensions.EXT_texture_integer) {
 +  /* both source and dest must be integer-valued, or neither */
 +  if (_mesa_is_format_integer_color(destTex-TexFormat) !=
 +  _mesa_is_integer_format(format)) {
 + _mesa_error(ctx, GL_INVALID_OPERATION,
 + glTexSubImage%d(integer/non-integer format mismatch),
^
I guess you missed one 'D' here. Other than that,

Reviewed-by: Yuanhan Liu yuanhan@linux.intel.com

 + dimensions);
 + return GL_TRUE;
 +  }
 +   }
 +
 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 mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] Reducing update_texture_state overhead

2012-02-15 Thread Eric Anholt
Here's an attempt at cutting update_texture_state overhead, which has
been noted a few times as a big CPU consumer in Mesa.  It looks pretty
promising on gen7 (though the binding table patches in the other
series would make the impact of patch 2 less significant).  However,
with the GPU hangs in the second patch, I'm wondering whether I'm
missing some reason that _NEW_TEXTURE would need to be flagged in this
function.

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


[Mesa-dev] [PATCH 2/2] mesa: Only flag _NEW_TEXTURE on texture updates when a texture changes.

2012-02-15 Thread Eric Anholt
There are three reasons left for this function flagging it:
Re-computing completeness (and therefore the number of levels),
binding a new current texture object, or you're doing some sort of
fixed function (and nobody cares).  For other cases, like rebinding a
shader, we no longer trigger the driver recomputing every piece of
texture state.

Improves a VS state change microbenchmark by 14.8284% +/- 1.02% (n=10)
on gen7.  On the other hand, it causes GPU hangs on nexuiz.  Any clue
why this might be insufficient?
---
 src/mesa/main/texstate.c |   34 ++
 1 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/src/mesa/main/texstate.c b/src/mesa/main/texstate.c
index 2dbf530..86b8b3d 100644
--- a/src/mesa/main/texstate.c
+++ b/src/mesa/main/texstate.c
@@ -577,7 +577,6 @@ update_texture_state( struct gl_context *ctx )
 */
 
/* TODO: only set this if there are actual changes */
-   ctx-NewState |= _NEW_TEXTURE;
 
/*
 * Update texture unit state.
@@ -588,6 +587,7 @@ update_texture_state( struct gl_context *ctx )
   GLbitfield enabledFragTargets = 0x0;
   GLbitfield enabledTargets = 0x0;
   GLuint texIndex;
+  struct gl_texture_object *oldObj = texUnit-_Current;
 
   /* Get the bitmask of texture target enables.
* enableBits will be a mask of the TEXTURE_*_BIT flags indicating
@@ -609,10 +609,18 @@ update_texture_state( struct gl_context *ctx )
 
   enabledTargets = enabledVertTargets | enabledFragTargets;
 
+  /* This is not flagged with _NEW_TEXTURE, because the test for changing 
of
+   * the _Current texture object will cover every case where _ReallyEnabled
+   * could have changed.
+   */
   texUnit-_ReallyEnabled = 0x0;
 
   if (enabledTargets == 0x0) {
  /* neither vertex nor fragment processing uses this unit */
+if (texUnit-_ReallyEnabled) {
+   ctx-NewState |= _NEW_TEXTURE;
+   texUnit-_ReallyEnabled = 0x0;
+}
  continue;
   }
 
@@ -626,6 +634,10 @@ update_texture_state( struct gl_context *ctx )
  if (enabledTargets  (1  texIndex)) {
 struct gl_texture_object *texObj = texUnit-CurrentTex[texIndex];
 if (!texObj-_Complete) {
+  /* While _Complete is only consumed by us, this call also updates
+ _MaxLevel, which drivers do need to see.
+   */
+  ctx-NewState |= _NEW_TEXTURE;
_mesa_test_texobj_completeness(ctx, texObj);
 }
 if (texObj-_Complete) {
@@ -652,22 +664,26 @@ update_texture_state( struct gl_context *ctx )
 _mesa_reference_texobj(texUnit-_Current, texObj);
 texUnit-_ReallyEnabled = 1  texTarget;
  }
- else {
-/* fixed-function: texture unit is really disabled */
-continue;
- }
   }
+
+  if (oldObj != texUnit-_Current)
+ctx-NewState |= _NEW_TEXTURE;
}
 
 
/* Determine which texture coordinate sets are actually needed */
if (fprog) {
   const GLuint coordMask = (1  MAX_TEXTURE_COORD_UNITS) - 1;
+  GLuint new_coord_units = ((fprog-InputsRead  FRAG_ATTRIB_TEX0) 
+   coordMask);
   /* Note that this gets consumed by update_ffvs_texture_state(). */
-  ctx-Texture._EnabledCoordUnits
- = (fprog-InputsRead  FRAG_ATTRIB_TEX0)  coordMask;
+  if (ctx-Texture._EnabledCoordUnits != new_coord_units) {
+ctx-NewState |= _NEW_TEXTURE;
+ctx-Texture._EnabledCoordUnits = new_coord_units;
+  }
}
else {
+  ctx-NewState |= _NEW_TEXTURE;
   update_fffs_texture_state(ctx);
}
 
@@ -675,8 +691,10 @@ update_texture_state( struct gl_context *ctx )
 * program.  Those state flags are only used in the case of fixed function
 * vertex shading, in the tnl pipeline or ff_vertex_shader.
 */
-   if (!vprog)
+   if (!vprog) {
+  ctx-NewState |= _NEW_TEXTURE;
   update_ffvs_texture_state(ctx);
+   }
 }
 
 
-- 
1.7.9

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


[Mesa-dev] [PATCH 1/2] mesa: Avoid computing fixed function texture state setup if possible.

2012-02-15 Thread Eric Anholt
None of the consumers of this state will be called while the fs or vs
is in place, and the update_texture_state() call will get re-called on
fs/vs change, so it should be safe to skip the computation.  Improves
the performance of a VS state change microbenchmark by 1.60186% +/-
0.443318% (n=20).
---
 src/mesa/main/texstate.c |  117 +++--
 1 files changed, 70 insertions(+), 47 deletions(-)

diff --git a/src/mesa/main/texstate.c b/src/mesa/main/texstate.c
index 187ec9c..2dbf530 100644
--- a/src/mesa/main/texstate.c
+++ b/src/mesa/main/texstate.c
@@ -476,6 +476,68 @@ update_tex_combine(struct gl_context *ctx, struct 
gl_texture_unit *texUnit)
}
 }
 
+static void
+update_fffs_texture_state(struct gl_context *ctx)
+{
+   GLuint unit;
+
+   ctx-Texture._EnabledUnits = 0;
+   ctx-Texture._EnabledCoordUnits = 0;
+
+   for (unit = 0; unit  ctx-Const.MaxCombinedTextureImageUnits; unit++) {
+  struct gl_texture_unit *texUnit = ctx-Texture.Unit[unit];
+
+  if (!texUnit-_ReallyEnabled)
+continue;
+
+  ctx-Texture._EnabledUnits |= (1  unit);
+  ctx-Texture._EnabledCoordUnits |= (1  unit);
+
+  update_tex_combine(ctx, texUnit);
+   }
+}
+
+static void
+update_ffvs_texture_state(struct gl_context *ctx)
+{
+   GLuint unit;
+
+   ctx-Texture._GenFlags = 0x0;
+   ctx-Texture._TexMatEnabled = 0x0;
+   ctx-Texture._TexGenEnabled = 0x0;
+
+   /* Setup texgen for those texture coordinate sets that are in use */
+   for (unit = 0; unit  ctx-Const.MaxTextureCoordUnits; unit++) {
+  struct gl_texture_unit *texUnit = ctx-Texture.Unit[unit];
+
+  texUnit-_GenFlags = 0x0;
+
+  if (!(ctx-Texture._EnabledCoordUnits  (1  unit)))
+continue;
+
+  if (texUnit-TexGenEnabled) {
+if (texUnit-TexGenEnabled  S_BIT) {
+   texUnit-_GenFlags |= texUnit-GenS._ModeBit;
+}
+if (texUnit-TexGenEnabled  T_BIT) {
+   texUnit-_GenFlags |= texUnit-GenT._ModeBit;
+}
+if (texUnit-TexGenEnabled  R_BIT) {
+   texUnit-_GenFlags |= texUnit-GenR._ModeBit;
+}
+if (texUnit-TexGenEnabled  Q_BIT) {
+   texUnit-_GenFlags |= texUnit-GenQ._ModeBit;
+}
+
+ctx-Texture._TexGenEnabled |= ENABLE_TEXGEN(unit);
+ctx-Texture._GenFlags |= texUnit-_GenFlags;
+  }
+
+  ASSERT(unit  Elements(ctx-TextureMatrixStack));
+  if (ctx-TextureMatrixStack[unit].Top-type != MATRIX_IDENTITY)
+ctx-Texture._TexMatEnabled |= ENABLE_TEXMAT(unit);
+   }
+}
 
 /**
  * \note This routine refers to derived texture matrix values to
@@ -491,7 +553,6 @@ update_texture_state( struct gl_context *ctx )
GLuint unit;
struct gl_program *fprog = NULL;
struct gl_program *vprog = NULL;
-   GLbitfield enabledFragUnits = 0x0;
 
if (ctx-Shader.CurrentVertexProgram 
ctx-Shader.CurrentVertexProgram-LinkStatus) {
@@ -518,11 +579,6 @@ update_texture_state( struct gl_context *ctx )
/* TODO: only set this if there are actual changes */
ctx-NewState |= _NEW_TEXTURE;
 
-   ctx-Texture._EnabledUnits = 0x0;
-   ctx-Texture._GenFlags = 0x0;
-   ctx-Texture._TexMatEnabled = 0x0;
-   ctx-Texture._TexGenEnabled = 0x0;
-
/*
 * Update texture unit state.
 */
@@ -601,59 +657,26 @@ update_texture_state( struct gl_context *ctx )
 continue;
  }
   }
-
-  /* if we get here, we know this texture unit is enabled */
-
-  ctx-Texture._EnabledUnits |= (1  unit);
-
-  if (enabledFragTargets)
- enabledFragUnits |= (1  unit);
-
-  update_tex_combine(ctx, texUnit);
}
 
 
/* Determine which texture coordinate sets are actually needed */
if (fprog) {
   const GLuint coordMask = (1  MAX_TEXTURE_COORD_UNITS) - 1;
+  /* Note that this gets consumed by update_ffvs_texture_state(). */
   ctx-Texture._EnabledCoordUnits
  = (fprog-InputsRead  FRAG_ATTRIB_TEX0)  coordMask;
}
else {
-  ctx-Texture._EnabledCoordUnits = enabledFragUnits;
+  update_fffs_texture_state(ctx);
}
 
-   /* Setup texgen for those texture coordinate sets that are in use */
-   for (unit = 0; unit  ctx-Const.MaxTextureCoordUnits; unit++) {
-  struct gl_texture_unit *texUnit = ctx-Texture.Unit[unit];
-
-  texUnit-_GenFlags = 0x0;
-
-  if (!(ctx-Texture._EnabledCoordUnits  (1  unit)))
-continue;
-
-  if (texUnit-TexGenEnabled) {
-if (texUnit-TexGenEnabled  S_BIT) {
-   texUnit-_GenFlags |= texUnit-GenS._ModeBit;
-}
-if (texUnit-TexGenEnabled  T_BIT) {
-   texUnit-_GenFlags |= texUnit-GenT._ModeBit;
-}
-if (texUnit-TexGenEnabled  R_BIT) {
-   texUnit-_GenFlags |= texUnit-GenR._ModeBit;
-}
-if (texUnit-TexGenEnabled  Q_BIT) {
-   texUnit-_GenFlags |= texUnit-GenQ._ModeBit;
-}
-
-ctx-Texture._TexGenEnabled |= ENABLE_TEXGEN(unit);
-ctx-Texture._GenFlags |= texUnit-_GenFlags;
-  }
-
- 

Re: [Mesa-dev] [PATCH 2/2] mesa: Only flag _NEW_TEXTURE on texture updates when a texture changes.

2012-02-15 Thread Stéphane Marchesin
On Wed, Feb 15, 2012 at 18:01, Eric Anholt e...@anholt.net wrote:
 There are three reasons left for this function flagging it:
 Re-computing completeness (and therefore the number of levels),
 binding a new current texture object, or you're doing some sort of
 fixed function (and nobody cares).  For other cases, like rebinding a
 shader, we no longer trigger the driver recomputing every piece of
 texture state.

 Improves a VS state change microbenchmark by 14.8284% +/- 1.02% (n=10)
 on gen7.  On the other hand, it causes GPU hangs on nexuiz.  Any clue
 why this might be insufficient?
 ---
  src/mesa/main/texstate.c |   34 ++
  1 files changed, 26 insertions(+), 8 deletions(-)

 diff --git a/src/mesa/main/texstate.c b/src/mesa/main/texstate.c
 index 2dbf530..86b8b3d 100644
 --- a/src/mesa/main/texstate.c
 +++ b/src/mesa/main/texstate.c
 @@ -577,7 +577,6 @@ update_texture_state( struct gl_context *ctx )
     */

    /* TODO: only set this if there are actual changes */

I think that comment should go away too.

 -   ctx-NewState |= _NEW_TEXTURE;

    /*
     * Update texture unit state.
 @@ -588,6 +587,7 @@ update_texture_state( struct gl_context *ctx )
       GLbitfield enabledFragTargets = 0x0;
       GLbitfield enabledTargets = 0x0;
       GLuint texIndex;
 +      struct gl_texture_object *oldObj = texUnit-_Current;

       /* Get the bitmask of texture target enables.
        * enableBits will be a mask of the TEXTURE_*_BIT flags indicating
 @@ -609,10 +609,18 @@ update_texture_state( struct gl_context *ctx )

       enabledTargets = enabledVertTargets | enabledFragTargets;

 +      /* This is not flagged with _NEW_TEXTURE, because the test for 
 changing of
 +       * the _Current texture object will cover every case where 
 _ReallyEnabled
 +       * could have changed.
 +       */
       texUnit-_ReallyEnabled = 0x0;

       if (enabledTargets == 0x0) {
          /* neither vertex nor fragment processing uses this unit */
 +        if (texUnit-_ReallyEnabled) {
 +           ctx-NewState |= _NEW_TEXTURE;
 +           texUnit-_ReallyEnabled = 0x0;
 +        }
          continue;
       }

 @@ -626,6 +634,10 @@ update_texture_state( struct gl_context *ctx )
          if (enabledTargets  (1  texIndex)) {
             struct gl_texture_object *texObj = texUnit-CurrentTex[texIndex];
             if (!texObj-_Complete) {
 +              /* While _Complete is only consumed by us, this call also 
 updates
 +                 _MaxLevel, which drivers do need to see.
 +               */
 +              ctx-NewState |= _NEW_TEXTURE;
                _mesa_test_texobj_completeness(ctx, texObj);
             }
             if (texObj-_Complete) {
 @@ -652,22 +664,26 @@ update_texture_state( struct gl_context *ctx )
             _mesa_reference_texobj(texUnit-_Current, texObj);
             texUnit-_ReallyEnabled = 1  texTarget;
          }
 -         else {
 -            /* fixed-function: texture unit is really disabled */
 -            continue;
 -         }
       }
 +
 +      if (oldObj != texUnit-_Current)
 +        ctx-NewState |= _NEW_TEXTURE;
    }


    /* Determine which texture coordinate sets are actually needed */
    if (fprog) {
       const GLuint coordMask = (1  MAX_TEXTURE_COORD_UNITS) - 1;
 +      GLuint new_coord_units = ((fprog-InputsRead  FRAG_ATTRIB_TEX0) 
 +                               coordMask);
       /* Note that this gets consumed by update_ffvs_texture_state(). */
 -      ctx-Texture._EnabledCoordUnits
 -         = (fprog-InputsRead  FRAG_ATTRIB_TEX0)  coordMask;
 +      if (ctx-Texture._EnabledCoordUnits != new_coord_units) {
 +        ctx-NewState |= _NEW_TEXTURE;
 +        ctx-Texture._EnabledCoordUnits = new_coord_units;
 +      }
    }
    else {
 +      ctx-NewState |= _NEW_TEXTURE;
       update_fffs_texture_state(ctx);
    }

 @@ -675,8 +691,10 @@ update_texture_state( struct gl_context *ctx )
     * program.  Those state flags are only used in the case of fixed function
     * vertex shading, in the tnl pipeline or ff_vertex_shader.
     */
 -   if (!vprog)
 +   if (!vprog) {
 +      ctx-NewState |= _NEW_TEXTURE;
       update_ffvs_texture_state(ctx);
 +   }
  }


 --
 1.7.9

 ___
 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 v3 1/2] configure.ac: Enable GLX_USE_TLS if possible.

2012-02-15 Thread tom fogal

On 02/15/2012 05:15 PM, Matt Turner wrote:

On Wed, Feb 15, 2012 at 7:52 AM, tf (mobile)tfo...@sci.utah.edu  wrote:

Even if the system supports tls, the x server may not have been built with it.  
As i recall, there is an issue with mismatching tls between x and drivers... 
Can a non-tls server load a tls driver?


I don't think mismatching TLS/non-TLS Mesa and X server works, but I
don't think that's a combination anyone cares to support (or is
possible to support?).


Depends on the mismatch.  If X is TLS and Mesa isn't, no problem.  If 
Mesa is TLS and X isn't, drivers won't load.  There was some vague 
interest on getting that to work a while back, but nobody wanted to do 
the work and it wasn't considered very important.



Anyway, if there's an issue there, this check must be more complicated -- it can't be just, 
does the system support this,  it would instead need to be can the system support 
this and is it enabled in the x server.


The X server isn't required to compile Mesa, so it doesn't make sense
to try to check if the X server supports TLS.


This is a silly argument: we can very easily detect the cases in which 
Mesa is built to be used as an X driver or not.


 I think the idea is, if

the system supports TLS, let's default to enabled for both Mesa and
the X server, and if you do something else and it breaks then you get
to keep the pieces.


I am concerned about systems which --disable-tls when building X, and 
omit such a flag when building Mesa in a relevant configuration.  I 
think we should have an AC_MSG_ERROR for that case.


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


Re: [Mesa-dev] [PATCH v3 1/2] configure.ac: Enable GLX_USE_TLS if possible.

2012-02-15 Thread tom fogal

On 02/16/2012 01:57 AM, Zhigang Gong wrote:

-Original Message-
From: tf (mobile) [mailto:tfo...@sci.utah.edu]
Sent: Wednesday, February 15, 2012 8:53 PM
To: zhigang.g...@linux.intel.com
Cc: dbn.li...@gmail.com; nob...@dreamwidth.org;
mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH v3 1/2] configure.ac: Enable GLX_USE_TLS
if possible.

Even if the system supports tls, the x server may not have been
built with it. As i recall, there is an issue with mismatching tls
between x and drivers... Can a non-tls server load a tls driver?



AFAIK, a mesa built without TLS can work with a xserver built with
TLS enabled, Actually, this is current situation. Xserver will enable
TLS if the system support It, but mesa will not enable TLS by default
no matter the system support it.

IMO, the only relevant part may be the indirect GLX in xserver which
will load DRI driver and thus mesa. But xserver has its own glapi.c
and will not share the mesa's glapi.c, then the mismatch will not be
a big issue. If I missed anything, please correct me.


I think you're forgetting the converse case: when the X server is 
non-TLS and Mesa is.



Anyway, I still think it's better to keep the xserver and mesa have
the same configuration when they are built/running on the same
system. Right?


Yes, I agree.


Besides that issue, I think mesa should check whether the system
support TLS and tls model before enable it. So I still want this
patchset get accepted.

Any comment?


Checking is arguably good.  Auto-enabling is what I really want/like.

I am just relaying the message that there was concern here.  Previously 
there were parties (i.e. Eric on the Mesa side) who were pretty wary of 
said changes.. I tried your patch almost verbatim a while back, before 
implementing this in the X server.  Those parties should be made aware 
this is going in and should give their LGTMs.


This is all in the archives.

-tom


Anyway, if there's an issue there, this check must be more
complicated -- it can't be just, does the system support this,
it would instead need to be can the system support this and is it
enabled in the x server.

IIRC Eric and ajax and Dan know more, but there should also be a
discussion of this in the xorg-devel and/or mesa archives.

-tom

On 15.02.2012, at 12:41, zhigang.g...@linux.intel.com wrote:


From: Zhigang Gongzhigang.g...@linux.intel.com

If the system support tls, we prefer to enable it by default just as
xserver does. Actually, the checking code is copied from
xserver/configure.ac.
According to nobled's suggestion, move the checking before

enable_asm.

As if tls_model is not supported, then asm may can't work correctly.
Then we check the tls model first, and if doesn't support it, we need
to disable the asm code.
Here is the reference:
https://bugs.freedesktop.org/show_bug.cgi?id=35268

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