* Re: [RFC i-g-t 3/7] benchmarks/Makefile: Don't build benchmarks that depend on libdrm_intel. [not found] <2415-57460380-2b-6216b000@184007648> @ 2016-05-25 22:06 ` Robert Foss 2016-06-05 23:01 ` Robert Foss 0 siblings, 1 reply; 3+ messages in thread From: Robert Foss @ 2016-05-25 22:06 UTC (permalink / raw) To: marius.c.vlad, intel-gfx Forward to ML. On 2016-05-25 03:55 PM, Emil Velikov wrote: > On Wednesday, May 25, 2016 19:18 BST, robert.foss@collabora.com wrote: >> From: Robert Foss <robert.foss@collabora.com> >> >> 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@collabora.com> >> --- >> 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 > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFC i-g-t 3/7] benchmarks/Makefile: Don't build benchmarks that depend on libdrm_intel. 2016-05-25 22:06 ` [RFC i-g-t 3/7] benchmarks/Makefile: Don't build benchmarks that depend on libdrm_intel Robert Foss @ 2016-06-05 23:01 ` Robert Foss 0 siblings, 0 replies; 3+ messages in thread From: Robert Foss @ 2016-06-05 23:01 UTC (permalink / raw) To: marius.c.vlad, intel-gfx, Emil Velikov > On 2016-05-25 03:55 PM, Emil Velikov wrote: >> On Wednesday, May 25, 2016 19:18 BST, robert.foss@collabora.com wrote: >>> From: Robert Foss <robert.foss@collabora.com> >>> >>> 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@collabora.com> >>> --- >>> 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 About the LIBDRM_INTEL_BENCHMARKS -> changes (and the corresponding changes to tools/lib/etc.) this would change the Android.mk behavior too. The Android.mk changes would make it deviate from the the standard expected behaviour with having XXX_PROGRAMS being assumed to contain the program listing. Is that a desired consequence? >> > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 3+ messages in thread
* [RFC i-g-t 0/7] Remove compile time depencencies on libdrm_intel.
@ 2016-05-25 18:18 robert.foss
2016-05-25 18:18 ` [RFC i-g-t 3/7] benchmarks/Makefile: Don't build benchmarks that depend " robert.foss
0 siblings, 1 reply; 3+ messages in thread
From: robert.foss @ 2016-05-25 18:18 UTC (permalink / raw)
To: daniel.vetter, daniel.stone, marius.c.vlad, tomeu.vizoso,
emil.velikov, chris
Cc: intel-gfx
From: Robert Foss <robert.foss@collabora.com>
Changes since v1:
- Replaced the automake flags HAVE_VC4/NOUVEAU/INTEL with HAVE_LIBDRM_XXX.
- Move conditionals from Makefile.sources to Arduino.mk/Makefile.am.
- Removed duplicated i915_drm.h symbols from intel_drm_stubs.h.
- Replaced igt_require with igt_require_f to communicate stubs being the cause
of failure.
- Rename intel_drm_stubs to intel_bufmgr.
- Moved intel_bufmgr to lib/stubs/drm.
- Remove header inclusion changes in favor for inclusion of stubs in
lib/stubs/drm using build scripts.
- Rebased on trunk.
Robert Foss (7):
configure.ac: Test for libdrm_intel and build for it if present.
configure.ac: Harmonize HAVE_XXX flag for all drm platforms to
HAVE_LIBDRM_XXX.
benchmarks/Makefile: Don't build benchmarks that depend on
libdrm_intel.
tools/Makefile: Don't build tools that depend on libdrm_intel.
tools/Makefile: Format whitespace.
demos/Makefile: Don't build tools that depend on libdrm_intel.
lib/stubs: Add stubs for intel_bufmgr.
benchmarks/Android.mk | 6 +
benchmarks/Makefile.am | 5 +-
benchmarks/Makefile.sources | 14 +-
configure.ac | 22 ++-
demos/Android.mk | 5 +-
demos/Makefile.am | 8 +-
demos/Makefile.sources | 7 +
lib/Makefile.am | 9 +-
lib/stubs/drm/intel_bufmgr.c | 275 +++++++++++++++++++++++++++
lib/stubs/drm/intel_bufmgr.h | 430 +++++++++++++++++++++++++++++++++++++++++++
tests/Makefile.am | 4 +-
tools/Android.mk | 8 +
tools/Makefile.am | 8 +
tools/Makefile.sources | 73 ++++----
14 files changed, 818 insertions(+), 56 deletions(-)
create mode 100644 demos/Makefile.sources
create mode 100644 lib/stubs/drm/intel_bufmgr.c
create mode 100644 lib/stubs/drm/intel_bufmgr.h
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 3+ messages in thread* [RFC i-g-t 3/7] benchmarks/Makefile: Don't build benchmarks that depend on libdrm_intel. 2016-05-25 18:18 [RFC i-g-t 0/7] Remove compile time depencencies " robert.foss @ 2016-05-25 18:18 ` robert.foss 0 siblings, 0 replies; 3+ messages in thread From: robert.foss @ 2016-05-25 18:18 UTC (permalink / raw) To: daniel.vetter, daniel.stone, marius.c.vlad, tomeu.vizoso, emil.velikov, chris Cc: intel-gfx From: Robert Foss <robert.foss@collabora.com> 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@collabora.com> --- 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 + + + $(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 + AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/lib AM_CFLAGS = $(DRM_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS) $(LIBUNWIND_CFLAGS) \ $(WERROR_CFLAGS) diff --git a/benchmarks/Makefile.sources b/benchmarks/Makefile.sources index bc4f2b5..a2742f3 100644 --- a/benchmarks/Makefile.sources +++ b/benchmarks/Makefile.sources @@ -1,10 +1,7 @@ benchmarksdir=$(libexecdir)/intel-gpu-tools/benchmarks + benchmarks_PROGRAMS = \ - intel_upload_blit_large \ - intel_upload_blit_large_gtt \ - intel_upload_blit_large_map \ - intel_upload_blit_small \ gem_blt \ gem_create \ gem_exec_ctx \ @@ -17,6 +14,13 @@ benchmarks_PROGRAMS = \ gem_prw \ gem_set_domain \ gem_syslatency \ - gem_userptr_benchmark \ kms_vblank \ $(NULL) + +LIBDRM_INTEL_BENCHMARKS = \ + intel_upload_blit_large \ + intel_upload_blit_large_gtt \ + intel_upload_blit_large_map \ + intel_upload_blit_small \ + gem_userptr_benchmark \ + $(NULL) -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-06-05 23:01 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <2415-57460380-2b-6216b000@184007648>
2016-05-25 22:06 ` [RFC i-g-t 3/7] benchmarks/Makefile: Don't build benchmarks that depend on libdrm_intel Robert Foss
2016-06-05 23:01 ` Robert Foss
2016-05-25 18:18 [RFC i-g-t 0/7] Remove compile time depencencies " robert.foss
2016-05-25 18:18 ` [RFC i-g-t 3/7] benchmarks/Makefile: Don't build benchmarks that depend " robert.foss
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).