On Wednesday, May 25, 2016 19:18 BST, robert.foss@xxxxxxxxxxxxx wrote:
From: Robert Foss <robert.foss@xxxxxxxxxxxxx>
Use the HAS_INTEL automake flag to avoid building benchmarks that won't
compile unless libdrm_intel is available in the build system.
Signed-off-by: Robert Foss <robert.foss@xxxxxxxxxxxxx>
---
benchmarks/Android.mk | 6 ++++++
benchmarks/Makefile.am | 5 ++++-
benchmarks/Makefile.sources | 14 +++++++++-----
3 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/benchmarks/Android.mk b/benchmarks/Android.mk
index 207a177..08b6816 100644
--- a/benchmarks/Android.mk
+++ b/benchmarks/Android.mk
@@ -34,4 +34,10 @@ endef
benchmark_list := $(benchmarks_PROGRAMS)
+ifeq ($(HAVE_LIBDRM_INTEL),true)
+ benchmark_list += $(LIBDRM_INTEL_BENCHMARKS)
+endif
+
+
+
Add just one blank line. Yet again... one should set HAVE_LIBDRM_INTEL
so that things don't regress/change. Something like the following
should do it
--- a/Android.mk
+++ b/Android.mk
@@ -1,2 +1,4 @@
+HAVE_LIBDRM_INTEL := true
+
include $(call all-named-subdir-makefiles, lib tests tools benchmarks)
I'd keep ^^ either as separate patch (1.1/7) or just fold it into 1/7
and update the commit message.
$(foreach item,$(benchmark_list),$(eval $(call
add_benchmark,$(item))))
diff --git a/benchmarks/Makefile.am b/benchmarks/Makefile.am
index 49d2f64..7400dd0 100644
--- a/benchmarks/Makefile.am
+++ b/benchmarks/Makefile.am
@@ -1,6 +1,9 @@
-
include Makefile.sources
+if HAVE_LIBDRM_INTEL
+ benchmarks_PROGRAMS += $(LIBDRM_INTEL_BENCHMARKS)
+endif
+
This is absolutely correct, sadly the existing Makefile.sources
(ab)use makes things hard to grasp.
The 'hard to grasp' part being - this (and other) Makefile.am files
are missing the definition of the *PROGRAMS, *LTLIBRARIES, etc.
variables thus the whole file (and this hunk and particular makes
things harder to read.
Ideally one will give clearer (non-autoconf specific) names of the
variables in the Makefile.sources and move the PROGRAMS... bits into
Makefile.am. That is rather evasive so I'm not sure if you're up for it.
Example (note it has some ~unrelated comments/nitpicks)
diff --git a/benchmarks/Android.mk b/benchmarks/Android.mk
index 207a177..dfe34bc 100644
--- a/benchmarks/Android.mk
+++ b/benchmarks/Android.mk
@@ -32,6 +32,6 @@ endef
#================#
-benchmark_list := $(benchmarks_PROGRAMS)
+benchmark_list := $(benchmarks_prog_list)
$(foreach item,$(benchmark_list),$(eval $(call add_benchmark,$(item))))
diff --git a/benchmarks/Makefile.am b/benchmarks/Makefile.am
index 2c2d100..5396db1 100644
--- a/benchmarks/Makefile.am
+++ b/benchmarks/Makefile.am
@@ -3,15 +3,23 @@ include Makefile.sources
AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/lib
AM_CFLAGS = $(DRM_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS)
$(LIBUNWIND_CFLAGS)
+
+benchmarks_PROGRAMS = $(benchmarks_prog_list)
LDADD = $(top_builddir)/lib/libintel_tools.la
+# Sees to be unused by IGT. Add a comment and move it after gem_foo
+benchmarksdir=$(libexecdir)/intel-gpu-tools/benchmarks
benchmarks_LTLIBRARIES = gem_exec_tracer.la
gem_exec_tracer_la_LDFLAGS = -module -avoid-version -no-undefined
+# there should be a detection in configure.ac as some platforms don't
use/have libdl.so
gem_exec_tracer_la_LIBADD = -ldl
+# one could/should use AX_PTHREADS in configure and
PTHREAD_CFLAGS/PTHREAD_LIBS through the project.
+# unless they use pthread and don't strictly require the locking.
then they can use the pthread-stubs package.
gem_latency_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
gem_latency_LDADD = $(LDADD) -lpthread
gem_syslatency_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
gem_syslatency_LDADD = $(LDADD) -lpthread -lrt
+# spaces around = ?
EXTRA_DIST=README
diff --git a/benchmarks/Makefile.sources b/benchmarks/Makefile.sources
index 81607a5..51f59e5 100644
--- a/benchmarks/Makefile.sources
+++ b/benchmarks/Makefile.sources
@@ -1,6 +1,4 @@
-benchmarksdir=$(libexecdir)/intel-gpu-tools/benchmarks
-
-benchmarks_PROGRAMS = \
+benchmarks_prog_list = \
intel_upload_blit_large \
intel_upload_blit_large_gtt \
intel_upload_blit_large_map \
--- a/benchmarks/Makefile.sources
+++ b/benchmarks/Makefile.sources
@@ -1,10 +1,7 @@
benchmarksdir=$(libexecdir)/intel-gpu-tools/benchmarks
+
Please don't add unneeded whitespace changes.
+LIBDRM_INTEL_BENCHMARKS = \
With the above suggestions in place this could become
intel_benchmarks_prog_list. Don't think there's a difference between
upper/lower case and/or short/long names. Go with what IGT people are
happy.
-Emil
