Hi Marek,
Must admit that the current "split"/location of the new winsys looks a
bit strange. I'm thinking that if one places the new winsys alongside
the radeon one (i.e. winsys/amdgpu/drm) things should still work and
thus we'll result in shorter and cleaner patch. See below for more details.
On 20/04/15 22:23, Marek Olšák wrote:
> From: Marek Olšák <marek.ol...@amd.com>
>
> ---
> configure.ac | 5 +
> src/gallium/Makefile.am | 1 +
> src/gallium/drivers/r300/Automake.inc | 6 +-
> src/gallium/drivers/r600/Automake.inc | 6 +-
> src/gallium/drivers/radeonsi/Automake.inc | 6 +-
> src/gallium/targets/pipe-loader/Makefile.am | 12 +-
> src/gallium/winsys/radeon/amdgpu/Android.mk | 40 ++
> src/gallium/winsys/radeon/amdgpu/Makefile.am | 12 +
> src/gallium/winsys/radeon/amdgpu/Makefile.sources | 8 +
> src/gallium/winsys/radeon/amdgpu/amdgpu_bo.c | 643
> ++++++++++++++++++++++
> src/gallium/winsys/radeon/amdgpu/amdgpu_bo.h | 75 +++
> src/gallium/winsys/radeon/amdgpu/amdgpu_cs.c | 578 +++++++++++++++++++
> src/gallium/winsys/radeon/amdgpu/amdgpu_cs.h | 149 +++++
> src/gallium/winsys/radeon/amdgpu/amdgpu_public.h | 14 +
> src/gallium/winsys/radeon/amdgpu/amdgpu_winsys.c | 491 +++++++++++++++++
> src/gallium/winsys/radeon/amdgpu/amdgpu_winsys.h | 80 +++
> src/gallium/winsys/radeon/drm/radeon_drm_winsys.c | 8 +
> src/gallium/winsys/radeon/radeon_winsys.h | 4 +
> 18 files changed, 2129 insertions(+), 9 deletions(-)
> create mode 100644 src/gallium/winsys/radeon/amdgpu/Android.mk
> create mode 100644 src/gallium/winsys/radeon/amdgpu/Makefile.am
> create mode 100644 src/gallium/winsys/radeon/amdgpu/Makefile.sources
> create mode 100644 src/gallium/winsys/radeon/amdgpu/amdgpu_bo.c
> create mode 100644 src/gallium/winsys/radeon/amdgpu/amdgpu_bo.h
> create mode 100644 src/gallium/winsys/radeon/amdgpu/amdgpu_cs.c
> create mode 100644 src/gallium/winsys/radeon/amdgpu/amdgpu_cs.h
> create mode 100644 src/gallium/winsys/radeon/amdgpu/amdgpu_public.h
> create mode 100644 src/gallium/winsys/radeon/amdgpu/amdgpu_winsys.c
> create mode 100644 src/gallium/winsys/radeon/amdgpu/amdgpu_winsys.h
>
> diff --git a/configure.ac b/configure.ac
> index 095e23e..f22975f 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -68,6 +68,7 @@ AC_SUBST([OSMESA_VERSION])
> dnl Versions for external dependencies
> LIBDRM_REQUIRED=2.4.38
> LIBDRM_RADEON_REQUIRED=2.4.56
> +LIBDRM_AMDGPU_REQUIRED=2.4.60
I guess this will need to be changed once the libdrm patches land ?
> LIBDRM_INTEL_REQUIRED=2.4.60
> LIBDRM_NVVIEUX_REQUIRED=2.4.33
> LIBDRM_NOUVEAU_REQUIRED="2.4.33 libdrm >= 2.4.41"
> @@ -2091,6 +2092,7 @@ if test -n "$with_gallium_drivers"; then
> xr300)
> HAVE_GALLIUM_R300=yes
> PKG_CHECK_MODULES([RADEON], [libdrm_radeon >=
> $LIBDRM_RADEON_REQUIRED])
> + PKG_CHECK_MODULES([AMDGPU], [libdrm_amdgpu >=
> $LIBDRM_AMDGPU_REQUIRED])
> gallium_require_drm "Gallium R300"
> gallium_require_drm_loader
> gallium_require_llvm "Gallium R300"
> @@ -2098,6 +2100,7 @@ if test -n "$with_gallium_drivers"; then
> xr600)
> HAVE_GALLIUM_R600=yes
> PKG_CHECK_MODULES([RADEON], [libdrm_radeon >=
> $LIBDRM_RADEON_REQUIRED])
> + PKG_CHECK_MODULES([AMDGPU], [libdrm_amdgpu >=
> $LIBDRM_AMDGPU_REQUIRED])
We can drop the above two hunks.
> gallium_require_drm "Gallium R600"
> gallium_require_drm_loader
> if test "x$enable_r600_llvm" = xyes -o "x$enable_opencl" = xyes;
> then
> @@ -2114,6 +2117,7 @@ if test -n "$with_gallium_drivers"; then
> xradeonsi)
> HAVE_GALLIUM_RADEONSI=yes
> PKG_CHECK_MODULES([RADEON], [libdrm_radeon >=
> $LIBDRM_RADEON_REQUIRED])
> + PKG_CHECK_MODULES([AMDGPU], [libdrm_amdgpu >=
> $LIBDRM_AMDGPU_REQUIRED])
> gallium_require_drm "radeonsi"
> gallium_require_drm_loader
> radeon_llvm_check "radeonsi"
> @@ -2384,6 +2388,7 @@ AC_CONFIG_FILES([Makefile
> src/gallium/winsys/intel/drm/Makefile
> src/gallium/winsys/nouveau/drm/Makefile
> src/gallium/winsys/radeon/drm/Makefile
> + src/gallium/winsys/radeon/amdgpu/Makefile
> src/gallium/winsys/svga/drm/Makefile
> src/gallium/winsys/sw/dri/Makefile
> src/gallium/winsys/sw/kms-dri/Makefile
> diff --git a/src/gallium/Makefile.am b/src/gallium/Makefile.am
> index ede6e21..fa526d4 100644
> --- a/src/gallium/Makefile.am
> +++ b/src/gallium/Makefile.am
> @@ -63,6 +63,7 @@ endif
> ## the radeon winsys - linked in by r300, r600 and radeonsi
> if NEED_RADEON_DRM_WINSYS
> SUBDIRS += winsys/radeon/drm
> +SUBDIRS += winsys/radeon/amdgpu
Move it to the if HAVE_GALLIUM_RADEONSI block ? It is the only driver
that can use the new winsys.
> endif
>
> ## swrast/softpipe
> diff --git a/src/gallium/drivers/r300/Automake.inc
> b/src/gallium/drivers/r300/Automake.inc
> index 9334973..cfcd61c 100644
> --- a/src/gallium/drivers/r300/Automake.inc
> +++ b/src/gallium/drivers/r300/Automake.inc
> @@ -5,9 +5,11 @@ TARGET_CPPFLAGS += -DGALLIUM_R300
> TARGET_LIB_DEPS += \
> $(top_builddir)/src/gallium/drivers/r300/libr300.la \
> $(RADEON_LIBS) \
> - $(INTEL_LIBS)
> + $(LIBDRM_LIBS) \
> + $(AMDGPU_LIBS)
>
> TARGET_RADEON_WINSYS = \
> - $(top_builddir)/src/gallium/winsys/radeon/drm/libradeonwinsys.la
> + $(top_builddir)/src/gallium/winsys/radeon/drm/libradeonwinsys.la \
> + $(top_builddir)/src/gallium/winsys/radeon/amdgpu/libamdgpuwinsys.la
>
> endif
> diff --git a/src/gallium/drivers/r600/Automake.inc
> b/src/gallium/drivers/r600/Automake.inc
> index 914eea3..2bb34b0 100644
> --- a/src/gallium/drivers/r600/Automake.inc
> +++ b/src/gallium/drivers/r600/Automake.inc
> @@ -5,10 +5,12 @@ TARGET_CPPFLAGS += -DGALLIUM_R600
> TARGET_LIB_DEPS += \
> $(top_builddir)/src/gallium/drivers/r600/libr600.la \
> $(RADEON_LIBS) \
> - $(LIBDRM_LIBS)
> + $(LIBDRM_LIBS) \
> + $(AMDGPU_LIBS)
>
> TARGET_RADEON_WINSYS = \
> - $(top_builddir)/src/gallium/winsys/radeon/drm/libradeonwinsys.la
> + $(top_builddir)/src/gallium/winsys/radeon/drm/libradeonwinsys.la \
> + $(top_builddir)/src/gallium/winsys/radeon/amdgpu/libamdgpuwinsys.la
>
The above r300/r600 changes can be dropped.
> TARGET_RADEON_COMMON = \
> $(top_builddir)/src/gallium/drivers/radeon/libradeon.la
> diff --git a/src/gallium/drivers/radeonsi/Automake.inc
> b/src/gallium/drivers/radeonsi/Automake.inc
> index 8686fff..200a254 100644
> --- a/src/gallium/drivers/radeonsi/Automake.inc
> +++ b/src/gallium/drivers/radeonsi/Automake.inc
> @@ -5,10 +5,12 @@ TARGET_CPPFLAGS += -DGALLIUM_RADEONSI
> TARGET_LIB_DEPS += \
> $(top_builddir)/src/gallium/drivers/radeonsi/libradeonsi.la \
> $(RADEON_LIBS) \
> - $(LIBDRM_LIBS)
> + $(LIBDRM_LIBS) \
> + $(AMDGPU_LIBS)
>
> TARGET_RADEON_WINSYS = \
> - $(top_builddir)/src/gallium/winsys/radeon/drm/libradeonwinsys.la
> + $(top_builddir)/src/gallium/winsys/radeon/drm/libradeonwinsys.la \
> + $(top_builddir)/src/gallium/winsys/radeon/amdgpu/libamdgpuwinsys.la
>
> TARGET_RADEON_COMMON = \
> $(top_builddir)/src/gallium/drivers/radeon/libradeon.la
> diff --git a/src/gallium/targets/pipe-loader/Makefile.am
> b/src/gallium/targets/pipe-loader/Makefile.am
> index 967cdb7..3527090 100644
> --- a/src/gallium/targets/pipe-loader/Makefile.am
> +++ b/src/gallium/targets/pipe-loader/Makefile.am
> @@ -124,9 +124,11 @@ nodist_EXTRA_pipe_r300_la_SOURCES = dummy.cpp
> pipe_r300_la_LIBADD = \
> $(PIPE_LIBS) \
> $(top_builddir)/src/gallium/winsys/radeon/drm/libradeonwinsys.la \
> + $(top_builddir)/src/gallium/winsys/radeon/amdgpu/libamdgpuwinsys.la \
> $(top_builddir)/src/gallium/drivers/r300/libr300.la \
> $(LIBDRM_LIBS) \
> - $(RADEON_LIBS)
> + $(RADEON_LIBS) \
> + $(AMDGPU_LIBS)
>
> endif
>
> @@ -138,10 +140,12 @@ nodist_EXTRA_pipe_r600_la_SOURCES = dummy.cpp
> pipe_r600_la_LIBADD = \
> $(PIPE_LIBS) \
> $(top_builddir)/src/gallium/winsys/radeon/drm/libradeonwinsys.la \
> + $(top_builddir)/src/gallium/winsys/radeon/amdgpu/libamdgpuwinsys.la \
> $(top_builddir)/src/gallium/drivers/radeon/libradeon.la \
> $(top_builddir)/src/gallium/drivers/r600/libr600.la \
> $(LIBDRM_LIBS) \
> - $(RADEON_LIBS)
> + $(RADEON_LIBS) \
> + $(AMDGPU_LIBS)
>
Ditto.
> endif
>
> @@ -153,10 +157,12 @@ nodist_EXTRA_pipe_radeonsi_la_SOURCES = dummy.cpp
> pipe_radeonsi_la_LIBADD = \
> $(PIPE_LIBS) \
> $(top_builddir)/src/gallium/winsys/radeon/drm/libradeonwinsys.la \
> + $(top_builddir)/src/gallium/winsys/radeon/amdgpu/libamdgpuwinsys.la \
> $(top_builddir)/src/gallium/drivers/radeon/libradeon.la \
> $(top_builddir)/src/gallium/drivers/radeonsi/libradeonsi.la \
> $(LIBDRM_LIBS) \
> - $(RADEON_LIBS)
> + $(RADEON_LIBS) \
> + $(AMDGPU_LIBS)
>
> endif
>
> diff --git a/src/gallium/winsys/radeon/amdgpu/Android.mk
> b/src/gallium/winsys/radeon/amdgpu/Android.mk
> new file mode 100644
> index 0000000..a10312f
> --- /dev/null
> +++ b/src/gallium/winsys/radeon/amdgpu/Android.mk
> @@ -0,0 +1,40 @@
> +# Mesa 3-D graphics library
> +#
> +# Copyright (C) 2011 Chia-I Wu <olva...@gmail.com>
> +# Copyright (C) 2011 LunarG 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, 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 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.
> +
> +LOCAL_PATH := $(call my-dir)
> +
> +# get C_SOURCES
> +include $(LOCAL_PATH)/Makefile.sources
> +
> +include $(CLEAR_VARS)
> +
> +LOCAL_SRC_FILES := $(C_SOURCES)
> +
> +LOCAL_C_INCLUDES := \
> + $(DRM_TOP) \
> + $(DRM_TOP)/include/drm
> +
You might want to resync with the latest winsys/radeon/drm/Android.mk. I
have some further changes which I'll push shortly.
You might want to add the new subdir in the src/gallium/Android.mk file.
Something like the following just just work
ifneq ($(filter radeonsi, $(MESA_GPU_DRIVERS)),)
SUBDIRS += drivers/radeonsi
+SUBDIRS += winsys/amdgpu/drm
endif
> --- /dev/null
> +++ b/src/gallium/winsys/radeon/amdgpu/Makefile.am
> @@ -0,0 +1,12 @@
> +include Makefile.sources
> +include $(top_srcdir)/src/gallium/Automake.inc
> +
> +AM_CFLAGS = \
> + $(GALLIUM_WINSYS_CFLAGS) \
> + $(AMDGPU_CFLAGS)
> +
> +AM_CXXFLAGS = $(AM_CFLAGS)
There are no C++ files so we can drop this.
> +
> +noinst_LTLIBRARIES = libamdgpuwinsys.la
> +
> +libamdgpuwinsys_la_SOURCES = $(C_SOURCES)
> diff --git a/src/gallium/winsys/radeon/amdgpu/Makefile.sources
> b/src/gallium/winsys/radeon/amdgpu/Makefile.sources
> new file mode 100644
> index 0000000..0f55010
> --- /dev/null
> +++ b/src/gallium/winsys/radeon/amdgpu/Makefile.sources
> @@ -0,0 +1,8 @@
> +C_SOURCES := \
> + amdgpu_bo.c \
> + amdgpu_bo.h \
> + amdgpu_cs.c \
> + amdgpu_cs.h \
> + amdgpu_public.h \
> + amdgpu_winsys.c \
> + amdgpu_winsys.h
Thank you for adding the headers in here. Seems that the radeon winsys
has an explicit "radeon_drm" prefix, while none of these do. Any
particular reason behind the divergence, won't it cause problems with
the headers in libdrm_amdgpu ? If the new naming scheme is prefered, one
should update the header include guards.
> --- a/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> +++ b/src/gallium/winsys/radeon/drm/radeon_drm_winsys.c
> @@ -34,6 +34,7 @@
> #include "radeon_drm_bo.h"
> #include "radeon_drm_cs.h"
> #include "radeon_drm_public.h"
> +#include "../amdgpu/amdgpu_public.h"
>
> #include "pipebuffer/pb_bufmgr.h"
> #include "util/u_memory.h"
> @@ -643,6 +644,13 @@ PUBLIC struct radeon_winsys *
> radeon_drm_winsys_create(int fd, radeon_screen_create_t screen_create)
> {
> struct radeon_drm_winsys *ws;
> + struct radeon_winsys *amdgpu;
> +
> + /* First, try amdgpu. */
> + amdgpu = amdgpu_winsys_create(fd, screen_create);
> + if (amdgpu) {
> + return amdgpu;
> + }
>
If we move this into the pipe-loader/inline drm helper we can avoid
pulling in winsys/amdgpu into the r300/r600 drivers. Is there any
particular reason behind the current approach ?
We'll need to add the new symbol to
targets/{dri-vdpau.sym,dri/dri.sym,vdpau/vdpau.sym} for dri-vdpau
interop to work.
I admit neither one of the pipe-loader/inline drm helpers is
particularly pretty atm, although I hope to get to cleaning it up soon.
Cheers
Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
