On 10/27/2016 12:20 PM, Nayan Deshmukh wrote:
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....@amd.com>
Signed-off-by: Nayan Deshmukh <nayan26deshm...@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.

It's really matters if just over a few chars. which way do you think it will look better?


         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

scrn size always updated when resize event happens, and back buffer size is updated accordingly.

The major problem causing resize corruption is now you request pixmaps to X dri3 ext. with wrong size.

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,


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

Do you consider the VA-API case?

+   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.

Yeah, we should definitely keep previous code path for VA-API.

But this case is not fit to VA-API, because there is no "is_external_texture" for it.

+   if (buffer && buffer->is_external_texture &&
+       !scrn->output_texture)
+      allocate_new_buffer = true;


This always is false for VA-API, right?


You allocate a back buffer, but that cannot be used as output surface by
vdpau. right?
Yes. But this case won't occur anyway

Please remove that to avoid the confusion if that's not necessary.


  but I just added it for safety as any
state tracker would either set output texture or not for all frames.

If that's the case, the handling here is still not correct, like being said, the back buffer won't be the target for output.

Regards,
Leo

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;

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to