[Mesa-dev] [PATCH 03/18] winsys/amdgpu: add a new winsys for the new kernel driver
Emil Velikov
emil.l.velikov at gmail.com
Tue Apr 28 02:01:05 PDT 2015
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