[Mesa-dev] [PATCH 1/2] Add a dumb drm/kms winsys for software rendering

Mon Apr 28 14:52:11 PDT 2014

On 12/04/14 01:25, Giovanni Campagna wrote:
> From: Giovanni Campagna <gcampagna at src.gnome.org>
> 
> Add a new winsys and target that can be used with a dri2 state tracker and
> loader instead of drisw. This allows to use gbm as a dri2/image loader
> and avoid the extra copy from the backbuffer to the shadow frontbuffer.
Hi Giovanni,

A few misc comments below. Feel free to disregard the last one.

> ---
>  configure.ac                                      |   4 +-
>  src/gallium/targets/Makefile.am                   |   2 +-
>  src/gallium/targets/dri-swrast2/Makefile.am       |  72 +++++
>  src/gallium/targets/dri-swrast2/swrast2_drm_api.c |  65 +++++
>  src/gallium/winsys/Makefile.am                    |   2 +-
>  src/gallium/winsys/sw/kms-dri/Makefile.am         |  33 +++
>  src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c | 310 ++++++++++++++++++++++
>  src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.h |  37 +++
>  8 files changed, 522 insertions(+), 3 deletions(-)
>  create mode 100644 src/gallium/targets/dri-swrast2/Makefile.am
>  create mode 100644 src/gallium/targets/dri-swrast2/swrast2_drm_api.c
>  create mode 100644 src/gallium/winsys/sw/kms-dri/Makefile.am
>  create mode 100644 src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c
>  create mode 100644 src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.h
> 
> diff --git a/configure.ac b/configure.ac
> index c71fa26..7179abd 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1857,7 +1857,7 @@ if test -n "$with_gallium_drivers"; then
>              fi
>  
>              if test "x$enable_dri" = xyes; then
> -                GALLIUM_TARGET_DIRS="$GALLIUM_TARGET_DIRS dri-swrast"
> +                GALLIUM_TARGET_DIRS="$GALLIUM_TARGET_DIRS dri-swrast dri-swrast2"
Silly nitpick: Can you rename dri-swrast2 to something that is slightly more
explanatory ? This way distributions people will not need to scratch their
heads and wonder what on earth this is.

A oneliner in the release notes would be great as well.

>              fi
>              ;;
>          *)
> @@ -2073,6 +2073,7 @@ AC_CONFIG_FILES([Makefile
>  		src/gallium/targets/dri-ilo/Makefile
>  		src/gallium/targets/dri-nouveau/Makefile
>  		src/gallium/targets/dri-swrast/Makefile
> +		src/gallium/targets/dri-swrast2/Makefile
>  		src/gallium/targets/dri-vmwgfx/Makefile
>  		src/gallium/targets/egl-static/Makefile
>  		src/gallium/targets/gbm/Makefile
> @@ -2103,6 +2104,7 @@ AC_CONFIG_FILES([Makefile
>  		src/gallium/winsys/nouveau/drm/Makefile
>  		src/gallium/winsys/radeon/drm/Makefile
>  		src/gallium/winsys/svga/drm/Makefile
> +		src/gallium/winsys/sw/kms-dri/Makefile
>  		src/gallium/winsys/sw/dri/Makefile
>  		src/gallium/winsys/sw/fbdev/Makefile
>  		src/gallium/winsys/sw/null/Makefile
> diff --git a/src/gallium/targets/Makefile.am b/src/gallium/targets/Makefile.am
> index 871b31d..475962b 100644
> --- a/src/gallium/targets/Makefile.am
> +++ b/src/gallium/targets/Makefile.am
> @@ -120,7 +120,7 @@ endif
>  
>  if HAVE_GALLIUM_SOFTPIPE
>  if HAVE_DRI
> -SUBDIRS += dri-swrast
> +SUBDIRS += dri-swrast dri-swrast2
>  endif
>  endif
>  
> diff --git a/src/gallium/targets/dri-swrast2/Makefile.am b/src/gallium/targets/dri-swrast2/Makefile.am
> new file mode 100644
> index 0000000..d22ec82
> --- /dev/null
> +++ b/src/gallium/targets/dri-swrast2/Makefile.am
> @@ -0,0 +1,72 @@

I have pushed some changes to all gallium targets since you've done this work.
Please grab a fresh copy of dri-swrast/Makefile.am as a reference point and
update his one.

> +# Copyright © 2012 Intel Corporation
> +#
> +# Permission is hereby granted, free of charge, to any person obtaining a
> +# copy of this software and associated documentation files (the "Software"),
> +# to deal in the Software without restriction, including without limitation
> +# the rights to use, copy, modify, merge, publish, distribute, sublicense,
> +# and/or sell copies of the Software, and to permit persons to whom the
> +# Software is furnished to do so, subject to the following conditions:
> +#
> +# The above copyright notice and this permission notice (including the next
> +# paragraph) shall be included in all copies or substantial portions of the
> +# Software.
> +#
> +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> +# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> +# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> +# NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> +# HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> +# WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> +# DEALINGS IN THE SOFTWARE.
> +
> +include $(top_srcdir)/src/gallium/Automake.inc
> +
> +AM_CFLAGS = \
> +	$(GALLIUM_CFLAGS) \
> +	$(PTHREAD_CFLAGS) \
> +	$(LIBDRM_CFLAGS)
> +AM_CPPFLAGS = \
> +	-I$(top_srcdir)/src/gallium/winsys/sw/kms-dri \
> +	-I$(top_srcdir)/src/gallium/drivers \
> +	-I$(top_srcdir)/src/gallium/winsys \
> +	-I$(top_srcdir)/src/mesa \
> +	-I$(top_srcdir)/src/mapi \
> +	-DGALLIUM_RBUG \
> +	-DGALLIUM_TRACE \
> +	-DGALLIUM_SOFTPIPE
> +
> +dridir = $(DRI_DRIVER_INSTALL_DIR)
> +dri_LTLIBRARIES = swrast_dri2_dri.la
> +
> +swrast_dri2_dri_la_SOURCES = \
> +	swrast2_drm_api.c \
> +	$(top_srcdir)/src/mesa/drivers/dri/common/utils.c \
> +	$(top_srcdir)/src/mesa/drivers/dri/common/dri_util.c \
> +	$(top_srcdir)/src/mesa/drivers/dri/common/xmlconfig.c
> +
> +swrast_dri2_dri_la_LDFLAGS = -module -avoid-version -shared -no-undefined
> +
> +swrast_dri2_dri_la_LIBADD = \
> +	$(top_builddir)/src/mesa/libmesagallium.la \
> +	$(top_builddir)/src/gallium/auxiliary/libgallium.la \
> +	$(top_builddir)/src/gallium/state_trackers/dri/drm/libdridrm.la \
> +	$(top_builddir)/src/gallium/winsys/sw/kms-dri/libswkmsdri.la \
> +	$(top_builddir)/src/gallium/drivers/softpipe/libsoftpipe.la \
> +	$(top_builddir)/src/gallium/drivers/trace/libtrace.la \
> +	$(top_builddir)/src/gallium/drivers/rbug/librbug.la \
> +	$(GALLIUM_DRI_LIB_DEPS)
> +
> +nodist_EXTRA_swrast_dri2_dri_la_SOURCES = dummy.cpp
> +
> +if HAVE_MESA_LLVM
> +AM_CPPFLAGS += -DGALLIUM_LLVMPIPE
> +swrast_dri2_dri_la_LDFLAGS += $(LLVM_LDFLAGS)
> +swrast_dri2_dri_la_LIBADD += $(top_builddir)/src/gallium/drivers/llvmpipe/libllvmpipe.la $(LLVM_LIBS)
> +endif
> +
> +# Provide compatibility with scripts for the old Mesa build system for
> +# a while by putting a link to the driver into /lib of the build tree.
> +all-local: swrast_dri2_dri.la
> +	$(MKDIR_P) $(top_builddir)/$(LIB_DIR)/gallium
> +	ln -f .libs/swrast_dri2_dri.so $(top_builddir)/$(LIB_DIR)/gallium/swrast_dri2_dri.so
[snip]
> diff --git a/src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.h b/src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.h
> new file mode 100644
> index 0000000..e276a72
> --- /dev/null
> +++ b/src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.h
> @@ -0,0 +1,37 @@
> +/**************************************************************************
> + *
> + * Copyright 2009, VMware, Inc.
> + * All Rights Reserved.
> + * Copyright 2010 George Sapountzis <gsapountzis at gmail.com>
> + *           2013 Red Hat, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
> + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
> + * IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS BE LIABLE FOR
> + * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
> + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
> + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + **************************************************************************/
> +
Not a lawyer here, but adding three lines of copyright and a license for a
forward declaration and a function prototype is a bit of a overkill.

-Emil
> +#ifndef KMS_DRI_SW_WINSYS
> +#define KMS_DRI_SW_WINSYS
> +
> +struct sw_winsys;
> +
> +struct sw_winsys *kms_dri_create_winsys(int fd);
> +
> +#endif
> 