Re: [Mesa-dev] [PATCH 1/2] gallium: add PIPE_FLUSH_DEFERRED

[Marek Olšák]

On Mon, Jul 18, 2016 at 3:55 PM, Rob Clark  wrote:
> On Mon, Jul 18, 2016 at 9:24 AM, Marek Olšák  wrote:
>> On Mon, Jul 18, 2016 at 2:25 PM, Rob Clark  wrote:
>>> On Mon, Jul 18, 2016 at 8:16 AM, Marek Olšák  wrote:
 From: Marek Olšák 

 There are 2 uses:
 - Asynchronous flushing for multithreaded drivers.
 - Return a fence without flushing (mid-command-buffer fence). The driver
   can defer flushing until fence_finish is called.
>>>
>>> This should also be useful to me when I get a chance to rebase the
>>> gallium bits of the egl fence-fd patchset.  I guess the one question
>>> is what the behaviour is in screen->fence_finish().  I think I have a
>>> solution for that in freedreno (if I end up going the
>>> flush-from-u_queue route, since I'd end up with enough locking to
>>> flush without a ctx), although maybe that isn't the most general
>>> solution for other drivers.  I wonder if we should add an optional
>>> pipe_context ptr to fence_finish() for the cases when there is a
>>> context bound?
>>>
>>> Either way, I guess we need a bit more documentation about that.  With
>>> that resolved, r-b
>>
>> The behavior of fence_finish isn't changed. The only side effect can
>> be that fence_finish will wait a little longer. No guidance is given
>> as to how drivers should implement fence_finish with deferred flushes.
>> If some drivers can't do deferred flushes safely, they should just
>> ignore the flag.
>
> ok, mind adding something to that effect to the gallium docs?

Yes, I've added that comment of mine. (and your rb)

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

Re: [Mesa-dev] [PATCH 1/2] gallium: add PIPE_FLUSH_DEFERRED

[Rob Clark]

On Mon, Jul 18, 2016 at 9:24 AM, Marek Olšák  wrote:
> On Mon, Jul 18, 2016 at 2:25 PM, Rob Clark  wrote:
>> On Mon, Jul 18, 2016 at 8:16 AM, Marek Olšák  wrote:
>>> From: Marek Olšák 
>>>
>>> There are 2 uses:
>>> - Asynchronous flushing for multithreaded drivers.
>>> - Return a fence without flushing (mid-command-buffer fence). The driver
>>>   can defer flushing until fence_finish is called.
>>
>> This should also be useful to me when I get a chance to rebase the
>> gallium bits of the egl fence-fd patchset.  I guess the one question
>> is what the behaviour is in screen->fence_finish().  I think I have a
>> solution for that in freedreno (if I end up going the
>> flush-from-u_queue route, since I'd end up with enough locking to
>> flush without a ctx), although maybe that isn't the most general
>> solution for other drivers.  I wonder if we should add an optional
>> pipe_context ptr to fence_finish() for the cases when there is a
>> context bound?
>>
>> Either way, I guess we need a bit more documentation about that.  With
>> that resolved, r-b
>
> The behavior of fence_finish isn't changed. The only side effect can
> be that fence_finish will wait a little longer. No guidance is given
> as to how drivers should implement fence_finish with deferred flushes.
> If some drivers can't do deferred flushes safely, they should just
> ignore the flag.

ok, mind adding something to that effect to the gallium docs?

I believe, at least for egl fences (less sure about glx), it would be
possible to not have the restriction that driver must be able to flush
a fence without a context.  And for the context-bound case, passing an
optional pipe_context to fence_finish() (ie. NULL if no ctx bound)
would be sufficient:

"
If the sync object being blocked upon will not be signaled in finite
time (for example, by an associated fence command issued previously,
but not yet flushed to the graphics pipeline), then
eglClientWaitSyncKHR may wait forever. To help prevent this behavior
(footnote1), if the EGL_SYNC_FLUSH_COMMANDS_BIT_KHR bit is set in
, and  is unsignaled when eglClientWaitSyncKHR is
called, then the equivalent of Flush() will be performed for the
current API context (i.e., the context returned by
eglGetCurrentContext()) before blocking on . If no context is
current for the bound API, the EGL_SYNC_FLUSH_COMMANDS_BIT_KHR bit
is ignored.
"

BR,
-R
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] gallium: add PIPE_FLUSH_DEFERRED

[Marek Olšák]

On Mon, Jul 18, 2016 at 2:25 PM, Rob Clark  wrote:
> On Mon, Jul 18, 2016 at 8:16 AM, Marek Olšák  wrote:
>> From: Marek Olšák 
>>
>> There are 2 uses:
>> - Asynchronous flushing for multithreaded drivers.
>> - Return a fence without flushing (mid-command-buffer fence). The driver
>>   can defer flushing until fence_finish is called.
>
> This should also be useful to me when I get a chance to rebase the
> gallium bits of the egl fence-fd patchset.  I guess the one question
> is what the behaviour is in screen->fence_finish().  I think I have a
> solution for that in freedreno (if I end up going the
> flush-from-u_queue route, since I'd end up with enough locking to
> flush without a ctx), although maybe that isn't the most general
> solution for other drivers.  I wonder if we should add an optional
> pipe_context ptr to fence_finish() for the cases when there is a
> context bound?
>
> Either way, I guess we need a bit more documentation about that.  With
> that resolved, r-b

The behavior of fence_finish isn't changed. The only side effect can
be that fence_finish will wait a little longer. No guidance is given
as to how drivers should implement fence_finish with deferred flushes.
If some drivers can't do deferred flushes safely, they should just
ignore the flag.

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

Re: [Mesa-dev] [PATCH 1/2] gallium: add PIPE_FLUSH_DEFERRED

[Rob Clark]

On Mon, Jul 18, 2016 at 8:16 AM, Marek Olšák  wrote:
> From: Marek Olšák 
>
> There are 2 uses:
> - Asynchronous flushing for multithreaded drivers.
> - Return a fence without flushing (mid-command-buffer fence). The driver
>   can defer flushing until fence_finish is called.

This should also be useful to me when I get a chance to rebase the
gallium bits of the egl fence-fd patchset.  I guess the one question
is what the behaviour is in screen->fence_finish().  I think I have a
solution for that in freedreno (if I end up going the
flush-from-u_queue route, since I'd end up with enough locking to
flush without a ctx), although maybe that isn't the most general
solution for other drivers.  I wonder if we should add an optional
pipe_context ptr to fence_finish() for the cases when there is a
context bound?

Either way, I guess we need a bit more documentation about that.  With
that resolved, r-b

> This is required to make Bioshock Infinite faster, which creates
> 1000 fences (flushes) per frame.

*ouch*

BR,
-R

> ---
>  src/gallium/docs/source/context.rst| 5 +
>  src/gallium/include/pipe/p_defines.h   | 3 ++-
>  src/mesa/state_tracker/st_cb_syncobj.c | 2 +-
>  3 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/docs/source/context.rst 
> b/src/gallium/docs/source/context.rst
> index 05c6f11..2f803ac 100644
> --- a/src/gallium/docs/source/context.rst
> +++ b/src/gallium/docs/source/context.rst
> @@ -463,6 +463,11 @@ Flushing
>
>  ``flush``
>
> +PIPE_FLUSH_END_OF_FRAME: Whether the flush marks the end of frame.
> +PIPE_FLUSH_DEFERRED: It is not required to flush right away, but it is 
> required
> +to return a valid fence.
> +
> +
>
>  ``flush_resource``
>
> diff --git a/src/gallium/include/pipe/p_defines.h 
> b/src/gallium/include/pipe/p_defines.h
> index 62fa673..a2f5193 100644
> --- a/src/gallium/include/pipe/p_defines.h
> +++ b/src/gallium/include/pipe/p_defines.h
> @@ -348,7 +348,8 @@ enum pipe_transfer_usage
>   */
>  enum pipe_flush_flags
>  {
> -   PIPE_FLUSH_END_OF_FRAME = (1 << 0)
> +   PIPE_FLUSH_END_OF_FRAME = (1 << 0),
> +   PIPE_FLUSH_DEFERRED = (1 << 1),
>  };
>
>  /**
> diff --git a/src/mesa/state_tracker/st_cb_syncobj.c 
> b/src/mesa/state_tracker/st_cb_syncobj.c
> index ec2687f..69f2a28 100644
> --- a/src/mesa/state_tracker/st_cb_syncobj.c
> +++ b/src/mesa/state_tracker/st_cb_syncobj.c
> @@ -73,7 +73,7 @@ static void st_fence_sync(struct gl_context *ctx, struct 
> gl_sync_object *obj,
> assert(condition == GL_SYNC_GPU_COMMANDS_COMPLETE && flags == 0);
> assert(so->fence == NULL);
>
> -   pipe->flush(pipe, >fence, 0);
> +   pipe->flush(pipe, >fence, PIPE_FLUSH_DEFERRED);
>  }
>
>  static void st_check_sync(struct gl_context *ctx, struct gl_sync_object *obj)
> --
> 2.7.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] gallium: add PIPE_FLUSH_DEFERRED

[Edward O'Callaghan]

This series is,

Reviewed-by: Edward O'Callaghan 

On 07/18/2016 10:16 PM, Marek Olšák wrote:
> From: Marek Olšák 
> 
> There are 2 uses:
> - Asynchronous flushing for multithreaded drivers.
> - Return a fence without flushing (mid-command-buffer fence). The driver
>   can defer flushing until fence_finish is called.
> 
> This is required to make Bioshock Infinite faster, which creates
> 1000 fences (flushes) per frame.
> ---
>  src/gallium/docs/source/context.rst| 5 +
>  src/gallium/include/pipe/p_defines.h   | 3 ++-
>  src/mesa/state_tracker/st_cb_syncobj.c | 2 +-
>  3 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/src/gallium/docs/source/context.rst 
> b/src/gallium/docs/source/context.rst
> index 05c6f11..2f803ac 100644
> --- a/src/gallium/docs/source/context.rst
> +++ b/src/gallium/docs/source/context.rst
> @@ -463,6 +463,11 @@ Flushing
>  
>  ``flush``
>  
> +PIPE_FLUSH_END_OF_FRAME: Whether the flush marks the end of frame.
> +PIPE_FLUSH_DEFERRED: It is not required to flush right away, but it is 
> required
> +to return a valid fence.
> +
> +
>  
>  ``flush_resource``
>  
> diff --git a/src/gallium/include/pipe/p_defines.h 
> b/src/gallium/include/pipe/p_defines.h
> index 62fa673..a2f5193 100644
> --- a/src/gallium/include/pipe/p_defines.h
> +++ b/src/gallium/include/pipe/p_defines.h
> @@ -348,7 +348,8 @@ enum pipe_transfer_usage
>   */
>  enum pipe_flush_flags
>  {
> -   PIPE_FLUSH_END_OF_FRAME = (1 << 0)
> +   PIPE_FLUSH_END_OF_FRAME = (1 << 0),
> +   PIPE_FLUSH_DEFERRED = (1 << 1),
>  };
>  
>  /**
> diff --git a/src/mesa/state_tracker/st_cb_syncobj.c 
> b/src/mesa/state_tracker/st_cb_syncobj.c
> index ec2687f..69f2a28 100644
> --- a/src/mesa/state_tracker/st_cb_syncobj.c
> +++ b/src/mesa/state_tracker/st_cb_syncobj.c
> @@ -73,7 +73,7 @@ static void st_fence_sync(struct gl_context *ctx, struct 
> gl_sync_object *obj,
> assert(condition == GL_SYNC_GPU_COMMANDS_COMPLETE && flags == 0);
> assert(so->fence == NULL);
>  
> -   pipe->flush(pipe, >fence, 0);
> +   pipe->flush(pipe, >fence, PIPE_FLUSH_DEFERRED);
>  }
>  
>  static void st_check_sync(struct gl_context *ctx, struct gl_sync_object *obj)
> 



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