[Mesa-dev] [PATCH v3 3/9] gallium/auxiliary: implement sw_probe_wrapped

[Emil Velikov]

Hi David,

Just a few nitpicks which I've missed the previously

On 02/11/14 18:31, David Heidelberg wrote:
> Implement pipe_loader_sw_probe_wrapped which allows to use the wrapped
> software renderer backend when using the pipe loader.
> 
> Signed-off-by: David Heidelberg <david at ixit.cz>
> ---
>  src/gallium/auxiliary/pipe-loader/pipe_loader.h    | 11 +++++++++++
>  src/gallium/auxiliary/pipe-loader/pipe_loader_sw.c | 22
> ++++++++++++++++++++++
>  src/gallium/targets/gbm/Makefile.am                |  1 +
>  src/gallium/targets/opencl/Makefile.am             |  1 +
>  src/gallium/targets/xa/Makefile.am                 |  1 +
>  src/gallium/tests/trivial/Makefile.am              |  1 +
>  6 files changed, 37 insertions(+)
> 
> diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader.h
> b/src/gallium/auxiliary/pipe-loader/pipe_loader.h
> index 6127a6a..9f43f17 100644
> --- a/src/gallium/auxiliary/pipe-loader/pipe_loader.h
> +++ b/src/gallium/auxiliary/pipe-loader/pipe_loader.h
> @@ -166,6 +166,17 @@ pipe_loader_sw_probe_null(struct pipe_loader_device
> **devs);
>  int
>  pipe_loader_sw_probe(struct pipe_loader_device **devs, int ndev);
>  +/**
> + * Get a software device wrapped atop another device.
> + *
> + * This function is platform-specific.
> + *
> + * \sa pipe_loader_probe
> + */
> +boolean
> +pipe_loader_sw_probe_wrapped(struct pipe_loader_device **dev,
> +                             struct pipe_screen *screen);
> +
>  #ifdef HAVE_PIPE_LOADER_DRM
>   /**
> diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader_sw.c
> b/src/gallium/auxiliary/pipe-loader/pipe_loader_sw.c
> index b1b1ca6..b152f60 100644
> --- a/src/gallium/auxiliary/pipe-loader/pipe_loader_sw.c
> +++ b/src/gallium/auxiliary/pipe-loader/pipe_loader_sw.c
> @@ -29,8 +29,11 @@
>   #include "util/u_memory.h"
>  #include "util/u_dl.h"
> +#ifdef HAVE_PIPE_LOADER_DRI
>  #include "sw/dri/dri_sw_winsys.h"
> +#endif
Does this bring any benefit ? Afaics the header does not pull any
dependencies - drm, Xlib, etc so it should be safe as is.

>  #include "sw/null/null_sw_winsys.h"
> +#include "sw/wrapper/wrapper_sw_winsys.h"
>  #ifdef HAVE_PIPE_LOADER_XLIB
>  /* Explicitly wrap the header to ease build without X11 headers */
>  #include "sw/xlib/xlib_sw_winsys.h"
> @@ -140,6 +143,25 @@ pipe_loader_sw_probe(struct pipe_loader_device
> **devs, int ndev)
>     return i;
>  }
>  +boolean
> +pipe_loader_sw_probe_wrapped(struct pipe_loader_device **dev,
> +                             struct pipe_screen *screen)
> +{
> +   struct pipe_loader_sw_device *sdev =
> CALLOC_STRUCT(pipe_loader_sw_device);
> +
Can we check for CALLOC_STRUCT failure ?

> +   sdev->base.type = PIPE_LOADER_DEVICE_SOFTWARE;
> +   sdev->base.driver_name = "swrast";
> +   sdev->base.ops = &pipe_loader_sw_ops;
> +   sdev->ws = wrapper_sw_winsys_wrap_pipe_screen(screen);
> +
> +   if (!sdev->ws) {
> +      FREE(sdev);
> +      return FALSE;
> +   }
> +   *dev = &sdev->base;
> +   return TRUE;
> +}
> +
>  static void
>  pipe_loader_sw_release(struct pipe_loader_device **dev)
>  {
> diff --git a/src/gallium/targets/gbm/Makefile.am
> b/src/gallium/targets/gbm/Makefile.am
> index 2c9b425..679c994 100644
> --- a/src/gallium/targets/gbm/Makefile.am
> +++ b/src/gallium/targets/gbm/Makefile.am
> @@ -34,6 +34,7 @@ gbm_gallium_drm_la_SOURCES =
>  gbm_gallium_drm_la_LIBADD = \
>      $(top_builddir)/src/gallium/state_trackers/gbm/libgbm.la \
>      $(top_builddir)/src/gallium/auxiliary/libgallium.la \
> +    $(top_builddir)/src/gallium/winsys/sw/wrapper/libwsw.la \
You can avoid adding this for each target by add it to
GALLIUM_PIPE_LOADER_WINSYS_LIBS. Something like the following would do
the job.


--- a/src/gallium/Automake.inc
+++ b/src/gallium/Automake.inc

GALLIUM_PIPE_LOADER_WINSYS_LIBS = \
+      $(top_builddir)/src/gallium/winsys/sw/wrapper/libwsw.la \
       $(top_builddir)/src/gallium/winsys/sw/null/libws_null.la


Cheers,
Emil