Re: [Mesa-dev] [PATCH 2/2] r600: Handle failures in compute_memory_pool_finalize

2014-06-19 Thread Bruno Jimenez
Hi,

To which failure are you refering? Could you please send me a
test/program that I can try to track this down?

Thanks!
Bruno

On Thu, 2014-06-19 at 10:21 -0400, Jan Vesely wrote:
> Signed-off-by: Jan Vesely 
> CC: Bruno Jimenez 
> ---
> 
> The failure now hits assertion compute_memory_pool.c:408, instead of
> u_inlines.h:275:pipe_buffer_map_range: Assertion `offset < buffer->width0'
> 
>  src/gallium/drivers/r600/evergreen_compute.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/src/gallium/drivers/r600/evergreen_compute.c 
> b/src/gallium/drivers/r600/evergreen_compute.c
> index a2abf15..bd6e720 100644
> --- a/src/gallium/drivers/r600/evergreen_compute.c
> +++ b/src/gallium/drivers/r600/evergreen_compute.c
> @@ -659,7 +659,10 @@ static void evergreen_set_global_binding(
>   return;
>   }
>  
> - compute_memory_finalize_pending(pool, ctx_);
> + if (compute_memory_finalize_pending(pool, ctx_) == -1) {
> + /* XXX: Unset */
> + return;
> + }
>  
>   for (int i = 0; i < n; i++)
>   {
> @@ -967,7 +970,9 @@ void *r600_compute_global_transfer_map(
>   "%u (box.x)\n", buffer->chunk->id, box->x);
>  
> 
> - compute_memory_finalize_pending(pool, ctx_);
> + if (compute_memory_finalize_pending(pool, ctx_) == -1) {
> + return NULL;
> + }
>  
>   assert(resource->target == PIPE_BUFFER);
>   assert(resource->bind & PIPE_BIND_GLOBAL);


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


Re: [Mesa-dev] [PATCH 2/2] r600: Handle failures in compute_memory_pool_finalize

2014-06-19 Thread Jan Vesely
On Thu, 2014-06-19 at 17:12 +0200, Bruno Jimenez wrote:
> Hi,
> 
> To which failure are you refering? Could you please send me a
> test/program that I can try to track this down?

well, the compute_memory_finalize_pending() function can possibly return
-1 so it's prudent to check for it.

as for the testcase, I replaced the inside of 'if (need <= 0)' in the
previous patch with return -1 (to simulate failure). The I used GEGL
test op colors.xml to trigger the situation.

but gegl needs some extra patches to get working on current mesa/clover.
I can send you log with R600_DEBUG=compute if it helps.


regards,
Jan


> 
> Thanks!
> Bruno
> 
> On Thu, 2014-06-19 at 10:21 -0400, Jan Vesely wrote:
> > Signed-off-by: Jan Vesely 
> > CC: Bruno Jimenez 
> > ---
> > 
> > The failure now hits assertion compute_memory_pool.c:408, instead of
> > u_inlines.h:275:pipe_buffer_map_range: Assertion `offset < buffer->width0'
> > 
> >  src/gallium/drivers/r600/evergreen_compute.c | 9 +++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/gallium/drivers/r600/evergreen_compute.c 
> > b/src/gallium/drivers/r600/evergreen_compute.c
> > index a2abf15..bd6e720 100644
> > --- a/src/gallium/drivers/r600/evergreen_compute.c
> > +++ b/src/gallium/drivers/r600/evergreen_compute.c
> > @@ -659,7 +659,10 @@ static void evergreen_set_global_binding(
> > return;
> > }
> >  
> > -   compute_memory_finalize_pending(pool, ctx_);
> > +   if (compute_memory_finalize_pending(pool, ctx_) == -1) {
> > +   /* XXX: Unset */
> > +   return;
> > +   }
> >  
> > for (int i = 0; i < n; i++)
> > {
> > @@ -967,7 +970,9 @@ void *r600_compute_global_transfer_map(
> > "%u (box.x)\n", buffer->chunk->id, box->x);
> >  
> > 
> > -   compute_memory_finalize_pending(pool, ctx_);
> > +   if (compute_memory_finalize_pending(pool, ctx_) == -1) {
> > +   return NULL;
> > +   }
> >  
> > assert(resource->target == PIPE_BUFFER);
> > assert(resource->bind & PIPE_BIND_GLOBAL);
> 
> 

-- 
Jan Vesely 


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


Re: [Mesa-dev] [PATCH 2/2] r600: Handle failures in compute_memory_pool_finalize

2014-06-19 Thread Tom Stellard
On Thu, Jun 19, 2014 at 11:22:28AM -0400, Jan Vesely wrote:
> On Thu, 2014-06-19 at 17:12 +0200, Bruno Jimenez wrote:
> > Hi,
> > 
> > To which failure are you refering? Could you please send me a
> > test/program that I can try to track this down?
> 
> well, the compute_memory_finalize_pending() function can possibly return
> -1 so it's prudent to check for it.
> 
> as for the testcase, I replaced the inside of 'if (need <= 0)' in the
> previous patch with return -1 (to simulate failure). The I used GEGL
> test op colors.xml to trigger the situation.
> 
> but gegl needs some extra patches to get working on current mesa/clover.
> I can send you log with R600_DEBUG=compute if it helps.
> 

Have you ever looked into integrated the gegl tests with piglit, like we've
done for opencv.  This would make it much easier for other devs to execute
these tests.

-Tom

> 
> regards,
> Jan
> 
> 
> > 
> > Thanks!
> > Bruno
> > 
> > On Thu, 2014-06-19 at 10:21 -0400, Jan Vesely wrote:
> > > Signed-off-by: Jan Vesely 
> > > CC: Bruno Jimenez 
> > > ---
> > > 
> > > The failure now hits assertion compute_memory_pool.c:408, instead of
> > > u_inlines.h:275:pipe_buffer_map_range: Assertion `offset < buffer->width0'
> > > 
> > >  src/gallium/drivers/r600/evergreen_compute.c | 9 +++--
> > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/src/gallium/drivers/r600/evergreen_compute.c 
> > > b/src/gallium/drivers/r600/evergreen_compute.c
> > > index a2abf15..bd6e720 100644
> > > --- a/src/gallium/drivers/r600/evergreen_compute.c
> > > +++ b/src/gallium/drivers/r600/evergreen_compute.c
> > > @@ -659,7 +659,10 @@ static void evergreen_set_global_binding(
> > >   return;
> > >   }
> > >  
> > > - compute_memory_finalize_pending(pool, ctx_);
> > > + if (compute_memory_finalize_pending(pool, ctx_) == -1) {
> > > + /* XXX: Unset */
> > > + return;
> > > + }
> > >  
> > >   for (int i = 0; i < n; i++)
> > >   {
> > > @@ -967,7 +970,9 @@ void *r600_compute_global_transfer_map(
> > >   "%u (box.x)\n", buffer->chunk->id, box->x);
> > >  
> > > 
> > > - compute_memory_finalize_pending(pool, ctx_);
> > > + if (compute_memory_finalize_pending(pool, ctx_) == -1) {
> > > + return NULL;
> > > + }
> > >  
> > >   assert(resource->target == PIPE_BUFFER);
> > >   assert(resource->bind & PIPE_BIND_GLOBAL);
> > 
> > 
> 
> -- 
> Jan Vesely 



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

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


Re: [Mesa-dev] [PATCH 2/2] r600: Handle failures in compute_memory_pool_finalize

2014-06-19 Thread Jan Vesely
On Thu, 2014-06-19 at 08:34 -0700, Tom Stellard wrote:
> On Thu, Jun 19, 2014 at 11:22:28AM -0400, Jan Vesely wrote:
> > On Thu, 2014-06-19 at 17:12 +0200, Bruno Jimenez wrote:
> > > Hi,
> > > 
> > > To which failure are you refering? Could you please send me a
> > > test/program that I can try to track this down?
> > 
> > well, the compute_memory_finalize_pending() function can possibly return
> > -1 so it's prudent to check for it.
> > 
> > as for the testcase, I replaced the inside of 'if (need <= 0)' in the
> > previous patch with return -1 (to simulate failure). The I used GEGL
> > test op colors.xml to trigger the situation.
> > 
> > but gegl needs some extra patches to get working on current mesa/clover.
> > I can send you log with R600_DEBUG=compute if it helps.
> > 
> 
> Have you ever looked into integrated the gegl tests with piglit, like we've
> done for opencv.  This would make it much easier for other devs to execute
> these tests.

I planned to have a look after upstream gegl can run unmodified on
clover (at least some of the tests). But given that it might take some
time to reach that point, I'll try to look into it sooner.

regards,
Jan

> 
> -Tom
> 
> > 
> > regards,
> > Jan
> > 
> > 
> > > 
> > > Thanks!
> > > Bruno
> > > 
> > > On Thu, 2014-06-19 at 10:21 -0400, Jan Vesely wrote:
> > > > Signed-off-by: Jan Vesely 
> > > > CC: Bruno Jimenez 
> > > > ---
> > > > 
> > > > The failure now hits assertion compute_memory_pool.c:408, instead of
> > > > u_inlines.h:275:pipe_buffer_map_range: Assertion `offset < 
> > > > buffer->width0'
> > > > 
> > > >  src/gallium/drivers/r600/evergreen_compute.c | 9 +++--
> > > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/src/gallium/drivers/r600/evergreen_compute.c 
> > > > b/src/gallium/drivers/r600/evergreen_compute.c
> > > > index a2abf15..bd6e720 100644
> > > > --- a/src/gallium/drivers/r600/evergreen_compute.c
> > > > +++ b/src/gallium/drivers/r600/evergreen_compute.c
> > > > @@ -659,7 +659,10 @@ static void evergreen_set_global_binding(
> > > > return;
> > > > }
> > > >  
> > > > -   compute_memory_finalize_pending(pool, ctx_);
> > > > +   if (compute_memory_finalize_pending(pool, ctx_) == -1) {
> > > > +   /* XXX: Unset */
> > > > +   return;
> > > > +   }
> > > >  
> > > > for (int i = 0; i < n; i++)
> > > > {
> > > > @@ -967,7 +970,9 @@ void *r600_compute_global_transfer_map(
> > > > "%u (box.x)\n", buffer->chunk->id, box->x);
> > > >  
> > > > 
> > > > -   compute_memory_finalize_pending(pool, ctx_);
> > > > +   if (compute_memory_finalize_pending(pool, ctx_) == -1) {
> > > > +   return NULL;
> > > > +   }
> > > >  
> > > > assert(resource->target == PIPE_BUFFER);
> > > > assert(resource->bind & PIPE_BIND_GLOBAL);
> > > 
> > > 
> > 
> > -- 
> > Jan Vesely 
> 
> 
> 
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 

-- 
Jan Vesely 


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