Re: [PATCH] nouveau: make sure there's always room to emit a fence

[Samuel Pitoiset <samuel.pitoiset-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

Reviewed-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>

On 10/05/2015 09:21 PM, Ilia Mirkin wrote:
> I started seeing a lot of situations on nv30 where fence emission
> wouldn't fit into the previous buffer (causing assertions). This ensures
> that whenever checking for space, we always leave a bit of extra room
> for the fence emission commands. Adjusts the nv30 and nvc0 fence
> emission logic to bypass the space checking as well.
>
> Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
> Cc: mesa-stable@lists.freedesktop.org
> ---
>   src/gallium/drivers/nouveau/nouveau_winsys.h   | 2 ++
>   src/gallium/drivers/nouveau/nv30/nv30_screen.c | 4 +++-
>   src/gallium/drivers/nouveau/nv50/nv50_screen.c | 1 +
>   src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 3 ++-
>   4 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nouveau_winsys.h b/src/gallium/drivers/nouveau/nouveau_winsys.h
> index 389a229..a44fd3e 100644
> --- a/src/gallium/drivers/nouveau/nouveau_winsys.h
> +++ b/src/gallium/drivers/nouveau/nouveau_winsys.h
> @@ -24,6 +24,8 @@ PUSH_AVAIL(struct nouveau_pushbuf *push)
>   static inline bool
>   PUSH_SPACE(struct nouveau_pushbuf *push, uint32_t size)
>   {
> +   /* Provide a buffer so that fences always have room to be emitted */
> +   size += 8;
>      if (PUSH_AVAIL(push) < size)
>         return nouveau_pushbuf_space(push, size, 0, 0) == 0;
>      return true;
> diff --git a/src/gallium/drivers/nouveau/nv30/nv30_screen.c b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
> index 39267b3..335c163 100644
> --- a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
> +++ b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
> @@ -347,7 +347,9 @@ nv30_screen_fence_emit(struct pipe_screen *pscreen, uint32_t *sequence)
>   
>      *sequence = ++screen->base.fence.sequence;
>   
> -   BEGIN_NV04(push, NV30_3D(FENCE_OFFSET), 2);
> +   assert(PUSH_AVAIL(push) >= 3);
> +   PUSH_DATA (push, NV30_3D_FENCE_OFFSET |
> +              (2 /* size */ << 18) | (7 /* subchan */ << 13));

Is there some other places where we do something like this?
If so, maybe we should introduce NV30_FIFO_PKHDR_SQ.

>      PUSH_DATA (push, 0);
>      PUSH_DATA (push, *sequence);
>   }
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_screen.c b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
> index 6012ff6..812b246 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_screen.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
> @@ -388,6 +388,7 @@ nv50_screen_fence_emit(struct pipe_screen *pscreen, u32 *sequence)
>      /* we need to do it after possible flush in MARK_RING */
>      *sequence = ++screen->base.fence.sequence;
>   
> +   assert(PUSH_AVAIL(push) >= 5);
>      PUSH_DATA (push, NV50_FIFO_PKHDR(NV50_3D(QUERY_ADDRESS_HIGH), 4));
>      PUSH_DATAh(push, screen->fence.bo->offset);
>      PUSH_DATA (push, screen->fence.bo->offset);
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
> index 32da76c..afd91e6 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
> @@ -537,7 +537,8 @@ nvc0_screen_fence_emit(struct pipe_screen *pscreen, u32 *sequence)
>      /* we need to do it after possible flush in MARK_RING */
>      *sequence = ++screen->base.fence.sequence;
>   
> -   BEGIN_NVC0(push, NVC0_3D(QUERY_ADDRESS_HIGH), 4);
> +   assert(PUSH_AVAIL(push) >= 5);
> +   PUSH_DATA (push, NVC0_FIFO_PKHDR_SQ(NVC0_3D(QUERY_ADDRESS_HIGH), 4));
>      PUSH_DATAh(push, screen->fence.bo->offset);
>      PUSH_DATA (push, screen->fence.bo->offset);
>      PUSH_DATA (push, *sequence);

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau