Hi Emil, I think I have fixed everything that you suggested. You can review the branch here:
http://cgit.freedesktop.org/~mareko/mesa/log/?h=amdgpu Thanks, Marek On Tue, Apr 28, 2015 at 11:01 AM, Emil Velikov <emil.l.veli...@gmail.com> wrote: > 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