On Wed, 03 Apr 2019, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote:
> Quoting Jani Nikula (2019-04-03 14:32:36)
>> The below commits added dummy files to test that certain headers are
>> self-contained, i.e. compilable as standalone units:
>>
>> 39e2f501c1b4 ("drm/i915: Split struct intel_context definition to its own header")
>> 3a891a626794 ("drm/i915: Move intel_engine_mask_t around for use by i915_request_types.h")
>> 8b74594aa455 ("drm/i915: Split out i915_priolist_types into its own header")
>>
>> The idea is fine, but the implementation is a bit tedious and
>> inflexible, and does not really scale well.
>>
>> Implement the same in make using autogenerated dummy sources to include
>> the headers.
>>
>> v2 by Chris:
>> - Use patsubst
>> - Add .gitignore
>> - Add clean-files for generated dummy sources
>>
>> v3 by Jani:
>> - Fix make clean
>> - Add the tests to i915-y instead of extra-y
>>
>> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
>> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/.gitignore | 1 +
>> drivers/gpu/drm/i915/Makefile | 16 ++++---------
>> drivers/gpu/drm/i915/Makefile.header-test | 23 +++++++++++++++++++
>> .../i915/test_i915_active_types_standalone.c | 7 ------
>> .../test_i915_gem_context_types_standalone.c | 7 ------
>> .../test_i915_priolist_types_standalone.c | 7 ------
>> .../test_i915_scheduler_types_standalone.c | 7 ------
>> .../test_i915_timeline_types_standalone.c | 7 ------
>> .../test_intel_context_types_standalone.c | 7 ------
>> .../i915/test_intel_engine_types_standalone.c | 7 ------
>> .../test_intel_workarounds_types_standalone.c | 7 ------
>> 11 files changed, 28 insertions(+), 68 deletions(-)
>> create mode 100644 drivers/gpu/drm/i915/.gitignore
>> create mode 100644 drivers/gpu/drm/i915/Makefile.header-test
>> delete mode 100644 drivers/gpu/drm/i915/test_i915_active_types_standalone.c
>> delete mode 100644 drivers/gpu/drm/i915/test_i915_gem_context_types_standalone.c
>> delete mode 100644 drivers/gpu/drm/i915/test_i915_priolist_types_standalone.c
>> delete mode 100644 drivers/gpu/drm/i915/test_i915_scheduler_types_standalone.c
>> delete mode 100644 drivers/gpu/drm/i915/test_i915_timeline_types_standalone.c
>> delete mode 100644 drivers/gpu/drm/i915/test_intel_context_types_standalone.c
>> delete mode 100644 drivers/gpu/drm/i915/test_intel_engine_types_standalone.c
>> delete mode 100644 drivers/gpu/drm/i915/test_intel_workarounds_types_standalone.c
>>
>> diff --git a/drivers/gpu/drm/i915/.gitignore b/drivers/gpu/drm/i915/.gitignore
>> new file mode 100644
>> index 000000..cff45d
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/.gitignore
>> @@ -0,0 +1 @@
>> +header_test_*.c
>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>> index 30bf33..fbcb0904 100644
>> --- a/drivers/gpu/drm/i915/Makefile
>> +++ b/drivers/gpu/drm/i915/Makefile
>> @@ -32,10 +32,13 @@ CFLAGS_intel_fbdev.o = $(call cc-disable-warning, override-init)
>> subdir-ccflags-y += \
>> $(call as-instr,movntdqa (%eax)$(comma)%xmm0,-DCONFIG_AS_MOVNTDQA)
>>
>> +# Extra header tests
>> +include $(src)/Makefile.header-test
>> +
>> # Please keep these build lists sorted!
>>
>> # core driver code
>> -i915-y := i915_drv.o \
>> +i915-y += i915_drv.o \
>> i915_irq.o \
>> i915_memcpy.o \
>> i915_mm.o \
>> @@ -57,17 +60,6 @@ i915-$(CONFIG_COMPAT) += i915_ioc32.o
>> i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o intel_pipe_crc.o
>> i915-$(CONFIG_PERF_EVENTS) += i915_pmu.o
>>
>> -# Test the headers are compilable as standalone units
>> -i915-$(CONFIG_DRM_I915_WERROR) += \
>> - test_i915_active_types_standalone.o \
>> - test_i915_gem_context_types_standalone.o \
>> - test_i915_priolist_types_standalone.o \
>> - test_i915_scheduler_types_standalone.o \
>> - test_i915_timeline_types_standalone.o \
>> - test_intel_context_types_standalone.o \
>> - test_intel_engine_types_standalone.o \
>> - test_intel_workarounds_types_standalone.o
>> -
>> # GEM code
>> i915-y += \
>> i915_active.o \
>> diff --git a/drivers/gpu/drm/i915/Makefile.header-test b/drivers/gpu/drm/i915/Makefile.header-test
>> new file mode 100644
>> index 000000..e984cf
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/Makefile.header-test
>> @@ -0,0 +1,23 @@
>> +# SPDX-License-Identifier: MIT
>> +# Copyright © 2019 Intel Corporation
>> +
>> +# Test the headers are compilable as standalone units
>> +header_test := \
>> + i915_active_types.h \
>> + i915_gem_context_types.h \
>> + i915_priolist_types.h \
>> + i915_scheduler_types.h \
>> + i915_timeline_types.h \
>> + intel_context_types.h \
>> + intel_engine_types.h \
>> + intel_workarounds_types.h
>> +
>> +quiet_cmd_header_test = HDRTEST $@
>
> Did you look at the output alignment? v1 needed a bit of a tweak to look
> good here.
Argh. I didn't actually *look* at it. v4 on its way.
>
>> + cmd_header_test = echo "\#include \"$(<F)\"" > $@
>> +
>> +header_test_%.c: %.h
>> + $(call cmd,header_test)
>> +
>> +i915-$(CONFIG_DRM_I915_WERROR) += $(foreach h,$(header_test),$(patsubst %.h,header_test_%.o,$(h)))
>> +
>> +clean-files += $(foreach h,$(header_test),$(patsubst %.h,header_test_%.c,$(h)))
>
> Magic. Pure magic. Much thanks.
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> #v2 if you insist
> Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Thanks. Now off to making more headers self-contained!
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
