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

[Julien Isorce]

Hi Christian,

I can try to move the bind flags to pipe_video_buffer.

Thx
Julien

On 29 November 2015 at 19:04, Christian König <deathsimple at vodafone.de>
wrote:

> On 29.11.2015 18:26, Emil Velikov 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"
>>
>> 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.
>>   - missing R in function name ^^ suRface_ ...
>>   - 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)
>>
>
> Yeah, agree deduplicating that code would probably be a good idea.
>
> I would also prefer to have all the bind flags in the pipe_video_buffer as
> well.
>
> Regards,
> Christian.
>
>
>
>> Thanks
>> Emil
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>
> _______________________________________________
> 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/63e13313/attachment.html>