Re: [Mesa-dev] i965 GL_ARB_buffer_storage

[Kenneth Graunke]

On 02/27/2014 02:52 PM, Eric Anholt wrote:
> One thing I noticed while working on this was that we only reallocate buffer
> storage for INVALIDATE_BUFFER_BIT when UNSYNCHRONIZED_BIT is unset.  The
> ARB_mbr spec says that the contents "may be discarded", not "must be
> discarded".  However, while writing the glamor code I happened to type this
> for the wraparound case:
> 
> glamor_priv->vb = glMapBufferRange(GL_ARRAY_BUFFER,
>0, size,
>GL_MAP_WRITE_BIT |
>GL_MAP_INVALIDATE_BUFFER_BIT |
>GL_MAP_UNSYNCHRONIZED_BIT |
>GL_MAP_PERSISTENT_BIT |
>GL_MAP_COHERENT_BIT);
> 
> intending that the buffer storage get reallocated, and that we not worry about
> any synchronization after that.  My code would have been broken on the i965
> driver.  I'm wondering if this is the intended behavior of the spec, or if we
> want to treat the "may" as a "must".

I don't understand.  Your code here sets GL_MAP_INVALIDATE_BUFFER_BIT
and GL_MAP_UNSYNCHRONIZED_BIT, which would have reallocated the storage,
as you intended.  Was this a mispaste - you originally lacked the
GL_MAP_UNSYNCHRONIZED_BIT, were surprised, and eventually added it?

I've had several branches to throw out the storage when
GL_MAP_INVALIDATE_BUFFER_BIT was present, regardless of whether
UNSYNCHRONIZED_BIT was set.  I think it's a great idea.  I just couldn't
find any applications that hit that case, so I couldn't demonstrate an
improvement.

Of course, as Jose pointed out, there's no point in reallocating the
storage if it isn't busy.

--Ken






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

Re: [Mesa-dev] [PATCH 2/7] i965/fs: Save push constant location information.

[Kenneth Graunke]

On 03/13/2014 05:55 PM, Pohjolainen, Topi wrote:
> On Tue, Mar 11, 2014 at 11:48:51PM -0700, Kenneth Graunke wrote:
>> Previously, both move_uniform_array_access_to_pull_constants() and
>> setup_pull_constants() maintained stack-local arrays with this
>> information.  Storing this information will allow it to be used from
>> multiple functions, allowing us to split and move code around.
>>
>> We'll also eventually want to pass pull constant location information
>> to the SIMD16 compile.  Saving this information will help us do that.
>>
>> Unfortunately, the two functions *cannot* share the contents of the
>> array just yet.  remove_dead_constants() renumbers all the UNIFORM
>> registers to be contiguous starting at zero, so the two functions
>> talk about uniforms using different names.  We can't even remap them,
>> since move_uniform_array_access_to_pull_constants() deletes UNIFORM
>> registers that are only accessed with reladdr, so remove_dead_constants
>> can't even see them.
>>
>> This situation will improve in the next few patches.
>>
>> Signed-off-by: Kenneth Graunke 
>> ---
>>  src/mesa/drivers/dri/i965/brw_fs.cpp | 7 +--
>>  src/mesa/drivers/dri/i965/brw_fs.h   | 6 ++
>>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 1 +
>>  3 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
>> b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> index c24d2f8..8faf401 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> @@ -1827,7 +1827,7 @@ fs_visitor::remove_dead_constants()
>>  void
>>  fs_visitor::move_uniform_array_access_to_pull_constants()
>>  {
>> -   int pull_constant_loc[uniforms];
>> +   pull_constant_loc = ralloc_array(mem_ctx, int, uniforms);
>>  
>> for (unsigned int i = 0; i < uniforms; i++) {
>>pull_constant_loc[i] = -1;
>> @@ -1884,6 +1884,9 @@ 
>> fs_visitor::move_uniform_array_access_to_pull_constants()
>>}
>> }
>> invalidate_live_intervals();
>> +
>> +   ralloc_free(pull_constant_loc);
>> +   pull_constant_loc = NULL;
>>  }
>>  
>>  /**
>> @@ -1909,7 +1912,7 @@ fs_visitor::setup_pull_constants()
>>  */
>> unsigned int pull_uniform_base = max_uniform_components;
>>  
>> -   int pull_constant_loc[uniforms];
>> +   pull_constant_loc = ralloc_array(mem_ctx, int, uniforms);
>> for (unsigned int i = 0; i < uniforms; i++) {
>>if (i < pull_uniform_base) {
>>   pull_constant_loc[i] = -1;
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h 
>> b/src/mesa/drivers/dri/i965/brw_fs.h
>> index 86c95df..abfdb10 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.h
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
>> @@ -509,6 +509,12 @@ public:
>> /** Number of uniform variable components visited. */
>> unsigned uniforms;
>>  
>> +   /**
>> +* Array mapping UNIFORM register numbers to the pull parameter index,
>> +* or -1 if this uniform register isn't being uploaded as a pull 
>> constant.
>> +*/
>> +   int *pull_constant_loc;
>> +
>> /* This is the map from UNIFORM hw_reg + reg_offset as generated by
>>  * the visitor to the packed uniform number after
>>  * remove_dead_constants() that represents the actual uploaded
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp 
>> b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>> index 90272eb..404af30 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>> @@ -2971,6 +2971,7 @@ fs_visitor::fs_visitor(struct brw_context *brw,
>> this->regs_live_at_ip = NULL;
>>  
>> this->uniforms = 0;
>> +   this->pull_constant_loc = NULL;
> 
> Apart from 'dispatch_width' all the other members are initialised here in the
> body of the constructor, so it is consistent to have this here also. I'm just
> thinking if we would like to start using more the initialization list.

I don't have a strong preference.  I feel like it's mostly a stylistic
decision, and one I'm happy to leave up to the author.

--Ken



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

Re: [Mesa-dev] Wrong colors in 3D apps on big-endian systems

[Michel Dänzer]

On Don, 2014-03-13 at 10:38 +0100, Christian Zigotzky wrote:
> On 12.03.2014 05:35, Michel Dänzer wrote:
> >
> > Do the attached patches help?
> 
> Thank you very much for your effort. I've patched the latest git version 
> with your patches. Mesa works with the right colors but it doesn't have 
> 3D acceleration. Mesa 10.1.0 and 10.0.3 works with 3D acceleration.
[...]
> libGL error: failed to load driver: r600

This is probably because r600g claims not to support any of the colour
formats chosen by the Mesa/DRI state tracker.

Did you only apply these two patches, no other modifications to the Git
tree? What if you only apply the r600g patch?


-- 
Earthling Michel Dänzer|  http://www.amd.com
Libre software enthusiast  |Mesa and X developer

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

Re: [Mesa-dev] [PATCH 2/7] i965/fs: Save push constant location information.

[Pohjolainen, Topi]

On Tue, Mar 11, 2014 at 11:48:51PM -0700, Kenneth Graunke wrote:
> Previously, both move_uniform_array_access_to_pull_constants() and
> setup_pull_constants() maintained stack-local arrays with this
> information.  Storing this information will allow it to be used from
> multiple functions, allowing us to split and move code around.
> 
> We'll also eventually want to pass pull constant location information
> to the SIMD16 compile.  Saving this information will help us do that.
> 
> Unfortunately, the two functions *cannot* share the contents of the
> array just yet.  remove_dead_constants() renumbers all the UNIFORM
> registers to be contiguous starting at zero, so the two functions
> talk about uniforms using different names.  We can't even remap them,
> since move_uniform_array_access_to_pull_constants() deletes UNIFORM
> registers that are only accessed with reladdr, so remove_dead_constants
> can't even see them.
> 
> This situation will improve in the next few patches.
> 
> Signed-off-by: Kenneth Graunke 
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp | 7 +--
>  src/mesa/drivers/dri/i965/brw_fs.h   | 6 ++
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 1 +
>  3 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index c24d2f8..8faf401 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -1827,7 +1827,7 @@ fs_visitor::remove_dead_constants()
>  void
>  fs_visitor::move_uniform_array_access_to_pull_constants()
>  {
> -   int pull_constant_loc[uniforms];
> +   pull_constant_loc = ralloc_array(mem_ctx, int, uniforms);
>  
> for (unsigned int i = 0; i < uniforms; i++) {
>pull_constant_loc[i] = -1;
> @@ -1884,6 +1884,9 @@ 
> fs_visitor::move_uniform_array_access_to_pull_constants()
>}
> }
> invalidate_live_intervals();
> +
> +   ralloc_free(pull_constant_loc);
> +   pull_constant_loc = NULL;
>  }
>  
>  /**
> @@ -1909,7 +1912,7 @@ fs_visitor::setup_pull_constants()
>  */
> unsigned int pull_uniform_base = max_uniform_components;
>  
> -   int pull_constant_loc[uniforms];
> +   pull_constant_loc = ralloc_array(mem_ctx, int, uniforms);
> for (unsigned int i = 0; i < uniforms; i++) {
>if (i < pull_uniform_base) {
>   pull_constant_loc[i] = -1;
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h 
> b/src/mesa/drivers/dri/i965/brw_fs.h
> index 86c95df..abfdb10 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -509,6 +509,12 @@ public:
> /** Number of uniform variable components visited. */
> unsigned uniforms;
>  
> +   /**
> +* Array mapping UNIFORM register numbers to the pull parameter index,
> +* or -1 if this uniform register isn't being uploaded as a pull constant.
> +*/
> +   int *pull_constant_loc;
> +
> /* This is the map from UNIFORM hw_reg + reg_offset as generated by
>  * the visitor to the packed uniform number after
>  * remove_dead_constants() that represents the actual uploaded
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp 
> b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> index 90272eb..404af30 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -2971,6 +2971,7 @@ fs_visitor::fs_visitor(struct brw_context *brw,
> this->regs_live_at_ip = NULL;
>  
> this->uniforms = 0;
> +   this->pull_constant_loc = NULL;

Apart from 'dispatch_width' all the other members are initialised here in the
body of the constructor, so it is consistent to have this here also. I'm just
thinking if we would like to start using more the initialization list.

> this->params_remap = NULL;
> this->nr_params_remap = 0;
>  
> -- 
> 1.9.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] i965: Enable EWA anisotropic filtering algorithm

[Kenneth Graunke]

On 03/13/2014 03:30 PM, Kenneth Graunke wrote:
> On 03/04/2014 01:08 AM, Ian Romanick wrote:
>> From: Ian Romanick 
>>
>> Volume 4, part 1 of the Ivybridge PRM says, "Generally, the EWA
>> approximation algorithm results in higher image quality than the legacy
>> algorithm."  Using a classic anisotropic filtering "tunnel" demo, it
>> appears that there is *no* anisotropic filtering on IVB without this bit
>> set.
>>
>> Signed-off-by: Ian Romanick 
>> Cc: Eero Tamminen 
>> ---
>>  src/mesa/drivers/dri/i965/gen7_sampler_state.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/src/mesa/drivers/dri/i965/gen7_sampler_state.c 
>> b/src/mesa/drivers/dri/i965/gen7_sampler_state.c
>> index 968c410..709a783 100644
>> --- a/src/mesa/drivers/dri/i965/gen7_sampler_state.c
>> +++ b/src/mesa/drivers/dri/i965/gen7_sampler_state.c
>> @@ -82,6 +82,7 @@ gen7_update_sampler_state(struct brw_context *brw, int 
>> unit, int ss_index,
>> if (gl_sampler->MaxAnisotropy > 1.0) {
>>sampler->ss0.min_filter = BRW_MAPFILTER_ANISOTROPIC;
>>sampler->ss0.mag_filter = BRW_MAPFILTER_ANISOTROPIC;
>> +  sampler->ss0.aniso_algorithm = 1;
>>  
>>if (gl_sampler->MaxAnisotropy > 2.0) {
>>   sampler->ss3.max_aniso = MIN2((gl_sampler->MaxAnisotropy - 2) / 2,
> 
> *No* anisotropic filtering is a surprising result, but I can confirm
> that enabling the EWA algorithm significantly changes the rendering in
> one test I've looked at.  Without it, you get something like a polar
> rose, and with it, you get a circle (which is the expected result).
> 
> So I think we should go ahead with this.
> 
> Acked-by: Kenneth Graunke 

It also seems to be a 37% performance hit in that test, FWIW.

--Ken



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

Re: [Mesa-dev] [Mesa-stable] [PATCH] glsl: Allow overlapping locations for vertex input attributes

[Anuj Phogat]

On Tue, Mar 11, 2014 at 10:37 AM, Ian Romanick  wrote:

> On 03/10/2014 04:15 PM, Anuj Phogat wrote:
> >
> >
> >
> > On Mon, Mar 10, 2014 at 3:27 PM, Ian Romanick  > > wrote:
> >
> > On 03/10/2014 11:19 AM, Anuj Phogat wrote:
> > > Currently overlapping locations of input variables are not allowed
> > for all
> > > the shader types in OpenGL and OpenGL ES.
> > >
> > >>From OpenGL ES 3.0 spec, page 56:
> > >"Binding more than one attribute name to the same location is
> > referred
> > > to as aliasing, and is not permitted in OpenGL ES Shading
> Language
> > > 3.00 vertex shaders. LinkProgram will fail when this condition
> > exists.
> > > However, aliasing is possible in OpenGL ES Shading Language
> > 1.00 vertex
> > > shaders."
> > >
> > > Taking in to account what different versions of OpenGL and OpenGL
> > ES specs
> > > say about aliasing:
> > >- It is allowed only on vertex shader input attributes in
> > OpenGL (2.0 and
> > >  above) and OpenGL ES 2.0.
> > >- It is explictly disallowed in OpenGL ES 3.0.
> > >
> > > Fixes Khronos CTS failing test:
> > > explicit_attrib_location_vertex_input_aliased.test
> > > See more details about this at below mentioned khronos bug.
> > >
> > > Signed-off-by: Anuj Phogat  > >
> > > Cc: "9.2 10.0 10.1"  > >
> > > Bugzilla: Khronos #9609
> > > ---
> > >  src/glsl/linker.cpp | 74
> > +++--
> > >  1 file changed, 61 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> > > index a619bc8..9bd698b 100644
> > > --- a/src/glsl/linker.cpp
> > > +++ b/src/glsl/linker.cpp
> > > @@ -1798,10 +1798,12 @@
> > assign_attribute_or_color_locations(gl_shader_program *prog,
> > >* active attribute array, both of which require
> > multiple
> > >* contiguous generic attributes."
> > >*
> > > -  * Previous versions of the spec contain similar
> > language but omit
> > > -  * the bit about attribute arrays.
> > > +  * I think above text prohibits the aliasing of explicit
> and
> > > +  * automatic assignments. But, aliasing is allowed in
> manual
> > > +  * assignments of attribute locations. See below
> > comments for
> > > +  * the details.
> >
> > The text above (outside the diff) forbids things like:
> >
> > layout(location=0) in mat4 m;
> > layout(location=1) in vec4 v;
> >
> > where v and m now partially overlap because m consumes 4 slots (0
> > through 3, inclusive).
> >
> >
> > This should not be a problem as along as there's only one active
> attribute
> > in vertex shader. If a developer uses such locations, its his
> > responsibility
> > to use just one of them in any path in shader program. Khronos CTS test
> > also expects linking to happen in above case.
> >
> > OpenGL 4.0 spec makes similar looking statements at two different places.
> > First  for vertex shader input locations at page 60:
> >
> > "LinkProgram will fail if the attribute bindings assigned by BindAttri-
> > bLocation do not leave not enough space to assign a location for an
> > active matrix
> > attribute or an active attribute array, both of which require multiple
> > contiguous
> > generic attributes."
>
> Somewhere in the intervening specs, the language was changed.  In 4.4 it
> says:
>
> "LinkProgram will fail if the attribute bindings specified either
> by BindAttribLocation or explicitly set within the shader text do
> not leave not enough space to assign a location for an active
> matrix attribute or an active attribute array, both of which
> require multiple contiguous generic attributes."
>
> > This statement sounds little unclear to me. It doesn't say  what happens
> if
> > locations set in shader text don't leave enough space. It also doesn't
> make
> > clear who assigns the location in "to assign a location for an active
> > matrix".
> > explicit or automatic assignment?
>
> You may be right.  We'll have to prod other implementations.  I'll also
> do some spec archaeology.
>
Piglit tests I posted on mailing list passes on NVIDIA's OpenGL 4.3 linux
driver. NVIDIA allows the kind of overlapping you described above.

>
> > Second for fragment output locations at page 233:
> > "if the explicit binding assigments do not leave enough space for the
> linker
> > to automatically assign a location for a varying out array, which
> requires
> > multiple contiguous locations."
> >
> > This statement makes the condition more clear for linking error. Notice
> > the words
> >  'explicit' and 'automatically' in text from page 233.
> >
> > So I int

Re: [Mesa-dev] [PATCH] i965: Enable EWA anisotropic filtering algorithm

[Kenneth Graunke]

On 03/04/2014 01:08 AM, Ian Romanick wrote:
> From: Ian Romanick 
> 
> Volume 4, part 1 of the Ivybridge PRM says, "Generally, the EWA
> approximation algorithm results in higher image quality than the legacy
> algorithm."  Using a classic anisotropic filtering "tunnel" demo, it
> appears that there is *no* anisotropic filtering on IVB without this bit
> set.
> 
> Signed-off-by: Ian Romanick 
> Cc: Eero Tamminen 
> ---
>  src/mesa/drivers/dri/i965/gen7_sampler_state.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/gen7_sampler_state.c 
> b/src/mesa/drivers/dri/i965/gen7_sampler_state.c
> index 968c410..709a783 100644
> --- a/src/mesa/drivers/dri/i965/gen7_sampler_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_sampler_state.c
> @@ -82,6 +82,7 @@ gen7_update_sampler_state(struct brw_context *brw, int 
> unit, int ss_index,
> if (gl_sampler->MaxAnisotropy > 1.0) {
>sampler->ss0.min_filter = BRW_MAPFILTER_ANISOTROPIC;
>sampler->ss0.mag_filter = BRW_MAPFILTER_ANISOTROPIC;
> +  sampler->ss0.aniso_algorithm = 1;
>  
>if (gl_sampler->MaxAnisotropy > 2.0) {
>sampler->ss3.max_aniso = MIN2((gl_sampler->MaxAnisotropy - 2) / 2,

*No* anisotropic filtering is a surprising result, but I can confirm
that enabling the EWA algorithm significantly changes the rendering in
one test I've looked at.  Without it, you get something like a polar
rose, and with it, you get a circle (which is the expected result).

So I think we should go ahead with this.

Acked-by: Kenneth Graunke 



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

Re: [Mesa-dev] [Mesa-stable] [PATCH] mesa: return v.value_int64 when the requested type is TYPE_INT64

[Emil Velikov]

On 13/03/14 15:31, Ian Romanick wrote:

On 03/13/2014 01:31 AM, Emil Velikov wrote:

Fixes "Operands don't affect result" defect reported by Coverity.


D'oh.  So... we obviously don't have any piglit tests that hit these
cases.  Would you mind whipping up something simple?

Considering that these were in mesa for (afaics) ~3 years I was a bit 
"d'oh" myself. Never wrote any piglits before so if you can point me to 
ones that I can butcher, I mean rework, that would be great.


-Emil


Cc: "9.2 10.0 10.1"  
Signed-off-by: Emil Velikov 
---
  src/mesa/main/get.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
index b190851..88cf202 100644
--- a/src/mesa/main/get.c
+++ b/src/mesa/main/get.c
@@ -1997,7 +1997,7 @@ _mesa_GetBooleani_v( GLenum pname, GLuint index, 
GLboolean *params )
params[3] = INT_TO_BOOLEAN(v.value_int_4[3]);
break;
 case TYPE_INT64:
-  params[0] = INT64_TO_BOOLEAN(v.value_int);
+  params[0] = INT64_TO_BOOLEAN(v.value_int64);
break;
 default:
; /* nothing - GL error was recorded */
@@ -2042,7 +2042,7 @@ _mesa_GetIntegeri_v( GLenum pname, GLuint index, GLint 
*params )
params[3] = v.value_int_4[3];
break;
 case TYPE_INT64:
-  params[0] = INT64_TO_INT(v.value_int);
+  params[0] = INT64_TO_INT(v.value_int64);
break;
 default:
; /* nothing - GL error was recorded */
@@ -2067,7 +2067,7 @@ _mesa_GetInteger64i_v( GLenum pname, GLuint index, 
GLint64 *params )
params[3] = v.value_int_4[3];
break;
 case TYPE_INT64:
-  params[0] = v.value_int;
+  params[0] = v.value_int64;
break;
 default:
; /* nothing - GL error was recorded */





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

Re: [Mesa-dev] [PATCH] i965: Don't enable reset notification support on Gen4-5.

[Mika Kuoppala]

Ian Romanick  writes:

> On 03/12/2014 01:43 AM, Kenneth Graunke wrote:
>> arekm reported that using Chrome with GPU acceleration enabled on GM45
>> triggered the hw_ctx != NULL assertion in brw_get_graphics_reset_status.
>> 
>> We definitely do not want to advertise reset notification support on
>> Gen4-5 systems, since it needs hardware contexts, and we never even
>> request a hardware context on those systems.
>> 
>> Cc: Ian Romanick 
>> Signed-off-by: Kenneth Graunke 
>> ---
>>  src/mesa/drivers/dri/i965/intel_screen.c | 13 +
>>  1 file changed, 9 insertions(+), 4 deletions(-)
>> 
>> This may not be the best solution; if someone would like to send patches for
>> a better one, I'm more than happy to go with something else.  Checking for
>> hardware contexts is a bit awkward - we need to decide whether to advertise
>> support here in the screen, but we don't create hardware contexts until
>> context creation time, which is a fair bit later.  So, I'm not sure how to
>> do better than the generation check, for now.
>
> I'm confused.  I thought the ioctl was was supposed to fail with EINVAL
> in that case.  Mika, what's your suggestion?

I think two ioctl queries are needed to catch the combinations,
unfortunately :(

For gens without hw_contexts, we keep statistics per file
descriptor and thus we can ban batch submission through
file desc if has been sending hanging batches. This is
tied to reset stats so that if there is no hw_contexts,
you get statistics for your fd.

Here is how I do it in tests:

static bool gem_has_hw_contexts(int fd)
{
struct local_drm_i915_gem_context_create create;
int ret;

memset(&create, 0, sizeof(create));
ret = drmIoctl(fd, CONTEXT_CREATE_IOCTL, &create);

if (ret == 0) {
drmIoctl(fd, CONTEXT_DESTROY_IOCTL, &create);
return true;
}

return false;
}

static bool gem_has_reset_stats(int fd)
{
struct local_drm_i915_reset_stats rs;
int ret;

/* Carefully set flags and pad to zero, otherwise
   we get -EINVAL
*/
memset(&rs, 0, sizeof(rs));

ret = drmIoctl(fd, GET_RESET_STATS_IOCTL, &rs);
if (ret == 0)
return true;

/* If we get EPERM, we have support but did not
   have CAP_SYSADM */
if (ret == -1 && errno == EPERM)
return true;

return false;
}

-Mika


>> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c 
>> b/src/mesa/drivers/dri/i965/intel_screen.c
>> index 464cebf..12006be 100644
>> --- a/src/mesa/drivers/dri/i965/intel_screen.c
>> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
>> @@ -1342,13 +1342,18 @@ __DRIconfig **intelInitScreen2(__DRIscreen *psp)
>>  * no error.  If the ioctl is not supported, it always generate EINVAL.
>>  * Use this to determine whether to advertise the __DRI2_ROBUSTNESS
>>  * extension to the loader.
>> +*
>> +* Don't even try on pre-Gen6, since we don't attempt to use contexts 
>> there.
>>  */
>> -   struct drm_i915_reset_stats stats;
>> -   memset(&stats, 0, sizeof(stats));
>> +   if (intelScreen->devinfo->gen >= 6) {
>> +  struct drm_i915_reset_stats stats;
>> +  memset(&stats, 0, sizeof(stats));
>>  
>> -   const int ret = drmIoctl(psp->fd, DRM_IOCTL_I915_GET_RESET_STATS, 
>> &stats);
>> +  const int ret = drmIoctl(psp->fd, DRM_IOCTL_I915_GET_RESET_STATS, 
>> &stats);
>>  
>> -   intelScreen->has_context_reset_notification = (ret != -1 || errno != 
>> EINVAL);
>> +  intelScreen->has_context_reset_notification =
>> + (ret != -1 || errno != EINVAL);
>> +   }
>>  
>> psp->extensions = !intelScreen->has_context_reset_notification
>>? intelScreenExtensions : intelRobustScreenExtensions;
>> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 2/2] radeonsi: flush the dma ring in si_flush_from_st

[Niels Ole Salscheider]

Signed-off-by: Niels Ole Salscheider 
---
 src/gallium/drivers/radeonsi/si_pipe.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/gallium/drivers/radeonsi/si_pipe.c 
b/src/gallium/drivers/radeonsi/si_pipe.c
index 827e9fe..401bf6a 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.c
+++ b/src/gallium/drivers/radeonsi/si_pipe.c
@@ -65,6 +65,13 @@ static void si_flush_from_st(struct pipe_context *ctx,
 struct pipe_fence_handle **fence,
 unsigned flags)
 {
+   struct si_context *sctx = (struct si_context *)ctx;
+
+   if (sctx->b.rings.dma.cs) {
+   sctx->b.rings.dma.flush(sctx,
+   flags & PIPE_FLUSH_END_OF_FRAME ? 
RADEON_FLUSH_END_OF_FRAME : 0);
+   }
+
si_flush(ctx, fence,
 flags & PIPE_FLUSH_END_OF_FRAME ? RADEON_FLUSH_END_OF_FRAME : 
0);
 }
-- 
1.9.0

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

[Mesa-dev] [PATCH 1/2] radeon: Move DMA ring creation to common code

[Niels Ole Salscheider]

Signed-off-by: Niels Ole Salscheider 
---
 src/gallium/drivers/r600/r600_pipe.c  | 30 -
 src/gallium/drivers/r600/r600_pipe.h  |  1 -
 src/gallium/drivers/radeon/r600_pipe_common.c | 32 +++
 src/gallium/drivers/radeon/r600_pipe_common.h |  2 ++
 4 files changed, 34 insertions(+), 31 deletions(-)

diff --git a/src/gallium/drivers/r600/r600_pipe.c 
b/src/gallium/drivers/r600/r600_pipe.c
index 88fbdd8..982e18d 100644
--- a/src/gallium/drivers/r600/r600_pipe.c
+++ b/src/gallium/drivers/r600/r600_pipe.c
@@ -48,7 +48,6 @@ static const struct debug_named_value r600_debug_options[] = {
{ "nollvm", DBG_NO_LLVM, "Disable the LLVM shader compiler" },
 #endif
{ "nocpdma", DBG_NO_CP_DMA, "Disable CP DMA" },
-   { "nodma", DBG_NO_ASYNC_DMA, "Disable asynchronous DMA" },
 
/* shader backend */
{ "nosb", DBG_NO_SB, "Disable sb backend for graphics shaders" },
@@ -121,20 +120,6 @@ static void r600_flush_gfx_ring(void *ctx, unsigned flags)
r600_flush((struct pipe_context*)ctx, flags);
 }
 
-static void r600_flush_dma_ring(void *ctx, unsigned flags)
-{
-   struct r600_context *rctx = (struct r600_context *)ctx;
-   struct radeon_winsys_cs *cs = rctx->b.rings.dma.cs;
-
-   if (!cs->cdw) {
-   return;
-   }
-
-   rctx->b.rings.dma.flushing = true;
-   rctx->b.ws->cs_flush(cs, flags, 0);
-   rctx->b.rings.dma.flushing = false;
-}
-
 static void r600_flush_from_winsys(void *ctx, unsigned flags)
 {
struct r600_context *rctx = (struct r600_context *)ctx;
@@ -142,13 +127,6 @@ static void r600_flush_from_winsys(void *ctx, unsigned 
flags)
rctx->b.rings.gfx.flush(rctx, flags);
 }
 
-static void r600_flush_dma_from_winsys(void *ctx, unsigned flags)
-{
-   struct r600_context *rctx = (struct r600_context *)ctx;
-
-   rctx->b.rings.dma.flush(rctx, flags);
-}
-
 static void r600_destroy_context(struct pipe_context *context)
 {
struct r600_context *rctx = (struct r600_context *)context;
@@ -269,14 +247,6 @@ static struct pipe_context *r600_create_context(struct 
pipe_screen *screen, void
rctx->b.ws->cs_set_flush_callback(rctx->b.rings.gfx.cs, 
r600_flush_from_winsys, rctx);
rctx->b.rings.gfx.flushing = false;
 
-   rctx->b.rings.dma.cs = NULL;
-   if (rscreen->b.info.r600_has_dma && !(rscreen->b.debug_flags & 
DBG_NO_ASYNC_DMA)) {
-   rctx->b.rings.dma.cs = rctx->b.ws->cs_create(rctx->b.ws, 
RING_DMA, NULL);
-   rctx->b.rings.dma.flush = r600_flush_dma_ring;
-   rctx->b.ws->cs_set_flush_callback(rctx->b.rings.dma.cs, 
r600_flush_dma_from_winsys, rctx);
-   rctx->b.rings.dma.flushing = false;
-   }
-
rctx->allocator_fetch_shader = u_suballocator_create(&rctx->b.b, 64 * 
1024, 256,
 0, 
PIPE_USAGE_DEFAULT, FALSE);
if (!rctx->allocator_fetch_shader)
diff --git a/src/gallium/drivers/r600/r600_pipe.h 
b/src/gallium/drivers/r600/r600_pipe.h
index 6d627e5..a3827e3 100644
--- a/src/gallium/drivers/r600/r600_pipe.h
+++ b/src/gallium/drivers/r600/r600_pipe.h
@@ -197,7 +197,6 @@ struct r600_gs_rings_state {
 /* features */
 #define DBG_NO_LLVM(1 << 17)
 #define DBG_NO_CP_DMA  (1 << 18)
-#define DBG_NO_ASYNC_DMA   (1 << 19)
 /* shader backend */
 #define DBG_NO_SB  (1 << 21)
 #define DBG_SB_CS  (1 << 22)
diff --git a/src/gallium/drivers/radeon/r600_pipe_common.c 
b/src/gallium/drivers/radeon/r600_pipe_common.c
index 3aa718d..2e39aaf 100644
--- a/src/gallium/drivers/radeon/r600_pipe_common.c
+++ b/src/gallium/drivers/radeon/r600_pipe_common.c
@@ -43,6 +43,27 @@ static void r600_memory_barrier(struct pipe_context *ctx, 
unsigned flags)
 {
 }
 
+static void r600_flush_dma_ring(void *ctx, unsigned flags)
+{
+   struct r600_common_context *rctx = (struct r600_common_context *)ctx;
+   struct radeon_winsys_cs *cs = rctx->rings.dma.cs;
+
+   if (!cs->cdw) {
+   return;
+   }
+
+   rctx->rings.dma.flushing = true;
+   rctx->ws->cs_flush(cs, flags, 0);
+   rctx->rings.dma.flushing = false;
+}
+
+static void r600_flush_dma_from_winsys(void *ctx, unsigned flags)
+{
+   struct r600_common_context *rctx = (struct r600_common_context *)ctx;
+
+   rctx->rings.dma.flush(rctx, flags);
+}
+
 bool r600_common_context_init(struct r600_common_context *rctx,
  struct r600_common_screen *rscreen)
 {
@@ -77,6 +98,14 @@ bool r600_common_context_init(struct r600_common_context 
*rctx,
if (!rctx->uploader)
return false;
 
+   rctx->rings.dma.cs = NULL;
+   if (rscreen->info.r600_has_dma && !(rscreen->debug_flags & 
DBG_NO_ASYNC_DMA)) {
+   rctx->rings.dma.cs = rctx->ws->cs_create(rctx->ws, RING_DMA, 
NULL);
+   rctx->rings.dma.flush = r600_flush_dma_ring;
+   rct

Re: [Mesa-dev] [PATCH 07/12] mesa/sso: Implement _mesa_UseProgramStages

[Eric Anholt]

Ian Romanick  writes:

> On 03/05/2014 12:35 PM, Eric Anholt wrote:
>> Ian Romanick  writes:
>> 
>>> From: Gregory Hainaut 
>>>
>>> Now arb_separate_shader_object-GetProgramPipelineiv should pass.
>>>
>>> V3 (idr):
>>> * Change spec references to core OpenGL versions instead of issues in
>>>   the extension spec.
>>> * Split out from previous uber patch.
>>>
>>> v4 (idr): Use _mesa_has_geometry_shaders in _mesa_UseProgramStages to
>>> detect availability of geometry shaders.
>>>
>>> Reviewed-by: Ian Romanick 
>>> ---
>>>  src/mesa/main/pipelineobj.c | 115 
>>> 
>>>  1 file changed, 115 insertions(+)
>>>
>>> diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c
>>> index 849c781..7149578 100644
>>> --- a/src/mesa/main/pipelineobj.c
>>> +++ b/src/mesa/main/pipelineobj.c
>> 
>>> +   GLbitfield any_valid_stages = GL_VERTEX_SHADER_BIT | 
>>> GL_FRAGMENT_SHADER_BIT;
>>> +   if (_mesa_has_geometry_shaders(ctx))
>>> +  any_valid_stages |= GL_GEOMETRY_SHADER_BIT;
>>> +
>>> +   if (stages != GL_ALL_SHADER_BITS && (stages  & ~any_valid_stages) != 0) 
>>> {
>> 
>> Weird double space before &.
>> 
>>> +  _mesa_error(ctx, GL_INVALID_VALUE, "glUseProgramStages(Stages)");
>>> +  return;
>>> +   }
>> 
>>> +   if (program) {
>>> +  /* Section 2.11.1 (Shader Objects) of the OpenGL 3.1 spec (and 
>>> probably
>>> +   * earlier) says:
>>> +   *
>>> +   * "Commands that accept shader or program object names will
>>> +   * generate the error INVALID_VALUE if the provided name is not 
>>> the
>>> +   * name of either a shader or program object and 
>>> INVALID_OPERATION
>>> +   * if the provided name identifies an object that is not the
>>> +   * expected type."
>>> +   */
>>> +  struct gl_shader *sh = _mesa_lookup_shader(ctx, program);
>>> +  if (sh != NULL) {
>>> + _mesa_error(ctx, GL_INVALID_OPERATION,
>>> +   "glUseProgramStages(progam is a shader object)");
>>> + return;
>>> +  }
>> 
>> This block could get dropped into the shProg == NULL case below, right?
>> The code confused me as is.
>
> The first block is checking whether the name is a shader, and the other
> is checking that the name is a program.  The neat thing is the spec says
> using a shader where a program is expected gives one error, but using a
> name that is neither give a different error.
>
> You've never seen code like this elsewhere in Mesa because it's usually
> handled by _mesa_lookup_shader_program_err.  I'll change this code to
> use that function instead.

Yeah, I understood what was happening here.  I was assuming, though,
that if you have a successful lookup of a shProg, then you don't need to
throw an error if there happens to be a shader with the same name (if
that's even possible).  So the code doesn't need to do the extra lookup
in the success case or look confusing at all.

>>> +
>>> +  shProg = _mesa_lookup_shader_program(ctx, program);
>>> +  if (shProg == NULL) {
>>> + _mesa_error(ctx, GL_INVALID_VALUE,
>>> +   "glUseProgramStages(progam is not a program object)");
>>> + return;
>>> +  }
>> 
>> 
>>> +   /* Section 2.11.4 (Program Pipeline Objects) of the OpenGL 4.1 spec
>>> +* says:
>>> +*
>>> +* "If UseProgramStages is called with program set to zero or with a
>>> +* program object that contains no executable code for the given
>>> +* stages, it is as if the pipeline object has no programmable stage
>>> +* configured for the indicated shader stages."
>>> +*/
>>> +   if ((stages & GL_VERTEX_SHADER_BIT) != 0)
>>> +  _mesa_use_shader_program(ctx, GL_VERTEX_SHADER, shProg, pipe);
>>> +
>>> +   if ((stages & GL_FRAGMENT_SHADER_BIT) != 0)
>>> +  _mesa_use_shader_program(ctx, GL_FRAGMENT_SHADER, shProg, pipe);
>>> +
>>> +   if ((stages & GL_GEOMETRY_SHADER_BIT) != 0)
>>> +  _mesa_use_shader_program(ctx, GL_GEOMETRY_SHADER, shProg, pipe);
>>>  }
>> 
>> The spec cite here doesn't seem to be explaining the code it's next to,
>> to me.
>
> That is fair... and at least partially my fault.  Greg's original
> comment above this code was:
>
>/*
> *  7.  What happens if you have a program object current for a shader 
> stage,
> *but the program object doesn't contain an executable for that stage?
>
> *RESOLVED:  This is not an error; instead it is as though there were 
> no
> *program bound to that stage.  We have two different notions for
> *programs bound to shader stages.  A program is "current" for a stage
> *if it bound to that stage in the active program pipeline object.  A
> *program is "active" for a stage if it is current and it has an
> *executable for this stage.  In this case, the program would be 
> current
> *but not active.
>
> *When no program is active for a stage, the stage will be replaced 
> with
> 

Re: [Mesa-dev] [PATCH 5/8] i965/upload: Actually comment the upload code.

[Kenneth Graunke]

On 03/13/2014 09:05 AM, Ian Romanick wrote:
> On 03/13/2014 01:57 AM, Kenneth Graunke wrote:
>> diff --git a/src/mesa/drivers/dri/i965/intel_upload.c 
>> b/src/mesa/drivers/dri/i965/intel_upload.c
>> index ec3109b..c767b7d 100644
>> --- a/src/mesa/drivers/dri/i965/intel_upload.c
>> +++ b/src/mesa/drivers/dri/i965/intel_upload.c
>> @@ -25,7 +25,16 @@
>>  /**
>>   * @file intel_upload.c
>>   *
>> - * Batched upload via BOs.
>> + * A batched data upload mechanism which can efficiently copy user supplied
>> + * data into buffer objects.
>> + *
>> + * OpenGL allows applications to provide data in user arrays, which may
>> + * disappear or change contents at any time.  The GPU can't use this memory
>> + * directly, so we need to copy it into buffer objects.
>> + *
>> + * Often times, the amount of data we need to copy is small.  On non-LLC
>> + * platforms, it's more efficient to stage this data in a CPU-sized buffer
>  ^
> Is sized the word you mean here?  Or should it be CPU-side?

No, I meant a literally 22nm sized buffer... :)

Fixed, thanks.

--Ken





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

Re: [Mesa-dev] [PATCH] nouveau: add fogotten GL_COMPRESSED_INTENSITY to texture format list

[Francisco Jerez]

Ilia Mirkin  writes:

> Signed-off-by: Ilia Mirkin 

Reviewed-by: Francisco Jerez 

> ---
>  src/mesa/drivers/dri/nouveau/nouveau_texture.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/mesa/drivers/dri/nouveau/nouveau_texture.c 
> b/src/mesa/drivers/dri/nouveau/nouveau_texture.c
> index a8c0268..ae8dbd4 100644
> --- a/src/mesa/drivers/dri/nouveau/nouveau_texture.c
> +++ b/src/mesa/drivers/dri/nouveau/nouveau_texture.c
> @@ -225,6 +225,7 @@ nouveau_choose_tex_format(struct gl_context *ctx, GLenum 
> target,
>   case GL_INTENSITY12:
>   case GL_INTENSITY16:
>   case GL_INTENSITY8:
> + case GL_COMPRESSED_INTENSITY:
>   return MESA_FORMAT_I_UNORM8;
>  
>   case GL_RGB_S3TC:
> -- 
> 1.8.3.2


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

Re: [Mesa-dev] [PATCH 12/12] mesa/sso: Implement ValidateProgramPipeline

[Eric Anholt]

Ian Romanick  writes:

> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
>
> On 03/05/2014 01:00 PM, Eric Anholt wrote:
>> Ian Romanick  writes:
>> 
>>> From: Gregory Hainaut 
>>> 
>>> Implementation note: I don't use context for ralloc (don't know
>>> how).
>>> 
>>> The check on PROGRAM_SEPARABLE flags is also done when the
>>> pipeline isn't bound.  It doesn't make any sense in a DSA style
>>> API.
>>> 
>>> Maybe we could replace _mesa_validate_program by 
>>> _mesa_validate_program_pipeline.  For example we could recreate a
>>> dummy pipeline object.  However the new function checks also the 
>>> TEXTURE_IMAGE_UNIT number not sure of the impact.
>>> 
>>> V2: Fix memory leak with ralloc_strdup Formatting improvement
>>> 
>>> V3 (idr): * Actually fix the leak of the InfoLog. :) * Directly
>>> generate logs in to gl_pipeline_object::InfoLog via 
>>> ralloc_asprintf isntead of using a temporary buffer. * Split out
>>> from previous uber patch. * Change spec references to include
>>> section numbers, etc. * Fix a bug in checking that a different
>>> program isn't active in a stage between two stages that have the
>>> same program.  Specifically,
>>> 
>>> if (pipe->CurrentVertexProgram->Name ==
>>> pipe->CurrentGeometryProgram->Name && 
>>> pipe->CurrentGeometryProgram->Name !=
>>> pipe->CurrentVertexProgram->Name)
>>> 
>>> should have been
>>> 
>>> if (pipe->CurrentVertexProgram->Name ==
>>> pipe->CurrentFragmentProgram->Name && 
>>> pipe->CurrentGeometryProgram->Name !=
>>> pipe->CurrentVertexProgram->Name)
>>> 
>>> v4 (idr): Rework to use CurrentProgram array in loops.
>>> 
>>> Reviewed-by: Ian Romanick 
>> 
>>> diff --git a/src/mesa/main/pipelineobj.c
>>> b/src/mesa/main/pipelineobj.c index 3db97c0..d371e88 100644 ---
>>> a/src/mesa/main/pipelineobj.c +++ b/src/mesa/main/pipelineobj.c
>> 
>>> +   /* Section 2.11.11 (Shader Execution), subheading
>>> "Validation," of the +* OpenGL 4.1 spec says: +* +*
>>> "[INVALID_OPERATION] is generated by any command that transfers +
>>> * vertices to the GL if: +* +* ... +* +
>>> * - Any two active samplers in the current program object
>>> are of +*   different types, but refer to the same
>>> texture image unit. +* +* - The number of active
>>> samplers in the program exceeds the +*   maximum
>>> number of texture image units allowed." +*/ +   if
>>> (!_mesa_sampler_uniforms_pipeline_are_valid(pipe)) +  goto
>>> err; + +   pipe->Validated = GL_TRUE; +   return GL_TRUE;
>> 
>> What ensures that the sampler validate will be redone when sampler 
>> uniforms change?
>
> _mesa_valid_to_render calls _mesa_validate_program_pipeline.  If the
> application calls glValidateProgramPipeline (getting an error),
> changes the uniform, then calls glDrawArrays, the program will get
> revalidated.

I was thinking of the transition from a working pipeline with
non-overlapping samplers to overlapping samplers of different types.


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

[Mesa-dev] glx: Fix incorrect pdp assignment in dri2_bind_context().

[Brian Paul]

My coworker found this issue while testing with glretrace.  Looks 
correct to me but I'd like another reviewer (Kristian?)


-Brian

 Original Message 
Subject: [Review Request] glx: Fix incorrect pdp assignment in 
dri2_bind_context().

Date: Thu, 13 Mar 2014 10:16:31 -0700
From: Charmaine Lee 


pdp should be set to dpyPriv->dri2Display.
Fixes blank frame failure running glretrace ClearView.
---
 src/glx/dri2_glx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
index 75fc951..b6eaf1c 100644
--- a/src/glx/dri2_glx.c
+++ b/src/glx/dri2_glx.c
@@ -139,6 +139,7 @@ dri2_bind_context(struct glx_context *context, 
struct glx_context *old,

struct dri2_screen *psc = (struct dri2_screen *) pcp->base.psc;
struct dri2_drawable *pdraw, *pread;
__DRIdrawable *dri_draw = NULL, *dri_read = NULL;
+   struct glx_display *dpyPriv = psc->base.display;
struct dri2_display *pdp;

pdraw = (struct dri2_drawable *) driFetchDrawable(context, draw);
@@ -162,7 +163,7 @@ dri2_bind_context(struct glx_context *context, 
struct glx_context *old,

/* If the server doesn't send invalidate events, we may miss a
 * resize before the rendering starts.  Invalidate the buffers now
 * so the driver will recheck before rendering starts. */
-   pdp = (struct dri2_display *) psc->base.display;
+   pdp = (struct dri2_display *) dpyPriv->dri2Display;
if (!pdp->invalidateAvailable && pdraw) {
   dri2InvalidateBuffers(psc->base.dpy, pdraw->base.xDrawable);
   if (pread != pdraw && pread)
--
1.8.1.2

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

[Mesa-dev] [PATCH] nouveau: add fogotten GL_COMPRESSED_INTENSITY to texture format list

[Ilia Mirkin]

Signed-off-by: Ilia Mirkin 
---
 src/mesa/drivers/dri/nouveau/nouveau_texture.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/mesa/drivers/dri/nouveau/nouveau_texture.c 
b/src/mesa/drivers/dri/nouveau/nouveau_texture.c
index a8c0268..ae8dbd4 100644
--- a/src/mesa/drivers/dri/nouveau/nouveau_texture.c
+++ b/src/mesa/drivers/dri/nouveau/nouveau_texture.c
@@ -225,6 +225,7 @@ nouveau_choose_tex_format(struct gl_context *ctx, GLenum 
target,
case GL_INTENSITY12:
case GL_INTENSITY16:
case GL_INTENSITY8:
+   case GL_COMPRESSED_INTENSITY:
return MESA_FORMAT_I_UNORM8;
 
case GL_RGB_S3TC:
-- 
1.8.3.2

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

Re: [Mesa-dev] [PATCH 1/4] dri3: Fix indentation of buffer age code

[Adel Gadllah]

Am 2014-03-08 02:26, schrieb Eric Anholt:

---

Here's my series of cleanups and fixes for your EXT_buffer_age patch.
I've run it through piglit, and run gnome-shell with it now.

I'm proposing moving patch 3 before your patch, then squashing 1, 2,
and 4 in with yours.  Does this sound good?


Hi,

Sorry for the late response. Yes those changes make sense to me.

Reviewed-By: Adel Gadllah 

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

Re: [Mesa-dev] [PATCH] i965: Delete "fast color clear unsupported" performance warning.

[Ian Romanick]

On 03/13/2014 02:41 AM, Kenneth Graunke wrote:
> Applications frequently clear to colors other than 0.0 or 1.0, which
> prevents us from doing fast color clears.  In that case, we issue this
> performance warning on basically every glClear call, resulting in so
> much spam that it's nearly impossible to see any other messages.
> 
> Plus, I don't think it's useful.  We aren't suggesting a better way to
> do what the application developers want---we're just telling them it
> would be faster to do something they don't want.
> 
> Driver developers have no control over the clear color, so this message
> is totally useless to them.

I can go along with this logic.  If we still want a message like this,
we'd want a perf_debug_once kind of thing.

Reviewed-by: Ian Romanick 

> Signed-off-by: Kenneth Graunke 
> ---
>  src/mesa/drivers/dri/i965/brw_blorp_clear.cpp | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp 
> b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
> index 76f8299..3a96106 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
> @@ -155,8 +155,6 @@ is_color_fast_clear_compatible(struct brw_context *brw,
>  
> for (int i = 0; i < 4; i++) {
>if (color->f[i] != 0.0 && color->f[i] != 1.0) {
> - perf_debug("Clear color unsupported by fast color clear.  "
> -"Falling back to slow clear.\n");
>   return false;
>}
> }
> 

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

Re: [Mesa-dev] [PATCH 1/8] i965: Consolidate code for setting brw->ib.start_vertex_offset.

[Ian Romanick]

Patches 1, 2, 4, 6, 7, and 8 are

Reviewed-by: Ian Romanick 

I separately sent some questions about 3 and 5.

On 03/13/2014 01:57 AM, Kenneth Graunke wrote:
> This was set identically in three places.
> 
> Signed-off-by: Kenneth Graunke 
> ---
>  src/mesa/drivers/dri/i965/brw_draw_upload.c | 15 ++-
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> This is just a series of tidying and commenting, for the most part.
> Patch 7 has an actual change.
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c 
> b/src/mesa/drivers/dri/i965/brw_draw_upload.c
> index d42c074..3c537a5 100644
> --- a/src/mesa/drivers/dri/i965/brw_draw_upload.c
> +++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c
> @@ -841,12 +841,10 @@ static void brw_upload_indices(struct brw_context *brw)
> /* Turn into a proper VBO:
>  */
> if (!_mesa_is_bufferobj(bufferobj)) {
> -
>/* Get new bufferobj, offset:
> */
>intel_upload_data(brw, index_buffer->ptr, ib_size, ib_type_size,
>   &bo, &offset);
> -  brw->ib.start_vertex_offset = offset / ib_type_size;
> } else {
>offset = (GLuint) (unsigned long) index_buffer->ptr;
>  
> @@ -865,22 +863,21 @@ static void brw_upload_indices(struct brw_context *brw)
>  MAP_INTERNAL);
>  
>intel_upload_data(brw, map, ib_size, ib_type_size, &bo, &offset);
> -  brw->ib.start_vertex_offset = offset / ib_type_size;
>  
>ctx->Driver.UnmapBuffer(ctx, bufferobj, MAP_INTERNAL);
> } else {
> -   /* Use CMD_3D_PRIM's start_vertex_offset to avoid re-uploading
> -* the index buffer state when we're just moving the start index
> -* of our drawing.
> -*/
> -   brw->ib.start_vertex_offset = offset / ib_type_size;
> -
> bo = intel_bufferobj_buffer(brw, intel_buffer_object(bufferobj),
> offset, ib_size);
> drm_intel_bo_reference(bo);
> }
> }
>  
> +   /* Use 3DPRIMITIVE's start_vertex_offset to avoid re-uploading
> +* the index buffer state when we're just moving the start index
> +* of our drawing.
> +*/
> +   brw->ib.start_vertex_offset = offset / ib_type_size;
> +
> if (brw->ib.bo != bo) {
>drm_intel_bo_unreference(brw->ib.bo);
>brw->ib.bo = bo;
> 

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

Re: [Mesa-dev] [PATCH 5/8] i965/upload: Actually comment the upload code.

[Ian Romanick]

On 03/13/2014 01:57 AM, Kenneth Graunke wrote:
> diff --git a/src/mesa/drivers/dri/i965/intel_upload.c 
> b/src/mesa/drivers/dri/i965/intel_upload.c
> index ec3109b..c767b7d 100644
> --- a/src/mesa/drivers/dri/i965/intel_upload.c
> +++ b/src/mesa/drivers/dri/i965/intel_upload.c
> @@ -25,7 +25,16 @@
>  /**
>   * @file intel_upload.c
>   *
> - * Batched upload via BOs.
> + * A batched data upload mechanism which can efficiently copy user supplied
> + * data into buffer objects.
> + *
> + * OpenGL allows applications to provide data in user arrays, which may
> + * disappear or change contents at any time.  The GPU can't use this memory
> + * directly, so we need to copy it into buffer objects.
> + *
> + * Often times, the amount of data we need to copy is small.  On non-LLC
> + * platforms, it's more efficient to stage this data in a CPU-sized buffer
 ^
Is sized the word you mean here?  Or should it be CPU-side?

> + * and only copy it to a BO when we have a larger amount of data.
>   */
>  
>  #include "main/imports.h"

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

Re: [Mesa-dev] [PATCH 3/8] i965: Expand comments in brw_upload_indices().

[Ian Romanick]

On 03/13/2014 01:57 AM, Kenneth Graunke wrote:
> This function has three cases:
> 1. Data is in user arrays
> 2. Data is in a buffer object, but misaligned.
> 3. Data is acceptable as is.
> 
> Previously, there was a "Turn into a proper VBO" comment above all three
> cases, even though it only applies to case 1, and maybe case 2 (with a
> specific interpretation of "proper".)  This patch replaces that with
> more specific comments for each case.
> 
> The expanded comments explain where the alignment restrictions come from
> and give an example of how to produce misaligned data in OpenGL.
> 
> Signed-off-by: Kenneth Graunke 
> ---
>  src/mesa/drivers/dri/i965/brw_draw_upload.c | 20 ++--
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c 
> b/src/mesa/drivers/dri/i965/brw_draw_upload.c
> index f2945c1..98a8277 100644
> --- a/src/mesa/drivers/dri/i965/brw_draw_upload.c
> +++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c
> @@ -838,18 +838,25 @@ static void brw_upload_indices(struct brw_context *brw)
> ib_size = ib_type_size * index_buffer->count;
> bufferobj = index_buffer->obj;
>  
> -   /* Turn into a proper VBO:
> -*/
> if (!_mesa_is_bufferobj(bufferobj)) {
> -  /* Get new bufferobj, offset:
> -   */
> +  /* Copy the index data from user arrays into a buffer object. */
>intel_upload_data(brw, index_buffer->ptr, ib_size, ib_type_size,
>   &bo, &offset);
> } else {
>offset = (GLuint) (unsigned long) index_buffer->ptr;
>  
> -  /* If the index buffer isn't aligned to its element size, we have to
> -   * rebase it into a temporary.
> +  /* Even though the index data is in a buffer object, it may not meet
> +   * the hardware's alignment restrictions.  From the 
> 3DSTATE_INDEX_BUFFER
> +   * documentation, DWord 1:
> +   *
> +   * "This field contains the size-aligned (as specified by Index Format)
> +   *  Graphics Address of the first element of interest within the index
> +   *  buffer."
> +   *
> +   * OpenGL allows arbitrary byte offsets into the buffer.  For example,
> +   * glDrawElements with index == 1 and a type of GL_UNSIGNED_SHORT.
> +   *
> +   * If the data isn't aligned to the element size, copy it to a 
> temporary.

Have we actually encountered applications that do that?  The GL spec
does say:

"Clients must align data elements consistent with the requirements
of the client platform, with an additional base-level requirement
that an offset within a buffer to a datum comprising N basic
machine units be a multiple of N."

> */
>if ((ib_type_size - 1) & offset) {
>   perf_debug("copying index buffer to a temporary to work around "
> @@ -866,6 +873,7 @@ static void brw_upload_indices(struct brw_context *brw)
>  
>   ctx->Driver.UnmapBuffer(ctx, bufferobj, MAP_INTERNAL);
>} else {
> + /* The index data is already in an acceptable BO.  Use as is. */
>   bo = intel_bufferobj_buffer(brw, intel_buffer_object(bufferobj),
>   offset, ib_size);
>   drm_intel_bo_reference(bo);
> 

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

Re: [Mesa-dev] [PATCH] mesa/main: condition GL_DEPTH_STENCIL on ARB_depth_texture

[Ilia Mirkin]

On Thu, Mar 13, 2014 at 11:40 AM, Ian Romanick  wrote:
> On 03/13/2014 06:33 AM, Ilia Mirkin wrote:
>> On Thu, Mar 13, 2014 at 9:22 AM, Francisco Jerez  
>> wrote:
>>> Ilia Mirkin  writes:
>>>
 EXT_packed_depth_stencil is supported by all drivers, but
 ARB_depth_texture isn't (notably nouveau_vieux). This should avoid
 passing unexpected values down to ChooseTextureFormat.

 The EXT_packed_depth_stencil spec does not make any explicit references
 to requiring ARB_depth_texture in order to allow textures with that
 format, however if there is no dependency, ARB_depth_texture would be
 practically implied by the extension.

 Signed-off-by: Ilia Mirkin 
 ---

 It should also be noted that ARB_depth_texture became required in OpenGL 
 1.4,
 so this shouldn't affect too many drivers. Looks like i915 only has it
 disabled for gen2. i965 always enables it. Glancing at radeon, it seems 
 like
 the values are supported in ChooseTextureFormat, but not actually handled 
 in
 radeon_texstate or r200_texstate.

  src/mesa/main/teximage.c | 11 +++
  1 file changed, 3 insertions(+), 8 deletions(-)

 diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
 index a6c3581..a57a535 100644
 --- a/src/mesa/main/teximage.c
 +++ b/src/mesa/main/teximage.c
 @@ -160,6 +160,9 @@ _mesa_base_tex_format( struct gl_context *ctx, GLint 
 internalFormat )
   case GL_DEPTH_COMPONENT24:
   case GL_DEPTH_COMPONENT32:
  return GL_DEPTH_COMPONENT;
 + case GL_DEPTH_STENCIL:
 + case GL_DEPTH24_STENCIL8:
 +return GL_DEPTH_STENCIL;
   default:
  ; /* fallthrough */
}
 @@ -301,14 +304,6 @@ _mesa_base_tex_format( struct gl_context *ctx, GLint 
 internalFormat )
}
 }

 -   switch (internalFormat) {
 -   case GL_DEPTH_STENCIL:
 -   case GL_DEPTH24_STENCIL8:
 -  return GL_DEPTH_STENCIL;
 -   default:
 -  ; /* fallthrough */
 -   }
 -
 if (ctx->Extensions.EXT_texture_sRGB) {
switch (internalFormat) {
case GL_SRGB_EXT:
 --
 1.8.3.2
>>>
>>>
>>> Reviewed-by: Francisco Jerez 
>>
>> Thanks. I'm going to wait a while before pushing this to see if others
>> have differing opinions on the spec interpretation. Ian, Eric, Brian?
>> You guys seem to be fairly in-tune with this stuff.
>
> The body of the spec may not be clear, but the overview at least is:
>
> This extension provides a new data format, GL_DEPTH_STENCIL_EXT,
> that can be used with the glDrawPixels, glReadPixels, and
> glCopyPixels commands, as well as a packed data type,
> GL_UNSIGNED_INT_24_8_EXT, that is meant to be used with
> GL_DEPTH_STENCIL_EXT.  No other data types are supported with
> GL_DEPTH_STENCIL_EXT.  If ARB_depth_texture or SGIX_depth_texture is
> supported, GL_DEPTH_STENCIL_EXT/GL_UNSIGNED_INT_24_8_EXT data can
> also be used for textures; this provides a more efficient way to
> supply data for a 24-bit depth texture.
>
> I thought I cleaned all this up when I made EXT_packed_depth_stencil
> non-optional, but it looks like I missed it.  Thanks for taking care of
> this.
>
> Reviewed-by: Ian Romanick 

Great. Another possible interpretation would be that if
ARB_depth_texture is supported, then one is allowed to pass a
(GL_DEPTH_STENCIL_EXT, GL_UNSIGNED_INT_24_8_EXT) format/type data to a
GL_DEPTH_COMPONENT texture. But I much prefer to interpret it as "if
depth textures are supported, then they can also have a DEPTH_STENCIL
internal format", since that makes it workable on nouveau_vieux with
this simple change :)

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

Re: [Mesa-dev] [PATCH 2/8] glsl: Add wrapper function that calls ir_dereference::constant_referenced

[Ian Romanick]

On 03/12/2014 04:20 PM, Connor Abbott wrote:
> On Wed, Mar 12, 2014 at 6:49 PM, Ian Romanick  wrote:
>> From: Ian Romanick 
>>
>> Signed-off-by: Ian Romanick 
>> ---
>>  src/glsl/ir_constant_expression.cpp | 52 
>> +
>>  1 file changed, 36 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/glsl/ir_constant_expression.cpp 
>> b/src/glsl/ir_constant_expression.cpp
>> index a31e579..4149a0e 100644
>> --- a/src/glsl/ir_constant_expression.cpp
>> +++ b/src/glsl/ir_constant_expression.cpp
>> @@ -395,6 +395,37 @@ unpack_half_1x16(uint16_t u)
>>   * The offset is used when the reference is to a specific column of a 
>> matrix.
>>   */
>>  /*@{*/
>> +static bool
>> +constant_referenced(const ir_dereference *deref,
>> +struct hash_table *variable_context,
>> +ir_constant *&store, int &offset)
>> +{
>> +   switch (deref->ir_type) {
>> +   case ir_type_dereference_array:
>> +  ((ir_dereference_array *) 
>> deref)->constant_referenced(variable_context,
>> +store, offset);
>> +  break;
>> +
>> +   case ir_type_dereference_record:
>> +  ((ir_dereference_record *) 
>> deref)->constant_referenced(variable_context,
>> + store, offset);
>> +  break;
>> +
>> +   case ir_type_dereference_variable:
>> +  ((ir_dereference_variable *) 
>> deref)->constant_referenced(variable_context,
>> +   store, 
>> offset);
>> +  break;
>> +
>> +   default:
>> +  assert(!"Should not get here.");
>> +  store = NULL;
>> +  offset = 0;
>> +  break;
>> +   }
>> +
>> +   return store != NULL;
>> +}
>> +
>>  void
>>  ir_dereference_variable::constant_referenced(struct hash_table 
>> *variable_context,
>>  ir_constant *&store, int 
>> &offset) const
>> @@ -433,13 +464,8 @@ ir_dereference_array::constant_referenced(struct 
>> hash_table *variable_context,
>>return;
>> }
>>
>> -   deref->constant_referenced(variable_context, substore, suboffset);
>> -
>> -   if (!substore) {
>> -  store = 0;
>> -  offset = 0;
>> +   if (!::constant_referenced(deref, variable_context, substore, suboffset))
>>return;
>> -   }
> 
> Question: why do you have the :: here...

Without it GCC was trying to resolve constant_referenced to
ir_dereference::constant_referenced.  Since there isn't one that
matches this signatrue, it gave an error.

../../src/glsl/ir_constant_expression.cpp: In member function 'virtual void 
ir_dereference_array::constant_referenced(hash_table*, ir_constant*&, int&) 
const':
../../src/glsl/ir_constant_expression.cpp:467:73: error: no matching function 
for call to 'ir_dereference_array::constant_referenced(const ir_dereference*&, 
hash_table*&, ir_constant*&, int&) const'
../../src/glsl/ir_constant_expression.cpp:467:73: note: candidate is:
../../src/glsl/ir_constant_expression.cpp:443:1: note: virtual void 
ir_dereference_array::constant_referenced(hash_table*, ir_constant*&, int&) 
const
../../src/glsl/ir_constant_expression.cpp:443:1: note:   candidate expects 3 
arguments, 4 provided

It was a little annoying, but having a typo lead to a completely
different function being called would be another sort of annoying
bug... so I didn't curse it too much. :)

>>
>> const glsl_type *vt = array->type;
>> if (vt->is_array()) {
>> @@ -475,13 +501,8 @@ ir_dereference_record::constant_referenced(struct 
>> hash_table *variable_context,
>>return;
>> }
>>
>> -   deref->constant_referenced(variable_context, substore, suboffset);
>> -
>> -   if (!substore) {
>> -  store = 0;
>> -  offset = 0;
>> +   if (!::constant_referenced(deref, variable_context, substore, suboffset))
>>return;
>> -   }
> 
> and here...
> 
>>
>> store = substore->get_record_field(field);
>> offset = 0;
>> @@ -1814,9 +1835,8 @@ bool 
>> ir_function_signature::constant_expression_evaluate_expression_list(const s
>>
>>  ir_constant *store = NULL;
>>  int offset = 0;
>> -asg->lhs->constant_referenced(variable_context, store, offset);
>>
>> -if (!store)
>> +if (!constant_referenced(asg->lhs, variable_context, store, offset))
>> return false;
> 
> but not here...

This location isn't in the ir_deference class hierarchy, so it didn't
need the scope resolution.  I suspect this point may have failed to
compile with it.

>>
>>  ir_constant *value = 
>> asg->rhs->constant_expression_value(variable_context);
>> @@ -1847,9 +1867,9 @@ bool 
>> ir_function_signature::constant_expression_evaluate_expression_list(const s
>>
>>  ir_constant *store = NULL;
>>  int offset = 0;
>> -call->return_deref->constant_referenced(variable_context, store, 
>> offset);
>>
>> -if (!store)
>> +if (!constant_referenced(

Re: [Mesa-dev] [PATCH] mesa/main: condition GL_DEPTH_STENCIL on ARB_depth_texture

[Ian Romanick]

On 03/13/2014 06:33 AM, Ilia Mirkin wrote:
> On Thu, Mar 13, 2014 at 9:22 AM, Francisco Jerez  
> wrote:
>> Ilia Mirkin  writes:
>>
>>> EXT_packed_depth_stencil is supported by all drivers, but
>>> ARB_depth_texture isn't (notably nouveau_vieux). This should avoid
>>> passing unexpected values down to ChooseTextureFormat.
>>>
>>> The EXT_packed_depth_stencil spec does not make any explicit references
>>> to requiring ARB_depth_texture in order to allow textures with that
>>> format, however if there is no dependency, ARB_depth_texture would be
>>> practically implied by the extension.
>>>
>>> Signed-off-by: Ilia Mirkin 
>>> ---
>>>
>>> It should also be noted that ARB_depth_texture became required in OpenGL 
>>> 1.4,
>>> so this shouldn't affect too many drivers. Looks like i915 only has it
>>> disabled for gen2. i965 always enables it. Glancing at radeon, it seems like
>>> the values are supported in ChooseTextureFormat, but not actually handled in
>>> radeon_texstate or r200_texstate.
>>>
>>>  src/mesa/main/teximage.c | 11 +++
>>>  1 file changed, 3 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
>>> index a6c3581..a57a535 100644
>>> --- a/src/mesa/main/teximage.c
>>> +++ b/src/mesa/main/teximage.c
>>> @@ -160,6 +160,9 @@ _mesa_base_tex_format( struct gl_context *ctx, GLint 
>>> internalFormat )
>>>   case GL_DEPTH_COMPONENT24:
>>>   case GL_DEPTH_COMPONENT32:
>>>  return GL_DEPTH_COMPONENT;
>>> + case GL_DEPTH_STENCIL:
>>> + case GL_DEPTH24_STENCIL8:
>>> +return GL_DEPTH_STENCIL;
>>>   default:
>>>  ; /* fallthrough */
>>>}
>>> @@ -301,14 +304,6 @@ _mesa_base_tex_format( struct gl_context *ctx, GLint 
>>> internalFormat )
>>>}
>>> }
>>>
>>> -   switch (internalFormat) {
>>> -   case GL_DEPTH_STENCIL:
>>> -   case GL_DEPTH24_STENCIL8:
>>> -  return GL_DEPTH_STENCIL;
>>> -   default:
>>> -  ; /* fallthrough */
>>> -   }
>>> -
>>> if (ctx->Extensions.EXT_texture_sRGB) {
>>>switch (internalFormat) {
>>>case GL_SRGB_EXT:
>>> --
>>> 1.8.3.2
>>
>>
>> Reviewed-by: Francisco Jerez 
> 
> Thanks. I'm going to wait a while before pushing this to see if others
> have differing opinions on the spec interpretation. Ian, Eric, Brian?
> You guys seem to be fairly in-tune with this stuff.

The body of the spec may not be clear, but the overview at least is:

This extension provides a new data format, GL_DEPTH_STENCIL_EXT,
that can be used with the glDrawPixels, glReadPixels, and
glCopyPixels commands, as well as a packed data type,
GL_UNSIGNED_INT_24_8_EXT, that is meant to be used with
GL_DEPTH_STENCIL_EXT.  No other data types are supported with
GL_DEPTH_STENCIL_EXT.  If ARB_depth_texture or SGIX_depth_texture is
supported, GL_DEPTH_STENCIL_EXT/GL_UNSIGNED_INT_24_8_EXT data can
also be used for textures; this provides a more efficient way to
supply data for a 24-bit depth texture.

I thought I cleaned all this up when I made EXT_packed_depth_stencil
non-optional, but it looks like I missed it.  Thanks for taking care of
this.

Reviewed-by: Ian Romanick 

>   -ilia
> ___
> 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] [Mesa-stable] [PATCH] mesa: return v.value_int64 when the requested type is TYPE_INT64

[Ian Romanick]

On 03/13/2014 01:31 AM, Emil Velikov wrote:
> Fixes "Operands don't affect result" defect reported by Coverity.

D'oh.  So... we obviously don't have any piglit tests that hit these
cases.  Would you mind whipping up something simple?

> Cc: "9.2 10.0 10.1"  
> Signed-off-by: Emil Velikov 
> ---
>  src/mesa/main/get.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
> index b190851..88cf202 100644
> --- a/src/mesa/main/get.c
> +++ b/src/mesa/main/get.c
> @@ -1997,7 +1997,7 @@ _mesa_GetBooleani_v( GLenum pname, GLuint index, 
> GLboolean *params )
>params[3] = INT_TO_BOOLEAN(v.value_int_4[3]);
>break;
> case TYPE_INT64:
> -  params[0] = INT64_TO_BOOLEAN(v.value_int);
> +  params[0] = INT64_TO_BOOLEAN(v.value_int64);
>break;
> default:
>; /* nothing - GL error was recorded */
> @@ -2042,7 +2042,7 @@ _mesa_GetIntegeri_v( GLenum pname, GLuint index, GLint 
> *params )
>params[3] = v.value_int_4[3];
>break;
> case TYPE_INT64:
> -  params[0] = INT64_TO_INT(v.value_int);
> +  params[0] = INT64_TO_INT(v.value_int64);
>break;
> default:
>; /* nothing - GL error was recorded */
> @@ -2067,7 +2067,7 @@ _mesa_GetInteger64i_v( GLenum pname, GLuint index, 
> GLint64 *params )
>params[3] = v.value_int_4[3];
>break;
> case TYPE_INT64:
> -  params[0] = v.value_int;
> +  params[0] = v.value_int64;
>break;
> default:
>; /* nothing - GL error was recorded */
> 

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

Re: [Mesa-dev] [Mesa-stable] [PATCH V2] mesa: Fix glGetVertexAttribi(GL_VERTEX_ATTRIB_ARRAY_SIZE)

[Ian Romanick]

I like this fix much better.

Reviewed-by: Ian Romanick 

On 03/12/2014 06:20 PM, Anuj Phogat wrote:
> mesa currently returns 4 when GL_VERTEX_ATTRIB_ARRAY_SIZE is queried
> for a vertex array initially set up with size=GL_BGRA. This patch
> makes changes to return size=GL_BGRA as required by the spec.
> 
> Fixes Khronos OpenGL CTS test: vertex_array_bgra_basic.test
> 
> V2: Use array->Format instead of adding a new variable
> 
> Signed-off-by: Anuj Phogat 
> Cc: 
> ---
>  src/mesa/main/varray.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/mesa/main/varray.c b/src/mesa/main/varray.c
> index b4b6fa9..479d872 100644
> --- a/src/mesa/main/varray.c
> +++ b/src/mesa/main/varray.c
> @@ -738,7 +738,7 @@ get_vertex_array_attrib(struct gl_context *ctx, GLuint 
> index, GLenum pname,
> case GL_VERTEX_ATTRIB_ARRAY_ENABLED_ARB:
>return array->Enabled;
> case GL_VERTEX_ATTRIB_ARRAY_SIZE_ARB:
> -  return array->Size;
> +  return (array->Format == GL_BGRA) ? GL_BGRA : array->Size;
> case GL_VERTEX_ATTRIB_ARRAY_STRIDE_ARB:
>return array->Stride;
> case GL_VERTEX_ATTRIB_ARRAY_TYPE_ARB:
> 

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

Re: [Mesa-dev] [PATCH 1/2] radeonsi: Add DMA ring

[Marek Olšák]

On Thu, Mar 13, 2014 at 8:45 AM, Niels Ole Salscheider
 wrote:
> Signed-off-by: Niels Ole Salscheider 
> ---
>  src/gallium/drivers/radeonsi/si_hw_context.c |  3 +++
>  src/gallium/drivers/radeonsi/si_pipe.c   | 22 ++
>  2 files changed, 25 insertions(+)
>
> diff --git a/src/gallium/drivers/radeonsi/si_hw_context.c 
> b/src/gallium/drivers/radeonsi/si_hw_context.c
> index c952c8d..d9fba01 100644
> --- a/src/gallium/drivers/radeonsi/si_hw_context.c
> +++ b/src/gallium/drivers/radeonsi/si_hw_context.c
> @@ -123,6 +123,9 @@ void si_context_flush(struct si_context *ctx, unsigned 
> flags)
>  #endif
>
> /* Flush the CS. */
> +   if (ctx->b.rings.dma.cs) {
> +   ctx->b.ws->cs_flush(ctx->b.rings.dma.cs, flags, 0);
> +   }
> ctx->b.ws->cs_flush(ctx->b.rings.gfx.cs, flags, 0);

This is a wrong place for flushing the DMA ring, because it always
flushes the graphics ring with it. The DMA flush should be independent
from the graphics flush. See what r600g does.

>
>  #if SI_TRACE_CS
> diff --git a/src/gallium/drivers/radeonsi/si_pipe.c 
> b/src/gallium/drivers/radeonsi/si_pipe.c
> index 827e9fe..21cbedf 100644
> --- a/src/gallium/drivers/radeonsi/si_pipe.c
> +++ b/src/gallium/drivers/radeonsi/si_pipe.c
> @@ -74,6 +74,24 @@ static void si_flush_from_winsys(void *ctx, unsigned flags)
> si_flush((struct pipe_context*)ctx, NULL, flags);
>  }
>
> +static void si_flush_dma_from_st(void *ctx, unsigned flags)
> +{
> +   struct si_context *sctx = (struct si_context *)ctx;
> +   struct radeon_winsys_cs *cs = sctx->b.rings.dma.cs;
> +
> +   if (!cs->cdw) {
> +   return;
> +   }
> +
> +   sctx->b.ws->cs_flush(cs, flags, 0);
> +}
> +
> +static void si_flush_dma_from_winsys(void *ctx, unsigned flags)
> +{
> +   struct si_context *sctx = (struct si_context *)ctx;
> +   sctx->b.rings.dma.flush(sctx, flags);
> +}
> +

Instead of adding these two, you can move the same r600g code to
drivers/radeon/r600_pipe_common.c and just use it there. I'd like all
this DMA ring management to live in drivers/radeon, because it doesn't
depend on r600_context at all.

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

Re: [Mesa-dev] removal of _glthread_GetID() breaks libglapi.so's ABI

[Brian Paul]

On 03/12/2014 11:31 PM, Chia-I Wu wrote:

Hi Brian,

_glthread_GetID() was removed by this commit

commit 02cb04c68ffbdaffaf7513ddc951584cac29f209
Author: Brian Paul 
Date:   Tue Mar 4 15:24:16 2014 -0700

 mesa: remove remaining uses of _glthread_GetID()

It turns out the function, declared in glapi.h, is a part of
libglapi.so ABI.  I am now getting this error when building GL
applications

/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../x86_64-linux-gnu/libGL.so:
undefined reference to `_glthread_GetID'

(gcc finds distro-provided libGL.so, which finds my build of
libglapi.so because of LD_LIBRARY_PATH).

I wonder if we should give the symbol available.  It could simply return zero...



Yeah, Jose said this might be a problem too.  I was waiting to see.

I think adding the dummy function you suggest is fine.  Do you want to 
write a patch for that?


-Brian

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

Re: [Mesa-dev] [PATCH] winsys/radeon: Store GPU virtual memory addresses of BOs in a hash table

[Marek Olšák]

Reviewed-by: Marek Olšák 

Marek

On Thu, Mar 13, 2014 at 9:34 AM, Michel Dänzer  wrote:
> From: Michel Dänzer 
>
> This allows retrieving the existing BO and incrementing its reference count,
> instead of creating a separate winsys representation for it, when the kernel
> reports that the BO was already assigned a virtual memory address.
>
> This fixes problems with XWayland using radeonsi and the
> xf86-video-wlglamor driver, which calls GEM flink outside of the radeon
> winsys code and creates BOs from the flinked names using the same DRM file
> descriptor.
>
> Signed-off-by: Michel Dänzer 
> ---
>  src/gallium/winsys/radeon/drm/radeon_drm_bo.c | 74 
> ++-
>  1 file changed, 26 insertions(+), 48 deletions(-)
>
> diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c 
> b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
> index 2ac060b..7dd5e7f 100644
> --- a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
> @@ -120,6 +120,8 @@ struct radeon_bomgr {
>  struct util_hash_table *bo_names;
>  /* List of buffer handles. Protectded by bo_handles_mutex. */
>  struct util_hash_table *bo_handles;
> +/* List of buffer virtual memory ranges. Protectded by bo_handles_mutex. 
> */
> +struct util_hash_table *bo_vas;
>  pipe_mutex bo_handles_mutex;
>  pipe_mutex bo_va_mutex;
>
> @@ -260,48 +262,6 @@ static uint64_t radeon_bomgr_find_va(struct radeon_bomgr 
> *mgr, uint64_t size, ui
>  return offset;
>  }
>
> -static void radeon_bomgr_force_va(struct radeon_bomgr *mgr, uint64_t va, 
> uint64_t size)
> -{
> -size = align(size, 4096);
> -
> -pipe_mutex_lock(mgr->bo_va_mutex);
> -if (va >= mgr->va_offset) {
> -if (va > mgr->va_offset) {
> -struct radeon_bo_va_hole *hole;
> -hole = CALLOC_STRUCT(radeon_bo_va_hole);
> -if (hole) {
> -hole->size = va - mgr->va_offset;
> -hole->offset = mgr->va_offset;
> -list_add(&hole->list, &mgr->va_holes);
> -}
> -}
> -mgr->va_offset = va + size;
> -} else {
> -struct radeon_bo_va_hole *hole, *n;
> -uint64_t hole_end, va_end;
> -
> -/* Prune/free all holes that fall into the range
> - */
> -LIST_FOR_EACH_ENTRY_SAFE(hole, n, &mgr->va_holes, list) {
> -hole_end = hole->offset + hole->size;
> -va_end = va + size;
> -if (hole->offset >= va_end || hole_end <= va)
> -continue;
> -if (hole->offset >= va && hole_end <= va_end) {
> -list_del(&hole->list);
> -FREE(hole);
> -continue;
> -}
> -if (hole->offset >= va)
> -hole->offset = va_end;
> -else
> -hole_end = va;
> -hole->size = hole_end - hole->offset;
> -}
> -}
> -pipe_mutex_unlock(mgr->bo_va_mutex);
> -}
> -
>  static void radeon_bomgr_free_va(struct radeon_bomgr *mgr, uint64_t va, 
> uint64_t size)
>  {
>  struct radeon_bo_va_hole *hole;
> @@ -625,11 +585,19 @@ static struct pb_buffer *radeon_bomgr_create_bo(struct 
> pb_manager *_mgr,
>  radeon_bo_destroy(&bo->base);
>  return NULL;
>  }
> +pipe_mutex_lock(mgr->bo_handles_mutex);
>  if (va.operation == RADEON_VA_RESULT_VA_EXIST) {
> -radeon_bomgr_free_va(mgr, bo->va, size);
> -bo->va = va.offset;
> -radeon_bomgr_force_va(mgr, bo->va, size);
> +struct pb_buffer *b = &bo->base;
> +struct radeon_bo *old_bo =
> +util_hash_table_get(mgr->bo_vas, 
> (void*)(uintptr_t)va.offset);
> +
> +pipe_mutex_unlock(mgr->bo_handles_mutex);
> +pb_reference(&b, &old_bo->base);
> +return b;
>  }
> +
> +util_hash_table_set(mgr->bo_vas, (void*)(uintptr_t)bo->va, bo);
> +pipe_mutex_unlock(mgr->bo_handles_mutex);
>  }
>
>  if (rdesc->initial_domains & RADEON_DOMAIN_VRAM)
> @@ -667,6 +635,7 @@ static void radeon_bomgr_destroy(struct pb_manager *_mgr)
>  struct radeon_bomgr *mgr = radeon_bomgr(_mgr);
>  util_hash_table_destroy(mgr->bo_names);
>  util_hash_table_destroy(mgr->bo_handles);
> +util_hash_table_destroy(mgr->bo_vas);
>  pipe_mutex_destroy(mgr->bo_handles_mutex);
>  pipe_mutex_destroy(mgr->bo_va_mutex);
>  FREE(mgr);
> @@ -700,6 +669,7 @@ struct pb_manager *radeon_bomgr_create(struct 
> radeon_drm_winsys *rws)
>  mgr->rws = rws;
>  mgr->bo_names = util_hash_table_create(handle_hash, handle_compare);
>  mgr->bo_handles = util_hash_table_create(handle_hash, handle_compare);
> +mgr->bo_vas = util_hash_table_create(handle_hash, handle_compare);
>  pipe_mutex_init(mgr->bo_handles_mutex);
>  pipe_mutex_init(mgr->bo_va_mutex);
>
> @@ -999,11 +969,19 @@ done:
>  radeon_bo_destroy

Re: [Mesa-dev] [Mesa-stable] [PATCH] mesa: return v.value_int64 when the requested type is TYPE_INT64

[Brian Paul]

On 03/13/2014 02:31 AM, Emil Velikov wrote:

Fixes "Operands don't affect result" defect reported by Coverity.

Cc: "9.2 10.0 10.1"  
Signed-off-by: Emil Velikov 
---
  src/mesa/main/get.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
index b190851..88cf202 100644
--- a/src/mesa/main/get.c
+++ b/src/mesa/main/get.c
@@ -1997,7 +1997,7 @@ _mesa_GetBooleani_v( GLenum pname, GLuint index, 
GLboolean *params )
params[3] = INT_TO_BOOLEAN(v.value_int_4[3]);
break;
 case TYPE_INT64:
-  params[0] = INT64_TO_BOOLEAN(v.value_int);
+  params[0] = INT64_TO_BOOLEAN(v.value_int64);
break;
 default:
; /* nothing - GL error was recorded */
@@ -2042,7 +2042,7 @@ _mesa_GetIntegeri_v( GLenum pname, GLuint index, GLint 
*params )
params[3] = v.value_int_4[3];
break;
 case TYPE_INT64:
-  params[0] = INT64_TO_INT(v.value_int);
+  params[0] = INT64_TO_INT(v.value_int64);
break;
 default:
; /* nothing - GL error was recorded */
@@ -2067,7 +2067,7 @@ _mesa_GetInteger64i_v( GLenum pname, GLuint index, 
GLint64 *params )
params[3] = v.value_int_4[3];
break;
 case TYPE_INT64:
-  params[0] = v.value_int;
+  params[0] = v.value_int64;
break;
 default:
; /* nothing - GL error was recorded */



Reviewed-by: Brian Paul 

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

Re: [Mesa-dev] [PATCH] mesa/main: condition GL_DEPTH_STENCIL on ARB_depth_texture

[Brian Paul]

On 03/13/2014 07:33 AM, Ilia Mirkin wrote:

On Thu, Mar 13, 2014 at 9:22 AM, Francisco Jerez  wrote:

Ilia Mirkin  writes:


EXT_packed_depth_stencil is supported by all drivers, but
ARB_depth_texture isn't (notably nouveau_vieux). This should avoid
passing unexpected values down to ChooseTextureFormat.

The EXT_packed_depth_stencil spec does not make any explicit references
to requiring ARB_depth_texture in order to allow textures with that
format, however if there is no dependency, ARB_depth_texture would be
practically implied by the extension.

Signed-off-by: Ilia Mirkin 
---

It should also be noted that ARB_depth_texture became required in OpenGL 1.4,
so this shouldn't affect too many drivers. Looks like i915 only has it
disabled for gen2. i965 always enables it. Glancing at radeon, it seems like
the values are supported in ChooseTextureFormat, but not actually handled in
radeon_texstate or r200_texstate.

  src/mesa/main/teximage.c | 11 +++
  1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
index a6c3581..a57a535 100644
--- a/src/mesa/main/teximage.c
+++ b/src/mesa/main/teximage.c
@@ -160,6 +160,9 @@ _mesa_base_tex_format( struct gl_context *ctx, GLint 
internalFormat )
   case GL_DEPTH_COMPONENT24:
   case GL_DEPTH_COMPONENT32:
  return GL_DEPTH_COMPONENT;
+ case GL_DEPTH_STENCIL:
+ case GL_DEPTH24_STENCIL8:
+return GL_DEPTH_STENCIL;
   default:
  ; /* fallthrough */
}
@@ -301,14 +304,6 @@ _mesa_base_tex_format( struct gl_context *ctx, GLint 
internalFormat )
}
 }

-   switch (internalFormat) {
-   case GL_DEPTH_STENCIL:
-   case GL_DEPTH24_STENCIL8:
-  return GL_DEPTH_STENCIL;
-   default:
-  ; /* fallthrough */
-   }
-
 if (ctx->Extensions.EXT_texture_sRGB) {
switch (internalFormat) {
case GL_SRGB_EXT:
--
1.8.3.2



Reviewed-by: Francisco Jerez 


Thanks. I'm going to wait a while before pushing this to see if others
have differing opinions on the spec interpretation. Ian, Eric, Brian?
You guys seem to be fairly in-tune with this stuff.


Looks OK to me.  Do you want to tag this for the stable branches?

-Brian


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

[Mesa-dev] [PATCH 3/8] gbm: Add a native intel backend

[Ander Conselvan de Oliveira]

From: Ander Conselvan de Oliveira 

---
 src/gbm/Makefile.am|  14 ++-
 src/gbm/backends/intel/gbm_intel.c | 241 +
 src/gbm/backends/intel/gbm_intel.h |  74 
 src/gbm/main/backend.c |   2 +
 src/gbm/main/common_drm.h  |   1 +
 5 files changed, 330 insertions(+), 2 deletions(-)
 create mode 100644 src/gbm/backends/intel/gbm_intel.c
 create mode 100644 src/gbm/backends/intel/gbm_intel.h

diff --git a/src/gbm/Makefile.am b/src/gbm/Makefile.am
index 6abd2a7..4428e53 100644
--- a/src/gbm/Makefile.am
+++ b/src/gbm/Makefile.am
@@ -44,6 +44,16 @@ libgbm_la_LIBADD += \
libgbm_dri.la $(top_builddir)/src/mapi/shared-glapi/libglapi.la 
$(LIBDRM_LIBS)
 endif
 
-TESTS = gbm-symbols-check
+if HAVE_I965_DRI
+noinst_LTLIBRARIES = libgbm_intel.la
+libgbm_intel_la_SOURCES = \
+   backends/intel/gbm_intel.c
+
+libgbm_intel_la_CFLAGS = \
+   $(AM_CFLAGS) $(INTEL_CFLAGS)
+
+libgbm_la_LIBADD += \
+   libgbm_intel.la $(INTEL_LIBS)
+endif
 
-include $(top_srcdir)/install-lib-links.mk
+TESTS = gbm-symbols-check
diff --git a/src/gbm/backends/intel/gbm_intel.c 
b/src/gbm/backends/intel/gbm_intel.c
new file mode 100644
index 000..2632e89
--- /dev/null
+++ b/src/gbm/backends/intel/gbm_intel.c
@@ -0,0 +1,241 @@
+/*
+ * Copyright © 2011-2013 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ *Ander Conselvan de Oliveira 
+ */
+
+#include 
+#include 
+#include 
+
+#include "gbm_intel.h"
+
+#include "gbmint.h"
+
+static int
+gbm_intel_is_format_supported(struct gbm_device *gbm,
+uint32_t format,
+uint32_t usage)
+{
+   switch (format) {
+   case GBM_BO_FORMAT_XRGB:
+   case GBM_FORMAT_XRGB:
+  break;
+   case GBM_BO_FORMAT_ARGB:
+   case GBM_FORMAT_ARGB:
+  if (usage & GBM_BO_USE_SCANOUT)
+ return 0;
+  break;
+   default:
+  return 0;
+   }
+
+   if (usage & GBM_BO_USE_CURSOR_64X64 &&
+   usage & GBM_BO_USE_RENDERING)
+  return 0;
+
+   return 1;
+}
+
+static int
+gbm_intel_bo_write(struct gbm_bo *bo, const void *buf, size_t count)
+{
+   struct gbm_intel_bo *ibo = gbm_intel_bo(bo);
+   int ret;
+
+   ret = drm_intel_bo_map(ibo->bo, 1);
+   if (ret < 0)
+  return ret;
+
+   memcpy(ibo->bo->virtual, buf, count);
+
+   return drm_intel_bo_unmap(ibo->bo);
+}
+
+static void
+gbm_intel_bo_destroy(struct gbm_bo *_bo)
+{
+   struct gbm_intel_bo *ibo = gbm_intel_bo(_bo);
+
+   drm_intel_bo_unreference(ibo->bo);
+
+   free(ibo);
+}
+
+static inline int
+align(int value, int size)
+{
+   return (value + size - 1) & ~(size - 1);
+}
+
+static struct gbm_intel_bo *
+gbm_intel_bo_create_with_bo(struct gbm_device *gbm,
+uint32_t width, uint32_t height, uint32_t stride,
+uint32_t format, uint32_t usage,
+drm_intel_bo *bo)
+{
+   struct gbm_intel_bo *ibo;
+
+   ibo = calloc(1, sizeof *ibo);
+   if (!ibo)
+  return NULL;
+
+   ibo->bo = bo;
+
+   ibo->base.base.gbm = gbm;
+   ibo->base.base.width = width;
+   ibo->base.base.height = height;
+   ibo->base.base.stride = stride;
+   ibo->base.base.format = format;
+   ibo->base.base.handle.s32 = ibo->bo->handle;
+
+   return ibo;
+}
+
+static struct gbm_bo *
+gbm_intel_bo_create(struct gbm_device *gbm,
+uint32_t width, uint32_t height,
+uint32_t format, uint32_t usage)
+{
+   struct gbm_intel_device *igbm = gbm_intel_device(gbm);
+   struct gbm_intel_bo *ibo;
+   drm_intel_bo *bo;
+   uint32_t tiling;
+   unsigned long stride, flags = 0;
+
+   switch (format) {
+   case GBM_BO_FORMAT_XRGB:
+   case GBM_FORMAT_XRGB:
+   case GBM_BO_FORMAT_ARGB:
+   case GBM_FORMAT_ARGB:
+  break;
+   default:
+  return NULL;
+   }
+
+   tiling = I915_TILING_X;
+
+  

[Mesa-dev] [PATCH 8/8] gbm: Add entry points for mapping and unmapping bos

[Ander Conselvan de Oliveira]

From: Ander Conselvan de Oliveira 

Add gbm_bo_map() and gbm_bo_unmap(). This lets a user access the
contents of a bo using the CPU.
---
 src/gbm/backends/dri/gbm_dri.c |  3 +++
 src/gbm/backends/intel/gbm_intel.c | 24 
 src/gbm/backends/intel/gbm_intel.h |  2 ++
 src/gbm/main/gbm.c | 25 +
 src/gbm/main/gbm.h | 10 ++
 src/gbm/main/gbmint.h  |  2 ++
 6 files changed, 66 insertions(+)

diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_dri.c
index acf6b24..45d3614 100644
--- a/src/gbm/backends/dri/gbm_dri.c
+++ b/src/gbm/backends/dri/gbm_dri.c
@@ -345,6 +345,9 @@ gbm_dri_is_format_supported(struct gbm_device *gbm,
usage & GBM_BO_USE_RENDERING)
   return 0;
 
+   if (usage & GBM_BO_USE_MAP)
+  return 0;
+
return 1;
 }
 
diff --git a/src/gbm/backends/intel/gbm_intel.c 
b/src/gbm/backends/intel/gbm_intel.c
index 2429457..7573cd5 100644
--- a/src/gbm/backends/intel/gbm_intel.c
+++ b/src/gbm/backends/intel/gbm_intel.c
@@ -73,6 +73,24 @@ gbm_intel_bo_write(struct gbm_bo *bo, const void *buf, 
size_t count)
return drm_intel_bo_unmap(ibo->bo);
 }
 
+static void *
+gbm_intel_bo_map(struct gbm_bo *bo)
+{
+   struct gbm_intel_bo *ibo = gbm_intel_bo(bo);
+
+   drm_intel_bo_map(ibo->bo, 1);
+
+   return ibo->bo->virtual;
+}
+
+static void
+gbm_intel_bo_unmap(struct gbm_bo *bo)
+{
+   struct gbm_intel_bo *ibo = gbm_intel_bo(bo);
+
+   drm_intel_bo_unmap(ibo->bo);
+}
+
 static void
 gbm_intel_bo_destroy(struct gbm_bo *_bo)
 {
@@ -102,6 +120,7 @@ gbm_intel_bo_create_with_bo(struct gbm_device *gbm,
   return NULL;
 
ibo->bo = bo;
+   ibo->usage = usage;
 
ibo->base.base.gbm = gbm;
ibo->base.base.width = width;
@@ -144,6 +163,9 @@ gbm_intel_bo_create(struct gbm_device *gbm,
   tiling = I915_TILING_NONE;
}
 
+   if (usage & GBM_BO_USE_MAP)
+  tiling = I915_TILING_NONE;
+
if (usage & GBM_BO_USE_RENDERING)
   flags |= BO_ALLOC_FOR_RENDER;
 
@@ -220,6 +242,8 @@ gbm_intel_device_create(int fd)
igbm->base.base.bo_import = gbm_intel_bo_import;
igbm->base.base.is_format_supported = gbm_intel_is_format_supported;
igbm->base.base.bo_write = gbm_intel_bo_write;
+   igbm->base.base.bo_map = gbm_intel_bo_map;
+   igbm->base.base.bo_unmap = gbm_intel_bo_unmap;
igbm->base.base.bo_destroy = gbm_intel_bo_destroy;
igbm->base.base.destroy = gbm_intel_destroy;
igbm->base.base.surface_create = gbm_intel_surface_create;
diff --git a/src/gbm/backends/intel/gbm_intel.h 
b/src/gbm/backends/intel/gbm_intel.h
index af0689b..1958355 100644
--- a/src/gbm/backends/intel/gbm_intel.h
+++ b/src/gbm/backends/intel/gbm_intel.h
@@ -46,6 +46,8 @@ struct gbm_intel_device {
 struct gbm_intel_bo {
struct gbm_drm_bo base;
 
+   uint32_t usage;
+
drm_intel_bo *bo;
 };
 
diff --git a/src/gbm/main/gbm.c b/src/gbm/main/gbm.c
index 30785a6..b4532f4 100644
--- a/src/gbm/main/gbm.c
+++ b/src/gbm/main/gbm.c
@@ -249,6 +249,31 @@ gbm_bo_write(struct gbm_bo *bo, const void *buf, size_t 
count)
return bo->gbm->bo_write(bo, buf, count);
 }
 
+/** Map a buffer object in CPU accesible memory
+ *
+ * \param bo The buffer object to be mapped
+ * \return Pointer to the mapped buffer object or NULL on failure
+ */
+GBM_EXPORT void *
+gbm_bo_map(struct gbm_bo *bo)
+{
+   if (bo->gbm->bo_map)
+  return bo->gbm->bo_map(bo);
+   else
+  return NULL;
+}
+
+/** Unmap a preivously mapped buffer object
+ *
+ * \param The buffer object to be unmapped
+ */
+GBM_EXPORT void
+gbm_bo_unmap(struct gbm_bo *bo)
+{
+   if (bo->gbm->bo_unmap)
+  bo->gbm->bo_unmap(bo);
+}
+
 /** Get the gbm device used to create the buffer object
  *
  * \param bo The buffer object
diff --git a/src/gbm/main/gbm.h b/src/gbm/main/gbm.h
index 9d2a030..63ba624 100644
--- a/src/gbm/main/gbm.h
+++ b/src/gbm/main/gbm.h
@@ -207,6 +207,10 @@ enum gbm_bo_flags {
 * combinations.
 */
GBM_BO_USE_WRITE= (1 << 3),
+   /**
+* Buffer can be mapped with gbm_bo_map().
+*/
+   GBM_BO_USE_MAP = (1 << 4),
 };
 
 int
@@ -258,6 +262,12 @@ gbm_bo_get_handle(struct gbm_bo *bo);
 int
 gbm_bo_write(struct gbm_bo *bo, const void *buf, size_t count);
 
+void *
+gbm_bo_map(struct gbm_bo *bo);
+
+void
+gbm_bo_unmap(struct gbm_bo *bo);
+
 void
 gbm_bo_set_user_data(struct gbm_bo *bo, void *data,
 void (*destroy_user_data)(struct gbm_bo *, void *));
diff --git a/src/gbm/main/gbmint.h b/src/gbm/main/gbmint.h
index 549fa3e..83eb426 100644
--- a/src/gbm/main/gbmint.h
+++ b/src/gbm/main/gbmint.h
@@ -69,6 +69,8 @@ struct gbm_device {
struct gbm_bo *(*bo_import)(struct gbm_device *gbm, uint32_t type,
void *buffer, uint32_t usage);
int (*bo_write)(struct gbm_bo *bo, const void *buf, size_t data);
+   void *(*bo_map)(struct gbm_bo *bo);
+   void (*bo_unmap)(struct gbm_bo *bo);
void (*bo_destroy)(struct gbm_bo *bo);
 
struct gbm_surface *(*sur

[Mesa-dev] [PATCH 6/8] dri, i965: Add entry point for creating image from native handle

[Ander Conselvan de Oliveira]

From: Ander Conselvan de Oliveira 

The native handle is a pointer to drm_intel_bo for the i965 driver.
---
 include/GL/internal/dri_interface.h   | 13 +++-
 src/mesa/drivers/dri/i965/intel_regions.c | 50 ++-
 src/mesa/drivers/dri/i965/intel_regions.h |  6 
 src/mesa/drivers/dri/i965/intel_screen.c  | 37 +--
 4 files changed, 82 insertions(+), 24 deletions(-)

diff --git a/include/GL/internal/dri_interface.h 
b/include/GL/internal/dri_interface.h
index 6c2312a..552dd2a 100644
--- a/include/GL/internal/dri_interface.h
+++ b/include/GL/internal/dri_interface.h
@@ -1005,7 +1005,7 @@ struct __DRIdri2ExtensionRec {
  * extensions.
  */
 #define __DRI_IMAGE "DRI_IMAGE"
-#define __DRI_IMAGE_VERSION 8
+#define __DRI_IMAGE_VERSION 9
 
 /**
  * These formats correspond to the similarly named MESA_FORMAT_*
@@ -1239,6 +1239,17 @@ struct __DRIimageExtensionRec {
  enum __DRIChromaSiting vert_siting,
  unsigned *error,
  void *loaderPrivate);
+   /**
+* Create image from a driver-dependent handle. Depends on the shared
+* bufmgr extension.
+*
+* \since 9 ?
+*/
+   __DRIimage *(*createImageFromHandle)(__DRIscreen *screen,
+ int width, int height, int format,
+ void *handle, int pitch,
+ void *loaderPrivate);
+
 };
 
 
diff --git a/src/mesa/drivers/dri/i965/intel_regions.c 
b/src/mesa/drivers/dri/i965/intel_regions.c
index d891e09..745bfe6 100644
--- a/src/mesa/drivers/dri/i965/intel_regions.c
+++ b/src/mesa/drivers/dri/i965/intel_regions.c
@@ -172,30 +172,47 @@ intel_region_flink(struct intel_region *region, uint32_t 
*name)
 }
 
 struct intel_region *
-intel_region_alloc_for_handle(struct intel_screen *screen,
+intel_region_alloc_for_buffer(struct intel_screen *screen,
  GLuint cpp,
  GLuint width, GLuint height, GLuint pitch,
- GLuint handle, const char *name)
+ drm_intel_bo *buffer, const char *name)
 {
struct intel_region *region;
-   drm_intel_bo *buffer;
int ret;
uint32_t bit_6_swizzle, tiling;
 
-   buffer = intel_bo_gem_create_from_name(screen->bufmgr, name, handle);
-   if (buffer == NULL)
-  return NULL;
ret = drm_intel_bo_get_tiling(buffer, &tiling, &bit_6_swizzle);
if (ret != 0) {
-  fprintf(stderr, "Couldn't get tiling of buffer %d (%s): %s\n",
- handle, name, strerror(-ret));
-  drm_intel_bo_unreference(buffer);
+  fprintf(stderr, "Couldn't get tiling of buffer (%s): %s\n",
+ name, strerror(-ret));
   return NULL;
}
 
region = intel_region_alloc_internal(screen, cpp,
width, height, pitch, tiling, buffer);
-   if (region == NULL) {
+   if (region == NULL)
+  return NULL;
+
+   return region;
+}
+
+struct intel_region *
+intel_region_alloc_for_handle(struct intel_screen *screen,
+ GLuint cpp,
+ GLuint width, GLuint height, GLuint pitch,
+ GLuint handle, const char *name)
+{
+   struct intel_region *region;
+   drm_intel_bo *buffer;
+
+   buffer = intel_bo_gem_create_from_name(screen->bufmgr, name, handle);
+   if (buffer == NULL)
+  return NULL;
+
+   region = intel_region_alloc_for_buffer(screen, cpp, width, height, pitch,
+ buffer, name);
+
+   if (!region) {
   drm_intel_bo_unreference(buffer);
   return NULL;
}
@@ -214,22 +231,13 @@ intel_region_alloc_for_fd(struct intel_screen *screen,
 {
struct intel_region *region;
drm_intel_bo *buffer;
-   int ret;
-   uint32_t bit_6_swizzle, tiling;
 
buffer = drm_intel_bo_gem_create_from_prime(screen->bufmgr, fd, size);
if (buffer == NULL)
   return NULL;
-   ret = drm_intel_bo_get_tiling(buffer, &tiling, &bit_6_swizzle);
-   if (ret != 0) {
-  fprintf(stderr, "Couldn't get tiling of buffer (%s): %s\n",
- name, strerror(-ret));
-  drm_intel_bo_unreference(buffer);
-  return NULL;
-   }
 
-   region = intel_region_alloc_internal(screen, cpp,
-   width, height, pitch, tiling, buffer);
+   region = intel_region_alloc_for_buffer(screen, cpp, width, height, pitch,
+ buffer, name);
if (region == NULL) {
   drm_intel_bo_unreference(buffer);
   return NULL;
diff --git a/src/mesa/drivers/dri/i965/intel_regions.h 
b/src/mesa/drivers/dri/i965/intel_regions.h
index eb2123e..a35eae8 100644
--- a/src/mesa/drivers/dri/i965/intel_regions.h
+++ b/src/mesa/drivers/dri/i965/intel_regions.h
@@ -83,6 +83,12 @@ struct intel_region *intel_region_alloc(struct intel_screen 
*screen,
   

[Mesa-dev] [PATCH 4/8] gbm_drm: Keep a reference to drm native objects

[Ander Conselvan de Oliveira]

From: Ander Conselvan de Oliveira 

Add bo and bufmgr fields to gbm_drm_bo and gbm_drm_device respectively.
---
 src/gbm/backends/intel/gbm_intel.c | 4 
 src/gbm/main/common_drm.h  | 6 ++
 2 files changed, 10 insertions(+)

diff --git a/src/gbm/backends/intel/gbm_intel.c 
b/src/gbm/backends/intel/gbm_intel.c
index 2632e89..2429457 100644
--- a/src/gbm/backends/intel/gbm_intel.c
+++ b/src/gbm/backends/intel/gbm_intel.c
@@ -110,6 +110,8 @@ gbm_intel_bo_create_with_bo(struct gbm_device *gbm,
ibo->base.base.format = format;
ibo->base.base.handle.s32 = ibo->bo->handle;
 
+   ibo->base.bo = bo;
+
return ibo;
 }
 
@@ -232,6 +234,8 @@ gbm_intel_device_create(int fd)
   return NULL;
}
 
+   igbm->base.bufmgr = igbm->bufmgr;
+
return &igbm->base.base;
 }
 
diff --git a/src/gbm/main/common_drm.h b/src/gbm/main/common_drm.h
index 9fa0716..c49a6f9 100644
--- a/src/gbm/main/common_drm.h
+++ b/src/gbm/main/common_drm.h
@@ -40,10 +40,16 @@ struct gbm_drm_device {
struct gbm_device base;
enum gbm_drm_driver_type type;
char *driver_name;
+
+   /* Driver dependent buffer manager object */
+   void *bufmgr;
 };
 
 struct gbm_drm_bo {
struct gbm_bo base;
+
+   /* Driver dependent buffer object */
+   void *bo;
 };
 
 #endif
-- 
1.8.3.2

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

[Mesa-dev] [PATCH 5/8] dri, i965: Add an extension for sharing the drm bufmgr

[Ander Conselvan de Oliveira]

From: Ander Conselvan de Oliveira 

If the fd used by the DRI driver is going to be shared with something
else, like gbm, this lets the loader pass the bufmgr struct so it is
shared too.
---
 include/GL/internal/dri_interface.h  | 11 +++
 src/egl/drivers/dri2/egl_dri2.h  |  3 ++-
 src/mesa/drivers/dri/common/dri_util.c   |  2 ++
 src/mesa/drivers/dri/common/dri_util.h   |  1 +
 src/mesa/drivers/dri/i965/intel_screen.c |  9 +++--
 5 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/include/GL/internal/dri_interface.h 
b/include/GL/internal/dri_interface.h
index d028d05..6c2312a 100644
--- a/include/GL/internal/dri_interface.h
+++ b/include/GL/internal/dri_interface.h
@@ -1424,4 +1424,15 @@ struct __DRIimageDriverExtensionRec {
__DRIgetAPIMaskFunc  getAPIMask;
 };
 
+/**
+ * Allows the DRI screen to share a low-level (drm) bufmgr with gbm.
+ */
+#define __DRI_SHARED_BUFMGR "DRI_SHARED_BUFMGR"
+#define __DRI_SHARED_BUFMGR_VERSION 1
+
+typedef struct __DRIsharedBufmgrExtensionRec {
+   __DRIextension base;
+   void *bufmgr;
+} __DRIsharedBufmgrExtension;
+
 #endif
diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
index b8281e8..6030293 100644
--- a/src/egl/drivers/dri2/egl_dri2.h
+++ b/src/egl/drivers/dri2/egl_dri2.h
@@ -117,7 +117,8 @@ struct dri2_egl_display
 
__DRIdri2LoaderExtensiondri2_loader_extension;
__DRIswrastLoaderExtension  swrast_loader_extension;
-   const __DRIextension *extensions[5];
+   __DRIsharedBufmgrExtension  shared_bufmgr_extension;
+   const __DRIextension *extensions[6];
const __DRIextension**driver_extensions;
 
 #ifdef HAVE_X11_PLATFORM
diff --git a/src/mesa/drivers/dri/common/dri_util.c 
b/src/mesa/drivers/dri/common/dri_util.c
index aed73c7..3046d60 100644
--- a/src/mesa/drivers/dri/common/dri_util.c
+++ b/src/mesa/drivers/dri/common/dri_util.c
@@ -81,6 +81,8 @@ setupLoaderExtensions(__DRIscreen *psp,
psp->swrast_loader = (__DRIswrastLoaderExtension *) extensions[i];
 if (strcmp(extensions[i]->name, __DRI_IMAGE_LOADER) == 0)
psp->image.loader = (__DRIimageLoaderExtension *) extensions[i];
+   if (strcmp(extensions[i]->name, __DRI_SHARED_BUFMGR) == 0)
+   psp->dri2.bufmgr = (__DRIsharedBufmgrExtension *) extensions[i];
 }
 }
 
diff --git a/src/mesa/drivers/dri/common/dri_util.h 
b/src/mesa/drivers/dri/common/dri_util.h
index a37a0bb..49c380c 100644
--- a/src/mesa/drivers/dri/common/dri_util.h
+++ b/src/mesa/drivers/dri/common/dri_util.h
@@ -177,6 +177,7 @@ struct __DRIscreenRec {
__DRIdri2LoaderExtension *loader;
__DRIimageLookupExtension *image;
__DRIuseInvalidateExtension *useInvalidate;
+   __DRIsharedBufmgrExtension *bufmgr;
 } dri2;
 
 struct {
diff --git a/src/mesa/drivers/dri/i965/intel_screen.c 
b/src/mesa/drivers/dri/i965/intel_screen.c
index 464cebf..cbbef8f 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -942,8 +942,10 @@ static void
 intelDestroyScreen(__DRIscreen * sPriv)
 {
struct intel_screen *intelScreen = sPriv->driverPrivate;
+   __DRIscreen *spriv = intelScreen->driScrnPriv;
 
-   dri_bufmgr_destroy(intelScreen->bufmgr);
+   if (!spriv->dri2.bufmgr)
+  dri_bufmgr_destroy(intelScreen->bufmgr);
driDestroyOptionInfo(&intelScreen->optionCache);
 
free(intelScreen);
@@ -1059,7 +1061,10 @@ intel_init_bufmgr(struct intel_screen *intelScreen)
 
intelScreen->no_hw = getenv("INTEL_NO_HW") != NULL;
 
-   intelScreen->bufmgr = intel_bufmgr_gem_init(spriv->fd, BATCH_SZ);
+   if (spriv->dri2.bufmgr)
+  intelScreen->bufmgr = spriv->dri2.bufmgr->bufmgr;
+   else
+  intelScreen->bufmgr = intel_bufmgr_gem_init(spriv->fd, BATCH_SZ);
if (intelScreen->bufmgr == NULL) {
   fprintf(stderr, "[%s:%u] Error initializing buffer manager.\n",
  __func__, __LINE__);
-- 
1.8.3.2

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

[Mesa-dev] [PATCH 0/8] Native GBM backends and map/unmap support

[Ander Conselvan de Oliveira]

From: Ander Conselvan de Oliveira 

Hi,

This patch series implements an API for mapping an unapping GBM buffer
objects. The native intel backend was implemented so that the map and
unmap functionality wouldn't have to be implemented in the DRI image
extension. A new EGL platform is necessary, since platform_drm.c
assumes it is run on top of a gbm device with the dri backend.

This new platform is written so that it could support other native
backends, not only intel.

Comments are really welcome.

Thanks,
Ander

Ander Conselvan de Oliveira (8):
  egl/drm: Rename dri2_egl_surface field gbm_surf to gbm_dri_surf
  egl: Protect use of gbm_dri with ifdef HAVE_DRM_PLATFORM
  gbm: Add a native intel backend
  gbm_drm: Keep a reference to drm native objects
  dri, i965: Add an extension for sharing the drm bufmgr
  dri, i965: Add entry point for creating image from native handle
  egl/dri: Add gbm platform
  gbm: Add entry points for mapping and unmapping bos

 configure.ac  |   7 +-
 include/GL/internal/dri_interface.h   |  24 +-
 src/egl/drivers/dri2/Makefile.am  |   5 +
 src/egl/drivers/dri2/egl_dri2.c   |   8 +
 src/egl/drivers/dri2/egl_dri2.h   |  16 +-
 src/egl/drivers/dri2/platform_drm.c   |   6 +-
 src/egl/drivers/dri2/platform_gbm.c   | 486 ++
 src/egl/main/Makefile.am  |   5 +
 src/egl/main/egldisplay.c |   3 +-
 src/egl/main/egldisplay.h |   1 +
 src/gbm/Makefile.am   |  14 +-
 src/gbm/backends/dri/gbm_dri.c|   3 +
 src/gbm/backends/intel/gbm_intel.c| 270 +
 src/gbm/backends/intel/gbm_intel.h|  76 +
 src/gbm/main/backend.c|   2 +
 src/gbm/main/common_drm.h |  13 +
 src/gbm/main/gbm.c|  25 ++
 src/gbm/main/gbm.h|  10 +
 src/gbm/main/gbmint.h |   4 +
 src/mesa/drivers/dri/common/dri_util.c|   2 +
 src/mesa/drivers/dri/common/dri_util.h|   1 +
 src/mesa/drivers/dri/i965/intel_regions.c |  50 +--
 src/mesa/drivers/dri/i965/intel_regions.h |   6 +
 src/mesa/drivers/dri/i965/intel_screen.c  |  46 ++-
 24 files changed, 1044 insertions(+), 39 deletions(-)
 create mode 100644 src/egl/drivers/dri2/platform_gbm.c
 create mode 100644 src/gbm/backends/intel/gbm_intel.c
 create mode 100644 src/gbm/backends/intel/gbm_intel.h

-- 
1.8.3.2

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

[Mesa-dev] [PATCH 7/8] egl/dri: Add gbm platform

[Ander Conselvan de Oliveira]

From: Ander Conselvan de Oliveira 

This adds a GBM platform that works with native gbm backends, such
as the intel backend.
---
 configure.ac|   7 +-
 src/egl/drivers/dri2/Makefile.am|   5 +
 src/egl/drivers/dri2/egl_dri2.c |   6 +
 src/egl/drivers/dri2/egl_dri2.h |  11 +-
 src/egl/drivers/dri2/platform_gbm.c | 486 
 src/egl/main/Makefile.am|   5 +
 src/egl/main/egldisplay.c   |   3 +-
 src/egl/main/egldisplay.h   |   1 +
 src/gbm/main/common_drm.h   |   6 +
 src/gbm/main/gbmint.h   |   2 +
 10 files changed, 526 insertions(+), 6 deletions(-)
 create mode 100644 src/egl/drivers/dri2/platform_gbm.c

diff --git a/configure.ac b/configure.ac
index c5042f9..6a2aa07 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1406,11 +1406,11 @@ for plat in $egl_platforms; do
PKG_CHECK_MODULES([XCB_DRI2], [x11-xcb xcb-dri2 >= 
$XCBDRI2_REQUIRED xcb-xfixes])
;;
 
-   drm)
+   drm|gbm)
test "x$enable_gbm" = "xno" &&
-   AC_MSG_ERROR([EGL platform drm needs gbm])
+   AC_MSG_ERROR([EGL platform '$plat' needs gbm])
test "x$have_libdrm" != xyes &&
-   AC_MSG_ERROR([EGL platform drm requires libdrm >= 
$LIBDRM_REQUIRED])
+   AC_MSG_ERROR([EGL platform '$plat' requires libdrm >= 
$LIBDRM_REQUIRED])
;;
 
android|fbdev|gdi|null)
@@ -1445,6 +1445,7 @@ AM_CONDITIONAL(HAVE_EGL_PLATFORM_WAYLAND, echo 
"$egl_platforms" | grep 'wayland'
 AM_CONDITIONAL(HAVE_EGL_PLATFORM_DRM, echo "$egl_platforms" | grep 'drm' 
>/dev/null 2>&1)
 AM_CONDITIONAL(HAVE_EGL_PLATFORM_FBDEV, echo "$egl_platforms" | grep 'fbdev' 
>/dev/null 2>&1)
 AM_CONDITIONAL(HAVE_EGL_PLATFORM_NULL, echo "$egl_platforms" | grep 'null' 
>/dev/null 2>&1)
+AM_CONDITIONAL(HAVE_EGL_PLATFORM_GBM, echo "$egl_platforms" | grep 'gbm' 
>/dev/null 2>&1)
 
 AM_CONDITIONAL(HAVE_EGL_DRIVER_DRI2, test "x$HAVE_EGL_DRIVER_DRI2" != "x")
 
diff --git a/src/egl/drivers/dri2/Makefile.am b/src/egl/drivers/dri2/Makefile.am
index 0c1625a..5454fcc 100644
--- a/src/egl/drivers/dri2/Makefile.am
+++ b/src/egl/drivers/dri2/Makefile.am
@@ -62,3 +62,8 @@ if HAVE_EGL_PLATFORM_DRM
 libegl_dri2_la_SOURCES += platform_drm.c
 AM_CFLAGS += -DHAVE_DRM_PLATFORM
 endif
+
+if HAVE_EGL_PLATFORM_GBM
+libegl_dri2_la_SOURCES += platform_gbm.c
+AM_CFLAGS += -DHAVE_GBM_PLATFORM
+endif
diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 2e8f01b..343a776 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -633,6 +633,12 @@ dri2_initialize(_EGLDriver *drv, _EGLDisplay *disp)
  return EGL_TRUE;
   return dri2_initialize_drm(drv, disp);
 #endif
+#ifdef HAVE_GBM_PLATFORM
+   case _EGL_PLATFORM_GBM:
+  if (disp->Options.TestOnly)
+ return EGL_TRUE;
+  return dri2_initialize_gbm(drv, disp);
+#endif
 #ifdef HAVE_WAYLAND_PLATFORM
case _EGL_PLATFORM_WAYLAND:
   if (disp->Options.TestOnly)
diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
index 6030293..40fccf2 100644
--- a/src/egl/drivers/dri2/egl_dri2.h
+++ b/src/egl/drivers/dri2/egl_dri2.h
@@ -183,14 +183,18 @@ struct dri2_egl_surface
struct gbm_dri_surface *gbm_dri_surf;
 #endif
 
-#if defined(HAVE_WAYLAND_PLATFORM) || defined(HAVE_DRM_PLATFORM)
+#ifdef HAVE_GBM_PLATFORM
+   struct gbm_surface *gbm_surf;
+#endif
+
+#if defined(HAVE_WAYLAND_PLATFORM) || defined(HAVE_DRM_PLATFORM) || 
defined(HAVE_GBM_PLATFORM)
__DRIbuffer   *dri_buffers[__DRI_BUFFER_COUNT];
struct {
 #ifdef HAVE_WAYLAND_PLATFORM
   struct wl_buffer   *wl_buffer;
   __DRIimage *dri_image;
 #endif
-#ifdef HAVE_DRM_PLATFORM
+#if defined(HAVE_DRM_PLATFORM) || defined(HAVE_GBM_PLATFORM)
   struct gbm_bo   *bo;
 #endif
   int locked;
@@ -267,6 +271,9 @@ EGLBoolean
 dri2_initialize_drm(_EGLDriver *drv, _EGLDisplay *disp);
 
 EGLBoolean
+dri2_initialize_gbm(_EGLDriver *drv, _EGLDisplay *disp);
+
+EGLBoolean
 dri2_initialize_wayland(_EGLDriver *drv, _EGLDisplay *disp);
 
 EGLBoolean
diff --git a/src/egl/drivers/dri2/platform_gbm.c 
b/src/egl/drivers/dri2/platform_gbm.c
new file mode 100644
index 000..1e3fbb5
--- /dev/null
+++ b/src/egl/drivers/dri2/platform_gbm.c
@@ -0,0 +1,486 @@
+/*
+ * Copyright © 2011-2013 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission not

[Mesa-dev] [PATCH 2/8] egl: Protect use of gbm_dri with ifdef HAVE_DRM_PLATFORM

[Ander Conselvan de Oliveira]

From: Ander Conselvan de Oliveira 

Otherwise it fails to compile if the drm egl platform is disabled.
---
 src/egl/drivers/dri2/egl_dri2.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 892f1f4..2e8f01b 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -1894,10 +1894,12 @@ dri2_bind_wayland_display_wl(_EGLDriver *drv, 
_EGLDisplay *disp,
if (!dri2_dpy->wl_server_drm)
   return EGL_FALSE;
 
+#ifdef HAVE_DRM_PLATFORM
/* We have to share the wl_drm instance with gbm, so gbm can convert
 * wl_buffers to gbm bos. */
if (dri2_dpy->gbm_dri)
   dri2_dpy->gbm_dri->wl_drm = dri2_dpy->wl_server_drm;
+#endif
 
return EGL_TRUE;
 }
-- 
1.8.3.2

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

[Mesa-dev] [PATCH 1/8] egl/drm: Rename dri2_egl_surface field gbm_surf to gbm_dri_surf

[Ander Conselvan de Oliveira]

From: Ander Conselvan de Oliveira 

It actually contains a pointer to a gbm_dri_surface, so use a more
appropriate name.
---
 src/egl/drivers/dri2/egl_dri2.h | 2 +-
 src/egl/drivers/dri2/platform_drm.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
index dfc5927..b8281e8 100644
--- a/src/egl/drivers/dri2/egl_dri2.h
+++ b/src/egl/drivers/dri2/egl_dri2.h
@@ -179,7 +179,7 @@ struct dri2_egl_surface
 #endif
 
 #ifdef HAVE_DRM_PLATFORM
-   struct gbm_dri_surface *gbm_surf;
+   struct gbm_dri_surface *gbm_dri_surf;
 #endif
 
 #if defined(HAVE_WAYLAND_PLATFORM) || defined(HAVE_DRM_PLATFORM)
diff --git a/src/egl/drivers/dri2/platform_drm.c 
b/src/egl/drivers/dri2/platform_drm.c
index a2b387d..a0f6ccb 100644
--- a/src/egl/drivers/dri2/platform_drm.c
+++ b/src/egl/drivers/dri2/platform_drm.c
@@ -111,7 +111,7 @@ dri2_create_surface(_EGLDriver *drv, _EGLDisplay *disp, 
EGLint type,
   if (!window)
  return NULL;
   surf = gbm_dri_surface((struct gbm_surface *) window);
-  dri2_surf->gbm_surf = surf;
+  dri2_surf->gbm_dri_surf = surf;
   dri2_surf->base.Width =  surf->base.width;
   dri2_surf->base.Height = surf->base.height;
   surf->dri_private = dri2_surf;
@@ -123,7 +123,7 @@ dri2_create_surface(_EGLDriver *drv, _EGLDisplay *disp, 
EGLint type,
dri2_surf->dri_drawable =
   (*dri2_dpy->dri2->createNewDrawable) (dri2_dpy->dri_screen,
dri2_conf->dri_double_config,
-   dri2_surf->gbm_surf);
+   dri2_surf->gbm_dri_surf);
 
if (dri2_surf->dri_drawable == NULL) {
   _eglError(EGL_BAD_ALLOC, "dri2->createNewDrawable");
@@ -180,7 +180,7 @@ get_back_bo(struct dri2_egl_surface *dri2_surf)
 {
struct dri2_egl_display *dri2_dpy =
   dri2_egl_display(dri2_surf->base.Resource.Display);
-   struct gbm_dri_surface *surf = dri2_surf->gbm_surf;
+   struct gbm_dri_surface *surf = dri2_surf->gbm_dri_surf;
int i;
 
if (dri2_surf->back == NULL) {
-- 
1.8.3.2

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

Re: [Mesa-dev] [PATCH 0/7] EGL_MESA_configless_context

[Neil Roberts]

Kristian Høgsberg  writes:

> Thanks Neil, series applied to mesa and weston.

Great, thanks!

I've reposted the piglit patch to the piglit list in case someone wants
to review it there.

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

Re: [Mesa-dev] [PATCH] mesa/main: condition GL_DEPTH_STENCIL on ARB_depth_texture

[Ilia Mirkin]

On Thu, Mar 13, 2014 at 9:22 AM, Francisco Jerez  wrote:
> Ilia Mirkin  writes:
>
>> EXT_packed_depth_stencil is supported by all drivers, but
>> ARB_depth_texture isn't (notably nouveau_vieux). This should avoid
>> passing unexpected values down to ChooseTextureFormat.
>>
>> The EXT_packed_depth_stencil spec does not make any explicit references
>> to requiring ARB_depth_texture in order to allow textures with that
>> format, however if there is no dependency, ARB_depth_texture would be
>> practically implied by the extension.
>>
>> Signed-off-by: Ilia Mirkin 
>> ---
>>
>> It should also be noted that ARB_depth_texture became required in OpenGL 1.4,
>> so this shouldn't affect too many drivers. Looks like i915 only has it
>> disabled for gen2. i965 always enables it. Glancing at radeon, it seems like
>> the values are supported in ChooseTextureFormat, but not actually handled in
>> radeon_texstate or r200_texstate.
>>
>>  src/mesa/main/teximage.c | 11 +++
>>  1 file changed, 3 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
>> index a6c3581..a57a535 100644
>> --- a/src/mesa/main/teximage.c
>> +++ b/src/mesa/main/teximage.c
>> @@ -160,6 +160,9 @@ _mesa_base_tex_format( struct gl_context *ctx, GLint 
>> internalFormat )
>>   case GL_DEPTH_COMPONENT24:
>>   case GL_DEPTH_COMPONENT32:
>>  return GL_DEPTH_COMPONENT;
>> + case GL_DEPTH_STENCIL:
>> + case GL_DEPTH24_STENCIL8:
>> +return GL_DEPTH_STENCIL;
>>   default:
>>  ; /* fallthrough */
>>}
>> @@ -301,14 +304,6 @@ _mesa_base_tex_format( struct gl_context *ctx, GLint 
>> internalFormat )
>>}
>> }
>>
>> -   switch (internalFormat) {
>> -   case GL_DEPTH_STENCIL:
>> -   case GL_DEPTH24_STENCIL8:
>> -  return GL_DEPTH_STENCIL;
>> -   default:
>> -  ; /* fallthrough */
>> -   }
>> -
>> if (ctx->Extensions.EXT_texture_sRGB) {
>>switch (internalFormat) {
>>case GL_SRGB_EXT:
>> --
>> 1.8.3.2
>
>
> Reviewed-by: Francisco Jerez 

Thanks. I'm going to wait a while before pushing this to see if others
have differing opinions on the spec interpretation. Ian, Eric, Brian?
You guys seem to be fairly in-tune with this stuff.

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

Re: [Mesa-dev] [PATCH] mesa/main: condition GL_DEPTH_STENCIL on ARB_depth_texture

[Francisco Jerez]

Ilia Mirkin  writes:

> EXT_packed_depth_stencil is supported by all drivers, but
> ARB_depth_texture isn't (notably nouveau_vieux). This should avoid
> passing unexpected values down to ChooseTextureFormat.
>
> The EXT_packed_depth_stencil spec does not make any explicit references
> to requiring ARB_depth_texture in order to allow textures with that
> format, however if there is no dependency, ARB_depth_texture would be
> practically implied by the extension.
>
> Signed-off-by: Ilia Mirkin 
> ---
>
> It should also be noted that ARB_depth_texture became required in OpenGL 1.4,
> so this shouldn't affect too many drivers. Looks like i915 only has it
> disabled for gen2. i965 always enables it. Glancing at radeon, it seems like
> the values are supported in ChooseTextureFormat, but not actually handled in
> radeon_texstate or r200_texstate.
>
>  src/mesa/main/teximage.c | 11 +++
>  1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
> index a6c3581..a57a535 100644
> --- a/src/mesa/main/teximage.c
> +++ b/src/mesa/main/teximage.c
> @@ -160,6 +160,9 @@ _mesa_base_tex_format( struct gl_context *ctx, GLint 
> internalFormat )
>   case GL_DEPTH_COMPONENT24:
>   case GL_DEPTH_COMPONENT32:
>  return GL_DEPTH_COMPONENT;
> + case GL_DEPTH_STENCIL:
> + case GL_DEPTH24_STENCIL8:
> +return GL_DEPTH_STENCIL;
>   default:
>  ; /* fallthrough */
>}
> @@ -301,14 +304,6 @@ _mesa_base_tex_format( struct gl_context *ctx, GLint 
> internalFormat )
>}
> }
>  
> -   switch (internalFormat) {
> -   case GL_DEPTH_STENCIL:
> -   case GL_DEPTH24_STENCIL8:
> -  return GL_DEPTH_STENCIL;
> -   default:
> -  ; /* fallthrough */
> -   }
> -
> if (ctx->Extensions.EXT_texture_sRGB) {
>switch (internalFormat) {
>case GL_SRGB_EXT:
> -- 
> 1.8.3.2


Reviewed-by: Francisco Jerez 

Thanks.


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

[Mesa-dev] [PATCH] mesa/main: condition GL_DEPTH_STENCIL on ARB_depth_texture

[Ilia Mirkin]

EXT_packed_depth_stencil is supported by all drivers, but
ARB_depth_texture isn't (notably nouveau_vieux). This should avoid
passing unexpected values down to ChooseTextureFormat.

The EXT_packed_depth_stencil spec does not make any explicit references
to requiring ARB_depth_texture in order to allow textures with that
format, however if there is no dependency, ARB_depth_texture would be
practically implied by the extension.

Signed-off-by: Ilia Mirkin 
---

It should also be noted that ARB_depth_texture became required in OpenGL 1.4,
so this shouldn't affect too many drivers. Looks like i915 only has it
disabled for gen2. i965 always enables it. Glancing at radeon, it seems like
the values are supported in ChooseTextureFormat, but not actually handled in
radeon_texstate or r200_texstate.

 src/mesa/main/teximage.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
index a6c3581..a57a535 100644
--- a/src/mesa/main/teximage.c
+++ b/src/mesa/main/teximage.c
@@ -160,6 +160,9 @@ _mesa_base_tex_format( struct gl_context *ctx, GLint 
internalFormat )
  case GL_DEPTH_COMPONENT24:
  case GL_DEPTH_COMPONENT32:
 return GL_DEPTH_COMPONENT;
+ case GL_DEPTH_STENCIL:
+ case GL_DEPTH24_STENCIL8:
+return GL_DEPTH_STENCIL;
  default:
 ; /* fallthrough */
   }
@@ -301,14 +304,6 @@ _mesa_base_tex_format( struct gl_context *ctx, GLint 
internalFormat )
   }
}
 
-   switch (internalFormat) {
-   case GL_DEPTH_STENCIL:
-   case GL_DEPTH24_STENCIL8:
-  return GL_DEPTH_STENCIL;
-   default:
-  ; /* fallthrough */
-   }
-
if (ctx->Extensions.EXT_texture_sRGB) {
   switch (internalFormat) {
   case GL_SRGB_EXT:
-- 
1.8.3.2

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

Re: [Mesa-dev] Z24S8 textures on nv04/nv10

[Ilia Mirkin]

On Thu, Mar 13, 2014 at 7:36 AM, Francisco Jerez  wrote:
> Ilia Mirkin  writes:
>
>> On Thu, Mar 13, 2014 at 6:55 AM, Ilia Mirkin  wrote:
>>> On Thu, Mar 13, 2014 at 6:43 AM, Ilia Mirkin  wrote:
 On Thu, Mar 13, 2014 at 6:42 AM, Francisco Jerez  
 wrote:
> Ilia Mirkin  writes:
>
>> Hi Francisco,
>>
>> I'm looking at some piglit failures on nv10... full run at
>>
>> http://people.freedesktop.org/~imirkin/nv10-comparison/problems.html
>>
>> The specific one is:
>>
>> http://people.freedesktop.org/~imirkin/nv10-comparison/nv18-imirkin/spec/!OpenGL%201.1/copyteximage%201D.html
>>
>> Which fails in nouveau_choose_tex_format for GL_DEPTH24_STENCIL8. And
>> indeed, that case isn't handled. Looking through the #defines, I see a
>> NV20_3D_TEX_FORMAT_FORMAT_Z24 but not for the earlier versions. And
>> even in the nv20 case, it's not actually used in the driver.
>>
>> Does that mean that EXT_packed_depth_stencil should never have been
>> enabled on nouveau? I do see that it's a valid renderbuffer format
>> though. [It is now always enabled, since commit a92b9e60, but at the
>> time of the commit, it was also enabled for everything.
>>
>> Any advice?
>>
>
> AFAICT EXT_packed_depth_stencil only requires the implementation to
> support GL_DEPTH_STENCIL for texturing if ARB_depth_texture is supported
> in addition -- which, as you guessed correctly, could be supported on
> nv20 or higher.
>
> Shouldn't the piglit test be checking if the implementation supports
> depth texturing before doing that?

 Mmmaybe. But that's not such a big deal -- worst case, the piglit
 should get a gl error of some sort. Sounds like the real issue is that
 mesa core is letting the values through to the driver. I'll re-read
 EXT_packed_depth_stencil more carefully, I didn't see the
 ARB_depth_texture dependency, but I only scanned it very quickly.
>>>
>>> Aha, and in _mesa_base_tex_format, we have
>>>
>>>if (ctx->Extensions.ARB_depth_texture) {
>>>   switch (internalFormat) {
>>>  case GL_DEPTH_COMPONENT:
>>>  case GL_DEPTH_COMPONENT16:
>>>  case GL_DEPTH_COMPONENT24:
>>>  case GL_DEPTH_COMPONENT32:
>>> return GL_DEPTH_COMPONENT;
>>>  default:
>>> ; /* fallthrough */
>>>   }
>>>}
>>>
>>> followed by
>>>
>>>switch (internalFormat) {
>>>case GL_DEPTH_STENCIL:
>>>case GL_DEPTH24_STENCIL8:
>>>   return GL_DEPTH_STENCIL;
>>>default:
>>>   ; /* fallthrough */
>>>}
>>>
>>> And I think you're right about these only being available for 
>>> ARB_depth_texture:
>>>
>>> If ARB_depth_texture or SGIX_depth_texture is
>>> supported, GL_DEPTH_STENCIL_EXT/GL_UNSIGNED_INT_24_8_EXT data can
>>> also be used for textures; this provides a more efficient way to
>>> supply data for a 24-bit depth texture.
>>>
>>> This doesn't reference GL_DEPTH24_STENCIL8 explicitly, but I can only
>>> assume that's a spec bug.
>>
>> H... OTOH, that's perhaps more correctly interpreted as "You can
>> pass GL_DEPTH_STENCIL data to a GL_DEPTH_COMPONENT texture". Which
>> would in turn mean that EXT_packed_depth_stencil does require
>> GL_DEPTH_STENCIL texture support...
>>
>
> Why would they do something like that?  If ARB_depth_texture is not
> supported, I don't see how it would make sense to forbid "passing" Z24S8
> data to a texture, but still require support for it as internal format.

I think it's more like saying that if you _do_ have ARB_depth_texture,
then even though the ARB_depth_texture spec says nothing about letting
data passed to glTexImage2D being in Z24X8 format, you can still do
that for a texture with a DEPTH_COMPONENT internal format, instead of
it being in the unaligned Z24 format.

>
> I just checked what the nvidia blob does: It exposes
> EXT_packed_depth_stencil on nv1x and it doesn't support depth texturing,
> just like we do.

If this is a live system (rather than a saved-off glxinfo), mind
running the piglit test that I referenced originally and see what it
says for the GL_DEPTH_STENCIL bit? Perhaps they do something clever to
enable depth texturing...

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

Re: [Mesa-dev] Z24S8 textures on nv04/nv10

[Francisco Jerez]

Ilia Mirkin  writes:

> On Thu, Mar 13, 2014 at 6:55 AM, Ilia Mirkin  wrote:
>> On Thu, Mar 13, 2014 at 6:43 AM, Ilia Mirkin  wrote:
>>> On Thu, Mar 13, 2014 at 6:42 AM, Francisco Jerez  
>>> wrote:
 Ilia Mirkin  writes:

> Hi Francisco,
>
> I'm looking at some piglit failures on nv10... full run at
>
> http://people.freedesktop.org/~imirkin/nv10-comparison/problems.html
>
> The specific one is:
>
> http://people.freedesktop.org/~imirkin/nv10-comparison/nv18-imirkin/spec/!OpenGL%201.1/copyteximage%201D.html
>
> Which fails in nouveau_choose_tex_format for GL_DEPTH24_STENCIL8. And
> indeed, that case isn't handled. Looking through the #defines, I see a
> NV20_3D_TEX_FORMAT_FORMAT_Z24 but not for the earlier versions. And
> even in the nv20 case, it's not actually used in the driver.
>
> Does that mean that EXT_packed_depth_stencil should never have been
> enabled on nouveau? I do see that it's a valid renderbuffer format
> though. [It is now always enabled, since commit a92b9e60, but at the
> time of the commit, it was also enabled for everything.
>
> Any advice?
>

 AFAICT EXT_packed_depth_stencil only requires the implementation to
 support GL_DEPTH_STENCIL for texturing if ARB_depth_texture is supported
 in addition -- which, as you guessed correctly, could be supported on
 nv20 or higher.

 Shouldn't the piglit test be checking if the implementation supports
 depth texturing before doing that?
>>>
>>> Mmmaybe. But that's not such a big deal -- worst case, the piglit
>>> should get a gl error of some sort. Sounds like the real issue is that
>>> mesa core is letting the values through to the driver. I'll re-read
>>> EXT_packed_depth_stencil more carefully, I didn't see the
>>> ARB_depth_texture dependency, but I only scanned it very quickly.
>>
>> Aha, and in _mesa_base_tex_format, we have
>>
>>if (ctx->Extensions.ARB_depth_texture) {
>>   switch (internalFormat) {
>>  case GL_DEPTH_COMPONENT:
>>  case GL_DEPTH_COMPONENT16:
>>  case GL_DEPTH_COMPONENT24:
>>  case GL_DEPTH_COMPONENT32:
>> return GL_DEPTH_COMPONENT;
>>  default:
>> ; /* fallthrough */
>>   }
>>}
>>
>> followed by
>>
>>switch (internalFormat) {
>>case GL_DEPTH_STENCIL:
>>case GL_DEPTH24_STENCIL8:
>>   return GL_DEPTH_STENCIL;
>>default:
>>   ; /* fallthrough */
>>}
>>
>> And I think you're right about these only being available for 
>> ARB_depth_texture:
>>
>> If ARB_depth_texture or SGIX_depth_texture is
>> supported, GL_DEPTH_STENCIL_EXT/GL_UNSIGNED_INT_24_8_EXT data can
>> also be used for textures; this provides a more efficient way to
>> supply data for a 24-bit depth texture.
>>
>> This doesn't reference GL_DEPTH24_STENCIL8 explicitly, but I can only
>> assume that's a spec bug.
>
> H... OTOH, that's perhaps more correctly interpreted as "You can
> pass GL_DEPTH_STENCIL data to a GL_DEPTH_COMPONENT texture". Which
> would in turn mean that EXT_packed_depth_stencil does require
> GL_DEPTH_STENCIL texture support...
>

Why would they do something like that?  If ARB_depth_texture is not
supported, I don't see how it would make sense to forbid "passing" Z24S8
data to a texture, but still require support for it as internal format.

I just checked what the nvidia blob does: It exposes
EXT_packed_depth_stencil on nv1x and it doesn't support depth texturing,
just like we do.

> Anyone else have opinions on this?
>
>   -ilia


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

Re: [Mesa-dev] Z24S8 textures on nv04/nv10

[Ilia Mirkin]

On Thu, Mar 13, 2014 at 6:55 AM, Ilia Mirkin  wrote:
> On Thu, Mar 13, 2014 at 6:43 AM, Ilia Mirkin  wrote:
>> On Thu, Mar 13, 2014 at 6:42 AM, Francisco Jerez  
>> wrote:
>>> Ilia Mirkin  writes:
>>>
 Hi Francisco,

 I'm looking at some piglit failures on nv10... full run at

 http://people.freedesktop.org/~imirkin/nv10-comparison/problems.html

 The specific one is:

 http://people.freedesktop.org/~imirkin/nv10-comparison/nv18-imirkin/spec/!OpenGL%201.1/copyteximage%201D.html

 Which fails in nouveau_choose_tex_format for GL_DEPTH24_STENCIL8. And
 indeed, that case isn't handled. Looking through the #defines, I see a
 NV20_3D_TEX_FORMAT_FORMAT_Z24 but not for the earlier versions. And
 even in the nv20 case, it's not actually used in the driver.

 Does that mean that EXT_packed_depth_stencil should never have been
 enabled on nouveau? I do see that it's a valid renderbuffer format
 though. [It is now always enabled, since commit a92b9e60, but at the
 time of the commit, it was also enabled for everything.

 Any advice?

>>>
>>> AFAICT EXT_packed_depth_stencil only requires the implementation to
>>> support GL_DEPTH_STENCIL for texturing if ARB_depth_texture is supported
>>> in addition -- which, as you guessed correctly, could be supported on
>>> nv20 or higher.
>>>
>>> Shouldn't the piglit test be checking if the implementation supports
>>> depth texturing before doing that?
>>
>> Mmmaybe. But that's not such a big deal -- worst case, the piglit
>> should get a gl error of some sort. Sounds like the real issue is that
>> mesa core is letting the values through to the driver. I'll re-read
>> EXT_packed_depth_stencil more carefully, I didn't see the
>> ARB_depth_texture dependency, but I only scanned it very quickly.
>
> Aha, and in _mesa_base_tex_format, we have
>
>if (ctx->Extensions.ARB_depth_texture) {
>   switch (internalFormat) {
>  case GL_DEPTH_COMPONENT:
>  case GL_DEPTH_COMPONENT16:
>  case GL_DEPTH_COMPONENT24:
>  case GL_DEPTH_COMPONENT32:
> return GL_DEPTH_COMPONENT;
>  default:
> ; /* fallthrough */
>   }
>}
>
> followed by
>
>switch (internalFormat) {
>case GL_DEPTH_STENCIL:
>case GL_DEPTH24_STENCIL8:
>   return GL_DEPTH_STENCIL;
>default:
>   ; /* fallthrough */
>}
>
> And I think you're right about these only being available for 
> ARB_depth_texture:
>
> If ARB_depth_texture or SGIX_depth_texture is
> supported, GL_DEPTH_STENCIL_EXT/GL_UNSIGNED_INT_24_8_EXT data can
> also be used for textures; this provides a more efficient way to
> supply data for a 24-bit depth texture.
>
> This doesn't reference GL_DEPTH24_STENCIL8 explicitly, but I can only
> assume that's a spec bug.

H... OTOH, that's perhaps more correctly interpreted as "You can
pass GL_DEPTH_STENCIL data to a GL_DEPTH_COMPONENT texture". Which
would in turn mean that EXT_packed_depth_stencil does require
GL_DEPTH_STENCIL texture support...

Anyone else have opinions on this?

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

Re: [Mesa-dev] Z24S8 textures on nv04/nv10

[Ilia Mirkin]

On Thu, Mar 13, 2014 at 6:43 AM, Ilia Mirkin  wrote:
> On Thu, Mar 13, 2014 at 6:42 AM, Francisco Jerez  
> wrote:
>> Ilia Mirkin  writes:
>>
>>> Hi Francisco,
>>>
>>> I'm looking at some piglit failures on nv10... full run at
>>>
>>> http://people.freedesktop.org/~imirkin/nv10-comparison/problems.html
>>>
>>> The specific one is:
>>>
>>> http://people.freedesktop.org/~imirkin/nv10-comparison/nv18-imirkin/spec/!OpenGL%201.1/copyteximage%201D.html
>>>
>>> Which fails in nouveau_choose_tex_format for GL_DEPTH24_STENCIL8. And
>>> indeed, that case isn't handled. Looking through the #defines, I see a
>>> NV20_3D_TEX_FORMAT_FORMAT_Z24 but not for the earlier versions. And
>>> even in the nv20 case, it's not actually used in the driver.
>>>
>>> Does that mean that EXT_packed_depth_stencil should never have been
>>> enabled on nouveau? I do see that it's a valid renderbuffer format
>>> though. [It is now always enabled, since commit a92b9e60, but at the
>>> time of the commit, it was also enabled for everything.
>>>
>>> Any advice?
>>>
>>
>> AFAICT EXT_packed_depth_stencil only requires the implementation to
>> support GL_DEPTH_STENCIL for texturing if ARB_depth_texture is supported
>> in addition -- which, as you guessed correctly, could be supported on
>> nv20 or higher.
>>
>> Shouldn't the piglit test be checking if the implementation supports
>> depth texturing before doing that?
>
> Mmmaybe. But that's not such a big deal -- worst case, the piglit
> should get a gl error of some sort. Sounds like the real issue is that
> mesa core is letting the values through to the driver. I'll re-read
> EXT_packed_depth_stencil more carefully, I didn't see the
> ARB_depth_texture dependency, but I only scanned it very quickly.

Aha, and in _mesa_base_tex_format, we have

   if (ctx->Extensions.ARB_depth_texture) {
  switch (internalFormat) {
 case GL_DEPTH_COMPONENT:
 case GL_DEPTH_COMPONENT16:
 case GL_DEPTH_COMPONENT24:
 case GL_DEPTH_COMPONENT32:
return GL_DEPTH_COMPONENT;
 default:
; /* fallthrough */
  }
   }

followed by

   switch (internalFormat) {
   case GL_DEPTH_STENCIL:
   case GL_DEPTH24_STENCIL8:
  return GL_DEPTH_STENCIL;
   default:
  ; /* fallthrough */
   }

And I think you're right about these only being available for ARB_depth_texture:

If ARB_depth_texture or SGIX_depth_texture is
supported, GL_DEPTH_STENCIL_EXT/GL_UNSIGNED_INT_24_8_EXT data can
also be used for textures; this provides a more efficient way to
supply data for a 24-bit depth texture.

This doesn't reference GL_DEPTH24_STENCIL8 explicitly, but I can only
assume that's a spec bug.

I'll send a patch.

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

Re: [Mesa-dev] Z24S8 textures on nv04/nv10

[Ilia Mirkin]

On Thu, Mar 13, 2014 at 6:42 AM, Francisco Jerez  wrote:
> Ilia Mirkin  writes:
>
>> Hi Francisco,
>>
>> I'm looking at some piglit failures on nv10... full run at
>>
>> http://people.freedesktop.org/~imirkin/nv10-comparison/problems.html
>>
>> The specific one is:
>>
>> http://people.freedesktop.org/~imirkin/nv10-comparison/nv18-imirkin/spec/!OpenGL%201.1/copyteximage%201D.html
>>
>> Which fails in nouveau_choose_tex_format for GL_DEPTH24_STENCIL8. And
>> indeed, that case isn't handled. Looking through the #defines, I see a
>> NV20_3D_TEX_FORMAT_FORMAT_Z24 but not for the earlier versions. And
>> even in the nv20 case, it's not actually used in the driver.
>>
>> Does that mean that EXT_packed_depth_stencil should never have been
>> enabled on nouveau? I do see that it's a valid renderbuffer format
>> though. [It is now always enabled, since commit a92b9e60, but at the
>> time of the commit, it was also enabled for everything.
>>
>> Any advice?
>>
>
> AFAICT EXT_packed_depth_stencil only requires the implementation to
> support GL_DEPTH_STENCIL for texturing if ARB_depth_texture is supported
> in addition -- which, as you guessed correctly, could be supported on
> nv20 or higher.
>
> Shouldn't the piglit test be checking if the implementation supports
> depth texturing before doing that?

Mmmaybe. But that's not such a big deal -- worst case, the piglit
should get a gl error of some sort. Sounds like the real issue is that
mesa core is letting the values through to the driver. I'll re-read
EXT_packed_depth_stencil more carefully, I didn't see the
ARB_depth_texture dependency, but I only scanned it very quickly.

Thanks,

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

Re: [Mesa-dev] Z24S8 textures on nv04/nv10

[Francisco Jerez]

Ilia Mirkin  writes:

> Hi Francisco,
>
> I'm looking at some piglit failures on nv10... full run at
>
> http://people.freedesktop.org/~imirkin/nv10-comparison/problems.html
>
> The specific one is:
>
> http://people.freedesktop.org/~imirkin/nv10-comparison/nv18-imirkin/spec/!OpenGL%201.1/copyteximage%201D.html
>
> Which fails in nouveau_choose_tex_format for GL_DEPTH24_STENCIL8. And
> indeed, that case isn't handled. Looking through the #defines, I see a
> NV20_3D_TEX_FORMAT_FORMAT_Z24 but not for the earlier versions. And
> even in the nv20 case, it's not actually used in the driver.
>
> Does that mean that EXT_packed_depth_stencil should never have been
> enabled on nouveau? I do see that it's a valid renderbuffer format
> though. [It is now always enabled, since commit a92b9e60, but at the
> time of the commit, it was also enabled for everything.
>
> Any advice?
>

AFAICT EXT_packed_depth_stencil only requires the implementation to
support GL_DEPTH_STENCIL for texturing if ARB_depth_texture is supported
in addition -- which, as you guessed correctly, could be supported on
nv20 or higher.

Shouldn't the piglit test be checking if the implementation supports
depth texturing before doing that?

> Thanks,
>
>   -ilia


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

[Mesa-dev] Wrong colors in 3D apps on big-endian systems

[Christian Zigotzky]

On 13.03.2014 11:17, Timothy Arceri wrote:


This is nothing to do with the patches. You are running softpipe not the
the r600 driver.

libGL error: failed to load driver: r600
OpenGL vendor string: VMware, Inc.
OpenGL renderer string: Gallium 0.4 on softpipe

You can try running this to get more information on why the driver is
not loading:
$ LIBGL_DEBUG=verbose glxinfo | grep direct


Hi Timothy,

Thanks for the hint. I get some error messages:

libGL: OpenDriver: trying /usr/local/mesa-git-20140313/lib/dri/r600_dri.so
libGL: driver does not expose __driDriverGetExtensions_r600(): 
/usr/local/mesa-git-20140313/lib/dri/r600_dri.so: undefined symbol: 
__driDriverGetExtensions_r600
libGL: Can't open configuration file /home/christian/.drirc: No such 
file or directory.
libGL: Can't open configuration file /home/christian/.drirc: No such 
file or directory.

libGL error: failed to load driver: r600
libGL: OpenDriver: trying /usr/local/mesa-git-20140313/lib/dri/swrast_dri.so
libGL: driver does not expose __driDriverGetExtensions_swrast(): 
/usr/local/mesa-git-20140313/lib/dri/swrast_dri.so: undefined symbol: 
__driDriverGetExtensions_swrast
libGL: Can't open configuration file /home/christian/.drirc: No such 
file or directory.
libGL: Can't open configuration file /home/christian/.drirc: No such 
file or directory.

OpenGL vendor string: VMware, Inc.
OpenGL renderer string: Gallium 0.4 on softpipe
OpenGL version string: 2.1 Mesa 10.2.0-devel (git-551d459)
OpenGL shading language version string: 1.30
OpenGL extensions:

Rgds,

Christian

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

Re: [Mesa-dev] [PATCH 3/8] glsl: Fold implementation of ir_dereference_variable::constant_referenced into wrapper

[Pohjolainen, Topi]

On Wed, Mar 12, 2014 at 03:49:22PM -0700, Ian Romanick wrote:
> From: Ian Romanick 
> 
> Signed-off-by: Ian Romanick 
> ---
>  src/glsl/ir_constant_expression.cpp | 25 +
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/src/glsl/ir_constant_expression.cpp 
> b/src/glsl/ir_constant_expression.cpp
> index 4149a0e..c013dfd 100644
> --- a/src/glsl/ir_constant_expression.cpp
> +++ b/src/glsl/ir_constant_expression.cpp
> @@ -400,6 +400,12 @@ constant_referenced(const ir_dereference *deref,
>  struct hash_table *variable_context,
>  ir_constant *&store, int &offset)
>  {
> +   store = NULL;
> +   offset = 0;
> +
> +   if (variable_context == NULL)
> +  return false;
> +
> switch (deref->ir_type) {
> case ir_type_dereference_array:
>((ir_dereference_array *) deref)->constant_referenced(variable_context,
> @@ -411,15 +417,16 @@ constant_referenced(const ir_dereference *deref,
>   store, offset);
>break;
>  
> -   case ir_type_dereference_variable:
> -  ((ir_dereference_variable *) 
> deref)->constant_referenced(variable_context,
> -   store, 
> offset);
> +   case ir_type_dereference_variable: {
> +  const ir_dereference_variable *const dv =
> + (const ir_dereference_variable *) deref;

I was wondering why you introduced the explicit braces here as the surrounding
cases do not have them. But this is in order for the compiler to allow the
introduction of the new stack variable, right?

> +
> +  store = (ir_constant *) hash_table_find(variable_context, dv->var);
>break;
> +   }
>  
> default:
>assert(!"Should not get here.");
> -  store = NULL;
> -  offset = 0;
>break;
> }
>  
> @@ -430,13 +437,7 @@ void
>  ir_dereference_variable::constant_referenced(struct hash_table 
> *variable_context,
>ir_constant *&store, int &offset) 
> const
>  {
> -   if (variable_context) {
> -  store = (ir_constant *)hash_table_find(variable_context, var);
> -  offset = 0;
> -   } else {
> -  store = NULL;
> -  offset = 0;
> -   }
> +   ::constant_referenced(this, variable_context, store, offset);
>  }
>  
>  void
> -- 
> 1.8.1.4
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/8] glsl: Add wrapper function that calls ir_dereference::constant_referenced

[Pohjolainen, Topi]

On Wed, Mar 12, 2014 at 07:20:04PM -0400, Connor Abbott wrote:
> On Wed, Mar 12, 2014 at 6:49 PM, Ian Romanick  wrote:
> > From: Ian Romanick 
> >
> > Signed-off-by: Ian Romanick 
> > ---
> >  src/glsl/ir_constant_expression.cpp | 52 
> > +
> >  1 file changed, 36 insertions(+), 16 deletions(-)
> >
> > diff --git a/src/glsl/ir_constant_expression.cpp 
> > b/src/glsl/ir_constant_expression.cpp
> > index a31e579..4149a0e 100644
> > --- a/src/glsl/ir_constant_expression.cpp
> > +++ b/src/glsl/ir_constant_expression.cpp
> > @@ -395,6 +395,37 @@ unpack_half_1x16(uint16_t u)
> >   * The offset is used when the reference is to a specific column of a 
> > matrix.
> >   */
> >  /*@{*/
> > +static bool
> > +constant_referenced(const ir_dereference *deref,
> > +struct hash_table *variable_context,
> > +ir_constant *&store, int &offset)
> > +{
> > +   switch (deref->ir_type) {
> > +   case ir_type_dereference_array:
> > +  ((ir_dereference_array *) 
> > deref)->constant_referenced(variable_context,
> > +store, offset);
> > +  break;
> > +
> > +   case ir_type_dereference_record:
> > +  ((ir_dereference_record *) 
> > deref)->constant_referenced(variable_context,
> > + store, 
> > offset);
> > +  break;
> > +
> > +   case ir_type_dereference_variable:
> > +  ((ir_dereference_variable *) 
> > deref)->constant_referenced(variable_context,
> > +   store, 
> > offset);
> > +  break;
> > +
> > +   default:
> > +  assert(!"Should not get here.");
> > +  store = NULL;
> > +  offset = 0;
> > +  break;
> > +   }
> > +
> > +   return store != NULL;
> > +}
> > +
> >  void
> >  ir_dereference_variable::constant_referenced(struct hash_table 
> > *variable_context,
> >  ir_constant *&store, int 
> > &offset) const
> > @@ -433,13 +464,8 @@ ir_dereference_array::constant_referenced(struct 
> > hash_table *variable_context,
> >return;
> > }
> >
> > -   deref->constant_referenced(variable_context, substore, suboffset);
> > -
> > -   if (!substore) {
> > -  store = 0;
> > -  offset = 0;
> > +   if (!::constant_referenced(deref, variable_context, substore, 
> > suboffset))
> >return;
> > -   }
> 
> Question: why do you have the :: here...

Here the caller is "ir_dereference_variable::constant_referenced()". This is
telling the compiler to call the static newly introduced function instead of
recurring to here again. Same applies for the "ir_dereference_record()" below.

> 
> >
> > const glsl_type *vt = array->type;
> > if (vt->is_array()) {
> > @@ -475,13 +501,8 @@ ir_dereference_record::constant_referenced(struct 
> > hash_table *variable_context,
> >return;
> > }
> >
> > -   deref->constant_referenced(variable_context, substore, suboffset);
> > -
> > -   if (!substore) {
> > -  store = 0;
> > -  offset = 0;
> > +   if (!::constant_referenced(deref, variable_context, substore, 
> > suboffset))
> >return;
> > -   }
> 
> and here...
> 
> >
> > store = substore->get_record_field(field);
> > offset = 0;
> > @@ -1814,9 +1835,8 @@ bool 
> > ir_function_signature::constant_expression_evaluate_expression_list(const s
> >
> >  ir_constant *store = NULL;
> >  int offset = 0;
> > -asg->lhs->constant_referenced(variable_context, store, offset);
> >
> > -if (!store)
> > +if (!constant_referenced(asg->lhs, variable_context, store, 
> > offset))
> > return false;
> 
> but not here...

Here in turn there is no choice for the compiler to make as
"ir_function_signature" does not have a member function called
"constant_referenced()".

> 
> >
> >  ir_constant *value = 
> > asg->rhs->constant_expression_value(variable_context);
> > @@ -1847,9 +1867,9 @@ bool 
> > ir_function_signature::constant_expression_evaluate_expression_list(const s
> >
> >  ir_constant *store = NULL;
> >  int offset = 0;
> > -call->return_deref->constant_referenced(variable_context, store, 
> > offset);
> >
> > -if (!store)
> > +if (!constant_referenced(call->return_deref, variable_context,
> > +  store, offset))
> 
> or here? Compiler issues?
> 
> > return false;
> >
> >  ir_constant *value = 
> > call->constant_expression_value(variable_context);
> > --
> > 1.8.1.4
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_

Re: [Mesa-dev] Wrong colors in 3D apps on big-endian systems

[Timothy Arceri]

On Thu, 2014-03-13 at 10:38 +0100, Christian Zigotzky wrote:
> Hi Michel,
> 
> Thank you very much for your effort. I've patched the latest git version 
> with your patches. Mesa works with the right colors but it doesn't have 
> 3D acceleration. Mesa 10.1.0 and 10.0.3 works with 3D acceleration.

This is nothing to do with the patches. You are running softpipe not the
the r600 driver.

libGL error: failed to load driver: r600
OpenGL vendor string: VMware, Inc.
OpenGL renderer string: Gallium 0.4 on softpipe

You can try running this to get more information on why the driver is
not loading: 
$ LIBGL_DEBUG=verbose glxinfo | grep direct 

> 
> My system: Nemo board with a 1.8GHz dual core PA6T processer and an AMD 
> Radeon HD 6870 graphics card.
> 
> Mesa/standard (mesa-git-20140313):
> 
> glxinfo | grep OpenGL
> 
> libGL error: failed to load driver: r600
> OpenGL vendor string: VMware, Inc.
> OpenGL renderer string: Gallium 0.4 on softpipe
> OpenGL version string: 2.1 Mesa 10.2.0-devel (git-551d459)
> OpenGL shading language version string: 1.30
> OpenGL extensions:
> 
> Glxgears works with the right colors without 3D acceleration.
> 
> Mesa/llvmpipe (mesa-git-20140313):
> 
> OpenGL vendor string: VMware, Inc.
> OpenGL renderer string: Gallium 0.4 on llvmpipe (LLVM 3.3, 128 bits)
> OpenGL version string: 2.1 Mesa 10.2.0-devel (git-551d459)
> OpenGL shading language version string: 1.30
> OpenGL extensions:
> 
> Glxgears doesn't work with the following error messages:
> 
> LLVM ERROR: Do not know how to split the result of this operator!
> 
> Mesa/standard 10.1.0 with my changes
> 
> glxinfo | grep OpenGL
> 
> OpenGL vendor string: X.Org
> OpenGL renderer string: Gallium 0.4 on AMD BARTS
> OpenGL version string: 2.1 Mesa 10.1.0
> OpenGL shading language version string: 1.30
> OpenGL extensions:
> 
> Glxgears, Neverball, and SuperTuxKart work with the right colors and 3D 
> acceleration.
> 
> 
> Michel, do you have any tips for me, please? Thanks a lot for your help.
> 
> All the best,
> 
> Christian
> 
> ___
> 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] [RFC PATCH 3/4] mesa: Prefer non-swizzled formats when prefer_no_swizzle set

[Francisco Jerez]

Chris Forbes  writes:

> Actually, after poking around a bit more, I think this is correct...
>
> As of Brian's commit 657436da7ee, the packed formats are described as
> being laid out from the LSB. This is consistent with this patch, with
> the pack/unpack code, and the hardware documentation:
>
> mesa: update packed format layout comments
>
> Update the comments for the packed formats to accurately reflect the
> layout of the bits in the pixel.  For example, for the packed format
> MESA_FORMAT_R8G8B8A8, R is in the least significant position while A
> is in the most-significant position of the 32-bit word.
>
> Unfortunately the example higher up of how a type-P format is laid out
> is still backwards, which adds to the confusion.
>

Oh...  That's right...  But doesn't that mean that your choice is
backwards on BE machines?

> -- Chris
>
> On Wed, Mar 12, 2014 at 12:36 PM, Chris Forbes  wrote:
>> Yeah, you're right -- that looks bogus. There's been piles of
>> confusion recently in formats.h about which end things are laid out
>> from; the truth though is obviously in the pack/unpack code and the
>> hardware format documentation.
>>
>> On Wed, Mar 12, 2014 at 9:12 AM, Francisco Jerez  
>> wrote:
>>> Chris Forbes  writes:
>>>
 If prefer_no_swizzle is set, try:
 - The exact matching format
 - Formats with the required components in the correct order, plus a junk
   component
 - finally, swizzled formats (BGRA etc)

 Signed-off-by: Chris Forbes 
 ---
  src/mesa/main/texformat.c | 35 +++
  1 file changed, 31 insertions(+), 4 deletions(-)

 diff --git a/src/mesa/main/texformat.c b/src/mesa/main/texformat.c
 index 33725dc..7cb42bc 100644
 --- a/src/mesa/main/texformat.c
 +++ b/src/mesa/main/texformat.c
 @@ -79,11 +79,13 @@ _mesa_choose_tex_format(struct gl_context *ctx, GLenum 
 target,
} else if (type == GL_UNSIGNED_INT_2_10_10_10_REV) {
   RETURN_IF_SUPPORTED(MESA_FORMAT_B10G10R10A2_UNORM);
}
 -  RETURN_IF_SUPPORTED(MESA_FORMAT_A8B8G8R8_UNORM);
 -  RETURN_IF_SUPPORTED(MESA_FORMAT_B8G8R8A8_UNORM);
 -  break;
 +  /* fallthrough */

 case GL_RGBA8:
 +  if (prefer_no_swizzle) {
 + RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8A8_UNORM);
 + break;
 +  }
>>>
>>> Are you sure this is correct on little endian machines?  According to
>>> the docs, MESA_FORMAT_R8G8B8A8_UNORM would end up with the R component
>>> in the most significant byte (highest array index) and the A component
>>> in the least significant byte (lowest array index), which seems like the
>>> opposite of what GL_RGBA8 is specified to be by
>>> ARB_shader_image_load_store.
>>>
RETURN_IF_SUPPORTED(MESA_FORMAT_A8B8G8R8_UNORM);
RETURN_IF_SUPPORTED(MESA_FORMAT_B8G8R8A8_UNORM);
break;
 @@ -100,6 +102,9 @@ _mesa_choose_tex_format(struct gl_context *ctx, GLenum 
 target,

 /* deep RGBA formats */
 case GL_RGB10_A2:
 +  if (prefer_no_swizzle) {
 + RETURN_IF_SUPPORTED(MESA_FORMAT_R10G10B10A2_UNORM);
 +  }
>>>
>>> This looks correct for any endianness.
>>>
RETURN_IF_SUPPORTED(MESA_FORMAT_B10G10R10A2_UNORM);
RETURN_IF_SUPPORTED(MESA_FORMAT_B8G8R8A8_UNORM);
break;
 @@ -119,6 +124,11 @@ _mesa_choose_tex_format(struct gl_context *ctx, 
 GLenum target,
}
/* fallthrough */
 case GL_RGB8:
 +  if (prefer_no_swizzle) {
 + RETURN_IF_SUPPORTED(MESA_FORMAT_RGB_UNORM8);
 + RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8X8_UNORM);
 + RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8A8_UNORM);
>>>
>>> The last two seem wrong for LE too.
>>>
 +  }
RETURN_IF_SUPPORTED(MESA_FORMAT_BGR_UNORM8);
RETURN_IF_SUPPORTED(MESA_FORMAT_B8G8R8X8_UNORM);
RETURN_IF_SUPPORTED(MESA_FORMAT_B8G8R8A8_UNORM);
 @@ -454,10 +464,19 @@ _mesa_choose_tex_format(struct gl_context *ctx, 
 GLenum target,
break;
 case GL_RGB_SNORM:
 case GL_RGB8_SNORM:
 +  if (prefer_no_swizzle) {
 + RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8X8_SNORM);
 + RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8A8_SNORM);
 +  }
>>>
>>> Same here.
>>>
RETURN_IF_SUPPORTED(MESA_FORMAT_X8B8G8R8_SNORM);
 -  /* FALLTHROUGH */
 +  RETURN_IF_SUPPORTED(MESA_FORMAT_A8B8G8R8_SNORM);
 +  RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8A8_SNORM);
 +  break;
 case GL_RGBA_SNORM:
 case GL_RGBA8_SNORM:
 +  if (prefer_no_swizzle) {
 + RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8A8_SNORM);
 +  }
>>>
>>> And here.
>>>
RETURN_IF_SUPPORTED(MESA_FORMAT_A8B8G8R8_SNORM);
RETURN_IF_SUPPORTED(MESA_FORMAT_R8G8B8A8_SNORM);
break;
 

Re: [Mesa-dev] [PATCH 1/8] i965: Consolidate code for setting brw->ib.start_vertex_offset.

[Pohjolainen, Topi]

On Thu, Mar 13, 2014 at 01:57:09AM -0700, Kenneth Graunke wrote:
> This was set identically in three places.
> 
> Signed-off-by: Kenneth Graunke 
> ---
>  src/mesa/drivers/dri/i965/brw_draw_upload.c | 15 ++-
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> This is just a series of tidying and commenting, for the most part.
> Patch 7 has an actual change.

Really nice work! The series:

Reviewed-by: Topi Pohjolainen 

> 
> diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c 
> b/src/mesa/drivers/dri/i965/brw_draw_upload.c
> index d42c074..3c537a5 100644
> --- a/src/mesa/drivers/dri/i965/brw_draw_upload.c
> +++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c
> @@ -841,12 +841,10 @@ static void brw_upload_indices(struct brw_context *brw)
> /* Turn into a proper VBO:
>  */
> if (!_mesa_is_bufferobj(bufferobj)) {
> -
>/* Get new bufferobj, offset:
> */
>intel_upload_data(brw, index_buffer->ptr, ib_size, ib_type_size,
>   &bo, &offset);
> -  brw->ib.start_vertex_offset = offset / ib_type_size;
> } else {
>offset = (GLuint) (unsigned long) index_buffer->ptr;
>  
> @@ -865,22 +863,21 @@ static void brw_upload_indices(struct brw_context *brw)
>  MAP_INTERNAL);
>  
>intel_upload_data(brw, map, ib_size, ib_type_size, &bo, &offset);
> -  brw->ib.start_vertex_offset = offset / ib_type_size;
>  
>ctx->Driver.UnmapBuffer(ctx, bufferobj, MAP_INTERNAL);
> } else {
> -   /* Use CMD_3D_PRIM's start_vertex_offset to avoid re-uploading
> -* the index buffer state when we're just moving the start index
> -* of our drawing.
> -*/
> -   brw->ib.start_vertex_offset = offset / ib_type_size;
> -
> bo = intel_bufferobj_buffer(brw, intel_buffer_object(bufferobj),
> offset, ib_size);
> drm_intel_bo_reference(bo);
> }
> }
>  
> +   /* Use 3DPRIMITIVE's start_vertex_offset to avoid re-uploading
> +* the index buffer state when we're just moving the start index
> +* of our drawing.
> +*/
> +   brw->ib.start_vertex_offset = offset / ib_type_size;
> +
> if (brw->ib.bo != bo) {
>drm_intel_bo_unreference(brw->ib.bo);
>brw->ib.bo = bo;
> -- 
> 1.9.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

[Mesa-dev] [PATCH] i965: Delete "fast color clear unsupported" performance warning.

[Kenneth Graunke]

Applications frequently clear to colors other than 0.0 or 1.0, which
prevents us from doing fast color clears.  In that case, we issue this
performance warning on basically every glClear call, resulting in so
much spam that it's nearly impossible to see any other messages.

Plus, I don't think it's useful.  We aren't suggesting a better way to
do what the application developers want---we're just telling them it
would be faster to do something they don't want.

Driver developers have no control over the clear color, so this message
is totally useless to them.

Signed-off-by: Kenneth Graunke 
---
 src/mesa/drivers/dri/i965/brw_blorp_clear.cpp | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp 
b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
index 76f8299..3a96106 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
+++ b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
@@ -155,8 +155,6 @@ is_color_fast_clear_compatible(struct brw_context *brw,
 
for (int i = 0; i < 4; i++) {
   if (color->f[i] != 0.0 && color->f[i] != 1.0) {
- perf_debug("Clear color unsupported by fast color clear.  "
-"Falling back to slow clear.\n");
  return false;
   }
}
-- 
1.9.0

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

[Mesa-dev] Wrong colors in 3D apps on big-endian systems

[Christian Zigotzky]

On 12.03.2014 05:35, Michel Dänzer wrote:

On Die, 2014-03-11 at 17:51 +0100, Christian Zigotzky wrote:

On 11.03.2014 16:25, Richard Sandiford wrote:

I just mean changing instances of things like PIPE_FORMAT_A8R8G8B8_UNORM
to PIPE_FORMAT_ARGB_UNORM in the relevant parts of the r600 support
(which is I think what Michel also meant -- like I say, I'm not really
adding anything new here).  PIPE_FORMAT_ARGB_UNORM always has the
alpha channel in the low bits of a 32-bit int according to host endianness,
whereas PIPE_FORMAT_A8R8G8B8_UNORM always has the alpha channel in the
first byte of memory.

I can't say for sure which the "revelavant parts" are because I don't
know the r600 code.

Thanks,
Richard



I have replaced PIPE_FORMAT_A8R8G8B8_UNORM with
PIPE_FORMAT_ARGB_UNORM in the files r600_state.c and
evergreen_state.c.

That's not really a good idea, as the hardware supports both variants of
formats.


Do the attached patches help?



Hi Michel,

Thank you very much for your effort. I've patched the latest git version 
with your patches. Mesa works with the right colors but it doesn't have 
3D acceleration. Mesa 10.1.0 and 10.0.3 works with 3D acceleration.


My system: Nemo board with a 1.8GHz dual core PA6T processer and an AMD 
Radeon HD 6870 graphics card.


Mesa/standard (mesa-git-20140313):

glxinfo | grep OpenGL

libGL error: failed to load driver: r600
OpenGL vendor string: VMware, Inc.
OpenGL renderer string: Gallium 0.4 on softpipe
OpenGL version string: 2.1 Mesa 10.2.0-devel (git-551d459)
OpenGL shading language version string: 1.30
OpenGL extensions:

Glxgears works with the right colors without 3D acceleration.

Mesa/llvmpipe (mesa-git-20140313):

OpenGL vendor string: VMware, Inc.
OpenGL renderer string: Gallium 0.4 on llvmpipe (LLVM 3.3, 128 bits)
OpenGL version string: 2.1 Mesa 10.2.0-devel (git-551d459)
OpenGL shading language version string: 1.30
OpenGL extensions:

Glxgears doesn't work with the following error messages:

LLVM ERROR: Do not know how to split the result of this operator!

Mesa/standard 10.1.0 with my changes

glxinfo | grep OpenGL

OpenGL vendor string: X.Org
OpenGL renderer string: Gallium 0.4 on AMD BARTS
OpenGL version string: 2.1 Mesa 10.1.0
OpenGL shading language version string: 1.30
OpenGL extensions:

Glxgears, Neverball, and SuperTuxKart work with the right colors and 3D 
acceleration.



Michel, do you have any tips for me, please? Thanks a lot for your help.

All the best,

Christian

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

[Mesa-dev] [PATCH 7/8] i965/upload: Switch from < to <= in intel_upload_data().

[Kenneth Graunke]

intel_upload_data() uses <, while intel_upload_map() uses <=.
Otherwise, the two blocks of code are identical.  This appears to be a
typo; we should be able to stage the exact amount of data that will fit
in the staging buffer.

Signed-off-by: Kenneth Graunke 
---
 src/mesa/drivers/dri/i965/intel_upload.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/intel_upload.c 
b/src/mesa/drivers/dri/i965/intel_upload.c
index f6342c9..3abc314 100644
--- a/src/mesa/drivers/dri/i965/intel_upload.c
+++ b/src/mesa/drivers/dri/i965/intel_upload.c
@@ -159,7 +159,7 @@ intel_upload_data(struct brw_context *brw,
   brw->upload.buffer_len = 0;
}
 
-   if (size < sizeof(brw->upload.buffer)) {
+   if (size <= sizeof(brw->upload.buffer)) {
   /* The new data is small enough to fit in the staging buffer, so
* stage it to be copied to the BO later.
*/
-- 
1.9.0

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

[Mesa-dev] [PATCH 1/8] i965: Consolidate code for setting brw->ib.start_vertex_offset.

[Kenneth Graunke]

This was set identically in three places.

Signed-off-by: Kenneth Graunke 
---
 src/mesa/drivers/dri/i965/brw_draw_upload.c | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

This is just a series of tidying and commenting, for the most part.
Patch 7 has an actual change.

diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c 
b/src/mesa/drivers/dri/i965/brw_draw_upload.c
index d42c074..3c537a5 100644
--- a/src/mesa/drivers/dri/i965/brw_draw_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c
@@ -841,12 +841,10 @@ static void brw_upload_indices(struct brw_context *brw)
/* Turn into a proper VBO:
 */
if (!_mesa_is_bufferobj(bufferobj)) {
-
   /* Get new bufferobj, offset:
*/
   intel_upload_data(brw, index_buffer->ptr, ib_size, ib_type_size,
&bo, &offset);
-  brw->ib.start_vertex_offset = offset / ib_type_size;
} else {
   offset = (GLuint) (unsigned long) index_buffer->ptr;
 
@@ -865,22 +863,21 @@ static void brw_upload_indices(struct brw_context *brw)
 MAP_INTERNAL);
 
   intel_upload_data(brw, map, ib_size, ib_type_size, &bo, &offset);
-  brw->ib.start_vertex_offset = offset / ib_type_size;
 
   ctx->Driver.UnmapBuffer(ctx, bufferobj, MAP_INTERNAL);
} else {
- /* Use CMD_3D_PRIM's start_vertex_offset to avoid re-uploading
-  * the index buffer state when we're just moving the start index
-  * of our drawing.
-  */
- brw->ib.start_vertex_offset = offset / ib_type_size;
-
  bo = intel_bufferobj_buffer(brw, intel_buffer_object(bufferobj),
  offset, ib_size);
  drm_intel_bo_reference(bo);
}
}
 
+   /* Use 3DPRIMITIVE's start_vertex_offset to avoid re-uploading
+* the index buffer state when we're just moving the start index
+* of our drawing.
+*/
+   brw->ib.start_vertex_offset = offset / ib_type_size;
+
if (brw->ib.bo != bo) {
   drm_intel_bo_unreference(brw->ib.bo);
   brw->ib.bo = bo;
-- 
1.9.0

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

[Mesa-dev] [PATCH 3/8] i965: Expand comments in brw_upload_indices().

[Kenneth Graunke]

This function has three cases:
1. Data is in user arrays
2. Data is in a buffer object, but misaligned.
3. Data is acceptable as is.

Previously, there was a "Turn into a proper VBO" comment above all three
cases, even though it only applies to case 1, and maybe case 2 (with a
specific interpretation of "proper".)  This patch replaces that with
more specific comments for each case.

The expanded comments explain where the alignment restrictions come from
and give an example of how to produce misaligned data in OpenGL.

Signed-off-by: Kenneth Graunke 
---
 src/mesa/drivers/dri/i965/brw_draw_upload.c | 20 ++--
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c 
b/src/mesa/drivers/dri/i965/brw_draw_upload.c
index f2945c1..98a8277 100644
--- a/src/mesa/drivers/dri/i965/brw_draw_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c
@@ -838,18 +838,25 @@ static void brw_upload_indices(struct brw_context *brw)
ib_size = ib_type_size * index_buffer->count;
bufferobj = index_buffer->obj;
 
-   /* Turn into a proper VBO:
-*/
if (!_mesa_is_bufferobj(bufferobj)) {
-  /* Get new bufferobj, offset:
-   */
+  /* Copy the index data from user arrays into a buffer object. */
   intel_upload_data(brw, index_buffer->ptr, ib_size, ib_type_size,
&bo, &offset);
} else {
   offset = (GLuint) (unsigned long) index_buffer->ptr;
 
-  /* If the index buffer isn't aligned to its element size, we have to
-   * rebase it into a temporary.
+  /* Even though the index data is in a buffer object, it may not meet
+   * the hardware's alignment restrictions.  From the 3DSTATE_INDEX_BUFFER
+   * documentation, DWord 1:
+   *
+   * "This field contains the size-aligned (as specified by Index Format)
+   *  Graphics Address of the first element of interest within the index
+   *  buffer."
+   *
+   * OpenGL allows arbitrary byte offsets into the buffer.  For example,
+   * glDrawElements with index == 1 and a type of GL_UNSIGNED_SHORT.
+   *
+   * If the data isn't aligned to the element size, copy it to a temporary.
*/
   if ((ib_type_size - 1) & offset) {
  perf_debug("copying index buffer to a temporary to work around "
@@ -866,6 +873,7 @@ static void brw_upload_indices(struct brw_context *brw)
 
  ctx->Driver.UnmapBuffer(ctx, bufferobj, MAP_INTERNAL);
   } else {
+ /* The index data is already in an acceptable BO.  Use as is. */
  bo = intel_bufferobj_buffer(brw, intel_buffer_object(bufferobj),
  offset, ib_size);
  drm_intel_bo_reference(bo);
-- 
1.9.0

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

[Mesa-dev] [PATCH 6/8] i965/upload: Drop use of GL types.

[Kenneth Graunke]

Using GL types outside of the GL API is silly.

Signed-off-by: Kenneth Graunke 
---
 src/mesa/drivers/dri/i965/intel_buffer_objects.h | 10 +-
 src/mesa/drivers/dri/i965/intel_upload.c | 18 +-
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_buffer_objects.h 
b/src/mesa/drivers/dri/i965/intel_buffer_objects.h
index 2197707..a132408 100644
--- a/src/mesa/drivers/dri/i965/intel_buffer_objects.h
+++ b/src/mesa/drivers/dri/i965/intel_buffer_objects.h
@@ -85,16 +85,16 @@ drm_intel_bo *intel_bufferobj_buffer(struct brw_context 
*brw,
  uint32_t size);
 
 void intel_upload_data(struct brw_context *brw,
-  const void *ptr, GLuint size, GLuint align,
+  const void *ptr, unsigned size, unsigned align,
   drm_intel_bo **return_bo,
-  GLuint *return_offset);
+  unsigned *return_offset);
 
 void *intel_upload_map(struct brw_context *brw,
-  GLuint size, GLuint align);
+  unsigned size, unsigned align);
 void intel_upload_unmap(struct brw_context *brw,
-   const void *ptr, GLuint size, GLuint align,
+   const void *ptr, unsigned size, unsigned align,
drm_intel_bo **return_bo,
-   GLuint *return_offset);
+   unsigned *return_offset);
 
 void intel_upload_finish(struct brw_context *brw);
 
diff --git a/src/mesa/drivers/dri/i965/intel_upload.c 
b/src/mesa/drivers/dri/i965/intel_upload.c
index c767b7d..f6342c9 100644
--- a/src/mesa/drivers/dri/i965/intel_upload.c
+++ b/src/mesa/drivers/dri/i965/intel_upload.c
@@ -96,7 +96,7 @@ intel_upload_finish(struct brw_context *brw)
  * data, and allocate a new BO.
  */
 static void
-wrap_buffers(struct brw_context *brw, GLuint size)
+wrap_buffers(struct brw_context *brw, unsigned size)
 {
intel_upload_finish(brw);
 
@@ -123,11 +123,11 @@ wrap_buffers(struct brw_context *brw, GLuint size)
  */
 void
 intel_upload_data(struct brw_context *brw,
-  const void *ptr, GLuint size, GLuint align,
+  const void *ptr, unsigned size, unsigned align,
   drm_intel_bo **return_bo,
-  GLuint *return_offset)
+  unsigned *return_offset)
 {
-   GLuint base, delta;
+   unsigned base, delta;
 
/* Make sure the BO has enough space for this data. */
base = ALIGN_NPOT(brw->upload.offset, align);
@@ -200,9 +200,9 @@ intel_upload_data(struct brw_context *brw,
  * It is an (unchecked) error to mismatch calls to map/unmap.
  */
 void *
-intel_upload_map(struct brw_context *brw, GLuint size, GLuint align)
+intel_upload_map(struct brw_context *brw, unsigned size, unsigned align)
 {
-   GLuint base, delta;
+   unsigned base, delta;
char *ptr;
 
/* Make sure the BO has enough space for this data. */
@@ -262,11 +262,11 @@ intel_upload_map(struct brw_context *brw, GLuint size, 
GLuint align)
  */
 void
 intel_upload_unmap(struct brw_context *brw,
-   const void *ptr, GLuint size, GLuint align,
+   const void *ptr, unsigned size, unsigned align,
drm_intel_bo **return_bo,
-   GLuint *return_offset)
+   unsigned *return_offset)
 {
-   GLuint base;
+   unsigned base;
 
base = ALIGN_NPOT(brw->upload.offset, align);
if (size > sizeof(brw->upload.buffer)) {
-- 
1.9.0

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

[Mesa-dev] [PATCH 5/8] i965/upload: Actually comment the upload code.

[Kenneth Graunke]

This code literally had zero comments, which made it impenetrable for
basically everybody except the authors.

We still ought to comment the refcounting strategy.

Signed-off-by: Kenneth Graunke 
---
 src/mesa/drivers/dri/i965/brw_context.h  |  24 +++
 src/mesa/drivers/dri/i965/intel_upload.c | 110 ++-
 2 files changed, 133 insertions(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
b/src/mesa/drivers/dri/i965/brw_context.h
index 734d273..659af92 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -1048,10 +1048,34 @@ struct brw_context
bool no_batch_wrap;
 
struct {
+  /**
+   * A buffer object for holding miscellaneous data.
+   */
   drm_intel_bo *bo;
+
+  /**
+   * Offset into "bo" for where to put the next piece of data.
+   *
+   * This is incremented when intel_upload_data or intel_upload_map
+   * ask for space to store data; the data may actually be staged and
+   * not appear in the BO until later.
+   */
   GLuint offset;
+
+  /**
+   * The amount of staged data needing to be copied to the BO.
+   */
   uint32_t buffer_len;
+
+  /**
+   * Offset into "bo" representing the starting destination address for
+   * the next batched copy.
+   */
   uint32_t buffer_offset;
+
+  /**
+   * The CPU-side staging buffer containing data yet to be uploaded.
+   */
   char buffer[4096];
} upload;
 
diff --git a/src/mesa/drivers/dri/i965/intel_upload.c 
b/src/mesa/drivers/dri/i965/intel_upload.c
index ec3109b..c767b7d 100644
--- a/src/mesa/drivers/dri/i965/intel_upload.c
+++ b/src/mesa/drivers/dri/i965/intel_upload.c
@@ -25,7 +25,16 @@
 /**
  * @file intel_upload.c
  *
- * Batched upload via BOs.
+ * A batched data upload mechanism which can efficiently copy user supplied
+ * data into buffer objects.
+ *
+ * OpenGL allows applications to provide data in user arrays, which may
+ * disappear or change contents at any time.  The GPU can't use this memory
+ * directly, so we need to copy it into buffer objects.
+ *
+ * Often times, the amount of data we need to copy is small.  On non-LLC
+ * platforms, it's more efficient to stage this data in a CPU-sized buffer
+ * and only copy it to a BO when we have a larger amount of data.
  */
 
 #include "main/imports.h"
@@ -43,6 +52,7 @@
 
 #include "brw_context.h"
 
+/** The size of brw->upload.bo (the GPU buffer object). */
 #define INTEL_UPLOAD_SIZE (64*1024)
 
 /**
@@ -51,6 +61,15 @@
 #define ALIGN_NPOT(value, alignment) \
(((value) + (alignment) - 1) / (alignment) * (alignment))
 
+/**
+ * Finish any outstanding uploads.
+ *
+ * We may have staged data that hasn't yet been copied to the BO; calling
+ * this function forces an immediate copy.
+ *
+ * This is necessary at certain points, such as submitting a batchbuffer
+ * that wants to actually use the data.
+ */
 void
 intel_upload_finish(struct brw_context *brw)
 {
@@ -69,11 +88,22 @@ intel_upload_finish(struct brw_context *brw)
brw->upload.bo = NULL;
 }
 
+/**
+ * Handle running out of space in the BO.
+ *
+ * Called when attempting to upload a new piece of data.  If the existing BO
+ * doesn't have enough space, we need to finish uploading any existing staged
+ * data, and allocate a new BO.
+ */
 static void
 wrap_buffers(struct brw_context *brw, GLuint size)
 {
intel_upload_finish(brw);
 
+   /* Use the larger of size and INTEL_UPLOAD_SIZE.  This ensures that the
+* buffer is large enough to upload any arbitrary piece of data, but will
+* be reasonably sized even if we wrap when uploading a small bit of data.
+*/
if (size < INTEL_UPLOAD_SIZE)
   size = INTEL_UPLOAD_SIZE;
 
@@ -81,6 +111,16 @@ wrap_buffers(struct brw_context *brw, GLuint size)
brw->upload.offset = 0;
 }
 
+/**
+ * Upload some data (copy from CPU memory to a buffer object).
+ *
+ * @param[in]  brw   The i965 driver context.
+ * @param[in]  ptr   The source data to upload.
+ * @param[in]  size  The number of bytes to upload.
+ * @param[in]  align Alignment requirement in bytes.
+ * @param[out] return_bo A pointer to the BO which will contain the data.
+ * @param[out] return_offset Offset into return_bo where the data will be.
+ */
 void
 intel_upload_data(struct brw_context *brw,
   const void *ptr, GLuint size, GLuint align,
@@ -89,6 +129,7 @@ intel_upload_data(struct brw_context *brw,
 {
GLuint base, delta;
 
+   /* Make sure the BO has enough space for this data. */
base = ALIGN_NPOT(brw->upload.offset, align);
if (brw->upload.bo == NULL || base + size > brw->upload.bo->size) {
   wrap_buffers(brw, size);
@@ -96,10 +137,19 @@ intel_upload_data(struct brw_context *brw,
}
 
drm_intel_bo_reference(brw->upload.bo);
+
+   /* The new data will (eventually) end up in brw->upload.bo with an
+* offset of "base".  Return this s

[Mesa-dev] [PATCH 8/8] i965/upload: Rename "buffer" to "staging_buffer".

[Kenneth Graunke]

This clarifies the purpose of the buffer, and also avoids some confusion
between the two data stores---buffer and bo both are generic names.

To match this, also rename "buffer_len" to "staging_buffer_len."
Don't rename buffer_offset, as it's actually a bo offset.

Signed-off-by: Kenneth Graunke 
---
 src/mesa/drivers/dri/i965/brw_context.h  |  4 +--
 src/mesa/drivers/dri/i965/intel_upload.c | 50 
 2 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
b/src/mesa/drivers/dri/i965/brw_context.h
index 659af92..659f64f 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -1065,7 +1065,7 @@ struct brw_context
   /**
* The amount of staged data needing to be copied to the BO.
*/
-  uint32_t buffer_len;
+  uint32_t staging_buffer_len;
 
   /**
* Offset into "bo" representing the starting destination address for
@@ -1076,7 +1076,7 @@ struct brw_context
   /**
* The CPU-side staging buffer containing data yet to be uploaded.
*/
-  char buffer[4096];
+  char staging_buffer[4096];
} upload;
 
/**
diff --git a/src/mesa/drivers/dri/i965/intel_upload.c 
b/src/mesa/drivers/dri/i965/intel_upload.c
index 3abc314..cb488b0 100644
--- a/src/mesa/drivers/dri/i965/intel_upload.c
+++ b/src/mesa/drivers/dri/i965/intel_upload.c
@@ -76,12 +76,12 @@ intel_upload_finish(struct brw_context *brw)
if (!brw->upload.bo)
   return;
 
-   if (brw->upload.buffer_len) {
+   if (brw->upload.staging_buffer_len) {
   drm_intel_bo_subdata(brw->upload.bo,
brw->upload.buffer_offset,
-   brw->upload.buffer_len,
-   brw->upload.buffer);
-  brw->upload.buffer_len = 0;
+   brw->upload.staging_buffer_len,
+   brw->upload.staging_buffer);
+  brw->upload.staging_buffer_len = 0;
}
 
drm_intel_bo_unreference(brw->upload.bo);
@@ -150,26 +150,26 @@ intel_upload_data(struct brw_context *brw,
/* If there isn't enough space in the staging buffer, go upload the
 * staged data to free up space.
 */
-   if (brw->upload.buffer_len &&
-   brw->upload.buffer_len + delta + size > sizeof(brw->upload.buffer)) {
+   if (brw->upload.staging_buffer_len &&
+   brw->upload.staging_buffer_len + delta + size > 
sizeof(brw->upload.staging_buffer)) {
   drm_intel_bo_subdata(brw->upload.bo,
brw->upload.buffer_offset,
-   brw->upload.buffer_len,
-   brw->upload.buffer);
-  brw->upload.buffer_len = 0;
+   brw->upload.staging_buffer_len,
+   brw->upload.staging_buffer);
+  brw->upload.staging_buffer_len = 0;
}
 
-   if (size <= sizeof(brw->upload.buffer)) {
+   if (size <= sizeof(brw->upload.staging_buffer)) {
   /* The new data is small enough to fit in the staging buffer, so
* stage it to be copied to the BO later.
*/
-  if (brw->upload.buffer_len == 0)
+  if (brw->upload.staging_buffer_len == 0)
  brw->upload.buffer_offset = base;
   else
- brw->upload.buffer_len += delta;
+ brw->upload.staging_buffer_len += delta;
 
-  memcpy(brw->upload.buffer + brw->upload.buffer_len, ptr, size);
-  brw->upload.buffer_len += size;
+  memcpy(brw->upload.staging_buffer + brw->upload.staging_buffer_len, ptr, 
size);
+  brw->upload.staging_buffer_len += size;
} else {
   /* The new data is too large to fit in the (empty) staging buffer,
* which means it wouldn't benefit from batched uploading anyway.
@@ -218,26 +218,26 @@ intel_upload_map(struct brw_context *brw, unsigned size, 
unsigned align)
/* If there isn't enough space in the staging buffer, go upload the
 * staged data to free up space.
 */
-   if (brw->upload.buffer_len &&
-   brw->upload.buffer_len + delta + size > sizeof(brw->upload.buffer)) {
+   if (brw->upload.staging_buffer_len &&
+   brw->upload.staging_buffer_len + delta + size > 
sizeof(brw->upload.staging_buffer)) {
   drm_intel_bo_subdata(brw->upload.bo,
brw->upload.buffer_offset,
-   brw->upload.buffer_len,
-   brw->upload.buffer);
-  brw->upload.buffer_len = 0;
+   brw->upload.staging_buffer_len,
+   brw->upload.staging_buffer);
+  brw->upload.staging_buffer_len = 0;
}
 
-   if (size <= sizeof(brw->upload.buffer)) {
+   if (size <= sizeof(brw->upload.staging_buffer)) {
   /* The new data is small enough to fit in the staging buffer, so
* stage it to be copied to the BO later.
*/
-  if (brw->upload.buffer_len == 0)
+  if (brw->upload.staging_buffer_len == 0)
  brw->upload.buffer_offset = b

[Mesa-dev] [PATCH 4/8] i965/upload: Refactor open-coded ALIGN-like computations.

[Kenneth Graunke]

Sadly, we can't use actual ALIGN(), since that only supports
power-of-two values for the alignment parameter.

Signed-off-by: Kenneth Graunke 
---
 src/mesa/drivers/dri/i965/intel_upload.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_upload.c 
b/src/mesa/drivers/dri/i965/intel_upload.c
index ac16dcc..ec3109b 100644
--- a/src/mesa/drivers/dri/i965/intel_upload.c
+++ b/src/mesa/drivers/dri/i965/intel_upload.c
@@ -45,6 +45,12 @@
 
 #define INTEL_UPLOAD_SIZE (64*1024)
 
+/**
+ * Like ALIGN(), but works with a non-power-of-two alignment.
+ */
+#define ALIGN_NPOT(value, alignment) \
+   (((value) + (alignment) - 1) / (alignment) * (alignment))
+
 void
 intel_upload_finish(struct brw_context *brw)
 {
@@ -83,7 +89,7 @@ intel_upload_data(struct brw_context *brw,
 {
GLuint base, delta;
 
-   base = (brw->upload.offset + align - 1) / align * align;
+   base = ALIGN_NPOT(brw->upload.offset, align);
if (brw->upload.bo == NULL || base + size > brw->upload.bo->size) {
   wrap_buffers(brw, size);
   base = 0;
@@ -124,7 +130,7 @@ intel_upload_map(struct brw_context *brw, GLuint size, 
GLuint align)
GLuint base, delta;
char *ptr;
 
-   base = (brw->upload.offset + align - 1) / align * align;
+   base = ALIGN_NPOT(brw->upload.offset, align);
if (brw->upload.bo == NULL || base + size > brw->upload.bo->size) {
   wrap_buffers(brw, size);
   base = 0;
@@ -163,7 +169,7 @@ intel_upload_unmap(struct brw_context *brw,
 {
GLuint base;
 
-   base = (brw->upload.offset + align - 1) / align * align;
+   base = ALIGN_NPOT(brw->upload.offset, align);
if (size > sizeof(brw->upload.buffer)) {
   drm_intel_bo_subdata(brw->upload.bo, base, size, ptr);
   free((void*)ptr);
-- 
1.9.0

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

[Mesa-dev] [PATCH 2/8] i965: Fix indentation in brw_upload_indices().

[Kenneth Graunke]

Signed-off-by: Kenneth Graunke 
---
 src/mesa/drivers/dri/i965/brw_draw_upload.c | 38 ++---
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c 
b/src/mesa/drivers/dri/i965/brw_draw_upload.c
index 3c537a5..f2945c1 100644
--- a/src/mesa/drivers/dri/i965/brw_draw_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c
@@ -851,25 +851,25 @@ static void brw_upload_indices(struct brw_context *brw)
   /* If the index buffer isn't aligned to its element size, we have to
* rebase it into a temporary.
*/
-   if ((ib_type_size - 1) & offset) {
-  perf_debug("copying index buffer to a temporary to work around "
- "misaligned offset %d\n", offset);
-
-  GLubyte *map = ctx->Driver.MapBufferRange(ctx,
-offset,
-ib_size,
-GL_MAP_READ_BIT,
-bufferobj,
-MAP_INTERNAL);
-
-  intel_upload_data(brw, map, ib_size, ib_type_size, &bo, &offset);
-
-  ctx->Driver.UnmapBuffer(ctx, bufferobj, MAP_INTERNAL);
-   } else {
- bo = intel_bufferobj_buffer(brw, intel_buffer_object(bufferobj),
- offset, ib_size);
- drm_intel_bo_reference(bo);
-   }
+  if ((ib_type_size - 1) & offset) {
+ perf_debug("copying index buffer to a temporary to work around "
+"misaligned offset %d\n", offset);
+
+ GLubyte *map = ctx->Driver.MapBufferRange(ctx,
+   offset,
+   ib_size,
+   GL_MAP_READ_BIT,
+   bufferobj,
+   MAP_INTERNAL);
+
+ intel_upload_data(brw, map, ib_size, ib_type_size, &bo, &offset);
+
+ ctx->Driver.UnmapBuffer(ctx, bufferobj, MAP_INTERNAL);
+  } else {
+ bo = intel_bufferobj_buffer(brw, intel_buffer_object(bufferobj),
+ offset, ib_size);
+ drm_intel_bo_reference(bo);
+  }
}
 
/* Use 3DPRIMITIVE's start_vertex_offset to avoid re-uploading
-- 
1.9.0

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

[Mesa-dev] [PATCH] winsys/radeon: Store GPU virtual memory addresses of BOs in a hash table

[Michel Dänzer]

From: Michel Dänzer 

This allows retrieving the existing BO and incrementing its reference count,
instead of creating a separate winsys representation for it, when the kernel
reports that the BO was already assigned a virtual memory address.

This fixes problems with XWayland using radeonsi and the
xf86-video-wlglamor driver, which calls GEM flink outside of the radeon
winsys code and creates BOs from the flinked names using the same DRM file
descriptor.

Signed-off-by: Michel Dänzer 
---
 src/gallium/winsys/radeon/drm/radeon_drm_bo.c | 74 ++-
 1 file changed, 26 insertions(+), 48 deletions(-)

diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c 
b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
index 2ac060b..7dd5e7f 100644
--- a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
+++ b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
@@ -120,6 +120,8 @@ struct radeon_bomgr {
 struct util_hash_table *bo_names;
 /* List of buffer handles. Protectded by bo_handles_mutex. */
 struct util_hash_table *bo_handles;
+/* List of buffer virtual memory ranges. Protectded by bo_handles_mutex. */
+struct util_hash_table *bo_vas;
 pipe_mutex bo_handles_mutex;
 pipe_mutex bo_va_mutex;
 
@@ -260,48 +262,6 @@ static uint64_t radeon_bomgr_find_va(struct radeon_bomgr 
*mgr, uint64_t size, ui
 return offset;
 }
 
-static void radeon_bomgr_force_va(struct radeon_bomgr *mgr, uint64_t va, 
uint64_t size)
-{
-size = align(size, 4096);
-
-pipe_mutex_lock(mgr->bo_va_mutex);
-if (va >= mgr->va_offset) {
-if (va > mgr->va_offset) {
-struct radeon_bo_va_hole *hole;
-hole = CALLOC_STRUCT(radeon_bo_va_hole);
-if (hole) {
-hole->size = va - mgr->va_offset;
-hole->offset = mgr->va_offset;
-list_add(&hole->list, &mgr->va_holes);
-}
-}
-mgr->va_offset = va + size;
-} else {
-struct radeon_bo_va_hole *hole, *n;
-uint64_t hole_end, va_end;
-
-/* Prune/free all holes that fall into the range
- */
-LIST_FOR_EACH_ENTRY_SAFE(hole, n, &mgr->va_holes, list) {
-hole_end = hole->offset + hole->size;
-va_end = va + size;
-if (hole->offset >= va_end || hole_end <= va)
-continue;
-if (hole->offset >= va && hole_end <= va_end) {
-list_del(&hole->list);
-FREE(hole);
-continue;
-}
-if (hole->offset >= va)
-hole->offset = va_end;
-else
-hole_end = va;
-hole->size = hole_end - hole->offset;
-}
-}
-pipe_mutex_unlock(mgr->bo_va_mutex);
-}
-
 static void radeon_bomgr_free_va(struct radeon_bomgr *mgr, uint64_t va, 
uint64_t size)
 {
 struct radeon_bo_va_hole *hole;
@@ -625,11 +585,19 @@ static struct pb_buffer *radeon_bomgr_create_bo(struct 
pb_manager *_mgr,
 radeon_bo_destroy(&bo->base);
 return NULL;
 }
+pipe_mutex_lock(mgr->bo_handles_mutex);
 if (va.operation == RADEON_VA_RESULT_VA_EXIST) {
-radeon_bomgr_free_va(mgr, bo->va, size);
-bo->va = va.offset;
-radeon_bomgr_force_va(mgr, bo->va, size);
+struct pb_buffer *b = &bo->base;
+struct radeon_bo *old_bo =
+util_hash_table_get(mgr->bo_vas, (void*)(uintptr_t)va.offset);
+
+pipe_mutex_unlock(mgr->bo_handles_mutex);
+pb_reference(&b, &old_bo->base);
+return b;
 }
+
+util_hash_table_set(mgr->bo_vas, (void*)(uintptr_t)bo->va, bo);
+pipe_mutex_unlock(mgr->bo_handles_mutex);
 }
 
 if (rdesc->initial_domains & RADEON_DOMAIN_VRAM)
@@ -667,6 +635,7 @@ static void radeon_bomgr_destroy(struct pb_manager *_mgr)
 struct radeon_bomgr *mgr = radeon_bomgr(_mgr);
 util_hash_table_destroy(mgr->bo_names);
 util_hash_table_destroy(mgr->bo_handles);
+util_hash_table_destroy(mgr->bo_vas);
 pipe_mutex_destroy(mgr->bo_handles_mutex);
 pipe_mutex_destroy(mgr->bo_va_mutex);
 FREE(mgr);
@@ -700,6 +669,7 @@ struct pb_manager *radeon_bomgr_create(struct 
radeon_drm_winsys *rws)
 mgr->rws = rws;
 mgr->bo_names = util_hash_table_create(handle_hash, handle_compare);
 mgr->bo_handles = util_hash_table_create(handle_hash, handle_compare);
+mgr->bo_vas = util_hash_table_create(handle_hash, handle_compare);
 pipe_mutex_init(mgr->bo_handles_mutex);
 pipe_mutex_init(mgr->bo_va_mutex);
 
@@ -999,11 +969,19 @@ done:
 radeon_bo_destroy(&bo->base);
 return NULL;
 }
+pipe_mutex_lock(mgr->bo_handles_mutex);
 if (va.operation == RADEON_VA_RESULT_VA_EXIST) {
-radeon_bomgr_free_va(mgr, bo->va, bo->base.size);
-bo->va = va.offset;
-radeon_bomgr_force_va(mgr, bo->va, bo->base.size);
+struct

[Mesa-dev] [PATCH] mesa: return v.value_int64 when the requested type is TYPE_INT64

[Emil Velikov]

Fixes "Operands don't affect result" defect reported by Coverity.

Cc: "9.2 10.0 10.1"  
Signed-off-by: Emil Velikov 
---
 src/mesa/main/get.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
index b190851..88cf202 100644
--- a/src/mesa/main/get.c
+++ b/src/mesa/main/get.c
@@ -1997,7 +1997,7 @@ _mesa_GetBooleani_v( GLenum pname, GLuint index, 
GLboolean *params )
   params[3] = INT_TO_BOOLEAN(v.value_int_4[3]);
   break;
case TYPE_INT64:
-  params[0] = INT64_TO_BOOLEAN(v.value_int);
+  params[0] = INT64_TO_BOOLEAN(v.value_int64);
   break;
default:
   ; /* nothing - GL error was recorded */
@@ -2042,7 +2042,7 @@ _mesa_GetIntegeri_v( GLenum pname, GLuint index, GLint 
*params )
   params[3] = v.value_int_4[3];
   break;
case TYPE_INT64:
-  params[0] = INT64_TO_INT(v.value_int);
+  params[0] = INT64_TO_INT(v.value_int64);
   break;
default:
   ; /* nothing - GL error was recorded */
@@ -2067,7 +2067,7 @@ _mesa_GetInteger64i_v( GLenum pname, GLuint index, 
GLint64 *params )
   params[3] = v.value_int_4[3];
   break;
case TYPE_INT64:
-  params[0] = v.value_int;
+  params[0] = v.value_int64;
   break;
default:
   ; /* nothing - GL error was recorded */
-- 
1.9.0

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

Re: [Mesa-dev] [PATCH 07/20] targets/dri: use install-gallium-links.mk

[Steven Newbury]

On Thu, 2014-03-13 at 07:35 +, Emil Velikov wrote:
> On 13/03/14 07:09, Steven Newbury wrote:
> > On Tue, 2014-03-04 at 21:12 +, Emil Velikov wrote:
> >> Drop the duplication accross all dri targets.
> >
> > ...
> >
> >> diff --git a/src/gallium/targets/dri-ilo/Makefile.am 
> >> b/src/gallium/targets/dri-ilo/Makefile.am
> >> index 418e2ea..18d3c44 100644
> >> --- a/src/gallium/targets/dri-ilo/Makefile.am
> >> +++ b/src/gallium/targets/dri-ilo/Makefile.am
> >> @@ -59,9 +59,4 @@ ilo_dri_la_LDFLAGS += $(LLVM_LDFLAGS)
> >>   ilo_dri_la_LIBADD += $(LLVM_LIBS)
> >>   endif
> >>
> >> -# Provide compatibility with scripts for the old Mesa build system for
> >> -# a while by putting a link to the driver into /lib of the build tree.
> >> -all-local: ilo_dri.la
> >> -  $(MKDIR_P) $(top_builddir)/$(LIB_DIR)/gallium
> >> -  ln -f .libs/ilo_dri.so $(top_builddir)/$(LIB_DIR)/gallium/ilo_dri.so
> >> -  ln -sf ilo_dri.so $(top_builddir)/$(LIB_DIR)/gallium/i965_dri.so
> >
> > This doesn't work for ilo since dri_LTLIBRARIES isn't set. (It uses
> > "noinst_LTLIBRARIES = ilo_dri.la" instead.)
> >
> > It's also not handling the i965_dri.so symlink as is removed above.
> >
> >
> Are you really using ilo or you'd like to point out that I've 
> "unintentionally" swept it under the carpet :-P
> 
Not really... I've been having a play with gallium-nine, and it's also
the only way to get accelerated OpenVG on intel AFAIK! ;-)  As it
happens neither need the DRI driver, but I just noticed the build
failure. :-)


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

[Mesa-dev] [PATCH 1/2] radeonsi: Add DMA ring

[Niels Ole Salscheider]

Signed-off-by: Niels Ole Salscheider 
---
 src/gallium/drivers/radeonsi/si_hw_context.c |  3 +++
 src/gallium/drivers/radeonsi/si_pipe.c   | 22 ++
 2 files changed, 25 insertions(+)

diff --git a/src/gallium/drivers/radeonsi/si_hw_context.c 
b/src/gallium/drivers/radeonsi/si_hw_context.c
index c952c8d..d9fba01 100644
--- a/src/gallium/drivers/radeonsi/si_hw_context.c
+++ b/src/gallium/drivers/radeonsi/si_hw_context.c
@@ -123,6 +123,9 @@ void si_context_flush(struct si_context *ctx, unsigned 
flags)
 #endif
 
/* Flush the CS. */
+   if (ctx->b.rings.dma.cs) {
+   ctx->b.ws->cs_flush(ctx->b.rings.dma.cs, flags, 0);
+   }
ctx->b.ws->cs_flush(ctx->b.rings.gfx.cs, flags, 0);
 
 #if SI_TRACE_CS
diff --git a/src/gallium/drivers/radeonsi/si_pipe.c 
b/src/gallium/drivers/radeonsi/si_pipe.c
index 827e9fe..21cbedf 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.c
+++ b/src/gallium/drivers/radeonsi/si_pipe.c
@@ -74,6 +74,24 @@ static void si_flush_from_winsys(void *ctx, unsigned flags)
si_flush((struct pipe_context*)ctx, NULL, flags);
 }
 
+static void si_flush_dma_from_st(void *ctx, unsigned flags)
+{
+   struct si_context *sctx = (struct si_context *)ctx;
+   struct radeon_winsys_cs *cs = sctx->b.rings.dma.cs;
+
+   if (!cs->cdw) {
+   return;
+   }
+
+   sctx->b.ws->cs_flush(cs, flags, 0);
+}
+
+static void si_flush_dma_from_winsys(void *ctx, unsigned flags)
+{
+   struct si_context *sctx = (struct si_context *)ctx;
+   sctx->b.rings.dma.flush(sctx, flags);
+}
+
 static void si_destroy_context(struct pipe_context *context)
 {
struct si_context *sctx = (struct si_context *)context;
@@ -163,6 +181,10 @@ static struct pipe_context *si_create_context(struct 
pipe_screen *screen, void *
 
sctx->b.ws->cs_set_flush_callback(sctx->b.rings.gfx.cs, 
si_flush_from_winsys, sctx);
 
+   sctx->b.rings.dma.cs = sctx->b.ws->cs_create(sctx->b.ws, RING_DMA, 
NULL);
+   sctx->b.rings.dma.flush = si_flush_dma_from_st;
+   sctx->b.ws->cs_set_flush_callback(sctx->b.rings.dma.cs, 
si_flush_dma_from_winsys, sctx);
+
sctx->blitter = util_blitter_create(&sctx->b.b);
if (sctx->blitter == NULL)
goto fail;
-- 
1.9.0

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

[Mesa-dev] [PATCH 2/2] radeonsi: Implement DMA blit

[Niels Ole Salscheider]

This code is a slightly modified version of evergreen_dma_blit (and
evergreen_dma_copy as well as evergreen_dma_copy_tile).
It would be nice to share some of the code in the long term.

I have reused some "cik"-prefixed functions that also return the right
value for SI. I am not sure if they should be renamed.

Signed-off-by: Niels Ole Salscheider 
---
 src/gallium/drivers/radeonsi/si_hw_context.c |  65 +++
 src/gallium/drivers/radeonsi/si_pipe.h   |   7 +
 src/gallium/drivers/radeonsi/si_state.c  | 265 ++-
 src/gallium/drivers/radeonsi/sid.h   |  15 ++
 4 files changed, 346 insertions(+), 6 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_hw_context.c 
b/src/gallium/drivers/radeonsi/si_hw_context.c
index d9fba01..76583a3 100644
--- a/src/gallium/drivers/radeonsi/si_hw_context.c
+++ b/src/gallium/drivers/radeonsi/si_hw_context.c
@@ -25,6 +25,8 @@
  */
 
 #include "si_pipe.h"
+#include "sid.h"
+#include "../radeon/r600_cs.h"
 
 /* initialize */
 void si_need_cs_space(struct si_context *ctx, unsigned num_dw,
@@ -186,6 +188,69 @@ void si_begin_new_cs(struct si_context *ctx)
ctx->b.initial_gfx_cs_size = ctx->b.rings.gfx.cs->cdw;
 }
 
+void si_need_dma_space(struct si_context *ctx, unsigned num_dw)
+{
+   /* The number of dwords we already used in the DMA so far. */
+   num_dw += ctx->b.rings.dma.cs->cdw;
+   /* Flush if there's not enough space. */
+   if (num_dw > RADEON_MAX_CMDBUF_DWORDS) {
+   ctx->b.rings.dma.flush(ctx, RADEON_FLUSH_ASYNC);
+   }
+}
+
+void si_dma_copy(struct si_context *ctx,
+struct pipe_resource *dst,
+struct pipe_resource *src,
+uint64_t dst_offset,
+uint64_t src_offset,
+uint64_t size)
+{
+   struct radeon_winsys_cs *cs = ctx->b.rings.dma.cs;
+   unsigned i, ncopy, csize, sub_cmd, shift;
+   struct r600_resource *rdst = (struct r600_resource*)dst;
+   struct r600_resource *rsrc = (struct r600_resource*)src;
+
+   /* Mark the buffer range of destination as valid (initialized),
+* so that transfer_map knows it should wait for the GPU when mapping
+* that range. */
+   util_range_add(&rdst->valid_buffer_range, dst_offset,
+  dst_offset + size);
+
+   /* make sure that the dma ring is only one active */
+   ctx->b.rings.gfx.flush(ctx, RADEON_FLUSH_ASYNC);
+   dst_offset += r600_resource_va(&ctx->screen->b.b, dst);
+   src_offset += r600_resource_va(&ctx->screen->b.b, src);
+
+   /* see if we use dword or byte copy */
+   if (!(dst_offset & 0x3) && !(src_offset & 0x3) && !(size & 0x3)) {
+   size >>= 2;
+   sub_cmd = 0x00;
+   shift = 2;
+   } else {
+   sub_cmd = 0x40;
+   shift = 0;
+   }
+   ncopy = (size / 0x000f) + !!(size % 0x000f);
+
+   si_need_dma_space(ctx, ncopy * 5);
+   for (i = 0; i < ncopy; i++) {
+   csize = size < 0x000f ? size : 0x000f;
+   /* emit reloc before writting cs so that cs is always in 
consistent state */
+   r600_context_bo_reloc(&ctx->b, &ctx->b.rings.dma, rsrc, 
RADEON_USAGE_READ,
+ RADEON_PRIO_MIN);
+   r600_context_bo_reloc(&ctx->b, &ctx->b.rings.dma, rdst, 
RADEON_USAGE_WRITE,
+ RADEON_PRIO_MIN);
+   cs->buf[cs->cdw++] = SI_DMA_PACKET(SI_DMA_PACKET_COPY, sub_cmd, 
csize);
+   cs->buf[cs->cdw++] = dst_offset & 0x;
+   cs->buf[cs->cdw++] = src_offset & 0x;
+   cs->buf[cs->cdw++] = (dst_offset >> 32UL) & 0xff;
+   cs->buf[cs->cdw++] = (src_offset >> 32UL) & 0xff;
+   dst_offset += csize << shift;
+   src_offset += csize << shift;
+   size -= csize;
+   }
+}
+
 #if SI_TRACE_CS
 void si_trace_emit(struct si_context *sctx)
 {
diff --git a/src/gallium/drivers/radeonsi/si_pipe.h 
b/src/gallium/drivers/radeonsi/si_pipe.h
index 47dc8e7..45def1e 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.h
+++ b/src/gallium/drivers/radeonsi/si_pipe.h
@@ -171,6 +171,13 @@ void si_decompress_color_textures(struct si_context *sctx,
 void si_context_flush(struct si_context *ctx, unsigned flags);
 void si_begin_new_cs(struct si_context *ctx);
 void si_need_cs_space(struct si_context *ctx, unsigned num_dw, boolean 
count_draw_in);
+void si_need_dma_space(struct si_context *ctx, unsigned num_dw);
+void si_dma_copy(struct si_context *ctx,
+struct pipe_resource *dst,
+struct pipe_resource *src,
+uint64_t dst_offset,
+uint64_t src_offset,
+uint64_t size);
 
 /* si_pipe.c */
 void si_flush(struct pipe_context *ctx, struct pipe_fence_handle **fence,
diff --git a/src/gallium/drivers/radeonsi/si_state.c 
b/src/gallium/drivers/radeon

Re: [Mesa-dev] [PATCH 07/20] targets/dri: use install-gallium-links.mk

[Emil Velikov]

On 13/03/14 07:09, Steven Newbury wrote:

On Tue, 2014-03-04 at 21:12 +, Emil Velikov wrote:

Drop the duplication accross all dri targets.


...


diff --git a/src/gallium/targets/dri-ilo/Makefile.am 
b/src/gallium/targets/dri-ilo/Makefile.am
index 418e2ea..18d3c44 100644
--- a/src/gallium/targets/dri-ilo/Makefile.am
+++ b/src/gallium/targets/dri-ilo/Makefile.am
@@ -59,9 +59,4 @@ ilo_dri_la_LDFLAGS += $(LLVM_LDFLAGS)
  ilo_dri_la_LIBADD += $(LLVM_LIBS)
  endif

-# Provide compatibility with scripts for the old Mesa build system for
-# a while by putting a link to the driver into /lib of the build tree.
-all-local: ilo_dri.la
-   $(MKDIR_P) $(top_builddir)/$(LIB_DIR)/gallium
-   ln -f .libs/ilo_dri.so $(top_builddir)/$(LIB_DIR)/gallium/ilo_dri.so
-   ln -sf ilo_dri.so $(top_builddir)/$(LIB_DIR)/gallium/i965_dri.so


This doesn't work for ilo since dri_LTLIBRARIES isn't set. (It uses
"noinst_LTLIBRARIES = ilo_dri.la" instead.)

It's also not handling the i965_dri.so symlink as is removed above.


Are you really using ilo or you'd like to point out that I've 
"unintentionally" swept it under the carpet :-P


Chia-Wu

I take it that you've deliberately left the library as noinst, is that 
to prevent any confusion with the classic i965 ?
How would you feel on this topic - do we s/noinst_LT/lib_LT/ or just 
revert this hunk ?


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

Re: [Mesa-dev] [PATCH 07/20] targets/dri: use install-gallium-links.mk

[Steven Newbury]

On Tue, 2014-03-04 at 21:12 +, Emil Velikov wrote:
> Drop the duplication accross all dri targets.

...

> diff --git a/src/gallium/targets/dri-ilo/Makefile.am 
> b/src/gallium/targets/dri-ilo/Makefile.am
> index 418e2ea..18d3c44 100644
> --- a/src/gallium/targets/dri-ilo/Makefile.am
> +++ b/src/gallium/targets/dri-ilo/Makefile.am
> @@ -59,9 +59,4 @@ ilo_dri_la_LDFLAGS += $(LLVM_LDFLAGS)
>  ilo_dri_la_LIBADD += $(LLVM_LIBS)
>  endif
>  
> -# Provide compatibility with scripts for the old Mesa build system for
> -# a while by putting a link to the driver into /lib of the build tree.
> -all-local: ilo_dri.la
> - $(MKDIR_P) $(top_builddir)/$(LIB_DIR)/gallium
> - ln -f .libs/ilo_dri.so $(top_builddir)/$(LIB_DIR)/gallium/ilo_dri.so
> - ln -sf ilo_dri.so $(top_builddir)/$(LIB_DIR)/gallium/i965_dri.so

This doesn't work for ilo since dri_LTLIBRARIES isn't set. (It uses
"noinst_LTLIBRARIES = ilo_dri.la" instead.)

It's also not handling the i965_dri.so symlink as is removed above.


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