[Mesa-dev] [PATCH 4/4] xa: reuse surfaces

[Rob Clark]

On Wed, Apr 2, 2014 at 3:47 AM, Thomas Hellstrom <thellstrom at vmware.com> wrote:
> On 04/01/2014 05:04 PM, Rob Clark wrote:
>> From: Rob Clark <robclark at freedesktop.org>
>>
>> Otherwise it will trick the gallium driver into thinking that the render
>> target has actually changed (due to different pipe_surface pointing to
>> same underlying pipe_resource).  This is really badness for tiling GPUs
>> like adreno.
>
> Rob, if we want to cache gallium surfaces like this, we need to
> condition it on the same context being used.
> We can't reuse a surface created with one context for rendering with
> another context. (I know the mesa state tracker needs to
> fix this up as well).

oh.. ugg..  do we have any cases where multiple XA contexts are used?
Or could we take the easy way out and somehow forbid that?

> While this can be done easily within XA for non-shared xa_surfaces, I
> wonder what happens in the case of
> shared xa_surfaces? More specifically, is rendering allowed to be cached
> in the gallium surface until explicitly flushed to the texture? (I'm
> unsure about gallium surface semantics here).

I'm not quite sure either.  I need to flush rendering whenever the
render target actually changes (I'm just trying to prevent flushing
when the render target doesn't change, but only the pipe_surface ptr
does).  If you are using that surface as a texture, then presumably
you needed to change the render target.

I'm less sure about other drivers.

I suppose a different approach would simply be to cache the most
recent pipe_surface.  So whenever the render target does actually
change, we create a new pipe_surface.  But we avoid creating a new
surface for the same render target when it doesn't change.

BR,
-R

> /Thomas
>
>> Signed-off-by: Rob Clark <robclark at freedesktop.org>
>> ---
>>  src/gallium/state_trackers/xa/xa_context.c | 35 +++++++++++++++++++++++-------
>>  src/gallium/state_trackers/xa/xa_priv.h    |  3 ++-
>>  src/gallium/state_trackers/xa/xa_tracker.c |  1 +
>>  3 files changed, 30 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/gallium/state_trackers/xa/xa_context.c b/src/gallium/state_trackers/xa/xa_context.c
>> index 867999a..18f0b85 100644
>> --- a/src/gallium/state_trackers/xa/xa_context.c
>> +++ b/src/gallium/state_trackers/xa/xa_context.c
>> @@ -179,6 +179,25 @@ xa_surface_unmap(struct xa_surface *srf)
>>      }
>>  }
>>
>> +static int
>> +xa_srf_create(struct xa_context *ctx, struct xa_surface *srf)
>> +{
>> +    struct pipe_screen *screen = ctx->pipe->screen;
>> +    struct pipe_surface srf_templ;
>> +
>> +    if (!screen->is_format_supported(screen,  srf->tex->format,
>> +                                  PIPE_TEXTURE_2D, 0,
>> +                                  PIPE_BIND_RENDER_TARGET))
>> +     return -XA_ERR_INVAL;
>> +
>> +    u_surface_default_template(&srf_templ, srf->tex);
>> +    srf->srf = ctx->pipe->create_surface(ctx->pipe, srf->tex, &srf_templ);
>> +    if (!srf->srf)
>> +     return -XA_ERR_NORES;
>> +
>> +    return XA_ERR_NONE;
>> +}
>> +
>>  int
>>  xa_ctx_srf_create(struct xa_context *ctx, struct xa_surface *dst)
>>  {
>> @@ -188,15 +207,15 @@ xa_ctx_srf_create(struct xa_context *ctx, struct xa_surface *dst)
>>      if (ctx->srf)
>>       return -XA_ERR_INVAL;
>>
>> -    if (!screen->is_format_supported(screen,  dst->tex->format,
>> -                                  PIPE_TEXTURE_2D, 0,
>> -                                  PIPE_BIND_RENDER_TARGET))
>> -     return -XA_ERR_INVAL;
>> +    if (!dst->srf) {
>> +     int ret = xa_srf_create(ctx, dst);
>> +     if (ret != XA_ERR_NONE)
>> +         return ret;
>> +    }
>>
>> -    u_surface_default_template(&srf_templ, dst->tex);
>> -    ctx->srf = ctx->pipe->create_surface(ctx->pipe, dst->tex, &srf_templ);
>> -    if (!ctx->srf)
>> -     return -XA_ERR_NORES;
>> +    assert(dst->srf);
>> +
>> +    pipe_surface_reference(&ctx->srf, dst->srf);
>>
>>      return XA_ERR_NONE;
>>  }
>> diff --git a/src/gallium/state_trackers/xa/xa_priv.h b/src/gallium/state_trackers/xa/xa_priv.h
>> index 7a43386..5ade6c1 100644
>> --- a/src/gallium/state_trackers/xa/xa_priv.h
>> +++ b/src/gallium/state_trackers/xa/xa_priv.h
>> @@ -46,7 +46,7 @@
>>  #define XA_EXPORT
>>  #endif
>>
>> -#define XA_VB_SIZE (100 * 4 * 3 * 4)
>> +#define XA_VB_SIZE (170 * 4 * 3 * 4)
>>  #define XA_LAST_SURFACE_TYPE (xa_type_yuv_component + 1)
>>  #define XA_MAX_SAMPLERS 3
>>
>> @@ -65,6 +65,7 @@ struct xa_surface {
>>      struct pipe_resource template;
>>      struct xa_tracker *xa;
>>      struct pipe_resource *tex;
>> +    struct pipe_surface *srf;
>>      struct pipe_transfer *transfer;
>>      unsigned int flags;
>>      struct xa_format_descriptor fdesc;
>> diff --git a/src/gallium/state_trackers/xa/xa_tracker.c b/src/gallium/state_trackers/xa/xa_tracker.c
>> index 9add584..7fed20a 100644
>> --- a/src/gallium/state_trackers/xa/xa_tracker.c
>> +++ b/src/gallium/state_trackers/xa/xa_tracker.c
>> @@ -474,6 +474,7 @@ xa_surface_unref(struct xa_surface *srf)
>>      if (srf == NULL || --srf->refcount) {
>>       return;
>>      }
>> +    pipe_surface_reference(&srf->srf, NULL);
>>      pipe_resource_reference(&srf->tex, NULL);
>>      free(srf);
>>  }