[Mesa-dev] [PATCH 03/18] winsys/amdgpu: add a new winsys for the new kernel driver
Marek Olšák
maraeo at gmail.com
Tue Apr 28 07:28:36 PDT 2015
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.velikov at gmail.com> wrote:
> On 28 April 2015 at 01:02, Marek Olšák <maraeo at gmail.com> wrote:
>> On Tue, Apr 21, 2015 at 5:12 PM, Emil Velikov <emil.l.velikov at 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.olsak at 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 <olvaffe at 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
More information about the mesa-dev
mailing list