[Mesa-dev] [PATCH 1/2] st/va: ensure linear memory for dmabuf

[Julien Isorce]

On 29 November 2015 at 17:26, Emil Velikov <emil.l.velikov at gmail.com> wrote:

> On 27 November 2015 at 08:57, Julien Isorce <j.isorce at samsung.com> wrote:
> > In order to do zero-copy between two different devices
> > the memory should not be tiled.
> >
> > This is currently no way to set pipe_resource template's flag
> > from pipe_video_buffer template. So disabled_tiling is added.
> >
> > Choosed "disable" prefix so that CALLOC keeps tiling enabled
> > by default.
> >
> > Tested with GStreamer on a laptop that has 2 GPUs:
> > 1- gstvaapidecode:
> >    HW decoding and dmabuf export with nouveau driver on Nvidia GPU.
> > 2- glimagesink:
> >    EGLImage imports dmabuf on Intel GPU.
> >
> > Note that tiling is working if 1 and 2 are done on the same GPU.
> > So it is up to the application to set or not the flag:
> > VA_SURFACE_EXTBUF_DESC_ENABLE_TILING
> >
> > Signed-off-by: Julien Isorce <j.isorce at samsung.com>
> > ---
> >  src/gallium/auxiliary/vl/vl_video_buffer.c | 3 +++
> >  src/gallium/include/pipe/p_video_codec.h   | 1 +
> >  src/gallium/state_trackers/va/surface.c    | 5 +++++
> >  3 files changed, 9 insertions(+)
> >
> > diff --git a/src/gallium/auxiliary/vl/vl_video_buffer.c
> b/src/gallium/auxiliary/vl/vl_video_buffer.c
> > index 6cd2557..62f4aa9 100644
> > --- a/src/gallium/auxiliary/vl/vl_video_buffer.c
> > +++ b/src/gallium/auxiliary/vl/vl_video_buffer.c
> > @@ -253,6 +253,9 @@ vl_video_buffer_template(struct pipe_resource *templ,
> >     templ->bind = PIPE_BIND_SAMPLER_VIEW | PIPE_BIND_RENDER_TARGET;
> >     templ->usage = usage;
> >
> > +   if (tmpl->disable_tiling)
> > +      templ->bind |= PIPE_BIND_LINEAR;
> > +
> >     if (plane > 0) {
> >        if (tmpl->chroma_format == PIPE_VIDEO_CHROMA_FORMAT_420) {
> >           templ->width0 /= 2;
> > diff --git a/src/gallium/include/pipe/p_video_codec.h
> b/src/gallium/include/pipe/p_video_codec.h
> > index 196d00b..dbfffd9 100644
> > --- a/src/gallium/include/pipe/p_video_codec.h
> > +++ b/src/gallium/include/pipe/p_video_codec.h
> > @@ -125,6 +125,7 @@ struct pipe_video_buffer
> >     enum pipe_video_chroma_format chroma_format;
> >     unsigned width;
> >     unsigned height;
> > +   bool disable_tiling;
> >     bool interlaced;
> >
> >     /**
> > diff --git a/src/gallium/state_trackers/va/surface.c
> b/src/gallium/state_trackers/va/surface.c
> > index c052c8f..f7043ad 100644
> > --- a/src/gallium/state_trackers/va/surface.c
> > +++ b/src/gallium/state_trackers/va/surface.c
> > @@ -616,6 +616,11 @@ vlVaCreateSurfaces2(VADriverContextP ctx, unsigned
> int format,
> >
> >        switch (memory_type) {
> >        case VA_SURFACE_ATTRIB_MEM_TYPE_VA:
> > +         /* The application will clear the TILING flag when the surface
> is
> > +          * intended to be exported as dmabuf. */
> > +         templat.disable_tiling = memory_attibute &&
> > +            !(memory_attibute->flags &
> VA_SURFACE_EXTBUF_DESC_ENABLE_TILING);
> The condition seems to be flipped, no ? Currently it's doing
> "disable_tiling = ENABLE_TILING_BIT_SET"
>

Hi Emil, thx for the review. I do not think it is flipped but maybe I
missing something.


>
> Other than that, the idea is ok imho, although I'd appreciate
> Christian and others' feedback.
>
> A few things worth mentioning while looking around for this:
>  - suface_from_external_memory should (must) also know about
> disable_tiling.
>
Ack

>  - missing R in function name ^^ suRface_ ...
>
Ack

>  - sometimes radeon (see r600_video_buffer_create) completely
> overwrites the existing flags, as opposed to just set the linear bit.
> Bug, intentional, worth adding a comment ?
>  - in many cases nouveau won't create a linear surface as it's not
> using the above vl helper (hint, earlier suggesting about
> reworking/cleaning things up a bit, hint)
>

It is actually only useful to set it linear when it hits vl helper. For
nouveau, for NV12, it uses a special layout (
http://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/drivers/nouveau/nvc0/nvc0_miptree.c#n290)
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)

Cheers
Julien


>
> Thanks
> Emil
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151130/75ac604b/attachment-0001.html>