[Mesa-dev] [PATCH 1/3] vl/dri3: use external texture as back buffers
Nayan Deshmukh
nayan26deshmukh at gmail.com
Thu Oct 27 16:20:36 UTC 2016
On Thu, Oct 27, 2016 at 10:38:30AM -0400, Leo Liu wrote:
>
>
> On 10/24/2016 09:55 AM, Nayan Deshmukh wrote:
> > Suggested-by: Leo Liu <leo.liu at amd.com>
> > Signed-off-by: Nayan Deshmukh <nayan26deshmukh at gmail.com>
> > ---
> > src/gallium/auxiliary/vl/vl_winsys.h | 4 ++
> > src/gallium/auxiliary/vl/vl_winsys_dri3.c | 89 +++++++++++++++++++++++++++----
> > 2 files changed, 83 insertions(+), 10 deletions(-)
> >
> > diff --git a/src/gallium/auxiliary/vl/vl_winsys.h b/src/gallium/auxiliary/vl/vl_winsys.h
> > index 26db9f2..7c56b48 100644
> > --- a/src/gallium/auxiliary/vl/vl_winsys.h
> > +++ b/src/gallium/auxiliary/vl/vl_winsys.h
> > @@ -59,6 +59,10 @@ struct vl_screen
> > void *
> > (*get_private)(struct vl_screen *vscreen);
> > + void
> > + (*set_output_texture)(struct vl_screen *vscreen, struct pipe_resource *buffer,
> > + uint32_t width, uint32_t height);
> > +
>
> I think it's more appropriate if it's called "set_back_texture_from_output"
Yeah that makes more sense.
>
> > struct pipe_screen *pscreen;
> > struct pipe_loader_device *dev;
> > };
> > diff --git a/src/gallium/auxiliary/vl/vl_winsys_dri3.c b/src/gallium/auxiliary/vl/vl_winsys_dri3.c
> > index 2929928..44d6f4c 100644
> > --- a/src/gallium/auxiliary/vl/vl_winsys_dri3.c
> > +++ b/src/gallium/auxiliary/vl/vl_winsys_dri3.c
> > @@ -56,6 +56,7 @@ struct vl_dri3_buffer
> > struct xshmfence *shm_fence;
> > bool busy;
> > + bool is_external_texture;
> > uint32_t width, height, pitch;
> > };
> > @@ -71,6 +72,9 @@ struct vl_dri3_screen
> > xcb_special_event_t *special_event;
> > struct pipe_context *pipe;
> > + struct pipe_resource *output_texture;
> > + uint32_t output_texture_width;
> > + uint32_t output_texture_height;
> > struct vl_dri3_buffer *back_buffers[BACK_BUFFER_NUM];
> > int cur_back;
> > @@ -105,7 +109,8 @@ dri3_free_back_buffer(struct vl_dri3_screen *scrn,
> > xcb_free_pixmap(scrn->conn, buffer->pixmap);
> > xcb_sync_destroy_fence(scrn->conn, buffer->sync_fence);
> > xshmfence_unmap_shm(buffer->shm_fence);
> > - pipe_resource_reference(&buffer->texture, NULL);
> > + if (!buffer->is_external_texture)
> > + pipe_resource_reference(&buffer->texture, NULL);
> > if (buffer->linear_texture)
> > pipe_resource_reference(&buffer->linear_texture, NULL);
> > FREE(buffer);
> > @@ -236,13 +241,24 @@ dri3_alloc_back_buffer(struct vl_dri3_screen *scrn)
> > templ.format = PIPE_FORMAT_B8G8R8X8_UNORM;
> > templ.target = PIPE_TEXTURE_2D;
> > templ.last_level = 0;
> > - templ.width0 = scrn->width;
> > - templ.height0 = scrn->height;
> > + if (scrn->output_texture) {
> > + templ.width0 = (scrn->output_texture_width) ?
> > + scrn->output_texture_width :
> > + scrn->output_texture->width0;
> > + templ.height0 = (scrn->output_texture_height) ?
> > + scrn->output_texture_height :
> > + scrn->output_texture->height0;
> > + } else {
> > + templ.width0 = scrn->width;
> > + templ.height0 = scrn->height;
> > + }
> > templ.depth0 = 1;
> > templ.array_size = 1;
> > if (scrn->is_different_gpu) {
> > - buffer->texture = scrn->base.pscreen->resource_create(scrn->base.pscreen,
> > + buffer->texture = (scrn->output_texture) ?
> > + scrn->output_texture :
> > + scrn->base.pscreen->resource_create(scrn->base.pscreen,
> > &templ);
>
> Why not just make it in the same line?
>
I was just trying to limit characters to 78 as given in mesa coding style.
> > if (!buffer->texture)
> > goto unmap_shm;
> > @@ -257,7 +273,9 @@ dri3_alloc_back_buffer(struct vl_dri3_screen *scrn)
> > goto no_linear_texture;
> > } else {
> > templ.bind |= PIPE_BIND_SCANOUT | PIPE_BIND_SHARED;
> > - buffer->texture = scrn->base.pscreen->resource_create(scrn->base.pscreen,
> > + buffer->texture = (scrn->output_texture) ?
> > + scrn->output_texture :
> > + scrn->base.pscreen->resource_create(scrn->base.pscreen,
> > &templ);
>
> Same as above .
>
> > if (!buffer->texture)
> > goto unmap_shm;
> > @@ -271,11 +289,20 @@ dri3_alloc_back_buffer(struct vl_dri3_screen *scrn)
> > usage);
> > buffer_fd = whandle.handle;
> > buffer->pitch = whandle.stride;
> > + buffer->width = templ.width0;
> > + buffer->height = templ.height0;
> > + buffer->is_external_texture = (scrn->output_texture) ?
> > + true :
> > + false;
>
> Same as above.
>
> > + scrn->output_texture = NULL;
> > + scrn->output_texture_width = 0;
> > + scrn->output_texture_height = 0;
> > +
> > xcb_dri3_pixmap_from_buffer(scrn->conn,
> > (pixmap = xcb_generate_id(scrn->conn)),
> > scrn->drawable,
> > 0,
> > - scrn->width, scrn->height, buffer->pitch,
> > + buffer->width, buffer->height, buffer->pitch,
>
> The width/height here should be same as buffer texture width/height.
> That's case before, now that is changed to
>
> templ.width0 = (scrn->output_texture_width) ?
> scrn->output_texture_width :
> scrn->output_texture->width0;
> templ.height0 = (scrn->output_texture_height) ?
> scrn->output_texture_height :
> scrn->output_texture->height0;
>
I did this to support clipping for vdpau target display as we
may have to display a portion of the buffer instead of the entire buffer
In the patch you suggested you were using scrn width/height for the buffer
width/height but if there is a screen resize event then we are sending a small
buffer to X with extra width/height and would lead to error and hence I used
buffer width/height
>
> thus causing the corruption appears when resizing.
>
> > scrn->depth, 32,
> > buffer_fd);
> > xcb_dri3_fence_from_fd(scrn->conn,
> > @@ -287,8 +314,6 @@ dri3_alloc_back_buffer(struct vl_dri3_screen *scrn)
> > buffer->pixmap = pixmap;
> > buffer->sync_fence = sync_fence;
> > buffer->shm_fence = shm_fence;
> > - buffer->width = scrn->width;
> > - buffer->height = scrn->height;
> > xshmfence_trigger(buffer->shm_fence);
> > @@ -310,6 +335,7 @@ dri3_get_back_buffer(struct vl_dri3_screen *scrn)
> > {
> > struct vl_dri3_buffer *buffer;
> > struct pipe_resource *texture = NULL;
> > + bool allocate_new_buffer = false;
> > assert(scrn);
> > @@ -318,8 +344,30 @@ dri3_get_back_buffer(struct vl_dri3_screen *scrn)
> > return NULL;
> > buffer = scrn->back_buffers[scrn->cur_back];
> > - if (!buffer || buffer->width != scrn->width ||
> > - buffer->height != scrn->height) {
> > + /* This is normal case when our buffer is smaller
> > + * than the screen this will be same for external
> > + * texture
> > + */
>
> Why do you change to size comparison to < from != ?, it will be a waste when
> from larger window to small one.
Since I am setting buffer width/height to texture width/height which will be
always greater than the screen size as vdpau allocates slightly larger buffers
>
> > + if (!buffer || buffer->width < scrn->width ||
> > + buffer->height < scrn->height)
> > + allocate_new_buffer = true;
> > +
> > + /* If we were using a external texture buffer and
> > + * the texture is not provided then we need a new
> > + * buffer
> > + */
> > + if (buffer && buffer->is_external_texture &&
> > + !scrn->output_texture)
> > + allocate_new_buffer = true;
>
> Can you elaborate this case? I cannot think of any.
> the texture is output surface, it should be always provided, otherwise the
> where vdpau mixer render to.
vl_winsys_dri3.c is also used by va, and since we did not change their
code it will not set the output texture and would expect us to send a
buffer on which it will copy the output.
> You allocate a back buffer, but that cannot be used as output surface by
> vdpau. right?
Yes. But this case won't occur anyway but I just added it for safety as any
state tracker would either set output texture or not for all frames.
Regards,
Nayan
>
>
> Regards,
> Leo
>
>
> > +
> > + /* In case of a single gpu we need to get the
> > + * handle and pixmap for the texture that is set
> > + */
> > + if (buffer && buffer->is_external_texture &&
> > + !scrn->is_different_gpu)
> > + allocate_new_buffer = true;
> > +
> > + if (allocate_new_buffer) {
> > struct vl_dri3_buffer *new_buffer;
> > new_buffer = dri3_alloc_back_buffer(scrn);
> > @@ -332,6 +380,13 @@ dri3_get_back_buffer(struct vl_dri3_screen *scrn)
> > vl_compositor_reset_dirty_area(&scrn->dirty_areas[scrn->cur_back]);
> > buffer = new_buffer;
> > scrn->back_buffers[scrn->cur_back] = buffer;
> > + } else if (buffer->is_external_texture) {
> > + /* In case of different gpu we can reuse the linear
> > + * texture so we only need to set the external
> > + * texture for copying
> > + */
> > + buffer->texture = scrn->output_texture;
> > + scrn->output_texture = NULL;
> > }
> > pipe_resource_reference(&texture, buffer->texture);
> > @@ -627,6 +682,19 @@ vl_dri3_screen_get_private(struct vl_screen *vscreen)
> > }
> > static void
> > +vl_dri3_screen_set_output_texture(struct vl_screen *vscreen, struct pipe_resource *buffer,
> > + uint32_t width, uint32_t height)
> > +{
> > + struct vl_dri3_screen *scrn = (struct vl_dri3_screen *)vscreen;
> > +
> > + assert(scrn);
> > +
> > + scrn->output_texture = buffer;
> > + scrn->output_texture_width = width;
> > + scrn->output_texture_height = height;
> > +}
> > +
> > +static void
> > vl_dri3_screen_destroy(struct vl_screen *vscreen)
> > {
> > struct vl_dri3_screen *scrn = (struct vl_dri3_screen *)vscreen;
> > @@ -744,6 +812,7 @@ vl_dri3_screen_create(Display *display, int screen)
> > scrn->base.set_next_timestamp = vl_dri3_screen_set_next_timestamp;
> > scrn->base.get_private = vl_dri3_screen_get_private;
> > scrn->base.pscreen->flush_frontbuffer = vl_dri3_flush_frontbuffer;
> > + scrn->base.set_output_texture = vl_dri3_screen_set_output_texture;
> > return &scrn->base;
>
More information about the mesa-dev
mailing list