[PATCH weston v2 1/4] simple-dmabuf-drm: allow multiple backends

[Pekka Paalanen]

On Thu, 15 Mar 2018 17:33:18 +0100
Guido Günther <agx at sigxcpu.org> wrote:

> This allows to enable freedreno and intel backends at the same time
> building the prerequisites for adding further ones.
> ---
>  Makefile.am                 |  5 ++++-
>  clients/simple-dmabuf-drm.c | 53 +++++++++++++++++++++++++--------------------
>  configure.ac                | 24 +++++++++++---------
>  3 files changed, 47 insertions(+), 35 deletions(-)

Hi,

much better!

> @@ -278,7 +281,8 @@ drm_device_destroy(struct buffer *buf)
>  {
>  #ifdef HAVE_LIBDRM_INTEL
>  	drm_intel_bufmgr_destroy(buf->bufmgr);
> -#elif HAVE_LIBDRM_FREEDRENO
> +#endif
> +#ifdef HAVE_LIBDRM_FREEDRENO
>  	fd_device_del(buf->fd_dev);
>  #endif

This introduces a commit in the history, where both del functions can
be called in the same run. Usually I'd ask to reorder patches, so that
such a broken intermediate state does not exist, but this seems simple
enough to let land.

>  
> @@ -308,7 +312,8 @@ drm_device_init(struct buffer *buf)
>  		dev->map_bo = intel_map_bo;
>  		dev->unmap_bo = intel_unmap_bo;
>  	}
> -#elif HAVE_LIBDRM_FREEDRENO
> +#endif
> +#ifdef HAVE_LIBDRM_FREEDRENO
>  	else if (!strcmp(dev->name, "msm")) {
>  		dev->alloc_bo = fd_alloc_bo;
>  		dev->free_bo = fd_free_bo;
> diff --git a/configure.ac b/configure.ac
> index 0b326ccc..dc2fbe93 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -384,19 +384,23 @@ AC_ARG_ENABLE(simple-dmabuf-drm-client,
>                               [do not build the simple dmabuf drm client]),,
>                enable_simple_dmabuf_drm_client="auto")
>  if ! test "x$enable_simple_dmabuf_drm_client" = "xno"; then
> -  PKG_CHECK_MODULES(SIMPLE_DMABUF_DRM_CLIENT, [wayland-client libdrm egl],
> -    [PKG_CHECK_MODULES(LIBDRM_PLATFORM, [libdrm_freedreno],
> -      AC_DEFINE([HAVE_LIBDRM_FREEDRENO], [1], [Build freedreno dmabuf client]) have_simple_dmabuf_drm_client=freedreno,
> -    [PKG_CHECK_MODULES(LIBDRM_PLATFORM, [libdrm_intel],
> -      AC_DEFINE([HAVE_LIBDRM_INTEL], [1], [Build intel dmabuf client]) have_simple_dmabuf_drm_client=intel,
> -    have_simple_dmabuf_drm_client=unsupported)])],
> -  have_simple_dmabuf_drm_client=unsupported)
> -
> -  if test "x$have_simple_dmabuf_drm_client" = "xunsupported" -a "x$enable_simple_dmabuf_drm_client" = "xyes"; then
> +  PKG_CHECK_MODULES(SIMPLE_DMABUF_DRM_CLIENT, [wayland-client libdrm egl], [have_simple_dmabuf_libs=yes],
> +		    [have_simple_dmabuf_libs=no])
> +
> +  PKG_CHECK_MODULES(LIBDRM_PLATFORM_FREEDRENO, [libdrm_freedreno],
> +      AC_DEFINE([HAVE_LIBDRM_FREEDRENO], [1], [Build freedreno dmabuf client]) have_simple_dmabuf_drm_client=yes,
> +      [true])
> +  PKG_CHECK_MODULES(LIBDRM_PLATFORM_INTEL, [libdrm_intel],
> +      AC_DEFINE([HAVE_LIBDRM_INTEL], [1], [Build intel dmabuf client]) have_simple_dmabuf_drm_client=yes,
> +      [true])
> +
> +  if test "x$have_simple_dmabuf_drm_client" != "xyes" -o \
> +	  "x$have_simple_dmabuf_libs" = "xno" && \
> +	  "x$enable_simple_dmabuf_drm_client" = "xyes"; then

This is missing a call to 'test', isn't it?

>      AC_MSG_ERROR([DRM dmabuf client explicitly enabled, but libdrm_intel or libdrm_freedreno not found])
>    fi
>  
> -  if test "x$have_simple_dmabuf_drm_client" = "xfreedreno" -o "x$have_simple_dmabuf_drm_client" = "xintel"; then
> +  if test "x$have_simple_dmabuf_drm_client" = "xyes" -a "x$have_simple_dmabuf_libs"; then

The latter expression is always true I think... forgot to compare to
"xyes".

>      enable_simple_dmabuf_drm_client="yes"
>    fi
>  fi

I fixed those two issues before pushing. Thanks for correcting the
freedreno style. Please remember to add your Signed-off-by in the
future.

I still have a question about patch 4, but patches 1-3 are pushed:
   824e4995..4fc3a679  master -> master


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180316/a6bd1523/attachment.sig>