Am 08.06.2016 um 14:20 schrieb Julien Isorce:


On 8 June 2016 at 09:16, Christian König <deathsim...@vodafone.de <mailto:deathsim...@vodafone.de>> wrote:

    Am 02.06.2016 um 16:00 schrieb Julien Isorce:

        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.iso...@samsung.com
        <mailto:j.iso...@samsung.com>>



Thx for the review.

    NAK, it won't be possible to use the resulting video buffer with
    hardware decoding on AMD hardware.


But I restrict to these format:

+            case VA_FOURCC_RGBA:
+            case VA_FOURCC_RGBX:
+            case VA_FOURCC_BGRA:
+            case VA_FOURCC_BGRX:

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.

Yeah, ok that should work for now but is clearly not a good idea.


    Please add a bind flag to struct pipe_video_buffer instead so that
    we can specify if linear layout is requested or not.


Do you mean that resource = pscreen->resource_create(pscreen, &tmpl) does not honor the bind flag of the template. Maybe I can just checked if it was effective after that call, i.e. checking presence of PIPE_BIND_LINEAR in
resources[0]->bind.

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.


    This way the hardware driver can still reject the request if this
    would result in a surface which can't be decoded to.


For now it requires vpp since I explicitly restricted linear layout request to the rgbs format above. The reason behind is 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
NV12 has 2 planes.

In the spec the problem comes from the fact that a VAImage has only one VABufferID. It would require to define a new VABufferType which represents an array of VAImageBufferType, something like that.

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.



To go back to "add a bind flag to struct pipe_video_buffer instead ", the alternative is to bring back the first version of the patch but according to the first review, it was duplication of bind flag between pipe_video_buffer and pipe_resource so it would require quite of big of refactoring unless I miss understood the comment.

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.

Also in vdpau, the function vlVdpOutputSurfaceCreate is using PIPE_BIND_LINEAR flag and resource_create call,
like in the v2 of my patch.

Yeah, but that isn't a video buffer you try to create here. VA-API unfortunately doesn't properly distinct between those two things.

Regards,
Christian.


Not sure if I replied to your concerns, let me know.

Thx
Julien


    Regards,
    Christian.


        ---
          src/gallium/state_trackers/va/surface.c | 60
        ++++++++++++++++++++++++++++++++-
          1 file changed, 59 insertions(+), 1 deletion(-)

        diff --git a/src/gallium/state_trackers/va/surface.c
        b/src/gallium/state_trackers/va/surface.c
        index 8a6a397..b04ced4 100644
        --- a/src/gallium/state_trackers/va/surface.c
        +++ b/src/gallium/state_trackers/va/surface.c
        @@ -507,7 +507,9 @@ vlVaCreateSurfaces2(VADriverContextP ctx,
        unsigned int format,
             int i;
             int memory_type;
             int expected_fourcc;
        +   int linear_layout;
             VAStatus vaStatus;
        +   const enum pipe_format *resource_formats;
               if (!ctx)
                return VA_STATUS_ERROR_INVALID_CONTEXT;
        @@ -529,6 +531,7 @@ vlVaCreateSurfaces2(VADriverContextP ctx,
        unsigned int format,
             memory_attibute = NULL;
             memory_type = VA_SURFACE_ATTRIB_MEM_TYPE_VA;
             expected_fourcc = 0;
        +   resource_formats = NULL;
               for (i = 0; i < num_attribs && attrib_list; i++) {
                if ((attrib_list[i].type == VASurfaceAttribPixelFormat) &&
        @@ -569,8 +572,27 @@ vlVaCreateSurfaces2(VADriverContextP ctx,
        unsigned int format,
                return VA_STATUS_ERROR_UNSUPPORTED_RT_FORMAT;
             }
          +   /* The application will clear the TILING flag when the
        surface is
        +    * intended to be exported as dmabuf. */
        +   linear_layout = memory_attibute &&
        +      !(memory_attibute->flags &
        VA_SURFACE_EXTBUF_DESC_ENABLE_TILING);
        +
             switch (memory_type) {
             case VA_SURFACE_ATTRIB_MEM_TYPE_VA:
        +      if (linear_layout) {
        +         switch (expected_fourcc) {
        +            case VA_FOURCC_RGBA:
        +            case VA_FOURCC_RGBX:
        +            case VA_FOURCC_BGRA:
        +            case VA_FOURCC_BGRX:
        +               if (memory_attibute->num_planes != 1)
        +                  return VA_STATUS_ERROR_INVALID_PARAMETER;
        +               break;
        +            default:
        +               return VA_STATUS_ERROR_INVALID_PARAMETER;
        +         }
        +      }
        +
                break;
             case VA_SURFACE_ATTRIB_MEM_TYPE_DRM_PRIME:
                if (!memory_attibute)
        @@ -587,6 +609,13 @@ vlVaCreateSurfaces2(VADriverContextP ctx,
        unsigned int format,
             if (expected_fourcc) {
                templat.buffer_format =
        VaFourccToPipeFormat(expected_fourcc);
                templat.interlaced = 0;
        +
        +      if (linear_layout) {
        +         resource_formats = vl_video_buffer_formats(pscreen,
        + templat.buffer_format);
        +         if (!resource_formats)
        +            return VA_STATUS_ERROR_INVALID_PARAMETER;
        +      }
             } else {
                templat.buffer_format = pscreen->get_video_param
                      (
        @@ -621,7 +650,36 @@ vlVaCreateSurfaces2(VADriverContextP ctx,
        unsigned int format,
                  switch (memory_type) {
                case VA_SURFACE_ATTRIB_MEM_TYPE_VA:
        -         surf->buffer =
        drv->pipe->create_video_buffer(drv->pipe, &templat);
        +         if (linear_layout) {
        +            struct pipe_resource resource_template;
        +            struct pipe_resource *resources[VL_NUM_COMPONENTS];
        +            int depth = 1;
        +            int array_size = 1;
        +
        +            memset(resources, 0, sizeof(resources));
        +
        +            assert(resource_formats);
        + vl_video_buffer_template(&resource_template, &templat,
        +  resource_formats[0], depth, array_size,
        +  PIPE_USAGE_DEFAULT, 0);
        +
        +            /* Shared because VASurfaceAttribExternalBuffers
        is used. */
        +            assert(memory_attibute);
        +            resource_template.bind |= PIPE_BIND_LINEAR |
        PIPE_BIND_SHARED;
        +
        +            resources[0] = pscreen->resource_create(pscreen,
        + &resource_template);
        +            if (!resources[0]) {
        +                FREE(surf);
        +                goto no_res;
        +            }
        +
        +            surf->buffer =
        vl_video_buffer_create_ex2(drv->pipe, &templat,
        +   resources);
        +         } else {
        +            surf->buffer =
        drv->pipe->create_video_buffer(drv->pipe, &templat);
        +         }
        +
                   if (!surf->buffer) {
                      FREE(surf);
                      goto no_res;


    _______________________________________________
    mesa-dev mailing list
    mesa-dev@lists.freedesktop.org <mailto:mesa-dev@lists.freedesktop.org>
    https://lists.freedesktop.org/mailman/listinfo/mesa-dev



_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to