<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On 29 November 2015 at 17:26, Emil Velikov <span dir="ltr"><<a href="mailto:emil.l.velikov@gmail.com" target="_blank">emil.l.velikov@gmail.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=""><div class="h5">On 27 November 2015 at 08:57, Julien Isorce <<a href="mailto:j.isorce@samsung.com">j.isorce@samsung.com</a>> wrote:<br>
> 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">j.isorce@samsung.com</a>><br>
> ---<br>
>  src/gallium/auxiliary/vl/vl_video_buffer.c | 3 +++<br>
>  src/gallium/include/pipe/p_video_codec.h   | 1 +<br>
>  src/gallium/state_trackers/va/surface.c    | 5 +++++<br>
>  3 files changed, 9 insertions(+)<br>
><br>
> diff --git a/src/gallium/auxiliary/vl/vl_video_buffer.c b/src/gallium/auxiliary/vl/vl_video_buffer.c<br>
> index 6cd2557..62f4aa9 100644<br>
> --- a/src/gallium/auxiliary/vl/vl_video_buffer.c<br>
> +++ b/src/gallium/auxiliary/vl/vl_video_buffer.c<br>
> @@ -253,6 +253,9 @@ vl_video_buffer_template(struct pipe_resource *templ,<br>
>     templ->bind = PIPE_BIND_SAMPLER_VIEW | PIPE_BIND_RENDER_TARGET;<br>
>     templ->usage = usage;<br>
><br>
> +   if (tmpl->disable_tiling)<br>
> +      templ->bind |= PIPE_BIND_LINEAR;<br>
> +<br>
>     if (plane > 0) {<br>
>        if (tmpl->chroma_format == PIPE_VIDEO_CHROMA_FORMAT_420) {<br>
>           templ->width0 /= 2;<br>
> diff --git a/src/gallium/include/pipe/p_video_codec.h b/src/gallium/include/pipe/p_video_codec.h<br>
> index 196d00b..dbfffd9 100644<br>
> --- a/src/gallium/include/pipe/p_video_codec.h<br>
> +++ b/src/gallium/include/pipe/p_video_codec.h<br>
> @@ -125,6 +125,7 @@ struct pipe_video_buffer<br>
>     enum pipe_video_chroma_format chroma_format;<br>
>     unsigned width;<br>
>     unsigned height;<br>
> +   bool disable_tiling;<br>
>     bool interlaced;<br>
><br>
>     /**<br>
> diff --git a/src/gallium/state_trackers/va/surface.c b/src/gallium/state_trackers/va/surface.c<br>
> index c052c8f..f7043ad 100644<br>
> --- a/src/gallium/state_trackers/va/surface.c<br>
> +++ b/src/gallium/state_trackers/va/surface.c<br>
> @@ -616,6 +616,11 @@ vlVaCreateSurfaces2(VADriverContextP ctx, unsigned int format,<br>
><br>
>        switch (memory_type) {<br>
>        case VA_SURFACE_ATTRIB_MEM_TYPE_VA:<br>
> +         /* The application will clear the TILING flag when the surface is<br>
> +          * intended to be exported as dmabuf. */<br>
> +         templat.disable_tiling = memory_attibute &&<br>
> +            !(memory_attibute->flags & VA_SURFACE_EXTBUF_DESC_ENABLE_TILING);<br>
</div></div>The condition seems to be flipped, no ? Currently it's doing<br>
"disable_tiling = ENABLE_TILING_BIT_SET"<br></blockquote><div><br></div><div>Hi Emil, thx for the review. I do not think it is flipped but maybe I missing something.<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>
Other than that, the idea is ok imho, although I'd appreciate<br>
Christian and others' feedback.<br>
<br>
A few things worth mentioning while looking around for this:<br>
 - suface_from_external_memory should (must) also know about disable_tiling.<br></blockquote><div>Ack <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
 - missing R in function name ^^ suRface_ ...<br></blockquote><div>Ack <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
 - sometimes radeon (see r600_video_buffer_create) completely<br>
overwrites the existing flags, as opposed to just set the linear bit.<br>
Bug, intentional, worth adding a comment ?<br>
 - in many cases nouveau won't create a linear surface as it's not<br>
using the above vl helper (hint, earlier suggesting about<br>
reworking/cleaning things up a bit, hint)<br></blockquote><div> </div><div>It is actually only useful to set it linear when it hits vl helper. For nouveau, for NV12, it uses a special layout (<a href="http://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/drivers/nouveau/nvc0/nvc0_miptree.c#n290">http://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/drivers/nouveau/nvc0/nvc0_miptree.c#n290</a>) and also vaAcquireBufferHandle (dmabuf export) is not practicable since it requires the memory to be contiguous. So hence not useful to optionally set it linear (linear flag will be ignored with existing code as you noticed)<br><br></div><div>Cheers<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>
Thanks<br>
<span class=""><font color="#888888">Emil<br>
</font></span><div class=""><div class="h5">_______________________________________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</div></div></blockquote></div><br></div></div>