On 28 April 2015 at 01:02, Marek Olšák <mar...@gmail.com> wrote: > On Tue, Apr 21, 2015 at 5:12 PM, Emil Velikov <emil.l.veli...@gmail.com> > wrote: >> 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. > > I've moved it now and I'm in the middle of a rebase right now. > >> >> >> 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 ? > > Yes. > >> >>> 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. > > Done. > >> >>> 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. > > Addrlib is in C++. (in the next patch) > >> >>> + >>> +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. > > libdrm_amdgpu has only 2 public headers: amdgpu.h and amdgpu_drm.h. > They won't conflict. > >> >>> --- 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 ? > > Good idea. There was no particular reason other than it was easy to do. > Glad I could help :-)
-Emil _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev