Re: [Mesa-dev] [PATCH 3/6] etnaviv: honor PIPE_TRANSFER_UNSYNCHRONIZED flag

[Wladimir J. van der Laan]

On Fri, May 19, 2017 at 11:41:09AM +0200, Lucas Stach wrote:
> This gets rid of quite a bit of CPU/GPU sync on frequent vertex buffer
> uploads and I haven't seen any of the issues mentioned in the comment,
> so this one seems stale.

Interesting. I don't quite remember what prompted adding this, it may have to
do with other hacks to support single-vertex-stream GPUs which have since been
removed, so it's likely stale.

In any case heeding PIPE_TRANSFER_UNSYNCHRONIZED makes sense. It should not just
be ignored because the driver thinks it knows better.

> Ignore the flag if there exists a temporary resource, as those ones are
> never busy.

OK. 

Reviewed-By: Wladimir J. van der Laan 

> Signed-off-by: Lucas Stach 
> ---
>  src/gallium/drivers/etnaviv/etnaviv_transfer.c | 22 ++
>  1 file changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_transfer.c 
> b/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> index 269bd498f89f..a2cd4e6234dd 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> @@ -114,7 +114,7 @@ etna_transfer_unmap(struct pipe_context *pctx, struct 
> pipe_transfer *ptrans)
>}
> }
>  
> -   if (!trans->rsc)
> +   if (!trans->rsc && !(ptrans->usage & PIPE_TRANSFER_UNSYNCHRONIZED))
>etna_bo_cpu_fini(rsc->bo);
>  
> pipe_resource_reference(>rsc, NULL);
> @@ -260,19 +260,17 @@ etna_transfer_map(struct pipe_context *pctx, struct 
> pipe_resource *prsc,
> (rsc->layout == ETNA_LAYOUT_TILED &&
>  util_format_is_compressed(prsc->format));
>  
> -   /* Ignore PIPE_TRANSFER_UNSYNCHRONIZED and PIPE_TRANSFER_DONTBLOCK here.
> -* It appears that Gallium operates the index/vertex buffers in a
> -* circular fashion, and the CPU can catch up with the GPU and starts
> -* overwriting yet-to-be-processed entries, causing rendering corruption. 
> */
> -   uint32_t prep_flags = 0;
> +   if (trans->rsc || !(usage & PIPE_TRANSFER_UNSYNCHRONIZED)) {
> +  uint32_t prep_flags = 0;
>  
> -   if (usage & PIPE_TRANSFER_READ)
> -  prep_flags |= DRM_ETNA_PREP_READ;
> -   if (usage & PIPE_TRANSFER_WRITE)
> -  prep_flags |= DRM_ETNA_PREP_WRITE;
> +  if (usage & PIPE_TRANSFER_READ)
> + prep_flags |= DRM_ETNA_PREP_READ;
> +  if (usage & PIPE_TRANSFER_WRITE)
> + prep_flags |= DRM_ETNA_PREP_WRITE;
>  
> -   if (etna_bo_cpu_prep(rsc->bo, prep_flags))
> -  goto fail_prep;
> +  if (etna_bo_cpu_prep(rsc->bo, prep_flags))
> + goto fail_prep;
> +   }
>  
> /* map buffer object */
> void *mapped = etna_bo_map(rsc->bo);
> -- 
> 2.11.0
> 
> ___
> etnaviv mailing list
> etna...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/etnaviv
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/6] etnaviv: honor PIPE_TRANSFER_UNSYNCHRONIZED flag

[Lucas Stach]

Am Dienstag, den 30.05.2017, 17:40 +0200 schrieb Philipp Zabel:
> On Fri, 2017-05-19 at 11:41 +0200, Lucas Stach wrote:
> > This gets rid of quite a bit of CPU/GPU sync on frequent vertex buffer
> > uploads and I haven't seen any of the issues mentioned in the comment,
> > so this one seems stale.
> > 
> > Ignore the flag if there exists a temporary resource, as those ones are
> > never busy.
> > 
> > Signed-off-by: Lucas Stach 
> > ---
> >  src/gallium/drivers/etnaviv/etnaviv_transfer.c | 22 ++
> >  1 file changed, 10 insertions(+), 12 deletions(-)
> > 
> > diff --git a/src/gallium/drivers/etnaviv/etnaviv_transfer.c 
> > b/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> > index 269bd498f89f..a2cd4e6234dd 100644
> > --- a/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> > +++ b/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> > @@ -114,7 +114,7 @@ etna_transfer_unmap(struct pipe_context *pctx, struct 
> > pipe_transfer *ptrans)
> >}
> > }
> >  
> > -   if (!trans->rsc)
> > +   if (!trans->rsc && !(ptrans->usage & PIPE_TRANSFER_UNSYNCHRONIZED))
> 
> As we just talked about, this looks like it should be '||' ...

This is actually correct as-is, the || below is for the "have temp
resource" case, while this cpu_fini is for the "no temp resource" path.

This deserves a comment, which I'll add before pushing out.

Regards,
Lucas

> >etna_bo_cpu_fini(rsc->bo);
> >  
> > pipe_resource_reference(>rsc, NULL);
> > @@ -260,19 +260,17 @@ etna_transfer_map(struct pipe_context *pctx, struct 
> > pipe_resource *prsc,
> > (rsc->layout == ETNA_LAYOUT_TILED &&
> >  util_format_is_compressed(prsc->format));
> >  
> > -   /* Ignore PIPE_TRANSFER_UNSYNCHRONIZED and PIPE_TRANSFER_DONTBLOCK here.
> > -* It appears that Gallium operates the index/vertex buffers in a
> > -* circular fashion, and the CPU can catch up with the GPU and starts
> > -* overwriting yet-to-be-processed entries, causing rendering 
> > corruption. */
> > -   uint32_t prep_flags = 0;
> > +   if (trans->rsc || !(usage & PIPE_TRANSFER_UNSYNCHRONIZED)) {
> 
> ... for symmetry with etna_bo_cpu_prep call below.
> 
> > +  uint32_t prep_flags = 0;
> >  
> > -   if (usage & PIPE_TRANSFER_READ)
> > -  prep_flags |= DRM_ETNA_PREP_READ;
> > -   if (usage & PIPE_TRANSFER_WRITE)
> > -  prep_flags |= DRM_ETNA_PREP_WRITE;
> > +  if (usage & PIPE_TRANSFER_READ)
> > + prep_flags |= DRM_ETNA_PREP_READ;
> > +  if (usage & PIPE_TRANSFER_WRITE)
> > + prep_flags |= DRM_ETNA_PREP_WRITE;
> >  
> > -   if (etna_bo_cpu_prep(rsc->bo, prep_flags))
> > -  goto fail_prep;
> > +  if (etna_bo_cpu_prep(rsc->bo, prep_flags))
> > + goto fail_prep;
> > +   }
> >  
> > /* map buffer object */
> > void *mapped = etna_bo_map(rsc->bo);
> 
> regards
> Philipp
> 
> ___
> etnaviv mailing list
> etna...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/etnaviv


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

Re: [Mesa-dev] [PATCH 3/6] etnaviv: honor PIPE_TRANSFER_UNSYNCHRONIZED flag

[Philipp Zabel]

On Fri, 2017-05-19 at 11:41 +0200, Lucas Stach wrote:
> This gets rid of quite a bit of CPU/GPU sync on frequent vertex buffer
> uploads and I haven't seen any of the issues mentioned in the comment,
> so this one seems stale.
> 
> Ignore the flag if there exists a temporary resource, as those ones are
> never busy.
> 
> Signed-off-by: Lucas Stach 
> ---
>  src/gallium/drivers/etnaviv/etnaviv_transfer.c | 22 ++
>  1 file changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_transfer.c 
> b/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> index 269bd498f89f..a2cd4e6234dd 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> @@ -114,7 +114,7 @@ etna_transfer_unmap(struct pipe_context *pctx, struct 
> pipe_transfer *ptrans)
>}
> }
>  
> -   if (!trans->rsc)
> +   if (!trans->rsc && !(ptrans->usage & PIPE_TRANSFER_UNSYNCHRONIZED))

As we just talked about, this looks like it should be '||' ...

>etna_bo_cpu_fini(rsc->bo);
>  
> pipe_resource_reference(>rsc, NULL);
> @@ -260,19 +260,17 @@ etna_transfer_map(struct pipe_context *pctx, struct 
> pipe_resource *prsc,
> (rsc->layout == ETNA_LAYOUT_TILED &&
>  util_format_is_compressed(prsc->format));
>  
> -   /* Ignore PIPE_TRANSFER_UNSYNCHRONIZED and PIPE_TRANSFER_DONTBLOCK here.
> -* It appears that Gallium operates the index/vertex buffers in a
> -* circular fashion, and the CPU can catch up with the GPU and starts
> -* overwriting yet-to-be-processed entries, causing rendering corruption. 
> */
> -   uint32_t prep_flags = 0;
> +   if (trans->rsc || !(usage & PIPE_TRANSFER_UNSYNCHRONIZED)) {

... for symmetry with etna_bo_cpu_prep call below.

> +  uint32_t prep_flags = 0;
>  
> -   if (usage & PIPE_TRANSFER_READ)
> -  prep_flags |= DRM_ETNA_PREP_READ;
> -   if (usage & PIPE_TRANSFER_WRITE)
> -  prep_flags |= DRM_ETNA_PREP_WRITE;
> +  if (usage & PIPE_TRANSFER_READ)
> + prep_flags |= DRM_ETNA_PREP_READ;
> +  if (usage & PIPE_TRANSFER_WRITE)
> + prep_flags |= DRM_ETNA_PREP_WRITE;
>  
> -   if (etna_bo_cpu_prep(rsc->bo, prep_flags))
> -  goto fail_prep;
> +  if (etna_bo_cpu_prep(rsc->bo, prep_flags))
> + goto fail_prep;
> +   }
>  
> /* map buffer object */
> void *mapped = etna_bo_map(rsc->bo);

regards
Philipp

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