[Mesa-dev] [PATCH 02/14] vl/dri3: implement dri3 screen create and destroy

Leo Liu leo.liu at amd.com
Thu May 12 14:01:58 UTC 2016



On 05/12/2016 09:47 AM, Emil Velikov wrote:
> Hi Leo,
>
> On 11 May 2016 at 22:14, Leo Liu <leo.liu at amd.com> wrote:
>> On 05/11/2016 04:20 PM, Axel Davy wrote:
>>> On 11/05/2016 17:06, Leo Liu wrote:
>>>> Screen created with device fd returned from X server,
>>>> also will bail out to DRI2 with certain conditions.
>>>>
>>>> Signed-off-by: Leo Liu <leo.liu at amd.com>
>>>> ---
>>>>    configure.ac                              |  7 ++-
>>>>    src/gallium/auxiliary/vl/vl_winsys_dri3.c | 88
>>>> ++++++++++++++++++++++++++++++-
>>>>    2 files changed, 93 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/configure.ac b/configure.ac
>>>> index 023110e..8c3960a 100644
>>>> --- a/configure.ac
>>>> +++ b/configure.ac
>>>> @@ -1779,7 +1779,12 @@ if test "x$enable_xvmc" = xyes -o \
>>>>            "x$enable_vdpau" = xyes -o \
>>>>            "x$enable_omx" = xyes -o \
>>>>            "x$enable_va" = xyes; then
>>>> -    PKG_CHECK_MODULES([VL], [x11-xcb xcb xcb-dri2 >= $XCBDRI2_REQUIRED])
>>>> +    if test x"$enable_dri3" = xyes; then
>>>> +        PKG_CHECK_MODULES([VL], [xcb-dri3 xcb-present xcb-sync xshmfence
>>>>> = $XSHMFENCE_REQUIRED
>>>> +                                 x11-xcb xcb xcb-dri2 >=
>>>> $XCBDRI2_REQUIRED])
> We don't need xcb-dri2 in the above do we ?

Yes I think so. That's for all vl, includes building vl_winsys_dri.c.

>
>>>> +    else
>>>> +        PKG_CHECK_MODULES([VL], [x11-xcb xcb xcb-dri2 >=
>>>> $XCBDRI2_REQUIRED])
>>>> +    fi
>>>>        need_gallium_vl_winsys=yes
>>>>    fi
>>>>    AM_CONDITIONAL(NEED_GALLIUM_VL_WINSYS, test "x$need_gallium_vl_winsys"
>>>> = xyes)
>>>> diff --git a/src/gallium/auxiliary/vl/vl_winsys_dri3.c
>>>> b/src/gallium/auxiliary/vl/vl_winsys_dri3.c
>>>> index 2c3d3ae..c018379 100644
>>>> --- a/src/gallium/auxiliary/vl/vl_winsys_dri3.c
>>>> +++ b/src/gallium/auxiliary/vl/vl_winsys_dri3.c
>>>> @@ -25,7 +25,16 @@
>>>>     *
>>>>
>>>> **************************************************************************/
>>>>    +#include <fcntl.h>
>>>> +
>>>> +#include <X11/Xlib-xcb.h>
>>>> +#include <xcb/dri3.h>
>>>> +#include <xcb/present.h>
>>>> +
>>>> +#include "loader.h"
>>>> +
>>>>    #include "pipe/p_screen.h"
>>>> +#include "pipe-loader/pipe_loader.h"
>>>>      #include "util/u_memory.h"
>>>>    #include "vl/vl_winsys.h"
>>>> @@ -33,6 +42,8 @@
>>>>    struct vl_dri3_screen
>>>>    {
>>>>       struct vl_screen base;
>>>> +   xcb_connection_t *conn;
>>>> +   xcb_drawable_t drawable;
>>>>    };
>>>>      static void
>>>> @@ -82,7 +93,14 @@ vl_dri3_screen_get_private(struct vl_screen *vscreen)
>>>>    static void
>>>>    vl_dri3_screen_destroy(struct vl_screen *vscreen)
>>>>    {
>>>> -   /* TODO */
>>>> +   struct vl_dri3_screen *scrn = (struct vl_dri3_screen *)vscreen;
>>>> +
>>>> +   assert(vscreen);
>>>> +
>>>> +   scrn->base.pscreen->destroy(scrn->base.pscreen);
>>>> +   pipe_loader_release(&scrn->base.dev, 1);
>>>> +   FREE(scrn);
>>>> +
>>>>       return;
>>>>    }
>>>>    @@ -90,6 +108,13 @@ struct vl_screen *
>>>>    vl_dri3_screen_create(Display *display, int screen)
>>>>    {
>>>>       struct vl_dri3_screen *scrn;
>>>> +   const xcb_query_extension_reply_t *extension;
>>>> +   xcb_dri3_open_cookie_t open_cookie;
>>>> +   xcb_dri3_open_reply_t *open_reply;
>>>> +   xcb_get_geometry_cookie_t geom_cookie;
>>>> +   xcb_get_geometry_reply_t *geom_reply;
>>>> +   int is_different_gpu;
>>>> +   int fd;
>>>>         assert(display);
>>>>    @@ -97,6 +122,58 @@ vl_dri3_screen_create(Display *display, int screen)
>>>>       if (!scrn)
>>>>          return NULL;
>>>>    +   scrn->conn = XGetXCBConnection(display);
>>>> +   if (!scrn->conn)
>>>> +      goto free_screen;
>>>> +
>>>> +   xcb_prefetch_extension_data(scrn->conn , &xcb_dri3_id);
>>>> +   xcb_prefetch_extension_data(scrn->conn, &xcb_present_id);
>>>> +   extension = xcb_get_extension_data(scrn->conn, &xcb_dri3_id);
>>>> +   if (!(extension && extension->present))
>>>> +      goto free_screen;
>>>> +   extension = xcb_get_extension_data(scrn->conn, &xcb_present_id);
>>>> +   if (!(extension && extension->present))
>>>> +      goto free_screen;
>>>> +
>>>> +   open_cookie = xcb_dri3_open(scrn->conn, RootWindow(display, screen),
>>>> None);
>>>> +   open_reply = xcb_dri3_open_reply(scrn->conn, open_cookie, NULL);
>>>> +   if (!open_reply)
>>>> +      goto free_screen;
>>>> +   if (open_reply->nfd != 1) {
>>>> +      free(open_reply);
>>>> +      goto free_screen;
>>>> +   }
>>>> +
>>>> +   fd = xcb_dri3_open_reply_fds(scrn->conn, open_reply)[0];
>>>> +   if (fd < 0) {
>>>> +      free(open_reply);
>>>> +      goto free_screen;
>>>> +   }
>>>> +   fcntl(fd, F_SETFD, FD_CLOEXEC);
>>>> +   free(open_reply);
>>>> +
>>>> +   fd = loader_get_user_preferred_fd(fd, &is_different_gpu);
>>>> +   /* TODO support different GPU */
>>>> +   if (is_different_gpu)
>>>> +      goto free_screen;
>>>> +
>>>> +   geom_cookie = xcb_get_geometry(scrn->conn, RootWindow(display,
>>>> screen));
>>>> +   geom_reply = xcb_get_geometry_reply(scrn->conn, geom_cookie, NULL);
>>>> +   if (!geom_reply)
>>>> +      goto free_screen;
>>>> +   /* TODO support depth other than 24 */
>>>> +   if (geom_reply->depth != 24) {
>>>> +      free(geom_reply);
>>>> +      goto free_screen;
>>>> +   }
>>>> +   free(geom_reply);
>>>> +
>>>> +   if (pipe_loader_drm_probe_fd(&scrn->base.dev, fd))
>>>> +      scrn->base.pscreen = pipe_loader_create_screen(scrn->base.dev);
>>>> +
>>>> +   if (!scrn->base.pscreen)
>>>> +      goto release_pipe;
>>>> +
>>>>       scrn->base.destroy = vl_dri3_screen_destroy;
>>>>       scrn->base.texture_from_drawable =
>>>> vl_dri3_screen_texture_from_drawable;
>>>>       scrn->base.get_dirty_area = vl_dri3_screen_get_dirty_area;
>>>> @@ -106,4 +183,13 @@ vl_dri3_screen_create(Display *display, int screen)
>>>>       scrn->base.pscreen->flush_frontbuffer = vl_dri3_flush_frontbuffer;
>>>>         return &scrn->base;
>>>> +
>>>> +release_pipe:
>>>> +   if (scrn->base.dev)
>>>> +      pipe_loader_release(&scrn->base.dev, 1);
>>>> +   fd = -1;
>>>> +   close(fd);
>>>
>>> I assume mistake there ... or is close(-1) supposed to do something
>>> specific ?
>>
>> Good catch. it's wrong.
>>
>> pipe_loader_release do the close for fd.
>> Here should be:
>> if (scrn->base.dev)
>>     pipe_loader_release(&scrn->base.dev, 1);
>> else
>>     close(fd);
>>
>> will fix in v2.
>>
> Seems like I made this silly mistake in vl_winsys_dri.c where this was
> copied from. Leo might sending a patch for that one as well please ?
Sure. patch for that will follow up shortly.
> Also please squash this patch and 01/14.
patch 1 is to set infrastructure, and patch 2 and beyond is for 
implementation.

Thanks,
Leo

> Thanks

> Emil



More information about the mesa-dev mailing list