Am 04.11.2016 um 10:11 schrieb Nicolai Hähnle:
On 04.11.2016 09:47, Christian König wrote:Am 03.11.2016 um 22:06 schrieb Nicolai Hähnle:From: Nicolai Hähnle <[email protected]>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 codewould immediately delete and re-create, because the layer does not matchthe information in the GL texture object. This was correct behavior, because the imported resource because theimported resource is supposed to have the correct offset already applied.After that commit, the state tracker assumes that an existing sampler is correct at all times. Existing samplers are supposed to be deleted whenthey 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). This patch fixes the problem by simply not creating a sampler at all in st_vdpau_map_surface and relying on the generic texture code to do the right thing.A big NAK on this, cause this way GL clearly samples from the wrong layer for the classic interop case. What we probably should rather do is setting index=0 on line 202 when the DMA-buf import succeeded, cause resources imported using the DMA-buf method just have one layer.Then I'm confused about how this was ever supposed to work. Take a look at st_get_texture_sampler_view_from_stobj, in particular the problematic commit e5cc84dd43be066c1dd418e32f5ad258e31a150a.Note that before the change, there was: if (*sv) { if (check_sampler_swizzle(st, stObj, *sv, glsl_version) || (format != (*sv)->format) || gl_target_to_pipe(stObj->base.Target) != (*sv)->target ||stObj->base.MinLevel + stObj->base.BaseLevel != (*sv)->u.tex.first_level ||last_level(stObj) != (*sv)->u.tex.last_level || stObj->base.MinLayer != (*sv)->u.tex.first_layer || last_layer(stObj) != (*sv)->u.tex.last_layer) { pipe_sampler_view_reference(sv, NULL); } } if (!*sv) { *sv = st_create_texture_sampler_view_from_stobj(st, stObj,format, glsl_version);In particular, note the stObj->base.MinLayer != (*sv)->u.tex.first_layer part. Sure, the st_vdpau code would create a sampler with first_layer == 1, but that sampler would almost immediately get replaced by one with first_layer == 0...
Mhm, that clearly looks like a bug to me. The samplers from the old implementation should clearly stick around, otherwise we would sample from the wrong plane.
So at least I don't see how the patch I sent could regress anything (even in combination with Brian's earlier commit).
I'm pretty sure that worked as expected when I wrote the initial code, we used to use this with r600 and radeonsi as well. But yes could be that this broke in the meantime.
But yes, it's quite possible that the other path was simply broken before without anybody noticing it. How can I test exercising the non-DMABUF path?
Get NVidia hardware, install nouveau and test with Kodi. Regards, Christian.
Cheers, NicolaiRegards, Christian.Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98512 Cc: 13.0 <[email protected]> --- src/mesa/state_tracker/st_vdpau.c | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/mesa/state_tracker/st_vdpau.c b/src/mesa/state_tracker/st_vdpau.c index 7912057..770f0ff 100644 --- a/src/mesa/state_tracker/st_vdpau.c +++ b/src/mesa/state_tracker/st_vdpau.c @@ -182,21 +182,20 @@ 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; 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); @@ -226,31 +225,20 @@ 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; _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,
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
