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.
Marek
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
