[Mesa-dev] [PATCH 2/3] st/xa: use pipe-loader to get screen

[Rob Clark]

On Mon, Feb 10, 2014 at 7:37 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> Hi Rob
>
> On 08/02/14 22:41, Rob Clark wrote:
>> From: Rob Clark <robclark at freedesktop.org>
>>
>> This lets multiple gallium drivers use XA.
>>
>> Signed-off-by: Rob Clark <robclark at freedesktop.org>
>> ---
>>  configure.ac                                  |  8 ++--
>>  src/gallium/state_trackers/xa/Makefile.am     | 11 ++++-
>>  src/gallium/state_trackers/xa/xa_priv.h       |  1 +
>>  src/gallium/state_trackers/xa/xa_tracker.c    |  6 ++-
>>  src/gallium/state_trackers/xa/xatracker.pc.in |  9 ++++
>>  src/gallium/targets/Makefile.am               |  4 --
>>  src/gallium/targets/xa-vmwgfx/Makefile.am     | 61 ---------------------------
>>  src/gallium/targets/xa-vmwgfx/vmw_target.c    | 26 ------------
>>  src/gallium/targets/xa-vmwgfx/xatracker.pc.in |  9 ----
>>  9 files changed, 29 insertions(+), 106 deletions(-)
>>  create mode 100644 src/gallium/state_trackers/xa/xatracker.pc.in
>>  delete mode 100644 src/gallium/targets/xa-vmwgfx/Makefile.am
>>  delete mode 100644 src/gallium/targets/xa-vmwgfx/vmw_target.c
>>  delete mode 100644 src/gallium/targets/xa-vmwgfx/xatracker.pc.in
>>
>> diff --git a/configure.ac b/configure.ac
>> index 2424dde..6f490fc 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -1270,6 +1270,7 @@ dnl XA configuration
>>  dnl
>>  if test "x$enable_xa" = xyes; then
>>      GALLIUM_STATE_TRACKERS_DIRS="xa $GALLIUM_STATE_TRACKERS_DIRS"
>> +    enable_gallium_loader=yes
>>  fi
>>  AM_CONDITIONAL(HAVE_ST_XA, test "x$enable_xa" = xyes)
>>
>> @@ -1743,7 +1744,7 @@ if test "x$with_gallium_drivers" != x; then
>>          xsvga)
>>              HAVE_GALLIUM_SVGA=yes
>>              GALLIUM_DRIVERS_DIRS="$GALLIUM_DRIVERS_DIRS svga softpipe"
>> -            gallium_check_st "svga/drm" "dri-vmwgfx" "xa-vmwgfx"
>> +            gallium_check_st "svga/drm" "dri-vmwgfx" ""
>>              ;;
>>          xi915)
>>              HAVE_GALLIUM_I915=yes
>> @@ -1959,7 +1960,7 @@ AC_SUBST([XVMC_MAJOR], 1)
>>  AC_SUBST([XVMC_MINOR], 0)
>>
>>  AC_SUBST([XA_MAJOR], 2)
>> -AC_SUBST([XA_MINOR], 1)
>> +AC_SUBST([XA_MINOR], 2)
>>  AC_SUBST([XA_TINY], 0)
>>  AC_SUBST([XA_VERSION], "$XA_MAJOR.$XA_MINOR.$XA_TINY")
>>
>> @@ -2021,6 +2022,7 @@ AC_CONFIG_FILES([Makefile
>>               src/gallium/state_trackers/vdpau/Makefile
>>               src/gallium/state_trackers/vega/Makefile
>>               src/gallium/state_trackers/xa/Makefile
>> +             src/gallium/state_trackers/xa/xatracker.pc
>>               src/gallium/state_trackers/xvmc/Makefile
>>               src/gallium/targets/Makefile
>>               src/gallium/targets/dri-freedreno/Makefile
>> @@ -2043,8 +2045,6 @@ AC_CONFIG_FILES([Makefile
>>               src/gallium/targets/r600/xvmc/Makefile
>>               src/gallium/targets/libgl-xlib/Makefile
>>               src/gallium/targets/vdpau-nouveau/Makefile
>> -             src/gallium/targets/xa-vmwgfx/Makefile
>> -             src/gallium/targets/xa-vmwgfx/xatracker.pc
>>               src/gallium/targets/xvmc-nouveau/Makefile
>>               src/gallium/tests/trivial/Makefile
>>               src/gallium/tests/unit/Makefile
>> diff --git a/src/gallium/state_trackers/xa/Makefile.am b/src/gallium/state_trackers/xa/Makefile.am
>> index 7d0b366..37ec0c4 100644
>> --- a/src/gallium/state_trackers/xa/Makefile.am
>> +++ b/src/gallium/state_trackers/xa/Makefile.am
>> @@ -25,6 +25,8 @@ include $(top_srcdir)/src/gallium/Automake.inc
>>
>>  AM_CFLAGS = \
>>       -Wall -pedantic \
>> +     $(GALLIUM_PIPE_LOADER_DEFINES) \
>> +     -DPIPE_SEARCH_DIR=\"$(libdir)/gallium-pipe\" \
>>       $(GALLIUM_CFLAGS) \
>>       $(VISIBILITY_CFLAGS)
>>
>> @@ -33,12 +35,19 @@ AM_CPPFLAGS = \
>>       -I$(top_srcdir)/src/gallium/winsys \
>>       -I$(top_srcdir)/src/gallium/drivers
>>
>> +pkgconfigdir = $(libdir)/pkgconfig
>> +pkgconfig_DATA = xatracker.pc
>> +
>>  xa_includedir = $(includedir)
>>  xa_include_HEADERS = \
>>       xa_composite.h \
>>       xa_context.h \
>>       xa_tracker.h
>>
>> -noinst_LTLIBRARIES = libxatracker.la
>> +lib_LTLIBRARIES = libxatracker.la
>>
>>  libxatracker_la_SOURCES = $(C_SOURCES)
>> +libxatracker_la_LIBADD = \
>> +     $(GALLIUM_PIPE_LOADER_LIBS) \
>> +     $(top_builddir)/src/gallium/auxiliary/pipe-loader/libpipe_loader.la \
>> +     $(top_builddir)/src/gallium/auxiliary/libgallium.la
> Can you please keep this as is, and add a xatracker target in the
> targets folder. Similar to the way opencl and gbm are handled.

I confess to not completely understanding the separation between
target vs state tracker.. I guess I'll have a closer look at gbm and
opencl.

>> diff --git a/src/gallium/state_trackers/xa/xa_priv.h b/src/gallium/state_trackers/xa/xa_priv.h
>> index ee182e7..b99c214 100644
>> --- a/src/gallium/state_trackers/xa/xa_priv.h
>> +++ b/src/gallium/state_trackers/xa/xa_priv.h
>> @@ -74,6 +74,7 @@ struct xa_tracker {
>>      unsigned int format_map[XA_LAST_SURFACE_TYPE][2];
>>      int d_depth_bits_last;
>>      int ds_depth_bits_last;
>> +    struct pipe_loader_device *dev;
>>      struct pipe_screen *screen;
>>      struct xa_context *default_ctx;
>>  };
>> diff --git a/src/gallium/state_trackers/xa/xa_tracker.c b/src/gallium/state_trackers/xa/xa_tracker.c
>> index cda6501..840c361 100644
>> --- a/src/gallium/state_trackers/xa/xa_tracker.c
>> +++ b/src/gallium/state_trackers/xa/xa_tracker.c
>> @@ -30,6 +30,7 @@
>>  #include "xa_priv.h"
>>  #include "pipe/p_state.h"
>>  #include "pipe/p_format.h"
>> +#include "pipe-loader/pipe_loader.h"
>>  #include "state_tracker/drm_driver.h"
>>  #include "util/u_inlines.h"
>>
>> @@ -143,7 +144,8 @@ xa_tracker_create(int drm_fd)
>>      if (!xa)
>>       return NULL;
>>
>> -    xa->screen = driver_descriptor.create_screen(drm_fd);
>> +    if (pipe_loader_drm_probe_fd(&xa->dev, drm_fd))
>> +     xa->screen = pipe_loader_create_screen(xa->dev, PIPE_SEARCH_DIR);
>>      if (!xa->screen)
>>       goto out_no_screen;
>>
> Missing pipe_loader_release() in the error path, after
> xa->screen->destroy(xa->screen);

oh, right, I'll fix that

>> @@ -200,6 +202,7 @@ xa_tracker_destroy(struct xa_tracker *xa)
>>      free(xa->supported_formats);
>>      xa_context_destroy(xa->default_ctx);
>>      xa->screen->destroy(xa->screen);
>> +    pipe_loader_release(&xa->dev, 1);
>>      free(xa);
>>  }
>>
>> @@ -363,6 +366,7 @@ xa_surface_from_handle(struct xa_tracker *xa,
>>  {
>>      struct winsys_handle whandle;
>>      memset(&whandle, 0, sizeof(whandle));
>> +    whandle.type = DRM_API_HANDLE_TYPE_SHARED;
> Nice find. Worth separating from the rest and CC stable ?

oh, I suppose that should have been a separate patch.  But
DRM_API_HANDLE_TYPE_SHARED is zero, so it is more of a
stylistic/preventative fix than an actual functional change.  So not
sure how important it is for stable.

BR,
-R

> Many thanks for tackling this.
>
> -Emil