Re: [Mesa-dev] [PATCH 09/29] mesa: Add _mesa_swap2_copy and _mesa_swap4_copy
No good reason really, simply that the original functions seemed simpler
for the case of in-place swapping since you don't have to pass the dst
parameter explicitly, so I figured there was a marginal gain in letting
them stay, specially since their implementation is just an inline call
to the other version. Do you prefer the other solution?
Iago
On Wed, 2014-11-19 at 12:00 -0800, Jason Ekstrand wrote:
> Any reason why you added 2 new functions, instead of just altering the
> ones we have and updating where they are used?
>
>
> On Tue, Nov 18, 2014 at 1:23 AM, Iago Toral Quiroga
> wrote:
> We have _mesa_swap{2,4} but these do in-place byte-swapping
> only. The new
> functions receive an extra parameter so we can swap bytes on a
> source
> input array and store the results in a (possibly different)
> destination
> array.
>
> This is useful to implement byte-swapping in pixel uploads,
> since in this
> case we need to swap bytes on the src data which is owned by
> the
> application so we can't do an in-place byte swap.
> ---
> src/mesa/main/image.c | 25 +
> src/mesa/main/image.h | 10 --
> 2 files changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/src/mesa/main/image.c b/src/mesa/main/image.c
> index 4ea5f04..9ad97c5 100644
> --- a/src/mesa/main/image.c
> +++ b/src/mesa/main/image.c
> @@ -41,36 +41,45 @@
>
>
> /**
> - * Flip the order of the 2 bytes in each word in the given
> array.
> + * Flip the order of the 2 bytes in each word in the given
> array (src) and
> + * store the result in another array (dst). For in-place
> byte-swapping this
> + * function can be called with the same array for src and
> dst.
> *
> - * \param p array.
> + * \param dst the array where byte-swapped data will be
> stored.
> + * \param src the array with the source data we want to
> byte-swap.
> * \param n number of words.
> */
> void
> -_mesa_swap2( GLushort *p, GLuint n )
> +_mesa_swap2_copy( GLushort *dst, GLushort *src, GLuint n )
> {
> GLuint i;
> for (i = 0; i < n; i++) {
> - p[i] = (p[i] >> 8) | ((p[i] << 8) & 0xff00);
> + dst[i] = (src[i] >> 8) | ((src[i] << 8) & 0xff00);
> }
> }
>
>
>
> /*
> - * Flip the order of the 4 bytes in each word in the given
> array.
> + * Flip the order of the 4 bytes in each word in the given
> array (src) and
> + * store the result in another array (dst). For in-place
> byte-swapping this
> + * function can be called with the same array for src and
> dst.
> + *
> + * \param dst the array where byte-swapped data will be
> stored.
> + * \param src the array with the source data we want to
> byte-swap.
> + * \param n number of words.
> */
> void
> -_mesa_swap4( GLuint *p, GLuint n )
> +_mesa_swap4_copy( GLuint *dst, GLuint *src, GLuint n )
> {
> GLuint i, a, b;
> for (i = 0; i < n; i++) {
> - b = p[i];
> + b = src[i];
>a = (b >> 24)
> | ((b >> 8) & 0xff00)
> | ((b << 8) & 0xff)
> | ((b << 24) & 0xff00);
> - p[i] = a;
> + dst[i] = a;
> }
> }
>
> diff --git a/src/mesa/main/image.h b/src/mesa/main/image.h
> index abd84bf..79c6e68 100644
> --- a/src/mesa/main/image.h
> +++ b/src/mesa/main/image.h
> @@ -33,10 +33,16 @@ struct gl_context;
> struct gl_pixelstore_attrib;
>
> extern void
> -_mesa_swap2( GLushort *p, GLuint n );
> +_mesa_swap2_copy( GLushort *dst, GLushort *src, GLuint n );
>
> extern void
> -_mesa_swap4( GLuint *p, GLuint n );
> +_mesa_swap4_copy( GLuint *dst, GLuint *src, GLuint n );
> +
> +static inline void
> +_mesa_swap2( GLushort *p, GLuint n ) { _mesa_swap2_copy(p, p,
> n); }
> +
> +static inline void
> +_mesa_swap4( GLuint *p, GLuint n ) { _mesa_swap4_copy(p, p,
> n); }
>
> extern GLintptr
> _mesa_image_offset( GLuint dimensions,
> --
> 1.9.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
___
mesa-de
Re: [Mesa-dev] [PATCH 03/29] mesa: Do not assert on integer<->non-integer direct pack/unpack fast paths
It is explained here:
https://bugs.freedesktop.org/show_bug.cgi?id=84566#c35
So one example of this was a glReadPixels where we want to store the
pixel data as RGBA UINT, but the render buffer format we read from is
MESA_FORMAT_B8G8R8A8_UNORM. There are piglit tests that hit this case.
Iago
On Wed, 2014-11-19 at 12:04 -0800, Jason Ekstrand wrote:
> Can you remind me again as to what formats hit these paths? I
> remember you hitting them, but I'm still not really seeing how it
> happens.
>
> --Jason
>
>
> On Tue, Nov 18, 2014 at 1:23 AM, Iago Toral Quiroga
> wrote:
> We can have conversions from non-integer types to integer
> types, so remove
> the assertions for these in the pack/unpack fast paths. It
> could be that
> we do not have all the necessary pack/unpack functions in
> these cases though,
> so protect these paths with conditionals and let
> _mesa_format_convert use
> other paths to resolve these kind of conversions if necessary.
> ---
> src/mesa/main/format_utils.c | 16
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/src/mesa/main/format_utils.c
> b/src/mesa/main/format_utils.c
> index 1d65f2b..56a3b8d 100644
> --- a/src/mesa/main/format_utils.c
> +++ b/src/mesa/main/format_utils.c
> @@ -143,8 +143,8 @@ _mesa_format_convert(void *void_dst,
> uint32_t dst_format, size_t dst_stride,
> dst += dst_stride;
> }
> return;
> - } else if (dst_array_format.as_uint ==
> RGBA_UBYTE.as_uint) {
> - assert(!_mesa_is_format_integer_color(src_format));
> + } else if (dst_array_format.as_uint ==
> RGBA_UBYTE.as_uint &&
> + !_mesa_is_format_integer_color(src_format))
> {
> for (row = 0; row < height; ++row) {
> _mesa_unpack_ubyte_rgba_row(src_format, width,
> src, (uint8_t
> (*)[4])dst);
> @@ -152,8 +152,8 @@ _mesa_format_convert(void *void_dst,
> uint32_t dst_format, size_t dst_stride,
> dst += dst_stride;
> }
> return;
> - } else if (dst_array_format.as_uint ==
> RGBA_UINT.as_uint) {
> - assert(_mesa_is_format_integer_color(src_format));
> + } else if (dst_array_format.as_uint ==
> RGBA_UINT.as_uint &&
> + _mesa_is_format_integer_color(src_format)) {
> for (row = 0; row < height; ++row) {
> _mesa_unpack_uint_rgba_row(src_format, width,
> src, (uint32_t
> (*)[4])dst);
> @@ -174,8 +174,8 @@ _mesa_format_convert(void *void_dst,
> uint32_t dst_format, size_t dst_stride,
> dst += dst_stride;
> }
> return;
> - } else if (src_array_format.as_uint ==
> RGBA_UBYTE.as_uint) {
> - assert(!_mesa_is_format_integer_color(dst_format));
> + } else if (src_array_format.as_uint ==
> RGBA_UBYTE.as_uint &&
> + !_mesa_is_format_integer_color(dst_format))
> {
> for (row = 0; row < height; ++row) {
> _mesa_pack_ubyte_rgba_row(dst_format, width,
>(const uint8_t
> (*)[4])src, dst);
> @@ -183,8 +183,8 @@ _mesa_format_convert(void *void_dst,
> uint32_t dst_format, size_t dst_stride,
> dst += dst_stride;
> }
> return;
> - } else if (src_array_format.as_uint ==
> RGBA_UINT.as_uint) {
> - assert(_mesa_is_format_integer_color(dst_format));
> + } else if (src_array_format.as_uint ==
> RGBA_UINT.as_uint &&
> + _mesa_is_format_integer_color(dst_format)) {
> for (row = 0; row < height; ++row) {
> _mesa_pack_uint_rgba_row(dst_format, width,
> (const uint32_t
> (*)[4])src, dst);
> --
> 1.9.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 07/29] mesa: Add helper to convert a GL format and type to a mesa (array) format.
On Wed, 2014-11-19 at 12:11 -0800, Jason Ekstrand wrote:
> General comment: Maybe this would be better in gltypes rather than in
> mesa_formats
Ok, I'll move it.
> On Tue, Nov 18, 2014 at 1:23 AM, Iago Toral Quiroga
> wrote:
> ---
> src/mesa/main/formats.c | 285
>
> src/mesa/main/formats.h | 3 +
> 2 files changed, 288 insertions(+)
(...)
> +
> +/**
> +* Take an OpenGL format (GL_RGB, GL_RGBA, etc), OpenGL data
> type (GL_INT,
> +* GL_FOAT, etc) and return a matching mesa_array_format or a
> mesa_format
> +* otherwise (for non-array formats).
> +*
> +* This function will typically be used to compute a mesa
> format from a GL type
> +* so we can then call _mesa_format_convert. This function
> does
> +* not consider byte swapping, so it returns types assuming
> that no byte
> +* swapping is involved. If byte swapping is involved then
> clients are supposed
> +* to handle that on their side before calling
> _mesa_format_convert.
> +*
> +* This function returns an uint32_t that can pack a
> mesa_format or a
> +* mesa_array_format. Clients must check the mesa array format
> bit
> +* (MESA_ARRAY_FORMAT_BIT) on the return value to know if the
> returned
> +* format is a mesa_array_format or a mesa_format.
> +*/
> +uint32_t
> +_mesa_format_from_format_and_type(GLenum format, GLenum type)
> +{
> + mesa_array_format array_format;
> +
> + bool is_array_format = true;
> +
> + /* Map the OpenGL data type to an array format data type
> */
> + switch (type) {
> + case GL_UNSIGNED_BYTE:
> + array_format.type = MESA_ARRAY_FORMAT_TYPE_UBYTE;
> + break;
> + case GL_BYTE:
> + array_format.type = MESA_ARRAY_FORMAT_TYPE_BYTE;
> + break;
> + case GL_UNSIGNED_SHORT:
> + array_format.type = MESA_ARRAY_FORMAT_TYPE_USHORT;
> + break;
> + case GL_SHORT:
> + array_format.type = MESA_ARRAY_FORMAT_TYPE_SHORT;
> + break;
> + case GL_UNSIGNED_INT:
> + array_format.type = MESA_ARRAY_FORMAT_TYPE_UINT;
> + break;
> + case GL_INT:
> + array_format.type = MESA_ARRAY_FORMAT_TYPE_INT;
> + break;
> + case GL_HALF_FLOAT:
> + array_format.type = MESA_ARRAY_FORMAT_TYPE_HALF;
> + break;
> + case GL_FLOAT:
> + array_format.type = MESA_ARRAY_FORMAT_TYPE_FLOAT;
> + break;
> + case GL_UNSIGNED_INT_8_8_8_8:
> + case GL_UNSIGNED_INT_8_8_8_8_REV:
> + array_format.type = MESA_ARRAY_FORMAT_TYPE_UBYTE;
>
>
> If you put these in the GL type switch below as returning the
> MESA_FORMAT_R8G8B8A8 or whatever, then the code in
> mesa_format_get_array_format will fix up the swizzling for you.
Do you mean returning a mesa_format instead of a mesa_array_format for
the *8_8_8_8* types? Sure, we can do that.
>
> + break;
> + default:
> + is_array_format = false;
> + break;
> + }
> +
> + /* Next we extract array swizzle information from the
> OpenGL format */
> + if (is_array_format) {
> + is_array_format =
> +
> _mesa_array_format_set_swizzle_from_format(&array_format,
> format);
> + }
> +
> + /* If this is an array format type after checking data
> type and format,
> +* fill in the remaining data
> +*/
> + if (is_array_format) {
> + array_format.normalized = !
> _mesa_is_enum_format_integer(format);
> + array_format.num_channels =
> _mesa_components_in_format(format);
> + array_format.pad = 0;
> + array_format.array_format_bit = 1;
> +
> + /* Check if we need to swap the swizzle for
> GL_UNSIGNED_INT_8_8_8_8* */
> + bool swap_swizzle =
> + (type == GL_UNSIGNED_INT_8_8_8_8 &&
> _mesa_little_endian()) ||
> + (type == GL_UNSIGNED_INT_8_8_8_8_REV && !
> _mesa_little_endian());
> + if (swap_swizzle) {
> + GLubyte swizzle[4], tmp[4];
> + const GLubyte map_3210[6] = { 3, 2, 1, 0, 4, 5 };
> + int i;
> +
> + tmp[0] = array_format.swizzle_x;
> + tmp[1] = array_format.swizzle_y;
> + tmp[2] = array_format.swizzle_z;
> + tmp[3] = array_format.swizzle_w;
> +
> +
Re: [Mesa-dev] [PATCH 09/29] mesa: Add _mesa_swap2_copy and _mesa_swap4_copy
On Wed, 2014-11-19 at 14:15 -0600, Patrick Baggett wrote:
>
>
> On Tue, Nov 18, 2014 at 3:23 AM, Iago Toral Quiroga
> wrote:
> We have _mesa_swap{2,4} but these do in-place byte-swapping
> only. The new
> functions receive an extra parameter so we can swap bytes on a
> source
> input array and store the results in a (possibly different)
> destination
> array.
>
>
>
> If this is being split into an "in-place" and "different pointers"
> version, I think using the "restrict" keyword would be useful here to
> improve the performance. Then, the in-place one cannot be implemented
> as copy(p,p,n), but the code isn't overly complicated.
I did not know about 'restrict', thanks for the tip!.
It kind of defeats the original idea of not duplicating the code but it
is true that it is not particularly complex anyway, so maybe it is worth
it if Jason agrees with having two versions of the functions instead of
just one in the end. Jason, what do you think?
Iago
>
>
> This is useful to implement byte-swapping in pixel uploads,
> since in this
> case we need to swap bytes on the src data which is owned by
> the
> application so we can't do an in-place byte swap.
> ---
> src/mesa/main/image.c | 25 +
> src/mesa/main/image.h | 10 --
> 2 files changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/src/mesa/main/image.c b/src/mesa/main/image.c
> index 4ea5f04..9ad97c5 100644
> --- a/src/mesa/main/image.c
> +++ b/src/mesa/main/image.c
> @@ -41,36 +41,45 @@
>
>
> /**
> - * Flip the order of the 2 bytes in each word in the given
> array.
> + * Flip the order of the 2 bytes in each word in the given
> array (src) and
> + * store the result in another array (dst). For in-place
> byte-swapping this
> + * function can be called with the same array for src and
> dst.
> *
> - * \param p array.
> + * \param dst the array where byte-swapped data will be
> stored.
> + * \param src the array with the source data we want to
> byte-swap.
> * \param n number of words.
> */
> void
> -_mesa_swap2( GLushort *p, GLuint n )
> +_mesa_swap2_copy( GLushort *dst, GLushort *src, GLuint n )
> {
> GLuint i;
> for (i = 0; i < n; i++) {
> - p[i] = (p[i] >> 8) | ((p[i] << 8) & 0xff00);
> + dst[i] = (src[i] >> 8) | ((src[i] << 8) & 0xff00);
> }
> }
>
>
>
> /*
> - * Flip the order of the 4 bytes in each word in the given
> array.
> + * Flip the order of the 4 bytes in each word in the given
> array (src) and
> + * store the result in another array (dst). For in-place
> byte-swapping this
> + * function can be called with the same array for src and
> dst.
> + *
> + * \param dst the array where byte-swapped data will be
> stored.
> + * \param src the array with the source data we want to
> byte-swap.
> + * \param n number of words.
> */
> void
> -_mesa_swap4( GLuint *p, GLuint n )
> +_mesa_swap4_copy( GLuint *dst, GLuint *src, GLuint n )
> {
> GLuint i, a, b;
> for (i = 0; i < n; i++) {
> - b = p[i];
> + b = src[i];
>a = (b >> 24)
> | ((b >> 8) & 0xff00)
> | ((b << 8) & 0xff)
> | ((b << 24) & 0xff00);
> - p[i] = a;
> + dst[i] = a;
> }
> }
>
> diff --git a/src/mesa/main/image.h b/src/mesa/main/image.h
> index abd84bf..79c6e68 100644
> --- a/src/mesa/main/image.h
> +++ b/src/mesa/main/image.h
> @@ -33,10 +33,16 @@ struct gl_context;
> struct gl_pixelstore_attrib;
>
> extern void
> -_mesa_swap2( GLushort *p, GLuint n );
> +_mesa_swap2_copy( GLushort *dst, GLushort *src, GLuint n );
>
> extern void
> -_mesa_swap4( GLuint *p, GLuint n );
> +_mesa_swap4_copy( GLuint *dst, GLuint *src, GLuint n );
> +
> +static inline void
> +_mesa_swap2( GLushort *p, GLuint n ) { _mesa_swap2_copy(p, p,
> n); }
> +
> +static inline void
> +_mesa_swap4( GLuint *p, GLuint n ) { _mesa_swap4_copy(p, p,
> n); }
>
> extern GLintptr
> _mesa_image_offset( GLuint dimensions,
> --
> 1.9.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freed
Re: [Mesa-dev] [PATCH 14/20] mesa: Add non-normalized formats support for ubyte packing functions
On Thu, 2014-11-20 at 08:15 +0100, Samuel Iglesias Gonsálvez wrote:
> On Tue, 2014-11-18 at 11:08 -0800, Jason Ekstrand wrote:
> >
> >
> > On Tue, Nov 18, 2014 at 12:44 AM, Iago Toral Quiroga
> > wrote:
> > From: Samuel Iglesias Gonsalvez
> >
> > Signed-off-by: Samuel Iglesias Gonsalvez
> >
> > ---
> > src/mesa/main/format_pack.c.mako | 10 +-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/mesa/main/format_pack.c.mako
> > b/src/mesa/main/format_pack.c.mako
> > index b9f4656..97adf6e 100644
> > --- a/src/mesa/main/format_pack.c.mako
> > +++ b/src/mesa/main/format_pack.c.mako
> > @@ -84,7 +84,15 @@ pack_ubyte_${f.short_name()}(const GLubyte
> > src[4], void *dst)
> >%endif
> >
> >${channel_datatype(c)} ${c.name} =
> > - %if c.type == parser.UNSIGNED:
> > + %if not f.is_normalized():
> > + %if c.type == parser.FLOAT and c.size == 32:
> > +UBYTE_TO_FLOAT(src[${i}]);
> > + %elif c.type == parser.FLOAT and c.size == 16:
> > +_mesa_float_to_half(UBYTE_TO_FLOAT(src[${i}]));
> >
> >
> > Same question here as in the previous patch. Why are we using
> > UBYTE_TO_FLOAT?
> >
>
> This is what current format_pack.c is doing for those formats and some
> piglit tests complain if it is not there.
>
Jason, this looks correct to me: we are packing from an ubyte type to a
half float type, so first we need to convert from ubyte to float and
then downgrade the float to a half float, we don't currently have means
to convert directly from an ubyte to a half float, right?
Iago
> >
> > + %else:
> > +(${channel_datatype(c)}) src[${i}];
> > + %endif
> > + %elif c.type == parser.UNSIGNED:
> > %if f.colorspace == 'srgb' and c.name in 'rgb':
> > util_format_linear_to_srgb_8unorm(src[${i}]);
> > %else:
> > --
> > 1.9.1
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Using the 'f' suffix to create a float from an integer literal
On Thu, 2014-11-20 at 08:08 +0100, Iago Toral wrote: > On Wed, 2014-11-19 at 10:27 -0800, Ian Romanick wrote: > > On 11/19/2014 03:47 AM, Iago Toral Quiroga wrote: > > > Hi, > > > > > > I came across a GLSL test that checks that doing something like this in > > > a shader should fail: > > > > Is this one of the dEQP tests? > > Yes. > > > > float value = 1f; > > > > It seems like we have a test related to this in piglit somewhere... it > > looks like tests/shaders/glsl-floating-constant-120.shader_test uses > > that syntax, but it's not explicitly testing that feature. > > > > > However, this works fine in Mesa. Checking the spec I see: > > > > > > Floating-point constants are defined as follows. > > > floating-constant: > > >fractional-constant exponent-part(opt) floating-suffix(opt) > > >digit-sequence exponent-part floating-suffix(opt) > > > fractional-constant: > > >digit-sequence . digit-sequence > > >digit-sequence . > > >. digit-sequence > > > exponent-part: > > >e sign(opt) digit-sequence > > >E sign(opt) digit-sequence > > > sign: one of > > >+ - > > > digit-sequence: > > >digit > > >digit-sequence digit > > > floating-suffix: one of > > >f F > > > > > > which suggests that the test is correct and Mesa has a bug. According to > > > the above rules, however, something like this is fine: > > > > > > float f = 1e2f; > > > > > > which I find kind of weird if the other case is not valid, so I wonder > > > if there is a bug in the spec or this is on purpose for some reason that > > > I am missing. > > > > > > Then, to make matters worse, I read this in opengl.org wiki [1]: > > > > > > "A numeric literal that uses a decimal is by default of type float. To > > > create a float literal from an integer value, use the suffix f or F as > > > in C/C++." > > > > > > which contradicts the spec and the test and is aligned with the current > > > way Mesa works. > > > > > > So: does anyone know what version is right? Could this be a bug in the > > > spec? and if it is not, do we want to change the behavior to follow the > > > spec as it is now? > > > > The $64,000 question: What do other GLSL compilers (including, perhaps, > > glslang) do? This seems like one of the cases where nobody is likely to > > follow the spec, and some application will depend on that. If that's > > the case, I'll submit a spec bug. > > Good point. I'll try to check a few cases and reply here. Thanks! I did a quick test on AMD Radeon and nVidia proprietary drivers since I had these easily available. AMD fails to compile (so it follows the spec) but nVidia works (so same case as Mesa). This confirms your guess: different drivers are doing different things. Is this enough to file a spec bug? I imagine that the result on glslang won't change anything, but I can try to install it and test there too if you think that's interesting anyway. Iago ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Performance regression on Tegra/GK20A since commit 363b53f00069
Hi Pekka, On 11/19/2014 04:34 PM, Pekka Paalanen wrote: On Wed, 19 Nov 2014 15:32:38 +0900 Alexandre Courbot wrote: Some more information: CPU usage of the EGL app (glmark2 here) is much higher when this patch is applied, which I presume is what triggers the frame skips. On 11/19/2014 03:05 PM, Alexandre Courbot wrote: Hi guys, I am seeing a severe performance regression (lots frame drops when running EGL apps in Weston) on Tegra/GK20A since the following commit: commit 363b53f00069af718f64cf047f19ad5681a8bf6d Author: Marek Olšák Date: Sat Nov 1 14:31:09 2014 +0100 egl: remove egl_gallium from the loader Reverting said commit on top of master brings the expected performance back. I am not knowledgeable enough about Mesa to speculate about the reason, but could we try to investigate why this happens and how we could fix this? Hi, that sounds like you used to get egl_gallium as the EGL driver, and after that patch you get egl_dri2. These two have separate Wayland platform implementations (and probably all other platforms as well?). I think that might be a lead for investigation. EGL debug environment variable (EGL_LOG_LEVEL=debug) should confirm which EGL driver gets loaded. You can force the EGL driver with e.g. EGL_DRIVER=egl_dri2. You are spot on, EGL_LOG_LEVEL revealed that I was using the egl_gallium driver while this patch makes Wayland applications use egl_dri2. If I set EGL_DRIVER=egl_gallium things go back to the expected behavior. Note, that there are two different EGL platforms in play: DRM/GBM for Weston, and Wayland for the app. Have you confirmed the problem is in the app side? That is, does Weston itself run smoothly? Weston seems to be fine, and since setting EGL_DRIVER=egl_gallium after starting it brings things back to the previous behavior I believe we can consider it is not part of this problem. FYI, I've been slowly working on https://github.com/ppaalanen/wesgr but the Weston patches are not upstream yet, and I'm in the process of updating them currently. I hope this tool might give some indication whether the skips/stalls happen in Weston or not. You say "frame drops", how do you see that? Only on screen, or also in the app performance profile? How's the average framerate for the app? Looking at it again this is actually quite interesting. The misbehaving app is glmark2, and what happens is that despite working nicely otherwise, a given frame sometimes remain displayed for up to half a second. Now looking at the framerates reported by glmark2, I noticed that while they are capped at 60fps when using the gallium driver, the numbers are much higher when using dri2 (which is nice!). Excepted when these "stuck frames" happen, then the reported framerate drops dramatically, indicating that the app itself is also blocked by this. Interestingly, if I run weston-simple-egl with dri2, the framerate is again capped at 60fps, so this may be something specific to glmark2. Also, and I cannot explain why, but if there is other activity happening in Weston (e.g. another egl application, even another instance of glmark2 itself), the issue seems to not manifest itself. Just for my education, is the egl_gallium driver going to be removed? What are egl_gallium and egl_dri2 doing differently? Thanks, Alex. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 01/29] mesa: Add an implementation of a master convert function.
On Wed, 2014-11-19 at 11:28 -0800, Jason Ekstrand wrote:
> By and large, this looks good to me. Most of my comments are cosmetic
> or suggestions for added documentation. There is one issue that I
> think is subtly wrong with integer format conversion but that should
> be easy to fix.
>
> --Jason
>
> On Tue, Nov 18, 2014 at 1:23 AM, Iago Toral Quiroga
> wrote:
> From: Jason Ekstrand
>
> v2 by Iago Toral :
>
> - When testing if we can directly pack we should use the src
> format to check
> if we are packing from an RGBA format. The original code
> used the dst format
> for the ubyte case by mistake.
> - Fixed incorrect number of bits for dst, it was computed
> using the src format
> instead of the dst format.
> - If the dst format is an array format, check if it is signed.
> We were only
> checking this for the case where it was not an array format,
> but we need
> to know this in both scenarios.
> - Fixed incorrect swizzle transform for the cases where we
> convert between
> array formats.
> - Compute is_signed and bits only once and for the dst format.
> We were
> computing these for the src format too but they were
> overwritten by the
> dst values immediately after.
> - Be more careful when selecting the integer path.
> Specifically, check that
> both src and dst are integer types. Checking only one of
> them should suffice
> since OpenGL does not allow conversions between normalized
> and integer types,
> but putting extra care here makes sense and also makes the
> actual requirements
> for this path more clear.
> - The format argument for pack functions is the destination
> format we are
> packing to, not the source format (which has to be RGBA).
> - Expose RGBA_* to other files. These will come in handy
> when in need to
> test if a given array format is RGBA or in need to pass RGBA
> formats to
> mesa_format_convert.
>
> v3 by Samuel Iglesias :
>
> - Add an RGBA_INT definition.
> ---
> src/mesa/main/format_utils.c | 378
> +++
> src/mesa/main/format_utils.h | 10 ++
> src/mesa/main/formats.h | 15 +-
> 3 files changed, 399 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/main/format_utils.c
> b/src/mesa/main/format_utils.c
> index fcbbba4..c3815cb 100644
> --- a/src/mesa/main/format_utils.c
> +++ b/src/mesa/main/format_utils.c
> @@ -25,6 +25,384 @@
> #include "format_utils.h"
> #include "glformats.h"
> #include "macros.h"
> +#include "format_pack.h"
> +#include "format_unpack.h"
> +
> +mesa_array_format RGBA_FLOAT = {{
> + MESA_ARRAY_FORMAT_TYPE_FLOAT,
> + 0,
> + 4,
> + 0, 1, 2, 3,
> + 0, 1
> +}};
> +
> +mesa_array_format RGBA_UBYTE = {{
> + MESA_ARRAY_FORMAT_TYPE_UBYTE,
> + 1,
> + 4,
> + 0, 1, 2, 3,
> + 0, 1
> +}};
> +
> +mesa_array_format RGBA_UINT = {{
> + MESA_ARRAY_FORMAT_TYPE_UINT,
> + 0,
> + 4,
> + 0, 1, 2, 3,
> + 0, 1
> +}};
> +
> +mesa_array_format RGBA_INT = {{
> + MESA_ARRAY_FORMAT_TYPE_INT,
> + 0,
> + 4,
> + 0, 1, 2, 3,
> + 0, 1
> +}};
> +
> +static void
> +invert_swizzle(uint8_t dst[4], const uint8_t src[4])
> +{
> + int i, j;
> +
> + dst[0] = MESA_FORMAT_SWIZZLE_NONE;
> + dst[1] = MESA_FORMAT_SWIZZLE_NONE;
> + dst[2] = MESA_FORMAT_SWIZZLE_NONE;
> + dst[3] = MESA_FORMAT_SWIZZLE_NONE;
> +
> + for (i = 0; i < 4; ++i)
> + for (j = 0; j < 4; ++j)
> + if (src[j] == i && dst[i] ==
> MESA_FORMAT_SWIZZLE_NONE)
> +dst[i] = j;
> +}
> +
> +static GLenum
> +gl_type_for_array_format_datatype(enum
> mesa_array_format_datatype type)
> +{
> + switch (type) {
> + case MESA_ARRAY_FORMAT_TYPE_UBYTE:
> + return GL_UNSIGNED_BYTE;
> + case MESA_ARRAY_FORMAT_TYPE_USHORT:
> + return GL_UNSIGNED_SHORT;
> + case MESA_ARRAY_FORMAT_TYPE_UINT:
> + return GL_UNSIGNED_INT;
> + case MESA_ARRAY_FORMAT_TYPE_BYTE:
> + return GL_BYTE;
> + case MESA_ARRAY_FORMAT_TYPE_SHORT:
> +
Re: [Mesa-dev] [PATCH] i965: Don't call _mesa_load_state_parameters when nr_params == 0.
On Friday, November 14, 2014 10:49:04 AM Matt Turner wrote:
> On Thu, Nov 13, 2014 at 11:22 PM, Kenneth Graunke
> wrote:
> > Saves a tiny bit of CPU overhead.
> >
> > Signed-off-by: Kenneth Graunke
> > ---
> > src/mesa/drivers/dri/i965/brw_vs_surface_state.c | 10 +-
> > src/mesa/drivers/dri/i965/gen6_vs_state.c| 12 ++--
> > 2 files changed, 11 insertions(+), 11 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> > b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> > index 1cc96cf..4e18c7d 100644
> > --- a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> > +++ b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> > @@ -59,11 +59,6 @@ brw_upload_pull_constants(struct brw_context *brw,
> > int i;
> > uint32_t surf_index = prog_data->binding_table.pull_constants_start;
> >
> > - /* Updates the ParamaterValues[i] pointers for all parameters of the
> > -* basic type of PROGRAM_STATE_VAR.
> > -*/
> > - _mesa_load_state_parameters(&brw->ctx, prog->Parameters);
> > -
> > if (!prog_data->nr_pull_params) {
>
> This check is different from the one below and what the commit summary says.
>
> Assuming that's what you expected
>
> Reviewed-by: Matt Turner
Right. The point is to skip calling _mesa_load_state_parameters if you know
it won't do anything useful. If there are zero push parameters, the push code
doesn't need this to happen. Ditto for pull.
The fact that both pieces of code independently call it should make it okay
to check the # of push parameters and # of pull parameters separately - each
section of code is responsible for making it happen before it matters.
It's a bit stupid that we call it from both places. Eric apparently wrote a
branch to fix that, but never got it working:
http://cgit.freedesktop.org/~anholt/mesa/log/?h=core-loadstateparameters
In the meantime, I've renamed the patch to:
i965: Skip _mesa_load_state_parameters when there are zero parameters.
which still isn't great, but is better. I've also pushed it.
Thanks for the review!
signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 05/29] mesa: Consider internal base format in _mesa_format_convert
On Thu, 2014-11-20 at 08:42 +0100, Iago Toral wrote:
> On Wed, 2014-11-19 at 11:43 -0800, Jason Ekstrand wrote:
> > A couple of specific comments are below. More generally, why are you
> > only considering the base format on two cases? Do we never use it for
> > anything else?
>
> I thought about that too but when I looked at the original code it
> seemed that it only cared for the base format in these two scenarios, so
> I thought that maybe the conversions cases that could be affected are
> all handled in those two paths. I'll check again though, just in case I
> missed something.
I don't know how I came to that conclusion but it seems wrong after
looking at the original code in texstore.c, which considers the base
internal format in the integer, float and ubyte paths too, so we should
do the same in _mesa_format_convert. I'll fix that.
Iago
> > On Tue, Nov 18, 2014 at 1:23 AM, Iago Toral Quiroga
> > wrote:
> > Add a dst_internal_format parameter to _mesa_format_convert,
> > that represents
> > the base internal format for texture/pixel uploads, so we can
> > do the right
> > thing when the driver has selected a different internal format
> > for the target
> > dst format.
> > ---
> > src/mesa/main/format_utils.c | 65
> > +++-
> > src/mesa/main/format_utils.h | 2 +-
> > 2 files changed, 65 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/mesa/main/format_utils.c
> > b/src/mesa/main/format_utils.c
> > index fc59e86..5964689 100644
> > --- a/src/mesa/main/format_utils.c
> > +++ b/src/mesa/main/format_utils.c
> > @@ -303,7 +303,7 @@ _mesa_compute_component_mapping(GLenum
> > inFormat, GLenum outFormat, GLubyte *map)
> > void
> > _mesa_format_convert(void *void_dst, uint32_t dst_format,
> > size_t dst_stride,
> > void *void_src, uint32_t src_format,
> > size_t src_stride,
> > - size_t width, size_t height)
> > + size_t width, size_t height, GLenum
> > dst_internal_format)
> > {
> > uint8_t *dst = (uint8_t *)void_dst;
> > uint8_t *src = (uint8_t *)void_src;
> > @@ -422,6 +422,36 @@ _mesa_format_convert(void *void_dst,
> > uint32_t dst_format, size_t dst_stride,
> > if (src_array_format.as_uint && dst_array_format.as_uint)
> > {
> >assert(src_array_format.normalized ==
> > dst_array_format.normalized);
> >
> > + /* If the base format of our dst is not the same as the
> > provided base
> > + * format it means that we are converting to a
> > different format
> > + * than the one originally requested by the client.
> > This can happen when
> > + * the internal base format requested is not supported
> > by the driver.
> > + * In this case we need to consider the requested
> > internal base format to
> > + * compute the correct swizzle operation from src to
> > dst. We will do this
> > + * by computing the swizzle transform
> > src->rgba->base->rgba->dst instead
> > + * of src->rgba->dst.
> > + */
> > + mesa_format dst_mesa_format;
> > + if (dst_format & MESA_ARRAY_FORMAT_BIT)
> > + dst_mesa_format =
> > _mesa_format_from_array_format(dst_format);
> > + else
> > + dst_mesa_format = dst_format;
> >
> >
> > Let's put an extra line here so it doesn't get confused with the below
> > if statement
> >
> >
> > + if (dst_internal_format !=
> > _mesa_get_format_base_format(dst_mesa_format)) {
> > + /* Compute src2rgba as src->rgba->base->rgba */
> > + uint8_t rgba2base[4], base2rgba[4], swizzle[4];
> > + _mesa_compute_component_mapping(GL_RGBA,
> > dst_internal_format, rgba2base);
> > + _mesa_compute_component_mapping(dst_internal_format,
> > GL_RGBA, base2rgba);
> > +
> > + for (i = 0; i < 4; i++) {
> > +if (base2rgba[i] > MESA_FORMAT_SWIZZLE_W)
> > + swizzle[i] = base2rgba[i];
> > +else
> > + swizzle[i] =
> > src2rgba[rgba2base[base2rgba[i]]];
> >
> >
> > This doesn't work for composing three swizzles. If you get a ZERO or
> > ONE in rgba2base, you'll read the wrong memory from src2rgba.
>
> Actually, the problem is worse, because the mapping written by
> _mesa_compute_component_mapping is a 6-component mapping and we are
> passing a 4-component array. I'll fix this.
>
> >
> >
> > + }
>
Re: [Mesa-dev] [PATCH 14/20] mesa: Add non-normalized formats support for ubyte packing functions
On Thu, 2014-11-20 at 09:55 +0100, Iago Toral wrote:
> On Thu, 2014-11-20 at 08:15 +0100, Samuel Iglesias Gonsálvez wrote:
> > On Tue, 2014-11-18 at 11:08 -0800, Jason Ekstrand wrote:
> > >
> > >
> > > On Tue, Nov 18, 2014 at 12:44 AM, Iago Toral Quiroga
> > > wrote:
> > > From: Samuel Iglesias Gonsalvez
> > >
> > > Signed-off-by: Samuel Iglesias Gonsalvez
> > >
> > > ---
> > > src/mesa/main/format_pack.c.mako | 10 +-
> > > 1 file changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/mesa/main/format_pack.c.mako
> > > b/src/mesa/main/format_pack.c.mako
> > > index b9f4656..97adf6e 100644
> > > --- a/src/mesa/main/format_pack.c.mako
> > > +++ b/src/mesa/main/format_pack.c.mako
> > > @@ -84,7 +84,15 @@ pack_ubyte_${f.short_name()}(const GLubyte
> > > src[4], void *dst)
> > >%endif
> > >
> > >${channel_datatype(c)} ${c.name} =
> > > - %if c.type == parser.UNSIGNED:
> > > + %if not f.is_normalized():
> > > + %if c.type == parser.FLOAT and c.size == 32:
> > > +UBYTE_TO_FLOAT(src[${i}]);
> > > + %elif c.type == parser.FLOAT and c.size == 16:
> > > +_mesa_float_to_half(UBYTE_TO_FLOAT(src[${i}]));
> > >
> > >
> > > Same question here as in the previous patch. Why are we using
> > > UBYTE_TO_FLOAT?
> > >
> >
> > This is what current format_pack.c is doing for those formats and some
> > piglit tests complain if it is not there.
> >
>
> Jason, this looks correct to me: we are packing from an ubyte type to a
> half float type, so first we need to convert from ubyte to float and
> then downgrade the float to a half float, we don't currently have means
> to convert directly from an ubyte to a half float, right?
>
> Iago
>
Notice that we are converting from ubyte type source to float formats,
hence the use of UBYTE_TO_FLOAT().
I'm going to split the conditions in two (one for converting to non
integer formats, other for non normalized integer formats) to make it
clearer.
Sam
> > >
> > > + %else:
> > > +(${channel_datatype(c)}) src[${i}];
> > > + %endif
> > > + %elif c.type == parser.UNSIGNED:
> > > %if f.colorspace == 'srgb' and c.name in 'rgb':
> > > util_format_linear_to_srgb_8unorm(src[${i}]);
> > > %else:
> > > --
> > > 1.9.1
> > >
> > > ___
> > > mesa-dev mailing list
> > > mesa-dev@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > >
> > >
> > > ___
> > > mesa-dev mailing list
> > > mesa-dev@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> >
>
>
>
signature.asc
Description: This is a digitally signed message part
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Performance regression on Tegra/GK20A since commit 363b53f00069
On Thu, 20 Nov 2014 18:24:34 +0900 Alexandre Courbot wrote: > Hi Pekka, > > On 11/19/2014 04:34 PM, Pekka Paalanen wrote: > > On Wed, 19 Nov 2014 15:32:38 +0900 > > Alexandre Courbot wrote: > > > >> Some more information: CPU usage of the EGL app (glmark2 here) is much > >> higher when this patch is applied, which I presume is what triggers the > >> frame skips. > >> > >> On 11/19/2014 03:05 PM, Alexandre Courbot wrote: > >>> Hi guys, > >>> > >>> I am seeing a severe performance regression (lots frame drops when > >>> running EGL apps in Weston) on Tegra/GK20A since the following commit: > >>> > >>> commit 363b53f00069af718f64cf047f19ad5681a8bf6d > >>> Author: Marek Olšák > >>> Date: Sat Nov 1 14:31:09 2014 +0100 > >>> > >>> egl: remove egl_gallium from the loader > >>> > >>> Reverting said commit on top of master brings the expected performance > >>> back. I am not knowledgeable enough about Mesa to speculate about the > >>> reason, but could we try to investigate why this happens and how we > >>> could fix this? > > > > Hi, > > > > that sounds like you used to get egl_gallium as the EGL driver, and > > after that patch you get egl_dri2. These two have separate Wayland > > platform implementations (and probably all other platforms as well?). I > > think that might be a lead for investigation. EGL debug environment > > variable (EGL_LOG_LEVEL=debug) should confirm which EGL driver gets > > loaded. You can force the EGL driver with e.g. EGL_DRIVER=egl_dri2. > > You are spot on, EGL_LOG_LEVEL revealed that I was using the egl_gallium > driver while this patch makes Wayland applications use egl_dri2. If I > set EGL_DRIVER=egl_gallium things go back to the expected behavior. > > > > > Note, that there are two different EGL platforms in play: DRM/GBM for > > Weston, and Wayland for the app. Have you confirmed the problem is in > > the app side? That is, does Weston itself run smoothly? > > Weston seems to be fine, and since setting EGL_DRIVER=egl_gallium after > starting it brings things back to the previous behavior I believe we can > consider it is not part of this problem. Agreed. > > You say "frame drops", how do you see that? Only on screen, or also in > > the app performance profile? How's the average framerate for the app? > > Looking at it again this is actually quite interesting. The misbehaving > app is glmark2, and what happens is that despite working nicely > otherwise, a given frame sometimes remain displayed for up to half a > second. Now looking at the framerates reported by glmark2, I noticed > that while they are capped at 60fps when using the gallium driver, the > numbers are much higher when using dri2 (which is nice!). Excepted when > these "stuck frames" happen, then the reported framerate drops > dramatically, indicating that the app itself is also blocked by this. I have a hunch (wl_buffer.release not delivered in time, and client side EGL running out of available buffers), but confirming that would require a Wayland protocol dump up to such hickup. You could try to get one by setting the enviroment variable WAYLAND_DEBUG=client for glmark2. It will be a flood, especially if glmark2 succeeds in running at uncapped framerates. The trace will come to stderr, so you want to redirect that to file. You need to find out where in the trace the hickup happened. The timestamps are in milliseconds. I could then take a look (will need the whole trace). At this point, I think it would be best to open a bug report against Mesa, and continue there. Such freezes obviously should not happen on either EGL driver. Please add me to CC on the bug. > Interestingly, if I run weston-simple-egl with dri2, the framerate is > again capped at 60fps, so this may be something specific to glmark2. weston-simple-egl does not even try to exceed the monitor framerate. I'd expect glmark2 OTOH to set eglSwapInterval to 0 (unlimited), which means it should be limited only by Wayland roundtrips, when it replaces wl_surface's buffer and eventually needs to wait for one of the buffers to come back for re-use, so that it can continue rendering. > Also, and I cannot explain why, but if there is other activity happening > in Weston (e.g. another egl application, even another instance of > glmark2 itself), the issue seems to not manifest itself. That's probably even more proof that my hunch might be right. But it would also mean the bug is in Weston rather than Mesa. Or in the combination of the two, plus a race. We can easily move the bug report from Mesa to Weston if that's true, anyway. > Just for my education, is the egl_gallium driver going to be removed? That was recently discussed in the thread http://lists.freedesktop.org/archives/mesa-dev/2014-November/thread.html I'm not sure. > What are egl_gallium and egl_dri2 doing differently? I don't know exactly, but egl_gallium is a Gallium3D state tracker and can support only Gallium3D drivers, while egl_dri2 can load all kinds of Mesa
Re: [Mesa-dev] [PATCH 14/20] mesa: Add non-normalized formats support for ubyte packing functions
On Thu, 2014-11-20 at 12:14 +0100, Samuel Iglesias Gonsálvez wrote:
> On Thu, 2014-11-20 at 09:55 +0100, Iago Toral wrote:
> > On Thu, 2014-11-20 at 08:15 +0100, Samuel Iglesias Gonsálvez wrote:
> > > On Tue, 2014-11-18 at 11:08 -0800, Jason Ekstrand wrote:
> > > >
> > > >
> > > > On Tue, Nov 18, 2014 at 12:44 AM, Iago Toral Quiroga
> > > > wrote:
> > > > From: Samuel Iglesias Gonsalvez
> > > >
> > > > Signed-off-by: Samuel Iglesias Gonsalvez
> > > >
> > > > ---
> > > > src/mesa/main/format_pack.c.mako | 10 +-
> > > > 1 file changed, 9 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/src/mesa/main/format_pack.c.mako
> > > > b/src/mesa/main/format_pack.c.mako
> > > > index b9f4656..97adf6e 100644
> > > > --- a/src/mesa/main/format_pack.c.mako
> > > > +++ b/src/mesa/main/format_pack.c.mako
> > > > @@ -84,7 +84,15 @@ pack_ubyte_${f.short_name()}(const GLubyte
> > > > src[4], void *dst)
> > > >%endif
> > > >
> > > >${channel_datatype(c)} ${c.name} =
> > > > - %if c.type == parser.UNSIGNED:
> > > > + %if not f.is_normalized():
> > > > + %if c.type == parser.FLOAT and c.size == 32:
> > > > +UBYTE_TO_FLOAT(src[${i}]);
> > > > + %elif c.type == parser.FLOAT and c.size == 16:
> > > > +_mesa_float_to_half(UBYTE_TO_FLOAT(src[${i}]));
> > > >
> > > >
> > > > Same question here as in the previous patch. Why are we using
> > > > UBYTE_TO_FLOAT?
> > > >
> > >
> > > This is what current format_pack.c is doing for those formats and some
> > > piglit tests complain if it is not there.
> > >
> >
> > Jason, this looks correct to me: we are packing from an ubyte type to a
> > half float type, so first we need to convert from ubyte to float and
> > then downgrade the float to a half float, we don't currently have means
> > to convert directly from an ubyte to a half float, right?
> >
> > Iago
> >
>
> Notice that we are converting from ubyte type source to float formats,
> hence the use of UBYTE_TO_FLOAT().
>
> I'm going to split the conditions in two (one for converting to non
> integer formats, other for non normalized integer formats) to make it
> clearer.
>
> Sam
>
OK, forget all what we said here. I see your point: we are adding a
float conversion (using UBYTE_TO_FLOAT macro) when it is already covered
later.
I'm going to rewrite this patch. Sorry for the noise.
Sam
> > > >
> > > > + %else:
> > > > +(${channel_datatype(c)}) src[${i}];
> > > > + %endif
> > > > + %elif c.type == parser.UNSIGNED:
> > > > %if f.colorspace == 'srgb' and c.name in 'rgb':
> > > > util_format_linear_to_srgb_8unorm(src[${i}]);
> > > > %else:
> > > > --
> > > > 1.9.1
> > > >
> > > > ___
> > > > mesa-dev mailing list
> > > > mesa-dev@lists.freedesktop.org
> > > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > > >
> > > >
> > > > ___
> > > > mesa-dev mailing list
> > > > mesa-dev@lists.freedesktop.org
> > > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > >
> > >
> >
> >
> >
>
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: This is a digitally signed message part
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 16/20] mesa/formats: add new mesa formats and their pack/unpack functions.
On Tue, 2014-11-18 at 11:50 -0800, Jason Ekstrand wrote:
>
>
> On Tue, Nov 18, 2014 at 12:44 AM, Iago Toral Quiroga
> wrote:
> From: Samuel Iglesias Gonsalvez
>
> This will be used to refactor code in pack.c and support
> conversion
> to/from these types in a master convert function that will be
> added
> later.
>
> Signed-off-by: Samuel Iglesias Gonsalvez
>
> ---
> src/mesa/main/format_pack.c.mako | 36 -
> src/mesa/main/format_unpack.c.mako | 35 -
> src/mesa/main/formats.c| 104
> +
> src/mesa/main/formats.csv | 13 +
> src/mesa/main/formats.h| 15 ++
> src/mesa/swrast/s_texfetch.c | 13 +
> 6 files changed, 212 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/main/format_pack.c.mako
> b/src/mesa/main/format_pack.c.mako
> index b846702..c5ad623 100644
> --- a/src/mesa/main/format_pack.c.mako
> +++ b/src/mesa/main/format_pack.c.mako
> @@ -68,7 +68,7 @@ for f in formats:
> /* ubyte packing functions */
>
> %for f in rgb_formats:
> - %if f.name in ('MESA_FORMAT_R9G9B9E5_FLOAT',
> 'MESA_FORMAT_R11G11B10_FLOAT'):
> + %if f.name in ('MESA_FORMAT_R9G9B9E5_FLOAT',
> 'MESA_FORMAT_R11G11B10_FLOAT',
> 'MESA_FORMAT_A2R10G10B10_UNORM'):
><% continue %>
> %elif f.is_compressed():
><% continue %>
> @@ -137,6 +137,22 @@ pack_ubyte_${f.short_name()}(const
> GLubyte src[4], void *dst)
> %endfor
>
> static inline void
> +pack_ubyte_a2r10g10b10_unorm(const GLubyte src[4], void *dst)
> +{
> +uint8_t a = _mesa_unorm_to_unorm(src[3], 8, 2);
> +uint16_t r = _mesa_unorm_to_unorm(src[0], 8, 10);
> +uint16_t g = _mesa_unorm_to_unorm(src[1], 8, 10);
> +uint16_t b = _mesa_unorm_to_unorm(src[2], 8, 10);
> +
> +uint32_t d = 0;
> +d |= PACK(a, 0, 2);
> +d |= PACK(r, 2, 10);
> +d |= PACK(g, 12, 10);
> +d |= PACK(b, 22, 10);
> +(*(uint32_t *) dst) = d;
> +}
>
>
> The autogen code should be able to handle this. There's no reason for
> special-casing it. The only reason those other two are a special case
> is that they involve strange floating-bit computations.
>
OK, I fixed it.
>
> +
> +static inline void
> pack_ubyte_r9g9b9e5_float(const GLubyte src[4], void *dst)
> {
> GLuint *d = (GLuint *) dst;
> @@ -311,7 +327,7 @@ pack_int_r11g11b10_float(const GLint
> src[4], void *dst)
> /* float packing functions */
>
> %for f in rgb_formats:
> - %if f.name in ('MESA_FORMAT_R9G9B9E5_FLOAT',
> 'MESA_FORMAT_R11G11B10_FLOAT'):
> + %if f.name in ('MESA_FORMAT_R9G9B9E5_FLOAT',
> 'MESA_FORMAT_R11G11B10_FLOAT',
> 'MESA_FORMAT_A2R10G10B10_UNORM'):
><% continue %>
> %elif f.is_compressed():
><% continue %>
> @@ -373,6 +389,22 @@ pack_float_${f.short_name()}(const
> GLfloat src[4], void *dst)
> %endfor
>
> static inline void
> +pack_float_a2r10g10b10_unorm(const GLfloat src[4], void *dst)
> +{
> +uint8_t a = _mesa_float_to_unorm(src[3], 2);
> +uint16_t r = _mesa_float_to_unorm(src[0], 10);
> +uint16_t g = _mesa_float_to_unorm(src[1], 10);
> +uint16_t b = _mesa_float_to_unorm(src[2], 10);
> +
> +uint32_t d = 0;
> +d |= PACK(a, 0, 2);
> +d |= PACK(r, 2, 10);
> +d |= PACK(g, 12, 10);
> +d |= PACK(b, 22, 10);
> +(*(uint32_t *) dst) = d;
> +}
> +
> +static inline void
> pack_float_r9g9b9e5_float(const GLfloat src[4], void *dst)
> {
> GLuint *d = (GLuint *) dst;
> diff --git a/src/mesa/main/format_unpack.c.mako
> b/src/mesa/main/format_unpack.c.mako
> index 8fd3cdd..510aed2 100644
> --- a/src/mesa/main/format_unpack.c.mako
> +++ b/src/mesa/main/format_unpack.c.mako
> @@ -67,7 +67,7 @@ for f in formats:
> /* float unpacking functions */
>
> %for f in rgb_formats:
> - %if f.name in ('MESA_FORMAT_R9G9B9E5_FLOAT',
> 'MESA_FORMAT_R11G11B10_FLOAT'):
> + %if f.name in ('MESA_FORMAT_R9G9B9E5_FLOAT',
> 'MESA_FORMAT_R11G11B10_FLOAT',
> 'MESA_FORMAT_A2R10G10B10_UNORM'):
><% continue %>
> %elif f.is_compressed():
><% continue %>
> @@ -128,6 +12
Re: [Mesa-dev] Using the 'f' suffix to create a float from an integer literal
For what it's worth, I did a quick grep through the internal and public shader-db and I couldn't find anything using this. git grep -P '\b(? writes: > On Thu, 2014-11-20 at 08:08 +0100, Iago Toral wrote: >> On Wed, 2014-11-19 at 10:27 -0800, Ian Romanick wrote: >> > On 11/19/2014 03:47 AM, Iago Toral Quiroga wrote: >> > > Hi, >> > > >> > > I came across a GLSL test that checks that doing something like this in >> > > a shader should fail: >> > >> > Is this one of the dEQP tests? >> >> Yes. >> >> > > float value = 1f; >> > >> > It seems like we have a test related to this in piglit somewhere... it >> > looks like tests/shaders/glsl-floating-constant-120.shader_test uses >> > that syntax, but it's not explicitly testing that feature. >> > >> > > However, this works fine in Mesa. Checking the spec I see: >> > > >> > > Floating-point constants are defined as follows. >> > > floating-constant: >> > >fractional-constant exponent-part(opt) floating-suffix(opt) >> > >digit-sequence exponent-part floating-suffix(opt) >> > > fractional-constant: >> > >digit-sequence . digit-sequence >> > >digit-sequence . >> > >. digit-sequence >> > > exponent-part: >> > >e sign(opt) digit-sequence >> > >E sign(opt) digit-sequence >> > > sign: one of >> > >+ - >> > > digit-sequence: >> > >digit >> > >digit-sequence digit >> > > floating-suffix: one of >> > >f F >> > > >> > > which suggests that the test is correct and Mesa has a bug. According to >> > > the above rules, however, something like this is fine: >> > > >> > > float f = 1e2f; >> > > >> > > which I find kind of weird if the other case is not valid, so I wonder >> > > if there is a bug in the spec or this is on purpose for some reason that >> > > I am missing. >> > > >> > > Then, to make matters worse, I read this in opengl.org wiki [1]: >> > > >> > > "A numeric literal that uses a decimal is by default of type float. To >> > > create a float literal from an integer value, use the suffix f or F as >> > > in C/C++." >> > > >> > > which contradicts the spec and the test and is aligned with the current >> > > way Mesa works. >> > > >> > > So: does anyone know what version is right? Could this be a bug in the >> > > spec? and if it is not, do we want to change the behavior to follow the >> > > spec as it is now? >> > >> > The $64,000 question: What do other GLSL compilers (including, perhaps, >> > glslang) do? This seems like one of the cases where nobody is likely to >> > follow the spec, and some application will depend on that. If that's >> > the case, I'll submit a spec bug. >> >> Good point. I'll try to check a few cases and reply here. Thanks! > > I did a quick test on AMD Radeon and nVidia proprietary drivers since I > had these easily available. AMD fails to compile (so it follows the > spec) but nVidia works (so same case as Mesa). > > This confirms your guess: different drivers are doing different things. > Is this enough to file a spec bug? I imagine that the result on glslang > won't change anything, but I can try to install it and test there too if > you think that's interesting anyway. > > Iago > > ___ > 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] rtasm,translate: Re-enable SSE on Mingw64.
On 19/11/14 17:21, Jon TURNEY wrote: On 19/11/2014 15:25, Jose Fonseca wrote: No idea. But the impression I generally have is MinGW has come a long way since then (3 years ago.) I think there was at least one bug in mesa which prevented this from working, see commit cedfd79b Yes, you're probably right. That should have affected MSVC 64bits too though. We might have blamed MinGW unjustly. Though I find it surprising that if MSVC 64bit was broken around that time, we'd have found it. Anyway, all seems good now. Jose ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Removing unused opcodes (TGSI, Mesa IR)
On 19/11/14 21:17, Ilia Mirkin wrote: On Wed, Nov 19, 2014 at 3:45 PM, Jose Fonseca wrote: On 19/11/14 19:45, Ilia Mirkin wrote: On Wed, Nov 19, 2014 at 2:32 PM, Eric Anholt wrote: Eric Anholt writes: This series removes a bunch of unused opcodes, mostly from TGSI. It doesn't go as far as we could possibly go -- while I welcome discussion for future patch series deleting more, I hope that discussion doesn't derail the review process for these changes. I haven't messed with the subroutine stuff, since I don't know what people are planning with that. I also haven't messed with the pack/unpack opcodes in TGSI, since they might be useful for some of the GLSL packing stuff. Testing status: compile-tested ilo/r600/softpipe, touch-tested softpipe. Lots of "looks good", no Reviewed-by (other than Ian on the Mesa side). I'm probably going to push soon anyway if somebody doesn't actually give Reviewed-by or NAK. st/nine got merged. It uses ARR. And at the very least references TGSI_OPCODE_NRM even if it doesn't use it. It also has code that uses CND and DP2A although it's presently disabled via #define's. The patch attached should do it. I'm pretty sure that your series will cause st/nine to fail compiling. BTW, given it's not trivial to compile nine state-tracker, I think it might be better to not obfuscate which opcodes or symbols it uses with macros. It pays off to be upfront with these things, so that `git grep foo` finds everything that should be found. Hmmm... why is it not easy to build st/nine? I haven't looked at that in detail yet, but whatever's hard about it should get fixed. I somehow assumed it dependend on wine headers (*), but I was wrong -- I just tried and one can build with alone. It's actually straightforward. So apologies for mis-information. Jose (*) which is a problem for me as I can't even install wine on my Ubuntu devel machine as wine indirectly conflicts with nvidia drivers, as it depends on an opencl package that conflicts with nvidia opencl package) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Require micro-benchmarks for performance optimization oriented patches
Hi, > Honestly, I think I'm okay with our usual metrics like: > - Increased FPS in a game or benchmark > - Reduced number of instructions or memory accesses in a shader program > - Reduced memory consumption > - Significant cycle reduction in callgrind or better generated code > (ideally if it's a lot of code I'd like better justification) Profiling tools like callgrind are means for analyzing, not for measuring. The problem with profiler data is that the cost may have just been moved elsewhere, *and* grown: * Kcachegrind visualization for valgrind/callgrind data shows call counts and relative performance. If relative cost of a given function has decreased, that still doesn't tell anything about: - its absolute cost, i.e. - whether cost just moved somewhere else instead of total cost really decreasing * Callgrind reports instruction counts, not cycles. While they're a good indicator, it doesn't necessarily tell about real performance (instruction count e.g. doesn't take into account data or instruction cache misses) * Valgrind tracks only CPU utilization on the user-space. It doesn't notice increased CPU utilization at kernel side. * Valgrind tracks only single process, it doesn't notice increased CPU utilization in other processes (in graphics perf, X server side is sometimes relevant) * Valgrind doesn't track GPU utilization. Change may have moved more load there. -> Looking just at Callgrind data is NOT enough, there must also be some real measurement data. As to measurements... Even large performance improvements fail to show up with the wrong benchmark. One needs to know whether test-case performance is bound by what you were trying to optimize. If there's no such case known, simplest may be just to write a micro-benchmark for the change (preferably two, one for best and one for the worst case). - Eero PS. while analyzing memory usage isn't harder, measuring that is, because memory usage can be shared (both in user-space and on kernel buffers), and there's a large impact difference on whether memory is clean or dirty (unless your problem is running out of 32-bit address space when clean memory is as much of a problem). ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Using the 'f' suffix to create a float from an integer literal
On 11/20/2014 05:33 AM, Neil Roberts wrote: > For what it's worth, I did a quick grep through the internal and public > shader-db and I couldn't find anything using this. > > git grep -P '\b(? > If AMD disallows it then it seems like it would be reasonably safe to > disallow it in Mesa too. > > GCC disallows it too which could be an indication that people won't have > a habit of using it. So... the GLSL spec actually follows C? Then we should definitely follow the spec, and there's no need for a GLSL spec bug. If AMD disallows it, then there are not likely to be any applications that depend on it... so I agree with Neil that we're safe to disallow it too. I'm still curious about glslang... if glslang allows it, I'll file a bug against glslang. :) > - Neil > > Iago Toral writes: > >> On Thu, 2014-11-20 at 08:08 +0100, Iago Toral wrote: >>> On Wed, 2014-11-19 at 10:27 -0800, Ian Romanick wrote: On 11/19/2014 03:47 AM, Iago Toral Quiroga wrote: > Hi, > > I came across a GLSL test that checks that doing something like this in > a shader should fail: Is this one of the dEQP tests? >>> >>> Yes. >>> > float value = 1f; It seems like we have a test related to this in piglit somewhere... it looks like tests/shaders/glsl-floating-constant-120.shader_test uses that syntax, but it's not explicitly testing that feature. > However, this works fine in Mesa. Checking the spec I see: > > Floating-point constants are defined as follows. > floating-constant: >fractional-constant exponent-part(opt) floating-suffix(opt) >digit-sequence exponent-part floating-suffix(opt) > fractional-constant: >digit-sequence . digit-sequence >digit-sequence . >. digit-sequence > exponent-part: >e sign(opt) digit-sequence >E sign(opt) digit-sequence > sign: one of >+ - > digit-sequence: >digit >digit-sequence digit > floating-suffix: one of >f F > > which suggests that the test is correct and Mesa has a bug. According to > the above rules, however, something like this is fine: > > float f = 1e2f; > > which I find kind of weird if the other case is not valid, so I wonder > if there is a bug in the spec or this is on purpose for some reason that > I am missing. > > Then, to make matters worse, I read this in opengl.org wiki [1]: > > "A numeric literal that uses a decimal is by default of type float. To > create a float literal from an integer value, use the suffix f or F as > in C/C++." > > which contradicts the spec and the test and is aligned with the current > way Mesa works. > > So: does anyone know what version is right? Could this be a bug in the > spec? and if it is not, do we want to change the behavior to follow the > spec as it is now? The $64,000 question: What do other GLSL compilers (including, perhaps, glslang) do? This seems like one of the cases where nobody is likely to follow the spec, and some application will depend on that. If that's the case, I'll submit a spec bug. >>> >>> Good point. I'll try to check a few cases and reply here. Thanks! >> >> I did a quick test on AMD Radeon and nVidia proprietary drivers since I >> had these easily available. AMD fails to compile (so it follows the >> spec) but nVidia works (so same case as Mesa). >> >> This confirms your guess: different drivers are doing different things. >> Is this enough to file a spec bug? I imagine that the result on glslang >> won't change anything, but I can try to install it and test there too if >> you think that's interesting anyway. >> >> Iago >> >> ___ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/9] i915: Only use TEXCOORDTYPE_VECTOR with cube maps on gen2
On 08/06/2014 11:56 AM, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Check that the target is GL_TEXTURE_CUBE_MAP before emitting
> TEXCOORDTYPE_VECTOR texture coordinates.
>
> I'm not sure if the hardware would like CARTESIAN coordinates
> with cube maps, and as I'm too lazy to find out just emit the
> VECTOR coordinates for cube maps always. For other targets use
> CARTESIAN or HOMOGENOUS depending on the number of texture
> coordinates provided.
>
> Fixes rendering of the "electric" background texture in chromium-bsu
> main menu. We appear to be provided with three texture coordinates
> there (I'm guessing due to the funky texture matrix rotation it does).
> So the code would decide to use TEXCOORDTYPE_VECTOR instead of
> TEXCOORDTYPE_CARTESIAN even though we're dealing with a 2D texure.
> The results weren't what one might expect.
>
> demos/cubemap still works, which hopefully indicates that this doesn't
> break things.
I won't dare ask about a full piglit run on that hardware, but how about
bin/glean -o -v -v -v -t +texCube --quick
and
bin/cubemap -auto
from piglit?
These changes seem reasonable, and assuming those tests aren't made worse,
Reviewed-by: Ian Romanick
If you're excited about GEN2 bugs, wanna take a look at 79841? >:)
> Signed-off-by: Ville Syrjälä
> ---
> src/mesa/drivers/dri/i915/i830_vtbl.c | 37
> ++-
> 1 file changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i915/i830_vtbl.c
> b/src/mesa/drivers/dri/i915/i830_vtbl.c
> index 53d408b..0f22d86 100644
> --- a/src/mesa/drivers/dri/i915/i830_vtbl.c
> +++ b/src/mesa/drivers/dri/i915/i830_vtbl.c
> @@ -134,27 +134,28 @@ i830_render_start(struct intel_context *intel)
> GLuint mcs = (i830->state.Tex[i][I830_TEXREG_MCS] &
>~TEXCOORDTYPE_MASK);
>
> -switch (sz) {
> -case 1:
> -case 2:
> - emit = EMIT_2F;
> - sz = 2;
> - mcs |= TEXCOORDTYPE_CARTESIAN;
> - break;
> -case 3:
> +if (intel->ctx.Texture.Unit[i]._Current->Target ==
> GL_TEXTURE_CUBE_MAP) {
> emit = EMIT_3F;
> sz = 3;
> mcs |= TEXCOORDTYPE_VECTOR;
> - break;
> -case 4:
> - emit = EMIT_3F_XYW;
> - sz = 3;
> - mcs |= TEXCOORDTYPE_HOMOGENEOUS;
> - break;
> -default:
> - continue;
> -};
> -
> +} else {
> + switch (sz) {
> + case 1:
> + case 2:
> + case 3:
> + emit = EMIT_2F;
> + sz = 2;
> + mcs |= TEXCOORDTYPE_CARTESIAN;
> + break;
> + case 4:
> + emit = EMIT_3F_XYW;
> + sz = 3;
> + mcs |= TEXCOORDTYPE_HOMOGENEOUS;
> + break;
> + default:
> + continue;
> + }
> +}
>
> EMIT_ATTR(_TNL_ATTRIB_TEX0 + i, emit, 0);
> v2 |= VRTX_TEX_SET_FMT(count, SZ_TO_HW(sz));
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] i965: Rename two intelEmit*Blit functions to not use camel-case
Just clearing some old patch back log...
Patch 1 is
Reviewed-by: Ian Romanick
With Matt's nits about patch 2 fixed, it is also R-b, but I guess the
point of that patch is really to enable patch 3. Where do we stand on that?
On 10/01/2014 02:07 PM, Jason Ekstrand wrote:
> I think these are about the only remaining uses of camel-case in the i965
> driver. Let's make things more consistent.
>
> Signed-off-by: Jason Ekstrand
> ---
> src/mesa/drivers/dri/i965/intel_blit.c | 108
> -
> src/mesa/drivers/dri/i965/intel_blit.h | 51 ++--
> src/mesa/drivers/dri/i965/intel_copy_image.c | 30 +++
> src/mesa/drivers/dri/i965/intel_pixel_bitmap.c | 26 +++---
> 4 files changed, 108 insertions(+), 107 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_blit.c
> b/src/mesa/drivers/dri/i965/intel_blit.c
> index 73ab488..f9e2e30 100644
> --- a/src/mesa/drivers/dri/i965/intel_blit.c
> +++ b/src/mesa/drivers/dri/i965/intel_blit.c
> @@ -195,9 +195,9 @@ intel_miptree_blit(struct brw_context *brw,
> *represented per scan line’s worth of graphics data depends on the
> *color depth.
> *
> -* Furthermore, intelEmitCopyBlit (which is called below) uses a signed
> -* 16-bit integer to represent buffer pitch, so it can only handle buffer
> -* pitches < 32k.
> +* Furthermore, intel_emit_copy_blit (which is called below) uses a
> +* signed 16-bit integer to represent buffer pitch, so it can only
> +* handle buffer pitches < 32k.
> *
> * As a result of these two limitations, we can only use the blitter to do
> * this copy when the miptree's pitch is less than 32k.
> @@ -258,18 +258,18 @@ intel_miptree_blit(struct brw_context *brw,
>return false;
> }
>
> - if (!intelEmitCopyBlit(brw,
> - src_mt->cpp,
> - src_pitch,
> - src_mt->bo, src_mt->offset,
> - src_mt->tiling,
> - dst_mt->pitch,
> - dst_mt->bo, dst_mt->offset,
> - dst_mt->tiling,
> - src_x, src_y,
> - dst_x, dst_y,
> - width, height,
> - logicop)) {
> + if (!intel_emit_copy_blit(brw,
> + src_mt->cpp,
> + src_pitch,
> + src_mt->bo, src_mt->offset,
> + src_mt->tiling,
> + dst_mt->pitch,
> + dst_mt->bo, dst_mt->offset,
> + dst_mt->tiling,
> + src_x, src_y,
> + dst_x, dst_y,
> + width, height,
> + logicop)) {
>return false;
> }
>
> @@ -286,20 +286,20 @@ intel_miptree_blit(struct brw_context *brw,
> /* Copy BitBlt
> */
> bool
> -intelEmitCopyBlit(struct brw_context *brw,
> - GLuint cpp,
> - GLshort src_pitch,
> - drm_intel_bo *src_buffer,
> - GLuint src_offset,
> - uint32_t src_tiling,
> - GLshort dst_pitch,
> - drm_intel_bo *dst_buffer,
> - GLuint dst_offset,
> - uint32_t dst_tiling,
> - GLshort src_x, GLshort src_y,
> - GLshort dst_x, GLshort dst_y,
> - GLshort w, GLshort h,
> - GLenum logic_op)
> +intel_emit_copy_blit(struct brw_context *brw,
> + GLuint cpp,
> + GLshort src_pitch,
> + drm_intel_bo *src_buffer,
> + GLuint src_offset,
> + uint32_t src_tiling,
> + GLshort dst_pitch,
> + drm_intel_bo *dst_buffer,
> + GLuint dst_offset,
> + uint32_t dst_tiling,
> + GLshort src_x, GLshort src_y,
> + GLshort dst_x, GLshort dst_y,
> + GLshort w, GLshort h,
> + GLenum logic_op)
> {
> GLuint CMD, BR13, pass = 0;
> int dst_y2 = dst_y + h;
> @@ -435,17 +435,17 @@ intelEmitCopyBlit(struct brw_context *brw,
> }
>
> bool
> -intelEmitImmediateColorExpandBlit(struct brw_context *brw,
> - GLuint cpp,
> - GLubyte *src_bits, GLuint src_size,
> - GLuint fg_color,
> - GLshort dst_pitch,
> - drm_intel_bo *dst_buffer,
> - GLuint dst_offset,
> - uint32_t dst_tiling,
> - GLshort x, GLshort y,
> - GLshort w, GLshort h,
> - G
[Mesa-dev] [PATCH] egl: Expose EGL_KHR_get_all_proc_addresses and its client extension
Mesa already implements the behavior of EGL_KHR_get_all_proc_addresses and EGL_KHR_client_get_all_proc_addresses. This patch just exposes the extension strings. See: https://www.khronos.org/registry/egl/extensions/KHR/EGL_KHR_get_all_proc_addresses.txt Cc: Daniel Kurtz Cc: Frank Henigman Signed-off-by: Chad Versace --- You can find this on my branch 'EGL_KHR_get_all_proc_addresses'. http://github.com/chadversary/mesa/tree/EGL_KHR_get_all_proc_addresses src/egl/main/eglapi.c | 17 + src/egl/main/egldisplay.h | 1 + src/egl/main/eglglobals.c | 4 +++- src/egl/main/eglglobals.h | 1 + src/egl/main/eglmisc.c| 1 + 5 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c index 096c3d8..a8d1e4b 100644 --- a/src/egl/main/eglapi.c +++ b/src/egl/main/eglapi.c @@ -357,6 +357,23 @@ eglInitialize(EGLDisplay dpy, EGLint *major, EGLint *minor) /* limit to APIs supported by core */ disp->ClientAPIs &= _EGL_API_ALL_BITS; + + /* EGL_KHR_get_all_proc_addresses is a corner-case extension. The spec + * classifies it as an EGL display extension, though conceptually it's an + * EGL client extension. + * + * From the EGL_KHR_get_all_proc_addresses spec: + * + *The EGL implementation must expose the name + *EGL_KHR_client_get_all_proc_addresses if and only if it exposes + *EGL_KHR_get_all_proc_addresses and supports + *EGL_EXT_client_extensions. + * + * Mesa unconditionally exposes both client extensions mentioned above, + * so the spec requires that each EGLDisplay unconditionally exposes + * EGL_KHR_get_all_proc_addresses. + */ + disp->Extensions.KHR_get_all_proc_addresses = EGL_TRUE; } /* Update applications version of major and minor if not NULL */ diff --git a/src/egl/main/egldisplay.h b/src/egl/main/egldisplay.h index 13b9532..d4b9602 100644 --- a/src/egl/main/egldisplay.h +++ b/src/egl/main/egldisplay.h @@ -97,6 +97,7 @@ struct _egl_extensions EGLBoolean KHR_image_base; EGLBoolean KHR_image_pixmap; EGLBoolean KHR_vg_parent_image; + EGLBoolean KHR_get_all_proc_addresses; EGLBoolean KHR_gl_texture_2D_image; EGLBoolean KHR_gl_texture_cubemap_image; EGLBoolean KHR_gl_texture_3D_image; diff --git a/src/egl/main/eglglobals.c b/src/egl/main/eglglobals.c index cf669cf..56fe9e2 100644 --- a/src/egl/main/eglglobals.c +++ b/src/egl/main/eglglobals.c @@ -55,7 +55,8 @@ struct _egl_global _eglGlobal = true, /* EGL_EXT_platform_base */ true, /* EGL_EXT_platform_x11 */ true, /* EGL_EXT_platform_wayland */ - true /* EGL_MESA_platform_gbm */ + true, /* EGL_MESA_platform_gbm */ + true, /* EGL_KHR_client_get_all_proc_addresses */ }, /* ClientExtensionsString */ @@ -64,6 +65,7 @@ struct _egl_global _eglGlobal = " EGL_EXT_platform_x11" " EGL_EXT_platform_wayland" " EGL_MESA_platform_gbm" + " EGL_KHR_client_get_all_proc_addresses" }; diff --git a/src/egl/main/eglglobals.h b/src/egl/main/eglglobals.h index 9046ea2..a8cf6d6 100644 --- a/src/egl/main/eglglobals.h +++ b/src/egl/main/eglglobals.h @@ -56,6 +56,7 @@ struct _egl_global bool EXT_platform_x11; bool EXT_platform_wayland; bool MESA_platform_gbm; + bool KHR_get_all_proc_addresses; } ClientExtensions; const char *ClientExtensionString; diff --git a/src/egl/main/eglmisc.c b/src/egl/main/eglmisc.c index 388e4b5..2f49809 100644 --- a/src/egl/main/eglmisc.c +++ b/src/egl/main/eglmisc.c @@ -101,6 +101,7 @@ _eglUpdateExtensionsString(_EGLDisplay *dpy) _eglAppendExtension(&exts, "EGL_KHR_image"); _EGL_CHECK_EXTENSION(KHR_vg_parent_image); + _EGL_CHECK_EXTENSION(KHR_get_all_proc_addresses); _EGL_CHECK_EXTENSION(KHR_gl_texture_2D_image); _EGL_CHECK_EXTENSION(KHR_gl_texture_cubemap_image); _EGL_CHECK_EXTENSION(KHR_gl_texture_3D_image); -- 2.1.2.1.g5433a3e ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 02/29] mesa: Set normalized=true for float array formats.
On Wed, Nov 19, 2014 at 11:24 PM, Iago Toral wrote:
> Hi Jason,
>
> we discussed this some weeks ago actually, the detailed explanation is
> here:
> https://bugs.freedesktop.org/show_bug.cgi?id=84566#c5
>
> the short answer is that this is necessary because there is a normalized
> parameter to _mesa_swizzle_and_convert, and when we deal with float
> types we want to set this to true.
>
I went back and looked at that and I thought the result of the discussion
was to fix the assert in mesa_format_convert and compute the normalized
parameter correctly. After that, I thought this shouldn't be strictly
needed. It may still be a good idea for consistency, but I want to make
sure we're doing the right thing in mesa_format_convert
--Jason
> Iago
>
> On Wed, 2014-11-19 at 11:31 -0800, Jason Ekstrand wrote:
> > I'm not sure what I think about this. Does it make a functional
> > change other than consistancy?
> >
> > --Jason
> >
> >
> > On Tue, Nov 18, 2014 at 1:23 AM, Iago Toral Quiroga
> > wrote:
> > In order to check if a format is normalized Mesa does
> > something like this:
> > normalized = !_mesa_is_enum_format_integer(srcFormat);
> >
> > So all float types will set normalized to true. Since our
> > mesa_array_format
> > includes a normalized flag for each type we want to make it
> > consistent with
> > this.
> > ---
> > src/mesa/main/format_info.py | 3 ++-
> > src/mesa/main/format_utils.c | 2 +-
> > 2 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/mesa/main/format_info.py
> > b/src/mesa/main/format_info.py
> > index 315767d..d4bc276 100644
> > --- a/src/mesa/main/format_info.py
> > +++ b/src/mesa/main/format_info.py
> > @@ -220,9 +220,10 @@ for fmat in formats:
> > print ' {{ {0} }},'.format(', '.join(map(str,
> > fmat.swizzle)))
> > if fmat.is_array() and fmat.colorspace in ('rgb', 'srgb'):
> >chan = fmat.array_element()
> > + norm = chan.norm or chan.type == parser.FLOAT
> >print ' {0} ,'.format(', '.join([
> > get_array_format_datatype(chan),
> > - str(int(chan.norm)),
> > + str(int(norm)),
> > str(len(fmat.channels)),
> > str(fmat.swizzle[0]),
> > str(fmat.swizzle[1]),
> > diff --git a/src/mesa/main/format_utils.c
> > b/src/mesa/main/format_utils.c
> > index c3815cb..1d65f2b 100644
> > --- a/src/mesa/main/format_utils.c
> > +++ b/src/mesa/main/format_utils.c
> > @@ -30,7 +30,7 @@
> >
> > mesa_array_format RGBA_FLOAT = {{
> > MESA_ARRAY_FORMAT_TYPE_FLOAT,
> > - 0,
> > + 1,
> > 4,
> > 0, 1, 2, 3,
> > 0, 1
> > --
> > 1.9.1
> >
> > ___
> > 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 06/29] mesa: Avoid pack/unpack fast paths if base internal format != base format
On Wed, Nov 19, 2014 at 11:58 PM, Iago Toral wrote:
> On Wed, 2014-11-19 at 11:57 -0800, Jason Ekstrand wrote:
> > A couple of general comments on this patch:
> >
> >
> > 1) The prerequisites should be moved to before the first patch in the
> > series and it should be squashed into the patch that introduces the
> > function. There are one or two more patches which also modify it and
> > those should also be squashed in.
>
> Ok.
>
> >
> > 2) I wonder if giving _mesa_format_convert an internal swizzle
> > wouldn't be better than a destination internal format. There are a
> > couple of reasons for this:
> >
> > a) It's more general. If it doesn't cost us anything extra to do
> > it that way, we might as well.
>
> I think that would only work directly for conversions between array
> formats, but what if we have, for example, a texture upload from RGB565
> to a Luminance format (so that we get an RGBA base format)? that would
> not go though _mesa_swizzle_and_convert and would require to unpack to
> RGBA and then pack to the dst... and unless the client has provided the
> dst format as an array format that won't use _mesa_swizzle_and_convert
> either. That should not be a problem when the calls to
> _mesa_format_convert come from things like glTexImage or glReadPixels,
> since in these cases the compute the dst format from a GL type and if it
> is an array format we should get that, but in other cases that might not
> be the case...
>
I'm a bit confused about what you mean here. If the user passes in a
non-trivial swizzle and we have to pack and unpack on both sides then we
have to unpack, swizzle, and then repack. We would still have to do this
if all you pass in is an internal format. Fortunately, the
_mesa_swizzle_and_convert function can be used to do an in-place swizzle as
long as the source and destination types have the same number of bits per
pixel.
If one side of the pack/repack is an array format, we can just build the
swizzling into the one _mesa_swizzle_and_convert call.
> b) It removes the GL enum dependency from the _mesa_format_convert
> > c) I think this implementation misses the case where we download a
> > texture that is storred in a different format than its base format.
> > For instance, if you are downloading a GL_RGB texture as GL_RGBA but
> > it's storred as GL_RGBA. I think with the current implementaion,
> > you'll get the junk in the alpha component of the texture's backing
> > storage instead of a constant alpha value of 1.
>
> That's correct. In the implementation of readpixels and getteximage we
> had to rebase the results in some cases to account for that.
>
> Iago
>
> >
> > On Tue, Nov 18, 2014 at 1:23 AM, Iago Toral Quiroga
> > wrote:
> > In general, if the dst base internal format and the selected
> > dst format are
> > different we can't be sure that directly packing or unpacking
> > to the destination
> > format will produce correct results. In these cases we
> > probably want to fall
> > back to other paths (like mesa_swizzle_and_convert) that can
> > handle these
> > situations better.
> >
> > An example of this includes a luminance internal format for
> > which the driver
> > decided to use something like BGRA. In these case, unpacking
> > to BGRA won't
> > match the requirements of the luminance internal format.
> >
> > In the future we may want to allow these fast paths for
> > specific cases
> > where we know the direct pack/unpack functions will do the
> > right thing.
> > ---
> > src/mesa/main/format_utils.c | 137
> > +++
> > 1 file changed, 72 insertions(+), 65 deletions(-)
> >
> > diff --git a/src/mesa/main/format_utils.c
> > b/src/mesa/main/format_utils.c
> > index 5964689..34c90d9 100644
> > --- a/src/mesa/main/format_utils.c
> > +++ b/src/mesa/main/format_utils.c
> > @@ -331,65 +331,82 @@ _mesa_format_convert(void *void_dst,
> > uint32_t dst_format, size_t dst_stride,
> >dst_array_format.as_uint =
> > _mesa_format_to_array_format(dst_format);
> > }
> >
> > - /* Handle the cases where we can directly unpack */
> > - if (!(src_format & MESA_ARRAY_FORMAT_BIT)) {
> > - if (dst_array_format.as_uint == RGBA_FLOAT.as_uint)
> > {
> > - for (row = 0; row < height; ++row) {
> > -_mesa_unpack_rgba_row(src_format, width,
> > - src, (float (*)[4])dst);
> > -src += src_stride;
> > -dst += dst_stride;
> > - }
> > - return;
> > - } else if (dst_array_format.as_uint ==
> > RGBA_UBYTE.as_uint &&
> > - !_mesa_is_fo
Re: [Mesa-dev] [PATCH 09/29] mesa: Add _mesa_swap2_copy and _mesa_swap4_copy
On Thu, Nov 20, 2014 at 12:48 AM, Iago Toral wrote:
> On Wed, 2014-11-19 at 14:15 -0600, Patrick Baggett wrote:
> >
> >
> > On Tue, Nov 18, 2014 at 3:23 AM, Iago Toral Quiroga
> > wrote:
> > We have _mesa_swap{2,4} but these do in-place byte-swapping
> > only. The new
> > functions receive an extra parameter so we can swap bytes on a
> > source
> > input array and store the results in a (possibly different)
> > destination
> > array.
> >
> >
> >
> > If this is being split into an "in-place" and "different pointers"
> > version, I think using the "restrict" keyword would be useful here to
> > improve the performance. Then, the in-place one cannot be implemented
> > as copy(p,p,n), but the code isn't overly complicated.
>
> I did not know about 'restrict', thanks for the tip!.
>
> It kind of defeats the original idea of not duplicating the code but it
> is true that it is not particularly complex anyway, so maybe it is worth
> it if Jason agrees with having two versions of the functions instead of
> just one in the end. Jason, what do you think?
>
The restrict keyword is a C99 thing and I don't think it's supported in
MSVC so that would be a problem. If it won't build with MSVC then it's a
non-starter. If MSVC can handle "restrict", then I don't know that I care
much either way about 2 functions or 4
>
> Iago
>
> >
> >
> > This is useful to implement byte-swapping in pixel uploads,
> > since in this
> > case we need to swap bytes on the src data which is owned by
> > the
> > application so we can't do an in-place byte swap.
> > ---
> > src/mesa/main/image.c | 25 +
> > src/mesa/main/image.h | 10 --
> > 2 files changed, 25 insertions(+), 10 deletions(-)
> >
> > diff --git a/src/mesa/main/image.c b/src/mesa/main/image.c
> > index 4ea5f04..9ad97c5 100644
> > --- a/src/mesa/main/image.c
> > +++ b/src/mesa/main/image.c
> > @@ -41,36 +41,45 @@
> >
> >
> > /**
> > - * Flip the order of the 2 bytes in each word in the given
> > array.
> > + * Flip the order of the 2 bytes in each word in the given
> > array (src) and
> > + * store the result in another array (dst). For in-place
> > byte-swapping this
> > + * function can be called with the same array for src and
> > dst.
> > *
> > - * \param p array.
> > + * \param dst the array where byte-swapped data will be
> > stored.
> > + * \param src the array with the source data we want to
> > byte-swap.
> > * \param n number of words.
> > */
> > void
> > -_mesa_swap2( GLushort *p, GLuint n )
> > +_mesa_swap2_copy( GLushort *dst, GLushort *src, GLuint n )
> > {
> > GLuint i;
> > for (i = 0; i < n; i++) {
> > - p[i] = (p[i] >> 8) | ((p[i] << 8) & 0xff00);
> > + dst[i] = (src[i] >> 8) | ((src[i] << 8) & 0xff00);
> > }
> > }
> >
> >
> >
> > /*
> > - * Flip the order of the 4 bytes in each word in the given
> > array.
> > + * Flip the order of the 4 bytes in each word in the given
> > array (src) and
> > + * store the result in another array (dst). For in-place
> > byte-swapping this
> > + * function can be called with the same array for src and
> > dst.
> > + *
> > + * \param dst the array where byte-swapped data will be
> > stored.
> > + * \param src the array with the source data we want to
> > byte-swap.
> > + * \param n number of words.
> > */
> > void
> > -_mesa_swap4( GLuint *p, GLuint n )
> > +_mesa_swap4_copy( GLuint *dst, GLuint *src, GLuint n )
> > {
> > GLuint i, a, b;
> > for (i = 0; i < n; i++) {
> > - b = p[i];
> > + b = src[i];
> >a = (b >> 24)
> > | ((b >> 8) & 0xff00)
> > | ((b << 8) & 0xff)
> > | ((b << 24) & 0xff00);
> > - p[i] = a;
> > + dst[i] = a;
> > }
> > }
> >
> > diff --git a/src/mesa/main/image.h b/src/mesa/main/image.h
> > index abd84bf..79c6e68 100644
> > --- a/src/mesa/main/image.h
> > +++ b/src/mesa/main/image.h
> > @@ -33,10 +33,16 @@ struct gl_context;
> > struct gl_pixelstore_attrib;
> >
> > extern void
> > -_mesa_swap2( GLushort *p, GLuint n );
> > +_mesa_swap2_copy( GLushort *dst, GLushort *src, GLuint n );
> >
> > extern void
> > -_mesa_swap4( GLuint *p, GLuint n );
> > +_mesa_swap4_copy( GLuint *dst, GLuint *src, GLuint n );
> > +
> > +static inline
Re: [Mesa-dev] [PATCH 09/29] mesa: Add _mesa_swap2_copy and _mesa_swap4_copy
> > >> > The restrict keyword is a C99 thing and I don't think it's supported in > MSVC so that would be a problem. If it won't build with MSVC then it's a > non-starter. If MSVC can handle "restrict", then I don't know that I care > much either way about 2 functions or 4 > > MSVC uses "__restrict" which functions identically -- but if there doesn't already exist a #define around this "MSVC-ism", then I guess it may be more work then Iago was really signing up for. But it does exist. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 05/29] mesa: Consider internal base format in _mesa_format_convert
On Wed, Nov 19, 2014 at 11:42 PM, Iago Toral wrote:
> On Wed, 2014-11-19 at 11:43 -0800, Jason Ekstrand wrote:
> > A couple of specific comments are below. More generally, why are you
> > only considering the base format on two cases? Do we never use it for
> > anything else?
>
> I thought about that too but when I looked at the original code it
> seemed that it only cared for the base format in these two scenarios, so
> I thought that maybe the conversions cases that could be affected are
> all handled in those two paths. I'll check again though, just in case I
> missed something.
>
> > On Tue, Nov 18, 2014 at 1:23 AM, Iago Toral Quiroga
> > wrote:
> > Add a dst_internal_format parameter to _mesa_format_convert,
> > that represents
> > the base internal format for texture/pixel uploads, so we can
> > do the right
> > thing when the driver has selected a different internal format
> > for the target
> > dst format.
> > ---
> > src/mesa/main/format_utils.c | 65
> > +++-
> > src/mesa/main/format_utils.h | 2 +-
> > 2 files changed, 65 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/mesa/main/format_utils.c
> > b/src/mesa/main/format_utils.c
> > index fc59e86..5964689 100644
> > --- a/src/mesa/main/format_utils.c
> > +++ b/src/mesa/main/format_utils.c
> > @@ -303,7 +303,7 @@ _mesa_compute_component_mapping(GLenum
> > inFormat, GLenum outFormat, GLubyte *map)
> > void
> > _mesa_format_convert(void *void_dst, uint32_t dst_format,
> > size_t dst_stride,
> > void *void_src, uint32_t src_format,
> > size_t src_stride,
> > - size_t width, size_t height)
> > + size_t width, size_t height, GLenum
> > dst_internal_format)
> > {
> > uint8_t *dst = (uint8_t *)void_dst;
> > uint8_t *src = (uint8_t *)void_src;
> > @@ -422,6 +422,36 @@ _mesa_format_convert(void *void_dst,
> > uint32_t dst_format, size_t dst_stride,
> > if (src_array_format.as_uint && dst_array_format.as_uint)
> > {
> >assert(src_array_format.normalized ==
> > dst_array_format.normalized);
> >
> > + /* If the base format of our dst is not the same as the
> > provided base
> > + * format it means that we are converting to a
> > different format
> > + * than the one originally requested by the client.
> > This can happen when
> > + * the internal base format requested is not supported
> > by the driver.
> > + * In this case we need to consider the requested
> > internal base format to
> > + * compute the correct swizzle operation from src to
> > dst. We will do this
> > + * by computing the swizzle transform
> > src->rgba->base->rgba->dst instead
> > + * of src->rgba->dst.
> > + */
> > + mesa_format dst_mesa_format;
> > + if (dst_format & MESA_ARRAY_FORMAT_BIT)
> > + dst_mesa_format =
> > _mesa_format_from_array_format(dst_format);
> > + else
> > + dst_mesa_format = dst_format;
> >
> >
> > Let's put an extra line here so it doesn't get confused with the below
> > if statement
> >
> >
> > + if (dst_internal_format !=
> > _mesa_get_format_base_format(dst_mesa_format)) {
> > + /* Compute src2rgba as src->rgba->base->rgba */
> > + uint8_t rgba2base[4], base2rgba[4], swizzle[4];
> > + _mesa_compute_component_mapping(GL_RGBA,
> > dst_internal_format, rgba2base);
> > + _mesa_compute_component_mapping(dst_internal_format,
> > GL_RGBA, base2rgba);
> > +
> > + for (i = 0; i < 4; i++) {
> > +if (base2rgba[i] > MESA_FORMAT_SWIZZLE_W)
> > + swizzle[i] = base2rgba[i];
> > +else
> > + swizzle[i] =
> > src2rgba[rgba2base[base2rgba[i]]];
> >
> >
> > This doesn't work for composing three swizzles. If you get a ZERO or
> > ONE in rgba2base, you'll read the wrong memory from src2rgba.
>
> Actually, the problem is worse, because the mapping written by
> _mesa_compute_component_mapping is a 6-component mapping and we are
> passing a 4-component array. I'll fix this.
>
Right now we have 2 different swizzle formats floating around. Some with 6
components and some with 4. 4 works just fine as long as you are careful
when you compose swizzles. With 6 you can avoid the if statements because
4 and 5 simply pass through. I like 4 better (which is why I did it that
way in my stuff) but either wor
Re: [Mesa-dev] [PATCH 2/4] i965: Fix intel_miptree_map() signature to be more 64-bit safe
On Wed 19 Nov 2014, Kenneth Graunke wrote: On Wednesday, November 19, 2014 02:13:03 PM Ian Romanick wrote: On 11/18/2014 09:11 PM, Chad Versace wrote: > This patch should diminish the likelihood of pointer arithmetic overflow > bugs, like the one fixed by b69c7c5dac. > > Change the type of parameter 'out_stride' from int to ptrdiff_t. The > logic is that if you call intel_miptree_map() and use the value of > 'out_stride', then you must be doing pointer arithmetic on 'out_ptr'. > Using ptrdiff_t instead of int should make a little bit harder to hit > overflow bugs. So... we talked about this a little bit on Monday, and I don't think we ever had a conclusion. What happens if you have a 32-bit application running on a platform with 48-bit GPU address space? CC'ing Ben, who knows all the gory details. I don't really understand the problem - the GPU uses 48-bit addressing, and can access more than 4G...but we're talking about map, which makes a buffer available in an application's virtual address space...which is 32-bit in your example. It should always be placed at a < 4GB virtual address and work out fine. That said, I don't think the kernel ever uses >= 4GB address spaces today. Ben wrote 4GGGTT support and had both kernel and userspace patches to make it work, but I don't think it ever actually landed. I'm pretty sure shipping userspace is not quite 48-bit safe - there are a few buffers that have to be placed < 4GB (some hardware packets only take 32-bit addresses still), and I don't think any software is in place to make that happen. I agree with Ken. 48-bit isn't an issue that this patch is even trying to address. This patch is trying reduce overflow errors due to sloppy userspace code, errors that will happen (and have happened) with pre-48-bit GPUs. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 01/29] mesa: Add an implementation of a master convert function.
On Thu, Nov 20, 2014 at 1:48 AM, Iago Toral wrote:
> On Wed, 2014-11-19 at 11:28 -0800, Jason Ekstrand wrote:
> > By and large, this looks good to me. Most of my comments are cosmetic
> > or suggestions for added documentation. There is one issue that I
> > think is subtly wrong with integer format conversion but that should
> > be easy to fix.
> >
> > --Jason
> >
> > On Tue, Nov 18, 2014 at 1:23 AM, Iago Toral Quiroga
> > wrote:
> > From: Jason Ekstrand
> >
> > v2 by Iago Toral :
> >
> > - When testing if we can directly pack we should use the src
> > format to check
> > if we are packing from an RGBA format. The original code
> > used the dst format
> > for the ubyte case by mistake.
> > - Fixed incorrect number of bits for dst, it was computed
> > using the src format
> > instead of the dst format.
> > - If the dst format is an array format, check if it is signed.
> > We were only
> > checking this for the case where it was not an array format,
> > but we need
> > to know this in both scenarios.
> > - Fixed incorrect swizzle transform for the cases where we
> > convert between
> > array formats.
> > - Compute is_signed and bits only once and for the dst format.
> > We were
> > computing these for the src format too but they were
> > overwritten by the
> > dst values immediately after.
> > - Be more careful when selecting the integer path.
> > Specifically, check that
> > both src and dst are integer types. Checking only one of
> > them should suffice
> > since OpenGL does not allow conversions between normalized
> > and integer types,
> > but putting extra care here makes sense and also makes the
> > actual requirements
> > for this path more clear.
> > - The format argument for pack functions is the destination
> > format we are
> > packing to, not the source format (which has to be RGBA).
> > - Expose RGBA_* to other files. These will come in handy
> > when in need to
> > test if a given array format is RGBA or in need to pass RGBA
> > formats to
> > mesa_format_convert.
> >
> > v3 by Samuel Iglesias :
> >
> > - Add an RGBA_INT definition.
> > ---
> > src/mesa/main/format_utils.c | 378
> > +++
> > src/mesa/main/format_utils.h | 10 ++
> > src/mesa/main/formats.h | 15 +-
> > 3 files changed, 399 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/mesa/main/format_utils.c
> > b/src/mesa/main/format_utils.c
> > index fcbbba4..c3815cb 100644
> > --- a/src/mesa/main/format_utils.c
> > +++ b/src/mesa/main/format_utils.c
> > @@ -25,6 +25,384 @@
> > #include "format_utils.h"
> > #include "glformats.h"
> > #include "macros.h"
> > +#include "format_pack.h"
> > +#include "format_unpack.h"
> > +
> > +mesa_array_format RGBA_FLOAT = {{
> > + MESA_ARRAY_FORMAT_TYPE_FLOAT,
> > + 0,
> > + 4,
> > + 0, 1, 2, 3,
> > + 0, 1
> > +}};
> > +
> > +mesa_array_format RGBA_UBYTE = {{
> > + MESA_ARRAY_FORMAT_TYPE_UBYTE,
> > + 1,
> > + 4,
> > + 0, 1, 2, 3,
> > + 0, 1
> > +}};
> > +
> > +mesa_array_format RGBA_UINT = {{
> > + MESA_ARRAY_FORMAT_TYPE_UINT,
> > + 0,
> > + 4,
> > + 0, 1, 2, 3,
> > + 0, 1
> > +}};
> > +
> > +mesa_array_format RGBA_INT = {{
> > + MESA_ARRAY_FORMAT_TYPE_INT,
> > + 0,
> > + 4,
> > + 0, 1, 2, 3,
> > + 0, 1
> > +}};
> > +
> > +static void
> > +invert_swizzle(uint8_t dst[4], const uint8_t src[4])
> > +{
> > + int i, j;
> > +
> > + dst[0] = MESA_FORMAT_SWIZZLE_NONE;
> > + dst[1] = MESA_FORMAT_SWIZZLE_NONE;
> > + dst[2] = MESA_FORMAT_SWIZZLE_NONE;
> > + dst[3] = MESA_FORMAT_SWIZZLE_NONE;
> > +
> > + for (i = 0; i < 4; ++i)
> > + for (j = 0; j < 4; ++j)
> > + if (src[j] == i && dst[i] ==
> > MESA_FORMAT_SWIZZLE_NONE)
> > +dst[i] = j;
> > +}
> > +
> > +static GLenum
> > +gl_type_for_array_format_datatype(enum
> > mesa_array_format_datatype type)
> > +{
> > + switch (type) {
> > + case MESA_ARRAY_FORMAT_TYPE_UBYTE:
> > + return GL_UNSIGNED_BYTE;
> > + case MESA_ARRAY_FORMAT_TYPE_USHORT:
> >
Re: [Mesa-dev] [PATCH 1/9] i915: Only use TEXCOORDTYPE_VECTOR with cube maps on gen2
On Thu, Nov 20, 2014 at 09:59:00AM -0800, Ian Romanick wrote:
> On 08/06/2014 11:56 AM, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > Check that the target is GL_TEXTURE_CUBE_MAP before emitting
> > TEXCOORDTYPE_VECTOR texture coordinates.
> >
> > I'm not sure if the hardware would like CARTESIAN coordinates
> > with cube maps, and as I'm too lazy to find out just emit the
> > VECTOR coordinates for cube maps always. For other targets use
> > CARTESIAN or HOMOGENOUS depending on the number of texture
> > coordinates provided.
> >
> > Fixes rendering of the "electric" background texture in chromium-bsu
> > main menu. We appear to be provided with three texture coordinates
> > there (I'm guessing due to the funky texture matrix rotation it does).
> > So the code would decide to use TEXCOORDTYPE_VECTOR instead of
> > TEXCOORDTYPE_CARTESIAN even though we're dealing with a 2D texure.
> > The results weren't what one might expect.
> >
> > demos/cubemap still works, which hopefully indicates that this doesn't
> > break things.
>
> I won't dare ask about a full piglit run on that hardware,
I did actually try to run piglit on the 830, but it always
seemed to die in some X blit batch after a while :(
There have been some recent blt TLB workaround fixes in the
kernel though, so perhaps things are better now. And if not,
I do have a 855 that ought to be a bit less fragile. So
maybe I'll give it another go just for kicks :P
> but how about
>
> bin/glean -o -v -v -v -t +texCube --quick
>
> and
>
> bin/cubemap -auto
>
> from piglit?
pass->pass on both counts.
>
> These changes seem reasonable, and assuming those tests aren't made worse,
>
> Reviewed-by: Ian Romanick
>
> If you're excited about GEN2 bugs, wanna take a look at 79841? >:)
I'm thinking it should be fixed by:
commit dafae910d4fc791ba49f20e937cb918669f42944
Author: Ville Syrjälä
Date: Thu Jul 3 15:38:07 2014 +0300
i915: Accept GL_DEPTH_STENCIL GL_DEPTH_COMPONENT formats for
renderbuffers
I'll note it in the bug.
>
> > Signed-off-by: Ville Syrjälä
> > ---
> > src/mesa/drivers/dri/i915/i830_vtbl.c | 37
> > ++-
> > 1 file changed, 19 insertions(+), 18 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i915/i830_vtbl.c
> > b/src/mesa/drivers/dri/i915/i830_vtbl.c
> > index 53d408b..0f22d86 100644
> > --- a/src/mesa/drivers/dri/i915/i830_vtbl.c
> > +++ b/src/mesa/drivers/dri/i915/i830_vtbl.c
> > @@ -134,27 +134,28 @@ i830_render_start(struct intel_context *intel)
> > GLuint mcs = (i830->state.Tex[i][I830_TEXREG_MCS] &
> >~TEXCOORDTYPE_MASK);
> >
> > -switch (sz) {
> > -case 1:
> > -case 2:
> > - emit = EMIT_2F;
> > - sz = 2;
> > - mcs |= TEXCOORDTYPE_CARTESIAN;
> > - break;
> > -case 3:
> > +if (intel->ctx.Texture.Unit[i]._Current->Target ==
> > GL_TEXTURE_CUBE_MAP) {
> > emit = EMIT_3F;
> > sz = 3;
> > mcs |= TEXCOORDTYPE_VECTOR;
> > - break;
> > -case 4:
> > - emit = EMIT_3F_XYW;
> > - sz = 3;
> > - mcs |= TEXCOORDTYPE_HOMOGENEOUS;
> > - break;
> > -default:
> > - continue;
> > -};
> > -
> > +} else {
> > + switch (sz) {
> > + case 1:
> > + case 2:
> > + case 3:
> > + emit = EMIT_2F;
> > + sz = 2;
> > + mcs |= TEXCOORDTYPE_CARTESIAN;
> > + break;
> > + case 4:
> > + emit = EMIT_3F_XYW;
> > + sz = 3;
> > + mcs |= TEXCOORDTYPE_HOMOGENEOUS;
> > + break;
> > + default:
> > + continue;
> > + }
> > +}
> >
> > EMIT_ATTR(_TNL_ATTRIB_TEX0 + i, emit, 0);
> > v2 |= VRTX_TEX_SET_FMT(count, SZ_TO_HW(sz));
> >
--
Ville Syrjälä
Intel OTC
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/7] mesa: Silence unused parameter warnings in _mesa_validate_Draw functions
From: Ian Romanick
../../src/mesa/main/api_validate.c: In function '_mesa_validate_DrawElements':
../../src/mesa/main/api_validate.c:376:37: warning: unused parameter
'basevertex' [-Wunused-parameter]
../../src/mesa/main/api_validate.c: In function
'_mesa_validate_MultiDrawElements':
../../src/mesa/main/api_validate.c:394:65: warning: unused parameter
'basevertex' [-Wunused-parameter]
../../src/mesa/main/api_validate.c: In function
'_mesa_validate_DrawRangeElements':
../../src/mesa/main/api_validate.c:452:35: warning: unused parameter
'basevertex' [-Wunused-parameter]
../../src/mesa/main/api_validate.c: In function '_mesa_validate_DrawArrays':
../../src/mesa/main/api_validate.c:473:25: warning: unused parameter 'start'
[-Wunused-parameter]
../../src/mesa/main/api_validate.c: In function
'_mesa_validate_DrawElementsInstanced':
../../src/mesa/main/api_validate.c:590:44: warning: unused parameter
'basevertex' [-Wunused-parameter]
Signed-off-by: Ian Romanick
---
src/mesa/main/api_validate.c | 12 +---
src/mesa/main/api_validate.h | 12 +---
src/mesa/vbo/vbo_exec_array.c | 23 +++
3 files changed, 21 insertions(+), 26 deletions(-)
diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
index 276aab7..181a61d 100644
--- a/src/mesa/main/api_validate.c
+++ b/src/mesa/main/api_validate.c
@@ -381,7 +381,7 @@ validate_DrawElements_common(struct gl_context *ctx,
GLboolean
_mesa_validate_DrawElements(struct gl_context *ctx,
GLenum mode, GLsizei count, GLenum type,
- const GLvoid *indices, GLint basevertex)
+ const GLvoid *indices)
{
FLUSH_CURRENT(ctx, 0);
@@ -399,7 +399,7 @@ GLboolean
_mesa_validate_MultiDrawElements(struct gl_context *ctx,
GLenum mode, const GLsizei *count,
GLenum type, const GLvoid * const *indices,
- GLuint primcount, const GLint *basevertex)
+ GLuint primcount)
{
unsigned i;
@@ -457,7 +457,7 @@ GLboolean
_mesa_validate_DrawRangeElements(struct gl_context *ctx, GLenum mode,
GLuint start, GLuint end,
GLsizei count, GLenum type,
-const GLvoid *indices, GLint basevertex)
+const GLvoid *indices)
{
FLUSH_CURRENT(ctx, 0);
@@ -477,8 +477,7 @@ _mesa_validate_DrawRangeElements(struct gl_context *ctx,
GLenum mode,
* \return GL_TRUE if OK to render, GL_FALSE if error found
*/
GLboolean
-_mesa_validate_DrawArrays(struct gl_context *ctx,
- GLenum mode, GLint start, GLsizei count)
+_mesa_validate_DrawArrays(struct gl_context *ctx, GLenum mode, GLsizei count)
{
struct gl_transform_feedback_object *xfb_obj
= ctx->TransformFeedback.CurrentObject;
@@ -594,8 +593,7 @@ _mesa_validate_DrawArraysInstanced(struct gl_context *ctx,
GLenum mode, GLint fi
GLboolean
_mesa_validate_DrawElementsInstanced(struct gl_context *ctx,
GLenum mode, GLsizei count, GLenum type,
- const GLvoid *indices, GLsizei
numInstances,
- GLint basevertex)
+ const GLvoid *indices, GLsizei
numInstances)
{
FLUSH_CURRENT(ctx, 0);
diff --git a/src/mesa/main/api_validate.h b/src/mesa/main/api_validate.h
index 0bb91c6..0ce7b69 100644
--- a/src/mesa/main/api_validate.h
+++ b/src/mesa/main/api_validate.h
@@ -43,25 +43,24 @@ _mesa_valid_prim_mode(struct gl_context *ctx, GLenum mode,
const char *name);
extern GLboolean
-_mesa_validate_DrawArrays(struct gl_context *ctx,
- GLenum mode, GLint start, GLsizei count);
+_mesa_validate_DrawArrays(struct gl_context *ctx, GLenum mode, GLsizei count);
extern GLboolean
_mesa_validate_DrawElements(struct gl_context *ctx,
GLenum mode, GLsizei count, GLenum type,
- const GLvoid *indices, GLint basevertex);
+ const GLvoid *indices);
extern GLboolean
_mesa_validate_MultiDrawElements(struct gl_context *ctx,
GLenum mode, const GLsizei *count,
GLenum type, const GLvoid * const *indices,
- GLuint primcount, const GLint *basevertex);
+ GLuint primcount);
extern GLboolean
_mesa_validate_DrawRangeElements(struct gl_context *ctx, GLenum mode,
GLuint start, GLuint end,
GLsizei count, GLenum type,
-const GLvoid *indices, GLint basevertex);
+const GLvoid *indices);
extern GLboolean
@@ -71,8 +70,7 @@ _mesa_validate_DrawArraysInstanced
[Mesa-dev] [PATCH 4/7] mesa: Use unreachable instead of assert in check_valid_to_render
From: Ian Romanick
This is generally the prefered style these days.
Signed-off-by: Ian Romanick
---
src/mesa/main/api_validate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
index 181a61d..304d576 100644
--- a/src/mesa/main/api_validate.c
+++ b/src/mesa/main/api_validate.c
@@ -113,7 +113,7 @@ check_valid_to_render(struct gl_context *ctx, const char
*function)
break;
default:
- assert(!"Invalid API value in check_valid_to_render()");
+ unreachable("Invalid API value in check_valid_to_render()");
}
return GL_TRUE;
--
1.8.1.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/7] mesa: Generate GL_INVALID_OPERATION when drawing w/o a VAO in core profile
From: Ian Romanick
GL 3-ish versions of the spec are less clear that an error should be
generated here, so Ken (and I during review) just missed it in 1afe335.
Signed-off-by: Ian Romanick
Cc: Kenneth Graunke
---
src/mesa/main/api_validate.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
index bf4fa3e..006fca4 100644
--- a/src/mesa/main/api_validate.c
+++ b/src/mesa/main/api_validate.c
@@ -79,8 +79,16 @@ check_valid_to_render(struct gl_context *ctx, const char
*function)
break;
case API_OPENGL_CORE:
- if (ctx->Array.VAO == ctx->Array.DefaultVAO)
+ /* Section 10.4 (Drawing Commands Using Vertex Arrays) of the OpenGL 4.5
+ * Core Profile spec says:
+ *
+ * "An INVALID_OPERATION error is generated if no vertex array
+ * object is bound (see section 10.3.1)."
+ */
+ if (ctx->Array.VAO == ctx->Array.DefaultVAO) {
+ _mesa_error(ctx, GL_INVALID_OPERATION, "%s(no VAO bound)", function);
return GL_FALSE;
+ }
/* fallthrough */
case API_OPENGL_COMPAT:
{
--
1.8.1.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 7/7] i965: Reorder fields of brw_state_flags to plug a hole on 64-bit
From: Ian Romanick
Signed-off-by: Ian Romanick
---
src/mesa/drivers/dri/i965/brw_context.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_context.h
b/src/mesa/drivers/dri/i965/brw_context.h
index 656cbe8..d1b5af7 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -226,12 +226,12 @@ enum brw_state_id {
#define BRW_NEW_TEXTURE_BUFFER (1ull << BRW_STATE_TEXTURE_BUFFER)
struct brw_state_flags {
- /** State update flags signalled by mesa internals */
- GLuint mesa;
/**
* State update flags signalled as the result of brw_tracked_state updates
*/
uint64_t brw;
+ /** State update flags signalled by mesa internals */
+ GLuint mesa;
/**
* State update flags that used to be signalled by brw_state_cache.c
* searches.
--
1.8.1.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 0/7] valid-to-render house keeping
This little series is just a bit of house keeping in the valid-to-render path. There is a follow-up series in progress that tries to optimize some of these paths for CPU-bound workloads. The whole series has been reordered several times. Orignally patches 4 and 5 of this series were farther apart. As it stands now, they should probably be squashed (taking the commit message from patch 5). FWIW... I was not able to measure any performance delta on any test on any platform at even 80% confidence across this series. src/mesa/drivers/dri/i965/brw_context.h | 4 +- src/mesa/drivers/dri/i965/brw_draw.c| 7 +- src/mesa/main/api_validate.c| 239 src/mesa/main/api_validate.h| 12 +- src/mesa/vbo/vbo_exec_array.c | 23 ++- 5 files changed, 101 insertions(+), 184 deletions(-) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/7] mesa: Refactor common validation code to validate_DrawElements_common
From: Ian Romanick
Most of the code in _mesa_validate_DrawElements,
_mesa_validate_DrawRangeElements, and
_mesa_validate_DrawElementsInstanced was the same. Refactor this out to
common code.
As a side-effect, a bug in _mesa_validate_DrawElementsInstanced was
fixed. Previously this function would not generate an error when
check_valid_to_render failed if numInstances was 0.
Signed-off-by: Ian Romanick
---
src/mesa/main/api_validate.c | 168 +++
1 file changed, 43 insertions(+), 125 deletions(-)
diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
index 006fca4..276aab7 100644
--- a/src/mesa/main/api_validate.c
+++ b/src/mesa/main/api_validate.c
@@ -318,18 +318,12 @@ valid_elements_type(struct gl_context *ctx, GLenum type,
const char *name)
}
}
-/**
- * Error checking for glDrawElements(). Includes parameter checking
- * and VBO bounds checking.
- * \return GL_TRUE if OK to render, GL_FALSE if error found
- */
-GLboolean
-_mesa_validate_DrawElements(struct gl_context *ctx,
- GLenum mode, GLsizei count, GLenum type,
- const GLvoid *indices, GLint basevertex)
+static bool
+validate_DrawElements_common(struct gl_context *ctx,
+ GLenum mode, GLsizei count, GLenum type,
+ const GLvoid *indices,
+ const char *caller)
{
- FLUSH_CURRENT(ctx, 0);
-
/* From the GLES3 specification, section 2.14.2 (Transform Feedback
* Primitive Capture):
*
@@ -339,44 +333,60 @@ _mesa_validate_DrawElements(struct gl_context *ctx,
*/
if (_mesa_is_gles3(ctx) && _mesa_is_xfb_active_and_unpaused(ctx)) {
_mesa_error(ctx, GL_INVALID_OPERATION,
- "glDrawElements(transform feedback active)");
- return GL_FALSE;
+ "%s(transform feedback active)", caller);
+ return false;
}
if (count < 0) {
- _mesa_error(ctx, GL_INVALID_VALUE, "glDrawElements(count)" );
- return GL_FALSE;
+ _mesa_error(ctx, GL_INVALID_VALUE, "%s(count)", caller);
+ return false;
}
- if (!_mesa_valid_prim_mode(ctx, mode, "glDrawElements")) {
- return GL_FALSE;
+ if (!_mesa_valid_prim_mode(ctx, mode, caller)) {
+ return false;
}
- if (!valid_elements_type(ctx, type, "glDrawElements"))
- return GL_FALSE;
+ if (!valid_elements_type(ctx, type, caller))
+ return false;
- if (!check_valid_to_render(ctx, "glDrawElements"))
- return GL_FALSE;
+ if (!check_valid_to_render(ctx, caller))
+ return false;
/* Vertex buffer object tests */
if (_mesa_is_bufferobj(ctx->Array.VAO->IndexBufferObj)) {
/* use indices in the buffer object */
/* make sure count doesn't go outside buffer bounds */
if (index_bytes(type, count) > ctx->Array.VAO->IndexBufferObj->Size) {
- _mesa_warning(ctx, "glDrawElements index out of buffer bounds");
- return GL_FALSE;
+ _mesa_warning(ctx, "%s index out of buffer bounds", caller);
+ return false;
}
}
else {
/* not using a VBO */
if (!indices)
- return GL_FALSE;
+ return false;
}
if (count == 0)
- return GL_FALSE;
+ return false;
- return GL_TRUE;
+ return true;
+}
+
+/**
+ * Error checking for glDrawElements(). Includes parameter checking
+ * and VBO bounds checking.
+ * \return GL_TRUE if OK to render, GL_FALSE if error found
+ */
+GLboolean
+_mesa_validate_DrawElements(struct gl_context *ctx,
+ GLenum mode, GLsizei count, GLenum type,
+ const GLvoid *indices, GLint basevertex)
+{
+ FLUSH_CURRENT(ctx, 0);
+
+ return validate_DrawElements_common(ctx, mode, count, type, indices,
+ "glDrawElements");
}
@@ -451,58 +461,13 @@ _mesa_validate_DrawRangeElements(struct gl_context *ctx,
GLenum mode,
{
FLUSH_CURRENT(ctx, 0);
- /* From the GLES3 specification, section 2.14.2 (Transform Feedback
-* Primitive Capture):
-*
-* The error INVALID_OPERATION is also generated by DrawElements,
-* DrawElementsInstanced, and DrawRangeElements while transform feedback
-* is active and not paused, regardless of mode.
-*/
- if (_mesa_is_gles3(ctx) && _mesa_is_xfb_active_and_unpaused(ctx)) {
- _mesa_error(ctx, GL_INVALID_OPERATION,
- "glDrawElements(transform feedback active)");
- return GL_FALSE;
- }
-
- if (count < 0) {
- _mesa_error(ctx, GL_INVALID_VALUE, "glDrawRangeElements(count)" );
- return GL_FALSE;
- }
-
- if (!_mesa_valid_prim_mode(ctx, mode, "glDrawRangeElements")) {
- return GL_FALSE;
- }
-
if (end < start) {
_mesa_error(ctx, GL_INVALID_VALUE, "glDrawRangeElements(endArray.VAO->IndexBufferObj)) {
- /* use indices in the buffer object */
- /* make sure count doesn't go out
[Mesa-dev] [PATCH 5/7] mesa: Use current Mesa coding style in check_valid_to_render
From: Ian Romanick
This makes some others patches (still in my local tree) a bit cleaner.
NOTE: This and the previous patch can probably get squashed together.
Signed-off-by: Ian Romanick
---
src/mesa/main/api_validate.c | 49 ++--
1 file changed, 24 insertions(+), 25 deletions(-)
diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
index 304d576..7d98933 100644
--- a/src/mesa/main/api_validate.c
+++ b/src/mesa/main/api_validate.c
@@ -57,25 +57,25 @@ index_bytes(GLenum type, GLsizei count)
/**
* Check if OK to draw arrays/elements.
*/
-static GLboolean
+static bool
check_valid_to_render(struct gl_context *ctx, const char *function)
{
if (!_mesa_valid_to_render(ctx, function)) {
- return GL_FALSE;
+ return false;
}
switch (ctx->API) {
case API_OPENGLES2:
/* For ES2, we can draw if we have a vertex program/shader). */
if (!ctx->VertexProgram._Current)
-return GL_FALSE;
+return false;
break;
case API_OPENGLES:
/* For OpenGL ES, only draw if we have vertex positions
*/
if (!ctx->Array.VAO->VertexAttrib[VERT_ATTRIB_POS].Enabled)
-return GL_FALSE;
+return false;
break;
case API_OPENGL_CORE:
@@ -87,36 +87,35 @@ check_valid_to_render(struct gl_context *ctx, const char
*function)
*/
if (ctx->Array.VAO == ctx->Array.DefaultVAO) {
_mesa_error(ctx, GL_INVALID_OPERATION, "%s(no VAO bound)", function);
- return GL_FALSE;
+ return false;
}
/* fallthrough */
- case API_OPENGL_COMPAT:
- {
- const struct gl_shader_program *vsProg =
-ctx->_Shader->CurrentProgram[MESA_SHADER_VERTEX];
- GLboolean haveVertexShader = (vsProg && vsProg->LinkStatus);
- GLboolean haveVertexProgram = ctx->VertexProgram._Enabled;
- if (haveVertexShader || haveVertexProgram) {
-/* Draw regardless of whether or not we have any vertex arrays.
- * (Ex: could draw a point using a constant vertex pos)
- */
-return GL_TRUE;
- }
- else {
-/* Draw if we have vertex positions (GL_VERTEX_ARRAY or generic
- * array [0]).
- */
-return (ctx->Array.VAO->VertexAttrib[VERT_ATTRIB_POS].Enabled ||
-
ctx->Array.VAO->VertexAttrib[VERT_ATTRIB_GENERIC0].Enabled);
- }
+ case API_OPENGL_COMPAT: {
+ const struct gl_shader_program *const vsProg =
+ ctx->_Shader->CurrentProgram[MESA_SHADER_VERTEX];
+ const bool haveVertexShader = (vsProg && vsProg->LinkStatus);
+ const bool haveVertexProgram = ctx->VertexProgram._Enabled;
+
+ if (haveVertexShader || haveVertexProgram) {
+ /* Draw regardless of whether or not we have any vertex arrays.
+ * (Ex: could draw a point using a constant vertex pos)
+ */
+ return true;
+ } else {
+ /* Draw if we have vertex positions (GL_VERTEX_ARRAY or generic
+ * array [0]).
+ */
+ return (ctx->Array.VAO->VertexAttrib[VERT_ATTRIB_POS].Enabled ||
+ ctx->Array.VAO->VertexAttrib[VERT_ATTRIB_GENERIC0].Enabled);
}
break;
+ }
default:
unreachable("Invalid API value in check_valid_to_render()");
}
- return GL_TRUE;
+ return true;
}
--
1.8.1.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 6/7] i965: Just return void from brw_try_draw_prims
From: Ian Romanick
---
src/mesa/drivers/dri/i965/brw_draw.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_draw.c
b/src/mesa/drivers/dri/i965/brw_draw.c
index b28eaf2..4c2802a 100644
--- a/src/mesa/drivers/dri/i965/brw_draw.c
+++ b/src/mesa/drivers/dri/i965/brw_draw.c
@@ -347,7 +347,7 @@ static void brw_postdraw_set_buffers_need_resolve(struct
brw_context *brw)
/* May fail if out of video memory for texture or vbo upload, or on
* fallback conditions.
*/
-static bool brw_try_draw_prims( struct gl_context *ctx,
+static void brw_try_draw_prims( struct gl_context *ctx,
const struct gl_client_array *arrays[],
const struct _mesa_prim *prims,
GLuint nr_prims,
@@ -357,7 +357,6 @@ static bool brw_try_draw_prims( struct gl_context *ctx,
struct gl_buffer_object *indirect)
{
struct brw_context *brw = brw_context(ctx);
- bool retval = true;
GLuint i;
bool fail_next = false;
@@ -487,8 +486,6 @@ retry:
"available aperture space\n");
warned = true;
}
-
- retval = false;
}
}
}
@@ -506,7 +503,7 @@ retry:
brw_state_cache_check_size(brw);
brw_postdraw_set_buffers_need_resolve(brw);
- return retval;
+ return;
}
void brw_draw_prims( struct gl_context *ctx,
--
1.8.1.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/4] i965: Fix intel_miptree_map() signature to be more 64-bit safe
On 11/19/2014 09:18 PM, Kenneth Graunke wrote: > On Wednesday, November 19, 2014 02:13:03 PM Ian Romanick wrote: >> On 11/18/2014 09:11 PM, Chad Versace wrote: >>> This patch should diminish the likelihood of pointer arithmetic overflow >>> bugs, like the one fixed by b69c7c5dac. >>> >>> Change the type of parameter 'out_stride' from int to ptrdiff_t. The >>> logic is that if you call intel_miptree_map() and use the value of >>> 'out_stride', then you must be doing pointer arithmetic on 'out_ptr'. >>> Using ptrdiff_t instead of int should make a little bit harder to hit >>> overflow bugs. >> >> So... we talked about this a little bit on Monday, and I don't think we >> ever had a conclusion. What happens if you have a 32-bit application >> running on a platform with 48-bit GPU address space? > > CC'ing Ben, who knows all the gory details. > > I don't really understand the problem - the GPU uses 48-bit addressing, and > can access more than 4G...but we're talking about map, which makes a buffer > available in an application's virtual address space...which is 32-bit in your > example. It should always be placed at a < 4GB virtual address and work out > fine. Right. I must have been having a senior moment or something. > That said, I don't think the kernel ever uses >= 4GB address spaces today. > Ben wrote 4GGGTT support and had both kernel and userspace patches to make > it work, but I don't think it ever actually landed. I'm pretty sure > shipping userspace is not quite 48-bit safe - there are a few buffers that > have to be placed < 4GB (some hardware packets only take 32-bit addresses > still), and I don't think any software is in place to make that happen. > > --Ken ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] radeonsi: use minnum and maxnum LLVM intrinsics for MIN and MAX opcodes
From: Marek Olšák
So far it has been compiled into pretty ugly code (8 instructions or so
for either opcode).
---
src/gallium/drivers/radeonsi/si_shader.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/src/gallium/drivers/radeonsi/si_shader.c
b/src/gallium/drivers/radeonsi/si_shader.c
index ee08d1a..973bac2 100644
--- a/src/gallium/drivers/radeonsi/si_shader.c
+++ b/src/gallium/drivers/radeonsi/si_shader.c
@@ -2792,6 +2792,13 @@ int si_shader_create(struct si_screen *sscreen, struct
si_shader *shader)
bld_base->op_actions[TGSI_OPCODE_EMIT].emit = si_llvm_emit_vertex;
bld_base->op_actions[TGSI_OPCODE_ENDPRIM].emit = si_llvm_emit_primitive;
+ if (HAVE_LLVM >= 0x0306) {
+ bld_base->op_actions[TGSI_OPCODE_MAX].emit =
build_tgsi_intrinsic_nomem;
+ bld_base->op_actions[TGSI_OPCODE_MAX].intr_name =
"llvm.maxnum.f32";
+ bld_base->op_actions[TGSI_OPCODE_MIN].emit =
build_tgsi_intrinsic_nomem;
+ bld_base->op_actions[TGSI_OPCODE_MIN].intr_name =
"llvm.minnum.f32";
+ }
+
si_shader_ctx.radeon_bld.load_system_value = declare_system_value;
si_shader_ctx.tokens = sel->tokens;
tgsi_parse_init(&si_shader_ctx.parse, si_shader_ctx.tokens);
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 86480] Mesa 10.4.0-devel implementation error: unexpected format GL_DEPTH24_STENCIL8 in _mesa_choose_tex_format()
https://bugs.freedesktop.org/show_bug.cgi?id=86480 Benjamin Bellec changed: What|Removed |Added CC||b.bel...@gmail.com -- You are receiving this mail because: You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] radeonsi: use minnum and maxnum LLVM intrinsics for MIN and MAX opcodes
On Thu, Nov 20, 2014 at 10:21:07PM +0100, Marek Olšák wrote:
> From: Marek Olšák
>
Reviewed-by: Tom Stellard
> So far it has been compiled into pretty ugly code (8 instructions or so
> for either opcode).
> ---
> src/gallium/drivers/radeonsi/si_shader.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/src/gallium/drivers/radeonsi/si_shader.c
> b/src/gallium/drivers/radeonsi/si_shader.c
> index ee08d1a..973bac2 100644
> --- a/src/gallium/drivers/radeonsi/si_shader.c
> +++ b/src/gallium/drivers/radeonsi/si_shader.c
> @@ -2792,6 +2792,13 @@ int si_shader_create(struct si_screen *sscreen, struct
> si_shader *shader)
> bld_base->op_actions[TGSI_OPCODE_EMIT].emit = si_llvm_emit_vertex;
> bld_base->op_actions[TGSI_OPCODE_ENDPRIM].emit = si_llvm_emit_primitive;
>
> + if (HAVE_LLVM >= 0x0306) {
> + bld_base->op_actions[TGSI_OPCODE_MAX].emit =
> build_tgsi_intrinsic_nomem;
> + bld_base->op_actions[TGSI_OPCODE_MAX].intr_name =
> "llvm.maxnum.f32";
> + bld_base->op_actions[TGSI_OPCODE_MIN].emit =
> build_tgsi_intrinsic_nomem;
> + bld_base->op_actions[TGSI_OPCODE_MIN].intr_name =
> "llvm.minnum.f32";
> + }
> +
> si_shader_ctx.radeon_bld.load_system_value = declare_system_value;
> si_shader_ctx.tokens = sel->tokens;
> tgsi_parse_init(&si_shader_ctx.parse, si_shader_ctx.tokens);
> --
> 2.1.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/9] i915: Only use TEXCOORDTYPE_VECTOR with cube maps on gen2
On 11/20/2014 11:13 AM, Ville Syrjälä wrote: > On Thu, Nov 20, 2014 at 09:59:00AM -0800, Ian Romanick wrote: >> On 08/06/2014 11:56 AM, ville.syrj...@linux.intel.com wrote: >>> From: Ville Syrjälä >>> >>> Check that the target is GL_TEXTURE_CUBE_MAP before emitting >>> TEXCOORDTYPE_VECTOR texture coordinates. >>> >>> I'm not sure if the hardware would like CARTESIAN coordinates >>> with cube maps, and as I'm too lazy to find out just emit the >>> VECTOR coordinates for cube maps always. For other targets use >>> CARTESIAN or HOMOGENOUS depending on the number of texture >>> coordinates provided. >>> >>> Fixes rendering of the "electric" background texture in chromium-bsu >>> main menu. We appear to be provided with three texture coordinates >>> there (I'm guessing due to the funky texture matrix rotation it does). >>> So the code would decide to use TEXCOORDTYPE_VECTOR instead of >>> TEXCOORDTYPE_CARTESIAN even though we're dealing with a 2D texure. >>> The results weren't what one might expect. >>> >>> demos/cubemap still works, which hopefully indicates that this doesn't >>> break things. >> >> I won't dare ask about a full piglit run on that hardware, > > I did actually try to run piglit on the 830, but it always > seemed to die in some X blit batch after a while :( > > There have been some recent blt TLB workaround fixes in the > kernel though, so perhaps things are better now. And if not, > I do have a 855 that ought to be a bit less fragile. So > maybe I'll give it another go just for kicks :P > >> but how about >> >> bin/glean -o -v -v -v -t +texCube --quick >> >> and >> >> bin/cubemap -auto >> >> from piglit? > > pass->pass on both counts. > >> >> These changes seem reasonable, and assuming those tests aren't made worse, >> >> Reviewed-by: Ian Romanick >> >> If you're excited about GEN2 bugs, wanna take a look at 79841? >:) > > I'm thinking it should be fixed by: > > commit dafae910d4fc791ba49f20e937cb918669f42944 > Author: Ville Syrjälä > Date: Thu Jul 3 15:38:07 2014 +0300 > > i915: Accept GL_DEPTH_STENCIL GL_DEPTH_COMPONENT formats for > renderbuffers > > I'll note it in the bug. I only brought it up because there's another report (bug #86480) of what appears to be the same issue. The reporter says it's on 10.4.x. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/3] nine: Drop use of TGSI_OPCODE_CND.
From: Jose Fonseca
This was the only state tracker emitting it, and hardware was just having
to lower it anyway (or failing to lower it at all).
v2: Extracted from a larger patch by Jose (which also dropped DP2A), fixed
to actually not reference TGSI_OPCODE_CND. Change by anholt.
---
src/gallium/state_trackers/nine/nine_shader.c | 10 +-
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/src/gallium/state_trackers/nine/nine_shader.c
b/src/gallium/state_trackers/nine/nine_shader.c
index 85cc190..268612e 100644
--- a/src/gallium/state_trackers/nine/nine_shader.c
+++ b/src/gallium/state_trackers/nine/nine_shader.c
@@ -1374,7 +1374,6 @@ DECL_SPECIAL(CND)
}
cnd = tx_src_param(tx, &tx->insn.src[0]);
-#ifdef NINE_TGSI_LAZY_R600
cgt = tx_scratch(tx);
if (tx->version.major == 1 && tx->version.minor < 4) {
@@ -1387,13 +1386,6 @@ DECL_SPECIAL(CND)
ureg_CMP(tx->ureg, dst,
tx_src_param(tx, &tx->insn.src[1]),
tx_src_param(tx, &tx->insn.src[2]), ureg_negate(cnd));
-#else
-if (tx->version.major == 1 && tx->version.minor < 4)
-cnd = ureg_scalar(cnd, TGSI_SWIZZLE_W);
-ureg_CND(tx->ureg, dst,
- tx_src_param(tx, &tx->insn.src[1]),
- tx_src_param(tx, &tx->insn.src[2]), cnd);
-#endif
return D3D_OK;
}
@@ -2356,7 +2348,7 @@ struct sm1_op_info inst_table[] =
_OPI(EXPP, EXP, V(0,0), V(1,1), V(0,0), V(0,0), 1, 1, NULL),
_OPI(EXPP, EX2, V(2,0), V(3,0), V(0,0), V(0,0), 1, 1, NULL),
_OPI(LOGP, LG2, V(0,0), V(3,0), V(0,0), V(0,0), 1, 1, NULL),
-_OPI(CND, CND, V(0,0), V(0,0), V(0,0), V(1,4), 1, 3, SPECIAL(CND)),
+_OPI(CND, NOP, V(0,0), V(0,0), V(0,0), V(1,4), 1, 3, SPECIAL(CND)),
_OPI(DEF, NOP, V(0,0), V(3,0), V(0,0), V(3,0), 1, 0, SPECIAL(DEF)),
--
2.1.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/3] nine: Don't reference the dead TGSI_OPCODE_NRM.
From: Jose Fonseca The translation is lowering it to not using TGSI_OPCODE_NRM, anyway. v2: Extracted from a larger patch by Jose that also dropped DP2A usage. --- src/gallium/state_trackers/nine/nine_shader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/state_trackers/nine/nine_shader.c b/src/gallium/state_trackers/nine/nine_shader.c index 9b324c3..85cc190 100644 --- a/src/gallium/state_trackers/nine/nine_shader.c +++ b/src/gallium/state_trackers/nine/nine_shader.c @@ -2316,7 +2316,7 @@ struct sm1_op_info inst_table[] = _OPI(CRS, XPD, V(0,0), V(3,0), V(0,0), V(3,0), 1, 2, NULL), /* XXX: .w */ _OPI(SGN, SSG, V(2,0), V(3,0), V(0,0), V(0,0), 1, 3, SPECIAL(SGN)), /* ignore src1,2 */ _OPI(ABS, ABS, V(0,0), V(3,0), V(0,0), V(3,0), 1, 1, NULL), -_OPI(NRM, NRM, V(0,0), V(3,0), V(0,0), V(3,0), 1, 1, SPECIAL(NRM)), /* NRM doesn't fit */ +_OPI(NRM, NOP, V(0,0), V(3,0), V(0,0), V(3,0), 1, 1, SPECIAL(NRM)), /* NRM doesn't fit */ _OPI(SINCOS, SCS, V(2,0), V(2,1), V(2,0), V(2,1), 1, 3, SPECIAL(SINCOS)), _OPI(SINCOS, SCS, V(3,0), V(3,0), V(3,0), V(3,0), 1, 1, SPECIAL(SINCOS)), -- 2.1.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/3] nine: Don't use the otherwise-dead SFL opcode in an unreachable path.
--- src/gallium/state_trackers/nine/nine_shader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/state_trackers/nine/nine_shader.c b/src/gallium/state_trackers/nine/nine_shader.c index cc027b4..9b324c3 100644 --- a/src/gallium/state_trackers/nine/nine_shader.c +++ b/src/gallium/state_trackers/nine/nine_shader.c @@ -1615,7 +1615,7 @@ sm1_insn_flags_to_tgsi_setop(BYTE flags) case NINED3DSHADER_REL_OP_LE: return TGSI_OPCODE_SLE; default: assert(!"invalid comparison flags"); -return TGSI_OPCODE_SFL; +return TGSI_OPCODE_SGT; } } -- 2.1.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] nine: Don't use the otherwise-dead SFL opcode in an unreachable path.
Hi, Series looks good. You can add my r-b. Do you want also to remove the DP2A reference like did Jose patch ? Axel Davy On 20/11/2014 23:31, Eric Anholt wrote : --- src/gallium/state_trackers/nine/nine_shader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/state_trackers/nine/nine_shader.c b/src/gallium/state_trackers/nine/nine_shader.c index cc027b4..9b324c3 100644 --- a/src/gallium/state_trackers/nine/nine_shader.c +++ b/src/gallium/state_trackers/nine/nine_shader.c @@ -1615,7 +1615,7 @@ sm1_insn_flags_to_tgsi_setop(BYTE flags) case NINED3DSHADER_REL_OP_LE: return TGSI_OPCODE_SLE; default: assert(!"invalid comparison flags"); -return TGSI_OPCODE_SFL; +return TGSI_OPCODE_SGT; } } ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] nine: Drop use of TGSI_OPCODE_CND.
Reviewed-by: Jose Fonseca
for the series.
Jose
On 20/11/14 22:31, Eric Anholt wrote:
From: Jose Fonseca
This was the only state tracker emitting it, and hardware was just having
to lower it anyway (or failing to lower it at all).
v2: Extracted from a larger patch by Jose (which also dropped DP2A), fixed
to actually not reference TGSI_OPCODE_CND. Change by anholt.
---
src/gallium/state_trackers/nine/nine_shader.c | 10 +-
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/src/gallium/state_trackers/nine/nine_shader.c
b/src/gallium/state_trackers/nine/nine_shader.c
index 85cc190..268612e 100644
--- a/src/gallium/state_trackers/nine/nine_shader.c
+++ b/src/gallium/state_trackers/nine/nine_shader.c
@@ -1374,7 +1374,6 @@ DECL_SPECIAL(CND)
}
cnd = tx_src_param(tx, &tx->insn.src[0]);
-#ifdef NINE_TGSI_LAZY_R600
cgt = tx_scratch(tx);
if (tx->version.major == 1 && tx->version.minor < 4) {
@@ -1387,13 +1386,6 @@ DECL_SPECIAL(CND)
ureg_CMP(tx->ureg, dst,
tx_src_param(tx, &tx->insn.src[1]),
tx_src_param(tx, &tx->insn.src[2]), ureg_negate(cnd));
-#else
-if (tx->version.major == 1 && tx->version.minor < 4)
-cnd = ureg_scalar(cnd, TGSI_SWIZZLE_W);
-ureg_CND(tx->ureg, dst,
- tx_src_param(tx, &tx->insn.src[1]),
- tx_src_param(tx, &tx->insn.src[2]), cnd);
-#endif
return D3D_OK;
}
@@ -2356,7 +2348,7 @@ struct sm1_op_info inst_table[] =
_OPI(EXPP, EXP, V(0,0), V(1,1), V(0,0), V(0,0), 1, 1, NULL),
_OPI(EXPP, EX2, V(2,0), V(3,0), V(0,0), V(0,0), 1, 1, NULL),
_OPI(LOGP, LG2, V(0,0), V(3,0), V(0,0), V(0,0), 1, 1, NULL),
-_OPI(CND, CND, V(0,0), V(0,0), V(0,0), V(1,4), 1, 3, SPECIAL(CND)),
+_OPI(CND, NOP, V(0,0), V(0,0), V(0,0), V(1,4), 1, 3, SPECIAL(CND)),
_OPI(DEF, NOP, V(0,0), V(3,0), V(0,0), V(3,0), 1, 0, SPECIAL(DEF)),
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 6/7] i965: Just return void from brw_try_draw_prims
On Thursday, November 20, 2014 11:14:54 AM Ian Romanick wrote:
> From: Ian Romanick
Suggested commit message addition:
We used to use the return value to indicate whether software fallbacks
were necessary, but we haven't in years.
Reviewed-by: Kenneth Graunke
> ---
> src/mesa/drivers/dri/i965/brw_draw.c | 7 ++-
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_draw.c
> b/src/mesa/drivers/dri/i965/brw_draw.c
> index b28eaf2..4c2802a 100644
> --- a/src/mesa/drivers/dri/i965/brw_draw.c
> +++ b/src/mesa/drivers/dri/i965/brw_draw.c
> @@ -347,7 +347,7 @@ static void brw_postdraw_set_buffers_need_resolve(struct
> brw_context *brw)
> /* May fail if out of video memory for texture or vbo upload, or on
> * fallback conditions.
> */
> -static bool brw_try_draw_prims( struct gl_context *ctx,
> +static void brw_try_draw_prims( struct gl_context *ctx,
>const struct gl_client_array *arrays[],
>const struct _mesa_prim *prims,
>GLuint nr_prims,
> @@ -357,7 +357,6 @@ static bool brw_try_draw_prims( struct gl_context *ctx,
>struct gl_buffer_object *indirect)
> {
> struct brw_context *brw = brw_context(ctx);
> - bool retval = true;
> GLuint i;
> bool fail_next = false;
>
> @@ -487,8 +486,6 @@ retry:
> "available aperture space\n");
> warned = true;
> }
> -
> -retval = false;
> }
>}
>}
> @@ -506,7 +503,7 @@ retry:
> brw_state_cache_check_size(brw);
> brw_postdraw_set_buffers_need_resolve(brw);
>
> - return retval;
> + return;
> }
>
> void brw_draw_prims( struct gl_context *ctx,
>
signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/7] valid-to-render house keeping
On Thursday, November 20, 2014 11:14:48 AM Ian Romanick wrote: > This little series is just a bit of house keeping in the valid-to-render > path. There is a follow-up series in progress that tries to optimize > some of these paths for CPU-bound workloads. > > The whole series has been reordered several times. Orignally patches 4 > and 5 of this series were farther apart. As it stands now, they should > probably be squashed (taking the commit message from patch 5). I would not squash them. One is purely stylistic, and the other changes assert to unreachable. I don't feel strongly though. Patches 2-7 are: Reviewed-by: Kenneth Graunke > > FWIW... I was not able to measure any performance delta on any test on > any platform at even 80% confidence across this series. These changes are so small I would expect them to be basically unmeasurable. They're worthwhile anyway since they clean up the codebase. --Ken signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/7] mesa: Generate GL_INVALID_OPERATION when drawing w/o a VAO in core profile
On Thursday, November 20, 2014 11:14:49 AM Ian Romanick wrote:
> From: Ian Romanick
>
> GL 3-ish versions of the spec are less clear that an error should be
> generated here, so Ken (and I during review) just missed it in 1afe335.
>
> Signed-off-by: Ian Romanick
> Cc: Kenneth Graunke
> ---
> src/mesa/main/api_validate.c | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
> index bf4fa3e..006fca4 100644
> --- a/src/mesa/main/api_validate.c
> +++ b/src/mesa/main/api_validate.c
> @@ -79,8 +79,16 @@ check_valid_to_render(struct gl_context *ctx, const char
> *function)
>break;
>
> case API_OPENGL_CORE:
> - if (ctx->Array.VAO == ctx->Array.DefaultVAO)
> + /* Section 10.4 (Drawing Commands Using Vertex Arrays) of the OpenGL
> 4.5
> + * Core Profile spec says:
> + *
> + * "An INVALID_OPERATION error is generated if no vertex array
> + * object is bound (see section 10.3.1)."
> + */
> + if (ctx->Array.VAO == ctx->Array.DefaultVAO) {
> + _mesa_error(ctx, GL_INVALID_OPERATION, "%s(no VAO bound)",
> function);
> return GL_FALSE;
> + }
>/* fallthrough */
> case API_OPENGL_COMPAT:
>{
This seems okay - we were already prohibiting drawing, you're just adding a GL
error.
I thought we already did that, but I can't find any code to do so.
Reviewed-by: Kenneth Graunke
That said, I don't think we ever resolved whether prohibiting drawing is
correct,
given that we support ARB_ES3_compatibility, and ES3 allows this.
signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 03/29] mesa: Do not assert on integer<->non-integer direct pack/unpack fast paths
On Thu, Nov 20, 2014 at 12:29 AM, Iago Toral wrote:
> It is explained here:
> https://bugs.freedesktop.org/show_bug.cgi?id=84566#c35
>
> So one example of this was a glReadPixels where we want to store the
> pixel data as RGBA UINT, but the render buffer format we read from is
> MESA_FORMAT_B8G8R8A8_UNORM. There are piglit tests that hit this case.
>
I'm still not seeing how this is allowed. From the 4.2 core spec:
"If format is one of RED , GREEN , BLUE , RG , RGB , RGBA , BGR , or BGRA ,
then
red, green, blue, and alpha values are obtained from the selected buffer at
each
pixel location.
If format is an integer format and the color buffer is not an integer
format, or
if the color buffer is an integer format and format is not an integer
format, an
INVALID_OPERATION error is generated."
I also checked the 3.3 compatibility spec and it says the same thing. This
seems to indicate that that combination should result in
GL_INVALID_OPERATION.
>
> Iago
>
> On Wed, 2014-11-19 at 12:04 -0800, Jason Ekstrand wrote:
> > Can you remind me again as to what formats hit these paths? I
> > remember you hitting them, but I'm still not really seeing how it
> > happens.
> >
> > --Jason
> >
> >
> > On Tue, Nov 18, 2014 at 1:23 AM, Iago Toral Quiroga
> > wrote:
> > We can have conversions from non-integer types to integer
> > types, so remove
> > the assertions for these in the pack/unpack fast paths. It
> > could be that
> > we do not have all the necessary pack/unpack functions in
> > these cases though,
> > so protect these paths with conditionals and let
> > _mesa_format_convert use
> > other paths to resolve these kind of conversions if necessary.
> > ---
> > src/mesa/main/format_utils.c | 16
> > 1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/src/mesa/main/format_utils.c
> > b/src/mesa/main/format_utils.c
> > index 1d65f2b..56a3b8d 100644
> > --- a/src/mesa/main/format_utils.c
> > +++ b/src/mesa/main/format_utils.c
> > @@ -143,8 +143,8 @@ _mesa_format_convert(void *void_dst,
> > uint32_t dst_format, size_t dst_stride,
> > dst += dst_stride;
> > }
> > return;
> > - } else if (dst_array_format.as_uint ==
> > RGBA_UBYTE.as_uint) {
> > - assert(!_mesa_is_format_integer_color(src_format));
> > + } else if (dst_array_format.as_uint ==
> > RGBA_UBYTE.as_uint &&
> > + !_mesa_is_format_integer_color(src_format))
> > {
> > for (row = 0; row < height; ++row) {
> > _mesa_unpack_ubyte_rgba_row(src_format, width,
> > src, (uint8_t
> > (*)[4])dst);
> > @@ -152,8 +152,8 @@ _mesa_format_convert(void *void_dst,
> > uint32_t dst_format, size_t dst_stride,
> > dst += dst_stride;
> > }
> > return;
> > - } else if (dst_array_format.as_uint ==
> > RGBA_UINT.as_uint) {
> > - assert(_mesa_is_format_integer_color(src_format));
> > + } else if (dst_array_format.as_uint ==
> > RGBA_UINT.as_uint &&
> > + _mesa_is_format_integer_color(src_format)) {
> > for (row = 0; row < height; ++row) {
> > _mesa_unpack_uint_rgba_row(src_format, width,
> > src, (uint32_t
> > (*)[4])dst);
> > @@ -174,8 +174,8 @@ _mesa_format_convert(void *void_dst,
> > uint32_t dst_format, size_t dst_stride,
> > dst += dst_stride;
> > }
> > return;
> > - } else if (src_array_format.as_uint ==
> > RGBA_UBYTE.as_uint) {
> > - assert(!_mesa_is_format_integer_color(dst_format));
> > + } else if (src_array_format.as_uint ==
> > RGBA_UBYTE.as_uint &&
> > + !_mesa_is_format_integer_color(dst_format))
> > {
> > for (row = 0; row < height; ++row) {
> > _mesa_pack_ubyte_rgba_row(dst_format, width,
> >(const uint8_t
> > (*)[4])src, dst);
> > @@ -183,8 +183,8 @@ _mesa_format_convert(void *void_dst,
> > uint32_t dst_format, size_t dst_stride,
> > dst += dst_stride;
> > }
> > return;
> > - } else if (src_array_format.as_uint ==
> > RGBA_UINT.as_uint) {
> > - assert(_mesa_is_format_integer_color(dst_format));
> > + } else if (src_array_format.as_uint ==
> > RGBA_UINT.as_uint &&
> > +
Re: [Mesa-dev] [PATCH 03/29] mesa: Do not assert on integer<->non-integer direct pack/unpack fast paths
On Thu, Nov 20, 2014 at 9:33 PM, Jason Ekstrand
wrote:
>
>
> On Thu, Nov 20, 2014 at 12:29 AM, Iago Toral wrote:
>
>> It is explained here:
>> https://bugs.freedesktop.org/show_bug.cgi?id=84566#c35
>>
>> So one example of this was a glReadPixels where we want to store the
>> pixel data as RGBA UINT, but the render buffer format we read from is
>> MESA_FORMAT_B8G8R8A8_UNORM. There are piglit tests that hit this case.
>>
>
> I'm still not seeing how this is allowed. From the 4.2 core spec:
>
> "If format is one of RED , GREEN , BLUE , RG , RGB , RGBA , BGR , or BGRA
> , then
> red, green, blue, and alpha values are obtained from the selected buffer
> at each
> pixel location.
> If format is an integer format and the color buffer is not an integer
> format, or
> if the color buffer is an integer format and format is not an integer
> format, an
> INVALID_OPERATION error is generated."
>
> I also checked the 3.3 compatibility spec and it says the same thing.
> This seems to indicate that that combination should result in
> GL_INVALID_OPERATION.
>
I also just CC'd Ian, our local spec expert. Maybe he can shed a little
light on this.
>
>> Iago
>>
>> On Wed, 2014-11-19 at 12:04 -0800, Jason Ekstrand wrote:
>> > Can you remind me again as to what formats hit these paths? I
>> > remember you hitting them, but I'm still not really seeing how it
>> > happens.
>> >
>> > --Jason
>> >
>> >
>> > On Tue, Nov 18, 2014 at 1:23 AM, Iago Toral Quiroga
>> > wrote:
>> > We can have conversions from non-integer types to integer
>> > types, so remove
>> > the assertions for these in the pack/unpack fast paths. It
>> > could be that
>> > we do not have all the necessary pack/unpack functions in
>> > these cases though,
>> > so protect these paths with conditionals and let
>> > _mesa_format_convert use
>> > other paths to resolve these kind of conversions if necessary.
>> > ---
>> > src/mesa/main/format_utils.c | 16
>> > 1 file changed, 8 insertions(+), 8 deletions(-)
>> >
>> > diff --git a/src/mesa/main/format_utils.c
>> > b/src/mesa/main/format_utils.c
>> > index 1d65f2b..56a3b8d 100644
>> > --- a/src/mesa/main/format_utils.c
>> > +++ b/src/mesa/main/format_utils.c
>> > @@ -143,8 +143,8 @@ _mesa_format_convert(void *void_dst,
>> > uint32_t dst_format, size_t dst_stride,
>> > dst += dst_stride;
>> > }
>> > return;
>> > - } else if (dst_array_format.as_uint ==
>> > RGBA_UBYTE.as_uint) {
>> > - assert(!_mesa_is_format_integer_color(src_format));
>> > + } else if (dst_array_format.as_uint ==
>> > RGBA_UBYTE.as_uint &&
>> > + !_mesa_is_format_integer_color(src_format))
>> > {
>> > for (row = 0; row < height; ++row) {
>> > _mesa_unpack_ubyte_rgba_row(src_format, width,
>> > src, (uint8_t
>> > (*)[4])dst);
>> > @@ -152,8 +152,8 @@ _mesa_format_convert(void *void_dst,
>> > uint32_t dst_format, size_t dst_stride,
>> > dst += dst_stride;
>> > }
>> > return;
>> > - } else if (dst_array_format.as_uint ==
>> > RGBA_UINT.as_uint) {
>> > - assert(_mesa_is_format_integer_color(src_format));
>> > + } else if (dst_array_format.as_uint ==
>> > RGBA_UINT.as_uint &&
>> > + _mesa_is_format_integer_color(src_format)) {
>> > for (row = 0; row < height; ++row) {
>> > _mesa_unpack_uint_rgba_row(src_format, width,
>> > src, (uint32_t
>> > (*)[4])dst);
>> > @@ -174,8 +174,8 @@ _mesa_format_convert(void *void_dst,
>> > uint32_t dst_format, size_t dst_stride,
>> > dst += dst_stride;
>> > }
>> > return;
>> > - } else if (src_array_format.as_uint ==
>> > RGBA_UBYTE.as_uint) {
>> > - assert(!_mesa_is_format_integer_color(dst_format));
>> > + } else if (src_array_format.as_uint ==
>> > RGBA_UBYTE.as_uint &&
>> > + !_mesa_is_format_integer_color(dst_format))
>> > {
>> > for (row = 0; row < height; ++row) {
>> > _mesa_pack_ubyte_rgba_row(dst_format, width,
>> >(const uint8_t
>> > (*)[4])src, dst);
>> > @@ -183,8 +183,8 @@ _mesa_format_convert(void *void_dst,
>> > uint32_t dst_format, size_t dst_stride,
>> > dst += dst_stride;
>> > }
>> > return;
>> >
Re: [Mesa-dev] [PATCH 06/29] mesa: Avoid pack/unpack fast paths if base internal format != base format
On Thu, 2014-11-20 at 10:49 -0800, Jason Ekstrand wrote: > > > On Wed, Nov 19, 2014 at 11:58 PM, Iago Toral > wrote: > On Wed, 2014-11-19 at 11:57 -0800, Jason Ekstrand wrote: > > A couple of general comments on this patch: > > > > > > 1) The prerequisites should be moved to before the first > patch in the > > series and it should be squashed into the patch that > introduces the > > function. There are one or two more patches which also > modify it and > > those should also be squashed in. > > Ok. > > > > > 2) I wonder if giving _mesa_format_convert an internal > swizzle > > wouldn't be better than a destination internal format. > There are a > > couple of reasons for this: > > > > a) It's more general. If it doesn't cost us anything > extra to do > > it that way, we might as well. > > I think that would only work directly for conversions between > array > formats, but what if we have, for example, a texture upload > from RGB565 > to a Luminance format (so that we get an RGBA base format)? > that would > not go though _mesa_swizzle_and_convert and would require to > unpack to > RGBA and then pack to the dst... and unless the client has > provided the > dst format as an array format that won't use > _mesa_swizzle_and_convert > either. That should not be a problem when the calls to > _mesa_format_convert come from things like glTexImage or > glReadPixels, > since in these cases the compute the dst format from a GL type > and if it > is an array format we should get that, but in other cases that > might not > be the case... > > > I'm a bit confused about what you mean here. If the user passes in a > non-trivial swizzle and we have to pack and unpack on both sides then > we have to unpack, swizzle, and then repack. We would still have to > do this if all you pass in is an internal format. I was confused by the fact that we are currently not doing this in all paths (I mean, swizzling after unpacking if necessary), but you are right. > Fortunately, the _mesa_swizzle_and_convert function can be used to do > an in-place swizzle as long as the source and destination types have > the same number of bits per pixel. > > If one side of the pack/repack is an array format, we can just build > the swizzling into the one _mesa_swizzle_and_convert call. Yes, I get your point now. Thanks! > > > b) It removes the GL enum dependency from the > _mesa_format_convert > > c) I think this implementation misses the case where we > download a > > texture that is storred in a different format than its base > format. > > For instance, if you are downloading a GL_RGB texture as > GL_RGBA but > > it's storred as GL_RGBA. I think with the current > implementaion, > > you'll get the junk in the alpha component of the texture's > backing > > storage instead of a constant alpha value of 1. > > That's correct. In the implementation of readpixels and > getteximage we > had to rebase the results in some cases to account for that. > > Iago > > > > > On Tue, Nov 18, 2014 at 1:23 AM, Iago Toral Quiroga > > wrote: > > In general, if the dst base internal format and the > selected > > dst format are > > different we can't be sure that directly packing or > unpacking > > to the destination > > format will produce correct results. In these cases > we > > probably want to fall > > back to other paths (like mesa_swizzle_and_convert) > that can > > handle these > > situations better. > > > > An example of this includes a luminance internal > format for > > which the driver > > decided to use something like BGRA. In these case, > unpacking > > to BGRA won't > > match the requirements of the luminance internal > format. > > > > In the future we may want to allow these fast paths > for > > specific cases > > where we know the direct pack/unpack functions will > do the > > right thing. > > --- > > src/mesa/main/format_utils.c | 137 > > +++ > > 1 file changed, 72 insertions(+), 65 deletions(-) > > >
Re: [Mesa-dev] [PATCH 02/29] mesa: Set normalized=true for float array formats.
On Thu, 2014-11-20 at 10:40 -0800, Jason Ekstrand wrote:
>
>
> On Wed, Nov 19, 2014 at 11:24 PM, Iago Toral
> wrote:
> Hi Jason,
>
> we discussed this some weeks ago actually, the detailed
> explanation is
> here:
> https://bugs.freedesktop.org/show_bug.cgi?id=84566#c5
>
> the short answer is that this is necessary because there is a
> normalized
> parameter to _mesa_swizzle_and_convert, and when we deal with
> float
> types we want to set this to true.
>
>
> I went back and looked at that and I thought the result of the
> discussion was to fix the assert in mesa_format_convert and compute
> the normalized parameter correctly. After that, I thought this
> shouldn't be strictly needed. It may still be a good idea for
> consistency, but I want to make sure we're doing the right thing in
> mesa_format_convert
With this patch, in mesa_format_convert we simply take the "normalized"
value for mesa_swizzle_and_convert from the normalized field of the
array format, since we make sure that all float array formats will have
this set to 1.
Without this patch we would have to do something like this (pseudocode)
in mesa_format_convert:
normalized = array_format.normalized || array_format.type == FLOAT ||
array_format.type == HALF_FLOAT;
We can do it either way, I just think that the latter is a bit
inconsistent because:
a) why would we want to generate array formats with a normalized setting
of 0 if we then want to set normalized to true when they are involved?.
b) Other parts of Mesa check if a format is normalized by doing
normalized = !_mesa_is_enum_format_integer(srcFormat), which will make
float types normalized.
Iago
> Iago
>
> On Wed, 2014-11-19 at 11:31 -0800, Jason Ekstrand wrote:
> > I'm not sure what I think about this. Does it make a
> functional
> > change other than consistancy?
> >
> > --Jason
> >
> >
> > On Tue, Nov 18, 2014 at 1:23 AM, Iago Toral Quiroga
> > wrote:
> > In order to check if a format is normalized Mesa
> does
> > something like this:
> > normalized = !
> _mesa_is_enum_format_integer(srcFormat);
> >
> > So all float types will set normalized to true.
> Since our
> > mesa_array_format
> > includes a normalized flag for each type we want to
> make it
> > consistent with
> > this.
> > ---
> > src/mesa/main/format_info.py | 3 ++-
> > src/mesa/main/format_utils.c | 2 +-
> > 2 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/mesa/main/format_info.py
> > b/src/mesa/main/format_info.py
> > index 315767d..d4bc276 100644
> > --- a/src/mesa/main/format_info.py
> > +++ b/src/mesa/main/format_info.py
> > @@ -220,9 +220,10 @@ for fmat in formats:
> > print ' {{ {0} }},'.format(',
> '.join(map(str,
> > fmat.swizzle)))
> > if fmat.is_array() and fmat.colorspace in
> ('rgb', 'srgb'):
> >chan = fmat.array_element()
> > + norm = chan.norm or chan.type == parser.FLOAT
> >print ' {0} ,'.format(',
> '.join([
> > get_array_format_datatype(chan),
> > - str(int(chan.norm)),
> > + str(int(norm)),
> > str(len(fmat.channels)),
> > str(fmat.swizzle[0]),
> > str(fmat.swizzle[1]),
> > diff --git a/src/mesa/main/format_utils.c
> > b/src/mesa/main/format_utils.c
> > index c3815cb..1d65f2b 100644
> > --- a/src/mesa/main/format_utils.c
> > +++ b/src/mesa/main/format_utils.c
> > @@ -30,7 +30,7 @@
> >
> > mesa_array_format RGBA_FLOAT = {{
> > MESA_ARRAY_FORMAT_TYPE_FLOAT,
> > - 0,
> > + 1,
> > 4,
> > 0, 1, 2, 3,
> > 0, 1
> > --
> > 1.9.1
> >
> > ___
> > 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.freede
