[Mesa-stable] [PATCH v2] st/mesa: fix the layer of VDPAU surface samplers

Christian König deathsimple at vodafone.de
Fri Nov 4 13:35:43 UTC 2016


Am 04.11.2016 um 13:28 schrieb Nicolai Hähnle:
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>
> A (latent) bug in VDPAU interop was exposed by commit
> e5cc84dd43be066c1dd418e32f5ad258e31a150a.
>
> Before that commit, the st_vdpau code created samplers with
> first_layer == last_layer == 1 that the general texture handling code
> would immediately delete and re-create, because the layer does not match
> the information in the GL texture object.
>
> This was correct behavior at least in the DMABUF case, because the imported
> resource is supposed to have the correct offset already applied.  In the
> non-DMABUF case, this was just plain wrong but apparently nobody noticed.
>
> After that commit, the state tracker assumes that an existing sampler is
> correct at all times.  Existing samplers are supposed to be deleted when
> they may become invalid, and they will be created on-demand.  This meant
> that the sampler with first_layer == last_layer == 1 stuck around, leading
> to rendering artefacts (on radeonsi), command stream failures (on r600), and
> assertions (in debug builds everywhere).
>
> This patch fixes the problem by simply not creating a sampler at all in
> st_vdpau_map_surface.  We rely on the generic texture code to do the right
> thing, adding the layer_override to make the non-DMABUF case work.
>
> v2: add the layer_override
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98512
> Cc: 13.0 <mesa-stable at lists.freedesktop.org>
> Cc: Christian König <deathsimple at vodafone.de>
> Cc: Ilia Mirkin <imirkin at alum.mit.edu>

Reviewed-by: Christian König <christian.koenig at amd.com>.

> --
> This fixes the problem on radeon, but it would be good to get some testing
> on Nouveau as well. According to Christian, testing nouveau with Kodi should
> do the trick, probably mpv --hwdec=vdpau --vo=opengl should also work. Errors
> may only be noticeable as bad (de-)interlacing.
> ---
>   src/mesa/state_tracker/st_sampler_view.c | 15 +++++++++++----
>   src/mesa/state_tracker/st_texture.h      |  9 +++++++++
>   src/mesa/state_tracker/st_vdpau.c        | 20 +++++++-------------
>   3 files changed, 27 insertions(+), 17 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_sampler_view.c b/src/mesa/state_tracker/st_sampler_view.c
> index 9fe0bfe..2b2fa8b 100644
> --- a/src/mesa/state_tracker/st_sampler_view.c
> +++ b/src/mesa/state_tracker/st_sampler_view.c
> @@ -423,22 +423,26 @@ st_create_texture_sampler_view_from_stobj(struct st_context *st,
>         size = MIN2(stObj->pt->width0 - base, (unsigned)stObj->base.BufferSize);
>         if (!size)
>            return NULL;
>   
>         templ.u.buf.offset = base;
>         templ.u.buf.size = size;
>      } else {
>         templ.u.tex.first_level = stObj->base.MinLevel + stObj->base.BaseLevel;
>         templ.u.tex.last_level = last_level(stObj);
>         assert(templ.u.tex.first_level <= templ.u.tex.last_level);
> -      templ.u.tex.first_layer = stObj->base.MinLayer;
> -      templ.u.tex.last_layer = last_layer(stObj);
> +      if (stObj->layer_override) {
> +         templ.u.tex.first_layer = templ.u.tex.last_layer = stObj->layer_override;
> +      } else {
> +         templ.u.tex.first_layer = stObj->base.MinLayer;
> +         templ.u.tex.last_layer = last_layer(stObj);
> +      }
>         assert(templ.u.tex.first_layer <= templ.u.tex.last_layer);
>         templ.target = gl_target_to_pipe(stObj->base.Target);
>      }
>   
>      templ.swizzle_r = GET_SWZ(swizzle, 0);
>      templ.swizzle_g = GET_SWZ(swizzle, 1);
>      templ.swizzle_b = GET_SWZ(swizzle, 2);
>      templ.swizzle_a = GET_SWZ(swizzle, 3);
>   
>      return st->pipe->create_sampler_view(st->pipe, stObj->pt, &templ);
> @@ -471,22 +475,25 @@ st_get_texture_sampler_view_from_stobj(struct st_context *st,
>            unsigned base = stObj->base.BufferOffset;
>            unsigned size = MIN2(stObj->pt->width0 - base,
>                                 (unsigned) stObj->base.BufferSize);
>            assert(view->u.buf.offset == base);
>            assert(view->u.buf.size == size);
>         }
>         else {
>            assert(stObj->base.MinLevel + stObj->base.BaseLevel ==
>                   view->u.tex.first_level);
>            assert(last_level(stObj) == view->u.tex.last_level);
> -         assert(stObj->base.MinLayer == view->u.tex.first_layer);
> -         assert(last_layer(stObj) == view->u.tex.last_layer);
> +         assert(stObj->layer_override || stObj->base.MinLayer == view->u.tex.first_layer);
> +         assert(stObj->layer_override || last_layer(stObj) == view->u.tex.last_layer);
> +         assert(!stObj->layer_override ||
> +                (stObj->layer_override == view->u.tex.first_layer &&
> +                 stObj->layer_override == view->u.tex.last_layer));
>         }
>      }
>      else {
>         /* create new sampler view */
>         enum pipe_format format = get_sampler_view_format(st, stObj, samp);
>   
>         *sv = st_create_texture_sampler_view_from_stobj(st, stObj,
>                                                         format, glsl_version);
>   
>      }
> diff --git a/src/mesa/state_tracker/st_texture.h b/src/mesa/state_tracker/st_texture.h
> index 730843a..0ce7989 100644
> --- a/src/mesa/state_tracker/st_texture.h
> +++ b/src/mesa/state_tracker/st_texture.h
> @@ -101,20 +101,29 @@ struct st_texture_object
>       * cannot be reallocated and the format can only be changed with a sampler
>       * view or a surface.
>       */
>      GLboolean surface_based;
>   
>      /* If surface_based is true, this format should be used for all sampler
>       * views and surfaces instead of pt->format.
>       */
>      enum pipe_format surface_format;
>   
> +   /* When non-zero, samplers should use this layer instead of the one
> +    * specified by the GL state.
> +    *
> +    * This is used for VDPAU interop, where imported pipe_resources may be
> +    * array textures (containing layers with different fields) even though the
> +    * GL state describes one non-array texture per field.
> +    */
> +   uint layer_override;
> +
>      /** The glsl version of the shader seen during the previous validation */
>      unsigned prev_glsl_version;
>      /** The value of the sampler's sRGBDecode state at the previous validation */
>      GLenum prev_sRGBDecode;
>   };
>   
>   
>   static inline struct st_texture_image *
>   st_texture_image(struct gl_texture_image *img)
>   {
> diff --git a/src/mesa/state_tracker/st_vdpau.c b/src/mesa/state_tracker/st_vdpau.c
> index 7912057..0273815 100644
> --- a/src/mesa/state_tracker/st_vdpau.c
> +++ b/src/mesa/state_tracker/st_vdpau.c
> @@ -182,34 +182,36 @@ static void
>   st_vdpau_map_surface(struct gl_context *ctx, GLenum target, GLenum access,
>                        GLboolean output, struct gl_texture_object *texObj,
>                        struct gl_texture_image *texImage,
>                        const void *vdpSurface, GLuint index)
>   {
>      struct st_context *st = st_context(ctx);
>      struct st_texture_object *stObj = st_texture_object(texObj);
>      struct st_texture_image *stImage = st_texture_image(texImage);
>   
>      struct pipe_resource *res;
> -   struct pipe_sampler_view templ, **sampler_view;
>      mesa_format texFormat;
> +   uint layer_override = 0;
>   
>      if (output) {
>         res = st_vdpau_output_surface_dma_buf(ctx, vdpSurface);
>   
>         if (!res)
>            res = st_vdpau_output_surface_gallium(ctx, vdpSurface);
>   
>      } else {
>         res = st_vdpau_video_surface_dma_buf(ctx, vdpSurface, index);
>   
> -      if (!res)
> +      if (!res) {
>            res = st_vdpau_video_surface_gallium(ctx, vdpSurface, index);
> +         layer_override = index & 1;
> +      }
>      }
>   
>      if (!res) {
>         _mesa_error(ctx, GL_INVALID_OPERATION, "VDPAUMapSurfacesNV");
>         return;
>      }
>   
>      /* do we have different screen objects ? */
>      if (res->screen != st->pipe->screen) {
>         _mesa_error(ctx, GL_INVALID_OPERATION, "VDPAUMapSurfacesNV");
> @@ -226,51 +228,43 @@ st_vdpau_map_surface(struct gl_context *ctx, GLenum target, GLenum access,
>      texFormat = st_pipe_format_to_mesa_format(res->format);
>   
>      _mesa_init_teximage_fields(ctx, texImage,
>                                 res->width0, res->height0, 1, 0, GL_RGBA,
>                                 texFormat);
>   
>      pipe_resource_reference(&stObj->pt, res);
>      st_texture_release_all_sampler_views(st, stObj);
>      pipe_resource_reference(&stImage->pt, res);
>   
> -   u_sampler_view_default_template(&templ, res, res->format);
> -   templ.u.tex.first_layer = index & 1;
> -   templ.u.tex.last_layer = index & 1;
> -   templ.swizzle_r = GET_SWZ(stObj->base._Swizzle, 0);
> -   templ.swizzle_g = GET_SWZ(stObj->base._Swizzle, 1);
> -   templ.swizzle_b = GET_SWZ(stObj->base._Swizzle, 2);
> -   templ.swizzle_a = GET_SWZ(stObj->base._Swizzle, 3);
> -
> -   sampler_view = st_texture_get_sampler_view(st, stObj);
> -   *sampler_view = st->pipe->create_sampler_view(st->pipe, res, &templ);
> -
>      stObj->surface_format = res->format;
> +   stObj->layer_override = layer_override;
>   
>      _mesa_dirty_texobj(ctx, texObj);
>      pipe_resource_reference(&res, NULL);
>   }
>   
>   static void
>   st_vdpau_unmap_surface(struct gl_context *ctx, GLenum target, GLenum access,
>                          GLboolean output, struct gl_texture_object *texObj,
>                          struct gl_texture_image *texImage,
>                          const void *vdpSurface, GLuint index)
>   {
>      struct st_context *st = st_context(ctx);
>      struct st_texture_object *stObj = st_texture_object(texObj);
>      struct st_texture_image *stImage = st_texture_image(texImage);
>   
>      pipe_resource_reference(&stObj->pt, NULL);
>      st_texture_release_all_sampler_views(st, stObj);
>      pipe_resource_reference(&stImage->pt, NULL);
>   
> +   stObj->layer_override = 0;
> +
>      _mesa_dirty_texobj(ctx, texObj);
>   
>      st_flush(st, NULL, 0);
>   }
>   
>   #endif
>   
>   void
>   st_init_vdpau_functions(struct dd_function_table *functions)
>   {




More information about the mesa-stable mailing list