<div dir="ltr"><span class=""><br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div>To go back to "add a bind flag to struct
pipe_video_buffer instead ", the alternative is to bring
back the first version<br>
of the patch but according to the first review, it was
duplication of bind flag between pipe_video_buffer<br>
</div>
<div>and pipe_resource so it would require quite of big of
refactoring unless I miss understood the comment.<br>
</div>
</div>
</div>
</div>
</blockquote>
</span>
>> I obviously have missed that discussion, but yes that is exactly the
way we should go. The duplication is unproblematic as far as I can
see.<span class=""><br></span><br>The v1 patch and discussion is here: <a href="https://patchwork.freedesktop.org/patch/66382/">https://patchwork.freedesktop.org/patch/66382/</a><br>In that v1 should I just replace the specific "bool disable_tiling" by a generic bind flag ?<br><div class="gmail_extra"><br><div class="gmail_quote">On 8 June 2016 at 14:07, Christian König <span dir="ltr"><<a href="mailto:deathsimple@vodafone.de" target="_blank">deathsimple@vodafone.de</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 text="#000000" bgcolor="#FFFFFF"><div><div class="h5">
<div>Am 08.06.2016 um 14:20 schrieb Julien
Isorce:<br>
</div>
<blockquote type="cite">
<div dir="ltr"><br>
<div class="gmail_extra"><br>
<div class="gmail_quote">On 8 June 2016 at 09:16, Christian
König <span dir="ltr"><<a href="mailto:deathsimple@vodafone.de" target="_blank">deathsimple@vodafone.de</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"><span>Am 02.06.2016 um
16:00 schrieb Julien Isorce:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
In order to do zero-copy between two different devices<br>
the memory should not be tiled.<br>
<br>
This is currently no way to set pipe_resource
template's flag<br>
from pipe_video_buffer template. So disabled_tiling is
added.<br>
<br>
Choosed "disable" prefix so that CALLOC keeps tiling
enabled<br>
by default.<br>
<br>
Tested with GStreamer on a laptop that has 2 GPUs:<br>
1- gstvaapidecode:<br>
HW decoding and dmabuf export with nouveau driver
on Nvidia GPU.<br>
2- glimagesink:<br>
EGLImage imports dmabuf on Intel GPU.<br>
<br>
Note that tiling is working if 1 and 2 are done on the
same GPU.<br>
So it is up to the application to set or not the flag:<br>
VA_SURFACE_EXTBUF_DESC_ENABLE_TILING<br>
<br>
Signed-off-by: Julien Isorce <<a href="mailto:j.isorce@samsung.com" target="_blank"></a><a href="mailto:j.isorce@samsung.com" target="_blank">j.isorce@samsung.com</a>><br>
</blockquote>
<br>
</span></blockquote>
<div><br>
</div>
<div>Thx for the review.<br>
</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span></span>
NAK, it won't be possible to use the resulting video
buffer with hardware decoding on AMD hardware.<br>
</blockquote>
<div><br>
</div>
<div>But I restrict to these format:<br>
</div>
<div><br>
+ case VA_FOURCC_RGBA:<br>
+ case VA_FOURCC_RGBX:<br>
+ case VA_FOURCC_BGRA:<br>
+ case VA_FOURCC_BGRX:<br>
<br>
</div>
<div>So if the vaapi user request a linear layout, it will
fail if not one of these formats. So basically for now it
requires vpp.<br>
</div>
</div>
</div>
</div>
</blockquote>
<br></div></div>
Yeah, ok that should work for now but is clearly not a good idea.<span class=""><br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Please add a bind flag to struct pipe_video_buffer instead
so that we can specify if linear layout is requested or
not.<br>
</blockquote>
<div><br>
</div>
<div>Do you mean that resource =
pscreen->resource_create(pscreen, &tmpl) does not
honor the bind flag of the template.<br>
</div>
<div>Maybe I can just checked if it was effective after that
call, i.e. checking presence of PIPE_BIND_LINEAR in<br>
resources[0]->bind.</div>
</div>
</div>
</div>
</blockquote>
<br></span>
No, for resource_create() the flag should be honored. But we
shouldn't be using resource_create() to create the different planes
of video buffers if possible. We should use create_video_buffer() on
the pipe context if possible.<span class=""><br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
This way the hardware driver can still reject the request
if this would result in a surface which can't be decoded
to.<br>
</blockquote>
<div><br>
</div>
<div>For now it requires vpp since I explicitly restricted
linear layout request to the rgbs format above. The reason
behind is<br>
</div>
<div>that vaapi is limited to export 1 fd per surface.
Problem is that for at least nouveau, it uses 1 pipe
resource per plane, and<br>
</div>
<div>NV12 has 2 planes.<br>
<br>
</div>
<div>In the spec the problem comes from the fact that a
VAImage has only one VABufferID. It would require to
define<br>
</div>
<div>a new VABufferType which represents an array of
VAImageBufferType, something like that.<br>
</div>
</div>
</div>
</div>
</blockquote>
<br></span>
Yeah, I know that is one of the many deficits VA-API unfortunately
has. It should work with the Radeon implementation, but only because
UVD requires both planes to be in the same buffer object.<span class=""><br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div><br>
<br>
</div>
<div>To go back to "add a bind flag to struct
pipe_video_buffer instead ", the alternative is to bring
back the first version<br>
of the patch but according to the first review, it was
duplication of bind flag between pipe_video_buffer<br>
</div>
<div>and pipe_resource so it would require quite of big of
refactoring unless I miss understood the comment.<br>
</div>
</div>
</div>
</div>
</blockquote>
<br></span>
I obviously have missed that discussion, but yes that is exactly the
way we should go. The duplication is unproblematic as far as I can
see.<span class=""><br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div>Also in vdpau, the function vlVdpOutputSurfaceCreate is
using PIPE_BIND_LINEAR flag and resource_create call,<br>
like in the v2 of my patch.<br>
</div>
</div>
</div>
</div>
</blockquote>
<br></span>
Yeah, but that isn't a video buffer you try to create here. VA-API
unfortunately doesn't properly distinct between those two things. <br>
<br>
Regards,<br>
Christian.<div><div class="h5"><br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div><br>
</div>
<div>Not sure if I replied to your concerns, let me know.<br>
</div>
<div><br>
</div>
<div>Thx<br>
</div>
<div>Julien<br>
</div>
<div> </div>
<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>
Christian.
<div>
<div><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>
src/gallium/state_trackers/va/surface.c | 60
++++++++++++++++++++++++++++++++-<br>
1 file changed, 59 insertions(+), 1 deletion(-)<br>
<br>
diff --git a/src/gallium/state_trackers/va/surface.c
b/src/gallium/state_trackers/va/surface.c<br>
index 8a6a397..b04ced4 100644<br>
--- a/src/gallium/state_trackers/va/surface.c<br>
+++ b/src/gallium/state_trackers/va/surface.c<br>
@@ -507,7 +507,9 @@
vlVaCreateSurfaces2(VADriverContextP ctx, unsigned
int format,<br>
int i;<br>
int memory_type;<br>
int expected_fourcc;<br>
+ int linear_layout;<br>
VAStatus vaStatus;<br>
+ const enum pipe_format *resource_formats;<br>
if (!ctx)<br>
return VA_STATUS_ERROR_INVALID_CONTEXT;<br>
@@ -529,6 +531,7 @@
vlVaCreateSurfaces2(VADriverContextP ctx, unsigned
int format,<br>
memory_attibute = NULL;<br>
memory_type = VA_SURFACE_ATTRIB_MEM_TYPE_VA;<br>
expected_fourcc = 0;<br>
+ resource_formats = NULL;<br>
for (i = 0; i < num_attribs &&
attrib_list; i++) {<br>
if ((attrib_list[i].type ==
VASurfaceAttribPixelFormat) &&<br>
@@ -569,8 +572,27 @@
vlVaCreateSurfaces2(VADriverContextP ctx, unsigned
int format,<br>
return
VA_STATUS_ERROR_UNSUPPORTED_RT_FORMAT;<br>
}<br>
+ /* The application will clear the TILING flag
when the surface is<br>
+ * intended to be exported as dmabuf. */<br>
+ linear_layout = memory_attibute &&<br>
+ !(memory_attibute->flags &
VA_SURFACE_EXTBUF_DESC_ENABLE_TILING);<br>
+<br>
switch (memory_type) {<br>
case VA_SURFACE_ATTRIB_MEM_TYPE_VA:<br>
+ if (linear_layout) {<br>
+ switch (expected_fourcc) {<br>
+ case VA_FOURCC_RGBA:<br>
+ case VA_FOURCC_RGBX:<br>
+ case VA_FOURCC_BGRA:<br>
+ case VA_FOURCC_BGRX:<br>
+ if (memory_attibute->num_planes
!= 1)<br>
+ return
VA_STATUS_ERROR_INVALID_PARAMETER;<br>
+ break;<br>
+ default:<br>
+ return
VA_STATUS_ERROR_INVALID_PARAMETER;<br>
+ }<br>
+ }<br>
+<br>
break;<br>
case VA_SURFACE_ATTRIB_MEM_TYPE_DRM_PRIME:<br>
if (!memory_attibute)<br>
@@ -587,6 +609,13 @@
vlVaCreateSurfaces2(VADriverContextP ctx, unsigned
int format,<br>
if (expected_fourcc) {<br>
templat.buffer_format =
VaFourccToPipeFormat(expected_fourcc);<br>
templat.interlaced = 0;<br>
+<br>
+ if (linear_layout) {<br>
+ resource_formats =
vl_video_buffer_formats(pscreen,<br>
+
templat.buffer_format);<br>
+ if (!resource_formats)<br>
+ return
VA_STATUS_ERROR_INVALID_PARAMETER;<br>
+ }<br>
} else {<br>
templat.buffer_format =
pscreen->get_video_param<br>
(<br>
@@ -621,7 +650,36 @@
vlVaCreateSurfaces2(VADriverContextP ctx, unsigned
int format,<br>
switch (memory_type) {<br>
case VA_SURFACE_ATTRIB_MEM_TYPE_VA:<br>
- surf->buffer =
drv->pipe->create_video_buffer(drv->pipe,
&templat);<br>
+ if (linear_layout) {<br>
+ struct pipe_resource resource_template;<br>
+ struct pipe_resource
*resources[VL_NUM_COMPONENTS];<br>
+ int depth = 1;<br>
+ int array_size = 1;<br>
+<br>
+ memset(resources, 0,
sizeof(resources));<br>
+<br>
+ assert(resource_formats);<br>
+
vl_video_buffer_template(&resource_template,
&templat,<br>
+
resource_formats[0], depth, array_size,<br>
+
PIPE_USAGE_DEFAULT, 0);<br>
+<br>
+ /* Shared because
VASurfaceAttribExternalBuffers is used. */<br>
+ assert(memory_attibute);<br>
+ resource_template.bind |=
PIPE_BIND_LINEAR | PIPE_BIND_SHARED;<br>
+<br>
+ resources[0] =
pscreen->resource_create(pscreen,<br>
+
&resource_template);<br>
+ if (!resources[0]) {<br>
+ FREE(surf);<br>
+ goto no_res;<br>
+ }<br>
+<br>
+ surf->buffer =
vl_video_buffer_create_ex2(drv->pipe,
&templat,<br>
+
resources);<br>
+ } else {<br>
+ surf->buffer =
drv->pipe->create_video_buffer(drv->pipe,
&templat);<br>
+ }<br>
+<br>
if (!surf->buffer) {<br>
FREE(surf);<br>
goto no_res;<br>
</blockquote>
<br>
</div>
</div>
<div>
<div>
_______________________________________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</div>
</blockquote>
<br>
</div></div></div>
</blockquote></div><br></div></div>