Re: [Mesa-dev] [PATCH] nouveau: avoid emitting new fences unnecessarily

[Samuel Pitoiset]

Does this fix those texelFetch piglit tests ? Or is it the second patch ?

Anyway, this patch is :

Reviewed-by: Samuel Pitoiset 

On 10/10/2015 08:12 AM, Ilia Mirkin wrote:

Right now we emit on every kick, but this is only necessary if something
will ever be able to observe that the fence completed. If there are no
refs, leave the fence alone and emit it another day.

This also happens to work around an issue for the kick handler -- a kick
can be a result of e.g. nouveau_bo_wait or explicit kick, or it can be
due to lack of space in the pushbuf. We want the emit to happen in the
current batch, so we want there to always be enough space. However an
explicit kick could take the reserved space for the implicitly-triggered
kick's fence emission if it happened right after. With the new mechanism,
hopefully there's no way to cause two fences to be emitted into the same
reserved space.

Signed-off-by: Ilia Mirkin 
Cc: mesa-sta...@lists.freedesktop.org
Fixes: 47d11990b (nouveau: make sure there's always room to emit a fence)
---
  src/gallium/drivers/nouveau/nouveau_fence.c | 12 +---
  1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nouveau_fence.c 
b/src/gallium/drivers/nouveau/nouveau_fence.c
index ee4e08d..18b1592 100644
--- a/src/gallium/drivers/nouveau/nouveau_fence.c
+++ b/src/gallium/drivers/nouveau/nouveau_fence.c
@@ -190,8 +190,10 @@ nouveau_fence_wait(struct nouveau_fence *fence)
 /* wtf, someone is waiting on a fence in flush_notify handler? */
 assert(fence->state != NOUVEAU_FENCE_STATE_EMITTING);
  
-   if (fence->state < NOUVEAU_FENCE_STATE_EMITTED)

+   if (fence->state < NOUVEAU_FENCE_STATE_EMITTED) {
+  PUSH_SPACE(screen->pushbuf, 8);
nouveau_fence_emit(fence);
+   }
  
 if (fence->state < NOUVEAU_FENCE_STATE_FLUSHED)

if (nouveau_pushbuf_kick(screen->pushbuf, screen->pushbuf->channel))
@@ -224,8 +226,12 @@ nouveau_fence_wait(struct nouveau_fence *fence)
  void
  nouveau_fence_next(struct nouveau_screen *screen)
  {
-   if (screen->fence.current->state < NOUVEAU_FENCE_STATE_EMITTING)
-  nouveau_fence_emit(screen->fence.current);
+   if (screen->fence.current->state < NOUVEAU_FENCE_STATE_EMITTING) {
+  if (screen->fence.current->ref > 1)
+ nouveau_fence_emit(screen->fence.current);
+  else
+ return;
+   }
  
 nouveau_fence_ref(NULL, >fence.current);
  


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

Re: [Mesa-dev] [PATCH] nouveau: avoid emitting new fences unnecessarily

[Ilia Mirkin]

On Sat, Oct 10, 2015 at 3:41 PM, Samuel Pitoiset
 wrote:
> Does this fix those texelFetch piglit tests ? Or is it the second patch ?

This patch "fixes" the initial texelFetch piglit failures. However it
creates some fresh texelFetch piglit failures -- that test is
interesting because it does a lot of draws with minimal state changes
between them. Those ones are fixed by the second patch. But really
these are all different problems, which interact with each other in
frustrating ways.

>
> Anyway, this patch is :
>
> Reviewed-by: Samuel Pitoiset 
>
>
> On 10/10/2015 08:12 AM, Ilia Mirkin wrote:
>>
>> Right now we emit on every kick, but this is only necessary if something
>> will ever be able to observe that the fence completed. If there are no
>> refs, leave the fence alone and emit it another day.
>>
>> This also happens to work around an issue for the kick handler -- a kick
>> can be a result of e.g. nouveau_bo_wait or explicit kick, or it can be
>> due to lack of space in the pushbuf. We want the emit to happen in the
>> current batch, so we want there to always be enough space. However an
>> explicit kick could take the reserved space for the implicitly-triggered
>> kick's fence emission if it happened right after. With the new mechanism,
>> hopefully there's no way to cause two fences to be emitted into the same
>> reserved space.
>>
>> Signed-off-by: Ilia Mirkin 
>> Cc: mesa-sta...@lists.freedesktop.org
>> Fixes: 47d11990b (nouveau: make sure there's always room to emit a fence)
>> ---
>>   src/gallium/drivers/nouveau/nouveau_fence.c | 12 +---
>>   1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/gallium/drivers/nouveau/nouveau_fence.c
>> b/src/gallium/drivers/nouveau/nouveau_fence.c
>> index ee4e08d..18b1592 100644
>> --- a/src/gallium/drivers/nouveau/nouveau_fence.c
>> +++ b/src/gallium/drivers/nouveau/nouveau_fence.c
>> @@ -190,8 +190,10 @@ nouveau_fence_wait(struct nouveau_fence *fence)
>>  /* wtf, someone is waiting on a fence in flush_notify handler? */
>>  assert(fence->state != NOUVEAU_FENCE_STATE_EMITTING);
>>   -   if (fence->state < NOUVEAU_FENCE_STATE_EMITTED)
>> +   if (fence->state < NOUVEAU_FENCE_STATE_EMITTED) {
>> +  PUSH_SPACE(screen->pushbuf, 8);
>> nouveau_fence_emit(fence);
>> +   }
>>if (fence->state < NOUVEAU_FENCE_STATE_FLUSHED)
>> if (nouveau_pushbuf_kick(screen->pushbuf,
>> screen->pushbuf->channel))
>> @@ -224,8 +226,12 @@ nouveau_fence_wait(struct nouveau_fence *fence)
>>   void
>>   nouveau_fence_next(struct nouveau_screen *screen)
>>   {
>> -   if (screen->fence.current->state < NOUVEAU_FENCE_STATE_EMITTING)
>> -  nouveau_fence_emit(screen->fence.current);
>> +   if (screen->fence.current->state < NOUVEAU_FENCE_STATE_EMITTING) {
>> +  if (screen->fence.current->ref > 1)
>> + nouveau_fence_emit(screen->fence.current);
>> +  else
>> + return;
>> +   }
>>nouveau_fence_ref(NULL, >fence.current);
>>
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] nouveau: avoid emitting new fences unnecessarily

[Ilia Mirkin]

Right now we emit on every kick, but this is only necessary if something
will ever be able to observe that the fence completed. If there are no
refs, leave the fence alone and emit it another day.

This also happens to work around an issue for the kick handler -- a kick
can be a result of e.g. nouveau_bo_wait or explicit kick, or it can be
due to lack of space in the pushbuf. We want the emit to happen in the
current batch, so we want there to always be enough space. However an
explicit kick could take the reserved space for the implicitly-triggered
kick's fence emission if it happened right after. With the new mechanism,
hopefully there's no way to cause two fences to be emitted into the same
reserved space.

Signed-off-by: Ilia Mirkin 
Cc: mesa-sta...@lists.freedesktop.org
Fixes: 47d11990b (nouveau: make sure there's always room to emit a fence)
---
 src/gallium/drivers/nouveau/nouveau_fence.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nouveau_fence.c 
b/src/gallium/drivers/nouveau/nouveau_fence.c
index ee4e08d..18b1592 100644
--- a/src/gallium/drivers/nouveau/nouveau_fence.c
+++ b/src/gallium/drivers/nouveau/nouveau_fence.c
@@ -190,8 +190,10 @@ nouveau_fence_wait(struct nouveau_fence *fence)
/* wtf, someone is waiting on a fence in flush_notify handler? */
assert(fence->state != NOUVEAU_FENCE_STATE_EMITTING);
 
-   if (fence->state < NOUVEAU_FENCE_STATE_EMITTED)
+   if (fence->state < NOUVEAU_FENCE_STATE_EMITTED) {
+  PUSH_SPACE(screen->pushbuf, 8);
   nouveau_fence_emit(fence);
+   }
 
if (fence->state < NOUVEAU_FENCE_STATE_FLUSHED)
   if (nouveau_pushbuf_kick(screen->pushbuf, screen->pushbuf->channel))
@@ -224,8 +226,12 @@ nouveau_fence_wait(struct nouveau_fence *fence)
 void
 nouveau_fence_next(struct nouveau_screen *screen)
 {
-   if (screen->fence.current->state < NOUVEAU_FENCE_STATE_EMITTING)
-  nouveau_fence_emit(screen->fence.current);
+   if (screen->fence.current->state < NOUVEAU_FENCE_STATE_EMITTING) {
+  if (screen->fence.current->ref > 1)
+ nouveau_fence_emit(screen->fence.current);
+  else
+ return;
+   }
 
nouveau_fence_ref(NULL, >fence.current);
 
-- 
2.4.9

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