On Sat, 2014-01-04 at 08:52 -0800, Tom Stellard wrote:
> On Sat, Jan 04, 2014 at 01:27:32AM +0100, Bruno Jiménez wrote:
> > ---
> > src/gallium/drivers/r600/compute_memory_pool.c | 18 +++++++-----------
> > 1 file changed, 7 insertions(+), 11 deletions(-)
> >
> > diff --git a/src/gallium/drivers/r600/compute_memory_pool.c
> > b/src/gallium/drivers/r600/compute_memory_pool.c
> > index 5374a48..954c890 100644
> > --- a/src/gallium/drivers/r600/compute_memory_pool.c
> > +++ b/src/gallium/drivers/r600/compute_memory_pool.c
> > @@ -320,21 +320,17 @@ int compute_memory_finalize_pending(struct
> > compute_memory_pool* pool,
> > int64_t need = item->size_in_dw+2048 -
> > (pool->size_in_dw - allocated);
> >
> > - need += 1024 - (need % 1024);
> >
> > - if (need > 0) {
> > - err = compute_memory_grow_pool(pool,
> > - pipe,
> > - pool->size_in_dw + need);
> > - }
> > - else {
> > + if (need <= 0) {
>
> It looks like we are changing the behavior of the code here,
> because we are no longer doing need += 1024 - (need % 1024) before
> checking the value of need.
>
> This code really does need to be cleaned up, but I want to make sure we
> aren't changing its behavior, or if we are we have a test case to show
> we aren't breaking things.
>
> If you don't think we are changing behavior here, can you explain why
> not?
>
> Thanks,
> Tom
You are completely wright, it changes the behaviour for need == 0 when
calculated. Still, I think it can be addressed just by checking need <
< 0.
I'll explain myself:
Suppose that need is 0 or positive, with
need += 1024 - (need % 1024)
we do something like ceil to the nearest multiple of
1024, thus 0 will be 1024, 512 will also be 1024,
1025 will be 2048 and so on. So now need is always
positive, and we do compute_memory_grow_pool, check
its output and continue.
If need was negative it after
need += 1024 - (need % 1024)
it will still be negative OR 0, so now we take the
else and recalculate need as
need = pool->size_in_dw/10
need += 1024 - (need % 1024)
then we do compute_memory_grow, check its output
and continue.
What I changed is that we just check need for being
positive or negative or 0, in the second case, need
is just pool->size_in_dw/10, after that, we calculate
need += 1024 - (need % 1024)
and do compute_memory_grow, check its output and
continue.
So, you were wright, I changed the behaviour for
need == 0. But just checking for need < 0 should
address it.
If this is OK, I'll prepare a version 2 of the patches
later for submission.
Thanks!
Bruno
> > need = pool->size_in_dw / 10;
> > - need += 1024 - (need % 1024);
> > - err = compute_memory_grow_pool(pool,
> > - pipe,
> > - pool->size_in_dw + need);
> > }
> >
> > + need += 1024 - (need % 1024);
> > +
> > + err = compute_memory_grow_pool(pool,
> > + pipe,
> > + pool->size_in_dw + need);
> > +
> > if(err == -1)
> > return -1;
> > }
> > --
> > 1.8.5.2
> >
> > _______________________________________________
> > 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
