<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
<br>
<div class="moz-cite-prefix">On 10/27/2016 01:20 PM, Nayan Deshmukh
wrote:<br>
</div>
<blockquote
cite="mid:CAFd4ddxrEZPUEQ7uQ066Y2dR974hBE6C8SjfY02GUCmGkE3fbw@mail.gmail.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<div dir="ltr"><br>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Thu, Oct 27, 2016 at 10:17 PM, Leo
Liu <span dir="ltr"><<a moz-do-not-send="true"
href="mailto:leo.liu@amd.com" target="_blank">leo.liu@amd.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div class="gmail-HOEnZb">
<div class="gmail-h5"><br>
<br>
On 10/27/2016 12:20 PM, Nayan Deshmukh wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px
0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
On Thu, Oct 27, 2016 at 10:38:30AM -0400, Leo Liu
wrote:<br>
<blockquote class="gmail_quote" style="margin:0px
0px 0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<br>
On 10/24/2016 09:55 AM, Nayan Deshmukh wrote:<br>
<blockquote class="gmail_quote" style="margin:0px
0px 0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
Suggested-by: Leo Liu <<a
moz-do-not-send="true"
href="mailto:leo.liu@amd.com" target="_blank"><a class="moz-txt-link-abbreviated" href="mailto:leo.liu@amd.com">leo.liu@amd.com</a></a>><br>
Signed-off-by: Nayan Deshmukh <<a
moz-do-not-send="true"
href="mailto:nayan26deshmukh@gmail.com"
target="_blank"><a class="moz-txt-link-abbreviated" href="mailto:nayan26deshmukh@gmail.com">nayan26deshmukh@gmail.com</a></a>><br>
---<br>
src/gallium/auxiliary/vl/vl_w<wbr>insys.h
| 4 ++<br>
src/gallium/auxiliary/vl/vl_w<wbr>insys_dri3.c
| 89 +++++++++++++++++++++++++++---<wbr>-<br>
2 files changed, 83 insertions(+), 10
deletions(-)<br>
<br>
diff --git a/src/gallium/auxiliary/vl/vl_<wbr>winsys.h
b/src/gallium/auxiliary/vl/vl_<wbr>winsys.h<br>
index 26db9f2..7c56b48 100644<br>
--- a/src/gallium/auxiliary/vl/vl_<wbr>winsys.h<br>
+++ b/src/gallium/auxiliary/vl/vl_<wbr>winsys.h<br>
@@ -59,6 +59,10 @@ struct vl_screen<br>
void *<br>
(*get_private)(struct vl_screen *vscreen);<br>
+ void<br>
+ (*set_output_texture)(struct vl_screen
*vscreen, struct pipe_resource *buffer,<br>
+ uint32_t width,
uint32_t height);<br>
+<br>
</blockquote>
I think it's more appropriate if it's called
"set_back_texture_from_output"<br>
</blockquote>
Yeah that makes more sense.<br>
<blockquote class="gmail_quote" style="margin:0px
0px 0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<blockquote class="gmail_quote" style="margin:0px
0px 0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
struct pipe_screen *pscreen;<br>
struct pipe_loader_device *dev;<br>
};<br>
diff --git a/src/gallium/auxiliary/vl/vl_<wbr>winsys_dri3.c
b/src/gallium/auxiliary/vl/vl_<wbr>winsys_dri3.c<br>
index 2929928..44d6f4c 100644<br>
--- a/src/gallium/auxiliary/vl/vl_<wbr>winsys_dri3.c<br>
+++ b/src/gallium/auxiliary/vl/vl_<wbr>winsys_dri3.c<br>
@@ -56,6 +56,7 @@ struct vl_dri3_buffer<br>
struct xshmfence *shm_fence;<br>
bool busy;<br>
+ bool is_external_texture;<br>
uint32_t width, height, pitch;<br>
};<br>
@@ -71,6 +72,9 @@ struct vl_dri3_screen<br>
xcb_special_event_t *special_event;<br>
struct pipe_context *pipe;<br>
+ struct pipe_resource *output_texture;<br>
+ uint32_t output_texture_width;<br>
+ uint32_t output_texture_height;<br>
struct vl_dri3_buffer
*back_buffers[BACK_BUFFER_NUM]<wbr>;<br>
int cur_back;<br>
@@ -105,7 +109,8 @@ dri3_free_back_buffer(struct
vl_dri3_screen *scrn,<br>
xcb_free_pixmap(scrn->conn,
buffer->pixmap);<br>
xcb_sync_destroy_fence(scrn->c<wbr>onn,
buffer->sync_fence);<br>
xshmfence_unmap_shm(buffer->sh<wbr>m_fence);<br>
- pipe_resource_reference(&buff<wbr>er->texture,
NULL);<br>
+ if (!buffer->is_external_texture)<br>
+ pipe_resource_reference(&buffe<wbr>r->texture,
NULL);<br>
if (buffer->linear_texture)<br>
pipe_resource_reference(&buffe<wbr>r->linear_texture,
NULL);<br>
FREE(buffer);<br>
@@ -236,13 +241,24 @@
dri3_alloc_back_buffer(struct vl_dri3_screen
*scrn)<br>
templ.format = PIPE_FORMAT_B8G8R8X8_UNORM;<br>
templ.target = PIPE_TEXTURE_2D;<br>
templ.last_level = 0;<br>
- templ.width0 = scrn->width;<br>
- templ.height0 = scrn->height;<br>
+ if (scrn->output_texture) {<br>
+ templ.width0 =
(scrn->output_texture_width) ?<br>
+
scrn->output_texture_width :<br>
+
scrn->output_texture->width0;<br>
+ templ.height0 =
(scrn->output_texture_height) ?<br>
+
scrn->output_texture_height :<br>
+
scrn->output_texture->height0;<br>
+ } else {<br>
+ templ.width0 = scrn->width;<br>
+ templ.height0 = scrn->height;<br>
+ }<br>
templ.depth0 = 1;<br>
templ.array_size = 1;<br>
if (scrn->is_different_gpu) {<br>
- buffer->texture =
scrn->base.pscreen->resource_c<wbr>reate(scrn->base.pscreen,<br>
+ buffer->texture =
(scrn->output_texture) ?<br>
+ scrn->output_texture
:<br>
+
scrn->base.pscreen->resource_c<wbr>reate(scrn->base.pscreen,<br>
&templ);<br>
</blockquote>
Why not just make it in the same line?<br>
<br>
</blockquote>
I was just trying to limit characters to 78 as given
in mesa coding style.<br>
</blockquote>
<br>
</div>
</div>
It's really matters if just over a few chars. which way do
you think it will look better?
<div>
<div class="gmail-h5"><br>
<br>
</div>
</div>
</blockquote>
<div>I would prefer it in a single line, anyways I just
checked that it used in single line in entire </div>
<div>vl code. </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div>
<div class="gmail-h5">
<blockquote class="gmail_quote" style="margin:0px 0px
0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<br>
<blockquote class="gmail_quote" style="margin:0px
0px 0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<blockquote class="gmail_quote" style="margin:0px
0px 0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
if (!buffer->texture)<br>
goto unmap_shm;<br>
@@ -257,7 +273,9 @@
dri3_alloc_back_buffer(struct vl_dri3_screen
*scrn)<br>
goto no_linear_texture;<br>
} else {<br>
templ.bind |= PIPE_BIND_SCANOUT |
PIPE_BIND_SHARED;<br>
- buffer->texture =
scrn->base.pscreen->resource_c<wbr>reate(scrn->base.pscreen,<br>
+ buffer->texture =
(scrn->output_texture) ?<br>
+ scrn->output_texture
:<br>
+
scrn->base.pscreen->resource_c<wbr>reate(scrn->base.pscreen,<br>
&templ);<br>
</blockquote>
Same as above .<br>
<br>
<blockquote class="gmail_quote" style="margin:0px
0px 0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
if (!buffer->texture)<br>
goto unmap_shm;<br>
@@ -271,11 +289,20 @@
dri3_alloc_back_buffer(struct vl_dri3_screen
*scrn)<br>
usage);<br>
buffer_fd = whandle.handle;<br>
buffer->pitch = whandle.stride;<br>
+ buffer->width = templ.width0;<br>
+ buffer->height = templ.height0;<br>
+ buffer->is_external_texture =
(scrn->output_texture) ?<br>
+ true :<br>
+ false;<br>
</blockquote>
Same as above.<br>
<br>
<blockquote class="gmail_quote" style="margin:0px
0px 0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
+ scrn->output_texture = NULL;<br>
+ scrn->output_texture_width = 0;<br>
+ scrn->output_texture_height = 0;<br>
+<br>
xcb_dri3_pixmap_from_buffer(sc<wbr>rn->conn,<br>
(pixmap =
xcb_generate_id(scrn->conn)),<br>
scrn->drawable,<br>
0,<br>
- scrn->width,
scrn->height, buffer->pitch,<br>
+
buffer->width, buffer->height,
buffer->pitch,<br>
</blockquote>
The width/height here should be same as buffer
texture width/height.<br>
That's case before, now that is changed to<br>
<br>
templ.width0 =
(scrn->output_texture_width) ?<br>
scrn->output_texture_width :<br>
scrn->output_texture->width0;<br>
templ.height0 =
(scrn->output_texture_height) ?<br>
scrn->output_texture_height :<br>
scrn->output_texture-><wbr>height0;<br>
<br>
</blockquote>
I did this to support clipping for vdpau target
display as we<br>
may have to display a portion of the buffer instead
of the entire buffer<br>
<br>
In the patch you suggested you were using scrn
width/height for the buffer<br>
width/height but if there is a screen resize event
then we are sending a small<br>
buffer to X with extra width/height and would lead
to error and hence I used<br>
buffer width/height<br>
</blockquote>
<br>
</div>
</div>
scrn size always updated when resize event happens, and
back buffer size is updated accordingly.<br>
<br>
</blockquote>
<div>Exactly but when this happens the back buffer size is
updated accordingly but the output texture will still</div>
<div>be of smaller size and won't get get updated until the
next frame so it will also cause errors when we have a</div>
<div>pixmap of screen size. I tried setting it to screen
size instead of buffer size and got the following errors</div>
<div>
<div><br>
</div>
<div>X11 error: BadAlloc (insufficient resources for
operation)</div>
<div>X11 error: BadDrawable (invalid Pixmap or Window
parameter)</div>
<div>X11 error: BadPixmap (invalid Pixmap parameter)</div>
</div>
</div>
</div>
</div>
</blockquote>
<br>
You got misunderstood it.<br>
<br>
You problem is here:<br>
<br>
templ.width0 = (scrn->output_texture_width) ?<br>
scrn->output_texture_width :<br>
scrn->output_texture->width0;<br>
templ.height0 = (scrn->output_texture_height) ?<br>
scrn->output_texture_height :<br>
scrn->output_texture->height0;<br>
<br>
should be like:<br>
<br>
templ.width0 = scrn->output_texture->width0;<br>
templ.height0 = scrn->output_texture->height0;<br>
<br>
<br>
requested pixmap needs size according to that back buffer, not clips
size, not scrn size.<br>
<br>
It was scrn size before, because the scrn size is same as back
buffer size.<br>
<br>
Regards,<br>
Leo<br>
<br>
<br>
<br>
<blockquote
cite="mid:CAFd4ddxrEZPUEQ7uQ066Y2dR974hBE6C8SjfY02GUCmGkE3fbw@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div><br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
The major problem causing resize corruption is now you
request pixmaps to X dri3 ext. with wrong size.<span
class="gmail-"><br>
<br>
xcb_dri3_pixmap_from_buffer(sc<wbr>rn->conn,<br>
(pixmap =
xcb_generate_id(scrn->conn)),<br>
scrn->drawable,<br>
0,<br>
- scrn->width,
scrn->height, buffer->pitch,<br>
+ buffer->width,
buffer->height, buffer->pitch,<br>
<br>
</span></blockquote>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex"><span class="gmail-"><br>
</span>
<div>
<div class="gmail-h5">
<blockquote class="gmail_quote" style="margin:0px 0px
0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<blockquote class="gmail_quote" style="margin:0px
0px 0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
thus causing the corruption appears when resizing.<br>
<br>
<blockquote class="gmail_quote" style="margin:0px
0px 0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
scrn->depth, 32,<br>
buffer_fd);<br>
xcb_dri3_fence_from_fd(scrn->c<wbr>onn,<br>
@@ -287,8 +314,6 @@
dri3_alloc_back_buffer(struct vl_dri3_screen
*scrn)<br>
buffer->pixmap = pixmap;<br>
buffer->sync_fence = sync_fence;<br>
buffer->shm_fence = shm_fence;<br>
- buffer->width = scrn->width;<br>
- buffer->height = scrn->height;<br>
xshmfence_trigger(buffer->shm_<wbr>fence);<br>
@@ -310,6 +335,7 @@ dri3_get_back_buffer(struct
vl_dri3_screen *scrn)<br>
{<br>
struct vl_dri3_buffer *buffer;<br>
struct pipe_resource *texture = NULL;<br>
+ bool allocate_new_buffer = false;<br>
assert(scrn);<br>
@@ -318,8 +344,30 @@ dri3_get_back_buffer(struct
vl_dri3_screen *scrn)<br>
return NULL;<br>
buffer =
scrn->back_buffers[scrn->cur_b<wbr>ack];<br>
- if (!buffer || buffer->width !=
scrn->width ||<br>
- buffer->height != scrn->height) {<br>
+ /* This is normal case when our buffer is
smaller<br>
+ * than the screen this will be same for
external<br>
+ * texture<br>
+ */<br>
</blockquote>
Why do you change to size comparison to < from
!= ?, it will be a waste when<br>
from larger window to small one.<br>
</blockquote>
Since I am setting buffer width/height to texture
width/height which will be<br>
always greater than the screen size as vdpau
allocates slightly larger buffers<br>
</blockquote>
<br>
</div>
</div>
Do you consider the VA-API case?<span class="gmail-"><br>
<br>
</span></blockquote>
<div>I did not give much thought to it, need to see how
VA-API allocates buffer. </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex"><span class="gmail-">
<blockquote class="gmail_quote" style="margin:0px 0px
0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<br>
<blockquote class="gmail_quote" style="margin:0px 0px
0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<blockquote class="gmail_quote" style="margin:0px
0px 0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
+ if (!buffer || buffer->width <
scrn->width ||<br>
+ buffer->height < scrn->height)<br>
+ allocate_new_buffer = true;<br>
+<br>
+ /* If we were using a external texture buffer
and<br>
+ * the texture is not provided then we need a
new<br>
+ * buffer<br>
+ */<br>
+ if (buffer &&
buffer->is_external_texture &&<br>
+ !scrn->output_texture)<br>
+ allocate_new_buffer = true;<br>
</blockquote>
Can you elaborate this case? I cannot think of any.<br>
the texture is output surface, it should be always
provided, otherwise the<br>
where vdpau mixer render to.<br>
</blockquote>
vl_winsys_dri3.c is also used by va, and since we did
not change their<br>
code it will not set the output texture and would
expect us to send a<br>
buffer on which it will copy the output.<br>
</blockquote>
<br>
</span>
Yeah, we should definitely keep previous code path for
VA-API.<br>
<br>
But this case is not fit to VA-API, because there is no
"is_external_texture" for it.<span class="gmail-"><br>
<br>
+ if (buffer && buffer->is_external_texture
&&<br>
+ !scrn->output_texture)<br>
+ allocate_new_buffer = true;<br>
<br>
<br>
</span>
This always is false for VA-API, right?</blockquote>
<div>Yes I will remove it then. </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex"><span class="gmail-"><br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px
0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<br>
<blockquote class="gmail_quote" style="margin:0px 0px
0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
You allocate a back buffer, but that cannot be used
as output surface by<br>
vdpau. right?<br>
</blockquote>
Yes. But this case won't occur anyway<br>
</blockquote>
<br>
</span>
Please remove that to avoid the confusion if that's not
necessary.<span class="gmail-"><br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px
0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
but I just added it for safety as any<br>
state tracker would either set output texture or not
for all frames.<br>
</blockquote>
<br>
</span>
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.<br>
<br>
Regards,<br>
Leo
<div class="gmail-HOEnZb">
<div class="gmail-h5"><br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px
0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<br>
Regards,<br>
Nayan<br>
<blockquote class="gmail_quote" style="margin:0px
0px 0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<br>
Regards,<br>
Leo<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0px
0px 0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
+<br>
+ /* In case of a single gpu we need to get
the<br>
+ * handle and pixmap for the texture that is
set<br>
+ */<br>
+ if (buffer &&
buffer->is_external_texture &&<br>
+ !scrn->is_different_gpu)<br>
+ allocate_new_buffer = true;<br>
+<br>
+ if (allocate_new_buffer) {<br>
struct vl_dri3_buffer *new_buffer;<br>
new_buffer =
dri3_alloc_back_buffer(scrn);<br>
@@ -332,6 +380,13 @@ dri3_get_back_buffer(struct
vl_dri3_screen *scrn)<br>
vl_compositor_reset_dirty_are<wbr>a(&scrn->dirty_areas[scrn->cur<wbr>_back]);<br>
buffer = new_buffer;<br>
scrn->back_buffers[scrn->cur_<wbr>back]
= buffer;<br>
+ } else if (buffer->is_external_texture) {<br>
+ /* In case of different gpu we can reuse
the linear<br>
+ * texture so we only need to set the
external<br>
+ * texture for copying<br>
+ */<br>
+ buffer->texture =
scrn->output_texture;<br>
+ scrn->output_texture = NULL;<br>
}<br>
pipe_resource_reference(&textu<wbr>re,
buffer->texture);<br>
@@ -627,6 +682,19 @@
vl_dri3_screen_get_private(str<wbr>uct vl_screen
*vscreen)<br>
}<br>
static void<br>
+vl_dri3_screen_set_output_tex<wbr>ture(struct
vl_screen *vscreen, struct pipe_resource
*buffer,<br>
+ uint32_t width,
uint32_t height)<br>
+{<br>
+ struct vl_dri3_screen *scrn = (struct
vl_dri3_screen *)vscreen;<br>
+<br>
+ assert(scrn);<br>
+<br>
+ scrn->output_texture = buffer;<br>
+ scrn->output_texture_width = width;<br>
+ scrn->output_texture_height = height;<br>
+}<br>
+<br>
+static void<br>
vl_dri3_screen_destroy(struct vl_screen
*vscreen)<br>
{<br>
struct vl_dri3_screen *scrn = (struct
vl_dri3_screen *)vscreen;<br>
@@ -744,6 +812,7 @@
vl_dri3_screen_create(Display *display, int
screen)<br>
scrn->base.set_next_timestamp =
vl_dri3_screen_set_next_timest<wbr>amp;<br>
scrn->base.get_private =
vl_dri3_screen_get_private;<br>
scrn->base.pscreen->flush_fron<wbr>tbuffer
= vl_dri3_flush_frontbuffer;<br>
+ scrn->base.set_output_texture =
vl_dri3_screen_set_output_text<wbr>ure;<br>
return &scrn->base;<br>
</blockquote>
</blockquote>
</blockquote>
<br>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</div>
</blockquote>
<br>
</body>
</html>