Stefan Schake <stsch...@gmail.com> writes:

> With the syncobj support in place, lets use it to implement the
> native fence fd extension. This mostly follows previous implementations
> in freedreno and etnaviv.

Could we include the name of the actual extension being exposed, in the
commit message?

> Signed-off-by: Stefan Schake <stsch...@gmail.com>
> ---
>  src/gallium/drivers/vc4/vc4_context.c | 47 ++++++++++++++++++++----
>  src/gallium/drivers/vc4/vc4_context.h |  5 +++
>  src/gallium/drivers/vc4/vc4_fence.c   | 68 
> ++++++++++++++++++++++++++++++++---
>  src/gallium/drivers/vc4/vc4_screen.c  |  6 ++--
>  src/gallium/drivers/vc4/vc4_screen.h  |  4 +--
>  5 files changed, 116 insertions(+), 14 deletions(-)
>
> diff --git a/src/gallium/drivers/vc4/vc4_context.c 
> b/src/gallium/drivers/vc4/vc4_context.c
> index 0deb3ef85e..d41f1b9a26 100644
> --- a/src/gallium/drivers/vc4/vc4_context.c
> +++ b/src/gallium/drivers/vc4/vc4_context.c
> @@ -37,30 +37,54 @@
>  #include "vc4_context.h"
>  #include "vc4_resource.h"
>  
> -void
> -vc4_flush(struct pipe_context *pctx)
> +static void
> +vc4_flush_sync(struct pipe_context *pctx, uint32_t *in_sync)
>  {
>          struct vc4_context *vc4 = vc4_context(pctx);
>  
>          struct hash_entry *entry;
>          hash_table_foreach(vc4->jobs, entry) {
>                  struct vc4_job *job = entry->data;
> -                vc4_job_submit(vc4, job);
> +                vc4_job_submit_sync(vc4, job, in_sync);
>          }
>  }
>  
> +void
> +vc4_flush(struct pipe_context *pctx)
> +{
> +        vc4_flush_sync(pctx, NULL);
> +}
> +
>  static void
>  vc4_pipe_flush(struct pipe_context *pctx, struct pipe_fence_handle **fence,
>                 unsigned flags)
>  {
>          struct vc4_context *vc4 = vc4_context(pctx);
>  
> -        vc4_flush(pctx);
> +        if (vc4->in_fence_fd >= 0) {
> +                /* This replaces the fence in the syncobj. */
> +                drmSyncobjImportSyncFile(vc4->fd, vc4->in_syncobj,
> +                                         vc4->in_fence_fd);
> +                close(vc4->in_fence_fd);
> +                vc4->in_fence_fd = -1;
> +                vc4_flush_sync(pctx, &vc4->in_syncobj);
> +        } else {
> +                vc4_flush(pctx);
> +        }

I suspect this is the wrong place to be doing the in-fence handling.
Imagine the sequence:

- vc4_fence_server_sync()
- Render from some shared object to a texture
- Render from the texture to the screen
- eglSwapBuffers().

This pipe_flush will happen at the top of eglSwapBuffers().  However,
the render from texture to screen will have already flushed the first
render job through vc4_flush_jobs_writing_resource() on its source
texture, so that job didn't wait on the fence like it was supposed to.

I think the solution might be to move this in_fence_fd logic to
vc4_job_submit()?

>  
>          if (fence) {
>                  struct pipe_screen *screen = pctx->screen;
> +                int fd = -1;
> +
> +                if (flags & PIPE_FLUSH_FENCE_FD) {
> +                        /* The vc4_fence takes ownership of the returned fd. 
> */
> +                        drmSyncobjExportSyncFile(vc4->fd, vc4->job_syncobj,
> +                                                 &fd);
> +                }
> +
>                  struct vc4_fence *f = vc4_fence_create(vc4->screen,
> -                                                       vc4->last_emit_seqno);
> +                                                       vc4->last_emit_seqno,
> +                                                       fd);
>                  screen->fence_reference(screen, fence, NULL);
>                  *fence = (struct pipe_fence_handle *)f;
>          }
> @@ -124,8 +148,12 @@ vc4_context_destroy(struct pipe_context *pctx)
>  
>          vc4_program_fini(pctx);
>  
> -        if (vc4->screen->has_syncobj)
> +        if (vc4->screen->has_syncobj) {
>                  drmSyncobjDestroy(vc4->fd, vc4->job_syncobj);
> +                drmSyncobjDestroy(vc4->fd, vc4->in_syncobj);
> +        }
> +        if (vc4->in_fence_fd >= 0)
> +                close(vc4->in_fence_fd);
>  
>          ralloc_free(vc4);
>  }
> @@ -161,12 +189,19 @@ vc4_context_create(struct pipe_screen *pscreen, void 
> *priv, unsigned flags)
>          vc4_query_init(pctx);
>          vc4_resource_context_init(pctx);
>  
> +        vc4_job_init(vc4);
> +        vc4_fence_context_init(vc4);
> +
>          vc4->fd = screen->fd;
>  
>          err = vc4_job_init(vc4);
>          if (err)
>                  goto fail;
>  
> +        err = vc4_fence_context_init(vc4);
> +        if (err)
> +                goto fail;
> +

Two vc4_fence_context_init()?

> +static int
> +vc4_fence_get_fd(struct pipe_screen *screen, struct pipe_fence_handle 
> *pfence)
> +{
> +        struct vc4_fence *fence = vc4_fence(pfence);
> +
> +        return dup(fence->fd);

There was a patch series recently for other drivers saying that we
should be doing "fcntl(fd, F_DUPFD_CLOEXEC, 3);" instead of
"dup(fd)". Same comment for vc4_fence_create_fd().

Attachment: signature.asc
Description: PGP signature

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

Reply via email to