[Mesa-dev] [PATCH 1/1] automake: r600 should only depend on libamd_common if opencl is enabled

Fri Jun 2 15:46:13 UTC 2017

On Thu, 2017-06-01 at 16:22 -0500, Aaron Watry wrote:
> On Thu, Jun 1, 2017 at 3:28 PM, Jan Vesely <jan.vesely at rutgers.edu> wrote:
> 
> > Signed-off-by: Jan Vesely <jan.vesely at rutgers.edu>
> > ---
> > Hi guys,
> > 
> > this is the first step towards dropping libamd_common dependency.
> > It's based on Emil's patches 3/5 and 4/5.
> > Enabling opencl still falls back to the old way of requiring libamd_common.
> > I'll try to address that in the next step (no time estimate, feel free to
> > beat me to it). I think we can drop part of those functions rather than
> > just copying them.
> > 
> 
> I did a build test of r600 on my laptop with an old libdrm before/after
> this patch with/without opencl enabled, and got:
> before, no CL: Build Failure
> after, no CL: Build succeeded
> before, with CL:  Build Failure
> after, with CL: Build Failure
> 
> I then went and updated libdrm/amdgpu, and the with/without CL build for
> r600 works again there.  I then built this on my r600 box and
> double-checked that clinfo and piglit's cl-program-tester ran and could
> execute a simple kernel (tests/cl/program/execute/builtin/geometric/
> length.cl), so I'd say that this is:
> Tested-By: Aaron Watry <awatry at gmail.com>

thanks. I also tested building r600g using --disable-llvm which builds
OK (not sure if that should be allowed since r300g config fails without
llvm)

> 
> And yes, I agree that we should remove the amdgpu dependency from
> libamd_common (or at least the parts that r600 depends on), but I'd be ok
> with doing that in a follow-up... That, or we also extend vinson's patch
> [0] and wrap it in a check for if opencl is enabled so that we get a
> configure-time failure if R600 is being built with CL and amdgpu's drm
> headers are missing/old.

I think that's a question for AMD devs. my current understanding is
that amdgpu_common is more like
amdgcn_shared_between_radv_and_radeonsi, yet both r300 and r600 use at
least the gpu_info header. I've found only two functions in ac_binary
that are needed by r600 clover, so I plan to copy them. having
common_amdgpu_common to be used by r300/r600/radeonsi/radv would be
nicer, but I'm not sure how much code would fall into that category.

Jan

> 
> --Aaron
> 
> [0] https://lists.freedesktop.org/archives/mesa-dev/2017-May/157327.html
> 
> 
> > 
> > Emil, I didn't find anything in android build that enabled opencl, so I
> > dropped it entirely.
> > 
> > regards,
> > Jan
> > 
> >  configure.ac                                | 3 ++-
> >  src/gallium/drivers/r600/Android.mk         | 5 -----
> >  src/gallium/drivers/r600/Automake.inc       | 2 +-
> >  src/gallium/targets/pipe-loader/Makefile.am | 2 +-
> >  4 files changed, 4 insertions(+), 8 deletions(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index 5caf316..e0996a0 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -2631,7 +2631,8 @@ AM_CONDITIONAL(HAVE_SWRAST_DRI, test
> > x$HAVE_SWRAST_DRI = xyes)
> >  AM_CONDITIONAL(HAVE_RADEON_VULKAN, test "x$HAVE_RADEON_VULKAN" = xyes)
> >  AM_CONDITIONAL(HAVE_INTEL_VULKAN, test "x$HAVE_INTEL_VULKAN" = xyes)
> > 
> > -AM_CONDITIONAL(HAVE_AMD_DRIVERS, test "x$HAVE_GALLIUM_R600" = xyes -o \
> > +AM_CONDITIONAL(HAVE_AMD_DRIVERS, test \( "x$HAVE_GALLIUM_R600" = xyes -a
> > \
> > +                                      "x$enable_opencl" = xyes \) -o \
> >                                        "x$HAVE_GALLIUM_RADEONSI" = xyes -o
> > \
> >                                        "x$HAVE_RADEON_VULKAN" = xyes)
> > 
> > diff --git a/src/gallium/drivers/r600/Android.mk
> > b/src/gallium/drivers/r600/Android.mk
> > index 87f433d..18c5bb6 100644
> > --- a/src/gallium/drivers/r600/Android.mk
> > +++ b/src/gallium/drivers/r600/Android.mk
> > @@ -30,11 +30,7 @@ include $(CLEAR_VARS)
> > 
> >  LOCAL_SRC_FILES := $(C_SOURCES) $(CXX_SOURCES)
> > 
> > -ifeq ($(MESA_ENABLE_LLVM),true)
> > -LOCAL_STATIC_LIBRARIES := libmesa_amd_common
> > -else
> >  LOCAL_C_INCLUDES += $(MESA_TOP)/src/amd/common
> > -endif
> > 
> >  LOCAL_SHARED_LIBRARIES := libdrm_radeon
> >  LOCAL_MODULE := libmesa_pipe_r600
> > @@ -45,7 +41,6 @@ include $(BUILD_STATIC_LIBRARY)
> >  ifneq ($(HAVE_GALLIUM_R600),)
> >  $(eval GALLIUM_LIBS += \
> >         $(LOCAL_MODULE) \
> > -       $(LOCAL_STATIC_LIBRARIES) \
> >         libmesa_winsys_radeon)
> >  $(eval GALLIUM_SHARED_LIBS += $(LOCAL_SHARED_LIBRARIES))
> >  endif
> > diff --git a/src/gallium/drivers/r600/Automake.inc
> > b/src/gallium/drivers/r600/Automake.inc
> > index fa45735..c96fe74 100644
> > --- a/src/gallium/drivers/r600/Automake.inc
> > +++ b/src/gallium/drivers/r600/Automake.inc
> > @@ -13,7 +13,7 @@ TARGET_RADEON_WINSYS = \
> >  TARGET_RADEON_COMMON = \
> >         $(top_builddir)/src/gallium/drivers/radeon/libradeon.la
> > 
> > -if HAVE_GALLIUM_LLVM
> > +if HAVE_AMD_DRIVERS
> >  TARGET_RADEON_COMMON += \
> >         $(top_builddir)/src/amd/common/libamd_common.la
> >  endif
> > diff --git a/src/gallium/targets/pipe-loader/Makefile.am
> > b/src/gallium/targets/pipe-loader/Makefile.am
> > index 5f629a2..c991533 100644
> > --- a/src/gallium/targets/pipe-loader/Makefile.am
> > +++ b/src/gallium/targets/pipe-loader/Makefile.am
> > @@ -131,7 +131,7 @@ pipe_r600_la_LIBADD = \
> >         $(LIBDRM_LIBS) \
> >         $(RADEON_LIBS)
> > 
> > -if HAVE_GALLIUM_LLVM
> > +if HAVE_AMD_DRIVERS
> >  pipe_r600_la_LIBADD += \
> >         $(top_builddir)/src/amd/common/libamd_common.la
> >  endif
> > --
> > 2.9.4
> > 
> > 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170602/f9c78bd1/attachment.sig>