<html>
  <head>
    <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <div class="moz-cite-prefix">Am 08.06.2016 um 15:53 schrieb Julien
      Isorce:<br>
    </div>
    <blockquote
cite="mid:CAHWPjbUMCwE-uECdMiJOtCnC8RB988Z-qk2Bg4gWnQYbyBqUyg@mail.gmail.com"
      type="cite">
      <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 moz-do-not-send="true"
          href="https://patchwork.freedesktop.org/patch/66382/">https://patchwork.freedesktop.org/patch/66382/</a><br>
      </div>
    </blockquote>
    <br>
    Thanks for pointing that out! That's more than 6 month ago and I
    barely remember what I had for lunch yesterday :).<br>
    <br>
    <blockquote
cite="mid:CAHWPjbUMCwE-uECdMiJOtCnC8RB988Z-qk2Bg4gWnQYbyBqUyg@mail.gmail.com"
      type="cite">
      <div dir="ltr">In that v1 should I just replace the specific "bool
        disable_tiling" by a generic bind flag ?<br>
      </div>
    </blockquote>
    <br>
    Yeah, that's what I wanted to say with my comment back then.<br>
    <br>
    Regards,<br>
    Christian.<br>
    <br>
    <blockquote
cite="mid:CAHWPjbUMCwE-uECdMiJOtCnC8RB988Z-qk2Bg4gWnQYbyBqUyg@mail.gmail.com"
      type="cite">
      <div dir="ltr">
        <div class="gmail_extra"><br>
          <div class="gmail_quote">On 8 June 2016 at 14:07, Christian
            König <span dir="ltr"><<a moz-do-not-send="true"
                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
                                moz-do-not-send="true"
                                href="mailto:deathsimple@vodafone.de"
                                target="_blank"><a class="moz-txt-link-abbreviated" href="mailto:deathsimple@vodafone.de">deathsimple@vodafone.de</a></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
                                    moz-do-not-send="true"
                                    href="mailto:j.isorce@samsung.com"
                                    target="_blank"><a class="moz-txt-link-abbreviated" href="mailto:j.isorce@samsung.com">j.isorce@samsung.com</a></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 moz-do-not-send="true"
                                    href="mailto:mesa-dev@lists.freedesktop.org"
                                    target="_blank">mesa-dev@lists.freedesktop.org</a><br>
                                  <a moz-do-not-send="true"
                                    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>
    </blockquote>
    <br>
  </body>
</html>